refactor: split the test-suite command out of the replay handler

[thymikee]

Summary

A pure move, split out of #1810 (now merged) so that PR could stay the fix.

handleSessionReplayCommands had grown into two unrelated halves: a three-line routing decision, and the whole test-suite orchestration — harness-flag admission, request translation, per-attempt video recording, shard resolution and the scheduler run. This moves the second half to session-test-suite-command.ts, beside session-replay-runtime.ts where the replay half already lives, and leaves session-replay.ts as the router it reads as.

• src/daemon/handlers/session-replay.ts: 413 → 34 lines
• src/daemon/handlers/session-test-suite-command.ts: new, 410 lines

No behavior change and no logic edits: the moved code is byte-identical apart from the test branch becoming a function body (dedented, with its params destructured at the top instead of inherited from the enclosing scope). buildNestedReplayFlags and bindReplayTestAttemptCancellation move with it.

Tests follow the split

AGENTS.md: "test files mirror source topology 1:1; when a source module splits, split its test file in the same PR." The tests move with the code they own:

Was	Now	Tests
session-replay.test.ts (560 lines, mixed)	session-test-suite-command-nested-flags.test.ts	5
	session-test-suite-command-flag-policy.test.ts	6
	session-test-suite-command-video.test.ts	1
session-replay-cancellation.test.ts	session-test-suite-command-cancellation.test.ts (renamed)	5

That move is checked as a move: 17 tests before, 17 after, with the before/after sets of test titles diffed and identical, so nothing was dropped, renamed, or skipped past the runner. Bodies were copied by line range; only per-file import headers are new.

Every one of those 12 tests was suite-owned, so the router was left with no test file of its own. session-replay.test.ts is therefore rewritten as the router's focused test rather than deleted, closing the 1:1 gap:

• replay reaches the script-source runtime (and the suite command is not called)
• test reaches the suite command with the whole parameter set — narrowing here is what would silently drop video recording, sharding and device binding
• an unrelated command returns null, so another handler family can claim it

Both destinations are mocked, so a mis-wired edge surfaces as the wrong marker rather than a device-level failure. Each case was proven non-vacuous by mutating exactly the routing it pins: swapping the two delegations fails the first two, and making the fallthrough claim the request fails the third.

Validation

pnpm format && pnpm check:affected --run green. Rebased onto main after #1810 merged (60f6356b0); this targets main directly and is out of draft.

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	2.28 MB	2.28 MB	+76 B
JS gzip	751.0 kB	750.9 kB	-106 B
npm tarball	872.4 kB	872.3 kB	-105 B
npm unpacked	3.05 MB	3.05 MB	+76 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	29.4 ms	28.3 ms	-1.0 ms
CLI --help	69.1 ms	68.6 ms	-0.5 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/session2.js	+76 B	-106 B

[thymikee]

The production move itself is behavior-preserving, but this is not ready yet. AGENTS.md requires tests to mirror source topology when a module splits: session-replay.test.ts is still a 560-line mixed aggregation importing both the router and the new suite-command module, and session-replay-cancellation.test.ts now tests only the new module under the old name. Move the suite-owned flag/video/admission/cancellation tests into sibling session-test-suite-command*.test.ts files and leave session-replay.test.ts focused on router/delegation. Also update the stale Stacking section now that #1810 merged and this PR targets main. Exact-head CI is still queued.

[thymikee]

Tests now follow the split; retargeted to main

You were right on both counts, and the first one is a rule I should have applied when I moved the module rather than after review — AGENTS.md is explicit that a source split splits its test file in the same PR.

Test topology. The suite-owned tests moved to siblings of the module that owns them:

Was	Now	Tests
session-replay.test.ts (560 lines, mixed)	session-test-suite-command-nested-flags.test.ts	5
	session-test-suite-command-flag-policy.test.ts	6
	session-test-suite-command-video.test.ts	1
session-replay-cancellation.test.ts	session-test-suite-command-cancellation.test.ts (renamed)	5

