Skip to content

refactor(daemon): own the replay script source fixture where it is used - #3704

Merged
thymikee merged 2 commits into
mainfrom
refactor/replay-script-source-fixtures
Oct 16, 2026
Merged

thymikee merged 2 commits into
mainfrom
refactor/replay-script-source-fixtures

Conversation

@thymikee

@thymikee thymikee commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

What

Removes replay-port/replay-script-source-fixtures (replay-port exports 30 → 29), the last fixture export in the replay packages. The loader-based helpers can't live in test-kit: they need replay-port, and replay-port now depends on test-kit, which would be a project-reference cycle. Each consumer therefore owns a private copy of only what it uses.

Fixture decision

Helper Importers by package Outcome Reason
withClientReplayScriptSources, replayScriptSourceBundleFor, maestroScriptSourceBundleFor daemon 22 files (20 tests, 2 support harnesses) daemon-private packages/daemon/src/__tests__/support/replay-script-source-fixture.ts (the original file, git mv) Used by daemon handler/router/replay-runtime tests (SessionStore, replay command); none is a replay-port test. Imports the loader through replay-port's existing production exports (script-source-bundle, source-discovery).
replayScriptSourceBundleFor, withClientReplayScriptSources root: 2 daemon-composition tests (specifier-only edits) and test/integration/provider-scenarios/harness.ts root-private src/__tests__/test-utils/replay-script-source.fixtures.ts Only the two helpers root uses. 45 lines in total, about 34 of code; written compactly rather than copied, same behaviour (passes through a request that already carries sources, or that has none to collect).
replayScriptSourceBundleFor, maestroScriptSourceBundleFor replay-port 1 (session-test-source-discovery-inspection.test.ts) replay-port-private src/replay-script-source.fixtures.ts, not exported Moved there in #3650; keeps only the two helpers it uses.
noMaestroIncludeSources 0 (maestro keeps its own in daemon-runtime-port-fixtures.ts) deleted Dead.

Commits

  1. git mv packages/replay-port/src/replay-script-source.fixtures.ts packages/daemon/src/__tests__/support/replay-script-source-fixture.ts (pure).
  2. The daemon file's own imports (production exports of replay-port) and noMaestroIncludeSources removed; the 22 daemon importers point at it relatively; the root fixture, the slim replay-port fixture and the three root specifiers; the replay-port export dropped.

Test-count parity

All 19 test files that import the fixture, run with AGENT_DEVICE_VITEST_MAX_WORKERS=1 --testTimeout=60000: 119 passed / 0 failed here. On origin/main, the same 19 files under normal timeouts: 107 passed / 12 failed, 119 total — the 12 are 5 s timeouts and the failing set changed between runs (host load average 150-580 while I measured), so I read the totals (119 = 119) as parity and the 60 s run as the pass evidence. CI is authoritative for green on this head. No vi.mock was added; the fixture files contain none.

Surface

replay-port exports 30 → 29; no new subpath, barrel or export *; test-kit untouched. replay-port, daemon and root each import only production exports of replay-port.

Gates

check:layering, check:gate-manifest, pnpm format, pnpm lint and tsc -b clean. check:affected --run not run locally: the shared host gate lock was queued behind other threads and load was extreme; CI runs it.

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

thymikee and others added 2 commits October 16, 2026 09:24
…on's test support

Pure git mv; the daemon owns the test stand-in for a client that collects replay sources, so the
file leaves replay-port. Imports, importers and the dead helper follow in the next commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The loader-based stand-in for a client leaves replay-port's exports: the daemon's tests import its
private copy, root keeps the two helpers its composition tests and provider harness use, and
replay-port keeps the two its own test uses. noMaestroIncludeSources had no importer and goes.

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

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.29 MB 5.29 MB -326 B
Package (unpacked) 5.29 MB 5.29 MB -326 B
Package (download) 1.59 MB 1.59 MB -71 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.1 ms 27.0 ms -1.1 ms
CLI --help 82.3 ms 79.3 ms -3.0 ms

@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.

All reported issues were addressed across 28 files

Reply with feedback, questions, or to request a fix.

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

@thymikee

Copy link
Copy Markdown
Member Author

This PR looks good at 203b1b1. CI is green: all 20 checks pass, and the touched tests ran there. I know of no conflicts.

Not blocking: withClientReplayScriptSources and replayScriptSourceBundleFor now live in two places, the daemon support fixture and the root test-utils fixture, with the same contract. A later change to client collection must then land in both. Could root tests import one owner through a single seam, or could the PR say why two owners are fine? You can take this or leave it.

On the open review thread: the cubic-dev-ai note on env forwarding in the support fixture does not apply to this change, so please resolve it (#3704 (comment)). The fixture is a plain move, and the old version also passed no env to the loader. No test depends on it today, so a follow-up could add it if wanted.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 16, 2026
@thymikee
thymikee merged commit 6a903c7 into main Oct 16, 2026
20 checks passed
@thymikee
thymikee deleted the refactor/replay-script-source-fixtures branch October 16, 2026 09:32
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-16 09:32 UTC

@thymikee

Copy link
Copy Markdown
Member Author

On the two copies of withClientReplayScriptSources / replayScriptSourceBundleFor (daemon support fixture and root test-utils): two owners is deliberate for now. A single owner would have to live in test-kit, and these helpers call replay-port's loader while replay-port now depends on test-kit, which is a project-reference cycle. The daemon cannot import root test code either. The duplicate is the small root subset (the two helpers root actually uses, about 34 lines of code), and both call the same production loader, so a change to client collection lands in script-source-bundle / source-discovery and only a change to the stand-in's own request handling needs both files. If the daemon's composition tests move fully into the daemon, the root copy shrinks to the provider harness's one helper.

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