fix(cli): make i18n extract --check's regenerate hint the invocation it was given - #16470
Conversation
…n it was given
`os i18n extract --check` printed a "Regenerate and commit" command assembled at
the print site from four things that happened to be in scope: the config
argument, the emitted locales minus the default one, `--fill` and `--out`. Every
other flag the operator passed was not in the expression, so it was not in the
advice. Driven on the reported invocation against a stack whose
`i18n.defaultLocale` is `zh-CN`:
$ os i18n extract objectstack.config.ts --locales=zh-CN --no-metadata-forms
--no-objects-only --filter=kpi_ --out=OUT --check
missing: ../../../../../tmp/os-i18n-repro-jNrZ/zh-CN.objects.generated.ts
Translation bundles have drifted from the schema. Regenerate and commit:
os i18n extract objectstack.config.ts --locales= --fill=empty --out=OUT
Three defects, each measured before the repair:
* `--locales=` came out EMPTY. The echo drops the default locale on the
grounds that `--locales` always re-adds it, and here the only locale asked
for WAS the default one. The `Skeleton summary` two lines up names `zh-CN`,
so one code path had the locale and the other did not.
* `--no-metadata-forms`, `--no-objects-only` and `--filter=kpi_` were never
candidates for the line. Running what it printed wrote 775 keys across two
files where the operator's own command writes 2 across one — including a
`metadata-forms` companion they had explicitly switched off.
* `missing:` printed a `../../../../../…` walk out of the cwd for a directory
the operator had just typed in full.
The second one is what it costs. A failing `--check` is self-healable in one
step; following the printed advice emitted a different key set, so the next
`--check` failed AGAIN — on `out of date:` rather than `missing:` — and printed
the same wrong command. Both halves driven end to end.
The hint is now a DELETION, never an assembly: this run's own argv (`this.argv`,
verified to hold the arguments as typed with the command id stripped) with the
`--check` token removed, shell-quoted so it survives a copy, and `--` honoured
so a positional `--check` is left alone. Nothing enumerates flags, so a flag
added to this command later is echoed without anyone remembering this print
site — which is the property the assembled expression could not have. When
`--check` is not in the argv the command cannot point at what it removed and
prints "re-run the same command without `--check`" instead, the degraded line
the report asked for; today's flag surface has no other way to set `--check`
(no `env`, no default, no `allowNo`), so that branch is defence rather than a
reachable path, and it is what keeps an approximation from ever becoming the
fallback.
Diagnostic paths are printed absolute once the cwd cannot reach them downwards,
and keep the short relative form otherwise. That covers `Wrote` as well as
`missing:` / `out of date:`: it is the same `path.relative(process.cwd(), file)`
expression, in the same function, with the same symptom — the repro's write leg
printed `Wrote ../../../../../tmp/os-i18n-repro-jNrZ/…` — and repairing the
`--check` half alone would have left the two lines disagreeing. All nine of this
repo's extract configs write in-tree and their output is unchanged.
The pin drives the real CLI. Its central case asserts the WHOLE token list
rather than probing for `--filter` and `--no-objects-only` by name, because a
name-probing pin inherits exactly the blind spot being closed: it would be green
for the next flag added. Its second case is the loop itself — the printed line
is executed VERBATIM through `sh -c`, with an `os` shim on `PATH` in front of
the source entry point, and the original `--check` must then pass. No
approximate command can satisfy that one, and quoting is exercised for real by a
case whose `--filter` value carries a `|`.
Fixes #14895
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
…8n-extract-check-hint
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5ee979735a09d96d356a921ab7d7bef993019244 && git checkout 5ee979735a09d96d356a921ab7d7bef993019244
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5a9138703d7461d2f586486509b2035d15450c16 e07c9f9bac90d80c2938b0b3d5d236d4da642c5c && git checkout -B drift-repro 5a9138703d7461d2f586486509b2035d15450c16 && git merge --no-ff e07c9f9bac90d80c2938b0b3d5d236d4da642c5c
node scripts/docs-audit/affected-docs.mjs --json 5a9138703d7461d2f586486509b2035d15450c16
|
Cleared to queue with the one by-design red — #16502 ruled C + D (director seat, decision batch #73, 2026-09-07)Maintainer reply, verbatim: 「同意」. The The rule text and the gate's contradictory failure output are being fixed on their own card (see #16502's ruling comment). The repository squash setting moves to Generated by Claude Code |
Ruling C executed — un-drafted and queued, carrying the one by-design red
What the ruling says, and what this seat did
⇒ Un-drafted at 09:17:12Z, auto-merge armed at 09:17:18Z. ⛔ Option A was never taken — this PR lands through the queue or it does not land. CI, collapsed latest-per-name before tallying44 raw check runs ⇒ 33 distinct names: 27 success, 5 skipped, 1 failure. ⭐ Collapsing was load-bearing twice here, not once:
One thing measured rather than assumed, because the echo was surprising
Positive control, read off ⇒ The queue's merge method governs the landing shape; the PR-level method in that echo is inert. ⭐ And the empty-vs-populated echo difference is explained by queue ownership timing, not by one call succeeding and the other failing: #16536 was Not this seat's, and not done hereRuling D — moving the repository setting Card #14895: Generated by Claude Code |
Fixes #14895
os i18n extract --checkprinted a "Regenerate and commit" command assembled at the print site from four things that happened to be in scope — the config argument, the emitted locales minus the default one,--filland--out. Every other flag the operator passed was not in the expression, so it was not in the advice.Reproduced first, on this branch's fork point
Driven against a fixture with the reporter's shape (
i18n.defaultLocale: 'zh-CN', onekpi_-prefixed object), frompackages/cli:All three reported defects present, and each measured separately:
--locales=empty. The echo dropped the default locale on the grounds that--localesalways re-adds it, and here the only locale asked for was the default one. TheSkeleton summarytwo lines up nameszh-CN— the report's own control: one code path had the locale, the other did not.--no-metadata-forms,--no-objects-only,--filter=kpi_were never candidates for that expression.missing:printed a../../../../../…walk out of the cwd, for a directory the operator had just typed in full.And the loop, driven end to end
Defect 2 is the one that costs something, so it was measured rather than reasoned about. Following the printed advice verbatim:
Two files instead of one, including a
metadata-formscompanion the operator had explicitly switched off. Re-running the original--checkthen failed again — onout of date:instead ofmissing:, exit 1 — and printed the same wrong command. A failure that heals itself in one step became a loop, and the printed advice was what stopped it healing.The repair — a deletion, never an assembly
The hint is now this run's own argv with the
--checktoken removed:The premise was verified rather than assumed: a temporary probe confirmed
this.argvholds["tmp/repro-14895/stack.config.ts","--locales=zh-CN","--no-metadata-forms","--no-objects-only","--filter=kpi_","--out=…","--check"]— the arguments as typed, with the command id stripped — alongsidethis.id === "i18n:extract"andconfig.bin === "os". The probe was reverted and the file's blob hash checked back to itsHEADvalue before any real edit.Why a deletion: an assembled command is wrong in one unbounded way — every flag that exists now, and every flag added later, has to be remembered at that print site or it silently goes missing. Nothing enumerates flags any more, so the echo is correct for flags this file has never heard of. Tokens are shell-quoted so the line survives a copy, and a
--terminator is honoured so a positional--checkis left alone.The degraded fallback is wired, and it is not what runs. If
--checkis not in the argv the command cannot point at what it removed, and printsre-run the same command without `--check`rather than guessing. Stated plainly: today's flag surface has no other way to set--check(noenv, no default, noallowNo), so that branch is defence, not a path a user can reach — it is what structurally prevents "assemble an approximation" from ever becoming the fallback. It is therefore not covered by a test, and this PR does not claim it is. Full echo turned out to be reachable, so the card's sanctioned fallback was not taken as the answer to defect 2.Defect 3, and one sibling line repaired with it
Diagnostic paths print absolute once the cwd cannot reach them downwards, and keep the short relative form otherwise.
The same repair covers the
Wroteline as well asmissing:/out of date:. Naming it explicitly rather than letting it ride: it is the samepath.relative(process.cwd(), file)expression, in the same function, with the same symptom — the reproduction's write leg above printedWrote ../../../../../tmp/os-i18n-repro-jNrZ/…— and repairing only the--checkhalf would have left two adjacent lines disagreeing about how to name the same file.The threshold is deliberate rather than "print everything absolutely": all nine of this repo's extract configs document a repo-relative
--out=packages/…and run from the repo root, so their output is byte-identical to before. Verified by reading the--out=out of all nine configs, not assumed.The pin, and its ablation
packages/cli/test/i18n-extract-check-hint.e2e.test.tsdrives the real CLI. Two shapes carry the weight:--filterand--no-objects-onlyby name — a name-probing pin would inherit exactly the blind spot being closed, and would be green for the next flag added.sh -c, with anosshim onPATHin front of the source entry point, and requires the original--checkto then pass. No approximate command can satisfy it. A second parameterisation carries a|in a--filtervalue, so the quoting is exercised by a real shell rather than asserted.Ablation — prediction written before the run. Mutation: restore only the old assembled expression, leaving the path repair in place. Predicted 4 red / 2 green, naming all six by name, with the two
displayPathcases staying green (a mutation that reds them would not be the one described), and predicting that the healing cases fail on the later{status, drift}assertion because the copied command still exits 0.Observed: 4 failed / 2 passed — exactly the four predicted, exactly the two predicted green.
One prediction missed, recorded rather than smoothed over: I predicted both healing cases would fail on the
{status, drift}assertion. The first did; the second got past it and failed one assertion later, on the file census (+ "zh-CN.metadata-forms.generated.ts"). With that case's filter the objects module happens to be byte-identical either way, so the surplus companion is the only thing that distinguishes them — which is precisely why that census assertion is in the test.The mutation was confirmed on disk before the run (injected text present, removed text absent, blob hash moved) and the restore was verified by state rather than by exit code:
git diff HEADempty and the file's blob hash back to itsHEADvalue.Verification
Gate families derived with
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackat heade07c9f9bac9, re-derived after mergingorigin/mainto clear aSTALE TREEwarning; the change set it read is exactly the three files below. All 57 derived commands measured, all green, each exit code captured before any pipe.Five needed a second pass, and none of the five was a finding:
check:dual-build-cjs-loads,check:i18n,check:i18n-coverage,check:i18n-walk-parityfirst returnedPREREQUISITE NOT MET(exit 3 / 1) — not measured. After building the closure they run green;check:i18nreports all 9 packages in sync, driving the built CLI that carries this change, so no committed bundle moves.check:type-check-debtexited 3 on a heap OOM caused by my ownNODE_OPTIONS=--max-old-space-size=4096, which is tighter than the 6144 MB ceiling the gate pins. Re-run without that override: green.Also run at this head:
pnpm --filter @objectstack/cli typecheck(green), with--listFilesused to confirm both changed files are genuinely inside the programs —extract.tsin thesrcprogram, the new pin intsconfig.test.json's — and that neither is among the three pre-existing ledgered files that error. The i18n-extract test family (10 files / 153 tests) and the CLI package's wholeunittier (181 files / 2453 passed, 6 expected-fail) are green, as is the repo-widepnpm lint(eslint . --no-inline-config, exit 0) — run in full, so no narrowing argument is needed.Not verified here, by design: the CI gate jobs themselves.
Notes for review
no. The mechanical floor is untripped and was checked against the diff rather than recalled: no newly exported symbol (displayPath,shellTokenandrerunWithoutCheckare all module-local, and not exporting the reconstruction helper was the deliberate choice), no key added to the--jsonpayload — that face returns before the--checkblock is reached — and nopackages/spec/src/**path. The one consideration that would flip it, named so it is a decision and not an omission: this does change a published CLI's stdout, which humans copy from. If CLI stdout counts as contract here, grade ityes.@objectstack/cli: patch) — user-visible CLI behaviour in a published package.#14894's repair is not revisited. Its settled semantics were read from PR fix(cli): honour--no-metadata-formswhatever--objects-onlyis set to #16120 (--metadata-formsis the only control over the baseline, independent of--objects-only) and followed; that reading is what makes the surplus companion file in the reproduction legible as harm.Files
packages/cli/src/commands/i18n/extract.tspackages/cli/test/i18n-extract-check-hint.e2e.test.ts(new).changeset/i18n-extract-check-hint-echoes-the-invocation.md(new)🤖 Generated with Claude Code
https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N