Skip to content

fix(cli): make i18n extract --check's regenerate hint the invocation it was given - #16470

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-14895-i18n-extract-check-hint
Sep 7, 2026
Merged

fix(cli): make i18n extract --check's regenerate hint the invocation it was given#16470
os-sales merged 2 commits into
mainfrom
claude/issue-14895-i18n-extract-check-hint

Conversation

@os-litant

@os-litant os-litant commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Fixes #14895

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.

Reproduced first, on this branch's fork point

Driven against a fixture with the reporter's shape (i18n.defaultLocale: 'zh-CN', one kpi_-prefixed object), from packages/cli:

$ os i18n extract tmp/repro-14895/stack.config.ts --locales=zh-CN --no-metadata-forms \
    --no-objects-only --filter=kpi_ --out=/tmp/os-i18n-repro-jNrZ --check

  Skeleton summary
    zh-CN        2 of 2 key(s) emitted

  X missing:    ../../../../../tmp/os-i18n-repro-jNrZ/zh-CN.objects.generated.ts

  X Translation bundles have drifted from the schema. Regenerate and commit:
  os i18n extract tmp/repro-14895/stack.config.ts --locales= --fill=empty --out=/tmp/os-i18n-repro-jNrZ

All three reported defects present, and each measured separately:

  1. --locales= empty. The echo dropped 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 — the report's own control: one code path had the locale, the other did not.
  2. Three flags dropped--no-metadata-forms, --no-objects-only, --filter=kpi_ were never candidates for that expression.
  3. 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:

  Skeleton summary
    zh-CN      775 of 776 key(s) emitted   objects 2 - metadataForms 773
  Wrote .../zh-CN.objects.generated.ts (2 keys)
  Wrote .../zh-CN.metadata-forms.generated.ts (773 keys)
  Generated 2 file(s)

Two files instead of one, including a metadata-forms companion the operator had explicitly switched off. Re-running the original --check then failed again — on out of date: instead of missing:, 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 --check token removed:

  os i18n extract tmp/repro-14895/stack.config.ts --locales=zh-CN --no-metadata-forms --no-objects-only --filter=kpi_ --out=/tmp/os-i18n-after-kkIt

