Skip to content

test(headless): run ordinary CLI command semantics in process - #2480

Merged
Astro-Han merged 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2387-headless-in-process-v2
Aug 8, 2026
Merged

test(headless): run ordinary CLI command semantics in process#2480
Astro-Han merged 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2387-headless-in-process-v2

Conversation

@UncertaintyDeterminesYou4ndMe

@UncertaintyDeterminesYou4ndMe UncertaintyDeterminesYou4ndMe commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Part of #2387 (Headless workspace; the CLI workspace is #2476 per the issue's separate-PR requirement).

Problem

The Headless suites started a Node subprocess per assertion for scenarios that only exercise argument validation, task-run business logic, and report verdicts. cli.test.ts was the sharpest case: 11 tests, 20 subprocess launches, 21.8s — each launch paying the full module-graph load of the headless CLI to test semantics that runMakaEvalCli already exposes as an exported Promise<number> router.

Change

No production changes. Every conversion goes through an already-exported entry:

  • cli.test.ts — 10 of 11 tests run through mapLegacyMakaHeadlessArgs + runMakaEvalCli, the same mapping and router the bin runs, with stdout/stderr captured at the process-stream seam and env overrides applied/restored around the call. The non-Headless-root test keeps the real bin route as the representative wiring contract (real exit code, stack-free stderr hygiene).
  • contamination-scan-cli.test.ts — 13 of 14 tests call the script's exported main(argv), with the executable footer's contract mirrored exactly (thrown error → stderr + exit 2, verdict codes returned). The no-argument rejection keeps the real subprocess as representative coverage of the footer itself, including its realpath main-module guard. The --markdown test gains an explicit assert.equal(code, 0) that was previously implicit in execFileAsync not rejecting.
  • New shared helper withCapturedProcessIo swaps and restores the process-wide stream writers; safe for the sequential node:test execution these files use.

An earlier revision of this branch also converted three pre-launch validation tests in harness-ab-cli.test.ts; #2462 deleted that suite on main, so that part is gone after rebase.

Deliberately not converted

  • runtime-policy-ab-cli.test.ts: its single test is the representative subprocess for run-runtime-policy-ab.mjs, whose main() is not exported — adding an export to shave 0.6s is a production touch the saving does not justify.
  • pi-cli-json-transport.test.ts already uses an injected fake child; task-agent-controller/harbor-cell subprocesses are the behavior under test.

Retained process contracts

Suite Real-subprocess coverage kept
cli.test.ts bin wiring end to end: legacy mapping, exit code, stack-free stderr (1 test / 2 launches)
contamination-scan-cli.test.ts executable footer: no-args → exit 2, realpath main-module guard (1 test)

Timing

node --test, same machine, warm build:

file before after spawns
cli.test.js 21.79s 13.12s 20 → 2
contamination-scan-cli.test.js 1.05s 0.63s 14 → 1

The residual 13s in cli.test.js is dominated by one heavy-task journey (7.4s) whose cost is task-run business logic, not startup tax — visible now that the tax is gone.

Tests pass across 3 consecutive rounds after the rebase onto #2462/#2473; typecheck, biome, and knip are clean (knip delta against baseline: run-contamination-scan.mjs moves from unused to used).

Supersedes #2479: that branch predated the #2462 merge and conflicted with it, so GitHub could not build the PR merge commit and never created check suites — which also explains the "CI never triggered" mystery there (my platform-failure guess in its closing comment was wrong).

cc @Astro-Han

Part of apache#2387. The Headless CLI and contamination-scan suites started a
Node subprocess per assertion for scenarios that only exercise argument
validation, task-run business logic, and report verdicts — semantics
the exported entry points already expose.

- cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs
  + runMakaEvalCli with stdout/stderr captured at the process-stream
  seam and env overrides applied and restored around the call. The
  non-Headless-root test keeps the real bin route as the representative
  wiring contract (real exit code, stack-free stderr).
- contamination-scan-cli.test.ts: 13 of 14 tests call the script's
  exported main(argv), mirroring the executable footer exactly (thrown
  error -> stderr + exit 2). The no-argument rejection keeps the real
  subprocess as representative coverage of that footer itself,
  including its realpath main-module guard.
- New shared helper withCapturedProcessIo swaps and restores the
  process-wide stream writers; safe for the sequential node:test runs
  these files use.

runtime-policy-ab-cli.test.ts is intentionally untouched: its single
test is the representative subprocess for run-runtime-policy-ab.mjs,
whose main() is not exported, and adding an export to shave 0.6s is
not warranted. (harness-ab-cli.test.ts was part of this change until
apache#2462 deleted that suite on main.)

Timing (node --test, local, warm build):
  cli.test.js                   21.8s -> 13.1s (spawns 20 -> 2)
  contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1)
Tests pass across 3 consecutive rounds.
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks — the boundary split here looks good. Ordinary Headless CLI semantics now use the existing exported entry points, while executable/footer behavior retains representative subprocess coverage. The shared IO helper is small enough as-is, and the heavy-task journey still protects a distinct two-invocation trace contract.

One non-blocking P3 suggestion:

The contamination-scan subprocess coverage now keeps only the no-argument exit 2 path. It no longer exercises a successfully parsed scan whose verdict must make the real executable exit 1. If the footer later regressed to always exit 0, the in-process verdict tests would still pass.

Keeping one “retrieval signal detected → real subprocess exit 1 + stdout report” case would preserve that fail-closed executable contract. If spawn count matters, one of the two similar Headless CLI error-process launches could be moved in process to offset it.

This does not need to block the PR. The current implementation and CI look sufficient to merge. Thanks!

@Astro-Han
Astro-Han merged commit 56ddb9f into apache:main Aug 8, 2026
11 checks passed
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.

3 participants