Skip to content

refactor(replay): move four daemon tests of replay-port code into replay-port - #3650

Merged
thymikee merged 3 commits into
mainfrom
refactor/replay-suite-tests-into-replay-port
Oct 15, 2026
Merged

thymikee merged 3 commits into
mainfrom
refactor/replay-suite-tests-into-replay-port

Conversation

@thymikee

@thymikee thymikee commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

What

Four daemon tests that only exercise replay-port modules move into replay-port, beside the module they test and importing it relatively. Their package export (./session-test-source-discovery) loses its last importer and is dropped. Second slice of the replay-package sweep (first: #3647, independent).

Commits

  1. refactor(replay): move four daemon tests of replay-port code beside their modules — git mv only. git diff -M --stat: four renames, 0 insertions / 0 deletions.
  2. refactor(replay): import the moved tests relatively — specifier rewrites only, plus one type: attachRemoteReplayTestArtifacts takes DaemonWireRequest, which daemon-private DaemonRequest only widened, so the test's req() helper is typed DaemonWireRequest from @agent-device/contracts/command.
  3. chore(gates): drop the replay-port session-test-source-discovery export.

Moves

From (packages/daemon/src/sessions/__tests__/replay-suite/) To (packages/replay-port/src/daemon-port/__tests__/) Module
session-test-suite-command-cancellation.test.ts test-command-cancellation.test.ts test-command.ts (bindReplayTestAttemptCancellation)
session-test-suite-command-nested-flags.test.ts test-command-nested-flags.test.ts test-command.ts (buildNestedReplayFlags)
session-test-suite-command-remote-artifacts.test.ts test-command-remote-artifacts.test.ts test-command.ts (attachRemoteReplayTestArtifacts)
session-test-source-discovery.test.ts session-test-source-discovery-inspection.test.ts session-test-source-discovery.ts

The last one takes an -inspection aspect suffix because replay-port already has a different session-test-source-discovery.test.ts (the lazy-backend-resolution cases). Both stay; the existing one is untouched.

None of them uses vi.mock.

Fixture decision (these tests' fixture imports)

Fixture Importers moved Outcome
test-kit/tmp-dir mkdtempForTestSync 2 tests replay-port's own tmp-dir.fixtures.ts (already there for the package's tests)
replay-port/replay-script-source-fixtures 1 test relative import of ../../replay-script-source.fixtures.ts; the export itself stays until test-kit's single entry lands (20 daemon + root importers remain)

Test-count parity (vitest run <files> --reporter=json, passed/failed)

Test Before (daemon path, a907a919a) After (replay-port path)
cancellation 5/0 5/0
nested-flags 6/0 6/0
remote-artifacts 3/0 3/0
source-discovery(-inspection) 4/0 4/0
existing session-test-source-discovery.test.ts (untouched) n/a 3/0

Total moved 18 → 18. Selection: destination is under packages/*/src/**/*.test.ts (unit-core include); vitest collected all five files by path. check:gate-manifest ok; check:affected selected the moved files (they appear in its related-test command).

Surface

replay-port exports 32 → 31 (cumulative with #3647: 33 → 31 against main). No new subpath, barrel or export *. bindReplayTestAttemptCancellation and buildNestedReplayFlags keep their module-level export for the colocated tests; their package subpath (./test-command) still has production and integration importers.

Remaining non-package importer of attachRemoteReplayTestArtifacts: test/integration/provider-scenarios/remote-test-artifacts-materialization.test.ts (root worker's file). Not touched.

Gates (on 1162edeac)

check:quick, check:layering, check:fallow, check:gate-manifest, pnpm format: clean. check:affected --run: all pass except scripts/__tests__/eager-closure-budgets.test.ts > no APPROVED_OVER_CEILING row is stale, which names packages/daemon/src/index.ts and fails on any branch off a907a919a (passes on a907a919a itself only because the check is deferred when merge-base equals head). Left stale by the daemon single-entry PR #3633; daemon thread to remove that row.

R83 and the colocation baseline are not on main yet, so this only shrinks what the seed will contain.

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 5 files

View guided diff | Turn on auto-fix | Re-trigger cubic

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.25 MB 5.25 MB 0 B
Package (unpacked) 5.25 MB 5.25 MB 0 B
Package (download) 1.58 MB 1.58 MB -2 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 29.1 ms 27.7 ms -1.4 ms
CLI --help 77.5 ms 79.4 ms +1.8 ms

thymikee and others added 3 commits October 15, 2026 14:34
…heir modules

Pure git mv; specifiers are rewritten in the next commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Specifier rewrites only, plus the one request type: attachRemoteReplayTestArtifacts takes a
DaemonWireRequest, which the daemon-private DaemonRequest only widened.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Its only importer moved into the package.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thymikee

Copy link
Copy Markdown
Member Author

This looks good at 1162ede. The four tests now sit beside the replay-port code they cover, and the diff touches only tests and a replay-port export entry. I did not run the tests or the layering scan.

Coverage fails only on the stale eager-closure-budgets row for packages/daemon/src/index.ts, and that row also fails at the merge-base, so this PR did not cause it. I did not read the Smoke Tests logs. The diff changes no device-facing code, so those failures look unrelated too. There are no conflicts. Nothing in this PR blocks merge. The Coverage failure only needs the stale APPROVED_OVER_CEILING row fixed.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 15, 2026
@thymikee
thymikee force-pushed the refactor/replay-suite-tests-into-replay-port branch from 1162ede to d9f07bd Compare October 15, 2026 12:43
@thymikee

Copy link
Copy Markdown
Member Author

Rebased onto main (80d5e22), which carries #3653's removal of the stale daemon approval row. On the new head: the five replay-port test files pass (18 moved + 3 existing) and eager-closure-budgets.test.ts is 1010/1010. check:quick, check:layering, check:gate-manifest and pnpm format are clean. CI is running.

@thymikee
thymikee merged commit de35001 into main Oct 15, 2026
22 checks passed
@thymikee
thymikee deleted the refactor/replay-suite-tests-into-replay-port branch October 15, 2026 13:36
@thymikee

Copy link
Copy Markdown
Member Author

This PR is ready at d9f07bd. The earlier review of 1162ede was clean, and the four daemon tests of replay-port code now live in replay-port. The diff touches 5 test files and removes one package.json export, with no production lines changed. A search of tracked files at this commit finds no importer of the removed subpath.

Not blocking: the comment at packages/replay-test/src/internal/tests/session-test-runtime.test.ts:14 still names the old daemon path for the cancellation contract, and it can point to packages/replay-port/src/daemon-port/tests/test-command-cancellation.test.ts instead, but you can take or leave this.

I ran no tests or gates, so the moved tests and the layering and eager-closure gates rest on your report and on CI. All 10 remaining checks are still queued, and none has failed. The merge now waits only for those checks to go green. There are no conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant