Skip to content

test(cli): restore the run-command process-boundary contracts - #3710

Open
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:test/2476-process-boundary-contracts
Open

test(cli): restore the run-command process-boundary contracts#3710
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:test/2476-process-boundary-contracts

Conversation

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor

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 MakaRunDeps seam covers the semantics of stdinIsTTY/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. Current main asserts none of these (stdinIsTTY/readStdin are exercised in process only; no test anywhere asserts SIGINT → 130; the remaining spawn in cli.test.ts covers profile/identity files, not the run path).

  • run-command-fixture.ts (new) — a subprocess entry over today's runMakaTextCliCore with 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-ready is 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 — one describe('maka run process contract') block with the six tests, assertions unchanged from the pre-deletion suite:

    Contract Tests
    Piped non-TTY stdin (run -, implicit stdin prompt, positional + stdin context) 3
    SIGINT → exit 130, empty stdout through a real pipe (plain run; during Graph completion wait) 2
    Fail-closed sandbox boundary reaching a non-interactive run 1

    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.
  • Fault injection (in dist/, reverted): disabling if (interrupted) return 130 fails exactly the two SIGINT tests; disabling the implicit stdin read fails exactly the two non-run - stdin tests.
  • Biome clean on both files; ASF header audit clean; tsc clean.
  • Not run: workspaces this change does not touch.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

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 Code trailers. Reviewed and submitted by a human.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor Author

The failing check is Check ASF source headers, and the five files it names are all from d807c0078 (#3250) on current main — none are touched by this PR (both files here carry the header; the audit passes locally against this branch's own changes). The in-flight fix is #3708; once it lands I will rebase to re-run CI on a base where the audit is green.

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
@UncertaintyDeterminesYou4ndMe
UncertaintyDeterminesYou4ndMe force-pushed the test/2476-process-boundary-contracts branch from a7aab6a to f5fff10 Compare August 24, 2026 11:55
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.

1 participant