fix(cli): os i18n extract --check --dry-run compares instead of exiting 0 having compared nothing - #16627
Conversation
`--check --dry-run --out=DIR` exited 0 having compared nothing: the `--dry-run` branch returned before the `--check` block was reached, so the same tree that failed `--check` with `Translation bundles have drifted from the schema` reported success once `--dry-run` was added. Both flags mean "write nothing", so the pair reads as the safest spelling for CI, and a check that cannot fail is indistinguishable from a check that finds nothing. The dump `--dry-run` asks for still happens; the return out of that branch is now conditional on `--check` being off, so the comparison below runs and reports. Nothing is written on either path. Same branch, same card: the `pass --out=<dir> to write` advice was printed even to runs that had just passed `--out`, which reads as "your directory was ignored". With an `--out` the line now names it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ
📓 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 1cb9a2274bc859dc3a8f66795c0c2ec5f4f37f75 && git checkout 1cb9a2274bc859dc3a8f66795c0c2ec5f4f37f75
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 55bbd92e3b5072d0f943891427c3a24e84c7ea90 5dc4c14e2bac9276e7a017b9dfb675544b25d1f1 && git checkout -B drift-repro 55bbd92e3b5072d0f943891427c3a24e84c7ea90 && git merge --no-ff 5dc4c14e2bac9276e7a017b9dfb675544b25d1f1
node scripts/docs-audit/affected-docs.mjs --json 55bbd92e3b5072d0f943891427c3a24e84c7ea90
|
Fixes #16480
os i18n extract --check --dry-run --out=DIRexited 0 having compared nothing. Both flags mean "write nothing", so the pair reads as the safest spelling to put in CI — and it was the one spelling that measured nothing.The defect, reproduced with its positive control
Driven on one drifted fixture, two invocations differing only by
--dry-run:... --out=DIR --checkmissing: DIR/zh-CN.objects.generated.ts, thenTranslation bundles have drifted from the schema... --out=DIR --check --dry-runDry run — no files written (pass --out=DIR to write).and nomissing:/out of date:/ in-sync line at allThe first run is the second one's control: the drift is provably there, and the second reported success.
Mechanism, read off the code rather than inferred from the exit codes: in
packages/cli/src/commands/i18n/extract.tstheif (flags['dry-run'] || !flags.out)branchreturned unconditionally, and it sits ahead of the--checkcomparison block. Nothing was compared.The fix — route (a), the graded default
The
returnout of that branch is now conditional on--checkbeing off. The stdout dump--dry-runasks for still happens; when--checkis on, execution falls through to the comparison and reports it. Nothing is written on either path — the write loop past the comparison stays unreachable, because--checkeither returns in sync or exits 1.Measured after the change, same fixture:
--check --dry-run, nothing committedmissing: ...+ drifted sentence--check --dry-run, committed but staleout of date: ...+ drifted sentence--check --dry-run, in sync1 bundle(s) are in sync with the schema--dry-run --out=DIR(no check)--dry-run, no--outpass --outadviceSame path, same card: the
--outadviceDry run — no files written (pass --out=DIR to write)was printed even when--outwas passed, which re-teaches the reader that their directory was ignored when it was not. A run with an--outnow names it; a run without one keeps the advice unchanged.The pin, and why it is shaped this way
packages/cli/test/i18n-extract-check-dry-run.test.ts— 5 cases, driving the real CLI.--checkthat compares nothing, so every case pins the reported drift beside the code. The ablation proves that is not a theoretical worry: with the fix reverted, the in-sync case showsstatus: +0on both sides and fails only oninSync: truevsfalse. An exit-code-only assertion would have stayed green there.The two drifted cases are stated as an equality against the same invocation without
--dry-run— the card's own method — and spell out the expected values, because an equality alone is also satisfied by two runs that are both broken. The in-sync case is the falsifier no unconditional failure can pass.Reviewer's call, flagged deliberately: the file is not named
.e2e, so it runs in the per-PR and merge-queue population rather than the nightly tier. Reasoning in the file header: this regression reads green, so between reintroduction and the next nightly, every run of the pair reports success about a comparison that is not happening. Measured cost of that choice on this (shared, contended) box: 7 CLI spawns, ~11s each, ~77s for the file. Renaming it to.e2emoves it to the nightly and costs nothing else.Verification
return): on-disk proof bygrep -cboth directions, blob hash differs from HEAD; 4 of 5 cases red, in the predicted direction (status: 0, drift: [], drifted: false). Restore leggit checkout HEAD -- ...proven by an emptygit diff HEADand a blob hash identical to HEAD's. The subject resolves throughsrc/(bin/run-dev.jsis the source entry point, by its own documented design), so there is nodistleg to prove.vitest run --project integration test/i18n-extract-check-dry-run.test.ts— 5 passed.packages/clitypecheck— exit 0.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands(no paths) at5dc4c14e2b. Reconciliation total: 58 commands — 56 exit 0, 2 exit 3. Artifact rosters run separately: 40 — 38 exit 0, 2 non-verdicts (one of the three was merely unwired and passes once wired against this PR, below).pnpm lint(eslint . --no-inline-config, whole repo, no narrowing): exit 0, 151s.⊘ NOT MEASURED — recorded as such, never as a pass:
pnpm check:dual-build-cjs-loadsPREREQUISITE NOT MET— reads built output; 12 packages have nodist/in this worktreepnpm check:i18n-coverageCOULD NOT MEASURE— 12 of 13 configs linted; the 13th needs@objectstack/connector-mcpbuiltnode scripts/check-single-claim-paths.mjsPR_NUMBER=16627— its own API fetch answersGitHub API 401 for /repos/.../pulls/16627/files. An environment/auth condition, not a verdict about this PRcheck:react-declaration-parityMANIFEST is not set— its registry side isobjectui's manifest, deliberately outside this session's repo scopeNone of the four is a finding about this diff; the first two are local build state that CI builds.
✓ Measured after the PR existed, which is the context it was missing:
node scripts/check-partof-closing-keyword.mjswired with this PR's real body and its 2 commits (PR_BODY,PR_COMMITS_FILE) — exit 0: "carries no Part-of/closing-keyword contradiction and no closing keyword bound to a card its own sentence says it is not closing, and its 2 commit message(s) carry no card-relation trailer". That matters here because this body names #16600, #14895, #16242 and #16470 alongside its oneFixes.Changeset
Owed, and decided by measuring that the change reaches published
distrather than by a remembered rule: afterpnpm --filter @objectstack/cli build,no files written tois present inpackages/cli/dist/commands/i18n/extract.js(1 hit, exit 0), with a non-zero control — a nonsense token in the same file returns 0 hits, exit 1, so the probe can fail.npm pack --dry-runconfirmsdist/commands/i18n/extract.jsis packed and 0src/files are.@objectstack/cliis not private.Graded⚠️ The one judgement worth a second opinion: a pipeline running
patch.--check --dry-runagainst drifted bundles starts failing on this release. The changeset says so in as many words. The failure is not new — the drift was already there and the old exit code was wrong about it — which is why this reads as a fix rather than a breaking change.Acceptance notes
os i18n extract --check --jsonexits 0 having compared nothing — the second false green on this command #16600:os i18n extract --check --jsonexits 0 having compared nothing — the second false green on this command, same class, different branch (if (flags.json) { ... return; }also sits ahead of the comparison). Measured here with the same positive control. ⛔ Deliberately not folded in: removing it requires deciding a new public payload contract (what a--json --checkrun reports when it finds drift, and with what exit code), which is a contract decision rather than the control-flow correction this card is.no, agreeing with triage. The mechanical floor is a new exported symbol or a new key on a published payload; this diff has neither — no export changes, and the--jsonpayload is untouched.os i18n extract --checkfailure prints a regenerate command with--locales=empty and every other flag dropped — following it yields a different bundle that fails --check again #14895's--checkfailure hint (PR fix(cli): makei18n extract --check's regenerate hint the invocation it was given #16470 fenced it deliberately and has merged) and cli:os i18n extract --no-objects-only --source-hashesdrops every non-objectsprovenance record — the committed-section list is the literal'objects', not the sections the module holds #16242's--source-hashesprovenance drop. Neither is addressed here..github/workflows/**in the co-occurrence set).objectuiorcloud, which are outside this session's repo scope and are therefore unmeasured, not zero. No caller in either tree was encountered while doing this work.extract.ts(22 open PRs; control — 9 touchpackages/**, 2 touchpackages/cli/**).content/docs/ui/translations.mdxdocuments--check— "--checkwrites nothing: it re-renders and diffs against--out, naming each stale file and printing the regenerate command" — which was true before and is true now, and holds in one more case after this change; it never mentions--dry-run.content/docs/protocol/kernel/i18n-standard.mdxdocuments only--locales/--out/--jsonfor this command and never mentions--checkor--dry-run. Neither page states or implies the old behaviour, so both are left alone.content/docs/pairsi18n extractwith--dry-run. The single co-occurrence iscontent/docs/releases/v17.mdx, where thedry-runsentence is about the retiredBatchOptions.validateOnlyand is unrelated; that page's ownos i18n extract --checksentence is accurate.content/docs/deployment/cli.mdx's--dry-runbelongs toos generate, and that page does not documenti18n extractat all.content/docs/releases/**was listed by the tool and deliberately not touched — release notes are written centrally at release time. Nothing in v15/v16/v17 was found to be factually wrong about this command, so there is nothing to hand off either.Generated by Claude Code