diff --git a/.fallowrc.json b/.fallowrc.json index 8918009194..61bf452910 100644 --- a/.fallowrc.json +++ b/.fallowrc.json @@ -78,6 +78,11 @@ "file": "src/sdk/*.ts", "exports": ["*"] }, + { + "comment": "#1642: its three consumers (src/__tests__/cli-device-status.test.ts, src/daemon/__tests__/device-claims.test.ts, src/daemon/__tests__/device-claim-prune.test.ts) all reach it the only way a Vitest module mock can — `vi.mock(path, async (importOriginal) => (await import('...host-process-mock.ts')).pinOwnProcessStartTime(importOriginal))`. Dependency analysis cannot follow that dynamic import to a consumer, the same limitation the daemon route-handler entry above records.", + "file": "src/__tests__/test-utils/host-process-mock.ts", + "exports": ["pinOwnProcessStartTime"] + }, { "comment": "Tool config default exports, loaded by the tool rather than imported.", "file": "{tsdown.config.ts,vitest.mutation.config.ts,website/rspress.config.ts}", diff --git a/CHANGELOG.md b/CHANGELOG.md index 052a1ff54b..92a0cc4f58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- `scroll` and `back` now accept `--settle` (with `--settle-quiet` and `--timeout`), collapsing scroll-then-observe and back-then-observe into one call (#1638). The response carries the same settled payload the touch commands return — verdict, changed-lines diff with fresh refs on added lines, the unchanged-interactive tail, and `refsGeneration` when the settled tree was stored — and is best-effort: it never fails the action. One difference is deliberate: `scroll`/`back` resolve no element, so the diff baseline is the session's stored pre-action tree ("the last tree you observed") rather than a freshly resolved pre-action capture. Both commands now also preserve the daemon on timeout, like the other settle-capable commands. - Security: repository `./agent-device.json` now accepts only project-safe automation defaults. It rejects daemon endpoint/auth/transport/server settings, tenant/run/lease selectors, provider/cloud and Metro connection fields, headers, executable reporter modules, local write destinations, and other operator-controlled values before local module loading or any daemon health/RPC request. Put remote endpoint and token together in protected CI environment variables, user config, an explicit `--config` file, or the existing `connect`/`--remote-config` workflow. Daemon auth tokens no longer travel in serialized command flags. - `viewport` is now rejected during capability admission on Apple targets instead of reaching the device and failing inside dispatch. No Apple backend can resize a screen — simulator and device geometry is fixed by the selected device type — so `viewport` on iOS/iPadOS/tvOS/macOS now fails with `UNSUPPORTED_OPERATION`, `viewport is not supported on this device`, and a hint pointing at `--platform web` and at picking a different simulator. `capabilities` no longer advertises `viewport` on Apple targets. Web viewport resizing (`agent-device viewport 1280 900 --platform web`) is unchanged, and Android was already denied. - `--save-script` is now accepted only by the commands that declare it — `open`, `close`, and `replay`. A hand-built daemon request (or a `batch` step) that set `saveScript` on any other command, such as `record` or `trace`, used to arm script publication and could write a `.ad` artifact; it is now rejected with `INVALID_ARGS` before the request reaches admission, the device, or any handler. CLI, Node, and MCP usage of `--save-script` on its documented commands is unchanged. diff --git a/CONTEXT.md b/CONTEXT.md index 4b82098fc9..3e6775ac2a 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -131,11 +131,16 @@ task touches: eligibility gate, and the module owns only these post-action markers — never ADR 0014 ref-frame expiry or the ADR 0012/0016 staged protocols. Distinct from the same-response settled observation below. -- Settled observation: opt-in (`--settle`) post-action payload on press/click/fill/longpress — the - quiet-window stable loop re-captures until the UI settles, and the response carries the diff vs the - pre-action tree (changed lines only, added lines with fresh refs, `refsGeneration` when the settled - tree was stored). Best-effort: never fails the action; `settled: false` plus a hint on never-quiet - content. +- Settled observation: opt-in (`--settle`) post-action payload on press/click/fill/longpress and, on + the generic route, scroll/back — the quiet-window stable loop re-captures until the UI settles, and + the response carries the diff vs the pre-action tree (changed lines only, added lines with fresh + refs, `refsGeneration` when the settled tree was stored). Best-effort: never fails the action; + `settled: false` plus a hint on never-quiet content. Which commands support it is a descriptor + trait (`postActionObservation`), and the CLI flags, MCP fields, timeout envelope, and ref-pinning + all derive from it. The two routes differ in ONE way, deliberately: the touch commands diff against + the freshly resolved pre-action capture, while scroll/back — which resolve nothing — diff against + the session's stored pre-action tree, so their diff reads "settled tree vs the last tree you + observed". - Resolution disclosure (ADR 0012 decision 2): additive `resolution` field on press/click/fill/longpress responses discloses how the acting path resolved its target — `runtime`/`unique` or `runtime`/`disambiguated` (with `matchCount`/`winnerDiagnostic`/`tiebreak`/ diff --git a/README.md b/README.md index e901291c1b..fcfadcf0f3 100644 --- a/README.md +++ b/README.md @@ -61,6 +61,8 @@ agent-device close Use refs only from the latest output. Do not assume an earlier `@eN` still identifies the same element. After a command with `--settle`, use the refs in its diff. Take another snapshot only if the diff omits what you need. +`--settle` works the same way on `scroll` and `back`, so scroll-then-observe and back-then-observe are one call too. + Snapshots use the app's accessibility tree. Clear labels, roles, and test IDs make agent runs more reliable. Use screenshots and videos as evidence or when accessibility data is poor. Use refs and selectors for actions and assertions when you can. ![agent-device demo showing Codex using agent-device to create a new contact in the iOS Contacts app from a simple prompt](./website/docs/public/agent-device-contacts.gif) diff --git a/packages/contracts/src/client-gesture.ts b/packages/contracts/src/client-gesture.ts index 823f318e4f..faa3d5ca35 100644 --- a/packages/contracts/src/client-gesture.ts +++ b/packages/contracts/src/client-gesture.ts @@ -135,9 +135,10 @@ export type RotateGestureOptions = DeviceCommandBaseOptions & { export type TransformGestureOptions = DeviceCommandBaseOptions & TransformGestureParams; -export type ScrollOptions = DeviceCommandBaseOptions & { - direction: ScrollInputDirection; - amount?: number; - pixels?: number; - durationMs?: number; -}; +export type ScrollOptions = DeviceCommandBaseOptions & + SettleCommandOptions & { + direction: ScrollInputDirection; + amount?: number; + pixels?: number; + durationMs?: number; + }; diff --git a/packages/contracts/src/navigation.ts b/packages/contracts/src/navigation.ts index 4f3e37b51a..90f916cd87 100644 --- a/packages/contracts/src/navigation.ts +++ b/packages/contracts/src/navigation.ts @@ -1,5 +1,6 @@ import type { BackMode } from './back-mode.ts'; import type { DeviceRotation } from './device-rotation.ts'; +import type { SettleObservation } from './interaction.ts'; import type { TvRemoteButton } from './tv-remote.ts'; /** @@ -18,11 +19,17 @@ export type HomeCommandResult = { message: string; }; -/** `back` — `{ action: 'back', mode, message: 'Back' }`; `mode` defaults to `'in-app'`. */ +/** + * `back` — `{ action: 'back', mode, message: 'Back' }`; `mode` defaults to + * `'in-app'`. The one field the generic route may add on top of the dispatch + * handler's literal return: `settle`, the opt-in `--settle` observation + * (#1638), attached after the command by the generic dispatcher. + */ export type BackCommandResult = { action: 'back'; mode: BackMode; message: string; + settle?: SettleObservation; }; /** `orientation` — `{ action: 'orientation', orientation, message: 'Rotated to ' }`. */ diff --git a/scripts/__tests__/help-conformance-sample-producers.ts b/scripts/__tests__/help-conformance-sample-producers.ts index 1a4eb9cfde..8e9a86a8c5 100644 --- a/scripts/__tests__/help-conformance-sample-producers.ts +++ b/scripts/__tests__/help-conformance-sample-producers.ts @@ -5,6 +5,7 @@ import { BROWSERSTACK_CONNECT_SAMPLE, DEVICE_IN_USE_SAMPLE, NOT_SETTLED_SAMPLE, + OFFSCREEN_TARGET_SNAPSHOT_SAMPLE, PRIVATE_AX_RECOVERY_SAMPLE, SETTLE_DIFF_SAMPLE, SETTLE_DIFF_SAMPLE_NOTES, @@ -208,6 +209,64 @@ export const SAMPLE_PRODUCERS: SampleProducer[] = [ }).trimEnd(); }, }, + { + name: 'OFFSCREEN_TARGET_SNAPSHOT_SAMPLE', + producer: 'the visible-first snapshot renderer with off-screen rows summarized', + sample: OFFSCREEN_TARGET_SNAPSHOT_SAMPLE, + render: () => { + // A settings-style scrollable list in an 800pt viewport: five rows fit, + // four more (Privacy & Security, Notifications, Wallpaper, Developer) are + // laid out below it, so visible-first presentation summarizes them and + // none of their refs reach the output. + const row = (index: number, ref: string, label: string, y: number) => ({ + index, + ref, + parentIndex: 2, + type: 'Cell', + label, + interactive: true, + hittable: y < 800, + rect: { x: 0, y, width: 390, height: 120 }, + }); + const nodes = [ + { + index: 0, + ref: 'e1', + type: 'Application', + label: 'Preferences', + rect: { x: 0, y: 0, width: 390, height: 800 }, + }, + { + index: 1, + ref: 'e2', + parentIndex: 0, + type: 'Window', + rect: { x: 0, y: 0, width: 390, height: 800 }, + }, + { + index: 2, + ref: 'e3', + parentIndex: 1, + type: 'CollectionView', + interactive: true, + rect: { x: 0, y: 60, width: 390, height: 740 }, + }, + row(3, 'e4', 'General', 60), + row(4, 'e5', 'Display', 190), + row(5, 'e6', 'Sounds', 320), + row(6, 'e7', 'Focus', 450), + row(7, 'e8', 'Screen Time', 580), + row(8, 'e9', 'Privacy & Security', 900), + row(9, 'e10', 'Notifications', 1030), + row(10, 'e11', 'Wallpaper', 1160), + row(11, 'e12', 'Developer', 1290), + ]; + return formatSnapshotText( + { nodes, backend: 'xctest', truncated: false }, + { interactiveOnly: true }, + ).trimEnd(); + }, + }, { name: 'DEVICE_IN_USE_SAMPLE', producer: 'the real session-open by-session conflict producer', diff --git a/scripts/help-conformance-cases.mjs b/scripts/help-conformance-cases.mjs index 563ed06209..456ff0c8b5 100644 --- a/scripts/help-conformance-cases.mjs +++ b/scripts/help-conformance-cases.mjs @@ -4,6 +4,7 @@ import { BROWSERSTACK_CONNECT_SAMPLE, DEVICE_IN_USE_SAMPLE, NOT_SETTLED_SAMPLE, + OFFSCREEN_TARGET_SNAPSHOT_SAMPLE, SETTLE_DIFF_SAMPLE, SETTLE_DIFF_SAMPLE_NOTES, SETTLE_TAIL_SAMPLE, @@ -377,6 +378,46 @@ Use the output already shown to determine whether the feed-search UI is present, { id: 'noRawCoordinateTarget', pattern: RAW_COORDINATE_TARGET }, ], }, + { + // #1638/#1650: the closed --settle grammar grew scroll and back, and this + // extension IS the feature's payoff — collapsing scroll-then-observe into + // one call. The old guidance framed settle as a mutation suffix, and + // scroll reads as navigation, so eligibility generalizing is exactly what + // this case checks. The task deliberately does not mention settle: the + // wanted row is off-screen with no ref anywhere in the output, the + // tempting pre-#1638 plan is `scroll` + a separate `snapshot -i`, and + // acceptance is the single settled call. + id: 'sample-output-offscreen-target-scrolls-settled', + docs: ['--help:first30'], + task: quiz( + OFFSCREEN_TARGET_SNAPSHOT_SAMPLE, + 'The task is to open the Notifications row of this list. What command should run next?', + ), + expectations: ['validPlanCommands', 'fullPrefix'], + matchers: [ + { + id: 'scrollsDownSettled', + pattern: /(?:^|\n)(?:agent-device\s+)?scroll\s+down\b[^\n]*--settle\b/i, + }, + ], + forbidden: [ + // The two-call habit this case exists to catch: a scroll line without + // --settle means a separate observation call is coming. + { + id: 'noUnsettledScroll', + pattern: /(?:^|\n)(?:agent-device\s+)?scroll\b(?:(?!--settle)[^\n])*(?=\n|$)/i, + }, + { id: 'noSnapshot', pattern: /\bsnapshot\b/i }, + { id: 'noWaitStable', pattern: /wait\s+stable/i }, + // Notifications never appears in the output, so any bare @eN press is a + // guessed ref, not a resolved target. + { + id: 'noGuessedRef', + pattern: /(?:^|\n)(?:agent-device\s+)?(?:press|click|fill|longpress)\s+@e\d/i, + }, + { id: 'noRawCoordinateTarget', pattern: RAW_COORDINATE_TARGET }, + ], + }, { id: 'sample-output-not-settled-needs-observe', docs: ['--help:first30'], diff --git a/scripts/help-conformance-sample-outputs.mjs b/scripts/help-conformance-sample-outputs.mjs index b431214801..9b03f0b69d 100644 --- a/scripts/help-conformance-sample-outputs.mjs +++ b/scripts/help-conformance-sample-outputs.mjs @@ -44,6 +44,24 @@ settled after 480ms: +2 -0 (~11 unchanged) + @e22 [text] "3 items"`, }; +// Visible-first snapshot of a scrollable list whose remaining rows sit below +// the viewport: the off-screen content is summarized, not listed as refs. The +// scroll-to-find quiz case hangs off this — the wanted row exists but no ref +// for it appears anywhere in the output. +export const OFFSCREEN_TARGET_SNAPSHOT_SAMPLE = { + command: 'agent-device snapshot -i', + output: `Snapshot: 8 visible nodes (12 total) +@e1 [application] "Preferences" +@e2 [window] +@e3 [collection] +@e4 [cell] "General" +@e5 [cell] "Display" +@e6 [cell] "Sounds" +@e7 [cell] "Focus" +@e8 [cell] "Screen Time" + [content below collection hidden]`, +}; + // Never-settled press: success response, no diff, NEVER_SETTLED_HINT attached. export const NOT_SETTLED_SAMPLE = { command: 'agent-device press @e12 --settle', diff --git a/src/cli/parser/__tests__/cli-help-topics.test.ts b/src/cli/parser/__tests__/cli-help-topics.test.ts index 74ffaebb8d..79edd59163 100644 --- a/src/cli/parser/__tests__/cli-help-topics.test.ts +++ b/src/cli/parser/__tests__/cli-help-topics.test.ts @@ -91,7 +91,7 @@ test('usage includes agent workflows, config, environment, and examples footers' assert.match(usageText, /Default app loop: agent-device open /); assert.match( usageText, - /Use --settle only on planned press, click, fill, or longpress commands; never add it to open, snapshot, or close/, + /Use --settle only on planned press, click, fill, longpress, scroll, or back commands; never add it to open, snapshot, or close/, ); assert.match(usageText, /type never accepts --settle/); assert.match(usageText, /explicit success confirmation is visible, stop/); diff --git a/src/cli/parser/cli-help.ts b/src/cli/parser/cli-help.ts index cecf158a2c..ed24aebfc2 100644 --- a/src/cli/parser/cli-help.ts +++ b/src/cli/parser/cli-help.ts @@ -79,7 +79,7 @@ const AGENT_START_LINES = [ // Haiku from 0/2 baseline to 4/4; generic structured-hint recovery passed 8/8 // uncoached output cases versus 7/8 with the longer special-case prose. 'Default app loop: agent-device open -> agent-device snapshot -i -> mutate a current target with --settle -> continue from that settled diff -> agent-device close.', - 'Use --settle only on planned press, click, fill, or longpress commands; never add it to open, snapshot, or close. type never accepts --settle: run agent-device type "text", then diff snapshot if verification is needed. Once the task\'s requested end state or an explicit success confirmation is visible, stop; do not tap transient follow-up controls or navigate away only to re-verify.', + 'Use --settle only on planned press, click, fill, longpress, scroll, or back commands; never add it to open, snapshot, or close. type never accepts --settle: run agent-device type "text", then diff snapshot if verification is needed. Once the task\'s requested end state or an explicit success confirmation is visible, stop; do not tap transient follow-up controls or navigate away only to re-verify.', 'Follow structured command hints before choosing a recovery action.', 'Targets are concrete refs or selectors: @e12, label="Query", role=button label="Submit".', 'Selector keys are only: id, role, text, label, value, appname, windowtitle, visible, hidden, editable, selected, focused, enabled, hittable. placeholder, index, and key are not selector keys.', diff --git a/src/commands/interaction/index.ts b/src/commands/interaction/index.ts index 80b91799b2..3e54f65e83 100644 --- a/src/commands/interaction/index.ts +++ b/src/commands/interaction/index.ts @@ -19,17 +19,9 @@ import type { TypeTextOptions, } from '@agent-device/contracts/client'; import type { CommandSchemaOverride } from '../../cli-schema/types.ts'; -import { - commandSupportsSettleObservation, - commandSupportsVerifyEvidence, -} from '../../core/command-descriptor/registry.ts'; -import { - REPEATED_TOUCH_FLAGS, - SELECTOR_SNAPSHOT_FLAGS, - SETTLE_FLAGS, -} from '../cli-grammar/flag-groups.ts'; -import { type FlagKey } from '../cli-grammar/flag-types.ts'; +import { REPEATED_TOUCH_FLAGS, SELECTOR_SNAPSHOT_FLAGS } from '../cli-grammar/flag-groups.ts'; import { defineExecutableCommand } from '../command-contract.ts'; +import { postActionObservationCliFlags } from '../post-action-observation-grammar.ts'; import { commonToClientOptions, toClientElementTarget, @@ -137,20 +129,15 @@ const interactionCliSchemas = { ], }, scroll: { - usageOverride: 'scroll [amount] [--pixels ] [--duration-ms ]', + usageOverride: + 'scroll [amount] [--pixels ] [--duration-ms ] [--settle]', positionalArgs: ['directionOrEdge', 'amount?'], - allowedFlags: ['pixels', 'durationMs'], + allowedFlags: ['pixels', 'durationMs', ...postActionObservationCliFlags('scroll')], }, } as const satisfies Record; type InteractionCommandMetadata = (typeof interactionCommandMetadata)[number]; type InteractionCommandName = InteractionCommandMetadata['name']; -function postActionObservationCliFlags(command: InteractionCommandName): readonly FlagKey[] { - const flags: FlagKey[] = []; - if (commandSupportsVerifyEvidence(command)) flags.push('verify'); - if (commandSupportsSettleObservation(command)) flags.push(...SETTLE_FLAGS); - return flags; -} const clickCommandDefinition = defineExecutableCommand(metadata('click'), (client, input) => client.interactions.click(toClickOptions(input)), @@ -318,6 +305,7 @@ const scrollCommandFacet = defineCommandFacet({ cliSchema: interactionCliSchemas.scroll, cliReader: interactionCliReaders.scroll, daemonWriter: interactionDaemonWriters.scroll, + cliOutputFormatter: interactionCliOutputFormatters.scroll, }); const getCommandFacet = defineCommandFacet({ diff --git a/src/commands/interaction/interactions.ts b/src/commands/interaction/interactions.ts index eaf41ba68a..110eb85fff 100644 --- a/src/commands/interaction/interactions.ts +++ b/src/commands/interaction/interactions.ts @@ -97,6 +97,7 @@ export const interactionCliReaders = { }, scroll: (positionals, flags) => ({ ...commonInputFromFlags(flags), + ...settleInputFromFlags(flags), direction: readScrollDirection(positionals[0]), amount: optionalCliNumber(positionals[1]), pixels: flags.pixels, diff --git a/src/commands/interaction/metadata.ts b/src/commands/interaction/metadata.ts index b80746fdd9..d732f3025e 100644 --- a/src/commands/interaction/metadata.ts +++ b/src/commands/interaction/metadata.ts @@ -16,11 +16,6 @@ import { type SwipeGesturePayload, type TransformGesturePayload, } from '@agent-device/contracts/interaction'; -import type { PostActionObservationSupportFor } from '../../core/command-descriptor/post-action-observation.ts'; -import { - commandSupportsSettleObservation, - commandSupportsVerifyEvidence, -} from '../../core/command-descriptor/registry.ts'; import { FIND_LOCATORS } from '@agent-device/selectors'; import { defineCommandMetadata } from '../command-contract.ts'; import { @@ -44,6 +39,7 @@ import { type InferCommandInput, } from '../command-input.ts'; import { defineFieldCommandMetadata } from '../field-command-contract.ts'; +import { postActionObservationFields } from '../post-action-observation-grammar.ts'; import { SCROLL_INPUT_DIRECTIONS } from './runtime/gestures.ts'; const FIND_ACTION_VALUES = [ @@ -80,37 +76,6 @@ const interactionCommandDescriptions = { type InteractionCommandName = keyof typeof interactionCommandDescriptions; -const verifyField = () => - booleanField( - 'Capture cheap post-action evidence (AX digest, node counts, changedFromBefore) instead of a follow-up snapshot.', - ); - -const settleFields = () => ({ - settle: booleanField( - 'After the action, wait for the UI to go quiet and return the settled diff vs the pre-action tree in the same response. Best-effort; never fails the action.', - ), - settleQuietMs: integerField('Settle: quiet window in milliseconds (default 500).', { min: 0 }), - timeoutMs: integerField('Settle: wait deadline in milliseconds (default 10000).', { min: 1 }), -}); - -type VerifyFieldMap = { verify: ReturnType }; -type SettleFieldMap = ReturnType; -type PostActionObservationFields = - PostActionObservationSupportFor extends 'settle-and-verify' - ? VerifyFieldMap & SettleFieldMap - : PostActionObservationSupportFor extends 'settle' - ? SettleFieldMap - : {}; - -function postActionObservationFields( - command: TName, -): PostActionObservationFields { - return { - ...(commandSupportsVerifyEvidence(command) ? { verify: verifyField() } : {}), - ...(commandSupportsSettleObservation(command) ? settleFields() : {}), - } as PostActionObservationFields; -} - const clickFields = { target: requiredField(interactionTargetField()), button: enumField(CLICK_BUTTONS, 'Pointer button for platforms that support mouse buttons.'), @@ -170,6 +135,7 @@ const scrollFields = { min: 0, max: SCROLL_DURATION_MAX_MS, }), + ...postActionObservationFields('scroll'), }; // #1271 stage 2 (ADR 0012 amendment): `get`/`is`/`find` are observation-only, diff --git a/src/commands/interaction/output.test.ts b/src/commands/interaction/output.test.ts index 4c9ad2ef29..1acbab2eef 100644 --- a/src/commands/interaction/output.test.ts +++ b/src/commands/interaction/output.test.ts @@ -109,6 +109,40 @@ describe('press CLI output', () => { ); }); + // ADR 0014: a settled diff activates a PARTIAL frame, so only the pinned form + // of the refs it issued is admitted. The diff has to hand the CLI caller that + // form directly or a copied `@e9` bounces with plain_ref_requires_complete_frame. + test('renders added-line refs pinned at the settle generation', () => { + const output = formatPress({ + message: 'Tapped (278, 817)', + x: 278, + y: 817, + settle: { + settled: true, + waitedMs: 1200, + refsGeneration: 41, + diff: { + summary: { additions: 1, removals: 1, unchanged: 8 }, + lines: [ + { kind: 'removed', text: '@e4 [button] "Search"' }, + { kind: 'added', text: '@e9 [text] "Notifications"', ref: 'e9' }, + ], + }, + }, + }); + + expect(output.text).toBe( + [ + 'Tapped (278, 817)', + 'settled after 1200ms: +1 -1 (~8 unchanged)', + // A removed line names an element that just left: rendered verbatim, + // never as a paste-ready target. + '- @e4 [button] "Search"', + '+ @e9~s41 [text] "Notifications"', + ].join('\n'), + ); + }); + test('prints the response warning after the tap line', () => { const output = formatPress({ message: 'Tapped (278, 817)', diff --git a/src/commands/interaction/output.ts b/src/commands/interaction/output.ts index 0ab316db8f..d8ec940954 100644 --- a/src/commands/interaction/output.ts +++ b/src/commands/interaction/output.ts @@ -3,7 +3,13 @@ import type { SnapshotNode } from '@agent-device/kernel/snapshot'; import type { CliOutput } from '../command-contract.ts'; import { displayLabel, formatRole } from '../../snapshot/snapshot-lines.ts'; import { readCommandMessage } from '../../utils/success-text.ts'; -import { messageCliOutput, resultOutput, type CliOutputFormatter } from '../output-common.ts'; +import { + messageCliOutput, + pinnedRefText, + resultOutput, + type CliOutputFormatter, +} from '../output-common.ts'; +import { appendResponseNotes, messageWithSettleOutput } from '../settle-output.ts'; function getCliOutput(params: { result: CommandRequestResult; format?: string }): CliOutput { const data = params.result as Record; @@ -16,17 +22,6 @@ function getCliOutput(params: { result: CommandRequestResult; format?: string }) return defaultCommandCliOutput(data); } -// ADR 0014: a reusable ref in a PARTIAL result renders in ready-to-copy -// `@eN~s` form so a human CLI caller can paste it into the next -// mutation without a separate pin step. A mutating result carries no -// `refsGeneration`, so its acted ref is never pinned. -function pinnedRefText(ref: unknown, refsGeneration: unknown): string | undefined { - if (typeof ref !== 'string' || ref.length === 0) return undefined; - if (typeof refsGeneration !== 'number') return undefined; - const body = ref.startsWith('@') ? ref.slice(1) : ref; - return `@${body}~s${refsGeneration}`; -} - function findCliOutput(result: CommandRequestResult): CliOutput { const data = result as Record; // Interactive find actions (click/fill/focus/type) carry the same success message as @@ -84,91 +79,12 @@ function tapCliOutput(result: CommandRequestResult): CliOutput { return { data, text: appendResponseNotes(`Tapped @${ref} (${x}, ${y})`, data) }; } -function messageWithSettleCliOutput(result: CommandRequestResult): CliOutput { - const data = result as Record; - const output = defaultCommandCliOutput(data); - return { data: output.data, text: appendResponseNotes(output.text, data) }; -} - -function appendResponseNotes( - text: string | null | undefined, - data: Record, -): string { - const warning = typeof data.warning === 'string' ? `\nWarning: ${data.warning}` : ''; - return `${text ?? ''}${warning}${formatSettleText(data.settle)}`; -} - -type SettleTextView = { - settled?: boolean; - waitedMs?: number; - hint?: string; - diff?: { - summary?: { additions?: number; removals?: number; unchanged?: number }; - lines?: Array<{ kind?: string; text?: string }>; - truncated?: boolean; - }; - tail?: Array<{ ref?: string; role?: string; label?: string }>; - tailTruncated?: boolean; - refsGeneration?: number; -}; - -/** - * Compact `--settle` (#1101) rendering appended to the tap line: the verdict, - * the changed-count summary, and the changed lines themselves (the payload the - * agent acts on). Empty for non-settle responses. - */ -function formatSettleText(settle: unknown): string { - if (!settle || typeof settle !== 'object') return ''; - const view = settle as SettleTextView; - const parts = [ - formatSettleVerdict(view), - ...formatSettleDiffLines(view.diff), - ...formatSettleTailLines(view), - ]; - if (view.hint) parts.push(`hint: ${view.hint}`); - return `\n${parts.join('\n')}`; -} - -function formatSettleDiffLines(diff: SettleTextView['diff']): string[] { - const lines = (diff?.lines ?? []).map( - (line) => `${line.kind === 'removed' ? '-' : '+'} ${line.text ?? ''}`, - ); - if (diff?.truncated) lines.push('… changed lines truncated'); - return lines; -} - -// Unchanged interactive tail: only present when the diff's added lines -// carried zero refs (modal-dismiss/toast-only diff), so the settled tree's -// remaining actionable elements would otherwise be invisible. -function formatSettleTailLines(view: SettleTextView): string[] { - const tail = view.tail ?? []; - if (tail.length === 0) return []; - const lines = [`unchanged interactive (${tail.length}):`]; - for (const entry of tail) { - const label = entry.label ? ` "${entry.label}"` : ''; - // ADR 0014: the settled tail refs are reusable, so render them pinned when - // the settle response carried its generation. - const ref = pinnedRefText(entry.ref, view.refsGeneration) ?? `@${entry.ref ?? ''}`; - lines.push(`= ${ref} [${entry.role ?? ''}]${label}`); - } - if (view.tailTruncated) { - lines.push('… more interactive elements not shown, use snapshot -i'); - } - return lines; -} - -function formatSettleVerdict(view: SettleTextView): string { - const verdict = view.settled === true ? 'settled' : 'not settled'; - const summary = view.diff?.summary; - if (!summary) return `${verdict} after ${view.waitedMs ?? 0}ms`; - return `${verdict} after ${view.waitedMs ?? 0}ms: +${summary.additions ?? 0} -${summary.removals ?? 0} (~${summary.unchanged ?? 0} unchanged)`; -} - export const interactionCliOutputFormatters = { click: resultOutput(tapCliOutput), press: resultOutput(tapCliOutput), - fill: resultOutput(messageWithSettleCliOutput), - longpress: resultOutput(messageWithSettleCliOutput), + fill: messageWithSettleOutput, + longpress: messageWithSettleOutput, + scroll: messageWithSettleOutput, get: ({ input, result }) => getCliOutput({ result: result as CommandRequestResult, diff --git a/src/commands/interaction/runtime/index.ts b/src/commands/interaction/runtime/index.ts index f46bbc532d..7259c3317c 100644 --- a/src/commands/interaction/runtime/index.ts +++ b/src/commands/interaction/runtime/index.ts @@ -53,6 +53,8 @@ import { type GestureCommandOptions, type GestureCommandResult, } from './gesture-command.ts'; +import { settleObservationCommand, type SettleObservationCommandOptions } from './settle.ts'; +import type { SettleObservation } from '@agent-device/contracts/interaction'; export type SelectorCommands = { find: RuntimeCommand; @@ -78,6 +80,13 @@ export type InteractionCommands = { longPress: RuntimeCommand; scroll: RuntimeCommand; gesture: RuntimeCommand; + /** + * #1638: the observation half of `--settle` for mutations that resolve no + * element (`scroll`/`back`). It performs no device action — the caller's + * command already did — so it lives here purely as the seam the generic + * daemon route reaches the settle engine through. + */ + settleObservation: RuntimeCommand; }; export type BoundSelectorCommands = { @@ -135,6 +144,7 @@ export type BoundInteractionCommands = { ) => Promise; scroll: BoundRuntimeCommand; gesture: BoundRuntimeCommand; + settleObservation: BoundRuntimeCommand; }; export const selectorCommands: SelectorCommands = { @@ -158,6 +168,7 @@ export const interactionCommands: InteractionCommands = { longPress: longPressCommand, scroll: scrollCommand, gesture: gestureCommand, + settleObservation: settleObservationCommand, }; export function bindSelectorCommands(runtime: AgentDeviceRuntime): BoundSelectorCommands { @@ -188,6 +199,7 @@ export function bindInteractionCommands(runtime: AgentDeviceRuntime): BoundInter interactionCommands.longPress(runtime, { ...options, target }), scroll: (options) => interactionCommands.scroll(runtime, options), gesture: (options) => interactionCommands.gesture(runtime, options), + settleObservation: (options) => interactionCommands.settleObservation(runtime, options), }; } diff --git a/src/commands/interaction/runtime/settle.ts b/src/commands/interaction/runtime/settle.ts index 0d2abaa275..96ca9d454a 100644 --- a/src/commands/interaction/runtime/settle.ts +++ b/src/commands/interaction/runtime/settle.ts @@ -16,6 +16,7 @@ import type { SettleParams, SettleTailEntry, } from '@agent-device/contracts/interaction'; +import type { RuntimeCommand } from '../../runtime-types.ts'; import type { CapturedSnapshot } from './selector-read-shared.ts'; import { DEFAULT_STABLE_QUIET_MS, @@ -26,10 +27,17 @@ import { } from './stable-capture.ts'; /** - * `--settle` (#1101): after a mutating interaction, wait for the UI to go - * quiet (wait stable's loop, shared via stable-capture.ts) and return the - * settled DIFF against the pre-action tree in the same response — one round - * trip instead of the interact → observe pair. + * `--settle` (#1101): after a mutating command, wait for the UI to go quiet + * (wait stable's loop, shared via stable-capture.ts) and return the settled + * DIFF against the pre-action tree in the same response — one round trip + * instead of the act → observe pair. + * + * Two entry points over one engine ({@link settleAfterAction}): + * {@link settleAfterInteraction} for the targeted touch commands, which take + * their baseline and proximity point from the resolution, and + * {@link settleObservationCommand} for the target-less generic route + * (`scroll`/`back`, #1638), which supplies the baseline itself and is reached + * as a runtime command because the daemon may not import `commands/`. * * Best-effort by contract: this module never throws. The action already * succeeded when it runs; observation quality is advisory (same principle as @@ -62,6 +70,50 @@ export async function settleAfterInteraction( runtime: AgentDeviceRuntime, options: CommandContext, params: SettleParams & { resolved: ResolvedInteractionTarget }, +): Promise { + return await settleAfterAction(runtime, options, { + ...params, + baselineNodes: resolveBaselineNodes(params.resolved), + actionPoint: params.resolved.point, + }); +} + +export type SettleObservationCommandOptions = CommandContext & + SettleParams & { + /** The pre-action tree the settled diff is taken against. */ + baselineNodes: SnapshotNode[]; + }; + +/** + * The target-less settle as a RUNTIME COMMAND (#1638), which is how the daemon + * reaches it: `scroll` and `back` run the generic route, and that dispatcher + * may not import the command surface (R2) — it composes an `AgentDeviceRuntime` + * and calls commands through it, exactly as the touch handlers do. Returns the + * observation alone; the target-less path has no `--verify` companion to feed, + * so the settled node list stays internal. + */ +export const settleObservationCommand: RuntimeCommand< + SettleObservationCommandOptions, + SettleObservation +> = async (runtime, options) => (await settleAfterAction(runtime, options, options)).observation; + +/** + * The target-less engine (#1638), for mutations that change the screen without + * resolving an element. Same loop, storage, hints, and diff bounds as the + * interaction entry point — only the two things a resolution would have + * supplied come from the caller: + * + * - `baselineNodes` is the diff baseline. On the generic route it is the + * session's STORED pre-action tree, which may be several commands older than + * the action, so the diff honestly reads "settled tree vs the last tree you + * observed" rather than press's freshly resolved pre-action capture. + * - `actionPoint` is absent: with no point there is nothing to self-echo + * against, so the tail's self-echo exclusion simply never fires. + */ +async function settleAfterAction( + runtime: AgentDeviceRuntime, + options: CommandContext, + params: SettleParams & { baselineNodes: SnapshotNode[]; actionPoint?: Point }, ): Promise { const quietMs = params.quietMs ?? DEFAULT_STABLE_QUIET_MS; const timeoutMs = params.timeoutMs ?? DEFAULT_STABLE_TIMEOUT_MS; @@ -72,44 +124,7 @@ export async function settleAfterInteraction( timeoutMs, resetBudgetOnPrivateAxRecovery: true, }); - const observation: SettleObservation = { - ...base, - settled: outcome.settled, - waitedMs: outcome.waitedMs, - captures: outcome.captures, - }; - if (!outcome.lastCapture) { - return { - observation: { - ...observation, - hint: outcome.stalled ? SETTLE_CAPTURE_STALLED_HINT : NEVER_SETTLED_HINT, - }, - }; - } - const { stored, session } = await storeSettledSnapshot(runtime, options, outcome.lastCapture); - const settledNodes = outcome.lastCapture.snapshot.nodes; - return { - observation: { - ...observation, - // The diff (with its added-line refs) is only attached when the settled - // tree actually became the stored session snapshot: those refs must be - // valid against the tree the next @ref command resolves on. The daemon - // treats `diff` presence as "this response issues refs". Unsettled - // captures are intentionally diff-less: they are not a stable - // observation, so surfacing refs would invite agents to act on - // advisory state. - ...(outcome.settled && stored - ? buildSettleDiffAndTail( - resolveBaselineNodes(params.resolved), - settledNodes, - params.resolved.point, - session?.appBundleId, - ) - : {}), - ...resolveSettleHint(outcome, stored, settledNodes.length), - }, - settledNodes, - }; + return await readSettledOutcome(runtime, options, params, base, outcome); } catch (error) { // Never fail the action over the observation: report that settling itself // broke and let the caller fall back to an explicit snapshot. @@ -122,6 +137,54 @@ export async function settleAfterInteraction( } } +/** Turns a finished stable-capture loop into the settled observation payload. */ +async function readSettledOutcome( + runtime: AgentDeviceRuntime, + options: CommandContext, + params: { baselineNodes: SnapshotNode[]; actionPoint?: Point }, + base: SettleObservation, + outcome: Awaited>, +): Promise { + const observation: SettleObservation = { + ...base, + settled: outcome.settled, + waitedMs: outcome.waitedMs, + captures: outcome.captures, + }; + if (!outcome.lastCapture) { + return { + observation: { + ...observation, + hint: outcome.stalled ? SETTLE_CAPTURE_STALLED_HINT : NEVER_SETTLED_HINT, + }, + }; + } + const { stored, session } = await storeSettledSnapshot(runtime, options, outcome.lastCapture); + const settledNodes = outcome.lastCapture.snapshot.nodes; + return { + observation: { + ...observation, + // The diff (with its added-line refs) is only attached when the settled + // tree actually became the stored session snapshot: those refs must be + // valid against the tree the next @ref command resolves on. The daemon + // treats `diff` presence as "this response issues refs". Unsettled + // captures are intentionally diff-less: they are not a stable + // observation, so surfacing refs would invite agents to act on + // advisory state. + ...(outcome.settled && stored + ? buildSettleDiffAndTail( + params.baselineNodes, + settledNodes, + params.actionPoint, + session?.appBundleId, + ) + : {}), + ...resolveSettleHint(outcome, stored, settledNodes.length), + }, + settledNodes, + }; +} + /** * `--settle --verify` composition: the settle loop's final capture doubles as * the verify evidence source, so the pair costs zero extra captures. Without a diff --git a/src/commands/output-common.ts b/src/commands/output-common.ts index e8dbe6c289..8597302ba0 100644 --- a/src/commands/output-common.ts +++ b/src/commands/output-common.ts @@ -17,3 +17,16 @@ export const messageOutput = resultOutput(messageCliOutput); export function messageCliOutput(result: Record): CliOutput { return { data: result, text: readCommandMessage(result) }; } + +/** + * ADR 0014: a reusable ref in a PARTIAL result renders in ready-to-copy + * `@eN~s` form so a human CLI caller can paste it into the next + * mutation without a separate pin step. A mutating result carries no + * `refsGeneration`, so its acted ref is never pinned. + */ +export function pinnedRefText(ref: unknown, refsGeneration: unknown): string | undefined { + if (typeof ref !== 'string' || ref.length === 0) return undefined; + if (typeof refsGeneration !== 'number') return undefined; + const body = ref.startsWith('@') ? ref.slice(1) : ref; + return `@${body}~s${refsGeneration}`; +} diff --git a/src/commands/post-action-observation-grammar.ts b/src/commands/post-action-observation-grammar.ts new file mode 100644 index 0000000000..2b4111bc14 --- /dev/null +++ b/src/commands/post-action-observation-grammar.ts @@ -0,0 +1,62 @@ +import type { PostActionObservationSupportFor } from '../core/command-descriptor/post-action-observation.ts'; +import { + commandSupportsSettleObservation, + commandSupportsVerifyEvidence, +} from '../core/command-descriptor/registry.ts'; +import { SETTLE_FLAGS } from './cli-grammar/flag-groups.ts'; +import type { FlagKey } from './cli-grammar/flag-types.ts'; +import { booleanField, integerField } from './command-input.ts'; + +/** + * The two caller-facing surfaces a command's post-action observation trait + * entitles it to (`--verify` / `--settle`, #1047/#1101): the metadata input + * fields (Node SDK options + MCP tool schema) and the CLI allowed flags. Both + * are materialized from the descriptor registry rather than hand-listed per + * command. This lives outside the interaction family because the trait does + * too: `scroll` and `back` carry it on the generic daemon route (#1638), and + * `back` is a system command. + * + * A descriptor gate (`post-action-observation.test.ts`) asserts, over every + * descriptor, that both surfaces are present exactly when the trait is — so a + * new settle-capable command cannot ship with a schema or grammar that hides + * the flags. + */ + +const verifyField = () => + booleanField( + 'Capture cheap post-action evidence (AX digest, node counts, changedFromBefore) instead of a follow-up snapshot.', + ); + +const settleFields = () => ({ + settle: booleanField( + 'After the action, wait for the UI to go quiet and return the settled diff vs the pre-action tree in the same response. Best-effort; never fails the action.', + ), + settleQuietMs: integerField('Settle: quiet window in milliseconds (default 500).', { min: 0 }), + timeoutMs: integerField('Settle: wait deadline in milliseconds (default 10000).', { min: 1 }), +}); + +type VerifyFieldMap = { verify: ReturnType }; +type SettleFieldMap = ReturnType; + +export type PostActionObservationFields = + PostActionObservationSupportFor extends 'settle-and-verify' + ? VerifyFieldMap & SettleFieldMap + : PostActionObservationSupportFor extends 'settle' + ? SettleFieldMap + : {}; + +export function postActionObservationFields( + command: TName, +): PostActionObservationFields { + return { + ...(commandSupportsVerifyEvidence(command) ? { verify: verifyField() } : {}), + ...(commandSupportsSettleObservation(command) ? settleFields() : {}), + } as PostActionObservationFields; +} + +export function postActionObservationCliFlags(command: string): readonly FlagKey[] { + const flags: FlagKey[] = []; + if (commandSupportsVerifyEvidence(command)) flags.push('verify'); + if (commandSupportsSettleObservation(command)) flags.push(...SETTLE_FLAGS); + return flags; +} diff --git a/src/commands/settle-output.ts b/src/commands/settle-output.ts new file mode 100644 index 0000000000..4fd3fe8b7e --- /dev/null +++ b/src/commands/settle-output.ts @@ -0,0 +1,115 @@ +import type { CliOutput } from './command-contract.ts'; +import { + messageCliOutput, + pinnedRefText, + resultOutput, + type CliOutputFormatter, +} from './output-common.ts'; + +/** + * Compact `--settle` (#1101) rendering appended to a command's own success + * line: the verdict, the changed-count summary, and the changed lines + * themselves (the payload the agent acts on). Empty for non-settle responses, + * so a formatter can append it unconditionally. + * + * Shared across families because the trait is (#1638): the touch commands + * render it next to their tap line, `scroll` and `back` next to theirs. + */ + +type SettleTextView = { + settled?: boolean; + waitedMs?: number; + hint?: string; + diff?: { + summary?: { additions?: number; removals?: number; unchanged?: number }; + lines?: Array<{ kind?: string; text?: string; ref?: string }>; + truncated?: boolean; + }; + tail?: Array<{ ref?: string; role?: string; label?: string }>; + tailTruncated?: boolean; + refsGeneration?: number; +}; + +/** `messageCliOutput` plus the response's warning and settle notes. */ +export const messageWithSettleOutput: CliOutputFormatter = resultOutput( + (result: Record): CliOutput => { + const output = messageCliOutput(result); + return { data: output.data, text: appendResponseNotes(output.text, result) }; + }, +); + +export function appendResponseNotes( + text: string | null | undefined, + data: Record, +): string { + const warning = typeof data.warning === 'string' ? `\nWarning: ${data.warning}` : ''; + return `${text ?? ''}${warning}${formatSettleText(data.settle)}`; +} + +function formatSettleText(settle: unknown): string { + if (!settle || typeof settle !== 'object') return ''; + const view = settle as SettleTextView; + const parts = [ + formatSettleVerdict(view), + ...formatSettleDiffLines(view), + ...formatSettleTailLines(view), + ]; + if (view.hint) parts.push(`hint: ${view.hint}`); + return `\n${parts.join('\n')}`; +} + +function formatSettleDiffLines(view: SettleTextView): string[] { + const diff = view.diff; + const lines = (diff?.lines ?? []).map( + (line) => + `${line.kind === 'removed' ? '-' : '+'} ${pinnedDiffLineText(line, view.refsGeneration)}`, + ); + if (diff?.truncated) lines.push('… changed lines truncated'); + return lines; +} + +/** + * ADR 0014: a settled diff publishes a PARTIAL frame, which admits only the + * pinned form of the refs it issued — so a CLI caller who copies a bare `@eN` + * out of the diff gets `plain_ref_requires_complete_frame`. Render the added + * line's ref in the same paste-ready `@eN~s` form the tail already uses. + * Only added lines carry a ref (`SettleDiffLine`); removed lines render + * verbatim, since nothing there is a target. + */ +function pinnedDiffLineText( + line: { text?: string; ref?: string }, + refsGeneration: number | undefined, +): string { + const text = line.text ?? ''; + const pinned = pinnedRefText(line.ref, refsGeneration); + if (!pinned || !line.ref) return text; + const plain = `@${line.ref}`; + return text.startsWith(`${plain} `) ? `${pinned}${text.slice(plain.length)}` : text; +} + +// Unchanged interactive tail: only present when the diff's added lines +// carried zero refs (modal-dismiss/toast-only diff), so the settled tree's +// remaining actionable elements would otherwise be invisible. +function formatSettleTailLines(view: SettleTextView): string[] { + const tail = view.tail ?? []; + if (tail.length === 0) return []; + const lines = [`unchanged interactive (${tail.length}):`]; + for (const entry of tail) { + const label = entry.label ? ` "${entry.label}"` : ''; + // ADR 0014: the settled tail refs are reusable, so render them pinned when + // the settle response carried its generation. + const ref = pinnedRefText(entry.ref, view.refsGeneration) ?? `@${entry.ref ?? ''}`; + lines.push(`= ${ref} [${entry.role ?? ''}]${label}`); + } + if (view.tailTruncated) { + lines.push('… more interactive elements not shown, use snapshot -i'); + } + return lines; +} + +function formatSettleVerdict(view: SettleTextView): string { + const verdict = view.settled === true ? 'settled' : 'not settled'; + const summary = view.diff?.summary; + if (!summary) return `${verdict} after ${view.waitedMs ?? 0}ms`; + return `${verdict} after ${view.waitedMs ?? 0}ms: +${summary.additions ?? 0} -${summary.removals ?? 0} (~${summary.unchanged ?? 0} unchanged)`; +} diff --git a/src/commands/system/index.test.ts b/src/commands/system/index.test.ts index 14cc3b7cc7..7f3a1160d1 100644 --- a/src/commands/system/index.test.ts +++ b/src/commands/system/index.test.ts @@ -27,6 +27,7 @@ import { tvRemoteDaemonWriter, systemCommandFamily, } from './index.ts'; +import { systemCliOutputFormatters } from './output.ts'; function flags(overrides: Partial = {}): CliFlags { return overrides as CliFlags; @@ -118,6 +119,50 @@ describe('system command interface', () => { ).toBeUndefined(); }); + // #1638: --settle has to survive BOTH back seams — the reader that builds the + // input and the writer that turns it into daemon request options. + test('back reader and writer carry the settle request through to daemon flags', () => { + const input = backCliReader([], flags({ settle: true, settleQuietMs: 250, timeoutMs: 8_000 })); + expect(input).toMatchObject({ settle: true, settleQuietMs: 250, timeoutMs: 8_000 }); + expect(backDaemonWriter(input).options).toMatchObject({ + settle: true, + settleQuietMs: 250, + timeoutMs: 8_000, + }); + expect(backCliReader([], flags()).settle).toBeUndefined(); + }); + + test('back CLI output renders the settled observation', () => { + const output = systemCliOutputFormatters.back({ + input: {}, + result: { + action: 'back', + mode: 'in-app', + message: 'Back', + settle: { + settled: true, + waitedMs: 300, + diff: { + summary: { additions: 1, removals: 2, unchanged: 5 }, + lines: [ + { kind: 'removed', text: '@e9 [button] "Save"' }, + { kind: 'added', text: '@e3 [button] "Edit"', ref: 'e3' }, + ], + }, + }, + }, + }); + + expect(output.text).toBe( + [ + 'Back', + 'settled after 300ms: +1 -2 (~5 unchanged)', + '- @e9 [button] "Save"', + '+ @e3 [button] "Edit"', + ].join('\n'), + ); + }); + test('orientation reader and writer normalize orientation', () => { expect(orientationCliReader(['left'], flags())).toMatchObject({ orientation: 'landscape-left', diff --git a/src/commands/system/index.ts b/src/commands/system/index.ts index 29bfa38f62..7d3f18c3be 100644 --- a/src/commands/system/index.ts +++ b/src/commands/system/index.ts @@ -16,6 +16,7 @@ import { optionalString, request, requiredDaemonString, + settleInputFromFlags, } from '../cli-grammar/common.ts'; import type { CliReader, DaemonWriter } from '../cli-grammar/types.ts'; import { defineExecutableCommand } from '../command-contract.ts'; @@ -32,6 +33,10 @@ import { projectCommandOutputSchemas, } from '../family/types.ts'; import { defineFieldCommandMetadata } from '../field-command-contract.ts'; +import { + postActionObservationCliFlags, + postActionObservationFields, +} from '../post-action-observation-grammar.ts'; import { NAVIGATION_COMMAND_PROJECTIONS } from './navigation-projection.ts'; import { systemCliOutputFormatters } from './output.ts'; @@ -71,6 +76,7 @@ const appStateCommandMetadata = defineFieldCommandMetadata( const backCommandMetadata = defineFieldCommandMetadata(BACK_COMMAND_NAME, backCommandDescription, { mode: enumField(BACK_MODES), + ...postActionObservationFields(BACK_COMMAND_NAME), }); const homeCommandMetadata = defineFieldCommandMetadata( @@ -172,8 +178,8 @@ const tvRemoteCommandDefinition = defineExecutableCommand( const appStateCliSchema = {} as const satisfies CommandSchemaOverride; const backCliSchema = { - usageOverride: 'back [--in-app|--system]', - allowedFlags: ['backMode'], + usageOverride: 'back [--in-app|--system] [--settle]', + allowedFlags: ['backMode', ...postActionObservationCliFlags(BACK_COMMAND_NAME)], } as const satisfies CommandSchemaOverride; const homeCliSchema = {} as const satisfies CommandSchemaOverride; @@ -210,6 +216,7 @@ export const appSwitcherCliReader: CliReader = (_positionals, flags) => commonIn export const backCliReader: CliReader = (_positionals, flags) => ({ ...commonInputFromFlags(flags), + ...settleInputFromFlags(flags), mode: flags.backMode, }); diff --git a/src/commands/system/navigation-projection.ts b/src/commands/system/navigation-projection.ts index 9102093b17..7d7ef98c66 100644 --- a/src/commands/system/navigation-projection.ts +++ b/src/commands/system/navigation-projection.ts @@ -10,6 +10,7 @@ import { type BackMode, type TvRemoteButton, } from '@agent-device/contracts/interaction'; +import type { SettleCommandOptions } from '@agent-device/contracts/client'; import type { ExecutableCommandProjection } from '../command-contract.ts'; declare const navigationCommandProjectionType: unique symbol; @@ -39,7 +40,17 @@ function defineNavigationCommandProjection< } export const NAVIGATION_COMMAND_PROJECTIONS = { - back: defineNavigationCommandProjection<{ mode?: BackMode }, BackCommandResult, false, 'back'>({ + // #1638: `back` carries the settle observation trait, so its options include + // the shared `--settle` triple and its result may carry the settled diff. + // The output schema stays the closed dispatch shape here; the opt-in + // observation property is grafted on where `settleObservationSchema` lives + // (src/mcp/command-output-schemas.ts), which this layer must not import. + back: defineNavigationCommandProjection< + { mode?: BackMode } & SettleCommandOptions, + BackCommandResult, + false, + 'back' + >({ clientMethod: 'back', outputSchema: { type: 'object', diff --git a/src/commands/system/output.ts b/src/commands/system/output.ts index 6ba5306409..c0559d3046 100644 --- a/src/commands/system/output.ts +++ b/src/commands/system/output.ts @@ -10,6 +10,7 @@ import { resultOutput, type CliOutputFormatter, } from '../output-common.ts'; +import { messageWithSettleOutput } from '../settle-output.ts'; function appStateCliOutput(result: AppStateCommandResult): CliOutput { return { @@ -42,7 +43,8 @@ function clipboardCliOutput(result: ClipboardCommandResult): CliOutput { export const systemCliOutputFormatters = { appstate: resultOutput(appStateCliOutput), - back: messageOutput, + // #1638: back is settle-capable, so its line carries the settled diff. + back: messageWithSettleOutput, home: messageOutput, orientation: messageOutput, 'app-switcher': messageOutput, diff --git a/src/core/command-descriptor/__tests__/post-action-observation.test.ts b/src/core/command-descriptor/__tests__/post-action-observation.test.ts index ee0521204c..4cbed78eb6 100644 --- a/src/core/command-descriptor/__tests__/post-action-observation.test.ts +++ b/src/core/command-descriptor/__tests__/post-action-observation.test.ts @@ -15,6 +15,8 @@ const SETTLE_OBSERVATION_COMMANDS = [ PUBLIC_COMMANDS.fill, PUBLIC_COMMANDS.longPress, PUBLIC_COMMANDS.press, + PUBLIC_COMMANDS.scroll, + PUBLIC_COMMANDS.back, ] as const; test('post-action observation descriptor traits are the source for settle command support', () => { @@ -31,6 +33,12 @@ test('post-action observation descriptor traits are the source for settle comman assert.equal(resolveCommandPostActionObservationSupport('fill'), 'settle-and-verify'); assert.equal(resolveCommandPostActionObservationSupport('longpress'), 'settle'); assert.equal(commandSupportsVerifyEvidence('longpress'), false); + // #1638: the generic-route pair resolves no element, so there is nothing to + // digest into --verify evidence — the settled diff IS the observation. + assert.equal(resolveCommandPostActionObservationSupport('scroll'), 'settle'); + assert.equal(resolveCommandPostActionObservationSupport('back'), 'settle'); + assert.equal(commandSupportsVerifyEvidence('scroll'), false); + assert.equal(commandSupportsVerifyEvidence('back'), false); }); test('post-action observation CLI flags follow descriptor traits', () => { diff --git a/src/core/command-descriptor/__tests__/timeout-policy.test.ts b/src/core/command-descriptor/__tests__/timeout-policy.test.ts index 3d9bba776d..287f21d180 100644 --- a/src/core/command-descriptor/__tests__/timeout-policy.test.ts +++ b/src/core/command-descriptor/__tests__/timeout-policy.test.ts @@ -68,10 +68,13 @@ test('daemon-preserving timeout commands are a bounded, reviewed set', () => { // Interaction commands joined in #1105: their target resolution runs the // same capture as snapshot, and resetting the daemon on a wedged capture // destroyed healthy app sessions. + // scroll/back joined in #1638: `--settle` gives them the same post-action + // capture loop, so a wedged bridge is now their dominant hang mode too. const preserving = commandDescriptors .filter((descriptor) => descriptor.timeoutPolicy.onTimeout === 'preserve-daemon') .map((descriptor) => descriptor.name); assert.deepEqual(preserving.sort(), [ + 'back', 'click', 'fill', 'find', @@ -79,6 +82,7 @@ test('daemon-preserving timeout commands are a bounded, reviewed set', () => { 'is', 'longpress', 'press', + 'scroll', 'snapshot', 'type', 'wait', diff --git a/src/core/command-descriptor/post-action-observation.ts b/src/core/command-descriptor/post-action-observation.ts index 355d9a7415..5938e1d8a7 100644 --- a/src/core/command-descriptor/post-action-observation.ts +++ b/src/core/command-descriptor/post-action-observation.ts @@ -1,10 +1,18 @@ export type PostActionObservationSupport = 'settle' | 'settle-and-verify'; +// `scroll` and `back` are settle-only (#1638): both mutate the screen without +// resolving an element, so there is no target to re-digest into `--verify` +// evidence — the settled diff IS the observation. They also run the generic +// daemon route rather than the interaction route, which is why the settle +// engine takes its diff baseline from the caller (the stored pre-action tree) +// instead of a resolution's `preActionNodes`. const POST_ACTION_OBSERVATION_BY_COMMAND = { click: 'settle-and-verify', press: 'settle-and-verify', fill: 'settle-and-verify', longpress: 'settle', + scroll: 'settle', + back: 'settle', } as const satisfies Record; export type PostActionObservationCommandName = keyof typeof POST_ACTION_OBSERVATION_BY_COMMAND; diff --git a/src/core/command-descriptor/registry.ts b/src/core/command-descriptor/registry.ts index 1889db6636..a84acf4426 100644 --- a/src/core/command-descriptor/registry.ts +++ b/src/core/command-descriptor/registry.ts @@ -287,10 +287,21 @@ const FILL_INTERACTION_RESPONSE_DATA_TRANSFORM = { }, } as const satisfies CommandResponseDataTransform; -function interactionTimeoutPolicy(command: string): CommandTimeoutPolicy { +/** + * A settle-capable command spends its `--timeout` on the post-action wait, so + * the envelope has to widen by that budget wherever the trait is declared — + * interaction route or generic route (#1638). `withoutObservation` is the + * policy the command would carry if the trait were dropped, so removing a + * trait restores the command's own envelope instead of silently leaving it on + * the settle one. + */ +function postActionObservationTimeoutPolicy( + command: string, + withoutObservation: CommandTimeoutPolicy, +): CommandTimeoutPolicy { return resolvePostActionObservationSupport(command) !== undefined ? SETTLE_FLAG_PRESERVE_DAEMON_TIMEOUT_POLICY - : PRESERVE_DAEMON_TIMEOUT_POLICY; + : withoutObservation; } function postActionObservation(command: string): PostActionObservationSupport { @@ -903,7 +914,7 @@ export const RAW_COMMAND_DESCRIPTORS = [ androidBlockingDialogGuard: true, }, capability: { apple: APPLE_SIM_AND_DEVICE, android: ANDROID_ALL, linux: LINUX_DEVICE }, - timeoutPolicy: interactionTimeoutPolicy('click'), + timeoutPolicy: postActionObservationTimeoutPolicy('click', PRESERVE_DAEMON_TIMEOUT_POLICY), postActionObservation: postActionObservation('click'), responseDataTransform: TOUCH_INTERACTION_RESPONSE_DATA_TRANSFORM, batchable: true, @@ -922,7 +933,7 @@ export const RAW_COMMAND_DESCRIPTORS = [ }, dispatch: {}, capability: { apple: APPLE_SIM_AND_DEVICE, android: ANDROID_ALL, linux: LINUX_DEVICE }, - timeoutPolicy: interactionTimeoutPolicy('fill'), + timeoutPolicy: postActionObservationTimeoutPolicy('fill', PRESERVE_DAEMON_TIMEOUT_POLICY), postActionObservation: postActionObservation('fill'), responseDataTransform: FILL_INTERACTION_RESPONSE_DATA_TRANSFORM, batchable: true, @@ -965,7 +976,7 @@ export const RAW_COMMAND_DESCRIPTORS = [ }, dispatch: {}, capability: { apple: APPLE_SIM_AND_DEVICE, android: ANDROID_ALL, linux: LINUX_DEVICE }, - timeoutPolicy: interactionTimeoutPolicy('press'), + timeoutPolicy: postActionObservationTimeoutPolicy('press', PRESERVE_DAEMON_TIMEOUT_POLICY), postActionObservation: postActionObservation('press'), responseDataTransform: TOUCH_INTERACTION_RESPONSE_DATA_TRANSFORM, batchable: true, @@ -983,7 +994,7 @@ export const RAW_COMMAND_DESCRIPTORS = [ }, dispatch: {}, capability: ALL_DEVICE_COMMAND_CAPABILITY, - timeoutPolicy: interactionTimeoutPolicy('type'), + timeoutPolicy: postActionObservationTimeoutPolicy('type', PRESERVE_DAEMON_TIMEOUT_POLICY), batchable: true, }, { @@ -995,7 +1006,7 @@ export const RAW_COMMAND_DESCRIPTORS = [ recordingEffect: 'observes-app', daemon: { route: 'interaction', refFrameEffect: 'preserve' }, capability: ALL_DEVICE_COMMAND_CAPABILITY, - timeoutPolicy: interactionTimeoutPolicy('get'), + timeoutPolicy: postActionObservationTimeoutPolicy('get', PRESERVE_DAEMON_TIMEOUT_POLICY), batchable: true, }, { @@ -1016,7 +1027,7 @@ export const RAW_COMMAND_DESCRIPTORS = [ recordingEffect: 'observes-app', daemon: { route: 'interaction', refFrameEffect: 'preserve' }, capability: ALL_DEVICE_COMMAND_CAPABILITY, - timeoutPolicy: interactionTimeoutPolicy('is'), + timeoutPolicy: postActionObservationTimeoutPolicy('is', PRESERVE_DAEMON_TIMEOUT_POLICY), batchable: true, }, @@ -1030,6 +1041,8 @@ export const RAW_COMMAND_DESCRIPTORS = [ ...GENERIC_MUTATING_LINUX_DEVICE_COMMAND_TRAITS.capability, vega: VEGA_VVD, }, + timeoutPolicy: postActionObservationTimeoutPolicy('back', DEFAULT_TIMEOUT_POLICY), + postActionObservation: postActionObservation('back'), }, { name: 'gesture', @@ -1103,6 +1116,8 @@ export const RAW_COMMAND_DESCRIPTORS = [ ...(ownerFilesEnabled ? { ownerFiles: ['src/commands/interaction/index.ts'] as const } : {}), catalog: { group: 'public' }, ...GENERIC_MUTATING_LINUX_DEVICE_COMMAND_TRAITS, + timeoutPolicy: postActionObservationTimeoutPolicy('scroll', DEFAULT_TIMEOUT_POLICY), + postActionObservation: postActionObservation('scroll'), }, { name: 'swipe', diff --git a/src/daemon/__tests__/generic-settle.test.ts b/src/daemon/__tests__/generic-settle.test.ts new file mode 100644 index 0000000000..9c4500e036 --- /dev/null +++ b/src/daemon/__tests__/generic-settle.test.ts @@ -0,0 +1,360 @@ +import { beforeEach, expect, test, vi } from 'vitest'; +import type { SnapshotBackend } from '@agent-device/kernel/snapshot'; +import type { CommandFlags } from '@agent-device/contracts/command'; +import { makeIosSession } from '../../__tests__/test-utils/session-factories.ts'; +import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts'; +import { activateCompleteRefFrame } from '../ref-frame.ts'; +import { setSessionSnapshot } from '../session-snapshot.ts'; +import type { SessionStore } from '../session-store.ts'; +import type { DaemonRequest, DaemonResponse, SessionState } from '../types.ts'; +import { buildSnapshotState } from '../handlers/snapshot-capture.ts'; + +// #1638 `--settle` on the GENERIC daemon route (scroll/back): the settled diff, +// its refs, and the ref-frame/generation dance are the same contract the touch +// commands get — but the baseline is the session's stored pre-action tree, not +// a resolution, and the observation must run after the deferred-outcome +// markers. Quiet windows are tuned down so no test waits real time. + +vi.mock('../../core/dispatch.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + dispatchCommand: vi.fn(async () => ({})), + }; +}); + +vi.mock('../handlers/interaction-snapshot.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + captureSnapshotForSession: vi.fn(async () => ({ + nodes: [], + createdAt: 0, + backend: 'xctest' as const, + })), + }; +}); + +import { dispatchCommand } from '../../core/dispatch.ts'; +import { captureSnapshotForSession } from '../handlers/interaction-snapshot.ts'; +import { dispatchGenericCommand } from '../request-generic-dispatch.ts'; + +const mockDispatch = vi.mocked(dispatchCommand); +const mockCaptureSnapshotForSession = vi.mocked(captureSnapshotForSession); + +const BEFORE_NODES = [ + { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 390, height: 844 } }, + { + index: 1, + parentIndex: 0, + type: 'Button', + label: 'Continue', + rect: { x: 10, y: 20, width: 120, height: 44 }, + hittable: true, + }, +]; + +const AFTER_NODES = [ + { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 390, height: 844 } }, + { + index: 1, + parentIndex: 0, + type: 'Button', + label: 'Load more', + rect: { x: 10, y: 20, width: 120, height: 44 }, + hittable: true, + }, +]; + +const SETTLE_FLAGS = { settle: true, settleQuietMs: 25, timeoutMs: 2_000 } satisfies CommandFlags; + +type SettlePayload = { + settled: boolean; + captures: number; + quietMs: number; + timeoutMs: number; + refsGeneration?: number; + diff?: { + summary: { additions: number; removals: number; unchanged: number }; + lines: Array<{ kind: string; text: string; ref?: string }>; + }; + tail?: Array<{ ref: string; role: string; label?: string }>; + hint?: string; +}; + +/** Capture observations made from inside the emulated capture, in call order. */ +const captureObservations: Array<{ postGestureStabilizationPending: boolean }> = []; + +async function emulateCaptureSnapshotForSession( + session: SessionState, + flags: CommandFlags | undefined, + sessionStore: SessionStore, + contextFromFlags: ( + flags: CommandFlags | undefined, + appBundleId?: string, + traceLogPath?: string, + ) => Record, + options: { interactiveOnly: boolean }, +) { + captureObservations.push({ + postGestureStabilizationPending: session.postGestureStabilization !== undefined, + }); + const effectiveFlags = { ...(flags ?? {}), snapshotInteractiveOnly: options.interactiveOnly }; + const snapshotData = (await mockDispatch( + session.device, + 'snapshot', + [], + effectiveFlags.out, + contextFromFlags(effectiveFlags, session.appBundleId, session.trace?.outPath), + )) as { nodes?: never[]; backend?: SnapshotBackend }; + const snapshot = buildSnapshotState(snapshotData ?? {}, effectiveFlags); + setSessionSnapshot(session, snapshot); + sessionStore.set(session.name, session); + return snapshot; +} + +function mockCommandDispatch(snapshots: Array) { + let snapshotCalls = 0; + mockDispatch.mockImplementation(async (_device, command) => { + if (command === 'snapshot') { + const nodes = snapshots[Math.min(snapshotCalls, snapshots.length - 1)]; + snapshotCalls += 1; + return { nodes, backend: 'xctest' }; + } + return {}; + }); +} + +const contextFromFlags = () => ({}) as never; + +function seedSession(sessionName: string, sessionStore: SessionStore): SessionState { + const session = makeIosSession(sessionName); + setSessionSnapshot(session, buildSnapshotState({ nodes: BEFORE_NODES, backend: 'xctest' }, {})); + activateCompleteRefFrame(session); + sessionStore.set(sessionName, session); + return session; +} + +async function dispatchGeneric(params: { + sessionName: string; + sessionStore: SessionStore; + session: SessionState; + command: string; + positionals?: string[]; + flags?: CommandFlags; +}): Promise { + const req: DaemonRequest = { + token: 't', + session: params.sessionName, + command: params.command, + positionals: params.positionals ?? [], + ...(params.flags ? { flags: params.flags } : {}), + }; + return await dispatchGenericCommand({ + req, + session: params.session, + sessionName: params.sessionName, + logPath: '', + sessionStore: params.sessionStore, + contextFromFlags, + }); +} + +function expectOkData(response: DaemonResponse): Record { + expect(response.ok).toBe(true); + if (response.ok !== true) throw new Error('expected an ok daemon response'); + return (response.data ?? {}) as Record; +} + +beforeEach(() => { + captureObservations.length = 0; + mockDispatch.mockReset(); + mockDispatch.mockResolvedValue({}); + mockCaptureSnapshotForSession.mockReset(); + mockCaptureSnapshotForSession.mockImplementation(emulateCaptureSnapshotForSession); +}); + +test('scroll --settle answers with the settled diff against the stored pre-action tree', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'generic-settle-scroll'; + const session = seedSession(sessionName, sessionStore); + // Every settle capture sees the post-scroll tree; the diff baseline is the + // tree already stored on the session, not a capture taken here. + mockCommandDispatch([AFTER_NODES]); + + const response = await dispatchGeneric({ + sessionName, + sessionStore, + session, + command: 'scroll', + positionals: ['down'], + flags: { ...SETTLE_FLAGS }, + }); + + const settle = expectOkData(response).settle as SettlePayload; + expect(settle.settled).toBe(true); + expect(settle.quietMs).toBe(25); + expect(settle.timeoutMs).toBe(2_000); + expect(settle.diff?.summary).toEqual({ additions: 1, removals: 1, unchanged: 1 }); + expect(settle.diff?.lines).toContainEqual({ + kind: 'added', + text: expect.stringContaining('Load more'), + ref: 'e2', + }); + // `SettleDiffLine`: removed lines never carry a ref — their refs would name + // nodes of the REPLACED tree, and ref bodies are index-derived, so `@e2` on a + // removed line is a different element in the settled tree that took its slot. + // Enforced in snapshot-diff (removed lines are built without one); asserted + // here because this route is the one that diffs against the STORED session + // snapshot, whose nodes all carry refs, so it is where a regression would + // first publish one. + const removed = settle.diff?.lines.find((line) => line.kind === 'removed'); + expect(removed?.text).toContain('Continue'); + expect(removed?.ref).toBeUndefined(); + + // The settled tree became the stored snapshot, and its refs were published: + // a partial frame is active at the generation the payload reports. + const stored = sessionStore.get(sessionName) as SessionState; + expect(stored.refFrameState).toBe('active'); + expect(settle.refsGeneration).toBe(stored.snapshotGeneration); + expect(stored.snapshot?.nodes.some((node) => node.label === 'Load more')).toBe(true); +}); + +test('back --settle answers with the settled diff alongside the command result', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'generic-settle-back'; + const session = seedSession(sessionName, sessionStore); + mockDispatch.mockImplementation(async (_device, command) => { + if (command === 'snapshot') return { nodes: AFTER_NODES, backend: 'xctest' }; + return { action: 'back', mode: 'in-app', message: 'Back' }; + }); + + const response = await dispatchGeneric({ + sessionName, + sessionStore, + session, + command: 'back', + flags: { ...SETTLE_FLAGS }, + }); + + const data = expectOkData(response); + // The observation rides ALONGSIDE the command's own closed result shape. + expect(data.action).toBe('back'); + expect(data.message).toBe('Back'); + expect((data.settle as SettlePayload).diff?.summary).toEqual({ + additions: 1, + removals: 1, + unchanged: 1, + }); +}); + +test('the settle observation runs after the post-gesture stabilization marker', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'generic-settle-order'; + const session = seedSession(sessionName, sessionStore); + mockCommandDispatch([AFTER_NODES]); + + await dispatchGeneric({ + sessionName, + sessionStore, + session, + command: 'scroll', + positionals: ['down'], + flags: { ...SETTLE_FLAGS }, + }); + + // #1542 ordering: scroll marks a pending stabilization, and settle's FIRST + // capture must already see it so the capture folds the stabilization in + // instead of racing it. + expect(captureObservations.length).toBeGreaterThan(0); + expect(captureObservations[0]?.postGestureStabilizationPending).toBe(true); +}); + +test('scroll without --settle takes no observation captures and issues no refs', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'generic-settle-off'; + const session = seedSession(sessionName, sessionStore); + mockCommandDispatch([AFTER_NODES]); + + const response = await dispatchGeneric({ + sessionName, + sessionStore, + session, + command: 'scroll', + positionals: ['down'], + }); + + expect(expectOkData(response).settle).toBeUndefined(); + expect(captureObservations).toEqual([]); + // ADR 0014: the leaf side-effect seam expired the frame and nothing + // re-published it. + expect((sessionStore.get(sessionName) as SessionState).refFrameState).toBe('expired'); +}); + +test('a settle observation that cannot build a runtime degrades instead of failing the action', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'generic-settle-evicted'; + // The session the router handed us is no longer in the store — evicted + // between dispatch and observation. Building the settle runtime throws + // SESSION_NOT_FOUND, and the observation is best-effort: the scroll already + // happened, so the response keeps its result and simply carries no settle. + const session = makeIosSession(sessionName); + setSessionSnapshot(session, buildSnapshotState({ nodes: BEFORE_NODES, backend: 'xctest' }, {})); + activateCompleteRefFrame(session); + mockCommandDispatch([AFTER_NODES]); + + const response = await dispatchGeneric({ + sessionName, + sessionStore, + session, + command: 'scroll', + positionals: ['down'], + flags: { ...SETTLE_FLAGS }, + }); + + const data = expectOkData(response); + expect(data.settle).toBeUndefined(); + expect(captureObservations).toEqual([]); +}); + +test('a generic command without the observation trait ignores a stray settle flag', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'generic-settle-traitless'; + const session = seedSession(sessionName, sessionStore); + mockDispatch.mockResolvedValue({ action: 'home', message: 'Home' }); + + // A hand-built daemon request can put settle flags on any command; only the + // descriptor trait admits them to the observation path. `home` has no trait, + // so the flag is ignored: no captures, no settle payload, no rejection. + const response = await dispatchGeneric({ + sessionName, + sessionStore, + session, + command: 'home', + flags: { settle: true }, + }); + + expect(expectOkData(response).settle).toBeUndefined(); + expect(captureObservations).toEqual([]); +}); + +test('an orphaned --settle-quiet is rejected before the command dispatches', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'generic-settle-orphan'; + const session = seedSession(sessionName, sessionStore); + mockDispatch.mockRejectedValue(new Error('dispatch must not run for an orphaned settle flag')); + + const response = await dispatchGeneric({ + sessionName, + sessionStore, + session, + command: 'scroll', + positionals: ['down'], + flags: { settleQuietMs: 25 }, + }); + + expect(response.ok).toBe(false); + if (response.ok !== false) throw new Error('expected a rejected daemon response'); + expect(response.error?.code).toBe('INVALID_ARGS'); + expect(response.error?.message).toContain('--settle-quiet'); +}); diff --git a/src/daemon/generic-settle.ts b/src/daemon/generic-settle.ts new file mode 100644 index 0000000000..90276ed9d8 --- /dev/null +++ b/src/daemon/generic-settle.ts @@ -0,0 +1,118 @@ +import type { CommandFlags } from '@agent-device/contracts/command'; +import type { SettleObservation, SettleParams } from '@agent-device/contracts/interaction'; +import type { SnapshotNode } from '@agent-device/kernel/snapshot'; +import { commandSupportsSettleObservation } from '../core/command-descriptor/registry.ts'; +import type { ContextFromFlags } from './handlers/interaction-common.ts'; +import { readSettleRequest, settleFlagGuardResponse } from './handlers/interaction-flags.ts'; +import { createInteractionRuntime } from './handlers/interaction-runtime.ts'; +import { captureSnapshotForSession } from './handlers/interaction-snapshot.ts'; +import { issueSettleRefs } from './session-snapshot.ts'; +import type { SessionStore } from './session-store.ts'; +import type { DaemonRequest, DaemonResponse, SessionState } from './types.ts'; + +/** + * `--settle` on the generic daemon route (#1638): `scroll` and `back` change + * the screen without resolving an element, so they never reach the interaction + * handler that owns the touch commands' settle wiring — but scroll-then-observe + * and back-then-observe are exactly the pairs an agent wants collapsed into one + * call. This module is the generic route's half of that contract. + * + * It is reached through a LAZY SEAM (`await import`) from + * request-generic-dispatch, and hands back a CLOSURE rather than a plan object, + * so that dispatcher needs no static edge here — not even a type-only one. That + * is not incidental: observing a settled screen runs the interaction runtime, a + * subgraph the generic dispatcher otherwise never touches, and a static edge + * folds all ~18 of its files into the daemon-server type cycle (R10 caught + * exactly that). An ordinary scroll/back pays neither the import nor the + * comprehension cost. + * + * Two seams, and the order between them is load-bearing: + * + * 1. {@link planGenericSettleObservation} runs BEFORE the command dispatches and + * freezes the diff baseline. The baseline is the session's STORED pre-action + * tree, so the diff reads "settled tree vs the last tree you observed" — it + * may be several commands old, unlike press's freshly resolved pre-action + * capture. That is the honest #1101 contract for a route with no resolution + * step, and the settle loop's own captures overwrite `session.snapshot`, so + * reading it afterwards would compare the settled tree against itself. + * 2. The returned observer runs AFTER the command AND after + * `markDeferredInteractionOutcome`. Marking first is what lets settle's very + * first capture fold in the #1542 post-gesture stabilization instead of + * racing it — every capture on this path goes through the same deferred + * outcome resolution a plain snapshot would. + */ + +/** Runs the settle loop and returns the observation to attach as `data.settle`. */ +export type GenericSettleObserver = () => Promise; + +export type GenericSettlePlan = { response: DaemonResponse } | { observe?: GenericSettleObserver }; + +type GenericSettleContext = { + req: DaemonRequest; + session: SessionState; + sessionName: string; + logPath: string; + sessionStore: SessionStore; + contextFromFlags: ContextFromFlags; +}; + +/** + * Rejects an orphaned `--settle-quiet` up front (the shared flag grammar), and + * otherwise returns the observer this dispatch owes — with the pre-action + * baseline already captured. + */ +export function planGenericSettleObservation( + params: GenericSettleContext & { command: string; flags: CommandFlags | undefined }, +): GenericSettlePlan { + if (!commandSupportsSettleObservation(params.command)) return {}; + const invalidSettleFlags = settleFlagGuardResponse(params.command, params.flags); + if (invalidSettleFlags) return { response: invalidSettleFlags }; + const settle = readSettleRequest(params.flags); + if (!settle) return {}; + const baselineNodes = params.session.snapshot?.nodes ?? []; + return { observe: async () => await observeSettled(params, settle, baselineNodes) }; +} + +/** + * Best-effort like the settle engine itself: the action already succeeded, so a + * runtime that cannot even be constructed (session evicted mid-request) + * degrades to no observation rather than failing the response. `refsGeneration` + * is folded in when the settled diff actually published refs (ADR 0014 — see + * `settle-ref-issuance.ts`). + */ +async function observeSettled( + context: GenericSettleContext, + settle: SettleParams, + baselineNodes: SnapshotNode[], +): Promise { + const runtime = createGenericSettleRuntime(context); + if (!runtime) return undefined; + // R2: the daemon reaches the settle engine through the runtime command + // surface, never by importing `commands/` — the same seam the touch handlers + // use for press/fill. + const observation = await runtime.interactions.settleObservation({ + ...settle, + baselineNodes, + session: context.sessionName, + requestId: context.req.meta?.requestId, + }); + const refsGeneration = issueSettleRefs(context.session, observation); + return refsGeneration === undefined ? observation : { ...observation, refsGeneration }; +} + +function createGenericSettleRuntime( + context: GenericSettleContext, +): ReturnType | undefined { + try { + return createInteractionRuntime({ + req: context.req, + sessionName: context.sessionName, + logPath: context.logPath, + sessionStore: context.sessionStore, + contextFromFlags: context.contextFromFlags, + captureSnapshotForSession, + }); + } catch { + return undefined; + } +} diff --git a/src/daemon/handlers/interaction-flags.ts b/src/daemon/handlers/interaction-flags.ts index 0e024b1e94..bd37f86e54 100644 --- a/src/daemon/handlers/interaction-flags.ts +++ b/src/daemon/handlers/interaction-flags.ts @@ -1,5 +1,4 @@ import type { CommandFlags } from '@agent-device/contracts/command'; -import type { PostActionObservationCommandName } from '../../core/command-descriptor/post-action-observation.ts'; import type { SettleParams } from '@agent-device/contracts/interaction'; import type { DaemonResponse } from '../types.ts'; import { errorResponse } from './response.ts'; @@ -42,9 +41,12 @@ export function unsupportedRefSnapshotFlags(flags: CommandFlags | undefined): st * for a bare `--timeout` without `--settle`: older touch commands silently * ignored it. Only `--settle-quiet` is settle-specific enough to reject when * orphaned. + * + * `command` names the command in the error message only; callers gate on the + * descriptor trait (`commandSupportsSettleObservation`) before reaching here. */ export function settleFlagGuardResponse( - command: PostActionObservationCommandName, + command: string, flags: CommandFlags | undefined, ): DaemonResponse | null { if (!flags || flags.settle === true) return null; diff --git a/src/daemon/handlers/interaction-touch.ts b/src/daemon/handlers/interaction-touch.ts index fa530dd8ca..cb9ac475d1 100644 --- a/src/daemon/handlers/interaction-touch.ts +++ b/src/daemon/handlers/interaction-touch.ts @@ -38,7 +38,7 @@ import { type DirectIosSelectorTarget, } from '../direct-ios-selector.ts'; import { expireRefFrame } from '../ref-frame.ts'; -import { markSessionPartialRefsIssued, resolveRefStalenessWarning } from '../session-snapshot.ts'; +import { issueSettleRefs, resolveRefStalenessWarning } from '../session-snapshot.ts'; import type { DaemonResponse, SessionState } from '../types.ts'; import { assertAndroidPressStayedInApp, @@ -326,41 +326,10 @@ async function buildTargetedTouchResponsePayloads(params: { referenceFrame, extra, staleRefsWarning: params.staleRefsWarning, - settleRefsGeneration: settleRefsGenerationIssue(session, result), + settleRefsGeneration: issueSettleRefs(session, result.settle), }); } -/** - * #1101 `--settle`: a settle observation carrying a diff hands the client refs - * minted from the freshly stored settled tree (added lines carry them), which - * makes the response ref-issuing like snapshot/find: it activates a PARTIAL - * frame (ADR 0014) authorizing exactly those bodies, and the stored tree's - * generation rides inside the settle payload for MCP per-ref pinning. Without a - * diff — never captured, or sparse-quality capture that was not stored — nothing - * was issued and the frame is left as the press's leaf seam expired it. - */ -function settleRefsGenerationIssue( - session: SessionState, - result: PressCommandResult | FillCommandResult | LongPressCommandResult, -): number | undefined { - if (!result.settle?.diff) return undefined; - // ADR 0014: a settled diff publishes the refs it exposed, so it activates a - // PARTIAL frame authorizing exactly those bodies (not the whole tree). - markSessionPartialRefsIssued(session, collectSettleIssuedRefBodies(result.settle)); - return session.snapshotGeneration; -} - -/** The reusable refs a settled diff exposed: added diff lines, `refs`, `tail`. */ -function collectSettleIssuedRefBodies(settle: NonNullable): string[] { - const bodies: string[] = []; - for (const line of settle.diff?.lines ?? []) { - if (line.ref) bodies.push(line.ref); - } - for (const entry of settle.refs ?? []) bodies.push(entry.ref); - for (const entry of settle.tail ?? []) bodies.push(entry.ref); - return bodies; -} - function readLongPressResultDuration(result: TargetedTouchResult): number | undefined { return 'durationMs' in result ? result.durationMs : undefined; } @@ -787,7 +756,7 @@ function buildFillResponsePayloads(params: { referenceFrame, extra: { text: params.text, ...maestroFallback.extra }, staleRefsWarning: params.staleRefsWarning, - settleRefsGeneration: settleRefsGenerationIssue(session, result), + settleRefsGeneration: issueSettleRefs(session, result.settle), }); } diff --git a/src/daemon/request-generic-dispatch.ts b/src/daemon/request-generic-dispatch.ts index 828aad2c9a..3aa666c9a4 100644 --- a/src/daemon/request-generic-dispatch.ts +++ b/src/daemon/request-generic-dispatch.ts @@ -1,4 +1,6 @@ import type { CommandFlags } from '@agent-device/contracts/command'; +import type { SettleObservation } from '@agent-device/contracts/interaction'; +import { commandSupportsSettleObservation } from '../core/command-descriptor/registry.ts'; import { dispatchCommand } from '../core/dispatch.ts'; import { requireCommandSupported } from './handlers/response.ts'; import { SessionStore } from './session-store.ts'; @@ -51,6 +53,19 @@ export async function dispatchGenericCommand(params: { const readinessResponse = await ensureGenericCommandReady(session, platformCommand); if (readinessResponse) return readinessResponse; + // #1638: freeze the settled diff's baseline before anything can mutate the + // screen or the stored snapshot — including the Android dialog preflight. + const settlePlan = await planGenericSettleObservation({ + req, + session, + sessionName: params.sessionName, + logPath, + sessionStore, + contextFromFlags, + command: platformCommand, + flags: req.flags, + }); + if ('response' in settlePlan) return settlePlan.response; const preflightReadiness = await ensureNoAndroidBlockingDialogReady(session, platformCommand); if ('response' in preflightReadiness) return preflightReadiness.response; @@ -117,9 +132,54 @@ export async function dispatchGenericCommand(params: { flags: req.flags, }); + // Strictly after the deferred-outcome markers so settle's first capture folds + // in the #1542 post-gesture stabilization, and after the recorded action so + // the session history keeps the ACTION's own timing rather than the + // observation wait that followed it. + if (settlePlan.observe) { + const settle = await settlePlan.observe(); + if (settle) data = { ...(data ?? {}), settle }; + } + return { ok: true, data: data ?? {} }; } +/** + * `--settle` (#1638) is opt-in and its observation runs the interaction + * runtime — a subgraph this dispatcher otherwise never touches, and one that a + * static edge would fold into the daemon-server type cycle. Reach it through a + * lazy seam instead, gated on the caller actually passing a settle flag, and + * hold only the returned closure so no type edge exists either. A scroll/back + * without settle, and every non-settle generic leaf, load nothing. + */ +async function planGenericSettleObservation(params: { + req: DaemonRequest; + session: SessionState; + sessionName: string; + logPath: string; + sessionStore: SessionStore; + contextFromFlags: ( + flags: CommandFlags | undefined, + appBundleId?: string, + traceLogPath?: string, + ) => DaemonCommandContext; + command: string; + flags: CommandFlags | undefined; +}): Promise< + { response: DaemonResponse } | { observe?: () => Promise } +> { + if (!commandSupportsSettleObservation(params.command) || !usesSettleFlags(params.flags)) { + return {}; + } + const settle = await import('./generic-settle.ts'); + return settle.planGenericSettleObservation(params); +} + +/** Any settle flag at all — `--settle-quiet` alone still owes the caller its rejection. */ +function usesSettleFlags(flags: CommandFlags | undefined): boolean { + return flags?.settle === true || flags?.settleQuietMs !== undefined; +} + async function ensureNoAndroidBlockingDialogReady( session: SessionState, platformCommand: string, diff --git a/src/daemon/session-event-action.ts b/src/daemon/session-event-action.ts index 121119cf07..258021c1dc 100644 --- a/src/daemon/session-event-action.ts +++ b/src/daemon/session-event-action.ts @@ -296,7 +296,8 @@ const SAFE_ACTION_FLAG_SPECS: Record = { [PUBLIC_COMMANDS.fill]: textEntrySafeFlagSpec(), [PUBLIC_COMMANDS.type]: textEntrySafeFlagSpec(), [PUBLIC_COMMANDS.scroll]: { - numbers: [{ source: 'pixels' }, { source: 'durationMs' }], + booleans: [{ source: 'settle' }], + numbers: [{ source: 'pixels' }, { source: 'durationMs' }, { source: 'settleQuietMs' }], }, [PUBLIC_COMMANDS.gesture]: gestureSafeFlagSpec(), [PUBLIC_COMMANDS.swipe]: gestureSafeFlagSpec(), @@ -322,6 +323,8 @@ const SAFE_ACTION_FLAG_SPECS: Record = { numbers: [{ source: 'snapshotDepth', output: 'depth' }], }, [PUBLIC_COMMANDS.back]: { + booleans: [{ source: 'settle' }], + numbers: [{ source: 'settleQuietMs' }], enums: [{ source: 'backMode', output: 'mode', values: BACK_MODES }], }, [PUBLIC_COMMANDS.record]: { diff --git a/src/daemon/session-snapshot.ts b/src/daemon/session-snapshot.ts index 8e77f6f316..14939cfe86 100644 --- a/src/daemon/session-snapshot.ts +++ b/src/daemon/session-snapshot.ts @@ -1,4 +1,5 @@ import { randomInt } from 'node:crypto'; +import type { SettleObservation } from '@agent-device/contracts/interaction'; import type { SnapshotState } from '@agent-device/kernel/snapshot'; import { activatePartialRefFrame, refFrameEpoch, refFrameState } from './ref-frame.ts'; import type { SessionState } from './types.ts'; @@ -120,6 +121,40 @@ export function markSessionPartialRefsIssued(session: SessionState, refs: Iterab activatePartialRefFrame(session, scope); } +/** + * #1101 `--settle`: a settle observation carrying a diff hands the client refs + * minted from the freshly stored settled tree (added lines carry them), which + * makes the response ref-issuing like snapshot/find: it activates a PARTIAL + * frame (ADR 0014) authorizing exactly those bodies, and returns the stored + * tree's generation to ride inside the settle payload for MCP per-ref pinning. + * Without a diff — never captured, or a sparse-quality capture that was not + * stored — nothing was issued and the frame is left as the action's own + * side-effect seam expired it. + * + * Every route that can attach a settle observation goes through here (the + * touch commands and the generic scroll/back route, #1638), so the issuance + * rule has one implementation beside the partial-frame primitive it wraps. + */ +export function issueSettleRefs( + session: SessionState, + settle: SettleObservation | undefined, +): number | undefined { + if (!settle?.diff) return undefined; + markSessionPartialRefsIssued(session, collectSettleIssuedRefBodies(settle)); + return session.snapshotGeneration; +} + +/** The reusable refs a settled diff exposed: added diff lines, `refs`, `tail`. */ +function collectSettleIssuedRefBodies(settle: SettleObservation): string[] { + const bodies: string[] = []; + for (const line of settle.diff?.lines ?? []) { + if (line.ref) bodies.push(line.ref); + } + for (const entry of settle.refs ?? []) bodies.push(entry.ref); + for (const entry of settle.tail ?? []) bodies.push(entry.ref); + return bodies; +} + /** * Warning for a ref pinned to a generation (`@e12~s3`) whose epoch no longer * matches the session's current ref-frame epoch (`refFrameEpoch`) — NOT the diff --git a/src/mcp/__tests__/command-tools.test.ts b/src/mcp/__tests__/command-tools.test.ts index 1b860ba423..61a292afb1 100644 --- a/src/mcp/__tests__/command-tools.test.ts +++ b/src/mcp/__tests__/command-tools.test.ts @@ -2,7 +2,10 @@ import assert from 'node:assert/strict'; import { test } from 'vitest'; import type { AgentDeviceClient } from '../../client/client-types.ts'; import { createCommandToolExecutor, listCommandTools } from '../command-tools.ts'; -import { resolveCommandRecordsSessionAction } from '../../core/command-descriptor/registry.ts'; +import { + commandSupportsSettleObservation, + resolveCommandRecordsSessionAction, +} from '../../core/command-descriptor/registry.ts'; import { COMMAND_OUTPUT_SCHEMAS } from '../command-output-schemas.ts'; import { AppError } from '@agent-device/kernel/errors'; import { NAVIGATION_COMMAND_PROJECTIONS } from '../../commands/system/navigation-projection.ts'; @@ -447,10 +450,21 @@ test('MCP tv remote outputSchema advertises button values', () => { test('MCP navigation output schemas are projected from the canonical executable contracts', () => { for (const [name, projection] of Object.entries(NAVIGATION_COMMAND_PROJECTIONS)) { - assert.equal( - COMMAND_OUTPUT_SCHEMAS[name as keyof typeof COMMAND_OUTPUT_SCHEMAS], - projection.outputSchema, - ); + const schema = COMMAND_OUTPUT_SCHEMAS[name as keyof typeof COMMAND_OUTPUT_SCHEMAS]; + if (!commandSupportsSettleObservation(name)) { + assert.equal(schema, projection.outputSchema, `${name}: must be the projection itself`); + continue; + } + // #1638: a settle-capable navigation command adds exactly ONE property on + // top of its projected dispatch shape — the opt-in `--settle` observation, + // grafted where `settleObservationSchema` lives because the projection + // layer sits below the MCP schema module. Everything else must still come + // from the projection verbatim. + const observed = schema as { properties?: Record; required?: unknown }; + const { settle, ...projectedProperties } = observed.properties ?? {}; + assert.ok(settle, `${name}: settle-capable schema must advertise the observation`); + assert.deepEqual(projectedProperties, projection.outputSchema?.properties); + assert.deepEqual(observed.required, projection.outputSchema?.required); } }); diff --git a/src/mcp/__tests__/tool-ref-pins.test.ts b/src/mcp/__tests__/tool-ref-pins.test.ts index 28ec0bbe70..ee12b83d34 100644 --- a/src/mcp/__tests__/tool-ref-pins.test.ts +++ b/src/mcp/__tests__/tool-ref-pins.test.ts @@ -544,3 +544,103 @@ test('ref-pin store leaves existing pins untouched for a mutating find without r ); assert.deepEqual(pinned, { session: 'demo', target: { kind: 'ref', ref: '@e2~s7' } }); }); + +// --- #1638: the generic-route settle commands pin on identical terms --- + +test('ref-pin store merges per-ref pins from a scroll settle diff', () => { + const pins = makeStore(); + + pins.mergeCommandResult( + 'snapshot', + { nodes: [{ ref: 'e2' }, { ref: 'e37' }], truncated: false, refsGeneration: 7 }, + undefined, + 'demo', + ); + pins.mergeCommandResult( + 'scroll', + { + direction: 'down', + message: 'Scrolled down', + settle: { + settled: true, + waitedMs: 800, + captures: 3, + quietMs: 500, + timeoutMs: 10_000, + refsGeneration: 8, + diff: { + summary: { additions: 1, removals: 1, unchanged: 1 }, + lines: [ + { kind: 'removed', text: '@e2 [cell] "General"' }, + { kind: 'added', text: '@e4 [cell] "Developer"', ref: 'e4' }, + ], + }, + }, + }, + undefined, + 'demo', + ); + + // The scrolled-in ref pins at the settle generation; a ref the settle never + // republished keeps its older snapshot pin, so the daemon still warns on it. + assert.deepEqual( + pins.pinInput('press', { session: 'demo', target: { kind: 'ref', ref: '@e4' } }, undefined), + { session: 'demo', target: { kind: 'ref', ref: '@e4~s8' } }, + ); + assert.deepEqual( + pins.pinInput('press', { session: 'demo', target: { kind: 'ref', ref: '@e37' } }, undefined), + { session: 'demo', target: { kind: 'ref', ref: '@e37~s7' } }, + ); +}); + +test('ref-pin store merges per-ref pins from a back settle tail', () => { + const pins = makeStore(); + + pins.mergeCommandResult( + 'back', + { + action: 'back', + mode: 'in-app', + message: 'Back', + settle: { + settled: true, + waitedMs: 770, + captures: 3, + quietMs: 500, + timeoutMs: 10_000, + refsGeneration: 12, + diff: { summary: { additions: 0, removals: 2, unchanged: 3 }, lines: [] }, + tail: [{ ref: 'e6', role: 'cell', label: 'Camera' }], + }, + }, + undefined, + 'demo', + ); + + assert.deepEqual( + pins.pinInput('press', { session: 'demo', target: { kind: 'ref', ref: '@e6' } }, undefined), + { session: 'demo', target: { kind: 'ref', ref: '@e6~s12' } }, + ); +}); + +test('ref-pin store leaves pins untouched for a scroll with no settle payload', () => { + const pins = makeStore(); + + pins.mergeCommandResult( + 'snapshot', + { nodes: [{ ref: 'e2' }], truncated: false, refsGeneration: 7 }, + undefined, + 'demo', + ); + pins.mergeCommandResult( + 'scroll', + { direction: 'down', message: 'Scrolled down' }, + undefined, + 'demo', + ); + + assert.deepEqual( + pins.pinInput('press', { session: 'demo', target: { kind: 'ref', ref: '@e2' } }, undefined), + { session: 'demo', target: { kind: 'ref', ref: '@e2~s7' } }, + ); +}); diff --git a/src/mcp/command-output-schemas.ts b/src/mcp/command-output-schemas.ts index ac95440481..45183d933a 100644 --- a/src/mcp/command-output-schemas.ts +++ b/src/mcp/command-output-schemas.ts @@ -304,6 +304,14 @@ const targetShutdownResultSchema: JsonSchema = objectSchema( ['success', 'exitCode', 'stdout', 'stderr'], ); +/** Grafts the opt-in `--settle` observation onto an otherwise closed schema. */ +function withSettleObservation(schema: JsonSchema): JsonSchema { + return { + ...schema, + properties: { ...(schema.properties ?? {}), settle: settleObservationSchema }, + }; +} + const tapInteractionResponseDataSchema = interactionResponseDataSchema({ properties: { evidence: interactionEvidenceSchema, @@ -380,6 +388,11 @@ export const COMMAND_OUTPUT_SCHEMAS = { // packages/contracts/src/navigation.ts, projected from executable command contracts. ...projectedSystemCommandOutputSchemas, + // #1638: the projected navigation schema is the closed dispatch shape. `back` + // is settle-capable on the generic route, so the opt-in observation is added + // here — the projection layer sits below this module and cannot reach + // `settleObservationSchema`. + back: withSettleObservation(projectedSystemCommandOutputSchemas.back), // packages/contracts/src/wait.ts — compact public daemon projection. wait: objectSchema( diff --git a/src/mcp/tool-ref-pins.ts b/src/mcp/tool-ref-pins.ts index 78926188a2..6fc921ceab 100644 --- a/src/mcp/tool-ref-pins.ts +++ b/src/mcp/tool-ref-pins.ts @@ -1,6 +1,11 @@ +import type { SettleObservation } from '@agent-device/contracts/interaction'; import type { SnapshotNode } from '@agent-device/kernel/snapshot'; -import type { CommandName } from '../commands/command-metadata.ts'; +import { isCommandName, type CommandName } from '../commands/command-metadata.ts'; import type { CommandExecutionResult } from '../commands/command-surface.ts'; +import { + commandDescriptors, + commandSupportsSettleObservation, +} from '../core/command-descriptor/registry.ts'; import { asOptionalRecord } from '../utils/parsing.ts'; export type ToolRefPinStore = { @@ -52,20 +57,24 @@ export function createToolRefPinStore(): ToolRefPinStore { const REF_ISSUING_TOOLS: ReadonlySet = new Set(['snapshot', 'find'] as const); /** - * `--settle` (#1101) makes an interaction response CONDITIONALLY ref-issuing: - * when it carries `settle.diff` + `settle.refsGeneration`, the diff's added - * lines hand out refs minted from the freshly stored settled tree. These tools - * are NOT in REF_ISSUING_TOOLS on purpose — a plain (non-settle) press carries - * no generation, and treating that as "issuing response without a generation" + * `--settle` (#1101) makes a response CONDITIONALLY ref-issuing: when it + * carries `settle.diff` + `settle.refsGeneration`, the diff's added lines hand + * out refs minted from the freshly stored settled tree. These tools are NOT in + * REF_ISSUING_TOOLS on purpose — a plain (non-settle) press carries no + * generation, and treating that as "issuing response without a generation" * would clear the scope's pins on every ordinary tap. Absent or diff-less * settle payloads leave pins untouched. + * + * Derived from the descriptor trait rather than hand-listed: a command that + * grows `--settle` (scroll/back, #1638) issues refs the moment it can produce a + * settled diff, and a hand list would silently stop pinning them. */ -const SETTLE_REF_ISSUING_TOOLS: ReadonlySet = new Set([ - 'press', - 'click', - 'fill', - 'longpress', -] as const); +const SETTLE_REF_ISSUING_TOOLS: ReadonlySet = new Set( + commandDescriptors + .map((descriptor) => descriptor.name) + .filter(isCommandName) + .filter((name) => commandSupportsSettleObservation(name)), +); const TARGET_REF_TOOLS: ReadonlySet = new Set([ 'press', @@ -111,11 +120,7 @@ function mergeCommandResult( ): void { const scopeKey = makeScopeKey(stateDir, session); if (SETTLE_REF_ISSUING_TOOLS.has(name)) { - mergeSettleIssuedRefPins( - refPinsByScope, - scopeKey, - result as CommandExecutionResult<'press' | 'click' | 'fill' | 'longpress'>, - ); + mergeSettleIssuedRefPins(refPinsByScope, scopeKey, readSettleObservation(result)); return; } if (!REF_ISSUING_TOOLS.has(name)) return; @@ -199,9 +204,8 @@ function mergeFindRefPins( function mergeSettleIssuedRefPins( refPinsByScope: Map>, scopeKey: string, - result: CommandExecutionResult<'press' | 'click' | 'fill' | 'longpress'>, + settle: SettleObservation | undefined, ): void { - const { settle } = result; if (settle?.refsGeneration === undefined) return; const issuedRefs = [...(settle.diff?.lines ?? []), ...(settle.refs ?? []), ...(settle.tail ?? [])] .map((entry) => entry.ref) @@ -209,6 +213,16 @@ function mergeSettleIssuedRefPins( mergeIntoScopedPins(refPinsByScope, scopeKey, issuedRefs, settle.refsGeneration); } +/** + * The settle payload as this layer reads it. `scroll`/`back` results are not in + * the typed-result spine (CommandResultMap), so the field is read structurally + * rather than through a per-command result union. + */ +function readSettleObservation(result: CommandExecutionResult): SettleObservation | undefined { + const settle = (result as { settle?: unknown }).settle; + return settle !== null && typeof settle === 'object' ? (settle as SettleObservation) : undefined; +} + /** Shared merge-only tail: skip empty issuance, else create-or-reuse the scope's pin map and record. */ function mergeIntoScopedPins( refPinsByScope: Map>, diff --git a/test/output-economy/output-economy.baseline.json b/test/output-economy/output-economy.baseline.json index 5e575e2f4a..a0bbe3a704 100644 --- a/test/output-economy/output-economy.baseline.json +++ b/test/output-economy/output-economy.baseline.json @@ -21,7 +21,7 @@ "shape": "$.nodeCount:number|$.refs:array|$.refsGeneration:number|$.refs[].label:string|$.refs[].ref:string|$.refs[]:object|$.truncated:boolean|$.visibility.partial:boolean|$.visibility.reasons:array|$.visibility.totalNodeCount:number|$.visibility.visibleNodeCount:number|$.visibility:object|$:object" }, "settle.default.text": { - "bytes": 153, + "bytes": 161, "lines": 5, "refs": 3, "hints": 0, @@ -168,7 +168,7 @@ "shape": "text" }, "workflow.mutation-confirm.cli.text": { - "bytes": 153, + "bytes": 161, "lines": 5, "refs": 3, "hints": 0, diff --git a/test/output-economy/output-economy.waivers.json b/test/output-economy/output-economy.waivers.json index 903f2df98b..51550ef0b4 100644 --- a/test/output-economy/output-economy.waivers.json +++ b/test/output-economy/output-economy.waivers.json @@ -6,5 +6,13 @@ "workflow.mutation-tail.cli.text": { "bytes": 173, "reason": "ADR 0014: the routine-workflow mutation-tail step renders the same pinned tail refs in human-CLI text (+8 bytes). See settle-tail.default.text." + }, + "settle.default.text": { + "bytes": 161, + "reason": "ADR 0014 (#1638 follow-up): the settled diff's ADDED lines now render their refs pinned too (+8 bytes for the two added-line pins), closing the gap where the tail was paste-ready but the diff — the primary payload — was not. A settled diff activates a PARTIAL frame, which admits only the pinned form, so a caller copying the bare @eN the diff handed them got plain_ref_requires_complete_frame. Same policy as settle-tail.default.text: JSON and MCP keep the single response-level refsGeneration; only human-CLI text carries the pins." + }, + "workflow.mutation-confirm.cli.text": { + "bytes": 161, + "reason": "ADR 0014 (#1638 follow-up): the routine-workflow mutation-confirm step renders the same pinned added-line refs in human-CLI text (+8 bytes). See settle.default.text." } } diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 6e393a0e28..e0082cfa9b 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -407,7 +407,7 @@ On iOS simulators it uses private XCTest synthesis for a continuous two-finger p On Android, `gesture transform` injects a geometric two-finger path. App recognizers may report non-exact pan, scale, and rotation values, so verify qualitative state such as `pan changed yes`, `pinch changed yes`, and `rotate changed yes` unless the app explicitly promises exact centroid metrics. If exact app-state values matter, prefer isolated `gesture pan`, `gesture pinch`, or `gesture rotate` commands. `scroll` accepts either a relative amount (`0.5` means roughly half of the viewport on that axis) or `--pixels ` for a fixed-distance gesture. Large distances are clamped to the usable drag band so the gesture stays reliable across Android, iOS, and macOS. Default snapshot text output is visible-first, so off-screen interactive content is summarized instead of shown as tappable refs. -When a target only appears in an off-screen summary, use `scroll ` and then take a fresh `snapshot -i`. For repeated checks, a small shell loop is enough: +When a target only appears in an off-screen summary, use `scroll --settle`: the response waits for the UI to go quiet and returns the diff against the tree you last observed, with fresh refs on the added lines, so no follow-up `snapshot -i` is needed. `back --settle` does the same for navigation. Both are best-effort and never fail the action. For repeated checks without settle, a small shell loop is enough: ```bash previous=''