Skip to content

feat: add scale-only screenshot sizing - #1617

Merged
thymikee merged 6 commits into
callstack:mainfrom
szdziedzic:szdziedzic-codex/screenshot-scale
Aug 6, 2026
Merged

feat: add scale-only screenshot sizing#1617
thymikee merged 6 commits into
callstack:mainfrom
szdziedzic:szdziedzic-codex/screenshot-scale

Conversation

@szdziedzic

Copy link
Copy Markdown
Contributor

Summary

  • add scale-only screenshot sizing across CLI, config/env, Node.js/MCP, daemon execution, and replay scripts
  • resize captured PNGs centrally before artifact publication so local and matching-version remote daemons share the same behavior
  • remove screenshot and video max-size behavior, including Android/iOS recording plumbing and the obsolete Swift resize helper
  • keep recording focused on FPS, quality, and touch-overlay controls; older clients sending the removed recording field now receive an explicit error

The change touches 58 files. Scope expanded from the screenshot command family into recording because video scaling was intentionally removed rather than left as a hidden or partially supported path.

Known gap: a newer client talking to an older remote daemon can have the optional screenshot scale field ignored. A follow-up capability/acknowledgement mechanism is needed for guaranteed behavior across daemon version skew; matching-version remote daemons resize before artifact download.

Validation

Formatting, lint, typecheck, production build, and fallow audit passed. The unit suite passed 2,972 tests; smoke passed 9 cases with one expected live-web skip. Cross-platform integration passed 10 Node cases with six expected live-device skips and 85 provider-integration tests. The provider scenario verifies a 64x64 screenshot becomes 32x32 at scale 0.5. Both iOS and macOS XCTest runner builds passed. CLI help exposes screenshot --scale only, and recording --max-size fails as an unknown flag. No live EAS Simulator or physical-device session was run.

@szdziedzic
szdziedzic force-pushed the szdziedzic-codex/screenshot-scale branch from 33e52c7 to 0ddbcaf Compare August 5, 2026 12:37
@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member

The matching-version architecture is sound: scale projects through CLI/config, Node/MCP and the daemon, then the daemon resizes the captured PNG centrally before artifact publication. The 64×64 → 32×32 provider scenario also exercises that production route rather than only the helper. This head is not ready, though:

  • P1 — preserve or explicitly refuse released .ad syntax. Removing --max-size recognition from the screenshot and record script parsers turns a released line such as screenshot "x.png" --max-size 1024 or record start "x.mp4" --max-size 1024 into extra positionals. The handlers consume the normal path/action fields and silently run at native size. Add frozen replay-compat witnesses for both shipped forms and either retain legacy execution or return an actionable migration refusal; sizing must not disappear silently.
  • P1/P2 — fail closed for old-client/new-daemon screenshots. A released client sends flags.screenshotMaxSize; the new screenshot projection ignores it and returns a full-size artifact. Recording already detects the raw retired field and returns an explicit error. Screenshot needs equivalent compatibility handling (or preserved semantics). The disclosed inverse skew—new scale sent to an older daemon—is still a residual product gap until command capability/acknowledgement exists.
  • Rebase/ownership blocker. This commit is based on the pre-package tree, hundreds of commits behind current main, conflicts, and edits superseded src/contracts / src/replay owners. Rebase and port the change into @agent-device/contracts and @agent-device/ad-script, preserving current screenshot pixelDensity, recording scope/backend/session behavior, package façades, and current docs.
  • Gate omission. scripts/integration-progress-model.ts still classifies screenshotMaxSize and does not classify screenshotScale; update the device-observable flag row. Also include the required quoted red proof for the new dimension regression (without central scale resize it must remain 64×64 rather than 32×32).
  • Readiness evidence. This head has no CI and the PR reports no live device run. After the rebase and green authoritative gates, show at least one simulator/emulator native-vs---scale 0.3 PNG dimension comparison (ideally with --overlay-refs) and that the session remains usable. Because this also removes recording plumbing across Android and Apple, smoke a default native recording start/stop on the affected backends or state the unverified backend as residual risk.

