Skip to content

fix: code-graph --api-policy: an unexpected error may exit 1, the same code as a ratchet miss - #628

Merged
usirin merged 2 commits into
mainfrom
build/625-api-crash-exit-code-d285de4e
Oct 11, 2026
Merged

usirin merged 2 commits into
mainfrom
build/625-api-crash-exit-code-d285de4e

Conversation

@usirin

@usirin usirin commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

code-graph --api (alone, with --api-base, or with --api-policy) used to end a crash with a Node stack and exit 1, the same code the ratchet uses for a miss. A CI job reading only the exit code could not tell the two apart.

Now any error in the run that is not a refused input prints one line, code-graph: unexpected error: <message>, with no stack and nothing on stdout, and exits 3. An --out file that cannot be written is one of these, so it exits 3, not 2. Exits 0, 1 and 2 are unchanged.

  • packages/code-graph/src/api/cli.ts: runApiView no longer rethrows; it reports the line and resolves to API_CRASH_EXIT (3).
  • packages/code-graph/src/api/crash.test.ts: the real command line on all 11 fixture branches (plain exit kept, then twice with a bad --out), the view and the diff with a bad --out, and runApiView with a throwing emit on view, diff and ratchet.
  • packages/code-graph/src/engine/tsgo.ts: closing a tsgo session now releases its open snapshots and sends the server SIGKILL before the API's own close. The server shares this process's stderr, and the API's close (stdin closed, then SIGTERM) could let it print context canceled there, which is the second line CI saw. src/engine/tsgo.test.ts pins the order.
  • packages/code-graph/SPEC.md §13.6 and the "Published API" section of docs/reference/cli.md name exit 3, and say the emit's diagnostics warning stays its own line.
  • A patch changeset for @demlik/code-graph.

The exit code changes for the --api mode only. The other modes (--tree --out and the --ci gates) still crash with exit 1, as the issue leaves out of scope.

Run here: pnpm typecheck and pnpm lint from the root, and the whole packages/code-graph suite (75 files, 1075 tests), all passing. I did not run the root pnpm test; CI answers that.

The stray line did not reproduce on this machine: 160 runs of the real command line at the old head, on macOS, printed nothing extra. The fix rests on what SIGKILL is, a signal the server cannot handle, and on the order test.

Fixes #625

Deviations

  • Declined guidance — Said: handle the rejection of the --api run in packages/code-graph/src/index.ts. Did: caught the error one level down, in runApiView in src/api/cli.ts, and left index.ts as it was. Why: runApiView already maps a refused input to exit 2 there, and its emit is injected, so one place owns every exit of the mode and a unit test can reach it. Disposition: stated here.
  • Guard or gate bypassed — Said: fabrika build check must be green before a push. Did: pushed with build check red on the codeowners-cp guard alone, on both the code and prose surfaces, in both rounds. Why: the guard lists paths this repo does not have (packages/fabrika-cli/src/ci/, biome-plugins/ and four more) and this diff does not touch .github/CODEOWNERS; it is the known upstream fault in build check's codeowners-cp step fails on paths demlik doesn't have #289. Disposition: stated here; the repo's typecheck and lint and the code-graph tests were run by hand and pass.
  • Out-of-scope change — Said: the issue covers the --api mode only. Did: changed how every tsgo session closes, in src/engine/tsgo.ts, so the modes that read types through it (--edges, --find, --blast and the rest) end the server the same way. Why: the stray stderr line comes from the server every mode shares, and src/engine/tsgo.ts is the one module that owns it; a fix inside --api alone would leave the same line in the other modes. Disposition: stated here; the changeset names it.
  • Out-of-scope change — Said: code-graph reads tsgo through its API. Did: killServer reads the server's child process off two private fields of the pinned tsgo build (client.channel.child). Why: the build spawns the server with stderr inherited and offers no option for it and no handle on the process. Disposition: stated here; a pin that moves those fields throws at close, so every test that reads types fails rather than the line coming back.
  • Scope narrowing — Said: a crash prints exactly one code-graph: stderr line. Did: left the emit's diagnostics warning (code-graph: warning: tsgo reported N diagnostic(s)) in place, so a crash on a package with type errors prints that line and then the crash line. Why: the warning is §13.3's own documented line and is true whether or not the run then crashes; the fixture has no diagnostics, so the first criterion's run prints one line. Disposition: stated here and in SPEC §13.6 and the CLI reference.

