Skip to content

refactor(11a-2): package-local fakes at the daemon ports; root-own command-surface composition tests - #3519

Merged
thymikee merged 7 commits into
mainfrom
refactor/daemon-test-fakes
Oct 14, 2026
Merged

thymikee merged 7 commits into
mainfrom
refactor/daemon-test-fakes

Conversation

@thymikee

@thymikee thymikee commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

Summary

Second half of the daemon test-boundary prep (after #3498). This PR cuts its own clusters — command surface, platform-resource-cleanup, provider admission/credentials — from src/daemon/ test imports. It does not empty the daemon tree: the createRequestHandler test defaults (real root cleanup and claim recovery behind ~33 router suites) and the runtime-gateway/lifecycle, runtime-hints, host-diagnostics, claim-recovery, and perf-capture clusters are deferred to #3544 (stacked on #3517). The zero-edge target for src/daemon/ holds only at the top of the 11a stack (#3517 → #3544), not at this PR alone; the remaining residue below #3544 is the 11a-3 data-fixture cluster.

  • Command-surface split (evidence-driven): a canned CreateDaemonCommandSurface fake was installed and all 124 fixture users run against it. 42 files failed (266 tests): their assertions traverse real root command code, so they relocate to src/__tests__/daemon-composition/ (subtrees preserved) with the real factory, which itself moved there (command-surface-fixture.ts wraps root createCommandSurfaceAgentDevice). The 82 survivors get support/command-surface-fake.ts. Real-surface runners and the invokeFindHandler surface parameter keep the relocated suites meaningful. Owner review applied the same rule to find-read-only / find-ambiguous-match (selector find/wait route → root-owned) and re-swept all 388 src/daemon suites: none reaches selectors.*.
  • Cleanup port: one fixture per port — teardown/close suites inject platformResourceCleanupFixture() and assert the stopSnapshotHelper / closeManagedBrowser port members (rejecting override for the cleanup-failure channel). The composed adapter's platform filtering (Android stop, web-device close, non-web no-dispatch) is pinned root-owned under daemon-composition/, with the non-web case armed via the fake agent-browser and stderr.
  • Provider ports: daemon suites inject support/provider-credentials-fixture.ts (opaque deterministic fingerprint key, no copied digest format) and support/provider-device-admission-fixture.ts; the credential-digest family of lease.test.ts (real fingerprint + manifest behavior) split to daemon-composition/handlers/lease-provider-credentials.test.ts.

Validation

Head 8e599d3: check:affected --run, check:quick, check:layering, check:di-seams, check:gate-manifest, check:fallow, plus the eager-closure-budgets and test-file-size-ratchet pins, all pass. Full unit-core lane through the host lock: 13,469 tests / 0 failures. Name-parity vs the main baseline (5680da519, 13,422 names): every baseline assertion survives exactly once; the diffs are parameterized names carrying run-dependent ports/timestamps (matched 1:1) and names added/renamed by main's own rebased commits (#3500/#3504/#3505/#3493/#3473). Fresh-context self-review ran on the rebased diff: two dead-code findings fixed (the close fixture's snapshot-helper mock seam the port assertions superseded; the admission fixture's unused reset()); the enforcement-table commit-hygiene finding was rejected — folding it would rewrite commits the reviewer verified unchanged under a three-PR stack.

@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.20 MB 5.20 MB 0 B
Package (unpacked) 5.20 MB 5.20 MB 0 B
Package (download) 1.56 MB 1.56 MB +9 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.2 ms 27.1 ms -0.0 ms
CLI --help 76.8 ms 76.1 ms -0.7 ms

@thymikee

Copy link
Copy Markdown
Member Author

Thanks for this PR. At 2c7a8d0 I found two test regressions and one duplicated format, and unit-core and Coverage are red because of them.

The shared close fixture in session-close-shutdown.fixtures.ts (line 55) swaps the real platform cleanup for a no-op but keeps the old name through an as platformResourceCleanup alias. Teardown and close tests still assert that the Android snapshot-helper stop and the managed-browser close are called, but the no-op port never forwards to them. So session-teardown-resources.test.ts fails 4 tests and session-close-resource-cleanup.test.ts fails "close stops Android snapshot helper session". After this PR, nothing under src/daemon proves that close asks the platform to stop the snapshot helper or close the web session. The rule: every daemon suite that observes a platform finalization injects the port fixture and asserts the port member. Please use platformResourceCleanupFixture() in these suites, assert stopSnapshotHelper and closeManagedBrowser, and use a rejecting override for the cleanup-failure test. That export has no consumers today, which is what fallow flags. Any assertion that needs the real Android or web mechanics should move to daemon-composition. Please drop the alias so the swap is visible.

invokeFindHandler in find-handler-fixture.ts (line 25) now defaults to fakeCreateCommandSurface. find-read-only.test.ts and find-ambiguous-match.test.ts still run the selector find/wait route and were not moved, so the fake refuses loudly and 6 tests fail. This is the PR's own sorting rule (fails against the fake, so relocate), applied to find-production-route but not to these two files. Please move both to src/tests/daemon-composition/interaction/ with testCreateCommandSurface. Then re-run the whole fake-surface sweep and confirm that no src/daemon suite reaches selectors.*.

digest() in provider-credentials-fixture.ts (line 26) is a verbatim copy of digest() in src/provider-credential-fingerprint.ts (lines 62-69): the sha256, the 16-character slice and the v1: prefix. The fixture's own doc says digests are opaque to daemon policy, so the fake should not reproduce the format. Otherwise there are two copies that can drift apart. An opaque deterministic key would do, for example JSON.stringify of the sorted defined pairs, or undefined when empty.

Could the sort rule be applied all the way? Teardown suites inject platformResourceCleanupFixture and assert port calls, the two find suites move with their siblings, and credentials keep an opaque fake. Then the separate no-op constant and the hidden alias go away, and one fixture per port remains. I would also land the clusters listed as in progress in the PR body (runtime gateway, hints, diagnostics, claim recovery, perf) as separate PRs, so this one stays reviewable at 159 files.

The failing checks look related to this change. All 11 unit-core failures are in suites whose fixtures this PR rewired (session-close-shutdown.fixtures.ts and find-handler-fixture.ts), and Coverage fails with them. Lint & Format flags provider-device-admission-fixture.ts and lease.test.ts in oxfmt. Fallow flags the unused exports fakeCommandSurface and platformResourceCleanupFixture. Smoke Tests is still queued, and the PR does not touch any production route it runs.

I did not run the name-parity inventory, so I have not verified that the lease.test.ts to lease-provider-credentials split keeps every assertion. I also did not read all of the roughly 100 renamed test files. I relied on the rename stats and spot checks of import-only hunks. Since this is a draft with port clusters still in progress, this head may be intermediate.

Before merge, please wire platformResourceCleanupFixture into the teardown and close suites and move the two find suites to daemon-composition. Then run pnpm format and re-run fallow and unit-core.

thymikee added a commit that referenced this pull request Oct 12, 2026
…y suites root-side

Owner review on #3519:
- Teardown/close suites inject platformResourceCleanupFixture() and assert the
  stopSnapshotHelper / closeManagedBrowser port members, with a rejecting
  override for the cleanup-failure channel. The no-op constant and the
  as-platformResourceCleanup alias are gone; one fixture per port. The adapter's
  web-device filter (a composition decision) is pinned root-owned in
  daemon-runtime-web-close-teardown.
- find-read-only and find-ambiguous-match run the selector find/wait route, so
  they relocate to src/__tests__/daemon-composition/interaction/ behind
  testCreateCommandSurface, per the PR's own sorting rule.
- providerCredentialsFixture drops the copied digest format for an opaque
  deterministic key over the sorted defined pairs.
- fakeCommandSurface becomes module-private (fallow's unused export).
@thymikee

Copy link
Copy Markdown
Member Author

Addressed the review in 3916a64 (plus 46fef50, your own #3502 row-retirement cherry-pick — the three test-kit approval rows read stale against the updated merge-base):

  • Cleanup port: teardown/close suites now inject platformResourceCleanupFixture() and assert stopSnapshotHelper / closeManagedBrowser; the cleanup-failure test uses a rejecting override. No-op constant and the as platformResourceCleanup alias are deleted — one fixture per port, members are spies (overrides stay spy-visible). The adapter's web-device filter was composition, so the non-web no-dispatch assertion moved to daemon-composition/server/daemon-runtime-web-close-teardown; the agent-browser argv assertions were already pinned there.
  • Find suites: find-read-only and find-ambiguous-match moved to src/__tests__/daemon-composition/interaction/ with testCreateCommandSurface threaded through runFindClickScenario/invokeFindHandler. Fake-surface sweep re-run over all 388 src/daemon suites (2902 tests): no suite reaches selectors.* (the fake's loud refusal never fires) and no daemon file references the real factory.
  • Credentials fake: digest() replaced by an opaque deterministic key (JSON of the sorted defined pairs, undefined when empty); no sha256/v1: format copy left in the fixture.
  • Fallow/format: fakeCommandSurface is module-private; pnpm format run repo-wide; check:fallow, check:quick, check:layering, check:di-seams, check:gate-manifest all pass.
  • Scope: the in-progress clusters (runtime gateway, hints, diagnostics, claim recovery, perf) leave this PR; they land as WIP commits on a new refactor/daemon-runtime-fakes branch stacked on refactor/daemon-test-data-fakes (refactor(daemon): cut daemon test edges into root data fixtures and fakes [11a-3] #3517), PR base set there.

thymikee added a commit that referenced this pull request Oct 12, 2026
…y suites root-side

Owner review on #3519:
- Teardown/close suites inject platformResourceCleanupFixture() and assert the
  stopSnapshotHelper / closeManagedBrowser port members, with a rejecting
  override for the cleanup-failure channel. The no-op constant and the
  as-platformResourceCleanup alias are gone; one fixture per port. The adapter's
  web-device filter (a composition decision) is pinned root-owned in
  daemon-runtime-web-close-teardown.
- find-read-only and find-ambiguous-match run the selector find/wait route, so
  they relocate to src/__tests__/daemon-composition/interaction/ behind
  testCreateCommandSurface, per the PR's own sorting rule.
- providerCredentialsFixture drops the copied digest format for an opaque
  deterministic key over the sorted defined pairs.
- fakeCommandSurface becomes module-private (fallow's unused export).
@thymikee
thymikee force-pushed the refactor/daemon-test-fakes branch from 46fef50 to 57b6b9b Compare October 12, 2026 18:39
@thymikee

Copy link
Copy Markdown
Member Author

Base refreshed onto origin/main (the #3502 cherry-pick dropped as already applied); check:quick and the eager-closure budgets pass on the new head.

@thymikee

Copy link
Copy Markdown
Member Author

The earlier CI failures are fixed in 57b6b9b: the no-op cleanup fixture and the find fixture default are corrected, the 11 failing suites pass, and all 19 checks are green. No conflicts. One test gap remains, and the PR body still needs the final gate results before it leaves draft.

The Android teardown case in daemon-runtime-web-close-teardown.test.ts would not catch a wrong close. It installs no managed agent-browser and discards stderr. If someone removes the params.device.platform !== 'web' guard in closeManagedBrowser in src/platform-runtime-resource-cleanup.ts, runManagedAgentBrowser throws missingManagedToolError before any runCmd. Teardown reports that only as a web_browser failure on the discarded stderr, so expect(mockRunCmd).not.toHaveBeenCalled() still passes. The PR names this test as the root-owned pin for the composition filter, so that decision would have no real coverage. Please capture stderr and assert it is '', since a mistaken close surfaces /web_browser/ there. Alternatively, call installFakeManagedAgentBrowser(root) before teardown so a mistaken close reaches mockRunCmd.

Is one fake per daemon port the goal here (platformResourceCleanupFixture, providerCredentialsFixture, fakeCreateCommandSurface)? createRequestHandler in src/daemon/__tests__/test-device-runtime-gateway.ts still injects the real root platformResourceCleanup and createClaimRecoveryGateway into 33 router suites. Should that helper default to platformResourceCleanupFixture() and a claim-recovery fake in this PR, with suites that need real adapter behavior moved under src/__tests__/daemon-composition/? Or should the PR body drop the claim that no file under src/daemon/ imports a non-package src/ module until 11a-3 lands? The body also still lists the runtime/gateway, hints, diagnostics, claim-recovery and perf clusters as in progress, so the summary should match the code.

I did not run the test-name parity inventory against main or the mutation checks. I did not read all of the roughly 100 renamed files and relied on rename stats and the lease split's name comparison. I treated the upstream commits pulled in by the rebase (mutation tooling, limrun, appstate) as upstream and did not review them. I did not look further at the inline port literal in request-execution-scope-lease-expiry.test.ts, which is outside this delta.

Nothing else stops this delta. Before taking the PR out of draft, please record the final gate set and the name-parity result in the PR body. Also land or explicitly defer the in-progress clusters.

@thymikee

Copy link
Copy Markdown
Member Author

Follow-up in 773f78b:

  • Non-web teardown case now bites: the test installs the fake agent-browser (a mistaken close hits mockRunCmd) and asserts captured stderr is '' (the web_browser cleanup-failure channel). Proven: removing the device.platform !== 'web' guard in closeManagedBrowser fails the test on the mockRunCmd call; the production edit was not committed.
  • PR body: zero-edge wording corrected — this PR cuts its clusters; the createRequestHandler defaults (~33 router suites) and the gateway/hints/diagnostics/claim-recovery/perf clusters are deferred to refactor(daemon): package-local runtime fakes for the remaining daemon ports [11a-4] #3544, and the zero-edge target holds only at the top of the stack. "In progress" removed. Validation records the final gate set (quick/layering/di-seams/gate-manifest/fallow + eager-closure/size pins, all pass) and the re-measured lane: 1,580 files / 13,435 tests / 0 failures, name-parity against the main baseline holds (only parameterized run-dependent names and main's own rebased test additions differ).

@thymikee

Copy link
Copy Markdown
Member Author

This PR is ready, and I found nothing that needs to change at 773f78b. The earlier unit-core, Lint and Fallow failures were fixed at 57b6b9b, and all 18 checks are green. There are no conflicts. The last delta touches one test file only.

Not blocking: the comment at https://github.com/callstack/agent-device/blob/773f78b/packages/contracts/src/snapshot-runtime.test.ts#L117 still points to the old src/daemon/__tests__/wait-runtime.test.ts, so you can change it to src/__tests__/daemon-composition/wait-runtime.test.ts or leave it.

I did not run the mutation (removing the platform guard) or the test locally. I read closeManagedBrowser and the seam, and I rely on your description for the rest. The name-parity inventory against main and the unit-core counts (1,580 files, 13,435 tests) are your numbers, and I did not re-run them. I sampled assertions on the lease split only, not all of the roughly 100 renamed files.

Nothing else from review stands in the way. The PR is still a draft, so you decide when to mark it ready, and #3544 carries the deferred zero-edge work.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 13, 2026
@thymikee
thymikee added this pull request to stack #3564 October 13, 2026 06:02
thymikee added a commit that referenced this pull request Oct 13, 2026
…y suites root-side

Owner review on #3519:
- Teardown/close suites inject platformResourceCleanupFixture() and assert the
  stopSnapshotHelper / closeManagedBrowser port members, with a rejecting
  override for the cleanup-failure channel. The no-op constant and the
  as-platformResourceCleanup alias are gone; one fixture per port. The adapter's
  web-device filter (a composition decision) is pinned root-owned in
  daemon-runtime-web-close-teardown.
- find-read-only and find-ambiguous-match run the selector find/wait route, so
  they relocate to src/__tests__/daemon-composition/interaction/ behind
  testCreateCommandSurface, per the PR's own sorting rule.
- providerCredentialsFixture drops the copied digest format for an opaque
  deterministic key over the sorted defined pairs.
- fakeCommandSurface becomes module-private (fallow's unused export).
@thymikee
thymikee force-pushed the refactor/daemon-test-fakes branch from 773f78b to 8f501eb Compare October 13, 2026 06:23
@thymikee

Copy link
Copy Markdown
Member Author

Coordinator: rebased onto main c13c1bd, which was in conflict. Two main changes needed follow-up, both folded into the relocation commit (11a-2a):

  • fix(settle): observe a settled screen through the capture the request admitted #3553 added src/daemon/interaction/internal/__tests__/interaction-touch-prepare.test.ts, which uses the real command surface. Against the fake surface, all 5 of its tests fail, so by this PR's sort rule it moved unchanged to src/__tests__/daemon-composition/interaction/.
  • generic-settle.test.ts (already root-owned here) gained a selector-capture-fixture import on main, which now points at the daemon fixture path.
    The interaction-ios-tap-outcome import conflict was resolved by keeping main's new import plus this PR's relocated path. Checks: check:quick, check:layering and check:affected --run (1538 tests) all pass.

@thymikee

Copy link
Copy Markdown
Member Author

This PR is ready at 8f501eb. The rebase conflicts in the test import paths are resolved, and the earlier clean review of 773f78b still holds. All 19 checks pass. I did not run the tests or gates locally, so the 1,538 affected tests and the 13,435 unit-core tests are the author's counts. I did not re-audit the 164-file rename set. Commits 2-5 are unchanged, and commit 1 differs only in the expected rebase hunks. The rebase added no fakes, only a relocation. Not blocking, and you can take or leave it: the section comment at https://github.com/callstack/agent-device/blob/8f501eb/packages/contracts/src/snapshot-runtime.test.ts#L117 still says the poll-deadline behavior is proven in src/daemon/__tests__/wait-runtime.test.ts, which this PR moved to src/__tests__/daemon-composition/wait-runtime.test.ts, and it is the only old path I found still referenced. Nothing else stands in the way of merge. The PR is a draft waiting for your ready decision, and the deferred zero-edge work is tracked in #3544.

@thymikee

Copy link
Copy Markdown
Member Author

This still looks good at f13636c. Since the clean review at 8f501eb, the only change is a comment edit in packages/contracts/src/snapshot-runtime.test.ts. It now points at the relocated src/__tests__/daemon-composition/wait-runtime.test.ts, which exists at this head. That answers the earlier note.

The failing iOS smoke job timed out in runner_rebuild with prepare_deadline_expired. That is the runner prepare route, and this PR only changes tests, so the failure looks unrelated. Please re-run that job. There are no conflicts.

@thymikee
thymikee marked this pull request as ready for review October 13, 2026 10:45

@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 165 files

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

@thymikee

Copy link
Copy Markdown
Member Author

This PR is ready. The code in 8e599d3 looks good, and the fixes since the earlier review at f13636c hold: the removed mock and the two test fixtures in the new delta look correct. The PR has no production-code changes, and no conflicts. I did not re-verify the 165-file move from the earlier reviews, but the range-diff shows commits 1-6 unchanged. I did not run the test suite locally. I checked by grep only that no test still reaches the real Android helper stop. The only checks not passing are two Smoke Tests, still queued or running with no failure shown. The earlier failure was an iOS runner prepare timeout, and this change does not touch that route. Wait for the Smoke Tests to finish, re-run them if the timeout comes back, then merge.

Not blocking, and fine to take or leave: lease-provider-credentials.test.ts copies allocateRequest() from lease.test.ts, which still uses its own copy, so one shared builder in src/daemon/__tests__/support would remove the duplicate.

@thymikee

Copy link
Copy Markdown
Member Author

All review items closed at 8e599d3 — 19/19 checks green, mergeable, no open threads:

  • Stale pin comment in packages/contracts/src/snapshot-runtime.test.ts repointed at the relocated wait-runtime suite (f13636c); tree-wide sweep found no other stale relocated-path references.
  • Fresh-context self-review of the full rebased diff: removed the close fixture's now-dead stopAndroidSnapshotHelperSessionForDevice mock seam/alias and the admission fixture's unused reset() (8e599d3). The enforcement-table commit-hygiene finding was rejected deliberately — folding it would rewrite commits review verified unchanged under the three-PR stack.
  • Validation on this head: check:affected --run + quick/layering/di-seams/gate-manifest/fallow + eager-closure/size pins pass; unit-core lane 13,469 tests / 0 failures; baseline name-parity holds (only run-dependent parameterized names and main's own rebased additions differ).

thymikee added a commit that referenced this pull request Oct 13, 2026
…y suites root-side

Owner review on #3519:
- Teardown/close suites inject platformResourceCleanupFixture() and assert the
  stopSnapshotHelper / closeManagedBrowser port members, with a rejecting
  override for the cleanup-failure channel. The no-op constant and the
  as-platformResourceCleanup alias are gone; one fixture per port. The adapter's
  web-device filter (a composition decision) is pinned root-owned in
  daemon-runtime-web-close-teardown.
- find-read-only and find-ambiguous-match run the selector find/wait route, so
  they relocate to src/__tests__/daemon-composition/interaction/ behind
  testCreateCommandSurface, per the PR's own sorting rule.
- providerCredentialsFixture drops the copied digest format for an opaque
  deterministic key over the sorted defined pairs.
- fakeCommandSurface becomes module-private (fallow's unused export).
@thymikee
thymikee force-pushed the refactor/daemon-test-fakes branch from 8e599d3 to df0af2a Compare October 13, 2026 15:10
@thymikee

Copy link
Copy Markdown
Member Author

Coordinator: rebased onto main 352ad624d (18 new commits, including the vitest 5 upgrade #3549). One conflict, in src/daemon/__tests__/request-router-developer-dir.test.ts: main rewrote test.sequential(...) as test(..., { concurrent: false }, ...), while this PR swapped the real command surface for fakeCreateCommandSurface. Resolved by keeping both. No other content changes. Verified: check:quick, check:layering, and check:affected --run (235 files, 1553 tests) pass. New head df0af2a13.

@thymikee

Copy link
Copy Markdown
Member Author

The PR is ready to merge at df0af2a. The earlier clean review of 8e599d3 still holds, and the new commits are a rebase plus a trivial conflict resolution. All 19 CI checks pass, and I know of no conflicts. I did not rerun the test gates. I rely on the green CI and on your local check:affected run.

Not blocking: the real android leg of platformResourceCleanup.stopSnapshotHelper is no longer pinned by any root-owned test, since the close and teardown tests now use port spies and the mechanics mock is gone (d5aac6e re-pinned the web leg, and the iOS sessionless leg is covered). If you want it back, add one root-owned case that mocks the android mechanics module, calls the real stopSnapshotHelper, and asserts an android device reaches stopAndroidSnapshotHelperSessionForDevice(device) while a non-android device does not. You can take this or leave it.

Nothing else must happen before merge.

Relocate the 42 daemon test files whose assertions traverse real root
command code (proven: they fail against a canned CreateDaemonCommandSurface
and pass against the real factory) into src/__tests__/daemon-composition/,
preserving subtrees, and move command-surface-fixture itself there since it
wraps the root createCommandSurfaceAgentDevice factory.

The remaining 82 daemon suites now inject a package-local fake surface at
the same port (support/command-surface-fake): inert capture shapes and loud
selector refusals, matching the pilot evidence for every surviving suite.
The real-surface interaction runners move to a sibling root-owned module,
and invokeFindHandler grows an explicit surface parameter so the relocated
find-production-route suite keeps driving the composed find->interaction
leg. Dispatch-disclosure driver ownership and the boundary-fault evidence
path follow the relocated files.
Daemon suites pass the composed cleanup capability down as a port value and
assert which finalization the neutral orchestration requested, never how a
platform performs it. They now inject a package-local noop fixture at that
port (support/platform-resource-cleanup-fixture, with per-member overrides).
snapshot-session-cleanup.test.ts asserts the real adapter's platform
branching over mocked platform packages, so it relocates to the root-owned
composition subtree instead of being converted.
…n ports

Daemon suites treat the credential fingerprint as opaque — they assert
refusal/reuse on equality, never digest content — so request-router-open,
human-control-router-fixture, and the router fixture default now inject a
package-local fixture at DaemonProviderCredentials that digests the given
environment. The lease suite's provider-credentials family asserts the real
reader's provider/manifest-aware digest semantics (and the plugin.fixtures
manifest registry), so it moves to the root-owned composition subtree as
lease-provider-credentials.test.ts.

Provider ownership arrives through the daemon's own ProviderDeviceAdmission
install point: snapshot-settings-handler and session-relaunch-close install
a package-local admission over runtime fakes instead of the root ambient
provider-runtime scope.
…y suites root-side

Owner review on #3519:
- Teardown/close suites inject platformResourceCleanupFixture() and assert the
  stopSnapshotHelper / closeManagedBrowser port members, with a rejecting
  override for the cleanup-failure channel. The no-op constant and the
  as-platformResourceCleanup alias are gone; one fixture per port. The adapter's
  web-device filter (a composition decision) is pinned root-owned in
  daemon-runtime-web-close-teardown.
- find-read-only and find-ambiguous-match run the selector find/wait route, so
  they relocate to src/__tests__/daemon-composition/interaction/ behind
  testCreateCommandSurface, per the PR's own sorting rule.
- providerCredentialsFixture drops the copied digest format for an opaque
  deterministic key over the sorted defined pairs.
- fakeCommandSurface becomes module-private (fallow's unused export).
Owner follow-up: install the fake agent-browser so a mistaken close reaches the
process seam, and assert stderr stays empty (the cleanup-failure channel is how
such a close surfaces). Proven against a guard-removed closeManagedBrowser: the
test fails on the mockRunCmd call; the production edit was not committed.
Follow-up on the ready review: the contract suite's section comment still named
src/daemon/__tests__/wait-runtime.test.ts, which this PR moved to
src/__tests__/daemon-composition/wait-runtime.test.ts.
Self-review of the full diff: with every cleanup injection point taking
platformResourceCleanupFixture(), the close-shutdown fixture no longer reaches
the Android snapshot-helper mechanics entry, so its mock override, vi.mocked
wrapper, and exported alias are dead; the admission fixture's reset() had no
consumer and re-spelled setActive([]).
@thymikee

Copy link
Copy Markdown
Member Author

Coordinator: rebased onto main 15cfe9fa2, which had conflicts again. Main added one import to each of two tests this PR relocates: pendingSurfaceStabilization in generic-settle.test.ts and movingTargetBaseline in interaction-ios-tap-outcome.test.ts, both from deferred-interaction-outcome.ts. Resolved by keeping main's imports with their paths recomputed for the tests' root-owned location. No other content changes. Verified: check:quick, check:layering, and check:affected --run (235 files, 1559 tests) pass. New head af6925d20.

@thymikee
thymikee force-pushed the refactor/daemon-test-fakes branch from df0af2a to af6925d Compare October 13, 2026 17:56
thymikee added a commit that referenced this pull request Oct 13, 2026
… the mechanics leaf

From #3519's review, landed here so the daemon lane keeps no root mock:
one root-owned case calls the REAL platformResourceCleanup.stopSnapshotHelper
over the mocked android mechanics leaf and asserts an Android device
reaches stopAndroidSnapshotHelperSessionForDevice(device) while a
non-Android device does not. Both legs were proven to bite by
temporarily inverting and then deleting the platform guard.
@thymikee

Copy link
Copy Markdown
Member Author

This PR is ready. The earlier review at df0af2a was clean, and af6925d is a rebase with two import-path edits, so nothing new needs fixing.

Not blocking: lease-provider-credentials.test.ts still re-declares a small allocateRequest() that lease.test.ts keeps for its own cases, and you can share it through a daemon support fixture if a third user appears or leave it as is.

CI is still pending for Lint & Format, Coverage, Integration Tests and Smoke, with no failures so far. A failure would most likely come from import resolution in the relocated suites. I did not run the test suites myself, and I did not re-check the 13,469-test name-parity count or your local check:quick, check:layering and check:affected results. There are no conflicts. Merge once those four checks finish green.

thymikee added a commit that referenced this pull request Oct 13, 2026
… the mechanics leaf

From #3519's review, landed here so the daemon lane keeps no root mock:
one root-owned case calls the REAL platformResourceCleanup.stopSnapshotHelper
over the mocked android mechanics leaf and asserts an Android device
reaches stopAndroidSnapshotHelperSessionForDevice(device) while a
non-Android device does not. Both legs were proven to bite by
temporarily inverting and then deleting the platform guard.
thymikee added a commit that referenced this pull request Oct 14, 2026
… the mechanics leaf

From #3519's review, landed here so the daemon lane keeps no root mock:
one root-owned case calls the REAL platformResourceCleanup.stopSnapshotHelper
over the mocked android mechanics leaf and asserts an Android device
reaches stopAndroidSnapshotHelperSessionForDevice(device) while a
non-Android device does not. Both legs were proven to bite by
temporarily inverting and then deleting the platform guard.
thymikee added a commit that referenced this pull request Oct 14, 2026
… the mechanics leaf

From #3519's review, landed here so the daemon lane keeps no root mock:
one root-owned case calls the REAL platformResourceCleanup.stopSnapshotHelper
over the mocked android mechanics leaf and asserts an Android device
reaches stopAndroidSnapshotHelperSessionForDevice(device) while a
non-Android device does not. Both legs were proven to bite by
temporarily inverting and then deleting the platform guard.
@thymikee
thymikee merged commit 22d9934 into main Oct 14, 2026
19 checks passed
@thymikee
thymikee deleted the refactor/daemon-test-fakes branch October 14, 2026 07:54
thymikee added a commit that referenced this pull request Oct 14, 2026
…n ports [11a-4] (#3544)

* refactor(daemon): fake the host-diagnostics, perf-capture, android-observation, and platform-services ports in daemon suites

Daemon suites no longer import root platform-runtime modules: the
host-diagnostics, perf-capture, android-observation, and platform-services/
claim-recovery edges are answered by daemon-local port fixtures whose members
are spies or resolve through platform-package leaves (Safari/deep-link rules,
iOS app resolution, android package mechanics), while suites that pin what the
real composed root modules do keep a root-owned composition fixture at
src/__tests__/daemon-composition/.

Relocated to the composition lane: the perf-capture session-resource suite,
readiness spawn suites, the runner-retention trio, the foreign-dead-owner
claim-recovery cases, and three android system-dialog suites converted to port
spies where their assertions were daemon policy.

* refactor(daemon): fake the application-lifecycle runtime-binding port in daemon suites

The daemon lane now binds a canned lifecycle fixture built on the contracts
direct-lifecycle binding plus port spies (readiness pause, android discovery,
dispatch, hint apply/clear, runner prepare). The real composed fixture moves to
the root-owned daemon-composition lane, built on the same seam set so both lanes
observe one spy family. Hint-observation suites retarget to the fixture's port
spies (assertion bodies unchanged); suites whose assertions pin what the real
apple/android runtime modules do under composition relocate:
session-relaunch-close and session-open-url-prewarm whole-file, and the
close-finalization, shutdown-gating, recording-finalization, post-open
observation, runtime-hint lazy-evaluation, and android IME cancellation cases
as extracted composition siblings. The gateway and session-command harness take
an injectable lifecycle-operations loader so composition suites build the real
gateway behind the same handler harness.

Scan for daemon -> non-package src/ edges: 0.

* refactor(daemon): delete the five dead root-module mocks from daemon suites

The four replay-repair suites mocked clearRuntimeHintValues out of
platform-runtime-runtime-hints.ts, and session-open-runtime patched
createAppleApplicationTools for a prewarm spy. Nothing under src/daemon
imports either module any more (hint apply/clear reach the device through
the admitted binding's operations, observed at the fixture port spies),
so the mocks answered no call. Deleted; the corrected residue scan now
reports 0 edges.

* refactor(daemon): answer the platform-services and lifecycle ports with canned spies

Per review: the composition lane owns the platform rules, so the daemon
fixtures stop re-implementing them. Every services/lifecycle member is
now a plain vi.fn spy with a canned default a test overrides per case:

- platform-services-fixture: appleSessionObservation, deviceBootObservation,
  ensureLocalDeviceReady, resolveRequestedOpenSurface,
  validateOpenRelaunchTarget and resolveSessionAppBundleIdForTarget are
  spies; the open-target/surface/readiness/boot rule copies are deleted,
  and the shared fixtureResolveSessionAppBundleIdForTarget the lifecycle
  fixture imported goes with them.
- application-lifecycle-runtime-fixture: resolveOpenTarget,
  prepareApplicationOpen and openApplication are spies delegating to the
  contract's canonical direct-lifecycle members; the macOS frontmost rule
  and the android stopped-AVD discovery branch are gone.

Tests that pinned real rules move to the composition lane unchanged:
8 cases to session-lifecycle/session-open-existing.test.ts, 5 relaunch
guards to handlers/session-relaunch-guards.test.ts, and device-ready
whole-file (it pins what the composed readiness port does per family).
11 cases keep their names and assertion bodies daemon-side, retargeted
to the port spies (boot observation, open-target identity per case,
readiness failure injection). Suites under src/__tests__/daemon-composition
now use the lane's own root-composed services fixture.

Per-file test-name parity holds (2744 names, moved or kept); scan edges 0.

* chore(gates): commit the daemon residue scan the 11a slices measure with

The review could not find the scan the PR body reported, and the previous
ad-hoc copy under-reported edges: its single alternation bridged
`import|export … [\s\S]*? from`, so a vi.mock() specifier sitting between an
importOriginal type-import and the file's next static from clause was
swallowed by the earlier match and never reported. Five mocks of root
platform-runtime modules went unnoticed under it.

This scan matches every specifier form independently — statement-anchored
static import/export, side-effect import, dynamic import()/require(),
vi.mock/doMock/unmock/importMock/importActual/importOriginal with string or
template-literal arguments, and importOriginal<typeof import()> — and exits
1 on any relative edge leaving src/daemon. Run: node
scripts/daemon-edge-residue-scan.mjs. 11b's R11 package-boundary rule
(#3546) makes it permanent once the daemon is its own package.

* refactor(daemon): keep the canned-answer spies unexported and register the scan entry

Only the spies suites actually observe per case are exported; the rest stay
module-private behind the fixture factory. The residue scan joins the
.fallowrc entry list beside the other hand-run scripts.

* refactor(daemon): observe the boot probe at the services port in the open router suite

request-router-open pinned the reboot-stale claim answers by mocking the
apple simulator-boot leaf, which the daemon no longer loads; both cases now
arm observeBootTimeMsFixture, the port member the claim settlement actually
asks.

* refactor(daemon): take the harness lifecycle loader as a parameter, one shared type

Per review: the session-command harness picks its lifecycle-operations
loader the same way the gateway factories do — a per-call parameter —
so the module-global install (setHarnessLifecycleOperationsLoader), the
side-effect install module, and the direct install call in
session-open-existing.test.ts are gone, and nothing depends on import
order or per-file module isolation to pick the real composed lifecycle.
Composition suites pass applicationLifecycleRuntimeFixture through the
new handleSessionCommands lifecycleOperationsLoader option.

LifecycleOperationsLoader is declared once, exported from the daemon
lifecycle fixture; the harness and the gateway factories both import it
instead of declaring their own.

Callers of test-device-runtime-gateway.ts import the unavailable* helpers
from support/unavailable-device-runtime-gateway.ts directly; the gateway's
re-export is deleted.

* refactor(daemon): move the android discovery seam to the composition fixture

Nothing in the daemon lane calls discoverReadyAndroidEmulators any more
(the canned prepareApplicationOpenFixture performs no discovery leg), so
the spy moves to the composition fixture that still drives the real
android readiness through it. Dead import/reset lines in the two open
router suites go with it, and the readiness-pause doc now names
prepareApplicationOpenFixture as the daemon-lane failure seam.

* test(composition): pin the composed snapshot-helper stop rule against the mechanics leaf

From #3519's review, landed here so the daemon lane keeps no root mock:
one root-owned case calls the REAL platformResourceCleanup.stopSnapshotHelper
over the mocked android mechanics leaf and asserts an Android device
reaches stopAndroidSnapshotHelperSessionForDevice(device) while a
non-Android device does not. Both legs were proven to bite by
temporarily inverting and then deleting the platform guard.

* fix(gates): report residue edges at the keyword line and on native paths

The static forms anchor at the preceding ';' or newline, so the line count
from match.index pointed one line early for every anchored edge; count to
the keyword inside the match instead. The daemon-local skip compared against
forward slashes, which on Windows turned every daemon-local relative import
into a phantom edge; build the prefix with path.join and path.sep.

Generated with assistance.

* test(composition): park the orphan recorder on the current Node binary

The unit-core lane runs on Windows, where `sleep` is not a command and the
case failed before exercising recovery; `process.execPath` on a parked timer
spawns everywhere and still carries a real command and start time.

Generated with assistance.

* refactor(daemon): keep the fixture's relaunch order close, hints, open

Applying the admitted hints before the direct owner's close-before-relaunch
reversed the sequence every real owner sequences (apple closes, applies
hints, then opens). The fixture now closes first itself and opens as a
fresh launch so daemon suites cannot encode a reversed lifecycle.

Generated with assistance.

* refactor(daemon): carry the request signal into lifecycle gateway binds

Dropping scope.signal handed the loader a fresh signal, so a canceled
request left its bound lifecycle operations running; every bind now passes
the request's signal through the loader exactly like the composed gateway.

Generated with assistance.

* refactor(daemon): answer exact-owner binds with the requested owner

The unavailable fixture replaced every owner with the local one, so
exact-owner refusal tests never exercised the identity the caller selected;
an exact-owner intent now keeps intent.owner.

Generated with assistance.

* test(composition): prove lazy runtime-hints loading before the first reset

Resetting the evaluated flag in beforeEach erased the only evidence that
the mocked module had not been evaluated during test-module loading; the
first entry into a test now asserts the flag is still false, so a
regression to eager evaluation fails there instead of passing silently.

Generated with assistance.

* test(composition): restore the claims-dir override after each case

The suite left AGENT_DEVICE_CLAIMS_DIR pointing at its temp root, letting
later tests in the same worker inspect claims under the wrong root; the
previous value is captured at setup and restored in afterEach.

Generated with assistance.

* test(composition): drive close retention through the composed cleanup

The harness injected the cleanup fixture whose retention answer is a canned
false, so a policy that ignored hasScreenRecording still passed; these
composition cases now pass the production-composed platformResourceCleanup
and observe its real retention decision at the runner release.

Generated with assistance.

* test(daemon): answer the open-target identity once per case

The persistent per-case implementation survived clearAllMocks, so later
opens inherited this case's canned bundle id instead of the fixture's
directResolve answer; the override is now a one-shot implementation.

Generated with assistance.

* docs(tests): say what the one-shot rejection and the TTL pin actually cover

The runner-stop comment implied a retried call the test never makes (the
one-shot rejection is consumed by the pre-close stop; cleanup uses the
separate releaseRunnerOnClose port), and the readiness comment credited the
claim suites with TTL assertions this suite owns.

Generated with assistance.

* docs(tests): scope the post-open observation suite to what it composes

The suite injects the observation verdict at the binding seam (the composed
probe needs a live Simulator bridge), so its header said it pins more of
the Apple open than it does; name the canned seam and point the real
verdict reporting at the platform lane that pins it.

Generated with assistance.

* test(composition): put the interaction-prepare and integration suites back on the real port

interaction-touch-prepare.test.ts kept its import of the daemon's canned platform-services
fixture when the suite moved to the composition lane, so it answered port asks with canned
spies instead of the composed adapters its 30 sibling suites see. The two integration
fixtures moved from the real root fixture to the canned one silently when the canned module
took over the export name, because their import path and the call site never changed. All
three now import the root composition fixture: the command suites exercise the real
adapters and override a member only where the test is about that ask.

* test(composition): name the root fixtures after their lane

Both fixture lanes exported the same names: daemonPlatformServicesFixture and
applicationLifecycleRuntimeFixture existed as canned spies under the daemon and as the real
composed ports at the root, so an import that drifted between the two trees stayed invisible
in review — the call site read the same. The root composition fixtures now export
composedPlatformServicesFixture, composedClaimRecoveryGatewayFixture, and
composedLifecycleRuntimeFixture, and every root call site carries the new name, so each
import spells which lane it answers and a stray swap shows as a rename. The daemon's canned
fixtures keep their names for the daemon lane.

---------

Co-authored-by: apex-agent <apex@callstack.dev>
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-14 07:55 UTC

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