Please request a fresh review after the rebase; the ownership and production routes will be materially different.

@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member

Re-reviewed exact head 0ddbcaf. The rebase/ownership blocker is fixed: the change now uses the current @agent-device/contracts and @agent-device/ad-script owners, screenshotScale is classified in the progress model, and authoritative CI is green. Two blockers remain:

  • P1 — released max-size inputs still disappear silently. The .ad parser removes --max-size handling for both screenshot and record without frozen replay-compat witnesses; released lines therefore leave the flag/value as extra positionals, while the command readers consume the normal path/action slots and ignore the remainder, producing native-size success. The screenshot daemon also lacks recording’s explicit retired-field guard, so an old client sending screenshotMaxSize receives a full-size artifact. Released Node callers using { maxSize } and AGENT_DEVICE_SCREENSHOT_MAX_SIZE are likewise dropped. Preserve the shipped semantics or reject every retired path explicitly with actionable migration guidance.
  • P2 — the scale/removal evidence is not yet merge-grade. Exact-head iOS/Android smoke invokes screenshot --scale 0.5, but asserts only the returned path and a valid PNG signature, so it remains green if scale is ignored. The provider 64×64 → 32×32 assertion has no quoted red proof. Recording belongs to the full observability tiers, not the smoke tiers that ran here, and the PR body reports no live run. Please provide native-vs-scaled dimensions on a simulator/emulator (ideally with overlay refs and a subsequent command proving session usability), plus a live default recording start/stop on the affected Apple/Android paths, or explicitly downgrade the unverified backends to residual risk.

The PR remains draft; no readiness label should be applied yet.

Released sizing inputs must fail closed with migration guidance instead of
silently producing native-size artifacts:

- contracts: RETIRED_SCREENSHOT_MAX_SIZE declaration + SCREENSHOT_SCALE_LIMITS
  as the single source for the scale bounds and migration messages
- .ad parser: released 'screenshot ... --max-size N' and 'record start ...
  --max-size N' lines now refuse at parse time (frozen replay-compat witnesses)
- daemon: screenshot rejects old-client screenshotMaxSize like recording does;
  the recording guard now shares the same contract data
- Node client: screenshot/record daemon writers refuse the removed { maxSize }
  option before transport
- CLI: --max-size unknown-flag error carries the migration guidance
- config/env: stale screenshotMaxSize config keys and the retired
  AGENT_DEVICE_SCREENSHOT_MAX_SIZE env var are refused for sizing commands
  (other commands keep working)

Quality: numberField now reuses the canonical readOptionalNumber contract
helper (AppError bounds instead of plain Error); png-resize inlines one-use
wrappers and restores the worker-thread rationale; docs typo fixed.
@thymikee

thymikee commented Aug 6, 2026

Copy link
Copy Markdown
Member

Addressed both remaining blockers at a6881be0b (pushed to this branch).

P1 — every released max-size path now fails closed

One declaration in contracts (RETIRED_SCREENSHOT_MAX_SIZE, with per-command migration messages) is now consumed by every surface that used to honor the flag; nothing degrades silently anymore:

  • .ad scripts: screenshot "x.png" --max-size 1024 and record start "x.mp4" --max-size 1024 refuse at parse time with INVALID_ARGS + migration guidance. Frozen replay-compat witnesses pin both released forms in packages/ad-script/src/internal/__tests__/script.test.ts (commented as frozen — the inputs must never be updated).
  • Daemon (old client → new daemon): screenshot now rejects a raw screenshotMaxSize request flag exactly like recording does; the recording guard was rewired onto the same shared contract data. Witness tests on both routes.
  • Node API: capture.screenshot({ maxSize }) and recording.record({ maxSize }) refuse before transport. (MCP was already fail-closed via additionalProperties: false.)
  • Env: the derived-and-released AGENT_DEVICE_SCREENSHOT_MAX_SIZE refuses for screenshot/record and is inert for every other command (witnessed: devices still works with the stale var exported).
  • Config: a stale screenshotMaxSize key already failed closed generically; it now carries the migration message.
  • CLI: the unknown-flag error carries the migration guidance (live: Unknown flag: --max-size. screenshot --max-size was removed; use --scale <0.01-1> (or AGENT_DEVICE_SCREENSHOT_SCALE) to downscale proportionally. record --max-size was removed; recordings capture at native resolution.)

