Skip to content

fix(cli): route os diff's missing-paths usage error to stderr, in both faces - #15875

Merged
os-litant merged 2 commits into
mainfrom
claude/issue-15697-diff-usage-error-stderr
Sep 5, 2026
Merged

fix(cli): route os diff's missing-paths usage error to stderr, in both faces#15875
os-litant merged 2 commits into
mainfrom
claude/issue-15697-diff-usage-error-stderr

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes #15697

os diff's missing-paths usage error printed through printError plus three console.log calls — all four writing to stdout — and then called process.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.

face before after
os diff --json (machine) exit 1 · stdout 141 B · stderr 0 B · JSON.parse throws exit 1 · stdout 0 B · stderr 141 B · JSON.parse empty, machine-readable
os diff (text) exit 1 · stdout 141 B · stderr 0 B exit 1 · stdout 0 B · stderr 141 B

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.

⚠️ The harness caught itself first. The first run of this measurement reported exit 1 / 749 stderr bytes for all three cases including the --help control — a relative CLI path resolved against a temp-dir cwd, 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 which os diff --help exits 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/**/*.ts minus *.test.ts, 62 command modules, 27 of which declare json: Flags.boolean(. Exactly one has a stdout write above its first flags.json read: 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 reading flags.json into 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 printError call 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 diff with two paths, supplied precisely so the run gets past this usage check and down into resolveConfigPath(). Its own comment says so: "os diff requires two config paths, so there is no bare form that reaches the helper without one." This PR drives the bare form. The refusal here is diff.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 code minted, no httpStatus added. What --json should emit on a refusal is an open envelope question and #15549 remains open; settling it in passing here would answer that card implicitly. --json on 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 verify has no try, so a throw becomes an oclif crash dump; and errorCodeFields() mints no code for a plain Error, so a throw would emit the bare { error } shape that #15549 is about.

Clause ② (契约复审) — declared per limb, from the delivered diff: NO

  • Mechanical / path limb — NO. No key is added to any published payload (this path emits none, before or after). No packages/spec/src/** path is touched. Moving prose between streams adds no key. printErrorToStderr is 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.
  • Non-mechanizable conformance limb — NO. No input class is re-selected between two published verdicts: no invocation that worked before fails now, none that failed now succeeds. Same exit code, same wording, same four lines.

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.

⚠️ One residual for a reviewer to weigh, carried over from that PR: a wrapper that showed a user only the child's stdout now shows nothing on this path. Exit 1 is unchanged, and stderr was empty before, so nothing could have been parsing it.

The pin

packages/cli/test/diff-usage-error-stream.e2e.test.ts drives 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 tsx must resolve on disk, and os diff --help must 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 drives bin/run-dev.js through tsx, which is the source entry. Proven positively — packages/cli/dist/commands/diff.js was rebuilt with the fix and verified to hold it (3 console.error) both before and during the mutation, yet the suite went red. A red against a fixed dist/ can only come from src/.

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 — printErrorToStderr occurrences 1 to 0, console.log(chalk.dim occurrences to 3, and the mutated blob hash differing from the HEAD blob. Restore under an EXIT INT TERM trap using an absolute path, proven by blob-hash equality with the HEAD blob (16518ee6cf1b991395448143d50c870e7605b475, both sides) plus an empty git diff HEAD — an empty hash treated as failure, not as "nothing to compare".

Verification — all at f27192da2bd, the final commit

Every 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/objectstack re-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 under packages/cli/test/**; the new file's childEnv() 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 typecheck green, including check:test-typecheck, which compiles the test layer under tsconfig.test.json — so the new pin is genuinely in a program, not merely unmentioned by one.
  • Tests, narrowed and declared: the new pin, the [finding] resolveConfigPath prints human text to STDOUT then process.exit(1)s — 9 commands' --json face 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 of os diff is unaffected. The full CLI suite is CI's.
  • The new file is derived into the integration tier by the existing predicate (vitest list --project integration lists it), so there is no hand-maintained list to keep in step.
  • Lint, narrowed with its narrowing proven rather than skipped. eslint --no-inline-config --format json over the 2 changed .ts files: 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 one eslint.config.mjs which, in its own words, "never enables type-aware linting (no parserOptions.project, no typed @typescript-eslint rules) 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.
  • File-claim re-check before committing: git ls-remote then a tree-only fetch of all claude/* branches; 428 refs scanned, exactly one touches packages/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 session and gh is 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 (created 2026-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

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

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/kernel/services-checklist.mdx (via os diff (command, read off packages/cli/src/commands/diff.ts))
  • content/docs/protocol/backward-compatibility.mdx (via os diff (command, read off packages/cli/src/commands/diff.ts))
  • content/docs/protocol/objectql/schema.mdx (via os diff (command, read off packages/cli/src/commands/diff.ts))
What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 e75a9040b02b835ecfab00e1af878f5d7d6cf028packageMentionDocs.

Which tree this was computed on

This run read content/docs from ee28e6a6451b100c491d737ca4489544356154ce — the merge of head f27192da2bd7540f921ebefe89ab15924a5f9dbe into base e75a9040b02b835ecfab00e1af878f5d7d6cf028, 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 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

⚠️ 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 e75a9040b02b835ecfab00e1af878f5d7d6cf028 → 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 5, 2026
@os-litant
os-litant marked this pull request as ready for review September 5, 2026 13:15
@os-litant
os-litant enabled auto-merge September 5, 2026 13:15

@os-litant os-litant left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by the domain:cli execution seat (#6024). ⚠️ Recorded as a COMMENT rather than an approving review because GitHub refuses 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-review absent 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

@os-litant
os-litant added this pull request to the merge queue Sep 5, 2026
Merged via the queue into main with commit ad0b3e7 Sep 5, 2026
35 checks passed
@os-litant
os-litant deleted the claude/issue-15697-diff-usage-error-stderr branch September 5, 2026 13:38
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

2 participants