refactor: migrate screenshot to request-bound runtime

Merged
TH
3mo ago
+2103-1150

Summary

screenshot no longer reaches a platform through the legacy dispatcher. It resolves a plan, admits
its exact owner facts once, binds once, and captures through the bound device runtime — the ADR 0019
request-bound path, enforced by cutover rule R39. Part of #1739 (Wave 4).

Three things change for callers:

  • --overlay-refs fails closed up front. The annotation needs a snapshot of the same screen, so
    the snapshot is now part of the declared use. A target that can capture pixels but not a tree is
    refused before a PNG is written, instead of producing a file and then failing — and the refusal
    names the flag to drop:

    $ agent-device screenshot shot.png --overlay-refs --session s
    --overlay-refs annotates a capture with the refs of a snapshot taken on the same screen,
    which this target cannot capture.
    hint: Re-run screenshot without --overlay-refs.
    
  • Unsupported targets are refused by their owner, not by a hand-maintained bucket. The message is
    unchanged (screenshot is not supported on this device); the hint now comes from the owning
    runtime's fact and the payload carries details.reason. watchOS is the one cell that tightens: the
    capability bucket used to admit it and the Apple interactor then threw UNSUPPORTED_PLATFORM while
    being constructed. It is now an unavailable fact with a hint, refused before any binding.

  • capabilities derives the screenshot row from captureScreenshot, like apps, appstate,
    open, and viewport already do.

CLI grammar, flags, help text, output shape, and recorded session actions are unchanged.

Retiring the command leaf required retiring every dispatchCommand(device, 'screenshot', …) edge, so
two observation paths that captured a screenshot as evidence move onto the same admitted seam: the
sparse-snapshot fallback (sparse-fallback-screenshot.ts) and the Android snapshot-timeout evidence
capture (android-snapshot-timeout-evidence.ts). That is cutover scope, not scope creep — the
dispatch handler cannot be deleted while any caller survives.

Retirement list

  • DISPATCH_HANDLERS.screenshot and handleScreenshotCommand (src/core/dispatch.ts).
  • The descriptor's dispatch: {} projection and capability: ALL_DEVICE_COMMAND_CAPABILITY, plus
    'screenshot' in WEB_QUERY_COMMANDS and HARMONYOS_SUPPORTED_COMMANDS.
  • The generic route's requireCommandSupported for screenshot. Rather than adding a second hardcoded
    command name beside viewport's, the exemption is registry-derived
    (commandUsesDeviceRuntimeExecution): a device-runtime descriptor never consults a capability
    bucket. Viewport's hardcoded name goes with it, and the next unit's descriptor flip is the whole
    change.
  • The dispatchCommand-backed AgentDeviceBackend, the ScreenshotOutputPlacement type, and the
    outputPlacement parameter with both of its dispatch shapes — they were already behaviorally
    identical, because handleScreenshotCommand collapsed positionals[0] ?? outPath.
  • All screenshot special-casing in request-generic-dispatch.ts: executeScreenshotPlatformCommand,
    resolveScreenshotOutputPlacement, applyScreenshotOverlay, and the four positional/--out
    resolvers. The dispatcher went 425 → 317 LOC and no longer branches on a command name at all.
  • resolveCapturedScreenshotPath / hasStringPath in the Android evidence path (the operation
    returns void; the destination is the one the caller reserved).
  • Viewport's inline branch in request-router.ts, folded into one resolver so the router stays
    orchestration-only.
  • Duplication the unit would otherwise have added, caught by fallow on changed code:
    WebDriverPlatformRuntimeOptions replaces the same inline options object repeated at three call
    sites, and repeated x ? available : unavailable conjunctions in freezeUnavailableFacts,
    webRuntimeFacts, webDriverFacts, and two fixtures each collapse to one evaluation. The
    screenshot binder itself is union-free: each entry point supplies its own owner resolution, so
    there is no ownership discriminator to share and nothing to clone from the snapshot facet.
  • dispatchGenericCommand split (22 cyclomatic / 111 lines, and the only remaining
    over-threshold function in the changed set): its post-execution tail is now
    finalizeGenericCommand, and the three-clause recovered-dialog condition is
    withRecoveredDialogWarning. Both are below threshold.
  • The captureScreenshotUse facade re-export, which had no production consumer once the descriptor
    took the plan array — the use is module-private again.
  • Structural: screenshot-overlay.ts was 635 LOC — past the extract-before-adding-behavior
    tripwire. Its PNG raster/font renderer is now screenshot-overlay-draw.ts (153 LOC), leaving 486
    LOC that answer only "which node earns a ref, and where does its rect land". clamp was duplicated
    across both halves and now has one owner in screenshot-overlay-rects.ts.