P2 — evidence

Quoted red proof for the provider 64×64 → 32×32 assertion (central resize temporarily disabled, then restored):

FAIL |provider-integration| test/integration/provider-scenarios/macos-desktop.test.ts
AssertionError: Expected values to be strictly equal:  64 !== 32
 ❯ assertPngDimensions test/integration/provider-scenarios/assertions.ts:113:10
 ❯ Object.assert test/integration/provider-scenarios/macos-desktop.test.ts:321:15

Live iOS simulator (iPhone 17 Pro, iOS 26.2, dedicated boot, isolated --state-dir daemon, this branch's build):

step result
screenshot native.png 402×874 (pixelDensity 1)
screenshot --scale 0.3 121×262 (exact: 402→121, 874→262), verified on disk with sips
screenshot --scale 0.5 --overlay-refs 201×437 with refs burned for the final size
session usable after snapshot -i ok, click text=GeneralTapped text=General (201, 406)
record start/stop --hide-touches playable MP4 at native 1206×2622
record start/stop (default overlays) playable MP4 + gesture telemetry sidecar; 220×480 output is the pre-existing overlay medium-preset export, untouched by this PR

Live Android emulator (Pixel_7_CI, emulator-5554):

step result
screenshot 1080×2400
screenshot --scale 0.3 324×720 (exact)
record start/stop --hide-touches playable MP4, native 1080×2400, 5.8 s — exercises the rebuilt screenrecord command (no --size, no wm size probe)
session usable after press mid-recording ok, snapshot -i after stop ok

Live retired-path proofs on the same builds: CLI --max-size and exported AGENT_DEVICE_SCREENSHOT_MAX_SIZE both return the migration refusal quoted above.

Residual risk (explicitly unverified live): physical-iOS-device and macOS runner recording (recordStart no longer carries maxSize). Covered by the runner-protocol fixture round-trip and the macOS recording provider scenario (transcript asserts recordStart without maxSize); no hardware/runner session was exercised here.

Quality changes riding along

  • Scale bounds (0.01–1) had been hardcoded in five places with two message spellings; now SCREENSHOT_SCALE_LIMITS in contracts is the single source (flag definition, script parser, validator, numberField, png-resize).
  • numberField's bounds checks threw plain Error while its siblings threw AppError('INVALID_ARGS', …); it now reuses a new canonical readOptionalNumber next to readOptionalInteger in contracts, and the local duplicate is deleted.
  • png-resize single-use readPng/writeResizedPng wrappers inlined back (matching its sibling resizePngFile), and the worker-thread rationale comment restored.
  • screenshotFlagsFromPublicOptions now states its constraint: the base projection is fed generic cross-command option bags where a bare scale belongs to the pinch gesture, so only the screenshot-boundary variant may read it.
  • Docs: dropped the stray "Argent-style" phrasing; added a --max-size--scale migration line.

Gates on this head: typecheck, oxlint, format, layering guard (facade symbols updated), full unit suite (613 files / 5424 tests), all 42 provider-scenario files (149 tests), fallow audit + production-exports — all green locally.

* origin/main:
  chore: Update GitHub Sponsors usernames in FUNDING.yml
  fix: resolve Dependabot security alerts (callstack#1623)
  fix: update MCP registry namespace (callstack#1618)
  fix(ios): corroborate recorded tap outcomes (callstack#1605)
  fix(ios): never signal a recycled runner pid from a stale lease (callstack#1621)
  build: eliminate tsdown bundle warnings (callstack#1607)
  refactor(contracts): one viewport-root predicate for the whole repo (callstack#1613)
  refactor(contracts): name façade exports explicitly and retire the pin table (callstack#1614)
  refactor(ios): share one private-XCTest event bridge between gesture and text synthesis (callstack#1608)
  refactor(daemon): give the Maestro fallback and ambiguous-match details real types (callstack#1612)
  docs: clarify iOS drag synthesis profiles (callstack#1616)

# Conflicts:
#	scripts/layering/facade-symbols.ts
#	src/commands/capture/screenshot.ts
#	src/commands/recording/index.ts
@thymikee

thymikee commented Aug 6, 2026

Copy link
Copy Markdown
Member

Addendum: after the fix commit, the branch had gone stale against main (unmergeable, so no CI could trigger). Merged origin/main in as 219cd83f9:

  • refactor(contracts): name façade exports explicitly and retire the pin table #1614 landed after this head, so the conflicts were its help-text/facade refactor: help strings moved into facet text.cliDetail (ported this PR's --scale wording there — main's copy still said --max-size), the retired facade-symbols.ts pin table resolved as deleted, and the new contracts exports are now named explicitly in facades/capture.ts / facades/command.ts.
  • Main's new MCP undocumented-input gate correctly demanded the removed record.maxSize / screenshot.maxSize allowlist entries be dropped.

Authoritative CI on 219cd83f9 is green (25 pass / 3 skip). One iOS Smoke run initially failed on testBareTypeUsesTappedInputWhenSoftwareKeyboardIsHidden ("h" vs "hardware-keyboard") — a runner typing test untouched here, with the identical failure on main's own iOS run yesterday (run 31006064499); the re-run passed.

@thymikee

thymikee commented Aug 6, 2026

Copy link
Copy Markdown
Member

Re-reviewed exact head 219cd83f. The CLI/Node/env/config/daemon guards, centralized scale bounds, central PNG resize, quoted red proof, and current CI are sound, but this head is not ready:

  • P1 — released MCP maxSize still degrades silently. readFieldInput keeps only declared metadata fields, so after maxSize was removed from screenshot/record metadata, an MCP call carrying { maxSize: 1024 } loses the key before screenshotDaemonWriter / recordDaemonWriter can run their retired-field guards. The in-process MCP executor does not enforce the advertised additionalProperties: false; an exact-head route regression reaches transport and returns success/native-size behavior instead of migration guidance. Preserve a retired MCP input seam or validate before field projection, use the canonical migration refusal, and add real MCP-executor regressions for both screenshot and record.
  • P2 — the claimed frozen .ad witnesses are not release-provenance pinned. They live only in packages/ad-script/.../script.test.ts; no test/replay-compat corpus/manifest entries were added, so check:replay-compat cannot prove these exact forms shipped. Add mined/derived corpus refusals for released screenshot and record --max-size forms.
  • Evidence refresh. The live iOS/Android dimension and recording evidence is from pre-merge a6881be0b, not final 219cd83f; the merge incorporated material Apple/runtime changes, while exact-head smoke checks only a valid PNG/path. After the MCP fix, repeat exact-head native-vs-scaled dimensions and session continuity on iOS and Android; keep unverified recording backends explicit as residual risk.

No readiness label yet.

…e-provenance corpus witnesses

- readFieldInput silently dropped undeclared keys before the daemon writers
  could refuse them, so an MCP call carrying { maxSize } reached transport and
  returned native-size success. New retiredField() combinator declares the
  removed key in the field map: the projection seam refuses it with the
  canonical migration message and the JSON schema no longer advertises it.
  Real-route MCP executor regressions cover screenshot and record.
- replay-compat corpus: derived v0.20.5 witnesses for the released screenshot
  and record --max-size forms (SHA-256 pinned, new retired-capture-size
  coverage surface) so check:replay-compat proves the shipped syntax refuses
  with migration guidance instead of degrading silently.
@thymikee

thymikee commented Aug 6, 2026

Copy link
Copy Markdown
Member

All three items addressed at 4cf4eb4a8.

P1 — MCP retired input now refuses at the projection seam

You were right that my earlier additionalProperties: false claim was schema-advertised but unenforced in-process. Fixed at the exact seam where the drop happened: a new retiredField() combinator in command-input.ts declares the removed key in the field map, so readFieldInput refuses it with the canonical migration message before any client method or transport is reached, and fieldsInputSchema filters it so the tool schema still does not advertise it. Screenshot and record metadata each declare maxSize: retiredField(RETIRED_SCREENSHOT_MAX_SIZE.migration.<command>) exactly where the live field used to sit.

Regressions run the REAL route (no runCommand injection — createCommandToolExecutorcommand-surface.runCommand → field projection): executor.execute('screenshot', { maxSize: 1024 }) and execute('record', { action: 'start', maxSize: 720 }) both return isError: true with the migration text, plus a schema pin that neither tool advertises maxSize.

P2 — release-provenance corpus witnesses

Two derived entries under test/replay-compat/scripts/docs/ (new retired-capture-size coverage surface, added to REQUIRED_COVERAGE_KEYS so the type system keeps demanding a witness):

  • screenshot-max-size.v0.20.5.ad — cites packages/contracts/src/screenshot.ts@v0.20.5 (appendScreenshotScriptFlags emitted --max-size <px>); verdict fails INVALID_ARGS with the --scale migration hint.
  • record-max-size.v0.20.5.ad — cites packages/ad-script/src/internal/script-utils.ts@v0.20.5 (appendRecordActionScriptArgs emitted --max-size between --fps and --quality, reproduced in that recorded flag order); verdict fails INVALID_ARGS with the record migration hint.

Both SHA-256 pinned; v0.20.5 added to REPLAY_COMPAT_RELEASED_TAGS. pnpm check:replay-compat: "Verified 10 mined replay-compat scripts against their released blobs across 7 tags (14 derived entries are digest-pinned)." The script.test.ts copies remain as fast parser-seam witnesses and now point at the corpus as the provenance home.

Exact-head evidence (4cf4eb4a8, the pushed head)

iPhone 17 Pro sim (iOS 26.2), isolated state dir, this head's build:

step result
screenshot 402×874
screenshot --scale 0.3 121×262 (sips-verified on disk)
screenshot --scale 0.5 --overlay-refs 201×437
continuity click text=GeneralTapped text=General (201, 406); snapshot -i ok after recording
record start/stop --hide-touches playable MP4, native 1206×2622

Pixel_7_CI emulator:

step result
screenshot 1080×2400
screenshot --scale 0.3 324×720 (sips-verified)
continuity snapshot -i ok before and after recording; press mid-recording ok
record start/stop --hide-touches first two stops failed (pulled file is not a playable MP4); third attempt playable native 1080×2400, 8.7 s

The two Android stop failures are a pre-existing pull-before-finalize race, not this PR: after kill -2, screenrecord needs ~1–3 s to write the moov atom; record-trace-android-copy.ts pulls immediately and retries once after 750 ms, and its waitForStableFile runs on the local copy, which cannot detect an unfinalized remote file. Live proof on the same emulator, outside our code: pull immediately after kill → 3232 bytes, ffprobe "moov atom not found"; pull 3 s later → 65512 bytes, playable 1080×2400. The failing runs' own diagnostics show the signature (identical fileSize across both attempts). The copy path is byte-identical on main and untouched by this PR; filed separately for a remote-finalization wait. Residual risk unchanged: macOS runner and physical-iOS-device recording (recordStart without maxSize) remain protocol/provider-verified only.

Gates at this head: typecheck, lint, format, layering, 5470 unit tests, 42 provider files, corpus (55), check:replay-compat, fallow audit + production-exports — green locally; CI running on the push.

@thymikee

thymikee commented Aug 6, 2026

Copy link
Copy Markdown
Member

Re-reviewed exact head 4cf4eb4a. The prior code/evidence findings are substantively fixed: hidden retiredField declarations make MCP refuse removed maxSize inputs before projection/transport while keeping schemas clean; real executor-route tests cover screenshot and record; the v0.20.5 screenshot/record forms are now digest-pinned in the required replay-compat corpus; and the centralized PNG resize path has exact-head native-vs-scaled iOS/Android evidence plus session-continuity/default-recording coverage. The two initial Android MP4 failures match the separately identified pre-existing remote-finalization race, and the affected copy path is unchanged here.

This is still not merge-ready: the PR remains draft and GitHub reports CONFLICTING/DIRTY against current main, with no authoritative checks attached to exact head 4cf4eb4a. Rebase and resolve the current owners, run fresh CI, then request re-review of the conflict resolution. No readiness label yet.

* origin/main:
  ci: remove package smoke workflow (callstack#1624)
  fix(ios): pin tap-outcome corroboration probes to the baseline's backend (callstack#1634)
  refactor: sink backend.ts's cycle-closing types below both zones (callstack#1632) (callstack#1636)
  refactor(daemon): one interface for the deferred interaction outcome (callstack#1633)
  feat(ios): extend depth-capped private-AX captures via element-rooted requests (callstack#1627)

# Conflicts:
#	src/daemon/request-generic-dispatch.ts
@thymikee

thymikee commented Aug 6, 2026

Copy link
Copy Markdown
Member

Note: main moved again under the branch (#1632/#1634/#1624), which re-conflicted the PR and blocked CI from triggering; merged as d4b75dd4c (one import-block conflict in request-generic-dispatch.ts from #1633's markPostGestureStabilization removal — resolved to main's structure with the retired-flag guard imports kept). The delta vs the evidence head 4cf4eb4a8 is main's own commits plus that import wiring — no screenshot/recording surface changes — so the exact-head live evidence above carries over. Gates re-ran green after the merge (typecheck, lint, format, layering, full unit suite 5502, provider scenarios, replay-compat provenance, fallow).

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 6, 2026
@thymikee

thymikee commented Aug 6, 2026

Copy link
Copy Markdown
Member

Re-reviewed exact head d4b75dd. The merge resolution adopts main’s markDeferredInteractionOutcome path; screenshot/record scaling and retired-maxSize behavior are unchanged from reviewed head 4cf4eb4. The prior fixes remain valid: fail-closed MCP projection, replay-compat witnesses, centralized resize, and exact-head iOS/Android dimension plus session-continuity evidence. GitHub reports the branch clean and all substantive checks, including iOS/macOS/Android smoke, integration, coverage, replay-compat, and static gates, are green. No code findings; ready for human review. The PR is still draft, and the Validation section should be updated to summarize the later live evidence currently captured only in comments before merge.

@thymikee
thymikee marked this pull request as ready for review August 6, 2026 13:10
@thymikee
thymikee merged commit 3e4828d into callstack:main Aug 6, 2026
27 checks passed
gwdp added a commit to expo/skills that referenced this pull request Aug 14, 2026
…137)

## Why

agent-device [PR
#1617](callstack/agent-device#1617) adds a
scale-only `screenshot --scale` knob and removes `--max-size`. Our
controller reference told agents the opposite: that agent-device
screenshots are "full-resolution with no scale knob."

## How

- Rewrote the agent-device screenshot note in
`references/controllers.md`:
- `screenshot --scale <0.01–1>` resizes both dimensions, `1` = full
resolution.
- Token-conscious default via `AGENT_DEVICE_SCREENSHOT_SCALE` /
`screenshotScale` in config, overridden by an explicit `--scale`.
- `--max-size` is removed and older calls are refused with migration
guidance.
- Added a remote-session caveat: the resize runs on the daemon, so a
newer client against an older EAS session daemon can have `--scale`
silently ignored.
- Gated the whole thing on `v0.20.6` as a single token, easy to bump if
it ships as a different version.
- Bumped the plugin version to 1.9.7.

## Test Plan

- Cross-check every claim against agent-device#1617's own doc changes:
flag name, the `0.01–1` range, the env var / config key, and the
`--max-size` removal
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants