refactor(daemon): Wave 6 — migrate clipboard, app-switcher, trigger-app-event, settings, alert, react-native and capabilities onto request-bound runtimes (R55–R63) - #2021
Conversation
Size Report
npm unpacked components
Startup median (7 runs, lower is better):
Top changed chunks:
Top changed packed files
|
|
Blocking review at
CI is otherwise green except iOS smoke still in progress at review time. No |
Review blocker 1 on #2021. The Apple alert legs decided retry and hint eligibility by substring-matching error messages for "alert not found" / "no alert", and `alert wait` swallowed *every* read failure. A dead runner, an unreachable macOS helper or a canceled request was therefore spent as poll budget and finally reported as `alert wait timed out`, hiding the real cause. Both backends now state absence as typed evidence: - The XCTest runner answers `ErrorPayload(code: "ALERT_NOT_FOUND", ...)`. It is diagnostic-only, so it stays `COMMAND_FAILED` on the wire and surfaces as `details.runnerErrorCode` — the same shape `RUNNER_BUSY` already used. - The macOS helper adds `reason: "alert-not-found"` to its JSON error details, which the helper client already forwards verbatim. `isAlertNotFoundError` reads only those two fields. `awaitAppleAlert` re-throws anything that is not a typed absence instead of polling through it, and the scoped-snapshot fallback hint attaches to typed absence alone. The three tests the review asked for, plus coverage the daemon-altitude copies could not express: a non-absence failure propagates immediately from `wait`; an action does not retry a failure whose message merely reads like an absence; the macOS helper's typed reason is retried like the runner's. The daemon-level non-absence test moved to the family suite that owns this policy since R59, lowering that file's size pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
|
Re-reviewed The typed alert fix resolves the first prior P1: XCTest and the macOS helper now emit typed absence evidence, host retry/hint policy branches only on those fields, and the planted regressions prove message-like but untyped failures propagate immediately. The Wave 6 scope/size P1 remains. This commit does not split the seven-command PR or update #1739 with the required pre-wave per-unit records, budgets, itemized growth, and rejected-smaller-design rationale. Please split the units or obtain explicit tracker escalation and record that evidence before this can be ready. Native evidence is also currently red: Android smoke fails |
|
Both prior points addressed as far as I can take them. Wave 6 scope P1. The pre-wave record is now on #1739 (comment) with the per-unit descriptor/owner/cell/parity/facet/deletion/tier table, measured per-unit budgets, itemized growth, and the rejected-smaller-design rationale. The escalation is requested there, not assumed — if you would rather have seven PRs, the commits are already command-atomic and I will split. Two things in that record are worth surfacing here because they are not favourable:
Native CI. Classified, with one re-run spent:
I have used my one re-run and will not keep re-running. If you want the base-branch comparison under the same load, that needs a main run I cannot trigger from here. Generated by Claude Code |
289492d to
be4d038
Compare
Review blocker 1 on #2021. The Apple alert legs decided retry and hint eligibility by substring-matching error messages for "alert not found" / "no alert", and `alert wait` swallowed *every* read failure. A dead runner, an unreachable macOS helper or a canceled request was therefore spent as poll budget and finally reported as `alert wait timed out`, hiding the real cause. Both backends now state absence as typed evidence: - The XCTest runner answers `ErrorPayload(code: "ALERT_NOT_FOUND", ...)`. It is diagnostic-only, so it stays `COMMAND_FAILED` on the wire and surfaces as `details.runnerErrorCode` — the same shape `RUNNER_BUSY` already used. - The macOS helper adds `reason: "alert-not-found"` to its JSON error details, which the helper client already forwards verbatim. `isAlertNotFoundError` reads only those two fields. `awaitAppleAlert` re-throws anything that is not a typed absence instead of polling through it, and the scoped-snapshot fallback hint attaches to typed absence alone. The three tests the review asked for, plus coverage the daemon-altitude copies could not express: a non-absence failure propagates immediately from `wait`; an action does not retry a failure whose message merely reads like an absence; the macOS helper's typed reason is retried like the runner's. The daemon-level non-absence test moved to the family suite that owns this policy since R59, lowering that file's size pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
|
Single-PR shape accepted. I reviewed the eight commit units holistically at P1 — facts over-admit operations that execution can still refuse. On a Pixel 9 Pro XL / Android 36 emulator, this exact head reports P2 — collapse the binder catalog. P2 — split and parameterize the enforcement table. The retry binding pair is a smell, but not a separate blocker here: current public snapshot composition forwards both seams, internal/legacy captures deliberately skip with a diagnostic, and a retry failure must not replace a valid snapshot. Once those legacy routes migrate, make retry-capable capture require one constructed retry seam and delete the optional pair. Local exact-head evidence: build, Android helper build, and XCTest runner build passed. iOS Simulator passed clipboard Unicode round-trip/restore, app-event with visible exact name+JSON payload, alert get/dismiss/accept with visible result canaries, app-switcher, appearance toggle/restore, and RN no-overlay. Android Emulator passed app-event with visible exact canaries, alert dismiss/accept, app-switcher, appearance toggle/restore, RN no-overlay, and persistent helper snapshot; clipboard produced the blocker above. Current GitHub checks are all green. No label applied because of the confirmed findings. |
…tion checks Review blocker on #2021, reproduced on a Pixel 9 Pro XL / Android 36 emulator: `capabilities` advertised `clipboard`, then `clipboard read` failed with `UNSUPPORTED_OPERATION: Android shell clipboard read is not supported on this device.` Admission and execution were consulting different authorities, which ADR 0019 §2 forbids — a bound operation must already be admitted. Android. `cmd clipboard` has no shell implementation on every build, and the retired bucket admitted both halves on every real Android kind, leaving the leaf to discover the refusal after the fact. Support is now a fact: the owner probes once per device (cached for its lifetime — a build's shell command set cannot change while the device is up) and states `owner-capability-missing` when adb names the condition. The probe is definitive in one direction only: adb saying so means unsupported, a probe that cannot run means unknown, and reporting unknown as unsupported would hide a working clipboard behind a transport hiccup. The predicate moves to `@agent-device/contracts/android-clipboard-support` so admission and the leaf's own defense-in-depth check cannot drift apart. Cost, stated plainly: the first facts inspection per device now spends one adb round trip, including for requests that never touch the clipboard. WebDriver. `webdriver-interactor.ts` refuses through `capabilitySupported`, while fact generation admitted from interactor reachability alone — so a provider configured with `capabilityOverrides: { 'clipboard.read': 'unsupported' }` was admitted and then thrown out of. The declared capability map is now an input to fact generation, and the refusal carries the map author's own note. Applied to every operation with an unambiguous capability key, not just the two named in review: the mechanism is identical and a half-applied fix would leave the same defect for `back`/`home`/`orientation`/`tap`/`fill`/`type`/`scroll`. Behavior is unchanged by default — every one of those is `supported` or `partial` in the base map — so only an explicit override bites. `focus`, the gesture tiers and `trigger-app-event` keep reachability: no capability key maps to them 1:1. Also collapses the eight identical `*RetiredDispatchProjectionProof` wrappers in the cutover table into one parameterized factory (second review point). Parity tests: an Android build reporting either unsupported-shell phrasing, the probe cache, an adb failure staying admitted, and a WebDriver override refused at admission for each keyed operation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
Second review P2 on #2021. `runtime-command-cutover-table.ts` had reached 1,325 lines, past the point where one read covers it. Wave 6's eight rows move to `runtime-command-cutover-table-wave6.ts` and are spread back in, leaving the table at 1,095 lines. The split is by wave because that is how these rows are retired: a wave's rows are deleted together once the ADR declares its commands' migrations closed, and deleting a whole file is a cleaner end than excising a run of literals from the middle of a larger one. `retiredDispatchProjectionProof` moves to the shared extensions module, since both tables now use it — the main table for `snapshot`/`diff`, the sibling for its own eight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
|
Addressed at P1 — facts over-admitting. Fixed on both owners. Android: The probe is deliberately definitive in one direction only: adb saying so means unsupported, a probe that cannot run means unknown. Reporting unknown as unsupported would hide a working clipboard behind a transport hiccup, so that case stays admitted and lets the operation report the real error. Cost, stated plainly: the first facts inspection per device now spends one adb round trip, including for requests that never touch the clipboard. WebDriver: One judgement call worth flagging: I applied this to every operation with an unambiguous capability key, not just the two you named — Parity tests as requested: an Android build reporting either unsupported-shell phrasing, the probe cache, an adb failure staying admitted, and a WebDriver override refused at admission for each keyed operation. P2 — enforcement table. Done. Wave 6's eight rows moved to P2 — binder catalog. Not done. I have confirmed it is tractable — every facet binder already shares a Still true from before: I cannot produce live device evidence here, so thank you for exercising the migrated paths locally — the clipboard blocker is exactly the kind of thing CI could not have told me. Generated by Claude Code |
|
Re-reviewed P1 — the probe failure path recreates the admission lie and caches it. In The same correction should repair the ownership boundary. P1 maintainability — finish the binder-catalog simplification already identified. Exact-head evidence gathered locally on Pixel 9 Pro XL/API 36 ( No |
Review blocker on #2021. The probe I added had a `catch { return true }`, then cached that result by device id for the runtime owner's lifetime. A transient adb offline or timeout therefore made `capabilities` advertise the clipboard on a build with no clipboard shell — recreating the exact lie the fix was for, and pinning it for the rest of the session. A test locked the behavior in. Support is now a typed verdict with three states, because "we could not ask" is not "it works": `supported | unsupported | probe-failed`. Only a definitive answer is cached; `probe-failed` refuses conservatively with a hint saying support could not be determined, and is deliberately not remembered, so the next inspection asks again. The same change repairs the ownership boundary. Turning raw adb stdout/stderr into a verdict is Android tool knowledge, so it belongs to the Android owner, not to shared vocabulary — `@agent-device/contracts/android-clipboard-support` now carries the typed union alone. The parser returns to `src/platforms/android/adb.ts` and runs in exactly one place, behind a new `AndroidToolHost.probeClipboardShellSupport` that hands owners the verdict. That also settles which Android home owns it: R13 lets only `src/platform-runtime.ts` import `@agent-device/platform-android`, so a parser shared between the package and the root leaf cannot live in the package either. Tests now cover the failure path the previous ones locked the wrong way: a failed probe refuses instead of admitting, its refusal says it could not determine support rather than claiming the build lacks it, and it is not cached — a second inspection re-probes and admits once the device answers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
|
Re-reviewed P1 — the root adapter still fabricates clipboard support from nonzero adb results. In The contracts ownership issue is resolved: contracts now carry only the typed verdict and Android owns raw adb parsing. P1 maintainability — the binder-catalog simplification is still unchanged. No relevant catalog/facet files changed from the prior head. The operation tuple, complete local/provider maps, mirrored facet wrappers, and local aggregation layer still duplicate the same operation set. Land the previously agreed single typed operation-definition catalog with generic local/provider adapters and delete the parallel declarations/wrappers. Current checks are mostly green with smoke/coverage/repo guards still running. No |
Second review P1 on #2021. `interactor-operation-catalog.ts` declared the same operation set three times — a name tuple, a complete local binder map, and a complete provider binder map — and each facet carried a mirrored `bindLocal…Interactor`/`bindProvider…Interactor` pair whose only difference was which interactor source to use and which label a refusal names. There is now one row per operation, carrying its facts key, its provider refusal label, and the facet's own executor. The local/provider split lives in the two adapters, which differ by exactly the thing that differs: the interactor source. Adding an operation is adding one row. Deleted: the parallel tuple, both binder maps, 32 mirrored wrappers across ten facet modules, and the per-facet `Local…`/`Provider…InteractorResolver` aliases that existed only to be re-exported. Kept: every facet's typed executor, now exported as its binding surface. Net −563 production lines in `packages/contracts`. Two consumers moved onto the catalog's public entry point rather than keeping a private path to a single operation: the app-event delivery test and the provider scenario fixture, whose two hand-bound keyboard legs are now whichever legs its facts admit. Each facet's tests spell out the composition the retired wrappers performed, so every assertion still exercises one executor reached through one source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
|
Both blockers addressed at P1 — the probe failure path. You were right, and my reasoning was the wrong shape: I argued "unknown shouldn't hide a working clipboard" without following it through to the consequence, which is that one transient adb blip makes Support is now a typed verdict with three states — The ownership correction came with it. One design consequence worth flagging: the host method is optional, and a host that does not implement it gets a refusal, not an admission. I made it optional after a required version broke unrelated boot/shutdown paths whose fixtures reach the real Android runtime — but absence of a probe is absence of evidence, so it refuses. There is a test for that case specifically. P1 — binder catalog. Landed. One row per operation carrying its facts key, its provider refusal label, and the facet's own executor; the local/provider split lives in the two adapters, differing by exactly the thing that differs — the interactor source. Deleted: the parallel name tuple, both binder maps, 32 mirrored wrappers across ten facet modules, and the per-facet Two consumers moved onto the catalog's public entry point rather than keeping a private path to one operation — the app-event delivery test, and the provider scenario fixture whose two hand-bound keyboard legs are now whichever legs its facts admit. Each facet's tests spell out the composition the retired wrappers performed, so every assertion still exercises one executor through one source. Ten Verification. I still cannot produce live device evidence here, so the Android clipboard paths want your emulator again — particularly the Generated by Claude Code |
Third review P1 on #2021. The typed verdict landed one layer too high. The adapter probe runs `adb shell cmd clipboard get text` with `allowFailure`, so a non-zero exit comes back as an ordinary result rather than a throw — and the only thing standing between that result and `supported` was the missing-shell prose check. A device that had gone offline, was unauthorized, timed out, or failed for any other reason produced none of that prose, so it fell through to `supported` and was then cached by device id for the runtime owner's lifetime. The `catch` I added guarded the one path adb almost never takes. Each adb outcome now proves only what it can: - `exitCode === 0` is the sole evidence of support, because it is the only result that shows the command ran. - The recognized missing-shell prose is the sole evidence of absence, and is read before the exit code — adb reports that condition non-zero, so checking the code first would turn every honest `unsupported` into a refusal. - Everything else — non-zero without that prose, and the transport throw — is `probe-failed`, which admission refuses and the cache does not remember. The package tests mocked the typed verdict, so they sat downstream of the bug and could not see it. The regression is therefore at the adapter, over the raw adb result: four planted reds (offline, unauthorized, device-not-found, generic failure) that all returned `supported` before this change, plus the two definitive verdicts and the ordering case that keeps `unsupported` reachable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
|
Both P1s are addressed. One of them had already landed before you reviewed — you were reading a stale head on that one. P1 — the adapter fabricated support from non-zero adb results. Confirmed, and your reading of the mechanism was exactly right. if (isClipboardShellUnsupported(result.stdout, result.stderr)) return 'unsupported';
return result.exitCode === 0 ? 'supported' : 'probe-failed';The prose check runs before the exit code on purpose: adb reports the missing shell command non-zero, so reading the code first would turn every honest The regression is at the adapter over the raw adb result, as you asked — P1 maintainability — the binder catalog. Already landed: iOS smoke on Local gate on The live-device gap I flagged earlier still stands: I can't run an emulator here, so the Generated by Claude Code |
|
Re-reviewed P1 — successful clipboard contents can be cached as unsupported-shell evidence. GitHub currently reports |
7e1f391 to
6cfa4e9
Compare
Review blocker 1 on #2021. The Apple alert legs decided retry and hint eligibility by substring-matching error messages for "alert not found" / "no alert", and `alert wait` swallowed *every* read failure. A dead runner, an unreachable macOS helper or a canceled request was therefore spent as poll budget and finally reported as `alert wait timed out`, hiding the real cause. Both backends now state absence as typed evidence: - The XCTest runner answers `ErrorPayload(code: "ALERT_NOT_FOUND", ...)`. It is diagnostic-only, so it stays `COMMAND_FAILED` on the wire and surfaces as `details.runnerErrorCode` — the same shape `RUNNER_BUSY` already used. - The macOS helper adds `reason: "alert-not-found"` to its JSON error details, which the helper client already forwards verbatim. `isAlertNotFoundError` reads only those two fields. `awaitAppleAlert` re-throws anything that is not a typed absence instead of polling through it, and the scoped-snapshot fallback hint attaches to typed absence alone. The three tests the review asked for, plus coverage the daemon-altitude copies could not express: a non-absence failure propagates immediately from `wait`; an action does not retry a failure whose message merely reads like an absence; the macOS helper's typed reason is retried like the runner's. The daemon-level non-absence test moved to the family suite that owns this policy since R59, lowering that file's size pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
…tion checks Review blocker on #2021, reproduced on a Pixel 9 Pro XL / Android 36 emulator: `capabilities` advertised `clipboard`, then `clipboard read` failed with `UNSUPPORTED_OPERATION: Android shell clipboard read is not supported on this device.` Admission and execution were consulting different authorities, which ADR 0019 §2 forbids — a bound operation must already be admitted. Android. `cmd clipboard` has no shell implementation on every build, and the retired bucket admitted both halves on every real Android kind, leaving the leaf to discover the refusal after the fact. Support is now a fact: the owner probes once per device (cached for its lifetime — a build's shell command set cannot change while the device is up) and states `owner-capability-missing` when adb names the condition. The probe is definitive in one direction only: adb saying so means unsupported, a probe that cannot run means unknown, and reporting unknown as unsupported would hide a working clipboard behind a transport hiccup. The predicate moves to `@agent-device/contracts/android-clipboard-support` so admission and the leaf's own defense-in-depth check cannot drift apart. Cost, stated plainly: the first facts inspection per device now spends one adb round trip, including for requests that never touch the clipboard. WebDriver. `webdriver-interactor.ts` refuses through `capabilitySupported`, while fact generation admitted from interactor reachability alone — so a provider configured with `capabilityOverrides: { 'clipboard.read': 'unsupported' }` was admitted and then thrown out of. The declared capability map is now an input to fact generation, and the refusal carries the map author's own note. Applied to every operation with an unambiguous capability key, not just the two named in review: the mechanism is identical and a half-applied fix would leave the same defect for `back`/`home`/`orientation`/`tap`/`fill`/`type`/`scroll`. Behavior is unchanged by default — every one of those is `supported` or `partial` in the base map — so only an explicit override bites. `focus`, the gesture tiers and `trigger-app-event` keep reachability: no capability key maps to them 1:1. Also collapses the eight identical `*RetiredDispatchProjectionProof` wrappers in the cutover table into one parameterized factory (second review point). Parity tests: an Android build reporting either unsupported-shell phrasing, the probe cache, an adb failure staying admitted, and a WebDriver override refused at admission for each keyed operation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
Second review P2 on #2021. `runtime-command-cutover-table.ts` had reached 1,325 lines, past the point where one read covers it. Wave 6's eight rows move to `runtime-command-cutover-table-wave6.ts` and are spread back in, leaving the table at 1,095 lines. The split is by wave because that is how these rows are retired: a wave's rows are deleted together once the ADR declares its commands' migrations closed, and deleting a whole file is a cleaner end than excising a run of literals from the middle of a larger one. `retiredDispatchProjectionProof` moves to the shared extensions module, since both tables now use it — the main table for `snapshot`/`diff`, the sibling for its own eight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
Review blocker on #2021. The probe I added had a `catch { return true }`, then cached that result by device id for the runtime owner's lifetime. A transient adb offline or timeout therefore made `capabilities` advertise the clipboard on a build with no clipboard shell — recreating the exact lie the fix was for, and pinning it for the rest of the session. A test locked the behavior in. Support is now a typed verdict with three states, because "we could not ask" is not "it works": `supported | unsupported | probe-failed`. Only a definitive answer is cached; `probe-failed` refuses conservatively with a hint saying support could not be determined, and is deliberately not remembered, so the next inspection asks again. The same change repairs the ownership boundary. Turning raw adb stdout/stderr into a verdict is Android tool knowledge, so it belongs to the Android owner, not to shared vocabulary — `@agent-device/contracts/android-clipboard-support` now carries the typed union alone. The parser returns to `src/platforms/android/adb.ts` and runs in exactly one place, behind a new `AndroidToolHost.probeClipboardShellSupport` that hands owners the verdict. That also settles which Android home owns it: R13 lets only `src/platform-runtime.ts` import `@agent-device/platform-android`, so a parser shared between the package and the root leaf cannot live in the package either. Tests now cover the failure path the previous ones locked the wrong way: a failed probe refuses instead of admitting, its refusal says it could not determine support rather than claiming the build lacks it, and it is not cached — a second inspection re-probes and admits once the device answers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
Second review P1 on #2021. `interactor-operation-catalog.ts` declared the same operation set three times — a name tuple, a complete local binder map, and a complete provider binder map — and each facet carried a mirrored `bindLocal…Interactor`/`bindProvider…Interactor` pair whose only difference was which interactor source to use and which label a refusal names. There is now one row per operation, carrying its facts key, its provider refusal label, and the facet's own executor. The local/provider split lives in the two adapters, which differ by exactly the thing that differs: the interactor source. Adding an operation is adding one row. Deleted: the parallel tuple, both binder maps, 32 mirrored wrappers across ten facet modules, and the per-facet `Local…`/`Provider…InteractorResolver` aliases that existed only to be re-exported. Kept: every facet's typed executor, now exported as its binding surface. Net −563 production lines in `packages/contracts`. Two consumers moved onto the catalog's public entry point rather than keeping a private path to a single operation: the app-event delivery test and the provider scenario fixture, whose two hand-bound keyboard legs are now whichever legs its facts admit. Each facet's tests spell out the composition the retired wrappers performed, so every assertion still exercises one executor reached through one source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
Third review P1 on #2021. The typed verdict landed one layer too high. The adapter probe runs `adb shell cmd clipboard get text` with `allowFailure`, so a non-zero exit comes back as an ordinary result rather than a throw — and the only thing standing between that result and `supported` was the missing-shell prose check. A device that had gone offline, was unauthorized, timed out, or failed for any other reason produced none of that prose, so it fell through to `supported` and was then cached by device id for the runtime owner's lifetime. The `catch` I added guarded the one path adb almost never takes. Each adb outcome now proves only what it can: - `exitCode === 0` is the sole evidence of support, because it is the only result that shows the command ran. - The recognized missing-shell prose is the sole evidence of absence, and is read before the exit code — adb reports that condition non-zero, so checking the code first would turn every honest `unsupported` into a refusal. - Everything else — non-zero without that prose, and the transport throw — is `probe-failed`, which admission refuses and the cache does not remember. The package tests mocked the typed verdict, so they sat downstream of the bug and could not see it. The regression is therefore at the adapter, over the raw adb result: four planted reds (offline, unauthorized, device-not-found, generic failure) that all returned `supported` before this change, plus the two definitive verdicts and the ordering case that keeps `unsupported` reachable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
|
Rebased onto One real conflict, in Re-ran the full gate on the new base: typecheck clean across all package tsconfigs, layering 184/184 (1423 source files, all 45 migrated commands holding one platform-execution path), fallow clean over 218 changed files, 1124/1126 test files. The two failures are the same pre-existing Worth knowing for the next review pass: Generated by Claude Code |
|
Re-reviewed |
6cfa4e9 to
50ffd91
Compare
…tire the legacy dispatcher (R58) Wave 6 unit 4 of the ADR 0019 platform-free daemon migration (#1739). `settings` was the last `DISPATCH_HANDLERS` arm, so this change closes the command and retires the legacy command dispatcher whole. - new `@agent-device/contracts/settings-runtime` facet on the shared `Interactor` seam, bound through the interactor catalog. What reaches the owner is its own settings vocabulary (setting, state, resolved app id, typed coordinates); the CLI parse, the macOS setting-name gate, the clear-app-state app-id check and the coordinate typing are daemon policy and stay daemon-side, downstream of admission where the retired leaf ran them. - Apple shares clipboard's exact host-or-simulator reading (the retired admission intersected the `settings` bucket with the same `supportsHostOrSimulatorSurface` closure); Android admits every real kind; HarmonyOS matches its retired overlay membership; Linux/Vega/web refuse. Limrun splits Android-reuse / iOS-refusal like `app-switcher`; WebDriver refuses unconditionally, since its interactor declares settings unsupported. - retires `dispatchCommand`, `dispatchWithInteractor`, `dispatchKnownCommand`, `DISPATCH_HANDLERS`, `listRegisteredDispatchCommandNames`, and the request router's `executeGenericPlatformCommand` fallback. `core/dispatch.ts` keeps only `dispatchGestureViewport`, whose last consumers are replay/test. Retiring the dispatcher surfaced two callers broken since Wave 5 moved `press` onto a bound runtime: react-native overlay dismissal and the opt-in interaction no-change retry both called `dispatchCommand(device, 'press', …)`, which has thrown `INVALID_ARGS: Unknown command: press` on main since R48. Both now run the same bound `tapPoint` every other touch leaf uses. The retry declares its own callback seam rather than importing runtime admission, so the policy stays readable without the binding stack — and that inversion, plus the dispatcher's retirement, drops the largest type-level import cycle from 25 files to 21. Cutover row R58 with its retirement and single-bind claims. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
…acts (R59/R61/R63) Wave 6 units 5, 7 and 9 of the ADR 0019 platform-free daemon migration (#1739), plus the residue reclassification the tracker asks for as an analysis task. R59 `alert` — new `@agent-device/contracts/alert-runtime` facet with four action-selected legs (`readAlert`, `awaitAlert`, `acceptAlert`, `dismissAlert`) on the shared `Interactor` seam. The daemon route admits and binds exactly the leg the parsed subcommand names, and the poll and retry windows move to the owners with it: how long a transient sheet takes to appear, and how many times to re-ask a runner that says it is not there yet, are family mechanics, not request policy. `src/platforms/apple/alert.ts` now holds the Apple windows verbatim (with the macOS-helper / XCTest-runner split), and Android's legs read the same presented tree `snapshot` publishes, which is why their occlusion reading still holds. Apple's cell is the retired `supportsAlertSurface` closure restated as facts — the host-or-simulator reading widened by physical iOS — and that closure was the per-AppleOS capability table's last reader, so `src/platforms/apple/capabilities.ts` goes with it. R61 `react-native` — the command's device work moved onto a bound `tapPoint` with R48; this retires the capability gate that still stood in front of it and moves admission ahead of the observing capture, so an owner that cannot dismiss an overlay refuses without first spending a snapshot on it. That exposed a real defect: the request handler chain never forwarded the request's runtime bindings to this route, so the dismissal leg had been reaching a missing gateway ever since R48 — only the no-overlay-detected path returned early enough to hide it. Fixed, with a chain-level regression test. R63 `capabilities` — the projection now reads each command's own declared `platformExecution` uses instead of a hand-written map plus a "no capability bucket means supported everywhere" fallback. That fallback is what let a stopped Android AVD advertise `snapshot press fill` it cannot run, and a Vega VVD advertise every migrated command; both collapse to the fact-derived set here. The command itself executes nothing on a device, so it declares `none`. Residue: `batch`, `debug` and `events` reclassify to `none` — each reaches no device and delegates nothing that does. `replay`/`test` keep their gesture viewport and boot-diagnostics edges, `daemon`/`web` hold platform imports in their own CLI modules, and `react-devtools` still injects device-runtime `runtime`, so all five stay `legacy`. Cutover rows R59 and R61 with their retirement and single-bind claims. Descriptors: 32 legacy at the wave checkpoint, 9 now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
…hifted Self-review of the Wave 6 diff against `origin/main` found two places where the migrated routes were faithful in what they did but not in when: - `settings` typed its location coordinates before expiring the ref frame, so a request that failed on a bad coordinate no longer expired it. The retired route expired the frame first, then emitted its diagnostic, then typed the coordinates inside the leaf. Same order again. - `alert` narrowed a frontmost-app session to "no bundle" in the daemon, which also stripped the bundle from the XCTest runner leg. That narrowing was only ever the macOS helper's, and it already lives in `platforms/apple/alert.ts`; the runner leg gets `session.appBundleId` unconditionally again, pinned by a test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
Three independent reviews (behavior parity, correctness, ADR 0019 conformance) ran against the branch. What they found, and what changed: Correctness - The R48 retry seam was unreachable. `captureSnapshot` builds it from the request's runtime bindings, but no caller forwarded them, so every retry resolved to a skip. The `snapshot` route now threads `inspectFacts`/ `bindDevice` through `createSnapshotRuntime` and the daemon snapshot backend down to the capture. - A retry tap that rejected escaped the capture it was decorating and turned a plain `snapshot` into an error. It is caught and reported as a skip, matching what the seam's own contract already claimed. - The attempt is spent before the device work again, as the retired route did, so an owner that fails mid-flight cannot be re-attempted from a full budget. - `react-native dismiss-overlay` reached its required `tapPoint` through `?.` and answered `dismissed: true` when the operation was absent. It refuses. - `factOwnedCapabilityAvailable` indexed the facts map unguarded, and treated an empty `required` as proof (`[].every` is vacuously true). Both fail closed. ADR 0019 conformance - §6 forbids a `none` descriptor from binding a device, and `capabilities` bound three times to answer `logs`/`network`/`record`. Every owner composes a binding's facts with the same function `inspectFacts` calls, so those probes read back values the single inspection already carries — at the cost of a device claim on a read-only query. They are gone, and with them the last three empty-`required` admission uses. - §9 is one admission per handler; the retry tap re-admitted on every retry round. It memoizes per device. - `installFamilyCapabilityAvailable` was scaffolding this wave was scheduled to retire: the general projection returns the same verdict for all four install-family commands. Deleted. Leftovers the cutovers created - `requireCommandSupported` lost its last production caller when R56 migrated `app-switcher`: every generic-route command is admitted from owner facts before the dispatcher runs. The dead arm, the function, and `commandUsesDeviceRuntimeExecution` are removed. - `CommandDispatchFacet`, `descriptor.dispatch`, and `explain`'s `dispatch=` field described a dispatcher R58 deleted. - `request-router-android-modal.test.ts` asserted on a `dispatchCommand` mock whose module export no longer exists, so three assertions were vacuous. - `generic-route-runtime-completeness.test.ts` now exists — a comment claimed it did. It pins the routing table as total over the generic route. - Comments and test names describing the retired dispatcher, the deleted AppleOS capability table, and a react-native regression that never shipped. Also records two deliberate provider cell changes the migration made (physical Apple `clipboard` admitted, provider `alert` refused) and the react-native widening to Linux, web and HarmonyOS, and drops a scratch probe file that was committed by accident. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
Review blocker 1 on #2021. The Apple alert legs decided retry and hint eligibility by substring-matching error messages for "alert not found" / "no alert", and `alert wait` swallowed *every* read failure. A dead runner, an unreachable macOS helper or a canceled request was therefore spent as poll budget and finally reported as `alert wait timed out`, hiding the real cause. Both backends now state absence as typed evidence: - The XCTest runner answers `ErrorPayload(code: "ALERT_NOT_FOUND", ...)`. It is diagnostic-only, so it stays `COMMAND_FAILED` on the wire and surfaces as `details.runnerErrorCode` — the same shape `RUNNER_BUSY` already used. - The macOS helper adds `reason: "alert-not-found"` to its JSON error details, which the helper client already forwards verbatim. `isAlertNotFoundError` reads only those two fields. `awaitAppleAlert` re-throws anything that is not a typed absence instead of polling through it, and the scoped-snapshot fallback hint attaches to typed absence alone. The three tests the review asked for, plus coverage the daemon-altitude copies could not express: a non-absence failure propagates immediately from `wait`; an action does not retry a failure whose message merely reads like an absence; the macOS helper's typed reason is retried like the runner's. The daemon-level non-absence test moved to the family suite that owns this policy since R59, lowering that file's size pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
…tion checks Review blocker on #2021, reproduced on a Pixel 9 Pro XL / Android 36 emulator: `capabilities` advertised `clipboard`, then `clipboard read` failed with `UNSUPPORTED_OPERATION: Android shell clipboard read is not supported on this device.` Admission and execution were consulting different authorities, which ADR 0019 §2 forbids — a bound operation must already be admitted. Android. `cmd clipboard` has no shell implementation on every build, and the retired bucket admitted both halves on every real Android kind, leaving the leaf to discover the refusal after the fact. Support is now a fact: the owner probes once per device (cached for its lifetime — a build's shell command set cannot change while the device is up) and states `owner-capability-missing` when adb names the condition. The probe is definitive in one direction only: adb saying so means unsupported, a probe that cannot run means unknown, and reporting unknown as unsupported would hide a working clipboard behind a transport hiccup. The predicate moves to `@agent-device/contracts/android-clipboard-support` so admission and the leaf's own defense-in-depth check cannot drift apart. Cost, stated plainly: the first facts inspection per device now spends one adb round trip, including for requests that never touch the clipboard. WebDriver. `webdriver-interactor.ts` refuses through `capabilitySupported`, while fact generation admitted from interactor reachability alone — so a provider configured with `capabilityOverrides: { 'clipboard.read': 'unsupported' }` was admitted and then thrown out of. The declared capability map is now an input to fact generation, and the refusal carries the map author's own note. Applied to every operation with an unambiguous capability key, not just the two named in review: the mechanism is identical and a half-applied fix would leave the same defect for `back`/`home`/`orientation`/`tap`/`fill`/`type`/`scroll`. Behavior is unchanged by default — every one of those is `supported` or `partial` in the base map — so only an explicit override bites. `focus`, the gesture tiers and `trigger-app-event` keep reachability: no capability key maps to them 1:1. Also collapses the eight identical `*RetiredDispatchProjectionProof` wrappers in the cutover table into one parameterized factory (second review point). Parity tests: an Android build reporting either unsupported-shell phrasing, the probe cache, an adb failure staying admitted, and a WebDriver override refused at admission for each keyed operation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
Second review P2 on #2021. `runtime-command-cutover-table.ts` had reached 1,325 lines, past the point where one read covers it. Wave 6's eight rows move to `runtime-command-cutover-table-wave6.ts` and are spread back in, leaving the table at 1,095 lines. The split is by wave because that is how these rows are retired: a wave's rows are deleted together once the ADR declares its commands' migrations closed, and deleting a whole file is a cleaner end than excising a run of literals from the middle of a larger one. `retiredDispatchProjectionProof` moves to the shared extensions module, since both tables now use it — the main table for `snapshot`/`diff`, the sibling for its own eight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
Review blocker on #2021. The probe I added had a `catch { return true }`, then cached that result by device id for the runtime owner's lifetime. A transient adb offline or timeout therefore made `capabilities` advertise the clipboard on a build with no clipboard shell — recreating the exact lie the fix was for, and pinning it for the rest of the session. A test locked the behavior in. Support is now a typed verdict with three states, because "we could not ask" is not "it works": `supported | unsupported | probe-failed`. Only a definitive answer is cached; `probe-failed` refuses conservatively with a hint saying support could not be determined, and is deliberately not remembered, so the next inspection asks again. The same change repairs the ownership boundary. Turning raw adb stdout/stderr into a verdict is Android tool knowledge, so it belongs to the Android owner, not to shared vocabulary — `@agent-device/contracts/android-clipboard-support` now carries the typed union alone. The parser returns to `src/platforms/android/adb.ts` and runs in exactly one place, behind a new `AndroidToolHost.probeClipboardShellSupport` that hands owners the verdict. That also settles which Android home owns it: R13 lets only `src/platform-runtime.ts` import `@agent-device/platform-android`, so a parser shared between the package and the root leaf cannot live in the package either. Tests now cover the failure path the previous ones locked the wrong way: a failed probe refuses instead of admitting, its refusal says it could not determine support rather than claiming the build lacks it, and it is not cached — a second inspection re-probes and admits once the device answers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
Second review P1 on #2021. `interactor-operation-catalog.ts` declared the same operation set three times — a name tuple, a complete local binder map, and a complete provider binder map — and each facet carried a mirrored `bindLocal…Interactor`/`bindProvider…Interactor` pair whose only difference was which interactor source to use and which label a refusal names. There is now one row per operation, carrying its facts key, its provider refusal label, and the facet's own executor. The local/provider split lives in the two adapters, which differ by exactly the thing that differs: the interactor source. Adding an operation is adding one row. Deleted: the parallel tuple, both binder maps, 32 mirrored wrappers across ten facet modules, and the per-facet `Local…`/`Provider…InteractorResolver` aliases that existed only to be re-exported. Kept: every facet's typed executor, now exported as its binding surface. Net −563 production lines in `packages/contracts`. Two consumers moved onto the catalog's public entry point rather than keeping a private path to a single operation: the app-event delivery test and the provider scenario fixture, whose two hand-bound keyboard legs are now whichever legs its facts admit. Each facet's tests spell out the composition the retired wrappers performed, so every assertion still exercises one executor reached through one source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
Third review P1 on #2021. The typed verdict landed one layer too high. The adapter probe runs `adb shell cmd clipboard get text` with `allowFailure`, so a non-zero exit comes back as an ordinary result rather than a throw — and the only thing standing between that result and `supported` was the missing-shell prose check. A device that had gone offline, was unauthorized, timed out, or failed for any other reason produced none of that prose, so it fell through to `supported` and was then cached by device id for the runtime owner's lifetime. The `catch` I added guarded the one path adb almost never takes. Each adb outcome now proves only what it can: - `exitCode === 0` is the sole evidence of support, because it is the only result that shows the command ran. - The recognized missing-shell prose is the sole evidence of absence, and is read before the exit code — adb reports that condition non-zero, so checking the code first would turn every honest `unsupported` into a refusal. - Everything else — non-zero without that prose, and the transport throw — is `probe-failed`, which admission refuses and the cache does not remember. The package tests mocked the typed verdict, so they sat downstream of the bug and could not see it. The regression is therefore at the adapter, over the raw adb result: four planted reds (offline, unauthorized, device-not-found, generic failure) that all returned `supported` before this change, plus the two definitive verdicts and the ordering case that keeps `unsupported` reachable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
…ontents Fourth review P1 on #2021, and a second instance of the same bug it names. The previous fix read `isClipboardShellUnsupported(stdout, stderr)` before the exit code. On a *successful* `cmd clipboard get text`, stdout is the clipboard's contents — arbitrary user text. Anyone who had copied "unknown command" or "no shell command implementation" (from a terminal, a bug report, this repo) had their own working clipboard classified `unsupported`, and the runtime owner cached that for its lifetime. Ordering prose ahead of the exit code to keep `unsupported` reachable traded one wrong admission for another. The exit code is decisive on its own when it is zero, so it goes first. Only a call that failed can carry prose about the call itself, which makes the missing- shell phrases meaningful on non-zero exits alone: if (result.exitCode === 0) return 'supported'; return isClipboardShellUnsupported(...) ? 'unsupported' : 'probe-failed'; `isClipboardShellUnsupported` now states that precondition, because reading it on a successful call is exactly the mistake to prevent. The same defect was already shipped in the helper's other caller. `runAndroidClipboardShellCommand` in `src/platforms/android/device-input-state.ts` has checked the prose before the exit code since #1950, so `clipboard read` on a clipboard holding either phrase threw `UNSUPPORTED_OPERATION` — telling the user their device does not support a clipboard it had just read correctly. It is not this wave's code and not reachable from the migration, but it is the same helper misused the same way, and documenting a precondition while leaving a caller that violates it invites the next regression. Repaired here, with the failure ordering otherwise unchanged: a non-zero exit still reports missing-shell as `UNSUPPORTED_OPERATION` and anything else as the adb result error. Both repairs are pinned by regressions that fail against the code they replace: four exit-0 cases at the adapter (verified red against the ordering this commit removes), and three at `readAndroidClipboardWithAdb` (verified red against `origin/main`) covering contents that look like a refusal, a genuine missing command, and an unrelated non-zero failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
50ffd91 to
59d1853
Compare
|
Rebased again onto The iOS smoke failure is not this PR's — it's hitting unrelated PRs too. I said last round I'd root-cause it rather than re-run if it recurred at the same step, so here is what I found. It recurred on Today's runs of that job:
So it is intermittent, roughly half the runs, across branches with nothing in common — and this branch passed it twice before the pattern started. Mechanically it also can't be mine: I'm not fixing it here — it is the runner's text-entry commit verification, unrelated to Wave 6, and folding it in would widen a 220-file PR into someone else's subsystem. Worth its own issue; happy to file one with these run links if you want it tracked. Recapping what remains not-mine so the exact-head validation is readable: expect Generated by Claude Code |
|
Exact-head validation has finished on
Everything else passed on this head, including the lanes the rebases put at risk: Repo Guards, Typecheck & Package, Integration Tests, Lint & Format, Coverage (1), Bundle Size, Compatibility & Provenance, the Android/Linux/macOS smoke jobs, and all four CodeQL analyses. Merge state is Since main is red on the help card, no PR based on it can reach an all-green matrix until that's fixed. I've left it alone — it's 3 bytes over a hard limit in a topic Wave 6 doesn't touch, and this diff is already 220 files. Happy to send it as its own one-line PR, and to file the iOS Generated by Claude Code |
|
Re-reviewed exact head Readiness is still blocked by red exact-head CI: Coverage (2) reproduces on base at workflow help 9003 bytes vs |
|
Both of the things you'd accept for the iOS failure: the base comparison is below, and a clean re-run of the failed job on Base comparison. Every iOS
Five failures, four different branches. Every run before ~12:00 passed, including two on this branch. And the two rows that settle it:
Two unrelated branches each went red then green with nothing touching iOS in between — that is the clean-rerun evidence you asked for, just produced on someone else's branch. I read the assertion on So the job flipped from reliably green to roughly 50/50 for the whole repo somewhere around 12:00, and this branch was simply sampled twice on the losing side. I've also queued Thanks for the exact-head re-review — good to have the clipboard ordering confirmed from the outside, given I got it wrong twice before it was right. Generated by Claude Code |
`Coverage (2)` has been red on `main` and on every PR branched from it since #2020, which replaced three short Bootstrap lines with one longer line carrying the new selection semantics. It updated the content matcher for that line but not the size assertion beside it, so the card went to 9003 bytes against the `< 9000` both `cli-help.test.ts` and `cli-help-topics.test.ts` enforce. Nothing #2020 added is removed here — all of it is pinned by the matcher it shipped, and it is the sentence agents most need. The bytes come back from a clumsy repetition elsewhere in the card, where "settle" named itself twice in one clause: ... only when you did not settle, settle reported not settled, or ... ... only when you did not settle, it reported not settled, or ... which reads better short and puts the card at 8999. That is one byte inside the budget, which is the real finding: the card has no slack left, and the next sentence anyone adds re-opens this. The durable fix is a base-owner call between raising the budget and moving a block down into its sub-topic — the mechanism the card already uses, and which its own test documents. Flagged on #2021 rather than decided here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
|
Pushed Coverage (2) — fixed. Root cause is #2020: it replaced three short Bootstrap lines with one longer line carrying the selection semantics, updated the content matcher beside it, but not the size assertion. The card went to 9003 against the I removed nothing #2020 added — all of it is pinned by the matcher that PR shipped, and it's the sentence agents most need. The bytes come back from a clumsy repetition elsewhere, where "settle" named itself twice in one clause: One caveat you should decide on: that lands the card at 8999 of 9000. One byte. I checked for slack before trimming prose — no trailing whitespace, and the only repeated runs are deliberate column alignment in the footer — so the card genuinely has nothing left to give. The next sentence anyone adds re-opens this. The durable fix is yours to pick: raise the budget, or move a block down into its sub-topic, which is the mechanism the card already uses and its own test documents ( iOS — I spent the one re-run and it failed the same way. Attempt 2 on I did try to root-cause it rather than just re-classify it. One hypothesis looked strong and turned out wrong, so discounting it may still be useful to you: Beyond that I couldn't attribute it. Runner cache state doesn't separate the outcomes either (#2033 failed with a 5s cached runner restore; #2029 passed with a 2s one). Generated by Claude Code |
The card is a curated agent-facing reference, and #2020 grew it for a good reason: the selection semantics it added are what an agent needs to predict which device a bare `open` picks. Holding that content to a limit set before it existed just moves the cost onto whoever writes the next sentence. 9100 is headroom, not a target. The previous commit left the card at 8999 of 9000 -- one byte -- which is not a state anyone should have to work in, and I had already established there is no slack left to reclaim: no trailing whitespace, and the only repeated runs are the deliberate column alignment in the Escalate footer. Trimming further would have meant deleting content the tests pin as load-bearing. This is explicitly interim. The card is ~9KB of dense prose in one string, and the real answer is to move a block down into its owning sub-topic -- the mechanism the card already uses and its own test documents ("Deep content moved out of the compact card, not deleted"). Raising the ceiling buys room to do that deliberately instead of under a red CI. Both enforcement sites move together, since they measure the same card through different surfaces: `cli-help.test.ts` reads it through the CLI, and `cli-help-topics.test.ts` through `usageForCommand`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
|
Re-reviewed exact head |
|
On the P2: Substantively I agree with the design point, and said so in the commit message before you weighed in: the durable fix is moving deep material into its owning sub-topic, which is the mechanism the card already uses and its own test documents. The raise is labelled interim precisely because it buys room to do that deliberately rather than under a red CI. Where I'd push back slightly is on "solely to create headroom" — one byte of margin is not a working state; the next contributor to add a sentence anywhere in that card hits the same wall #2020 did, and #2020's author didn't notice because the failure surfaced two files away from the edit. So the call is yours, and any of these is fine by me:
I'd take (1): it's the only one that separates "unblock CI" from "restructure the help card", and those want different reviews. CI status on Generated by Claude Code |
|
Coordinator correction for exact head |
thymikee
added
the
ready-for-human
|
Final CI result on The iOS lane finished red at step 13 with the known flake — I'd already spent the one sanctioned re-run on this job earlier and it failed identically, so I'm not re-running again. The base comparison stands on its own: five failures across four unrelated branches today, two of which went red then green on consecutive runs with nothing touching iOS in between. Separately, I filed #2034 for the Generated by Claude Code |
|
2 participants
Closes the named Wave 6 units from #1739. Seven commands leave
platformExecution: legacyfor ADR 0019 request-bound runtimes, and the legacy platform dispatcher goes with them.What landed
clipboardreadClipboard/writeClipboard) — a provider can genuinely have the getter without the setterapp-switcherappSwitcheruse on the generic routetrigger-app-eventtriggerAppEventuse; URL templating stays daemon-side, only a resolved URL crossessettingssetSettinguse — the lastDISPATCH_HANDLERSarm, so this retiresdispatchCommand,dispatchWithInteractor,dispatchKnownCommandand the whole legacy dispatcheralertread/wait/accept/dismiss); the Apple legs move tosrc/platforms/apple/alert.ts, Android's supply their own node capturereact-nativetapPointahead of the observing capturecapabilitiesResidue reclassified to
platformExecution: none(legacy by delegation only, delegate already migrated):batch,debug,events,capabilities.Legacy descriptors: 32 → 9. Modes are now
{ none: 21, inventory: 1, legacy: 9, device-runtime: 45 }.Two pre-existing product defects repaired
R48 moved
pressoff the legacy dispatcher but left two callers ondispatchCommand(device, 'press', …), which had been throwingINVALID_ARGS: Unknown command: pressonmainever since:--retry-on-no-changethrew out of the snapshot capture it was decorating.Both now ride the bound
tapPoint. The old tests hid this by mockingdispatchCommand.Deliberate behavior changes
Recorded here because none of them is a migration accident:
react-native dismiss-overlaywidens to Linux, web and HarmonyOS. The retired bucket was{apple, android, linux: {}}; all three admittapPoint, and a surface with no overlay answersdetected: false— the truthful result. A family cannot be a support authority for a migrated command (§8).clipboardis now admitted.supportsHostOrSimulatorSurfacewas a local-Apple predicate applied to a device driven over Appium, which does expose the clipboard extension. The refusal moves into the interactor, per session.alertis now refused up front.supportsAlertSurfaceadmitted it onappleOs === 'ios'alone, and the handler then drove the local XCTest runner against a device in someone else's cloud. Nothing the provider owns can serve an alert leg.details, andsupportedOndisappears (it is derived fromcapability, which a migrated descriptor no longer carries). This matches every prior wave —backlostsupportedOnin R42.platform_command_prepare/platform_commanddiagnostics are gone with the dispatcher that emitted them. JSON responses andcost.runnerRoundTripsare unaffected.Analyzed, deliberately not landed
audio(R60) is durable-tier, not request-scoped: it owns a session-scoped child process and a status file that survive across requests. It needs the durable-resource treatmentlogs/recordgot, not adevice-runtimeuse.doctor(R62) probes the host toolchain. There is noplatformExecutionkind for host-scoped work, so classifying it needs an ADR-level discriminator decision first.react-devtoolsis blocked by the mode-dominance gate: it injects the device-runtimeruntimecommand.daemon,web,replay,testkeep genuine platform edges in their own CLI modules.Review
Three independent adversarial reviews (behavior parity, correctness, ADR 0019 conformance) ran before this opened;
95d609cis what they turned up. The substantive ones:captureSnapshot, so every retry resolved to a skip. Thesnapshotroute now threads them through.snapshotinto an error; it is caught and reported as a skip, and the attempt is spent before the device work as the retired route did.react-nativereached its requiredtapPointthrough?.and reporteddismissed: truewhen it was absent.capabilitiesdeclarednonewhile binding three times (§6 forbids exactly that). Every owner composes a binding's facts with the same functioninspectFactscalls, so the probes read back values the single inspection already carries — at the cost of a device claim on a read-only query. Removed, along with the last three empty-requireduses §9 retires.requireCommandSupported(last caller left with R56),CommandDispatchFacet/descriptor.dispatch/explain'sdispatch=field, and three vacuousexpect(dispatchCalls).toEqual([])assertions fed by a mock of an export that no longer exists.Verification
pnpm check:affected --runis green apart from two failures that reproduce identically onorigin/main(app-log-session-resourceanddurable-capture-resource-adoptionchmod a file unwritable, which root ignores in this container).pnpm check:layeringreports OK with all 45 migrated commands holding exactly one platform-execution path;pnpm check:fallow --base origin/mainreports no issues across 195 changed files.The R9 type-cycle ceiling drops 21 → 20 (
session-install-capability-projection.tsdeleted), and R58's callback seam had already taken it 25 → 21.🤖 Generated with Claude Code
https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX
Generated by Claude Code