Skip to content

fix(cli): os i18n extract --check --dry-run compares instead of exiting 0 having compared nothing - #16627

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-16480-check-dry-run-false-green
Sep 7, 2026
Merged

fix(cli): os i18n extract --check --dry-run compares instead of exiting 0 having compared nothing#16627
os-sales merged 2 commits into
mainfrom
claude/issue-16480-check-dry-run-false-green

Conversation

@claude

@claude claude Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #16480

os i18n extract --check --dry-run --out=DIR exited 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:

invocation exit what it printed
... --out=DIR --check 1 missing: DIR/zh-CN.objects.generated.ts, then Translation bundles have drifted from the schema
... --out=DIR --check --dry-run 0 Dry run — no files written (pass --out=DIR to write). and no missing: / out of date: / in-sync line at all

The 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.ts the if (flags['dry-run'] || !flags.out) branch returned unconditionally, and it sits ahead of the --check comparison block. Nothing was compared.

The fix — route (a), the graded default

The return out of that branch is now conditional on --check being off. The stdout dump --dry-run asks for still happens; when --check is 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 --check either returns in sync or exits 1.

Measured after the change, same fixture:

invocation exit reported wrote
--check --dry-run, nothing committed 1 missing: ... + drifted sentence nothing (dir still empty)
--check --dry-run, committed but stale 1 out of date: ... + drifted sentence nothing (stale bytes unchanged)
--check --dry-run, in sync 0 1 bundle(s) are in sync with the schema nothing
--dry-run --out=DIR (no check) 0 names the directory nothing
--dry-run, no --out 0 keeps the pass --out advice nothing

⚠️ Route (b) — refusing the pair as contradictory — was not taken and no reason to prefer it turned up.

Same path, same card: the --out advice

Dry run — no files written (pass --out=DIR to write) was printed even when --out was passed, which re-teaches the reader that their directory was ignored when it was not. A run with an --out now 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.

⚠️ A case asserting only the exit code would pass against a --check that 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 shows status: +0 on both sides and fails only on inSync: true vs false. 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 .e2e moves it to the nightly and costs nothing else.

Verification

  • Ablation (fix committed first; mutation = the pre-fix unconditional return): on-disk proof by grep -c both directions, blob hash differs from HEAD; 4 of 5 cases red, in the predicted direction (status: 0, drift: [], drifted: false). Restore leg git checkout HEAD -- ... proven by an empty git diff HEAD and a blob hash identical to HEAD's. The subject resolves through src/ (bin/run-dev.js is the source entry point, by its own documented design), so there is no dist leg to prove.
  • Tests: vitest run --project integration test/i18n-extract-check-dry-run.test.ts — 5 passed. packages/cli typecheck — exit 0.
  • Gates: derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (no paths) at 5dc4c14e2b. 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:

command why
pnpm check:dual-build-cjs-loads exit 3, PREREQUISITE NOT MET — reads built output; 12 packages have no dist/ in this worktree
pnpm check:i18n-coverage exit 3, COULD NOT MEASURE — 12 of 13 configs linted; the 13th needs @objectstack/connector-mcp built
node scripts/check-single-claim-paths.mjs exit 1 once wired with PR_NUMBER=16627 — its own API fetch answers GitHub API 401 for /repos/.../pulls/16627/files. An environment/auth condition, not a verdict about this PR
check:react-declaration-parity exit 1, MANIFEST is not set — its registry side is objectui's manifest, deliberately outside this session's repo scope

None 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.mjs wired 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 one Fixes.

Changeset

Owed, and decided by measuring that the change reaches published dist rather than by a remembered rule: after pnpm --filter @objectstack/cli build, no files written to is present in packages/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-run confirms dist/commands/i18n/extract.js is packed and 0 src/ files are. @objectstack/cli is not private.

Graded patch. ⚠️ The one judgement worth a second opinion: a pipeline running --check --dry-run against 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

  • Filed as cli: os i18n extract --check --json exits 0 having compared nothing — the second false green on this command #16600: os i18n extract --check --json exits 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 --check run reports when it finds drift, and with what exit code), which is a contract decision rather than the control-flow correction this card is.
  • Clause-② re-derived from the delivered diff: still 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 --json payload is untouched.
  • Out of scope and untouched, as dispatched: cli: os i18n extract --check failure prints a regenerate command with --locales= empty and every other flag dropped — following it yields a different bundle that fails --check again #14895's --check failure hint (PR fix(cli): make i18n 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-hashes drops every non-objects provenance record — the committed-section list is the literal 'objects', not the sections the module holds #16242's --source-hashes provenance drop. Neither is addressed here.
  • The p2 grade's escalation criterion still does not fire in this repo (measured at claim time: 0 commands carry both flags, no .github/workflows/** in the co-occurrence set). ⚠️ That reading is partial — it does not cover objectui or cloud, 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.
  • Serial clearance re-checked at push time: no open PR holds extract.ts (22 open PRs; control — 9 touch packages/**, 2 touch packages/cli/**).
  • Docs-drift rows re-verified by hand; nothing edited, because nothing is falsified. content/docs/ui/translations.mdx documents --check"--check writes 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.mdx documents only --locales / --out / --json for this command and never mentions --check or --dry-run. Neither page states or implies the old behaviour, so both are left alone.
  • The drift tool's stated blind spot, swept independently: no page under content/docs/ pairs i18n extract with --dry-run. The single co-occurrence is content/docs/releases/v17.mdx, where the dry-run sentence is about the retired BatchOptions.validateOnly and is unrelated; that page's own os i18n extract --check sentence is accurate. content/docs/deployment/cli.mdx's --dry-run belongs to os generate, and that page does not document i18n extract at 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

os-sales and others added 2 commits September 7, 2026 11:39
`--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
@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 2 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
  • 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 55bbd92e3b5072d0f943891427c3a24e84c7ea90packageMentionDocs.

Which tree this was computed on

This run read content/docs from 1cb9a2274bc859dc3a8f66795c0c2ec5f4f37f75 — the merge of head 5dc4c14e2bac9276e7a017b9dfb675544b25d1f1 into base 55bbd92e3b5072d0f943891427c3a24e84c7ea90, 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 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

⚠️ 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 55bbd92e3b5072d0f943891427c3a24e84c7ea90 → 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
@os-sales
os-sales marked this pull request as ready for review September 7, 2026 14:18
@os-sales
os-sales added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit 309bee5 Sep 7, 2026
41 checks passed
@os-sales
os-sales deleted the claude/issue-16480-check-dry-run-false-green branch September 7, 2026 14: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

Development

Successfully merging this pull request may close these issues.

[finding] os i18n extract --check --dry-run exits 0 having compared nothing — a --check that passes without checking, i.e. a false green in CI

1 participant