One deviation from your wording, deliberate and worth your call: session-replay.test.ts is deleted, not trimmed to router tests. All 12 of its tests were suite-owned — buildNestedReplayFlags projection, the raw replay-only flag rejections, and the video-finalization case — so there was nothing router-shaped to leave behind. The surviving 34-line router's delegation is already exercised by every harness-driven handler suite that goes through handleSessionCommands. I did not write new router tests because that would have made this PR stop being a pure move and stop being checkable as one; if you would rather have a dedicated router/delegation test, I will add it in a follow-up (or here, if you prefer).

Proven as a pure move, not asserted: 17 tests before, 17 after, and I diffed the before/after sets of test titles — identical, so nothing was dropped, renamed, or silently skipped by the runner. Bodies were copied by line range; only the per-file import headers are new. pnpm format && pnpm check:affected --run green.

Stacking section fixed. #1810 merged (60f6356b0), so this is rebased onto main and targets main directly — the body no longer claims a stack, and the PR is out of draft. Note GitHub had already auto-retargeted it when the base branch was deleted on merge; I verified my local rebase produced a tree identical to GitHub's auto-rebased head before force-pushing, so no content moved in the process.

Head is d3ec22674, diff is 6 files / +687 −629 — the module move plus the test files that follow it.

[thymikee]

Re-reviewed d3ec2267: the suite-owned tests are now properly split into focused session-test-suite-command files, and the production extraction remains behavior-preserving.

One topology gap remains: session-replay.test.ts was renamed wholesale, leaving the 34-line session-replay.ts router without a matching focused test. Add a small router test pinning replay → runtime, test → suite, and unrelated command → null, as required by the repo’s 1:1 source/test topology rule.

Also update the stale Stacking section: #1810 has merged, this PR targets main, and it is no longer draft. CI is green except iOS Smoke still running.

[thymikee]

Router test added; Stacking section actually removed this time

Both fixed in a3388f8e3.

Router test. session-replay.test.ts is now the router's own focused test rather than a deleted file — you were right that deleting it left the 1:1 rule unsatisfied. Three cases, through the real handleSessionReplayCommands entry:

• replay reaches the script-source runtime, and the suite command is not called
• test reaches the suite command with the whole parameter set — it asserts params is forwarded whole, because narrowing there is exactly what would silently drop video recording, sharding and device binding
• an unrelated command returns null, so another handler family can claim it

Both destinations are mocked, so a mis-wired edge shows up as the wrong marker instead of a device-level failure. Each case was proven non-vacuous against the mutation it actually pins, not just "it passes":

Mutation to session-replay.ts	Result
swap the two delegations	cases 1 and 2 fail, case 3 still passes
make the fallthrough claim the request instead of returning null	case 3 fails, cases 1 and 2 still pass

The router was restored byte-for-byte afterwards (git diff on it is empty).

Stacking section. Apologies — my earlier edit genuinely did not land, and I reported it as done. The cause: I passed --base main and the body in a single gh pr edit, the base change was rejected with "Cannot change the base branch because the pull request is part of a stack", and that error aborted the whole mutation including the body. I read the failure as affecting only the base and did not re-check. This time I verified by reading the body back: ## Stacking and the "Draft until #1810 merges" line are gone, and the body now describes a PR that targets main and is out of draft. (The base itself was already main — GitHub auto-retargeted it when the merged base branch was deleted.)

Head is a3388f8e3; pnpm format && pnpm check:affected --run green. CI on the previous head was 0 failing of 31 and is re-running on this one.

[thymikee]

iOS Smoke at a3388f8 failed in the Swift runner's own XCTest (RunnerTests.testBareTypeUsesTappedInputWhenSoftwareKeyboardIsHidden, 1 of 43; keyboard/focus flake) — this PR moves TypeScript handler/tests only and touches no runner code; main's iOS lane failed at 13:36Z on the same day. Re-ran the failed job; will update.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-18 13:56 UTC