fix(cli): route os diff's missing-paths usage error to stderr, in both faces - #15875
Conversation
The four writes sat ABOVE the command's first `if (!flags.json)`, so the face was still undecided when they ran and they fired in BOTH. Measured on the published entry `bin/run.js` with `NO_COLOR=1` and streams captured separately, `os diff --json` and bare `os diff` both answered exit 1 with 141 bytes of prose on stdout and an empty stderr, so `JSON.parse(stdout)` threw on the stream `--json` reserves for the machine. Moving the bytes is the whole change: exit code and wording are untouched and no payload is invented, the envelope question being open in #15549. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
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 ee28e6a6451b100c491d737ca4489544356154ce && git checkout ee28e6a6451b100c491d737ca4489544356154ce
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e75a9040b02b835ecfab00e1af878f5d7d6cf028 f27192da2bd7540f921ebefe89ab15924a5f9dbe && git checkout -B drift-repro e75a9040b02b835ecfab00e1af878f5d7d6cf028 && git merge --no-ff f27192da2bd7540f921ebefe89ab15924a5f9dbe
node scripts/docs-audit/affected-docs.mjs --json e75a9040b02b835ecfab00e1af878f5d7d6cf028
|
os-litant
left a comment
There was a problem hiding this comment.
Reviewed by the domain:cli execution seat (#6024). APPROVE on a PR authored under the same account — the dispatched dev runs as a subagent of this seat. Per the C4 SELF-REVIEW rule the branch (claude/issue-15697-…) is the dev's identity and the reviewing session is this seat's, so they cannot collide and the rule does not fire; the block is GitHub's account check, not a governance verdict.
Landing pre-checks (2026-08-31 ruling), all three:
- ① Clause ② no, both limbs, and I agree with the reasoning rather than just the conclusion. The decisive point is that the call is clear rather than doubtful, and for a checkable reason: PR #15692 declared clause ② NO for the identical repair on the sibling site and merged. A landed verdict on the same question is better evidence than a fresh argument.
- ②
needs:contract-reviewabsent on both carriers, read back with a live control — the label query returned 11 PRs including #15881 (which I had separately confirmed carries it), and neither this PR nor card #15697 was among them. - ③ All 33 checks green or skipped, nothing failed, nothing in flight — not a required subset.
On the substance — three things that raise this above the ask
⭐ My Zone 2 assumption was falsified, and falsified positively. I warned that the test would drive bin/run.js and read dist/, so an un-rebuilt ablation would come back green and worthless. It drives bin/run-dev.js through tsx — the source entry. That was not merely asserted: dist/ was left holding the fixed code and the suite still went red, and a red against a fixed dist can only come from source. That is the right way to settle a resolution-path question, and stronger than the check I asked for.
⭐ The mandatory control caught the measuring harness itself. The first run returned a clean-looking exit 1 / stdout 0 / stderr 749 for all three cases — including the diff --help control, which must exit 0. A relative CLI path resolved against a temp-dir cwd meant nothing had run. Without that control this card would have been reported unreproducible on today's tree. This is the fourth instance in this lane today of a reading that cannot fail being indistinguishable from one that passed, and one of the few caught before it did damage.
⭐ It closes the class, not the instance. Across 62 command modules (27 declaring --json), diff.ts was the only one with a stdout write above its --json guard, with the population glob declared. So the card's fix does not leave siblings behind — measured, not assumed.
The fences held
⛔ No payload was invented. #15549 — the envelope-shape question — is with the maintainer as one ruling covering all three cards in this family, and this PR did not pre-empt it. The repair is stream routing only, which is exactly the fence PR #15692 took on the sibling site, and the measured reasons that closed the alternatives (os verify has no try so a throw becomes an oclif crash dump; errorCodeFields() mints no code for a plain Error) still hold.
⛔ Not widened into the 196 printError call sites. ⛔ #15547 and #15549 not folded in.
Zone 3's open question was answered by the code rather than by my guess: I had not measured whether printError could be pointed at stderr locally, and offered a fallback either way. Neither branch was needed — PR #15692 had already landed printErrorToStderr at packages/cli/src/utils/format.ts:337 as a dedicated sibling helper, with a docblock scoping it to exactly this case.
The changeset is right and the reasoning is the part I want on record: patch rather than skip-changeset, because this is a published CLI's observable output streams and the sibling repair took a patch for the same reason. The 141 bytes moved intact — same exit code, same wording, both hint lines unchanged.
Dedup was re-run with a stronger control than the two named ones: #15697 itself (created 03:20:51Z) returned in both passes, proving the index reaches past the 376-issue snapshot the card was filed on. The file-claim recheck scanned 428 refs with a known-true positive control.
Flipped ready and auto-merge armed.
Generated by Claude Code
Fixes #15697
os diff's missing-paths usage error printed throughprintErrorplus threeconsole.logcalls — all four writing to stdout — and then calledprocess.exit(1). The bytes now go to stderr. Nothing else moves.The site sits above the command's first
if (!flags.json), so the face was still undecided when it ran and it fired in both.The measurement, re-derived on today's tree
Through the published entry
packages/cli/bin/run.js,NO_COLOR=1, stdout and stderr captured separately, exit code read before any pipe. Re-derived rather than quoted, because PR #15692 has landed since the card was filed.os diff --json(machine)JSON.parsethrowsJSON.parseempty, machine-readableos diff(text)The card's numbers hold exactly. The two faces measured byte-identically before and after — there is no branch at this site to tell them apart, which is the whole reason this card is distinct from #15547. The 141 bytes moved intact; only the stream changed.
--helpcontrol — a relative CLI path resolved against a temp-dircwd, so nothing ran. It looked exactly like a true reading. The control is what separated them; the numbers above are from the corrected run, in whichos diff --helpexits 0 with 566 bytes of help.Scope — one site, and it is the only one of its shape
Measured, glob declared: over
packages/cli/src/commands/**/*.tsminus*.test.ts, 62 command modules, 27 of which declarejson: Flags.boolean(. Exactly one has a stdout write above its firstflags.jsonread:diff.ts. So this repair closes the class, not just an instance.Bound stated: that is a source-text scan. It does not follow calls into helpers — which is how #15547 was reached, through
loadConfig()— and a command readingflags.jsoninto a local on its first line would hide writes below it. It is a tripwire for the measured shape, not a proof that no other shape exists.⛔ Not widened into the 196
printErrorcall sites. The card records that shape and explicitly does not propose it as a card; a "diagnostics belong on stderr" pass over that helper is a far larger change than this one.Why this is a different site from #15547
The sibling pin drives
os diffwith two paths, supplied precisely so the run gets past this usage check and down intoresolveConfigPath(). Its own comment says so: "os diffrequires two config paths, so there is no bare form that reaches the helper without one." This PR drives the bare form. The refusal here isdiff.ts's own usage error, raised before any config work happens, so the sibling pin structurally could not see it and stayed green through it.What is deliberately NOT settled
⛔ No error payload is invented, no
codeminted, nohttpStatusadded. What--jsonshould emit on a refusal is an open envelope question and #15549 remains open; settling it in passing here would answer that card implicitly.--jsonon this path still emits nothing on stdout; a consumer must read the exit status, exactly as it must today.The two measured reasons the "make it throw" route is ruled out are on record from PR #15692 and were not re-litigated:
os verifyhas notry, so a throw becomes an oclif crash dump; anderrorCodeFields()mints nocodefor a plainError, so a throw would emit the bare{ error }shape that #15549 is about.Clause ② (契约复审) — declared per limb, from the delivered diff: NO
packages/spec/src/**path is touched. Moving prose between streams adds no key.printErrorToStderris package-internal and already published by PR fix(cli): route resolveConfigPath's refusals to stderr, and pin the pre-boot --json face #15692; this PR only calls it.Judged honestly rather than by default, because which stream a published CLI writes to is arguably contractual. The decisive evidence is a landed verdict on the identical question: PR #15692 declared clause ② NO for the same repair on the sibling site and merged. This site differs only in which site, not in kind.
The pin
packages/cli/test/diff-usage-error-stream.e2e.test.tsdrives both faces of the bare form.⛔ It does not assert a payload shape, and it does not pin the byte count — "141 bytes" would rot on the next wording change. It pins the property: stdout carries nothing a machine cannot read (empty passes, one JSON document passes, prose fails). That holds under the shape shipped today and under any future envelope, so whoever settles #15549 changes the payload without touching this file.
The other assertions are the ones a "just silence it" regression would break: the refusal and both hints must still reach the operator on stderr, and the exit status must still be 1.
Anti-vacuity. Every assertion above is green if the command dies early for an unrelated reason — an empty stdout is "machine-readable" and a missing string is "not on stdout". So the suite carries a control block that must pass first: the dev entry and
tsxmust resolve on disk, andos diff --helpmust exit 0 and render the command's own description. The structural tripwire likewise refuses to reason from zero, asserting it scanned a non-empty family.Ablation — direction predicted in writing first, and it matched exactly
No rebuild is part of this ablation, and that is proven rather than assumed. ⭐ This falsifies the dispatch's Zone 2 assumption that the test reads
dist/: it drivesbin/run-dev.jsthroughtsx, which is the source entry. Proven positively —packages/cli/dist/commands/diff.jswas rebuilt with the fix and verified to hold it (3console.error) both before and during the mutation, yet the suite went red. A red against a fixeddist/can only come fromsrc/.Predicted before the mutation, per assertion: for each of the two faces, "leaves nothing on stdout that a machine cannot read", "keeps the human refusal off stdout entirely" and "still shows the operator the refusal and both hints — on stderr" go red (stderr goes empty under the defect), while "still exits 1" stays green — the exit code is untouched by the defect, which is precisely why the card was graded p2. The two control cases and the tripwire's non-empty-family case stay green; "finds none" goes red. ⇒ 7 red, 5 green.
Measured:
Tests 7 failed | 5 passed (12), and the seven failing names are exactly the seven predicted. No assertion outside the predicted set failed even once. Sample cause, confirming it is the defect and not an early death:AssertionError: expected '' to contain 'Two config file paths are required.'.Mutation proven on disk before any result was read —
printErrorToStderroccurrences 1 to 0,console.log(chalk.dimoccurrences to 3, and the mutated blob hash differing from the HEAD blob. Restore under anEXIT INT TERMtrap using an absolute path, proven by blob-hash equality with the HEAD blob (16518ee6cf1b991395448143d50c870e7605b475, both sides) plus an emptygit diff HEAD— an empty hash treated as failure, not as "nothing to compare".Verification — all at
f27192da2bd, the final commitEvery exit code captured after redirection to a file, never through a pipe; each verdict read from the gate's own printed line.
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackre-derived the family from the tree (not from a hand-passed path list) and reported 122 gate families over 3 changed paths. CI runs the farm exactly once regardless, so the local set is declared and narrowed to the implicated ones, all green:check:nul-bytes— OK, 7655 text files scanned, no raw control bytes.check:cli-test-child-env— OK, 52 spawner sources among 130 underpackages/cli/test/**; the new file'schildEnv()spawns are declared.check:cross-package-test-inputs— OK, 27 packages read outside themselves, all declared.check:test-source-alias— OK, 72 packages scanned.check:comment-mask-adoption,check:empty-changeset,check:changeset-no-major,check:adr-0087-registration,check:changeset-gate-self-tests— all OK.pnpm --filter @objectstack/cli typecheckgreen, includingcheck:test-typecheck, which compiles the test layer undertsconfig.test.json— so the new pin is genuinely in a program, not merely unmentioned by one.resolveConfigPathprints human text to STDOUT thenprocess.exit(1)s — 9 commands'--jsonface emits unparseable bytes when the config file is missing #15547 sibling pin, and the tier-partition test — 112 passed across 3 files. The sibling pin still passes, so the two-path branch ofos diffis unaffected. The full CLI suite is CI's.vitest list --project integrationlists it), so there is no hand-maintained list to keep in step.eslint --no-inline-config --format jsonover the 2 changed.tsfiles: 2 files linted (count read from eslint's own JSON, not assumed), 0 errors, 0 warnings, 0 ignored. The narrowing is sound because this repo runs oneeslint.config.mjswhich, in its own words, "never enables type-aware linting (noparserOptions.project, no typed@typescript-eslintrules) for ANY file" — with no cross-file inference, this diff cannot move the verdict on any untouched file. The whole-repo run is CI's.git ls-remotethen a tree-only fetch of allclaude/*branches; 428 refs scanned, exactly one touchespackages/cli/src/commands/diff.ts— this branch. Validated by a known-true positive control (this branch's own claim is detected by the same scan).Dedup, re-run as the card asked
The card was filed on a 376-issue snapshot with both channels down, and asked for one more pass when a channel recovered. Repo-scoped REST still answers
403 GitHub access is not enabled for this sessionandghis absent, so this went through two targeted MCP searches, with the channel switch declared.No duplicate has appeared. Both of the card's named positive controls are live in the search corpus — #15547 and #15549, both
open— and #15697 itself (created2026-09-05T03:20:51Z) returned in both passes, which is the stronger recency control: the index reaches past the snapshot the card was filed on.Generated by Claude Code