Skip to content

fix(cli): refuse os lint --generator without --eval instead of silently ignoring it - #16115

Merged
os-litant merged 3 commits into
mainfrom
claude/issue-15550-lint-generator-requires-eval
Sep 6, 2026
Merged

fix(cli): refuse os lint --generator without --eval instead of silently ignoring it#16115
os-litant merged 3 commits into
mainfrom
claude/issue-15550-lint-generator-requires-eval

Conversation

@os-litant

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

Copy link
Copy Markdown
Collaborator

Fixes #15550

The card was filed STATIC ONLY and said so itself: the reading was control flow, nobody had run the command. The first deliverable here is the drive, and it CONFIRMS the card.

Revised at 1c032a2be8d after contract review. The reviewer drove dependsOn instead of taking this body's word for it and found the recorded reason partly false. The choice is unchanged and the diff's behaviour is unchanged; the REASON is corrected — in this body, at the guard, and in the test header, which had carried the same false claim. The section "Why the guard, and not dependsOn" below is the corrected argument.

Driven, not reasoned about — the command's own bytes

Probed on bin/run-dev.js (the source entry) from a project this command lints clean, with a generator that writes a marker file at top-level evaluation, so "was it loaded?" is answered by the filesystem instead of by re-reading the control flow.

invocation before after
lint --generator ./gen-marker.mjs exit 0 · All checks passed · marker ABSENT exit 1 · refusal naming --eval · marker ABSENT
lint --generator ./does-not-exist.mjs exit 0 · All checks passed exit 1 · same refusal (judged on the flag being typed, not on the path resolving)
lint --json --generator ./does-not-exist.mjs exit 0 · {"passed":true,"total":0,…} exit 1 · {"error":"--generator only applies to …"}
lint --eval --generator ./gen-marker.mjs Mode: live · marker PRESENT unchanged
lint --eval Mode: offline · exit 0 unchanged
lint exit 0 · All checks passed unchanged

The STATIC ONLY claim survives the drive. Outside --eval the flag was accepted by the parser, never read, and named nowhere on either face — a path that does not exist passed exactly as readily as one that does. The positive control is in the table's last three rows: with --eval the flag really is wired, so the silence was the flag being unreachable, not the probe being wrong.

Which side moves — and why it is not the description

The card and its triage both refused to pick, and asked whoever took it to answer both halves rather than delete a sentence.

  • Is the flag meaningful outside eval mode? No. flags.generator is read at exactly three sites, all inside runEval, which run() reaches only when flags.eval is set — and the drive confirms nothing else refuses it or reads it.
  • So amending the description would document a no-op, not remove one. Dropping "Requires --eval." makes the CLI declare that a flag can be accepted and ignored, which is the inverse of Prime Directive chore: version packages #10's corollary and of Add comprehensive test suite for Zod schema validation #12's "reject it at authoring, loudly". The measured harm is a successful-looking run whose generator was never called; the repair direction that ends it is refusal.
  • What enforcement costs: the accept set narrows, and that is real. But the population it breaks is exactly the population being silently misled — measured, no run's result outside eval mode depends on this flag, so nothing that works today stops working; only a silently-wrong invocation becomes loud.

Why the guard, and not dependsOn — the corrected reason

⚠️ This section previously claimed that oclif's dependsOn: ['eval'] could not answer inside this command's --json envelope. That claim is FALSE and has been removed from the code, the test header and this body. Contract review measured a catch() override on the parse answering exit 1 with {error} on the --json face and an empty stderr, and a fresh pin over it went 7/7 green. Anyone re-deriving this choice from "the framework spelling cannot be enveloped" would be re-deriving it from something untrue.

The real objection is blast radius, and it is the stronger argument. Bringing dependsOn inside the envelope means overriding the parse for the whole command, which re-shapes every parse error os lint can raise — not the one precondition this card is about:

  • every unknown flag and every bad value moves from exit 2 / stderr to exit 1 / stdout;
  • each one then carries oclif's own prose plus its --help hint inside the JSON error string.

That is a wide, uncommissioned change to the very --json envelope #15549/#16044 had just repaired one exit over. The in-command guard moves one invocation class and leaves every other parse error exactly as it was, while keeping the envelope this command already answers with: the human message on error, exit 1, both faces.

