Repository navigation
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 81 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
2facf69 to
d411742
Compare
Size Report
Startup median (7 runs, lower is better):
|
|
|
The move drops real wait coverage at a47c287, so this is not ready to merge yet. The old daemon-composition wait-runtime.test.ts had 31 tests. At head I cannot find tests for runner-restart exhaustion, readiness exhaustion (runner-start and target-discovery), the steady-state-cancelled cases, the deadline abort as a TimeoutError, "the poll awaits the aborted capture", or the capture-stalled cases. A repo-wide grep for runnerRestartExhausted, readinessExhausted and readinessPhase finds no unit test that drives them. So the branches in https://github.com/callstack/agent-device/blob/a47c287/src/commands/interaction/runtime/wait-polling.ts#L236 (restart) and https://github.com/callstack/agent-device/blob/a47c287/src/commands/interaction/runtime/wait-polling.ts#L246 (readiness), and the deadline-abort ordering, are now unpinned. A regression in wait failure classification would pass CI. wait-selector.test.ts and wait-stable.test.ts cover only the landmark-refusal and stable-wait stall paths. The PR body says no assertion was dropped, but the title diff does not support that, and the mutation table cannot see missing tests. Please give every wait failure class a root-owner test that drives it, with the abort and TimeoutError cases in wait-deadline.test.ts. Then rerun the old-to-new title diff and list each merged or retired test with its destination in the PR body. Please spot-check generic-settle the same way, since the scroll and back sheet cases now rest on press-only equivalents. Not blocking, take or leave: the './testing/snapshot-interactor-capture' export in packages/daemon/package.json has no consumer left, because its four importers moved into the daemon package, so it can be deleted. The inline thread at #3742 (comment) no longer applies, because the duration wait now uses fake timers, so it can be resolved. All 23 checks pass, but the suites that lost tests no longer exist, so green CI cannot show the missing wait coverage. I ran no tests and no mutations in this review, so I did not verify the 52-suite mutation table or the 1,291-test pass. I compared tests by title, and I confirmed the drops only for wait-runtime; the rewritten settle, is-runtime, platform-providers and open-claim-recovery tests are not diffed assertion by assertion. Before merge, the dropped wait tests need to be restored or ported with a per-test mapping. |
a47c287 to
03d2a9e
Compare
|
Addressed at
|
Summary
Moved the daemon-composition suites to daemon owners, root command/module owners, and one public daemon composition test. The daemon package drops the 36 test exports freed by these consumers plus the unreferenced
snapshot-interactor-capturetest export. Shared daemon fakes gained a default-preserving option; the eager import-closure fixture now belongs to test-kit.Test count and ownership evidence
Original paths below are under src/tests/daemon-composition. Old counts are the counts at b7c93e3. Head counts are the complete destination suite counts and include any tests that already lived there; rows are not additive.
The application-lifecycle fixture/gateway and obsolete generic-settle/sheet-tree fixtures were retired after their consumers moved. The eager import-closure fixture moved to packages/test-kit. Its root and scripts consumers remain selected.
Title-level mapping for the reviewed suites
Each old test case in the wait, generic-settle, is-runtime, platform-provider, and open-claim-recovery suites maps to the destination that now drives its owning branch. Parameterized rows list every expanded case. No test in these suites was retired.
packages/daemon/src/execution/wait-runtime.test.ts::sleep-only wait records in its admitted lifetime across %spackages/daemon/src/execution/wait-runtime.test.ts::sleep-only wait records in its admitted lifetime across %spackages/daemon/src/execution/wait-runtime.test.ts::a duration wait inspects no facts and binds no devicepackages/daemon/src/execution/wait-runtime.test.ts::an observing wait binds through the shared selector entrypackages/daemon/src/execution/wait-runtime.test.ts::a text wait binds the capture use once and polls through the bound operationpackages/daemon/src/execution/wait-runtime.test.ts::a selector wait binds the capture use once and polls through the bound operationpackages/daemon/src/execution/wait-runtime.test.ts::a @ref wait binds the capture use once and polls through the bound operationpackages/daemon/src/execution/wait-runtime.test.ts::a stable wait binds the capture use once and polls through the bound operationpackages/daemon/src/execution/wait-runtime.test.ts::strict wait absent refuses --%s before device admissionpackages/daemon/src/execution/wait-runtime.test.ts::strict wait absent refuses --%s before device admissionpackages/daemon/src/execution/wait-runtime.test.ts::an unavailable capture fact refuses an observing wait before bindingpackages/daemon/src/execution/wait-runtime.test.ts::a provider owner that cannot capture fails closed instead of borrowing the local runtimepackages/daemon/src/execution/wait-runtime.test.ts::a timed-out wait decorates its failure through the same single bindingsrc/commands/interaction/runtime/wait-text.test.ts::a text wait is satisfied by the owner native reading when the tree never carries itsrc/commands/interaction/runtime/wait-text.test.ts::a native text hit does not make an older captured tree fresh for the next readpackages/daemon/src/execution/wait-runtime.test.ts::a text wait returns a native owner hit without capturing a snapshotsrc/commands/interaction/runtime/wait-text.test.ts::runtime wait falls back to a snapshot after a native text misspackages/daemon/src/execution/wait-runtime.test.ts::an unavailable conditional native reading preserves the capture-backed wait pathpackages/daemon/src/execution/wait-runtime.test.ts::native text failure cannot hide a canonical capture failurepackages/daemon/src/execution/wait-runtime.test.ts::an unavailable conditional native reading preserves the capture-backed wait pathpackages/daemon/src/execution/wait-runtime.test.ts::the native text reading runs in the same runner context as the capturesrc/commands/interaction/runtime/wait-deadline.test.ts::a poll deadline aborts the capture with a TimeoutError reasonsrc/commands/interaction/runtime/wait-deadline.test.ts::the wait deadline awaits the capture after aborting itsrc/commands/interaction/runtime/wait-text.test.ts::a stalled capture reports capture-stalled with no readable capturessrc/commands/interaction/runtime/wait-absent.test.ts::strict wait absent reports capture-stalled when every capture stallssrc/commands/interaction/runtime/wait-polling.test.ts::runner restart exhaustion reports typed evidence without a readable pollsrc/commands/interaction/runtime/wait-absent.test.ts::strict wait absent keeps runner-restart exhaustion as the deadline reasonsrc/commands/interaction/runtime/wait-absent.test.ts::strict wait absent preserves runner-restart exhaustion over an earlier present capturesrc/commands/interaction/runtime/wait-polling.test.ts::readiness exhaustion preserves the %s phasesrc/commands/interaction/runtime/wait-polling.test.ts::readiness exhaustion preserves the %s phasesrc/commands/interaction/runtime/wait-absent.test.ts::strict wait absent preserves readiness exhaustion over an earlier present capturesrc/commands/interaction/runtime/wait-polling.test.ts::an earlier retriable refusal outranks readiness work on the final pollsrc/commands/interaction/runtime/wait-absent.test.ts::strict wait absent keeps target-present evidence when a steady-state capture is cancelledsrc/commands/interaction/runtime/wait-text.test.ts::a positive wait whose steady-state capture is cancelled reports the deadline, not readinesssrc/commands/interaction/runtime/wait-text.test.ts::runtime wait times out on text the polled snapshot does not carrypackages/daemon/src/execution/generic-settle.test.ts::scroll --settle passes the stored baseline and attaches the observation with its issued refspackages/daemon/src/execution/generic-settle.test.ts::back --settle attaches the observation alongside the command resultsrc/commands/interaction/runtime/settle-generic-surface.test.ts::targetless scroll settle discloses the app-to-sheet surface changesrc/commands/interaction/runtime/settle-generic-surface.test.ts::targetless scroll settle discloses the sheet-to-app surface changesrc/commands/interaction/runtime/settle-generic-surface.test.ts::targetless back settle discloses the app-to-sheet surface changesrc/commands/interaction/runtime/settle-generic-surface.test.ts::targetless back settle discloses the sheet-to-app surface changesrc/commands/interaction/runtime/settle-generic-surface.test.ts::targetless back settle confirms a dismissed sheet from the interactive baselineand...from the full baselinesrc/commands/interaction/runtime/settle-generic-surface.test.ts::targetless back settle keeps the plain window when the baseline presented nothingpackages/daemon/src/execution/generic-settle.test.ts::generic settle drops an outdated presented-surface fact but keeps its stored nodesandsrc/commands/interaction/runtime/settle-generic-surface.test.ts::targetless back settle confirms a broad replacement when the baseline fact is unknownpackages/daemon/src/execution/generic-settle.test.ts::the settle observation runs after the post-gesture stabilization markerpackages/daemon/src/execution/generic-settle.test.ts::scroll --settle observes through the request-bound capture, once admittedpackages/daemon/src/execution/generic-settle.test.ts::scroll --settle still observes when the owner cannot capturepackages/daemon/src/execution/generic-settle.test.ts::scroll without --settle takes no observation captures and issues no refspackages/daemon/src/execution/generic-settle.test.ts::an ended generic lifetime is refused before dispatch or settle constructionpackages/daemon/src/execution/generic-settle.test.ts::a generic command without the observation trait ignores a stray settle flagpackages/daemon/src/execution/generic-settle.test.ts::an orphaned --settle-quiet is rejected before the command dispatchespackages/daemon/src/execution/generic-settle.test.ts::scroll stays successful when its lifetime retires during optional settle observationpackages/daemon/src/execution/generic-settle.test.ts::held generic settle capture respects its lifetime after ${change}packages/daemon/src/execution/generic-settle.test.ts::held generic settle capture respects its lifetime after ${change}src/commands/interaction/runtime/settle-default-quiet.test.ts::a settle the simulator snapshot bridge performs uses the shorter default windowsrc/commands/interaction/runtime/settle-default-quiet.test.ts::the Apple runner tree read (same channel, different producer) keeps the 500ms default windowsrc/commands/interaction/runtime/settle-default-quiet.test.ts::a runner read during settle keeps the 500ms window despite a bridge-issued baselinepackages/daemon/src/execution/selector-runtime-is-admission.test.ts::an admitted is inspects once, binds once, and answers through the bound capturepackages/daemon/src/execution/selector-runtime-is-admission.test.ts::is absent binds one unscoped readAny capture without rectanglespackages/daemon/src/execution/selector-runtime-is-admission.test.ts::is absent bypasses the cached snapshot and binds a fresh readAny capturesrc/commands/interaction/runtime/selector-is.test.ts::runtime is absent fails closed for a quality-less legacy iOS root-only capturepackages/daemon/src/execution/selector-runtime-is-admission.test.ts::is absent rejects depth and scope before binding with typed invalid argumentspackages/daemon/src/execution/selector-runtime-is-admission.test.ts::an unavailable capture fact refuses before any bindpackages/daemon/src/execution/selector-runtime-is-admission.test.ts::an iOS session with no tracked app is refused with the open hint, not answered from a displaced capturepackages/daemon/src/execution/selector-runtime-is-admission.test.ts::an iOS session WITH a tracked app still answers, so the refusal is the plan split and not an iOS banpackages/daemon/src/execution/selector-runtime-is-admission.test.ts::an Android session with no tracked app proceeds, because the owner advertises the without-active-app capturepackages/daemon/src/execution/selector-runtime-is-admission.test.ts::a refused request reaches the device by no route at allpackages/daemon/src/execution/selector-runtime-is-reads.test.ts::a failing is predicate is COMMAND_FAILED, never a zero-exit passpackages/daemon/src/execution/selector-runtime-is-admission.test.ts::a miss on a surface that never settled carries the unsettled fact, and the re-read captures afreshpackages/daemon/src/execution/selector-runtime-is-admission.test.ts::a read after a scroll that moved nothing carries the no-effect outcome, and the re-read reuses its settled treesrc/platform-runtime-provider-scope.test.ts::Apple provider scope uses the request device for Apple tool callspackages/daemon/src/server/request-platform-provider-context.test.ts::request provider context follows an explicit device selector for an existing sessionpackages/daemon/src/server/request-platform-provider-context.test.ts::request provider context skips sharded test orchestration requestssrc/platform-runtime-provider-scope.test.ts::concurrent provider scopes stay isolatedsrc/platform-runtime-provider-scope.test.ts::web provider scope uses the context devicesrc/platform-runtime-provider-scope.test.ts::generic Apple runner provider cannot borrow local recording authoritypackages/daemon/src/server/request-platform-provider-context.test.ts::request provider context preserves an explicitly empty request idsrc/platform-runtime-provider-scope.test.ts::Apple runner recording authority stays exact across recreated scopespackages/daemon/src/server/request-platform-provider-context.test.ts::request provider context follows an explicit sessionless web selectorpackages/daemon/src/server/__tests__/request-router-open-claim-recovery.test.ts::open composes claim recovery from the foreign owner state directoryThe daemon snapshot suite retains its response-success and stored-outcome assertions. Its user-facing no-effect warning assertion belongs to the root snapshot command suite, so it now lives in
src/commands/capture/runtime/snapshot.test.tsas an explicit no-effect warning case.Additional changed suites needed by the test-kit move: src/tests/daemon-composition/session-teardown-import-closure.test.ts: 3; scripts/tests/closure-growth-rule.test.ts: 13; scripts/tests/eager-closure-budgets.test.ts: 902; src/tests/cli-startup-import-closure.test.ts: 22; src/tests/snapshot-desktop-surface.test.ts: 4.
Classification: (a) daemon-owned tests now sit beside daemon modules and use package fakes; (b) only daemon-runtime-limrun-public-http.test.ts drives composed daemon behavior, through startDaemonRuntime and injected ports; (c) root command, platform gateway, and provider/plugin behavior sits beside its root owner. The shared command-surface option onlyStoreWhenSessionExists defaults to false. Rebased onto origin/main at 4c1c786, which includes #3728's seven daemon-export deletions. #3740 owns the Linux uninstall accepted limitation; its duplicate test and declaration change are absent from this branch.
Move commits and selection
Pure move commits precede edits: 1fab2e2 (27 R100 test moves), 12dba1c (1 R100 fixture move), 33c46ef (1 R100 test-kit fixture move), 0698e99 (1 R100 test move), and 13a6c58 (6 R100 test moves).
git show --find-renames=100%reports those similarities. Functional test/fake edits are in cd4cbc0; the gate update is f1d6991; review corrections are in 2757757 and 00e44c6; this coverage follow-up is 03d2a9e.src/commands/interaction/runtime/wait-text.tsmatches origin/main.vitest.config.ts selects root and package destinations in unit-core via src//.test.ts and packages//src//*.test.ts; it explicitly selects the touched scripts/tests gates. scripts/check-affected/checks.ts maps this suite to unit-ci, owned by the Coverage and Integration Tests jobs in .github/workflows/ci.yml.
check:gate-manifestpassed: 57 checks across 25 lanes. The Linux coverage smoke test and accepted-gap declaration are inherited from #3740 on main, not changed by this PR.Mock and spy ledger
All 36 surviving vi.mock/spy entries have a positive response or recorded-call assertion in their destination suites. The negative-only Apple runner call guard is break-proved below.
Mutation evidence
The original 52 changed destination Vitest suites were mutation-checked during the migration review. The post-review root snapshot, wait, and generic-settle follow-ups received separate focused mutation checks. The mutants were reverted immediately after each focused run. The shared request-router mutation failed all 11 router suites (58 tests); the snapshot mutations failed all 5 snapshot suites; root runtime/facade probes failed their 10 suites (83 tests); the closure-fixture mutation failed its 5 suites (13 tests). Focused mutation runs that threw before an awaited lifecycle completed also emitted mutation-induced slow-test warnings; the unmutated changed-file run passed.
resolveBoundGestureRuntimethrowsbindSessionPerfCapturethrowsensureAndroidBlockingSystemDialogReadythrowsresolveAppEventUrlthrowsdispatchGenericCommandthrowsdispatchFindReadOnlyViaRuntimethrowsdiscloseRequestDispatchthrowsresolveScreenshotGenericExecutionthrowsdispatchIsViaRuntimethrowsdispatchGetViaRuntimethrowsdispatchIsViaRuntimethrowsdispatchSnapshotViaRuntimethrowsresolveBoundSelectorCapturethrowscreateRequestHandlerthrowscreateRequestHandlerthrowscreateRequestHandlerthrowscreateRequestHandlerthrowscreateRequestHandlerthrowscreateRequestHandlerthrowscreateRequestHandlerthrowscreateRequestHandlerthrowscreateRequestHandlerthrowscreateRequestHandlerthrowscreateRequestHandlerthrowscreateDaemonHttpServerthrowsresolvePlatformProviderRequestContextthrowsdiscloseRequestDispatchthrowsdispatchSnapshotViaRuntimethrowsrunRequestHandlerChainthrowshandleSessionOpenCommandsthrowshandleSessionObservabilityCommandsthrowsdispatchSnapshotDiffViaRuntimethrowsbindAndroidApplicationLifecyclethrowsswipeLinuxreceives a duration increased by 1 mseagerClosureGraphOfthrowseagerClosureGraphOfthrowseagerClosureOfthrowseagerClosureOfthrowseagerClosureOfthrowscreateAgentDevicethrowscreateAgentDevicethrowscreateAgentDevicethrowsgetInteractorthrowsgetPluginthrowspublicNetworkOnlyadmission guardcreatePlatformRuntimeGatewaythrowscreatePlatformRuntimeGatewaythrowsstartApplePerfCapturethrowssampleMemorythrowscreateRequestPlatformProvidersthrowsThe negative-only Apple runner mock also has a break-proof: a temporary runner call was injected into
dispatchIsViaRuntime; its focused suite failed because the mocked runner recorded one call. Reverted.Post-review mutation evidence, from a disposable checkout at 61fea26 with the review patch applied: suppressing no-effect warning generation failed the root snapshot warning test; omitting the daemon capture's post-gesture stamp failed the stored-outcome assertion; and disabling the daemon's
findTextforwarding failed the native-hit case at its call assertion. Each mutation was reverted, and the disposable checkout was removed.The wait fallback correction in
00e44c6c3arestoressrc/commands/interaction/runtime/wait-text.tsto the main version and places the native-read failure regression inpackages/daemon/src/execution/wait-runtime.test.ts. The test driveshandleSnapshotCommandsthrough the realselectWaitObservationOperationsbinding. Its baseline suite passed 15/15. In a disposable checkout, removing the binding catch made this test fail: the response becameUNKNOWN/native observation failedinstead ofCOMMAND_FAILED/canonical capture failedwithcapture_failed. The mutation was reverted, the experiment diff matched the review patch, and the checkout was removed.At head
03d2a9e87, ten isolated production mutants were detected after their focused baselines passed, then reverted:The mutation checkout was at
03d2a9e87ab8eaaeab3bb923170d06f3232610e9; it was built independently, restored to a clean tree, and removed.Exports and remaining work
At the original base b7c93e3,
packages/daemon/package.jsonhad 78 exports / 77 testing entries. After #3728 merged, the rebase base had 71 / 70. This PR removes 37 more (36 freed by consumers and the unreferencedsnapshot-interactor-captureexport), leaving 34 total / 33 testing entries. R83 reports 46 expanded test-only exports against 83 at the merge-base; R84 reports 152 unmatched tests against 187. This PR adds no daemon testing-export subpaths, aliases, barrels, orexport *. The remaining daemon testing exports belong to #3733 and the coordinated final layer. The root-only export map, no-export-star pin, ADR 0033 update, and any AGENTS.md surface note remain for after both sweeps merge.Validation
Head:
03d2a9e87ab8eaaeab3bb923170d06f3232610e9, based on origin/main4c1c786968db9c303e71e9ace3abadbd3d0f032d.Passed at this head:
CI=true pnpm install --frozen-lockfile && pnpm build,pnpm format,pnpm check:quick,pnpm check:layering,pnpm check:fallow, andpnpm check:gate-manifest(57 checks / 25 lanes). Fallow found no issues in 87 changed files. Layering reports R83 at 46 test-only exports against 83 at the merge-base and R84 at 152 unmatched tests against 187. The focused changed suites passed 7 files / 85 tests; the affected Vitest lane passed 1,872 files / 15,731 tests (1 skipped).pnpm check:affected --run --head 03d2a9e87ab8eaaeab3bb923170d06f3232610e9passed withexactCommit=true. It selected 63 checks: 36 passed, 25 were skipped as GitHub-authoritative or workflow-dispatch manual, and 2 were deduplicated. Selection failed open to the full set because package and closure-tooling changes can affect any gate. The local receipt is.tmp/validation/run-RZcutt/receipt.json. GitHub-authoritative coverage, provider integration, native and replay lanes remain for PR CI.The Linux accepted-gap test and coverage declaration came from #3740 and were retained from main during the rebase.
🤖 Generated with Claude Code