🤖 Generated with Claude Code

#625)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@usirin

usirin commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

review-code: FAIL @ 6f4907a content:d3965bda7ae1 — required CI is red at head: the crash path prints two stderr lines

What has to change

The required check test-and-build is red at this head, on both the pull_request run (38105398220) and the push run (38105394142). Both fail the same new test, on the same branch:

src/api/crash.test.ts > on stable-removed the gate exits 1, and 3 with one line when --out cannot be written, at line 66.

The second gate run with the bad --out exited 3, but its stderr was two lines:

context canceled
code-graph: unexpected error: ENOENT: no such file or directory, open '<repo>/no-such-folder/out.txt'

The first run of the same command printed only the code-graph: line. So the real command line does not always print exactly one stderr line on the crash path. The PR body says the root test run passes; it does not in CI, twice out of two.

Where to look (my reading, not proven): context canceled is not a string in packages/code-graph/src. It reads like the tsgo process writing to the stderr it inherits when program.close() runs (src/api/view.ts:57). The crash path does not cause it, it only exposes it: runCli drops stderr on exit 0, so no earlier test looked. The fix belongs in the product, not in a looser test regex — criterion 1 asks for exactly one line.

Per criterion

  1. [FAIL] Crash vs miss by exit code, all 11 overlays. The exit-code half holds: runApiView (src/api/cli.ts:113-122) returns API_CRASH_EXIT (3) for any non-ApiInputError, and cleanExit then exitWith(3) leaves the process on 3. The fixture covers all 11 overlays and pins the list against branches/ (crash.test.ts:38-58). But "prints exactly one stderr line" is false at this head, per the CI run above.
  2. [PASS on the diff, unproven at head] Non-ApiInputError gives one fixed code and one line, on view, diff and ratchet: crash.test.ts:81-106 drives runApiView with a throwing emit in all three modes, and :69-74 drives the real CLI on view and diff. Those tests passed in CI. crashLine folds whitespace, so a multi-line message stays one line.
  3. [PASS] gate.test.ts and api.test.ts are not in the diff and passed in CI. The exit-2 arm is unchanged (cli.ts:115-118), and crash.test.ts:108-122 pins it.
  4. Doc rows: see review-doc.
  5. [FAIL] The changeset is there (.changeset/code-graph-api-crash-exit.md, patch). pnpm test does not pass: 1 failed, 4134 passed.

Standing checks

  • Test honesty: no existing test changed. Clean.
  • Comment discipline, small: the block comment that describes runApiView now sits above API_CRASH_EXIT, with crashLine between it and the function. Move it back onto the function while you are there.
  • Not on this fixture, but the same promise: src/api/view.ts:87 prints a code-graph: warning: stderr line when tsgo reports diagnostics, so a crash on such a package would also print more than one line. Worth a look in the same repair.

Deviations

Entry Matches the diff?
Caught in runApiView, not src/index.ts Yes. index.ts is untouched, and runApiView no longer rejects, so void apiRun.then(exitWith) has nothing left to drop. Sound.
Pushed with build check red on codeowners-cp Disclosed. Not something this gate can see either way.

Nothing undisclosed that this gate could see.

Verdict-written: 2026-10-11T02:38:41Z

@usirin

usirin commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

review-doc: FAIL @ 6f4907a content:d3965bda7ae1 — the docs promise one stderr line; CI at this head measured two

What has to change

Three doc files state the same runtime fact: a crash prints one stderr line. SPEC.md §13.6 and docs/reference/cli.md say "one stderr line, code-graph: unexpected error: <message>", and the changeset says "prints one line".

At this head that is measured false. Both CI runs (38105398220, 38105394142) show the real command line printing context canceled and then the code-graph: line on the crash path. Details are in the review-code verdict on this PR.

The wording is what the issue asks for, so I expect the text to stand as written once the code makes it true. It cannot pass at a head where the only measurement contradicts it.

Criterion 4

[PASS on the text] SPEC.md §13.6 names exit 3, says when it happens, and says an unwritable --out gets 3, not 2. The "Published API" section of docs/reference/cli.md says the same. Both exit tables carry 3 on all three rows.

Criterion 5, changeset half

[PASS] .changeset/code-graph-api-crash-exit.md is a patch for @demlik/code-graph and names exit 3. The test half is graded in review-code.