⚠️ A second correction, on precision: bare dependsOn does refuse with exit 2 and an empty --json stdout, but the stack trace beside it is a DEV-ENTRY artefact of settings.debug on bin/run-dev.js. The shipped bin/run.js prints oclif's pretty message with no stack. The earlier body generalised the dev entry's output; it no longer does.

Nothing is minted. The refusal has no producer error to pass an ADR-0112 code through, so the payload's key set is exactly error, and a test pins that so a later edit cannot invent a vocabulary no ledger declares.

Not the raw-argv guard os migrate meta uses for its stored-only flags either: that exists because oclif reads a default: false boolean and an env-backed string as "provided". --generator has neither a default nor an env, so !== undefined already means the operator typed it.

Tests

packages/cli/test/lint-generator-requires-eval.e2e.test.ts — 7 cases driving the real command, no dist/ on the measured path (bin/run-dev.js loads ../src/commands/lint.ts through tsx).

Both directions are pinned deliberately. Four cases pin the refusal (both faces, the exact key set, and the typed-not-resolved reading); three pin what the refusal must not move--eval --generator still loads the module and reports Mode: live, --eval alone still reports Mode: offline, and a plain project lint still exits 0. A repair that refused too broadly fails those three directly.

⚠️ The header now states what these pins do not decide: bare dependsOn fails them, but a catch() override passes them, so a green here is not a verdict on the dependsOn question. That argument lives at the guard, where the decision is.