Shared Wave 4 capture debt: screenshot-overlay-android.ts — declined, with evidence

Enumerated consumers: screenshot-overlay-android.ts has exactly one importer
(screenshot-overlay.ts:17), and screenshot-overlay.ts has two production importers — the
screenshot route (mine) and android-snapshot-timeout-evidence.ts:10, whose annotation path this
unit does not migrate (only its capture leg moves). The Android source-rect resolver is therefore not
reachable from screenshot alone, so retiring or relocating it here would either break that consumer
or claim a file whose remaining consumer I did not migrate. It stays assigned-but-open Wave 4
debt.
Its natural owner is whoever migrates that annotation path.

One correction for the tracker: resolveAndroidOverlaySourceRect is not platform-gated —
screenshot-overlay.ts calls it unconditionally and it self-selects on node shape. It imports
nothing from src/platforms/**, so it is not a daemon→platform dependency edge; it is Wave 6
branch/tag-floor material, not terminal-gate material.

The overlay source-rect resolution facet named in #1739's Wave 4 facet list is deliberately not
landed here: its only non-screenshot consumer is that same annotation path, so a contract facet now
would be a registration API with one contributor — the rule this tracker already applies to iOS
presentation.

Validation

pnpm check:unit (928 files, 7,010 tests green) and pnpm check:affected --run green locally, serialized against
the other Wave 4 worktrees under the shared lock. pnpm check:layering green with R39 in the migrated-command list; the
largest type-level import cycle dropped from 46 files to 26 because screenshot stopped threading
src/commands/** through generic dispatch, and the R9/R10 zone ceilings are lowered to match in this
change (commands and client leave the cycle entirely).

Red before green. Every new gate was seen failing first:

  • The R39 row went red twice before it went green — expected one narrowed captureScreenshot call, found 0 while the new modules were untracked, then found 2 once both plan branches called the
    operation directly. Funnelling both through selectScreenshotCapture is what makes the row's
    singular-execution claim true.

  • src/core/__tests__/dispatch-screenshot.test.ts asserts the legacy dispatcher answers
    INVALID_ARGS: Unknown command: screenshot and never reaches an interactor. Reverting only
    src/core/dispatch.ts to f3d5b3d92 and rerunning it goes red exactly as it should — legacy
    dispatch really did capture:

    - Error { "message": "rejected promise" }
    + { "message": "Saved screenshot: /tmp/out.png", "path": "/tmp/out.png" }
    
  • The exhaustive fact catalog rejected the change until all eight runtime owners classified
    captureScreenshot: Property 'captureScreenshot' is missing in type … but required in type …
    across every owner and fixture.

Tests moved off the dispatch mock. request-router-screenshot.test.ts no longer fakes screenshot
through vi.mock('core/dispatch.ts'); it binds a fake runtime at inspectFacts/bindDevice via a
new screenshotRuntimeFixture, and the binder-level cases split into
src/daemon/__tests__/screenshot-runtime.test.ts (907 → 639 + 203 lines, so neither approaches the
1,000-line ratchet). sparse-fallback-screenshot.test.ts and the Android-timeout leg of
snapshot-handler.test.ts moved to the same seam; the latter shrank, and its ratchet pin is lowered
from 2654 to 2640. The file still mocks dispatch for click/scroll, which are not migrated.

Two overlay expectations changed, and the new values are the truer ones: the fake now supplies the
backend the production contract requires, so Android's "rects are already device pixels, do not
project" rule applies exactly as it does on a real emulator. The old expectation was an artifact of a
fixture that omitted backend.

Rebased onto 8c06965d2 and re-verified end to end. Conflicts were with main's own commits,
not with siblings — none of the five wave-4 units has merged, so WEB_QUERY_COMMANDS /
HARMONYOS_SUPPORTED_COMMANDS merged clean and the predicted five-way conflict falls to whoever
lands second. Two resolutions worth naming:

  • #1873 made viewport a required cell on UnavailablePlatformRuntimeFacts so an owner
    cannot inherit the transport gap silently. screenshot follows it: also required. That surfaced
    every gap site — linux, vega, limrun, webdriver ×2, the gateway ×2, and three fixtures — each of
    which now states its own capture reason instead of falling back to network.
  • The shared interactor-binding extraction is gone. find (#1876) threads a per-capture
    AbortSignal through exactly the bindSnapshotInteractor body I had extracted, so that
    extraction was a competing shape on a seam find owns. packages/contracts/src/snapshot-runtime.ts
    is now byte-identical to main and this branch adds no diff to it; the screenshot facet dedupes
    within itself instead.

Live evidence — iOS Simulator (iPhone 17, iOS 26.2, UDID 1604B975-…), packaged daemon after
pnpm build && pnpm clean:daemon.
Captured at f3d5b3d92 before the change and re-run on this
branch; results are identical:

CaseBeforeAfter
screenshot positional.png402×874 @1x, valid PNGidentical
screenshot --out out-flag.png402×874 @1x, valid PNGidentical
screenshot (default)reserved temp agent-device-screenshot-*/screenshot.pngidentical
screenshot --overlay-refs12 refs, first e6 rect 16,168 370×90.33identical, ref-for-ref
--pixel-density 2—804×1748, logical 402×874, density 2
--scale 0.5—201×437, logical density correctly omitted
capabilitieslists screenshotlists screenshot, now from the fact

Two consecutive plain captures of a static screen are byte-identical (md5 equal) while the
--overlay-refs capture differs — which is what proves the annotation is drawn into the file rather
than only reported in the response. Session w4shot2 closed; session list is empty.

Tradeoffs and known gaps

  • Size budget missed on one axis. Shipped size is within budget (JS gzip +1.6 kB of a
    +3 kB budget; raw +5.1 kB, tarball +1.9 kB, unpacked +5.1 kB) and packages/** production is
    +10,629 B of a +14,000 B budget. But root src/** production is +12,495 B against a
    posted budget of ≤ −1,500 B, so the move-dominated claim does not hold for this unit. Itemized:
    screenshot-runtime-binding.ts +4,607 (the plan→admit→bind spine, which has no legacy
    counterpart to delete — legacy execution selection was one dispatchCommand line); +4,082 net
    across screenshot-runtime.ts / request-generic-dispatch.ts / dispatch.ts for behavior that
    moved out of the generic dispatcher plus its new finalizer; generic-runtime-execution.ts +1,274
    for the runtime-owned-leaf resolver; +475 net for the overlay module split; the remainder is the
    registry predicate, the two observation call sites, and the required-cell fills. I did not trim
    doc comments to buy the number back. Flagged for the wave-level decision rather than
    re-baselined.
  • src/daemon/screenshot-runtime.ts is 314 LOC, marginally over the 300 target and well under the
    500 extract line. Splitting it would add bytes to an already-missed budget for a 4% overshoot.
  • The daemon restates the capture command's result shape locally instead of importing
    ScreenshotCommandResult: R2 keeps daemon/ below the command surface, type-only imports
    included. Removing two unreachable typeof data.path !== 'string' guards is what surfaced it —
    the layering gate caught the import the same run.
  • captureSnapshot's optional captureData seam in snapshot-capture.ts stays optional and
    captureSnapshotWithInteractor stays: this is not the last selector unit. The unit that lands
    last makes it required and deletes the fallback.
  • Fact cells not exercised live: watchOS refusal, Vega refusal, and the provider (limrun/webdriver)
    cells are fixture-backed only. The assigned family for this unit is the iOS Simulator, and
    siblings own the other devices.
  • Expect a textual conflict with the find/get/is/wait units in src/core/capabilities.ts:
    all five delete one entry each from WEB_QUERY_COMMANDS and HARMONYOS_SUPPORTED_COMMANDS. The
    resolution is always "keep every sibling's deletion".

Docs/skills: not updated, and deliberately. No CLI grammar, flag, help, or output change; the
unsupported-message text is unchanged; website/docs/docs/commands.md's only screenshot support
claim (Vega reports unsupported) stays true under the new facts.

Scope: 64 files (33 production, 31 test). Scope expanded beyond the single generic-route leaf
only where the cutover forced it — the two observation capture sites named above.