Reduce mcp-cli-*.test.ts real-subprocess spawn count (currently ~40% of suite time)

[JSONbored]

Problem

test/unit/support/mcp-cli-harness.ts's run()/runAsync() spawn the real CLI as a subprocess
(execFileSync(process.execPath, [bin, ...args]), where bin is the compiled
packages/loopover-mcp/dist/bin/loopover-mcp.js) for every single call, deliberately — a subprocess is
the only way to exercise the real process entrypoint (argv parsing, exit codes, real stdout/stderr), and
v8 cannot Codecov-instrument a separate process. Ten files in test/unit/ call run()/runAsync()
hundreds of times combined, and one more (mcp-cli-maintain-tools.test.ts) spawns the CLI via a real
StdioClientTransport connection per test instead. Verified fresh on 2026-07-25 (a clean worktree off
origin/main, npm ci + npx turbo run build --filter=@loopover/engine --filter=@loopover/mcp):

• A single bare CLI invocation (node packages/loopover-mcp/dist/bin/loopover-mcp.js --help) costs
  ~0.27–0.40s real time, vs. ~0.06s for bare node -e "1". The ~250–350ms delta is the CLI's own
  module-graph resolution (loading its full dist/lib/*.js dependency tree) — not TypeScript
  stripping; the bin is pre-compiled, so there is no stripping cost to eliminate.
• test/unit/mcp-cli-packets.test.ts in isolation: 22 tests, 23.70s "tests" time (~1.08s/test).
• test/unit/mcp-cli-maintain.test.ts in isolation: 25 tests, 19.41s "tests" time.
• In a full-suite run (many files spawning subprocesses concurrently under vitest's worker pool), the
  same files ran ~3x slower than in isolation (CPU oversubscription, not a per-file regression) —
  see the historical full-suite numbers table below. Any timing claim in your PR must be based on
  isolated single-file runs (the commands in "Required verification" below), not full-suite numbers,
  which are dominated by whatever else happens to be running concurrently on the CI runner that day.

Historical full-suite timing (2026-07-24, for context only — re-measure your own baseline, don't cite these numbers in your PR)

File	Full-suite time (contended)
mcp-cli-packets.test.ts	43.1s
mcp-cli-maintain.test.ts	38.2s
mcp-cli-doctor.test.ts	34.6s
mcp-cli-basics.test.ts	31.9s
mcp-cli-profiles.test.ts	30.3s
mcp-cli-review-pr.test.ts	18.5s
mcp-cli-telemetry.test.ts	13.4s
mcp-cli-notifications.test.ts	13.3s
mcp-cli-pr-outcomes.test.ts	12.9s
mcp-cli-maintain-tools.test.ts	12.8s
mcp-cli-monitor-open-prs.test.ts	12.3s

Already ruled out as a free fix (don't waste time re-trying these): NODE_COMPILE_CACHE (Node 22's
built-in V8 compile cache) made no measurable difference. --pool=forks vs. the default threads pool
made no meaningful difference either. Do not propose a vitest workspace/projects concurrency split,
a maxWorkers/poolOptions change, or any edit to vitest.config.ts or .github/workflows/ci.yml —
that class of fix is explicitly out of scope for this issue (see "Out of scope" below). The only
in-scope lever is reducing how many real subprocesses these specific test files spawn.

Required scope — every file below, in one PR

Convert the subprocess-spawning test cases in these 11 files that don't genuinely need the real
process boundary (see "The conversion rule" below) to the existing in-process harness pattern. This is
the complete, final scope — do not submit a PR covering only some of these files; see "Why this is one
PR, not several" below.

File	Spawn mechanism	Real spawn count (verified 2026-07-25)
test/unit/mcp-cli-packets.test.ts	run()/runAsync()	37
test/unit/mcp-cli-maintain.test.ts	run()/runAsync()	53
test/unit/mcp-cli-doctor.test.ts	run()/runAsync()	27
test/unit/mcp-cli-basics.test.ts	run()/runAsync()	37
test/unit/mcp-cli-profiles.test.ts	run()/runAsync()	25
test/unit/mcp-cli-review-pr.test.ts	run()/runAsync()	13
test/unit/mcp-cli-telemetry.test.ts	run()/runAsync()	23
test/unit/mcp-cli-notifications.test.ts	run()/runAsync()	17
test/unit/mcp-cli-pr-outcomes.test.ts	run()/runAsync()	11
test/unit/mcp-cli-monitor-open-prs.test.ts	run()/runAsync()	10
test/unit/mcp-cli-maintain-tools.test.ts	new StdioClientTransport({ command: "node", args: [bin, "--stdio"], ... }) inside a beforeEach-style connect() helper, once per it() (~16 spawns — not the run() pattern; grepping for run( on this file undercounts it, since it only has one incidental run() call for a JSON-output check)	~16

Get each file's exact current count yourself before starting (counts drift as the suite evolves):

grep -cE "\brun\(|\brunAsync\(" test/unit/mcp-cli-packets.test.ts   # etc., one per file
grep -cE "new StdioClientTransport" test/unit/mcp-cli-maintain-tools.test.ts

Out of scope — do not touch these, and do not propose these classes of fix

• test/unit/mcp-discovery.test.ts — its slowness (12.0s historically) is from real git init/
  git config fixture setup (execFileSync("git", ...)), not CLI subprocess spawning. It doesn't
  call run()/runAsync()/StdioClientTransport at all. Wrong root cause for this issue; do not
  include it in your PR. (If its git-fixture overhead is worth fixing, that's a separate issue.)
• test/unit/mcp-local-telemetry-chokepoint.test.ts — it does spawn the CLI directly via
  execFileSync("node", [bin, ...]) (5 calls), but its test names ("records one event per invocation, not one per session", "telemetry disable returns the server to sending nothing") indicate it is
  specifically verifying behavior that depends on genuine process independence across separate
  invocations. Converting it to a shared in-process harness could silently make these assertions
  meaningless (state that should NOT persist between invocations trivially wouldn't, in a shared
  process, regardless of whether the underlying code is correct). Excluded from scope.
• Any change to vitest.config.ts, .github/workflows/ci.yml, or any poolOptions/maxWorkers/
  concurrency setting. Not this issue's lever (see "Already ruled out" above) — a PR touching any of
  these files for this issue will be closed regardless of what else it does correctly.
• Deleting or .skip()-ing any test case to reduce the count. The goal is fewer real subprocess
  spawns for equivalent coverage, not fewer test cases. See "Anti-patterns" below.

The conversion rule

For every run()/runAsync() call and every StdioClientTransport-per-test pattern in the 11
files above, read the test body and classify it. This is a closed rule — if a case doesn't clearly fit
"keep," convert it.

KEEP as a real subprocess (do not touch) if the test does ANY of:

• (a) Asserts on the process's exit behavior itself — expect(() => run(...)).toThrow(...) for a
  CLI-level argv/flag error, or otherwise checks that the process exited non-zero for a bad invocation.
• (b) Asserts on exact raw stdout/stderr text formatting the CLI's own presentation layer produces
  (table alignment, --help banner text, non---json human-readable output). The in-process harness
  calls tool handlers directly and never runs the CLI's own stdout-rendering code path, so this class of
  test is only meaningful against a real spawned process.
• (c) Is explicitly testing CLI bootstrap/entrypoint behavior itself — bare-invocation help text,
  version banner, env-var precedence resolved at process startup, argv parsing before any tool runs.
• (d) Depends on genuine process-to-process isolation (state must NOT leak between separate
  invocations) — same reasoning as the excluded mcp-local-telemetry-chokepoint.test.ts file above,
  applied per-test-case within an otherwise-in-scope file.

CONVERT to the in-process pattern if the test does none of the above, and instead:

• (e) Calls the CLI with --json (or equivalent) and asserts on the parsed JSON payload's business
  content — a tool's returned data shape, field values, computed results. This is testing the tool
  handler's logic, which the in-process harness reaches identically.
• (f) Asserts on a structured JSON error object the tool surfaced (not a process exit code) — same
  reasoning: testing the handler's error-surfacing logic, not process mechanics.

Worked example (mcp-cli-maintain-tools.test.ts, read directly — use this exact reasoning as your
template for the other 10 files):

• "registers all 7 maintain tools in the stdio server tool list" → convert (rule e — this is exactly
  what the StdioClientTransport→InMemoryTransport swap is for).
• "lists all 7 maintain tools via \loopover-mcp tools --json` with non-empty descriptions"` → convert
  (rule e).
• "${tool.name} proxies to its REST endpoint and returns the payload" (×7, one per tool) → convert
  (rule e).
• "${tool.name} surfaces an API failure as a tool error" (×7) → convert (rule f).
• "list_pending_actions advertises no status filter, which this server's route could not honour" →
  convert (rule e — asserting on returned tool-list metadata).
• "set_action_autonomy read-merge-writes so the other action classes survive" → convert (rule e —
  asserting on the tool's data-merge business logic).
• "rejects an unknown action class and an unknown autonomy level before any API call" → read the
  actual assertion before deciding. If it checks a thrown process exception (rule a), keep it; if it
  checks a structured JSON error the tool handler returned (rule f), convert it. Do not guess — this is
  exactly the kind of ambiguous case the rule exists to force you to actually read, not assume.

If you genuinely cannot classify a case after reading it carefully, leave it as a real subprocess test
(default to (a)–(d), the safe side) and note which case and why in your PR description — don't guess
wrong in the risky direction.

The conversion pattern — exact technical steps

Mirror test/unit/mcp-cli-contributor-profile-inprocess.test.ts exactly (read the whole file before
starting; this is not a rough sketch, it is the literal pattern to copy):

1. Import Client from @modelcontextprotocol/sdk/client/index.js and InMemoryTransport from
   @modelcontextprotocol/sdk/inMemory.js (not StdioClientTransport — that still spawns a real
   process).
2. Set every process.env.LOOPOVER_* var the bin reads at module load before importing it — the
   template does this with a beforeAll + dynamic await import(specifier) for exactly this reason
   (env must be set before the module loads, so the import has to happen after the env is set, hence
   dynamic rather than a static top-level import).
3. Import the bin's own exported functions/server directly
   (packages/loopover-mcp/bin/loopover-mcp.ts — the committed .ts source, not the compiled
   dist/bin/loopover-mcp.js) rather than spawning it. The isProcessEntrypoint guard already in that
   file is what lets a test import the module without it hijacking argv/binding stdin — you should not
   need to touch that guard, only rely on it.
4. For a case using StdioClientTransport today (mcp-cli-maintain-tools.test.ts), swap it for
   InMemoryTransport connecting directly to the bin's exported server object — no subprocess, no
   command/args, just an in-memory duplex pair.
5. For a case using run()/runAsync() with --json today, replace the subprocess call with a direct
   call into the bin's exported CLI function (the template's contributorProfileCli is the pattern —
   find the equivalent exported function for the tool/command under test) and assert on its return value
   or captured stdout the same way the template's captureStdout helper does.
6. Delete the original subprocess-based test case you just converted. Do not leave both versions —
   the existing -inprocess/-stdio file family in this repo was built to add Codecov coverage
   alongside an unconverted subprocess sibling; that is a different goal from this issue. Here, the
   in-process version replaces the subprocess version for cases classified "convert" — net subprocess
   spawn count must go down. Adding new files without removing the slow originals achieves nothing for
   this issue.
7. Where you're converting most, but not all, of a file's cases, add the in-process cases into the
   same file next to the kept subprocess ones (following mcp-cli-maintain-tools.test.ts's own
   existing structure, which already mixes both patterns) rather than creating a separate file, unless a
   file's kept-subprocess-case count drops to zero, in which case rename it to drop any transport-
   specific suffix ambiguity and delete the now-empty subprocess describe block entirely.

Deliverables (all required, none optional)

1. All 11 files updated per the rule above, in a single PR.
2. Every converted test case's original assertions preserved (same behavior verified, just via the
   faster transport) — this is a transport swap, not a test rewrite. Total assertion count across the
   changed files must not decrease.
3. Every kept subprocess test case still present, unmodified in what it verifies.
4. Required verification, pasted into the PR description as actual command output, not summarized:

   git stash   # or: check out main before your changes
   npx vitest run test/unit/mcp-cli-packets.test.ts test/unit/mcp-cli-maintain.test.ts test/unit/mcp-cli-doctor.test.ts test/unit/mcp-cli-basics.test.ts test/unit/mcp-cli-profiles.test.ts test/unit/mcp-cli-review-pr.test.ts test/unit/mcp-cli-telemetry.test.ts test/unit/mcp-cli-notifications.test.ts test/unit/mcp-cli-pr-outcomes.test.ts test/unit/mcp-cli-monitor-open-prs.test.ts test/unit/mcp-cli-maintain-tools.test.ts --reporter=verbose
   # note the "Duration" line and per-test times — this is your BEFORE baseline
   git stash pop   # or: check out your branch
   npx vitest run <same file list> --reporter=verbose
   # note the "Duration" line again — this is your AFTER number
   

   Paste both full outputs (or at minimum both summary lines) in the PR description, plus the before/
   after spawn count per file (the grep -cE commands above, run against main and against your
   branch). A PR claiming a speedup without this pasted evidence will be closed.
5. A minimum 50% reduction in total real subprocess spawn count across the 11 files (sum of the
   "Real spawn count" column above minus your post-PR count, verified by the grep commands), with a
   corresponding real wall-clock reduction shown in the before/after Duration lines. If you genuinely
   can't reach 50% because more cases than expected fall under "keep" per the rule, that's fine — but
   your PR description must show your case-by-case classification reasoning for every file so it's
   verifiable, not just a final number.
6. npm run test:ci green, unchanged coverage on everything outside the 11 files you touched.

Anti-patterns — a PR doing any of these will be closed, not requested-changes

• Deleting, .skip()-ing, or reducing the number of test scenarios to hit a smaller spawn count.
  The fix is transport (subprocess → in-process), never scenario coverage.
• Adding new -inprocess files alongside unconverted subprocess originals (this repo's existing
  -inprocess/-stdio files do this for a different reason — Codecov coverage — not applicable here;
  see step 6 above).
• Touching vitest.config.ts, .github/workflows/ci.yml, or any pool/concurrency setting.
• Converting mcp-local-telemetry-chokepoint.test.ts or mcp-discovery.test.ts.
• Converting a case that fits the "keep" rule (a)–(d) to hit the 50% target faster.
• Claiming a timing improvement without the pasted before/after command output from "Required
  verification" above.
• Splitting this into multiple PRs (see below).

Why this is one PR, not several

The 50% aggregate spawn-count target and the wall-clock claim only mean something measured across all
11 files together in one before/after comparison. A PR converting only 2-3 files would show a real but
small, easily-noise-dominated delta, and there's no way to verify the aggregate claim without the
whole scope landing together. This issue is a deliberate, explicit exception to this repo's usual
small-PR preference — do the whole thing, verify the whole thing, submit it as one PR.

Non-goals

• This does not touch the "why is full-suite time 3x isolated time" contention problem — that's a CI
  concurrency/scheduling question, explicitly out of scope (see above). This issue only reduces the real
  work each file does; it does not change how many files vitest runs concurrently.
• This does not change Codecov coverage requirements for these files — test/** is not measured by
  Codecov's patch-coverage gate regardless.

[linear-code]

JSO-5948