feat: support --settle on scroll and back (generic-route settle observation) - #1653
Closed
thymikee wants to merge 1 commit into
Closed
feat: support --settle on scroll and back (generic-route settle observation)#1653thymikee wants to merge 1 commit into
thymikee wants to merge 1 commit into
Conversation
…vation) Scroll-then-observe and back-then-observe are legitimate combos that the post-action observation registry never grew past the touch commands, so AppControlBench models kept burning a tool call on an INVALID_ARGS refusal. Registry: POST_ACTION_OBSERVATION_BY_COMMAND gains `scroll: 'settle'` and `back: 'settle'` — settle without verify, because neither resolves a target and there is no pre-action node to digest into evidence. Every downstream seam already derives from that map: CLI flags, MCP/SDK input fields, the daemon flag guard, and now the timeout policy (a new postActionObservationTimeoutPolicy keeps the map the enumerator, so granting a command --settle cannot leave its envelope too narrow for the wait it accepts). Engine: settleAfterInteraction is split into a target-less settleAfterAction plus a thin adapter, and exposed as a runtime command (`runtime.interactions.observeSettle`) so the generic route reaches settle through the same runtime seam every other settle-carrying command uses. Wiring: request-generic-dispatch reads the pre-action tree before dispatch and, after markDeferredInteractionOutcome (so settle's first capture folds the #1542 stabilization) and after Android's blocking-dialog postflight, attaches the observation as `data.settle` with the ADR 0014 refs-generation dance. The whole path sits behind a lazy import, like every daemon route: home/screenshot/orientation must not pay the interaction runtime's import cost, and the static edge would have folded that cluster into the daemon's largest type cycle. The diff baseline is the STORED pre-action snapshot, which can be several commands older than the action. That is the honest #1101 contract ("settled diff vs the pre-action tree") but is weaker than press's freshly-resolved baseline — a `back` right after an unsnapshotted `press` diffs root-to-root. Also extracts the shared settle CLI rendering so scroll/back print the same verdict/diff/tail block as the touch commands, and lifts the ADR 0014 settle ref-issuance rule into one function shared by both routes. Closes #1638
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
thymikee
added a commit
that referenced
this pull request
Aug 6, 2026
Two sessions independently built #1638 (PR #1650 and PR #1653) and converged on the same architecture — trait in the registry, one engine with two entry points, runtime-command seam, lazy import, preserve-daemon, stored-baseline honesty. #1650 continues; this folds in what #1653 did better: - The agent-facing help core loop (cli-help.ts) now names scroll and back as settle-capable. Without this, the benchmarked closed-grammar help line kept instructing agents that --settle is only for press/click/fill/longpress — actively steering the AppControlBench models away from what #1638 shipped. - issueSettleRefs moves into session-snapshot.ts, beside the partial-frame primitive it wraps, deleting the single-function settle-ref-issuance module. - Their seam tests: back reader→writer settle plumbing, back CLI settle rendering, and a trait-less generic command (home) ignoring a stray settle flag rather than observing or rejecting. What #1650 had that #1653 lacked, for the record: the SETTLE_REF_ISSUING_TOOLS registry derivation (without it, MCP never pins a scroll/back settle diff's refs and the partial frame rejects every follow-up), BackCommandResult.settle in contracts, back's MCP output schema, paste-ready pinned diff refs, and the docs/changelog/baseline surfaces.
Member
Author
|
Closing in favor of #1650 — both PRs implemented #1638 in parallel (coordination miss), and both sessions' independent assessments reached the same verdict. The two implementations converged on an identical architecture: Decided by one functional gap: this branch never touched What this PR did better is now IN #1650 (4cf36bc):
The follow-up this PR proposed (plain |
Member
Author
|
Reviewed exact head
Android blocking-dialog ordering remains fixture-only evidence. No readiness label. |
|
thymikee
added a commit
that referenced
this pull request
Aug 6, 2026
Two sessions independently built #1638 (PR #1650 and PR #1653) and converged on the same architecture — trait in the registry, one engine with two entry points, runtime-command seam, lazy import, preserve-daemon, stored-baseline honesty. #1650 continues; this folds in what #1653 did better: - The agent-facing help core loop (cli-help.ts) now names scroll and back as settle-capable. Without this, the benchmarked closed-grammar help line kept instructing agents that --settle is only for press/click/fill/longpress — actively steering the AppControlBench models away from what #1638 shipped. - issueSettleRefs moves into session-snapshot.ts, beside the partial-frame primitive it wraps, deleting the single-function settle-ref-issuance module. - Their seam tests: back reader→writer settle plumbing, back CLI settle rendering, and a trait-less generic command (home) ignoring a stray settle flag rather than observing or rejecting. What #1650 had that #1653 lacked, for the record: the SETTLE_REF_ISSUING_TOOLS registry derivation (without it, MCP never pins a scroll/back settle diff's refs and the partial frame rejects every follow-up), BackCommandResult.settle in contracts, back's MCP output schema, paste-ready pinned diff refs, and the docs/changelog/baseline surfaces.
thymikee
added a commit
that referenced
this pull request
Aug 6, 2026
* feat: support --settle on scroll and back (#1638) Scroll-then-observe and back-then-observe are legitimate agent pairs, but the post-action observation registry never grew past the touch commands, so `--settle` on either was rejected with INVALID_ARGS — burning a tool call each in AppControlBench's bsky-16. Both commands now carry the `settle` descriptor trait, and every surface derives from it rather than a hand list: CLI allowed flags, MCP/SDK input fields, the flag-sourced timeout envelope, and MCP ref-pinning. The CLI flag/metadata helpers moved out of the interaction family into post-action-observation-grammar.ts (back is a system command), and SETTLE_REF_ISSUING_TOOLS became a derivation — a hand list would have silently stopped pinning the new commands' refs. settleAfterInteraction and the new settleObservationCommand are two entry points over one engine: same loop, storage, hints, and diff bounds, with the target-less path supplying its own baseline and no proximity point. The daemon reaches that command through the runtime surface, never by importing `commands/` (R2) — the same seam the touch handlers use for press/fill — and generic-settle.ts is loaded through a lazy `await import` returning a closure, so the interaction runtime subgraph stays out of this dispatcher's static graph (a static edge folded ~18 files into the daemon-server type cycle; R10 caught it). Both of generic-settle's orderings are load-bearing and tested: the baseline is frozen before dispatch (and before the Android dialog preflight), and the observation runs after markDeferredInteractionOutcome so settle's first capture folds in the #1542 stabilization rather than racing it. The ADR 0014 "a settled diff publishes refs" rule moved to settle-ref-issuance.ts, shared by both routes. One divergence is deliberate: scroll/back resolve no element, so the diff baseline is the session's STORED pre-action tree — "settled tree vs the last tree you observed" — not press's freshly resolved pre-action capture. Both commands also switch to preserve-daemon on timeout, which changes the non-settle path too: with --settle their dominant hang mode is now a wedged accessibility bridge, and a timed-out capture must not reset the daemon and lose every session (#1105). The reviewed-set gate records it. Live-validated on an iOS 26.2 simulator (Settings): scroll --settle settled in 1786ms with a +6/-6 diff carrying fresh refs; back --settle in 771ms with +15/-6. Alternating cost runs, one call vs the pair it replaces: scroll 2.9-3.0s vs 5.3-5.6s, back 3.1-3.2s vs 4.7-5.1s. Those include the #1627 deep-capture extension. * fix: render settled-diff refs paste-ready in CLI output A settled diff activates a PARTIAL ref frame (ADR 0014), which admits only the pinned `@eN~s<gen>` form of the refs it issued. The unchanged-interactive tail already rendered that way, but the diff's own added lines rendered the bare `@eN` embedded in the snapshot line — so a CLI caller who copied the ref the diff just handed them got `plain_ref_requires_complete_frame` and had to append the generation by hand. Added lines now render pinned when the response carries `refsGeneration`, exactly like the tail. Removed lines render verbatim: they name elements that just left the screen, and `SettleDiffLine` never gives them a ref. This is not new to scroll/back — press/click/fill/longpress had the same gap since #1101. MCP was never affected: its ref-pin store rewrites plain refs on the way in, which is why the model never sees a suffix. Live: `scroll down --settle` now emits `+ @E14~s218078 [cell] "Game Center"`, and `press @E14~s218078` copied straight out of that line taps successfully. * test: record the pinned-diff-ref bytes in the output-economy baseline Rendering added diff-line refs pinned costs 8 bytes in the two settle CLI text samples (two `~s<gen>` suffixes). The output-economy baseline is the tripwire for exactly this, so the increase takes an explicit reviewed waiver rather than a silent baseline bump — the same one the settled TAIL's pins already carry, for the same ADR 0014 reason. Only `bytes` moves: lines, refs, hints, and shape are unchanged, which is the evidence that this is a suffix on existing refs and not a new payload. Caught by CI, not locally: `pnpm test:unit` runs unit-core and subprocess-stub only, while the Coverage lane runs every vitest project. * test: prove the generic settle degrades when its runtime cannot be built `createGenericSettleRuntime` catches and returns undefined so an observation that cannot even start does not fail an action that already succeeded. That was a claim in a docstring with nothing behind it — the one changed line the coverage gate reported uncovered (95/96). The test puts the session in the state the catch exists for: the router handed us a session that is no longer in the store, so building the settle runtime throws SESSION_NOT_FOUND. The response keeps its scroll result and simply carries no settle payload. Removing the try/catch fails it. * build: teach fallow that vi.mock reaches pinOwnProcessStartTime dynamically Not from this PR: #1642 added `pinOwnProcessStartTime` on main, and its three consumers reach it the only way a Vitest module mock can — `vi.mock(path, async (importOriginal) => (await import('...')).pinOwnProcessStartTime(...))`. Dependency analysis cannot follow that dynamic import to a consumer, so the export reads as dead the moment any PR pulls that file into its audit scope. This PR is the one that did. The entry records the consumers by path and the reason, matching the daemon route-handler entry directly above it, which exists for the same dynamic-`import()` limitation. * refactor: adopt the best of the parallel #1653 implementation Two sessions independently built #1638 (PR #1650 and PR #1653) and converged on the same architecture — trait in the registry, one engine with two entry points, runtime-command seam, lazy import, preserve-daemon, stored-baseline honesty. #1650 continues; this folds in what #1653 did better: - The agent-facing help core loop (cli-help.ts) now names scroll and back as settle-capable. Without this, the benchmarked closed-grammar help line kept instructing agents that --settle is only for press/click/fill/longpress — actively steering the AppControlBench models away from what #1638 shipped. - issueSettleRefs moves into session-snapshot.ts, beside the partial-frame primitive it wraps, deleting the single-function settle-ref-issuance module. - Their seam tests: back reader→writer settle plumbing, back CLI settle rendering, and a trait-less generic command (home) ignoring a stray settle flag rather than observing or rejecting. What #1650 had that #1653 lacked, for the record: the SETTLE_REF_ISSUING_TOOLS registry derivation (without it, MCP never pins a scroll/back settle diff's refs and the partial frame rejects every follow-up), BackCommandResult.settle in contracts, back's MCP output schema, paste-ready pinned diff refs, and the docs/changelog/baseline surfaces. * bench: help-conformance case for settled scroll-to-find planning The #1638 extension of the closed --settle grammar to scroll/back is the feature's entire payoff — collapsing scroll-then-observe into one call — and the closed command list is an enumerated N whose enumerator is this bench. The regex over the help text proves the sentence exists; this case checks whether a model plans differently because of it. One focused case, deliberately not coached: a pinned visible-first snapshot (rendered by formatSnapshotText, pinned by the sample-producers gate) whose wanted row is summarized off-screen with no ref anywhere in the output. The tempting pre-#1638 plan is `scroll` plus a separate `snapshot -i`; acceptance is the single settled call. Scoring was verified against eight plan shapes in both directions before recording. Model-backed record (claude-haiku-4-5, 3 trials, current help): 0/3 — but the decomposition is the finding. Settle eligibility GENERALIZED (3/3 trials put --settle on scroll unprompted; the mutation-suffix framing concern did not materialize) and the two-call habit is residual (1/3). All three trials failed on `scroll @e3 down --settle` — the pre-existing #1366 scroll-takes-no-target confusion, which the live CLI recovers with a dedicated hint but a single-shot bench cannot. The recorded gap is therefore a first-30 doc gap (nothing teaches that scroll takes no target), not a settle-eligibility gap; tuning the case until it passes would just delete the evidence.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1638.
scroll --settleandback --settlewere both rejected withINVALID_ARGS, so AppControlBench models burned a tool call each learning that. Scroll-then-observe and back-then-observe are legitimate combos; the post-action observation registry just never grew past the touch commands.What changed
Registry —
POST_ACTION_OBSERVATION_BY_COMMANDgainsscroll: 'settle'andback: 'settle'. Settle without verify: neither command resolves a target, so there is no pre-action node to digest into--verifyevidence. Every downstream seam already derives from that map (CLI flags, MCP/SDK input fields, daemon flag guard), and the timeout policy now does too — a newpostActionObservationTimeoutPolicykeeps the map the enumerator, so granting a command--settlecan't leave its request envelope too narrow for the settle wait it now accepts.Engine —
settleAfterInteractionsplits into a target-lesssettleAfterAction(caller-supplied diff baseline, optional proximity point) plus a thin adapter that maps a resolved target onto it. The target-less form is exposed as a runtime command,runtime.interactions.observeSettle, so the generic route reaches settle through the same runtime seam every other settle-carrying command uses rather than reaching into the engine.Wiring —
request-generic-dispatchreads the pre-action tree before dispatch and, aftermarkDeferredInteractionOutcomeand after Android's blocking-dialog postflight, attaches the observation asdata.settlewith the ADR 0014 refs-generation dance.--settle-quietwithout--settleis refused before any device work.Rendering — the settle verdict/diff/tail block is extracted into
src/commands/settle-output.ts, soscrollandbackprint exactly what the touch commands print.Dedup — the ADR 0014 settle ref-issuance rule (which refs a settled diff published, at which generation) now lives in one function,
issueSettleObservationRefs, shared by the touch and generic routes instead of a private copy ininteraction-touch.ts.Design notes worth reviewing
The diff baseline is the stored pre-action snapshot. Press diffs against the tree it just resolved its target on; the generic route has no such capture, so it diffs against
session.snapshot, which can be several commands older than the action. That is the honest reading of the #1101 contract ("the settled diff vs the pre-action tree"), but it is a weaker baseline — see the live evidence below for what that looks like in practice.Ordering is load-bearing. Settle runs after
markDeferredInteractionOutcomeso its first capture is the one that folds the #1542 post-gesture stabilization, and after Android's blocking-dialog postflight so a recovered dialog isn't what the settled diff describes. There is a revert-sensitive test for the first half (the settle capture runs after the post-gesture stabilization marker is placedfails if the call moves up).The settle path is behind a lazy import, like every daemon route. Observing pulls in the whole interaction runtime;
home/screenshot/orientation/tv-remoteshouldn't pay that in import cost, and the static edge grew the daemon's largest type cycle from 47 to 65 files (R9/R10 caught it). Behindimport()the cycle is unchanged at 47.back's MCP output schema does not declaresettle. Documenting it would require the settled-observation schema to live in a module that bothcommands/system/navigation-projection.tsandmcp/can import, and any such module sits on a member-to-member path in the type cycle (+1 node, permanently). These schemas are non-strict by design precisely so an additive opt-in field validates without every projection re-declaring it — the same reasoncostisn't declared everywhere.scrollhas no output schema at all (not inCommandResultMap), so declaring it forbackalone would have bought partial coverage for a permanent ratchet bump.back/scrollmoved toonTimeout: 'preserve-daemon'. The policy can't be flag-conditional, and--settlegives them the same wedged-capture hang mode that put the touch commands there in #1105. Preserving is the safe side: a reset loses every session on the daemon.Live validation
iOS 26.2 simulator, Settings (
com.apple.Preferences), this branch's built CLI:scroll down --settlescroll up --settleback --settle(real navigation diff)scroll down(no settle)scroll down --settlereturned exactly the rows that scrolled into view:back --settleafter a fresh in-page snapshot returned a genuine navigation diff (+5 -16 (~14 unchanged), General's contents removed and the Settings root added).back --settlewithout one returned+0 -0 (~19 unchanged)plus the unchanged-interactive tail — the stored-baseline caveat above, visible in practice.Ref issuance verified end to end: the settled diff carried
refsGeneration: 223763, a plain@e9was correctly refused (the current frame only authorizes its emitted refs), and the pinned@e9~s223763resolved and tapped.--settle-quiet 200without--settlewas refused before dispatch.Settle cost here is on cheap screens; the #1627 deep-capture extension will make hostile screens more expensive per capture until #1626 lands, as the issue notes.
Tests
src/daemon/__tests__/generic-settle.test.ts(new, 8 tests): stored-baseline diff, ref issuance + partial frame + generation,back's payload surviving alongside the observation, the stabilization-ordering guard, no-settle taking zero captures, a trait-less generic command (home) ignoring a stray settle flag, orphaned--settle-quietrefused before dispatch, and a broken observation never failing the action.scroll(interaction) andback(system), plusbackreader→writer settle plumbing.pnpm check:toolingandpnpm test:unitpass (one unrelatedrunner-clienttimeout under contention; passes in isolation).