Repository navigation
refactor(test-kit): drop command-registry by making the replay-script generator a root fixture - #3687
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 12 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Size Report
Startup median (7 runs, lower is better):
|
|
|
I found no code problems in ade81ad. The move of the fixtures out of test-kit and the dropped command-registry and daemon-contracts dependencies read as correct from the manifests and imports. I did not run the parity suite or the gates you list (check:quick for tsc -b acyclicity, layering, fallow, R83/R84). I checked them only by reading manifests and imports. CI is still pending. No job has failed. Smoke Tests are queued, and Compatibility & Provenance and Coverage are still running. Coverage runs the moved fixtures' tests, so watch that job. Smoke Tests drive device paths this diff does not touch. There are no conflicts. The cubic-dev-ai P2 thread on the fuzz envelope hash still applies (#3687 (comment)). The gap is real at head: CASE_GENERATION_INPUTS does not hash ../../src/commands/replay/replay-script.fixtures.ts. It also existed on main, so this PR did not add it, and a follow-up is fine. Nothing blocks the review. The next step is for CI to finish green. |
ade81ad to
9c50e07
Compare
… generator a root fixture The generator is derived from the command catalog and is used only by the root round-trip property and the fuzz target, so it lives beside them; the fuzz run envelope hashes it as a case-generation input. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
9c50e07 to
bf274d5
Compare
thymikee
changed the title
|
This PR is ready at bf274d5. The earlier finding on the fuzz hash list is fixed: scripts/fuzz/envelope.ts now lists the moved replay-script fixture, and the harness hash reads it. Not blocking, and you can take or leave these: the generation-graph walk in scripts/fuzz/envelope.test.ts only follows './' imports, so the new '../../src/...' entry is checked only for file existence, and it would help to list test-kit's property-arbitraries.ts in CASE_GENERATION_INPUTS or assert the fixture entry is present; and the PR body calls the fixture "unexported", but replay-script.fixtures.ts exports replayScriptArb and two modules import it, so "unexported from any package entry" would be more exact. The Cubic thread on the fuzz hash list is fixed at this head, so please resolve it: #3687 (comment). CI shows 24 checks with none failing, and the diff touches only test fixtures, the fuzz hash list and test-kit manifests. I did not run the fuzz envelope, harness, round-trip or ad-script tests, or the gates. I relied on CI, diffs and greps, and I reviewed the 11-file logical patch against the merge-base because the rebase onto 2a2f3f2 inflates the raw delta. No conflicts. Nothing else stands between this PR and merge. |
thymikee
added
the
ready-for-human
…ctions-1 * origin/main: feat(react-devtools): inspect React components and profile renders through the Metro inspector (#3613) refactor(daemon-contracts): drop the registration and claims fixture exports (#3689) refactor(test-kit): drop the daemon-contracts dependency (#3686) refactor(test-kit): drop command-registry by making the replay-script generator a root fixture (#3687) fix(ios): handle delayed launch confirmations without stopping XCTest (#3682) refactor(test-kit): move the dispatch-disclosure harness out of contracts (#3690) refactor(test-kit): host the target classification capture fixture (#3691) ci: fold six CI jobs into four with parallel and background steps; cancel superseded and closed-PR runs (#3658) build(typescript): move root project references into tsconfig.typecheck.json (#3688) chore(daemon): declare @agent-device/proxy as a devDependency, since only a test imports it (#3685) refactor(settings): declare settings once and admit them per setting (#3662) # Conflicts: # packages/platform-android/src/alert.ts # website/docs/docs/limrun.md
1 participant
What
Cuts test-kit's workspace dependency on
command-registry. The.adreplay-script generator (replayScriptArb) is derived fromPUBLIC_COMMANDS/INTERNAL_COMMANDS, which was the only reason test-kit imported command-registry. It has two consumers, both in root, so it becomes a private fixture beside them:replayScriptArbtest-kitproperty-arbitraries.tssrc/commands/replay/ad-script-round-trip.test.ts,scripts/fuzz/arbitraries.tssrc/commands/replay/replay-script.fixtures.ts(unexported; both consumers import it relatively). It still readsrefArb/formatReffrom test-kit.The code is moved verbatim; the other changes are two import specifiers, the generator's error-message path, the test-kit manifest/tsconfig/entry/lockfile (command-registry removed), and
docs/agents/testing.md.scripts/fuzz/envelope.tsalso adds the new file toCASE_GENERATION_INPUTS, so the fuzz run envelope'sconfigHashcovers the generator (review thread resolved).Scope with #3686
#3686 owns
spawnLegacyDaemonFixtureand thedaemon-contractscut; this PR no longer touches either. Until #3686 merges, test-kit still reachesselectorsthroughdaemon-contracts > command-registry, so selectors cannot yet declare test-kit. After both merge, test-kit reaches only capture-kit, contracts, host-kit and kernel. Expect a trivial conflict inpackages/test-kit/package.json/tsconfig.jsonwith #3686; the second to merge rebases.Fixture decision
replayScriptArbParity
vitest run <files> --reporter=json, origin/main against this branch:ad-script-round-trip,scripts/fuzz/{corpus-replay,harness,envelope},ad-script/script.test: 89/89 -> 89/89 (5 files). No test file moved, so selection is unchanged. Novi.mock.Surface
No package
exportschanged; test-kit's single"."entry losesreplayScriptArb. No barrels orexport *. test-kit dependencies 6 -> 5.Gates
check:quick,check:layering,check:fallow,check:gate-manifest,pnpm format,check:affected --run: all pass on head.🤖 Generated with Claude Code