Commit 2964477
authored
refactor: migrate back/home/orientation/tv-remote/keyboard to the request-bound device runtime (#1955)
* refactor: migrate back/home/orientation/tv-remote/keyboard to the request-bound device runtime
Continues the ADR 0019 platform-runtime migration (Wave 5 generic leaves):
five generic-route commands move off dispatchKnownCommand/Interactor legacy
dispatch onto fact-owned admission, one bind per handler. keyboard uses the
R35 action-selected single-bind pattern (status/dismiss/enter each admit and
bind independently). All 8 owner runtime packages gained fact-cell tests for
the new operations; six smoke-coverage integration oracles and nine
daemon/capability unit test files were updated for the retired capability-
catalog admission these commands no longer carry.
* refactor: extract shared interactor-resolution prelude in keyboard-runtime
bindKeyboardStatus/Dismiss/Enter repeated the same signal-check +
resolveInteractor call; factor it into resolveKeyboardInteractor so each
binder is a two-line call instead of a six-line copy. No behavior change —
the three contract-module mutants planted earlier in review still kill on
this shape.
* fix: refuse watchOS admission for back/home/orientation/keyboard; pin tv-remote non-TV parity
P1: watchOS has no constructible Apple interactor (XCUITest cannot drive its UI, ADR-0009),
matching the existing captureScreenshot/captureSnapshot/readTextAtPoint/findSelector pattern
in this same file. appleBackFact/appleHomeFact/appleMobileInputEligible admitted every
Apple OS but tvOS/macOS, wrongly including watchOS. Facts now refuse watchOS explicitly for
back, home, orientation, and keyboard dismiss/enter, with a fact-cell test asserting no
binding for every one of them.
P2: verified the daemon's generic-route capability gate already reproduced the retired
per-platform tv-remote hint text (message stays the generic "<command> is not supported on
this device", hint carries the owner-specific text) for every device that could reach
dispatch in the old system -- the retired handleTvRemoteCommand's own "supported only on TV
targets" check was unreachable there and only exercised by a test calling dispatchCommand
directly. Added a daemon-level test pinning the exact iOS and Android-mobile hint strings to
make that parity explicit instead of implicit.
Also fixes a fallow complexity finding the P1 test edit introduced by splitting the fact-cell
assertions into five small named helpers instead of one large function.
* fix: stop orientation-runtime.test.ts's router-join test from hitting real adb
Root-caused the CI-only Coverage failure (unreproducible locally in isolation,
reproducible 2/2 in the full CI run): every generic-route leaf this migration
touches carries `androidBlockingDialogGuard: true`, and `dispatchGenericCommand`
calls `ensureNoAndroidBlockingDialogReady` unconditionally for any
`platform: 'android'` session reaching the real request router -- regardless of
whether admission is fact-based or capability-based. That check calls
`getAndroidBlockingDialogFocus`, which shells out to the real `adb` binary.
orientation-runtime.test.ts's "request router joins..." test used a synthetic
`platform: 'android'` device through `createRequestHandler` (the real router),
without stubbing the platform ADB layer -- only the runtime gateway was mocked.
On a host with a real `adb` binary (my machine) the subprocess fails fast and
`allowFailure` tolerates it, costing ~800ms-1.1s but still succeeding. On a host
with no `adb` binary at all (CI's Coverage job, a plain unit-test lane with no
Android SDK) the spawn itself throws, which isn't something `allowFailure`
catches, producing exactly the observed `ok: false` unsupported-operation
response.
back/home/tv-remote's equivalent router-join tests already use Apple/Vega
devices, so they never reached this path. Switched orientation's fixture to
match -- Apple, since the fixture's facts/execution are fully synthetic and
platform-agnostic regardless.
Also: renamed the widely-shared 'emulator-5554'/'ios-simulator' device-id
literals in back/orientation/tv-remote/keyboard-runtime.test.ts to file-scoped
ids. Device claims for a `local-family` owner binding hit the real on-disk
`require-owner` claim file (keyed only by canonical device id), and 27+
pre-existing test files already share 'emulator-5554'; this migration added
three more consumers of it under a `require-owner` policy that reaches real
admission, which was worth eliminating as a source of doubt even though it
wasn't the actual root cause here.
* refactor: extract navigation/keyboard concepts into sibling modules; test the real Android dialog-guard path
packages/platform-apple/src/runtime.ts and packages/provider-limrun/src/app-log-runtime.ts
grew past the repo's 500-line extraction threshold. Move the new back/home/orientation/
tv-remote/keyboard facts and bindings into packages/platform-apple/src/navigation/runtime.ts
(new sibling module, matching deployment/runtime.ts's existing pattern), and the new keyboard
facts/bindings for limrun into the existing packages/provider-limrun/src/interaction-operations.ts
(which already held the sibling navigation logic).
Also fix orientation-runtime.test.ts's router-join test: it previously swapped its device
fixture from Android to Apple to dodge the real adb-backed blocking-dialog guard, which masked
the Android route that was actually failing in CI. Keep the Android fixture and stub
getAndroidBlockingDialogFocus instead, the same seam request-router-android-modal.test.ts
already uses.
* refactor: adopt granular contracts subpaths for back/home/orientation/tv-remote/keyboard
Following main's #1969 (facade granularization), give each of this branch's five
new contract modules their own package.json entry subpath and move every
value-importer (owner runtime packages, the daemon binders, and their tests) off
the wide @agent-device/contracts/platform facade onto the specific module that
owns the symbol — the same convention #1969 established for the rest of the
vocabulary. Keeps this migration's files out of the contracts-entry-closure gate
and out of the eager-evaluation cost #1969 measured for the daemon's permanent
hubs (registry.ts, dispatch.ts).
* refactor: shared navigation/keyboard binder table; dedupe keyboard admission; drop restated types
Addresses the review's finding 1 (seven per-owner copies of the same
"fact-keyed table of interactor binders" pattern) by extracting
bindAdmittedLocalInteractorOperations/bindAdmittedProviderInteractorOperations
into packages/contracts/src/interactor-operation-catalog.ts. Each owner now
requests the subset of back/home/setOrientation/tvRemote/keyboard{Status,
Dismiss,Enter} it admits, instead of hand-writing
`facts.operations.<key>.available ? bind…(resolver) : {}` per operation.
Applied across all seven call sites (apple, android, harmonyos, vega, linux,
webdriver, limrun) and collapsed limrun's two separate bind functions
(navigation, keyboard) into one shared call.
Finding 3 (resolveBoundKeyboardRuntime copy-pastes admit-then-wrap three
times): extracted a local admitKeyboardAction<...> helper mirroring
resolveBoundGenericRuntime's admit-then-defer shape, so the three action
branches (status/dismiss/enter) share one admission path.
Finding 6 (execute* helpers hand-restate a contract that can drift): back/
home/orientation/tv-remote/keyboard's execute functions are now typed off
`BoundDeviceRuntime<typeof xRuntimeUse>` (derived from the actual bind-use
value) instead of a hand-written `Readonly<{ operations: Readonly<{...}> }>`
shape. Also fixed provider-limrun's `RuntimeOperationUnavailability |
{ available: true }` restating RuntimeOperationFact by hand — folded away
entirely once the bind functions it typed were removed.
Finding 7 (naming/placement): platform-apple/runtime.ts's misleadingly-named
`captureOperations` bucket (held deployment/network/recording/find, not just
capture) collapsed into one flat `operations` object now that the navigation
bucket is a single function call instead of six ternaries.
Exported RuntimeAdmissionRequest from runtime-admission.ts (needed by the new
keyboard admission helper). Added packages/contracts/src/
interactor-operation-catalog.test.ts for the new shared binder table.
pnpm typecheck, check:fallow, check:layering, and the full unit-core suite
(1010 files / 7513 tests, one known contention-flake excluded) are green.
* refactor: split generic-mutating command traits from the legacy dispatch pair
Addresses the review's finding 5: GENERIC_MUTATING_LINUX_DEVICE_COMMAND_TRAITS
bundled two orthogonal things (daemon/recording traits, and the legacy
capability+dispatch pair migration strips), forcing every migrated
descriptor to hand-expand the constant minus two fields plus an explanatory
comment.
Split into GENERIC_MUTATING_COMMAND_TRAITS (the shared daemon/recording
traits) and LEGACY_LINUX_DEVICE_EXECUTION (the dispatch/capability pair).
back/home/orientation/tv-remote (this migration) and focus (an earlier one,
same pattern, previously a stale reference to the retired constant name)
now spread the trait constant directly instead of hand-expanding it; the
still-legacy `scroll` descriptor spreads both pieces, equivalent to the
retired constant.
pnpm typecheck, check:fallow, check:layering, and the registry/daemon test
suites are green.
* refactor: table-ify packages/contracts/src/keyboard-runtime.ts's three-way duplication
Finding 3's second half: the three bindKeyboardX functions and six
bindLocal/ProviderKeyboardXInteractor entry points differed only by method
name and label string. Replaced with one generic bindKeyboardAction<Key>
dispatching off the operation key (interactor[key], resolved from a small
label table) plus two shared local/provider dispatch helpers the six named
exports each call with their own key — collapsing three copies of the bind
logic into one and six near-duplicate entry-point bodies into one line each,
while keeping every exported name and type signature unchanged.
pnpm typecheck, check:fallow, check:layering, and pnpm check:affected --run
are green.
* refactor: parameterize runSessionOrSelectorDispatch with an execute strategy
Addresses the review's finding 2: handleKeyboardCommand re-implemented
runSessionOrSelectorDispatch's orchestration step for step (session/selector
guard, device resolve, ref-frame expiry, record) instead of reusing it,
because the shared function had no seam for keyboard's bind-and-execute
admission — only the legacy requireCommandSupported + dispatchCommand path.
That left the shared orchestrator with one caller instead of two, and set a
precedent that would fork a new copy for each of the 28 remaining
session-route migrations.
Gave runSessionOrSelectorDispatch an `execute` parameter: the orchestration
(guard, resolve device, admit-then-execute, expire ref frame if mutating,
derive and record next session) stays in one place, and callers supply their
own admission/execution strategy. Extracted `legacySessionDispatchExecute`
for the still-legacy capability-gate-then-dispatchCommand shape
`handleTriggerAppEventCommand` (the remaining legacy caller) now passes
explicitly, and `keyboardSessionExecute` for keyboard's bind-and-execute
shape. Deleted the now-fully-redundant `executeBoundKeyboardCommand` — its
result recording duplicated what the shared orchestrator's tail already
does.
pnpm typecheck, check:fallow, check:layering, the full daemon test suite
(321 files / 2271 tests), and pnpm check:affected --run are green.
* refactor: extract limrun facts-runtime.ts; discriminate KeyboardDismissResult by owner
app-log-runtime.ts was still 589 lines after the shared-abstraction fixes; moves fact
assembly (limrunAppLogFacts/limrunAppLogRecoveryFacts/limrunLifecycleFacts/deploymentOptions)
to a new facts-runtime.ts and the shared device-identity predicate to device.ts, the leaf
both files already depend on. app-log-runtime.ts is now 336 lines.
KeyboardDismissResult was an 11-field optional bag with executeKeyboardDismiss separately
re-deriving platform from the device and projecting subsets by hand. Each owner (android,
apple, harmonyos) now tags its own result with a `kind` discriminant, so an owner can only
ever produce its own shape, and the daemon derives the wire `platform` label from `kind`
instead of guessing from the device a second time. Wire output is unchanged.
* fix: expire ref frame before the mutating call, not after; extract session/selector dispatch; derive catalog operations from facts
runSessionOrSelectorDispatch awaited execute(device, session) — which bundled admission
and the mutating invocation together — before expiring the ref frame, so a rejecting or
timed-out invocation left a stale frame active (ADR 0014 requires expiry immediately
before the mutating call, with no success-only rollback). Split the execute thunk into
`prepare` (admission only) + a deferred `execute` invocation, so the orchestrator can
expire between them regardless of how the invocation resolves. Added a regression test
proving the frame still expires when the invocation rejects.
Extracted runSessionOrSelectorDispatch and its keyboard/trigger-app-event callers into a
new session-selector-dispatch.ts, matching this file's own convention of one file per
command-group (session.ts shrinks from 571 to well under its 500-line budget).
bindAdmittedLocalInteractorOperations/bindAdmittedProviderInteractorOperations accepted
both a facts object and a separately hand-maintained `operations` array naming the same
keys — a second source of truth that could drift from what the facts actually admit.
Removed the array; the binder now walks the fixed set of navigation operations and lets
each owner's own facts decide what binds, exactly as before but with one source of truth.
* style: reformat legacySessionDispatchExecute call in session-selector-dispatch.ts
* refactor: derive catalog operation list from one canonical tuple; move keyboard orchestration tests
NAVIGATION_INTERACTOR_OPERATIONS was declared as a plain readonly array independently
of the NavigationInteractorOperation union it walked, so a future union member could
compile without ever being added to the walk list, silently preventing an admitted
fact from binding. Made the tuple the single canonical value: the union type is now
derived from it via `(typeof TUPLE)[number]`, so LOCAL_BINDERS/PROVIDER_BINDERS'
Record<NavigationInteractorOperation, ...> completeness is checked against the same
tuple, not a separately hand-kept list. Added a regression test binding all seven
operations at once to pin the runtime walk, independent of the type-level guarantee.
Moved the four keyboard-orchestration tests (the two ADR 0014 ref-frame seam tests
plus the two session/selector-guard tests) out of the mixed appstate/perf test file
into a new session-selector-dispatch.test.ts, colocated with the file they exercise.
Strengthened the rejection regression test to assert the frame is already expired
from inside the rejecting keyboardDismiss callback itself, pinning the exact
pre-invocation seam rather than only checking the end state after the dispatch settles.
* fix: restore back/home/orientation/tv-remote/keyboard-runtime exports lost in rebase
Rebasing onto origin/main dropped these five package.json export entries during
conflict resolution (the granular-subpath commit's package.json changes silently
lost during merge). Restored, confirmed by pnpm typecheck across all 17 workspace
packages and the full unit-core suite (1023 files / 7581 tests).
* test: pin the exact point the live iOS email field value goes missing
Two prior CI runs on this PR saw the seeded email field ("ada@example") end up
containing only a typed suffix (".test") by the time the flow reads it back at
the end — after fill, keyboard dismiss, coordinate refocus, and type. Since this
PR touches executeKeyboardDismiss's response shaping, the reviewer asked to
disprove keyboard dismiss as the cause rather than assume the pre-existing
dropped-keystroke flake pattern applies.
Added two read-back checkpoints: right after seeding (before dismiss runs at
all) and right after dismiss (before the coordinate refocus + type steps that
follow). If both hold "ada@example", the loss happens during refocus/type, not
dismiss — matching the documented flake, not a regression in this PR's diff.
* refactor: make keyboard status/enter owner-discriminated too; trim review-round prose
KeyboardStatusResult and KeyboardEnterResult were bare objects; executeKeyboardStatus
and executeKeyboardEnter derived the wire platform label from device.platform via
keyboardPlatformLabel, the same re-derivation already fixed for dismiss. Each owner
now tags its own result with a kind (android's status/enter as 'ime-probe' and
'android-acknowledged', harmonyos's enter as 'harmonyos-acknowledged', apple's enter
as 'visibility-echo'), and the daemon derives platform from a kind-keyed lookup table
for all three actions. keyboardPlatformLabel and its isIosFamily import are gone —
nothing derives platform from the device anymore. Android and HarmonyOS's enter
acknowledgments are structurally identical (empty besides kind), so the discriminant
alone — not result shape — is what tells the daemon which owner actually ran.
Added a harmonyos enter test alongside the existing ios/android ones so all three
owners are covered for both dismiss and enter's kind-to-platform mapping.
Also trimmed several comments that narrated which PR review round motivated them
down to just the durable invariant or rationale — the type shape, test names, and
assertions already carry the proof.1 parent c4b1a61 commit 2964477
109 files changed
Lines changed: 6321 additions & 1532 deletions
File tree
- packages
- contracts
- src
- facades
- platform-android/src
- platform-apple/src
- platform-harmonyos/src
- platform-linux/src
- platform-vega/src
- platform-web/src
- provider-limrun/src
- provider-webdriver/src
- scripts/layering
- src
- __tests__
- contracts
- test-utils
- core
- __tests__
- command-descriptor
- __tests__
- interactors
- daemon
- __tests__
- handlers
- __tests__
- platforms
- android/__tests__
- apple
- __tests__
- vega
- __tests__
- test/integration
- android-emulator-e2e
- ios-simulator-e2e
- linux-e2e
- macos-e2e
- provider-scenarios
- tvos-e2e
- web-e2e
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
76 | 76 | | |
77 | 77 | | |
78 | 78 | | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
79 | 83 | | |
80 | 84 | | |
81 | 85 | | |
| |||
144 | 148 | | |
145 | 149 | | |
146 | 150 | | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
147 | 155 | | |
148 | 156 | | |
149 | 157 | | |
| |||
156 | 164 | | |
157 | 165 | | |
158 | 166 | | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
159 | 171 | | |
160 | 172 | | |
161 | 173 | | |
162 | 174 | | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
163 | 179 | | |
164 | 180 | | |
165 | 181 | | |
| |||
180 | 196 | | |
181 | 197 | | |
182 | 198 | | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
183 | 203 | | |
184 | 204 | | |
185 | 205 | | |
| |||
272 | 292 | | |
273 | 293 | | |
274 | 294 | | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
275 | 299 | | |
276 | 300 | | |
277 | 301 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
123 | 123 | | |
124 | 124 | | |
125 | 125 | | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
126 | 129 | | |
127 | 130 | | |
128 | 131 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
225 | 225 | | |
226 | 226 | | |
227 | 227 | | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
228 | 236 | | |
229 | 237 | | |
230 | 238 | | |
| |||
315 | 323 | | |
316 | 324 | | |
317 | 325 | | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
318 | 397 | | |
319 | 398 | | |
320 | 399 | | |
| |||
0 commit comments