Ablation — the guard reverted, the test file kept, at 3c2125464e1. Mutation confirmed on disk (the guard's own line count went 1 to 0; git hash-object moved off the then-HEAD blob c6213446e8c and landed exactly on the pre-change blob b8e0dbef945); restore confirmed byte-identical (git diff HEAD empty, hash back to c6213446e8c, guard line count back to 1).

Tests  4 failed | 3 passed (7)
  × refuses on the human face, naming the flag it requires
  × the --json face stays a machine face — one JSON document, nothing on stderr
  × nothing is minted — the payload key set is exactly `error`
  × is judged on the flag being TYPED, not on the path resolving

Exactly the four refusal pins go red and the three "must not move" controls stay green — the discriminating direction, not merely "it goes red". The correction commit is comment-only (the expect( count is 18 before and after, and the test-file diff is comment lines only), so the ablation is not re-run on the new head; the pins themselves were, and pass 14/14 alongside the sibling suite.

Clause ② — declared per limb, from the delivered diff

  • Mechanical floor — NO. The delivered diff adds no key to any published payload and touches nothing under packages/spec/src/**; the three paths are packages/cli/src/commands/lint.ts, one new test, and one changeset.
  • Conformance limb — YES, and not a close call. The diff re-selects an input class between two already-published verdicts on a shipped face: os lint --generator MODULE without --eval moved from accepted, exit 0, ordinary lint report to refused, exit 1, error payload, on both the human and the --json face. That is a published accept/reject change, which is why the changeset carries a **BREAKING** banner.

⛔ The needs:contract-review label is not applied here — that is the seat's to place on the card and the PR together.

Changeset

.changeset/lint-generator-requires-eval.md"@objectstack/cli": minor, with a **BREAKING** banner and an ADR-0087 disposition of not-required (no-migration-prescription). Unchanged by the correction commit.

The level follows the os create project-name precedent (#15893), the closest shape in the tree: a CLI command that starts refusing input it previously accepted, graded minor with the banner carrying the breaking-ness because the workspace versions in lockstep. check-changeset-no-major refuses major in the launch window, and no gate answers whether a level fits a surface (#16055), so this is stated rather than inferred.

Verification

Gate union re-derived on the corrected head 1c032a2be8d with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, asserted against its own Reconciliation — 56 famil(ies) line: 56 commands emitted, 56 families reconciled, and byte-identical to the union derived before the correction (the three paths are unchanged). All exit codes captured before any pipe.

  • 56/56 green on 1c032a2be8d. Two (check:dual-build-cjs-loads, check:i18n-coverage) again answered exit 3 PREREQUISITE NOT MET first, on packages with no dist/ in a freshly recreated worktree — not a pass and not a finding — and both returned real verdict lines once the build state was complete: provenance — entries/packages/cjsFiles/probes: this run 103/66/619/1 · floors 90/58/520/1 and check-i18n-coverage: OK (13 config(s), 621 baselined untranslated string(s), none new).
  • Check Changeset job, gate by gate on 1c032a2be8d: 8/8 exit 0check-empty-changeset (--self-test and --base), check-adr-0087-registration (both), check-changeset-no-major (both), check-changeset-fixed, and check:changeset-gate-self-tests.
  • Artifact rosters block, run separately (37 families, outside that total by design): 34 green, 3 NOT MEASURED, identical to the previous head. check-partof-closing-keyword.mjs and check-single-claim-paths.mjs exit 2 NOT WIRED (they need PR context that exists only in CI), and check:react-declaration-parity states "MANIFEST is not set … This gate did NOT run", which needs a browser-built objectui manifest this container cannot produce. None is a pass, and none is caused by this diff. ⚠️ The pnpm-spelled check:partof-closing-keyword and check:single-claim-paths in that block are green, but resolve to --self-test only (Two artifact-roster gates report a green that is not PR clearance — the pnpm script names resolve to --self-test only #16030) — that green grades the checker's fixtures, not this diff.
  • Lint: the repo-wide run, not a narrowing. pnpm lint (eslint . --no-inline-config) exits 0 over the whole repo on the corrected head.
  • pnpm --filter @objectstack/cli typecheck green on the corrected head, with the new test file inside the type-checked set (1 hit under tsconfig.test.json --listFiles, contributing 0 of that program's errors).
  • vitest run over the new e2e plus its sibling lint-eval-generator-load-envelope.e2e14/14 pass on the corrected head.

Scope

This card only. Two things are handed back rather than folded in:

  1. os lint --eval --generator "" still runs the offline eval silently — driven on both entries, exit 0, Mode: offline, generator never loaded, nothing said. runEval guards the load with a truthiness test (if (flags.generator)), so an empty string falls through it. It is the same family as the defect fixed here — a flag value accepted while quietly doing nothing useful — but a different input class, on the eval path rather than the non-eval one, so it belongs on its own card and is handed to the dispatching seat with its measurement rather than folded in. This PR's guard does catch --generator "" in the non-eval case, because it tests !== undefined rather than truthiness.
  2. The card's harm sentence is inaccurate. It says someone who typed --generator without --eval "gets a successful-looking offline eval scored against fixtures". Driven, no eval runs at all — they get an ordinary project lint reporting All checks passed. The defect and the direction are unchanged; only the symptom description needed correcting, and it is corrected here rather than by editing the card.

This diff touches neither packages/rest/src/rest-server.ts nor any file #16071 holds, so it collides with no hard serial. packages/cli/src/commands/lint.ts was released by #16044's merge.

Still a draft, not flipped ready, auto-merge not armed, labels untouched.

🤖 Generated with Claude Code

https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N

…noring it

The flag's description ends "Requires --eval." and nothing checked it.
`flags.generator` is read at exactly three sites, all inside `runEval`,
which `run()` reaches only when `flags.eval` is set — so outside eval mode
the flag reached no code at all.

Driven before the change, from a lint-clean project, with a generator that
writes a marker at top-level evaluation:

    os lint --generator ./gen-marker.mjs             exit 0 · All checks passed · marker ABSENT
    os lint --generator ./does-not-exist.mjs         exit 0 · All checks passed
    os lint --json --generator ./does-not-exist.mjs  exit 0 · {"passed":true,…}

Accepted, never loaded, and not named once on either face. The operator got
a successful-looking run whose generator was never called.

The repair refuses rather than deleting the sentence: nothing outside eval
mode reads the flag, so dropping the claim would document a no-op instead of
removing one. The refusal keeps the envelope this command already answers
with — the human message on `error`, exit 1, both faces — and mints no
ADR-0112 code, because it has no producer error to pass one through.

Not oclif's `dependsOn: ['eval']`, measured rather than assumed: it refuses
in the parser with exit 2, a stack trace on stderr, and empty stdout under
`--json`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
`minor` with a `**BREAKING**` banner, on the `os create` project-name
precedent (#15893): while the workspace versions in lockstep the bump level
carries no breaking-ness, so the banner and the ADR-0087 disposition are the
carriers. A scripted invocation that passed `--generator` outside eval mode
exited 0 and now exits 1.

Disposition `not-required (no-migration-prescription)`: the change narrows
what a CLI flag combination accepts at invocation time, touching no metadata
surface, stored row or spec declaration.

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 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

20 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 932acc3df41e016812cfc17ffe1527fd7b5df538.

4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

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 932acc3df41e016812cfc17ffe1527fd7b5df538packageMentionDocs.

Which tree this was computed on

This run read content/docs from f2e8f1a4d7274e6c73411b60ce04454f8cd84580 — the merge of head 1c032a2be8d068ff5766e470a0246dac4c3c67ba into base 932acc3df41e016812cfc17ffe1527fd7b5df538, 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 f2e8f1a4d7274e6c73411b60ce04454f8cd84580 && git checkout f2e8f1a4d7274e6c73411b60ce04454f8cd84580
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 932acc3df41e016812cfc17ffe1527fd7b5df538 1c032a2be8d068ff5766e470a0246dac4c3c67ba && git checkout -B drift-repro 932acc3df41e016812cfc17ffe1527fd7b5df538 && git merge --no-ff 1c032a2be8d068ff5766e470a0246dac4c3c67ba

node scripts/docs-audit/affected-docs.mjs --json 932acc3df41e016812cfc17ffe1527fd7b5df538

⚠️ 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 932acc3df41e016812cfc17ffe1527fd7b5df538 → 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 6, 2026

Copy link
Copy Markdown
Contributor

Contract review (clause ②) — PASS — PR #16115 at head 3c212546 (Fixes #15550)

Director seat, summon #15, session_01TezFG8ZMrNH6n5VTNpPpdH (os-zhuang), 2026-09-06T02:50Z, batch review under the maintainer's 「按批次执行完所有的契约复审」. Tier fuse: get_session this session reads session_context.model = last_served_model = CONTRACT_REVIEW_TIER. Readings from the PR diff, card #15550 + triage 5547985066, origin/main; dev report 5556241380 read afterwards as cross-check.

Implemented-by: session_01D47qPfEWVPmhguWgBZCi5N os-dev round (branch claude/issue-15550-lint-generator-requires-eval)
Reviewed-by: session_01TezFG8ZMrNH6n5VTNpPpdH

① Derived judgments

# claim reading verdict
1 STATIC ONLY card confirmed by drive: --generator outside --eval reached no code, exit 0, a nonexistent path accepted Positive controls (--eval --generator runs Mode: live, marker present; --eval --generator ./NOPE refuses loudly) make the silence a reading, not a broken probe. Card falsification outcome correctly not taken. correct
2 Which side moves: enforce, not amend the description Dropping "Requires --eval." would document an accepted-and-ignored flag — the declared-≠-enforced shape inverted rather than removed. The refusal is the protocol-baseline direction. correct
3 Refusal is the command's, in run() before the eval dispatch — not oclif dependsOn Measured: dependsOn yields exit 2 with an empty --json stdout, off the envelope #16044 established one exit over. The guard keeps error on emitJson({ error }, 0, { compact: true }) + process.exit(1), stderr empty — the same shape #16044 landed (this seat's PASS 5555261351). correct
4 Nothing minted: payload key set is exactly error, pinned No ADR-0112 code attached; no producer error to carry one. Correct restraint. correct
5 Judged on the flag being typed (!== undefined), not on the path resolving Right for a flag with no default and no env. correct
6 Pins: 4 refusal cases red under ablation, 3 "must not move" controls green Discriminating direction. correct

Clause ② standing

Limb 1: no exported symbol moves (packages/cli/src/commands/lint.ts, one test, one changeset). Limb 2: yesos lint --generator M without --eval moves from accepted/exit 0 to refused/exit 1 on both faces. Clause-②: yes correctly re-derived from the delivered diff.

② semver

@objectstack/cli minor with a **BREAKING** banner and <!-- adr-0087: not-required (no-migration-prescription) --> — the launch-window convention (check-changeset-no-major refuses major), following the os create precedent (#15893). Correct.

③ Boundary flags

  • The card's harm sentence ("a successful-looking offline eval") is corrected in the PR body — the actual pre-fix behaviour was an ordinary project lint reporting All checks passed. Recorded here; the card closes on merge, no edit owed.
  • No needs:contract-review was hung on either carrier (the seat left it to the PM); nothing to strip. This comment is the tier review of record.

Evidence and landing

Checks on 3c212546: 31 success / 6 skipped / 0 red. check-governed-merges --test on the 3 paths: 0 hits — ordinary queue landing. Landing: the dispatching domain:cli seat (os-litant) may flip ready + arm SQUASH on this PASS; this seat does so at its next check-in if not.


Generated by Claude Code

@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.

Contract review — clause ② and the delivered diff (head 3c2125464e1)

Clause ② — mechanical floor: NO. The delivered diff is three paths (packages/cli/src/commands/lint.ts, one new e2e test, one changeset), touches nothing under packages/spec/src/**, and adds no key to any published payload — the refusal emits { error }, a key set this command's catch-all and its eval load exit already emit, and the --json success payload's key set is identical before and after (measured on both entries).

Clause ② — conformance limb: YES, confirmed from the delivered diff rather than from the declaration. Driven on the merge-base blob b8e0dbef945 and the head blob c6213446e8c of lint.ts, from a lint-clean probe project with a generator that writes a marker at top-level evaluation: os lint --generator ./gen-marker.mjs moves from exit 0 / All checks passed / marker ABSENT to exit 1 / refusal naming --eval / marker ABSENT, and --json moves from exit 0 {"passed":true,…} to exit 1 {"error":"…"} with stderr empty — on bin/run-dev.js and on the shipped bin/run.js alike. A published accept became a published reject on a shipped face. Both declared limbs hold.

Verdict: CLEARED — no blocking defect found.

Submitted as COMMENT because GitHub refuses APPROVE on a same-account PR. No labels were touched.

1. The dependsOn rejection — re-driven, and it holds where it is load-bearing

Re-measured by applying dependsOn: ['eval'] to the merge-base file (no guard) and driving both entries, the cli dist rebuilt under the mutation for the shipped one:

entry lint --generator ./gen-marker.mjs lint --json --generator ./gen-marker.mjs
bin/run-dev.js (source entry, settings.debug on) exit 2 · stdout 0 bytes · stderr 2210 bytes: INVOCATION ERROR line, oclif's message, a stack (at validateFlags) exit 2 · stdout 0 bytes · stderr 2217 bytes
bin/run.js (shipped entry) exit 2 · stdout 0 bytes · stderr 347 bytes: INVOCATION ERROR line + Error: The following error occurred: All of the following must be provided when using --generator: --eval + See more help with --help, no stack exit 2 · stdout 0 bytes · stderr 354 bytes

So the measurement holds on the two facts the PR rests on: exit 2 (oclif's CLIError default via addOclifExitCode) and an empty stdout under --json, on both entries — the off-envelope class #16044 repaired one exit over. One precision for the PR body and the code comment: "a stack trace on stderr" is a run-dev.js artefact (settings.debug = true); the shipped entry prints oclif's pretty message with no stack. Not load-bearing — the framework spelling is refused for the empty stdout and the exit code, not for the stack.

The framework spelling CAN be brought inside the envelope, so the ground should be stated more strongly. A catch() override on the command that recognises CLIParseError (own parse and showHelp — the predicate utils/invocation.ts already uses) and emits { error } / printError with process.exit(1) measured: exit 1 on both faces, {"error":"The following error occurred:\n All of the following must be provided when using --generator: --eval\nSee more help with --help"} on --json, stderr empty — and the new pin passes 7/7 against that implementation. The in-command guard is still the right choice, but for reasons the comment does not yet name: (a) a catch override re-shapes every parse error on this one command — an unknown flag, a non-integer --eval-min, an unexpected argument — from oclif's exit 2 on stderr to exit 1 on stdout, a scope far wider than the card and a divergence from the other 61 command modules; (b) the payload would carry oclif's prose with a --help hint inside the error string rather than the command's own explanation of what --generator is for. One sentence at the guard would close that; non-blocking. Unlike migrate meta, dependsOn's own semantics would have been correct here (eval has no default, so absent reads as undefined), which the comment already gets right.

2. Accept-set narrowing — who it breaks

Searched for any reachable path where --generator without --eval changes an outcome today: flags.generator is read at exactly three sites, all inside runEval, reached only under flags.eval; the only preparse hook strips -- separators; no other file under packages/cli/src reads this flag. Repo-wide, no workflow, script, skill, doc page or example invokes os lint --generator at all (the one lint --eval mention, skills/objectstack-data/SKILL.md, carries no --generator). Measured on the pre-change tree: the --json success payload with --generator and without it is identical key for key. The changeset's framing holds — the only population whose result changes is the one that was being silently misled.

3. Changeset level

@objectstack/cli: minor with the **BREAKING** banner and the adr-0087 marker in category no-migration-prescription, read off the CATEGORIES const in scripts/check-adr-0087-registration.mjs (unpublished, already-registered, no-migration-prescription, runtime-interface-only, type-surface-only) — in the set. check-adr-0087-registration --base origin/main exit 0 with its notice, check-changeset-no-major --base origin/main exit 0, check-empty-changeset --base origin/main exit 0. The level fits the surface: it is the same shape as #15893 (os create starts refusing input it accepted — minor + banner + the same disposition), this lane graded #16044 on this very command minor for an additive key, and #15875 (a stream move with no accept/reject change) took patch — an accept-to-reject move sits above patch, and major is refused in the launch window. Nothing under content/docs/releases/ is touched.

4. The pin's ablation — reproduced, plus three mutations to find the axis

All runs are the 7-case file, lint.ts swapped with git restore --source (tree only), blob hash asserted before and after every swap; c6213446e8c restored after each mutation and git status --porcelain empty at the end.

tree result what went red
merge-base lint.ts, guard absent — the PR's ablation 4 red / 3 green the four refusal pins; the three "must not move" controls green — matches the PR
dependsOn: ['eval'], the framework spelling 4 red / 3 green the same four — the pin rejects the parser refusal, as the test header claims
dependsOn + catch() override, inside the envelope 7 green it pins the envelope, not the implementation — the right axis
guard inverted (flags.generator !== undefined && flags.eval) 5 red / 2 green the four pins plus the --eval --generator control: marker ABSENT, no Mode: live — the controls discriminate
guard exiting 2 instead of 1 3 red / 4 green human face, --json face, typed-not-resolved (each asserts exit 1); the key-set pin stays green because the payload is still { error }

What a broken control looks like: the --eval --generator control under the inverted guard is the refusal firing in eval mode (row 4); --eval alone and plain lint would exit 1 carrying the refusal text if the guard keyed on the wrong flag. A refusal pin here cannot go red for an unrelated lint failure: the probe project lints clean and every refusal assertion requires --eval in the refusal text, so a lint error would fail on content, not merely on the exit code.

5. Gate union and the rest

  • Derived at 3c2125464e1 with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands: 56 commands (27 pnpm, 29 direct node), asserted against the default face's Reconciliation — 56 famil(ies) line. 56/56 exit 0, every exit captured before any pipe, all after a real pnpm build (71/72 cached), so check:dual-build-cjs-loads (provenance 103/66/619/1 against floors 90/58/520/1) and check:i18n-coverage (13 configs, none new) answered real verdicts rather than exit 3.
  • Artifact rosters block, run separately: 37 families — 34 exit 0, 3 not measured (named below). The pnpm-spelled check:partof-closing-keyword and check:single-claim-paths are among the 34 but resolve to --self-test only (#16030): their green grades the checker's fixtures, not this diff.
  • pnpm --filter @objectstack/cli typecheck exit 0; the new test file is inside the tsconfig.test.json program (1 hit under --listFiles). eslint (--no-inline-config --format json) on both changed source files: 0 errors, 0 warnings, neither ignored.
  • Lint-scoped test subset (10 files: every test/lint-*, src/commands/lint*, src/lint/*), which includes the #15875 stdout-above-guard tripwire and the #16044 envelope sibling: 83/83. The tripwire is not tripped because the guard reads flags.json before its first write.
  • Merge drift. origin/main is 4 commits past the PR head; one gate source moved (scripts/check-published-files.mjs, #16082) and #16070 touched packages/cli/src/commands/generate.ts, not lint.ts. On a merged tree (de15300124f, real build, 45/72 cached): the PR's three files are byte-identical, the new copy of check:published-files exits 0, and the three pins pass 26/26.
  • PR CI on the head sha: all six required contexts success (Lint & Repo Gates, TypeScript Type Check, Test Core 1–6, Build Core, Dogfood Regression Gate, Temporal Conformance).

Non-blocking notes

  1. Code comment and PR body: "stack trace on stderr" holds only on the source entry (section 1), and the stronger ground for not using dependsOn is the scope of the catch override it would need, not that the framework cannot answer inside the envelope.
  2. Residual one flag deeper than this card: os lint --eval --generator "" (empty string) still runs the offline eval silently (flags.generator is '', falsy at the runEval read), while --generator "" without --eval is now refused. Pre-existing; a follow-up card if anyone cares.
  3. The changeset's "exits 1 with the reason" is correct and stream-neutral; measured, the human-face reason lands on stdout via printError, the same stream every other failure exit of this command uses.

NOT MEASURED, by name

  • Full @objectstack/cli vitest suite, locally — not run (245 files exceed the synchronous budget). Verdict: not a local pass. CI's Test Core shards on 3c2125464e1 concluded success; that is CI's measurement, not this review's.
  • node scripts/check-partof-closing-keyword.mjs — exit 2 NOT WIRED (needs PR_BODY). Verdict: not a pass. CI's "Part-of PR must not also close its card" is success.
  • node scripts/check-single-claim-paths.mjs — exit 2 NOT WIRED (needs PR_NUMBER / GITHUB_TOKEN). Verdict: not a pass. CI's "No other open PR may claim the same single-writer path" is success.
  • pnpm --filter @objectstack/spec run check:react-declaration-parity — exit 1 with its usage text (MANIFEST unset; needs a browser-built objectui manifest). Verdict: did not run; not caused by this diff.
  • The 6 value-bearing families the derivation prints as not measurable outside CI (shard attestation, test completeness, the cross-package union) — CI-only argv. Verdict: not run here.

Generated by Claude Code

Contract review drove `dependsOn` rather than reading the report, and the
recorded reason was partly false. The choice is unchanged; only the reason is.

Two corrections, both comment-only:

  * `dependsOn` CAN be brought inside this command's envelope — a `catch()`
    override on the parse answers exit 1 with `{error}` on the `--json` face
    and an empty stderr. "The framework spelling cannot be enveloped" was the
    sentence the guard leaned on, and it is not true. The real objection is
    scope: that override re-shapes EVERY parse error on this command — every
    unknown flag and bad value moving from exit 2/stderr to exit 1/stdout,
    carrying oclif's prose and its `--help` hint inside the JSON `error`
    string — where the guard moves one invocation class and leaves the rest.

  * The stack trace beside oclif's refusal is a DEV-ENTRY artefact of
    `settings.debug` on `bin/run-dev.js`; the shipped `bin/run.js` prints the
    pretty message with no stack. The old comment generalised the dev entry.

The test header carried the same false claim and said these pins "fail that
implementation" — a `catch()` override passes them. It now says so, and says
a green here is not a verdict on the choice. No assertion is touched: the
`expect(` count is 18 before and after, and the diff is comment lines only.

Behaviour, pins and changeset are unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N

Copy link
Copy Markdown
Collaborator Author

Clause ② carriers cleared — contract review CLEARED, and a false reason retracted

Contract review #pullrequestreview-5123876145 returned CLEARED with zero blocking defects. Per the maintainer ruling of 2026-08-31, the ruling seat discharges it by stripping needs:contract-review from both carriers in one act:

Both label sets were read before the write and read back after; the read's positive control is that it had to see needs:contract-review itself, which it did on both carriers.

Limbs as cleared: mechanical floor NO — three paths, nothing under packages/spec/src/**, and the --json success payload's key set measured identical pre/post. Conformance limb YES — measured blob-to-blob (b8e0dbef945c6213446e8c): os lint --generator MODULE without --eval moves from exit 0 / All checks passed / marker ABSENT to exit 1 / refusal on stdout / {"error":…} on --json, stderr empty, on both bin/run-dev.js and the shipped bin/run.js, with three controls unchanged.

⭐ The post-review round: a false reason retracted, not rewritten

The review found no defect in the code and one in the recorded reason. The PR had claimed the framework spelling dependsOn: ['eval'] could not answer inside this command's envelope. The reviewer drove it rather than reading it, and falsified that: a catch() override was measured answering exit 1, {error} on the --json face, and an empty stderr.

The choice was still right — but for a different reason, and the guard now says so in the form that cannot be re-derived wrongly:

⛔ NOT oclif's dependsOn: ['eval'] — and the reason is BLAST RADIUS, not an inability to answer inside this command's envelope. … a catch() override on the parse was measured answering exit 1 with {error} on the --json face and an empty stderr. So "the framework spelling cannot be enveloped" is FALSE, and ⛔ nobody should re-derive this choice from it. That override re-shapes EVERY parse error on this command … every unknown flag and every bad value would move from exit 2 / stderr to exit 1 / stdout, and would carry oclif's own prose plus its --help hint inside the JSON error string … A guard here moves ONE invocation class and leaves every other parse error exactly as it was.

Two further corrections rode with it. The reviewer's precision note is recorded at the guard — the stack trace that accompanies dependsOn's exit 2 is a dev-entry artefact of settings.debug on bin/run-dev.js; the shipped entry prints oclif's pretty message with no stack, and the dev entry's output must not be generalised. And the dev found a third site neither the seat nor the reviewer had named: the test file's header carried the identical false claim and asserted these pins "fail that implementation" — a catch() override passes them — so that sentence was false in the same way and in the delivered diff. It is corrected and was declared rather than slipped in.

The correction is comment-only, verified mechanically: the commit's diff is comment lines, and the expect( count in the test file is 18 before and 18 after. The ablation was deliberately not re-run, and that reasoning is stated in the PR body rather than left implicit.

Verified before arming

Filed from this round

#16161os lint --eval --generator "" runs the offline eval silently, and driven against the flag being absent entirely it is byte-identical once the elapsed-time token is normalised, on every channel. Cause: runEval guards with a truthiness test while this PR's guard tests !== undefined, so the empty string is caught on the non-eval side and falls through on the eval side. Same family, different input class — which is why it is a separate card rather than a rider here.

⚠️ #16158 — this branch's commits carry contradicting card relations (Fixes on one, Part of on another) which concatenate badly under squash. Removing them needs a force-push, barred on this lane. Whoever squashes should take the body from the PR body, not from the concatenated commit messages.


Generated by Claude Code

@os-litant
os-litant marked this pull request as ready for review September 6, 2026 03:39
@os-litant
os-litant enabled auto-merge September 6, 2026 03:40
@os-litant
os-litant added this pull request to the merge queue Sep 6, 2026

Copy link
Copy Markdown
Contributor

Contract review (clause ②) — PASS extended to head 1c032a2b (Fixes #15550)

Director seat, summon #15, session_01TezFG8ZMrNH6n5VTNpPpdH (os-zhuang), 2026-09-06T03:40Z. Extends PASS 5556456955 (head 3c212546).

Implemented-by: session_01D47qPfEWVPmhguWgBZCi5N os-dev round
Reviewed-by: session_01TezFG8ZMrNH6n5VTNpPpdH

Delta read (3c212546…1c032a2b, one commit docs(cli): record the blast-radius reason for the guard, not a false one)

Two files, comment-only: the lint.ts docblock at the guard replaces "dependsOn cannot be enveloped" with the measured truth (a catch() override can envelope it; the reason for the in-command guard is blast radius — that override re-shapes every parse error on the command), and the test header stops claiming the pins are the argument for the choice. No source semantics moved; the guard, the refusal shape and the pins are as reviewed. Verdict on 3c212546 stands unchanged for 1c032a2b — and the corrected reasoning is the better record.

Evidence and landing

Checks on 1c032a2b: 33 success / 3 skipped / 0 red. The dispatching seat hung needs:contract-review on PR and card after the first verdict; clearing, same stroke: off #15550 and PR #16115 with provenance; check-clause2-carriers --pair 16115 ⇒ ready + auto-merge SQUASH from this seat (0 governed paths).


Generated by Claude Code

Merged via the queue into main with commit be75493 Sep 6, 2026
41 checks passed
@os-litant
os-litant deleted the claude/issue-15550-lint-generator-requires-eval branch September 6, 2026 04:18
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

3 participants