test(cli): restore the run-command process-boundary contracts - #3710
Conversation
|
The failing check is |
a7aab6a to
f5fff10
Compare
f5fff10 to
b8bb8cb
Compare
Restores the six contracts apache#2476 kept on a real subprocess and 802855c dropped with the process fixture: piped non-TTY stdin (run -, implicit stdin prompt, positional plus stdin context), SIGINT delivery observed as exit 130 with empty stdout, and the fail-closed sandbox boundary reaching a non-interactive run. The injected MakaRunDeps seam covers these semantics in process but not the boundary itself. The new fixture is a minimal subprocess entry over runMakaTextCliCore with default environment deps and four scripted scenarios — none of the in-process duplication the removal targeted. Generated-by: Claude Code
b8bb8cb to
5be8d6f
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Adds a 157-line subprocess entry and four scripted scenarios covering three maka run contracts that only hold across a real process boundary. SIGINT currently appears nowhere in the repo's tests.
No P-graded issues. I checked why these were removed originally: duplication and fake-only coverage, not flakiness. Fault injection failed in exactly the right cases both times.
Two ungraded notes. sandbox-boundary is entirely within the seam and doesn't need a subprocess, and the three stdin scenarios compress to one — the two SIGINT cases carry nearly all the value, and ~160 lines would do. More substantively, this exercises a synthetic process boundary: it bypasses production's resolveMakaCliExitCode and the exit grace timer, so it can't catch a regression in either.
AI-assisted review: a Claude Code subagent analysed the change; I verified the deletion rationale from history and the fault-injection results myself. No tests run. AI review is not independent human review.
Summary
Restores the six process-boundary contracts that were retained on a real subprocess in #2476 and lost when 802855c removed the process fixture — as invited in the closing review of #2476 (Refs #2476, refs #2387).
The injected
MakaRunDepsseam covers the semantics ofstdinIsTTY/readStdin, but not the boundary itself: a real pipe feeding the prompt, a SIGINT delivered by the operating system observed as exit 130 with nothing on stdout, and the fail-closed sandbox path traversing a real child process. Currentmainasserts none of these (stdinIsTTY/readStdinare exercised in process only; no test anywhere asserts SIGINT → 130; the remaining spawn incli.test.tscovers profile/identity files, not the run path).run-command-fixture.ts(new) — a subprocess entry over today'srunMakaTextCliCorewith default environment deps (real stdin, real SIGINT handler, real exit code) and a minimal scripted adapter: four scenarios (echo,sandbox-boundary,slow,graph-wait), no env-var assertion matrix, none of the in-process duplication that 802855c removed.fixture-readyis written only from inside the run (after the core has installed its SIGINT handler), so the test's signal cannot race the default disposition.run-command.test.ts— onedescribe('maka run process contract')block with the six tests, assertions unchanged from the pre-deletion suite:run -, implicit stdin prompt, positional + stdin context)Both signal tests carry bounded SIGKILL guards (never-ready, never-exiting) so a regression fails the suite instead of hanging it.
No production changes.
Verification
packages/cli:node --test dist/__tests__/run-command.test.js— 11/11, three consecutive rounds (the six subprocess tests add ~0.8s total); full CLI suite 442/442.dist/, reverted): disablingif (interrupted) return 130fails exactly the two SIGINT tests; disabling the implicit stdin read fails exactly the two non-run -stdin tests.tscclean.AI use
Select exactly one:
Tool(s) and scope: Claude Code authored the fixture and tests (ported from the pre-deletion suite) and ran the verification; commits carry
Generated-by: Claude Codetrailers. Reviewed and submitted by a human.Checklist
Does this PR entail a change in behavior?