The premise was verified rather than assumed: a temporary probe confirmed this.argv holds ["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 — alongside this.id === "i18n:extract" and config.bin === "os". The probe was reverted and the file's blob hash checked back to its HEAD value 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 --check is left alone.

The degraded fallback is wired, and it is not what runs. If --check is not in the argv the command cannot point at what it removed, and prints re-run the same command without `--check` rather than guessing. Stated plainly: today's flag surface has no other way to set --check (no env, no default, no allowNo), 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 Wrote line as well as missing: / out of date:. Naming it explicitly rather than letting it ride: it is the same path.relative(process.cwd(), file) expression, in the same function, with the same symptom — the reproduction's write leg above printed Wrote ../../../../../tmp/os-i18n-repro-jNrZ/… — and repairing only the --check half 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.ts drives the real CLI. Two shapes carry the weight:

  • The central case asserts the whole token list rather than probing for --filter and --no-objects-only by name — a name-probing pin would inherit exactly the blind spot being closed, and would be green for the next flag added.
  • The healing case executes the printed line verbatim through sh -c, with an os shim on PATH in front of the source entry point, and requires the original --check to then pass. No approximate command can satisfy it. A second parameterisation carries a | in a --filter value, 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 displayPath cases 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.

x echoes this run's own argv with `--check` removed, flag for flag
    Received: "os i18n extract  --locales= --fill=empty --out=/tmp/.../spelling"
x following the printed command makes the next --check pass (reported invocation)
    { status: 1, drift: [".../heal-reported-invocation/zh-CN.objects.generated.ts"] }
x following the printed command makes the next --check pass (shell-hostile flag value)
x removes only the --check token, leaving spelling and order alone

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 HEAD empty and the file's blob hash back to its HEAD value.

Verification

Gate families derived with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack at head e07c9f9bac9, re-derived after merging origin/main to clear a STALE TREE warning; 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-parity first returned PREREQUISITE NOT MET (exit 3 / 1) — not measured. After building the closure they run green; check:i18n reports all 9 packages in sync, driving the built CLI that carries this change, so no committed bundle moves.
  • check:type-check-debt exited 3 on a heap OOM caused by my own NODE_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 --listFiles used to confirm both changed files are genuinely inside the programs — extract.ts in the src program, the new pin in tsconfig.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 whole unit tier (181 files / 2453 passed, 6 expected-fail) are green, as is the repo-wide pnpm 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

  • Clause 2 graded no. The mechanical floor is untripped and was checked against the diff rather than recalled: no newly exported symbol (displayPath, shellToken and rerunWithoutCheck are all module-local, and not exporting the reconstruction helper was the deliberate choice), no key added to the --json payload — that face returns before the --check block is reached — and no packages/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 it yes.
  • Changeset added (@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-forms whatever --objects-only is set to #16120 (--metadata-forms is 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.ts
  • packages/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

…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
@github-actions github-actions Bot added the size/m label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 5 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/protocol/kernel/i18n-standard.mdx (via os i18n extract (command, read off packages/cli/src/commands/i18n/extract.ts))
  • content/docs/ui/translations.mdx (via os i18n extract (command, read off packages/cli/src/commands/i18n/extract.ts))

3 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v15.mdx (via os i18n extract (command, read off packages/cli/src/commands/i18n/extract.ts))
  • content/docs/releases/v16.mdx (via os i18n extract (command, read off packages/cli/src/commands/i18n/extract.ts))
  • content/docs/releases/v17.mdx (via os i18n extract (command, read off packages/cli/src/commands/i18n/extract.ts))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 5a9138703d7461d2f586486509b2035d15450c16packageMentionDocs.

Which tree this was computed on

This run read content/docs from 5ee979735a09d96d356a921ab7d7bef993019244 — the merge of head e07c9f9bac90d80c2938b0b3d5d236d4da642c5c into base 5a9138703d7461d2f586486509b2035d15450c16, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 5a9138703d7461d2f586486509b2035d15450c16 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 7, 2026

Copy link
Copy Markdown
Contributor

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 Part-of PR must not also close its card red on this PR is by design on a pushed branch (the gate's own header :108-112), the gate is not a required context, it has no merge_group trigger, and the residual risk of landing was measured at zero (body and commit name the same card, same relationship). ⇒ This PR may enter the queue carrying that single red. The PM lane un-drafts and queues it; ⛔ no history rewrite, ⛔ no manual merge.

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 PR_BODY by the maintainer's own hand.


Generated by Claude Code

os-sales commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Ruling C executed — un-drafted and queued, carrying the one by-design red

domain:cli seat, session session_01YFY46JydE1gMxQG1TqBcMZ, 2026-09-07T09:17Z.

⚠️ This seat was late. The ruling landed at 06:20:46Z (5565958378, director batch #73, verbatim 「同意」, options C + D) and I was still reporting this card as awaiting the maintainer nearly three hours later. The cause is the exact error I recorded a standing correction for earlier this round on #15892: I restated a card's status from an earlier reading instead of re-reading its thread. The correction was written; it was not applied here. Recording it as a miss rather than letting the fix look timely.

What the ruling says, and what this seat did

C — PR #16470 enters the queue carrying this one red. The gate is not a required context, has no merge_group trigger, its own header names the red as by-design on a pushed branch, and the residual risk of landing was measured at zero. ⛔ Option A (manual merge outside the queue) is refused: 「队列是唯一被认可的落地路径」… The PM lane un-drafts and queues it.

⇒ 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 tallying

44 raw check runs ⇒ 33 distinct names: 27 success, 5 skipped, 1 failure.

⭐ Collapsing was load-bearing twice here, not once:

  • Part-of PR must not also close its cardthe single red, and exactly the by-design one the ruling cleared.
  • Check Changeset — a failure at 03:39:08Z superseded by a success at 04:03:33Z. An uncollapsed tally would have published two reds and made this PR look like it had an unruled second blocker on top of the ruled one.

⚠️ The raw list is never the reading: the endpoint returns every run per name, and three names here carry two or three runs apiece.

One thing measured rather than assumed, because the echo was surprising

enable_pr_auto_merge was called with SQUASH and echoed back method: MERGE — and echoed populated fields, where the same call on #16536 an hour earlier echoed empty. Two discrepancies at once, and the expensive failure mode (landing as a merge commit, in a lane where every other PR squashes, which would also put ruling D's squash_merge_commit_message change out of reach of this PR) was worth ruling out by measurement rather than by reasoning about GitHub's semantics.

Positive control, read off origin/main: the eight most recent landings each have exactly one parent — including #16537 (b1b978c8d) and #16543 (9407e1865), armed by this seat with the identical call an hour earlier.

0344f402d parents=af7edfe13   (#15885)
af7edfe13 parents=b60f48b52   (#16542)
b60f48b52 parents=9407e1865   (#16532)
9407e1865 parents=b1b978c8d   (#16543)  ← armed by this seat
b1b978c8d parents=f48f3f1b2   (#16537)  ← armed by this seat
f48f3f1b2 parents=32c917d5d   (#16481)
32c917d5d parents=76c8c5aeb   (#16527)
76c8c5aeb parents=b0529e148   (#16523)

⇒ 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 clean and the queue took it immediately, while this PR reads mergeable_state: blocked and ordinary auto-merge is recorded first. That also re-confirms the refined reading of this call this seat recorded earlier — an empty echo is not a failure.

Not this seat's, and not done here

Ruling D — moving the repository setting squash_merge_commit_message from COMMIT_MESSAGES to PR_BODY — is the maintainer's own change in repository settings. #16516 (skills lane, governed surface) carries the rule text and the gate's self-contradictory failure output; it already exists, so ⛔ no new card was opened for it. Decision card #16502 stays pm:blocked behind those two, as the director seat set it — ⛔ this seat does not touch another seat's disposition.

Card #14895: pm:blockedpm:dispatched. It carries Fixes #14895, so it closes on merge.


Generated by Claude Code

@os-sales
os-sales added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit c14c70c Sep 7, 2026
42 of 45 checks passed
@os-sales
os-sales deleted the claude/issue-14895-i18n-extract-check-hint branch September 7, 2026 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

4 participants