Hygiene

  • Right surface: yes. The contract is in SPEC, the lookup copy in the CLI reference, the release note in the changeset.
  • One mode per doc: both edits are reference text inside reference sections. No drift.
  • Supersession and status: nothing replaced, no status lines touched.
  • Claims trace: FAIL, the one-line claim above.
  • Prose craft: short, plain sentences. "So exit 1 always means a ratchet miss, never a crash" is the line a reader needs. Clean.

Verdict-written: 2026-10-11T02:38:58Z

…ng on stderr (#625)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@usirin

usirin commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

Repair round 1, answering the two FAIL verdicts at 6f4907a. The fix is commit b695a2a, already the PR head; this run changed no file and pushed nothing.

How this note comes late: the builder that made b695a2a died after its push and before its note. Two more spawns died too. This run adopted the last one's stranded claim (the lane ledger records it as spawn-dead), re-read the whole diff against the five criteria of the issue, and re-ran the tests.

Findings, each one answered:

  1. review-code, two stderr lines on the crash path (context canceled). Fixed in packages/code-graph/src/engine/tsgo.ts: closing a session now releases its open snapshots and sends the tsgo server SIGKILL before the API's own close, so the server cannot print on its way out. src/engine/tsgo.test.ts pins that order. The test was not loosened. Required check test-and-build is green at b695a2a on both runs; it was red on both at 6f4907a.
  2. review-code, the runApiView comment sat above API_CRASH_EXIT. Moved back onto the function in src/api/cli.ts.
  3. review-code, the diagnostics warning at src/api/view.ts:87 is a second stderr line on a package with type errors. Left in place on purpose, and said so: SPEC §13.6 and the CLI reference now state the warning is a line of its own and comes first. The ratchet fixture has no diagnostics, so criterion 1's run prints one line. It is in ## Deviations as a scope narrowing.
  4. review-doc, the one-line claim was measured false. The text stands; the code now makes it true (see 1).

Look at first:

  • killServer in src/engine/tsgo.ts reads the server's child process off two private fields of the pinned tsgo build. It throws if a pin moves them. Disclosed in ## Deviations.
  • The stray line never reproduced on macOS, before or after. The proof it is gone is CI: 2 of 2 red before, 2 of 2 green after. That is a small sample.

Run in this tree at b695a2a: vitest run src/api src/engine in packages/code-graph, 8 files and 97 tests passing; src/api/crash.test.ts three more times, 18 of 18 each. build check is red on the codeowners-cp guard alone, on code and prose, as the PR body already discloses (#289).

— at b695a2a

@usirin

usirin commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

review-code: PASS @ b695a2a content:c081a2a0e7b8 — crash exits 3 with one line; required CI green at head

What changed since the last round

The round-1 FAIL was a second stderr line, context canceled, on the crash path in CI. The repair (src/engine/tsgo.ts:119-145) kills the tsgo server with SIGKILL before the API's own close. The required check test-and-build is green at this head on both runs (38106267756, 38106270517). It was red on both at the old head, on this same test.

Per criterion

  1. [PASS] Crash vs miss by exit code, all 11 overlays. src/api/crash.test.ts:38-67 drives runGate on a ratchetRepository() repo for each overlay: the plain run keeps its 0 or 1, the bad --out run exits 3, stderr matches one code-graph: unexpected error: ENOENT: … line with no stack, stdout is empty, and a second run gives an equal result. :53-57 pins the list against branches/, and the head holds exactly those 11 folders. CI ran it green.
  2. [PASS] src/api/cli.ts:114-121 maps any non-ApiInputError to one report and API_CRASH_EXIT (3). crashLine (:91-92) folds whitespace, so a multi-line message stays one line. cleanExit sets exit 2 and prints the line, then exitWith(3) overwrites the code (src/cli.ts:55-63), so the process ends on 3. Tests: crash.test.ts:82-106 on view, diff and ratchet with a throwing emit; :69-74 on the real command line for view and diff; :76-78 pins 3 as not 0, 1 or 2.
  3. [PASS] gate.test.ts and api.test.ts are not in the diff and passed in CI. The exit-2 arm is unchanged (cli.ts:115-118), and crash.test.ts:108-122 pins it.
  4. Doc rows: see review-doc.
  5. [PASS] The changeset is there (.changeset/code-graph-api-crash-exit.md, patch, names exit 3). The required CI job runs typecheck, lint and test, and is green at this head.

Behaviour claims I traced

  • "The server writes to our stderr": true at the pinned build 7.0.0-dev.20260707.2. syncChannel.js spawns it with stderr inherit.
  • "API.close closes stdin then sends SIGTERM": true. syncChannel.js close() destroys stdin, then calls child.kill() with the default signal.
  • "client.channel.child exists": true at the pin (api.js:40, client.js:37, syncChannel.js:126). killServer throws if a pin moves it.
  • Order matters: api.close() disposes any snapshot still active, which is a request to the server. close (tsgo.ts:121-126) disposes every live snapshot before the kill, so nothing talks to a dead server. The whole suite green in CI agrees.

Things to know, not blocking

  • The proof that the stray line is gone is 2 green CI runs against 2 red ones, plus the fact that SIGKILL cannot be handled. That is a small sample. The builder could not reproduce the line on macOS.
  • A crash on a package with type errors prints two code-graph: lines: the §13.3 diagnostics warning, then the crash line. Criterion 2 says "one code-graph: stderr line". I read the warning as the emit's line, not the crash's, and the exit code alone still tells crash from miss. It is disclosed and written into SPEC §13.6. The owner can rule otherwise.
  • tsgo.test.ts checks the order of signals with a spy. It proves the mechanism, not the absence of the line; crash.test.ts covers that.

Standing checks

  • Test honesty: no existing test changed. Clean.
  • Release containment: the changeset states the new exit code and the tsgo close change for every mode that reads types. Matches the diff.
  • Comment discipline: the runApiView comment is back on the function. The two new comments in tsgo.ts state constraints the code cannot show. Clean.
  • Staleness traps: none.

Deviations

Entry Matches the diff?
Caught in runApiView, not src/index.ts Yes. index.ts is untouched and runApiView no longer rejects.
Pushed with build check red on codeowners-cp Disclosed. Not something this gate can see.
tsgo close changed for every mode Yes. src/engine/tsgo.ts is the shared module; the changeset names it.
killServer reads private fields of the pinned build Yes, as traced above.
Diagnostics warning stays a second line Yes. src/api/view.ts:84-89 is untouched; the docs say so.

Nothing undisclosed that this gate could see.

Verdict-written: 2026-10-11T02:59:03Z

@usirin

usirin commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

review-doc: PASS @ b695a2a content:c081a2a0e7b8 — docs name exit 3 and CI now measures the one-line claim true

What changed since the last round

The round-1 FAIL was the one-line claim, measured false in CI. The code now makes it true: test-and-build is green at this head on both runs (38106267756, 38106270517), and that job runs the test that checks for exactly one stderr line on all 11 fixture branches. The docs also gained one sentence each on the diagnostics warning.

Criterion 4

[PASS] packages/code-graph/SPEC.md §13.6 names exit 3 on all three table rows, says it is for any error that is not a refused input, and says an unwritable --out exits 3, not 2. The "Published API" section of docs/reference/cli.md (table and the paragraph under it) says the same.

Criterion 5, changeset half

[PASS] .changeset/code-graph-api-crash-exit.md is a patch for @demlik/code-graph. It names exit 3 and the tsgo close change. The test half is graded in review-code.

Hygiene

  • Right surface: yes. The contract is in SPEC, the lookup copy in the CLI reference, the release note in the changeset.
  • One mode per doc: all three edits are reference text inside reference sections. No drift.
  • Supersession and status: nothing replaced, no status lines touched.
  • Claims trace: the one-line claim is backed by the CI run above. "The diagnostics warning is a line of its own and comes first" traces to src/api/view.ts:84-89, which warns before the names are read. In cli.md, "the diagnostics warning above" points at the same section's opening paragraph, which does describe it; SPEC points at §13.3, which does too. The changeset's context canceled claim rests on 2 green runs against 2 red ones, a small sample, stated as such in the PR body.
  • Prose craft: short, plain sentences. Nothing needs a second read. Clean.

One thing to know: on a package with type errors a crash prints two code-graph: lines, the warning and then the crash line. The docs say this plainly, so they are true as written.

Verdict-written: 2026-10-11T02:59:21Z

@usirin
usirin added this pull request to the merge queue Oct 11, 2026
Merged via the queue into main with commit b45811c Oct 11, 2026
2 checks passed
@usirin
usirin deleted the build/625-api-crash-exit-code-d285de4e branch October 11, 2026 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

code-graph --api-policy: an unexpected error may exit 1, the same code as a ratchet miss

1 participant