test: route every stdout/stderr write swap through stream-capture

[thymikee]

What

Makes stream-capture.ts the one place that changes process.stdout.write / process.stderr.write (done-check: git grep -E 'std(out|err)\.write =|vi\.spyOn\(process\.(stdout|stderr), .write.\)' is empty outside the helper and its own leak test).

• Deletes captureStderr from daemon-http-fixture.ts (its restore assigned a bound copy, leaving an own-property shadow for the rest of the file); callers use beginStdoutCapture/beginStderrCapture.
• Migrates every hand-rolled swap: generic/screenshot stdout captures, the scripts lanes (check-affected, coverage-changed, coverage-crap incl. the new blocking-suite capture() helper, fuzz/worker.ts via the discard option, help-conformance sample producers).
• Migrates all seven vi.spyOn(process.stdout|stderr, 'write') sites (per review): a spyOn write fake has the same own-property leak shape as the assignment swaps.
• Slims the helper surface: beginStdoutCapture/beginStderrCapture are the primitives; scoped with* forms derive from them; withInteractiveStderr(Async) fold into { interactive: true }; withStreamTtyAsync is deleted.

Verification

• Head 4a723e346, 33 files, +409/−604 (1,013 gross; the 13-line overshoot is retiring the three dead APPROVED_OVER_CEILING rows refactor(move): extract generic test helpers into @agent-device/test-kit #3483 left on main, which keeps the blocking Coverage job green).
• Full unit lane (13,397 tests), scripts node:test lanes, fuzz:parsers, harness, and check:affected --run all green locally.

Closes #3421

[cubic-dev-ai]

All reported issues were addressed across 25 files

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	5.17 MB	5.17 MB	0 B
Package (unpacked)	5.17 MB	5.17 MB	0 B
Package (download)	1.55 MB	1.55 MB	+8 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	21.9 ms	26.4 ms	+4.5 ms
CLI --help	60.8 ms	68.3 ms	+7.6 ms

[thymikee]

This PR is ready at 5d8e05c. Not blocking: the title says "every stdout/stderr write swap", but seven vi.spyOn(process.stdout|stderr, 'write') mocks remain, for example in cloud-connect-auth.test.ts:84 and daemon-client-startup-race.test.ts:511, so you can narrow the title to assignment swaps or move those sites onto beginStdoutCapture and beginStderrCapture in a follow-up, or leave it as is.

All 21 checks are green, including the fuzz lane, and I found no conflicts. I read the diff but did not run the tests locally. The mutation check on the discard regression is your claim, and I only confirmed from the diff that the old passthrough-first order would count the Buffer on the marker write. Nothing else stands between this PR and merge.

On the open review thread: the cubic-dev-ai P2 on the write wrapper in stream-capture.ts no longer applies, because the discard branch now comes first and the marker-write regression test pins it. Please resolve it: #3473 (comment)

[thymikee]

The code verdict at 5d8e05c is unchanged and still clean. This branch now conflicts with main, so it cannot merge as is. Please rebase on main. If the rebase only resolves the conflict, I will check the resolution and nothing else.

[thymikee]

Rebased onto origin/main post-merge of #3436 (git rebase --onto origin/main cefb002fe); the #3436 commits are gone from the branch and only the migration commit remains. Conflicts were limited to scripts/coverage-crap/run.test.ts (the new blocking-suite capture() helper migrated to the shared primitive; no cross-stream order-dependent assertions remain).

Also took the maintainer's note: all seven vi.spyOn(process.stdout|stderr, 'write') sites now use beginStdoutCapture/beginStderrCapture — the plugins test uses read()/read().slice() instead of mock-call accumulation, the startup-race notice assertion reads stderrCapture.read(), and remote-connection-macos-app no longer imports vi at all. Done-grep across the tree (assignment or spy) is empty outside stream-capture.ts and its leak test.

Head: 4ee03f292 (31 files, +405/−542, 947 gross). pnpm format, typecheck, lint, full unit lane, scripts lanes, and check:affected --run all green.

[thymikee]

The code looks good at 4ee03f2, and the conflict from the earlier review at 5d8e05c is fixed after the rebase. All 21 checks are passing, and the change is test and tooling only, with production net +2/-1. I did not run the test suites, and I did not re-diff the other 30 migrated files line by line; they are unchanged from the earlier reviewed head apart from the rebase. The one resolved thread was not re-read, and no threads are open.

Not blocking: the local captureStderr(run) in https://github.com/callstack/agent-device/blob/4ee03f2/src/commands/output/error.test.ts#L22 is now a one-line shim over withCapturedStderrAsync, and its name matches the deleted daemon-http-fixture helper, so you could call the shared helper directly at the 8 call sites or keep the wrapper, either is fine.

Simplicity question: discard has one caller, the fuzz worker in scripts/fuzz/worker.ts, which silences its own stderr for its whole life and never calls restore(). It also forces a precedence rule against passthroughBufferWrites and a regression test for a combination no caller uses. Could the worker keep its one-line no-op write, and could discard and that test go, so stream-capture stays a pure capture-and-restore test fake?

[thymikee]

Folded in two Codex review items on previously merged code, amended into the single commit:

1. stream-capture.test.ts sentinel test now saves the pre-existing stderr.write property descriptor and restores it in finally (deleting the property only when it was originally inherited), matching the discard test's pattern — it no longer clobbers a pre-existing own write on failure paths.
2. client-metro-startup-cleanup.test.ts: the control-flow-narrating comment above the EACCES test is deleted; test names and assertions unchanged (AGENTS.md keeps narration out of comments).

Head: b1721984c (32 files, +408/−548, 956 gross — still ≤ 1,000). pnpm format, typecheck, both touched test files, and check:affected --run green under the gate lock.

[thymikee]

Rebased onto current origin/main (e36f11740). Single conflict in scripts/coverage-crap/run.test.ts: #3486 had reworked that suite's local capture() into a descriptor-safe hand-rolled swap with a CapturedStream type. Since routing that swap through the shared helper is exactly this PR's job, the resolution keeps the beginStdoutCapture/beginStderrCapture form and drops the now-dead local type and its narration comment — the helper's restore already owns the descriptor invariant (pinned in stream-capture.test.ts).

Head: b1bd3c558 (32 files, +408/−570, 978 gross). Done-grep still empty outside the helper. The grown 20-test coverage-crap lane, typecheck, full unit lane (13,397), and check:affected --run all green under the gate lock.

[thymikee]

Rebased onto origin/main again (5680da519) after the test-kit extraction (#3483/#3484/#3498) landed. Conflicts were seven import blocks (mkdtempForTestSync/loopback now come from @agent-device/test-kit/*; the stream-capture imports stay repo-relative) plus the coverage-crap capture() hunk resolved onto the shared helpers as before, dropping the superseded local CapturedStream machinery.

Head: 71f911bf9 (32 files, +408/−570, 978 gross). Done-grep empty outside the helper. Typecheck, the migrated consumer tests, the 40-test scripts lanes, fuzz:parsers, full unit lane, and check:affected --run green under the gate lock. One unit-lane failure reproduces identically on a bare probe commit at origin/main (eager-closure-budgets: the three test-kit APPROVED_OVER_CEILING rows go unreadable one merge after #3483 — the deferral documented in staleApprovalRows, not introduced here; CI on #3498 itself stayed green); CI will confirm.

[thymikee]

Coverage red on this head is not from this PR's diff — it has two failures:

1. eager-closure-budgets > no APPROVED_OVER_CEILING row is stale: deterministic breakage that refactor(move): extract generic test helpers into @agent-device/test-kit #3483 left on main. Its three test-kit approval rows are readable only while the entries are first-introduced, so the first commit whose merge-base carries them (any rebase past refactor(move): extract generic test helpers into @agent-device/test-kit #3483) must retire them — exactly the deferral documented in staleApprovalRows. A bare probe commit at origin/main with no diff fails identically, and an unrelated PR's Coverage failed on this same test. The maintainer opened test(budgets): retire the three test-kit approval rows #3483 left dead #3502 to retire the rows; this PR will go green on the next rebase after that lands. Folding the deletion here instead would also push this PR to 1,010 gross, over its stated 1,000 budget.
2. fuzz/harness.test.ts > worker startup budget: 5s wall-clock timeout under CI runner contention; 13/13 green locally on this tree (twice), and the file's only change in this PR is a one-line stderr-sink swap. Rerunning the failed job to confirm.

[thymikee]

This PR is ready at 71f911b. The conflict from the earlier review (#3473 (comment)) is fixed, and the code looks right to me: the discard branch now runs before the passthrough branch, and the marker-write regression test pins that order. I did not run tests or local gates, so the green counts in the PR description are the author's own.

Not blocking, and fine to take or leave: src/__tests__/test-utils/stream-capture.ts has no imports and is now used by five files in scripts/ and by scripts/fuzz/worker.ts through a relative path, while #3483 made @agent-device/test-kit the home for generic test helpers, so a follow-up PR could move it and its test to packages/test-kit/src/stream-capture.ts, add the ./stream-capture export, and update the importers.

Coverage is the only failing check. It fails on the eager-closure-budgets test "no APPROVED_OVER_CEILING row is stale", which flags the three packages/test-kit rows from #3483. This diff touches neither that file nor test-kit, and stream-capture.ts has no imports, so I think the failure is not from this PR. I did not reproduce it on a bare main. No conflicts are known. Before merge, #3502 (which retires those rows) needs to land and this branch needs a rebase past it, then CI needs a rerun. No code change is needed here.

On the review threads, the cubic-dev-ai P2 thread on the discard branch order no longer applies, because the code and the marker-write test cover it, so the author can resolve it: #3473 (comment)

[thymikee]

Head 4a723e346 folds in the Coverage fix directly: the three test-kit APPROVED_OVER_CEILING rows in eager-closure-budgets.ts that #3483 left unreadable-on-next-merge-base are retired (content-wise identical to the maintainer's open #3502 — that PR becomes redundant once this lands, sorry for the overlap; #3473 was already the one queued for merge).

Head: 4a723e346 (33 files, +409/−604, 1,013 gross — 13 over the previous 1,000 budget solely from the dead-row deletion, coordinator-approved). Done-grep still empty outside the helper. eager-closure-budgets 838/838, full unit lane 13,434 green, scripts lanes and check:affected --run green under the gate lock.

[thymikee]

The PR is ready at 4a723e3. The earlier review at 71f911b was clean, and the new delta also holds up: the stream-capture helper now checks the discard case before the passthrough branch, and its test covers that. The cubic-dev-ai P2 thread on that ordering is fixed at this head, so please resolve it: #3473 (comment)

Not blocking: the change to https://github.com/callstack/agent-device/blob/4a723e3/scripts/__tests__/eager-closure-budgets.ts#L133 deletes three stale approval rows left by #3483. #3502 already landed the same fix, so a rebase should drop these rows from this PR and bring it back under the 1,000-line budget.

I did not run any test lane, so the 838/838 and 13,434 counts are your numbers, not mine. I also did not read all 33 changed test files. I checked the delta, the helper, and a search for remaining hand-rolled stdout/stderr swaps at this commit.

CI is still pending, with two Smoke Tests jobs queued and none failed. The diff is test files, a test-only budget table, and a 2-line change in scripts/fuzz/worker.ts. The smoke route only touches test/integration/smoke-open-remote-config.test.ts, which just swaps to the helper. There are no conflicts. Before merge, the Smoke Tests need to finish green.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-12 16:04 UTC