diff --git a/CONTEXT.md b/CONTEXT.md index fa4346cba7..695050ebbb 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -196,8 +196,8 @@ alone. **Presentation options**: The policy input controlling how one snapshot acquisition becomes a public projection. -**Snapshot presentation facet**: -The host-side owner of the neutral acquisition-to-presentation contract and TypeScript policies. +**Snapshot policy facet**: +The host-side owner of neutral snapshot policy: presentation, freshness, timeout and overlay. Platform acquisition supplies raw facts and a fold policy; runner-side Swift presentation remains separate across the process boundary. diff --git a/docs/adr/0004-ios-snapshot-backend-strategy.md b/docs/adr/0004-ios-snapshot-backend-strategy.md index dcc9895191..0784570f3c 100644 --- a/docs/adr/0004-ios-snapshot-backend-strategy.md +++ b/docs/adr/0004-ios-snapshot-backend-strategy.md @@ -78,10 +78,61 @@ capture-plan tier before the process boundary. The contract fixture under `contracts/fixtures/snapshot-presentation-conformance.json` is the shared proof between those runtimes; it does not imply that Swift and TypeScript share an implementation. -This is the first ownership slice of the Wave 4 debt tracked by #1983. Freshness recovery, -timeout evidence, and screenshot-overlay policy retain their existing daemon adapters until their -neutral host seams are extracted; new consumers must use the facet rather than add another daemon -presentation path. +The same split now holds for the three remaining Wave 4 policies tracked by #1983, so +`src/snapshot/` is the host-side owner of snapshot policy generally rather than of presentation +alone: + +- **Freshness recovery.** The freshness window, the Android staleness classification and its + thresholds, and the retry loop live in `src/snapshot/snapshot-freshness/`. The loop is + parameterized by a classifier and a retry schedule, so "how long may a backend lag behind a real + transition" is a policy input rather than a constant the loop owns. The schedule is stated as a + duration budget; the loop derives the deadline from the window's `markedAt` itself, so the + budget is always spent from the action and a caller has no absolute instant it could get wrong. + `src/daemon/session-snapshot-freshness.ts` keeps only what needs a session: reading and retiring + the window on store-owned `SessionState`, and choosing the comparison baseline from snapshot + lineage. It remains the declared R7 owner of `androidSnapshotFreshness`. +- **Timeout evidence.** Whether a capture failed because the hierarchy never arrived is decided + once, at the deepest boundary that has the evidence, from machine-defined values only: + `snapshot-capture-failure-reason.ts` maps the helper's structured `errorType` field + (`java.util.concurrent.TimeoutException`, by exact equality) and the SIGKILL exit code 137 to + the typed reason `accessibility-timeout` (`ANDROID_CAPTURE_FAILURE_REASONS` in + `@agent-device/contracts/android-snapshot-quality`). The helper-result, session-protocol, and + killed-instrumentation error constructors attach it; every layer above rewraps it rather than + reclassifying. No message shape is consulted anywhere on that path, so rewording helper or + wrapper prose cannot move the reason, and prose that merely reads like a timeout does not become + one — both directions are asserted end to end against the real producer. + `src/snapshot/snapshot-timeout-policy.ts` reads the reason; the human-facing hint is derived + from it rather than decided alongside it. + + The published `details.androidSnapshotTimeoutScreenshot` payload is vocabulary in + `@agent-device/contracts/snapshot-timeout-evidence`, a union whose arms encode which claims can + coexist. The annotated arm carries a non-empty ref tuple, so "annotated with zero refs" is not a + state a caller can build, and no arm stores a ref count: a count beside the refs is a second + source of truth the type system cannot hold in step, so it is derived from the refs instead. + The daemon keeps the ordering that genuinely needs it: resolving a bound screenshot runtime, + writing the artifact, annotating it from the stored observation, and emitting the diagnostics. +- **Screenshot-overlay policy.** Which Android nodes earn an overlay ref, and what rectangle an + overlay for one of them covers, live in `src/snapshot/screenshot-overlay/`. The daemon keeps + approved artifact and ref assembly only: ranking, projection to screenshot pixels, drawing, and + PNG IO. + +`scripts/layering/snapshot-presentation-boundary.test.ts` enforces the direction for the whole +facet: nothing under `src/snapshot/` may import `src/daemon/`. It carries a positive control, +because a filter that stopped matching would look identical to a boundary being obeyed. + +The residual call sites #1983 also named are audited and deliberately left in place. +`src/daemon/direct-ios-selector.ts` carries no presentation policy: `isLocalIosRunnerSession` and +`readSimpleIosSelectorTarget` are session routing (device family, provider ownership, the +stabilization window), while `deriveDirectIosNodeSelector` and `isDirectIosSelectorFallbackError` +are selector derivation and ADR 0011 delegation-on-error. The latter two are pure and +daemon-independent, but their owner would be the selector pipeline governed by R19, not this +facet; moving them under ADR 0004 would widen it to a boundary it does not decide. The +observation and interaction consumers — `selector-capture-runtime.ts`, +`deferred-interaction-outcome.ts`, `snapshot-capture.ts` and +`interaction-touch-android-freshness.ts` — now reach freshness only through the facet or its +session binding. + +New consumers must use the facet rather than add another daemon presentation path. ## Regression Notes diff --git a/fallow-baselines/health.json b/fallow-baselines/health.json index 9d5836c329..04dad3792d 100644 --- a/fallow-baselines/health.json +++ b/fallow-baselines/health.json @@ -112,11 +112,6 @@ "count": 1 } }, - "src/daemon/android-snapshot-freshness.ts": { - "crap_moderate": { - "count": 1 - } - }, "src/daemon/client/daemon-client-lifecycle.ts": { "complexity_high": { "count": 1 @@ -575,7 +570,6 @@ "src/daemon/handlers/session.ts:complexity", "src/replay/script-utils.ts:high impact", "packages/selectors/src/internal/predicates.ts:high impact", - "src/daemon/android-snapshot-freshness.ts:high impact", "src/platforms/boot-diagnostics.ts:complexity", "src/compat/maestro/support.ts:high impact", "src/daemon/session-routing.ts:high impact", diff --git a/packages/contracts/package.json b/packages/contracts/package.json index d708cc4f7b..8d9fccd2d9 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -307,6 +307,10 @@ "types": "./src/snapshot-runtime.ts", "default": "./src/snapshot-runtime.ts" }, + "./snapshot-timeout-evidence": { + "types": "./src/snapshot-timeout-evidence.ts", + "default": "./src/snapshot-timeout-evidence.ts" + }, "./startup-recovery-fence": { "types": "./src/startup-recovery-fence.ts", "default": "./src/startup-recovery-fence.ts" diff --git a/packages/contracts/src/android-snapshot-quality.ts b/packages/contracts/src/android-snapshot-quality.ts index c63ef09255..df699c1317 100644 --- a/packages/contracts/src/android-snapshot-quality.ts +++ b/packages/contracts/src/android-snapshot-quality.ts @@ -33,3 +33,36 @@ export function isUnreadableCaptureContentError(error: unknown): boolean { const reason = details?.androidSnapshotHelperFailureReason; return isAndroidContentRecoveryReason(reason); } + +/** + * Why an Android capture failed at the mechanism level, as a typed reason rather than a message + * shape. `accessibility-timeout` means the hierarchy never arrived — the helper reported a + * structured timeout, or its instrumentation was killed before it could answer. + * + * The producer decides this once, at the platform boundary, and publishes the decision. Readers + * consume the reason instead of re-deriving it from hint text: the hint is human-facing prose + * that may be reworded, and two readers sniffing it will drift apart (#1983). + */ +export const ANDROID_CAPTURE_FAILURE_REASONS = ['accessibility-timeout'] as const; + +export type AndroidCaptureFailureReason = (typeof ANDROID_CAPTURE_FAILURE_REASONS)[number]; + +const ANDROID_CAPTURE_FAILURE_REASON_SET: ReadonlySet = new Set( + ANDROID_CAPTURE_FAILURE_REASONS, +); + +export function isAndroidCaptureFailureReason( + value: unknown, +): value is AndroidCaptureFailureReason { + return typeof value === 'string' && ANDROID_CAPTURE_FAILURE_REASON_SET.has(value); +} + +/** The typed reason a thrown Android capture failure carries, when its producer named one. */ +export function readAndroidCaptureFailureReason( + error: unknown, +): AndroidCaptureFailureReason | undefined { + if (!error || typeof error !== 'object') return undefined; + const details = (error as { details?: Record }).details; + const reason = details?.androidCaptureFailureReason; + return isAndroidCaptureFailureReason(reason) ? reason : undefined; +} diff --git a/packages/contracts/src/facades/platform.ts b/packages/contracts/src/facades/platform.ts index d2c0444a4c..566fb599cf 100644 --- a/packages/contracts/src/facades/platform.ts +++ b/packages/contracts/src/facades/platform.ts @@ -14,11 +14,17 @@ export type { AndroidInputOwnershipSource, } from '../android-input-ownership.ts'; export { + ANDROID_CAPTURE_FAILURE_REASONS, ANDROID_CONTENT_RECOVERY_REASONS, isAndroidContentRecoveryReason, isUnreadableCaptureContentError, + isAndroidCaptureFailureReason, + readAndroidCaptureFailureReason, +} from '../android-snapshot-quality.ts'; +export type { + AndroidCaptureFailureReason, + AndroidContentRecoveryReason, } from '../android-snapshot-quality.ts'; -export type { AndroidContentRecoveryReason } from '../android-snapshot-quality.ts'; export { ANDROID_SYSTEM_CHROME_PACKAGE, hasAndroidSystemChromeProvenance, diff --git a/packages/contracts/src/snapshot-timeout-evidence.test.ts b/packages/contracts/src/snapshot-timeout-evidence.test.ts new file mode 100644 index 0000000000..daef963689 --- /dev/null +++ b/packages/contracts/src/snapshot-timeout-evidence.test.ts @@ -0,0 +1,93 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import type { ScreenshotOverlayRef } from '@agent-device/kernel/snapshot'; +import { + snapshotTimeoutCaptureFailed, + snapshotTimeoutEvidenceOverlayCounts, + snapshotTimeoutEvidenceOverlayFailed, + snapshotTimeoutEvidenceWithOverlayRefs, + snapshotTimeoutEvidenceWithoutOverlaySource, + type SnapshotTimeoutEvidence, +} from './snapshot-timeout-evidence.ts'; + +function overlayRef(ref: string): ScreenshotOverlayRef { + return { + ref, + label: ref, + rect: { x: 0, y: 0, width: 10, height: 10 }, + overlayRect: { x: 0, y: 0, width: 10, height: 10 }, + center: { x: 5, y: 5 }, + }; +} + +test('a failed capture publishes no path and no overlay claim', () => { + const evidence = snapshotTimeoutCaptureFailed('adb screencap exited 1'); + assert.deepEqual(evidence, { captureFailed: true, error: 'adb screencap exited 1' }); + assert.deepEqual(snapshotTimeoutEvidenceOverlayCounts(evidence), { + overlayRefCount: undefined, + overlayRefsAnnotated: undefined, + }); +}); + +test('a capture with no stored observation still discloses that refs were requested', () => { + const evidence = snapshotTimeoutEvidenceWithoutOverlaySource('/tmp/shot.png'); + assert.deepEqual(evidence, { + path: '/tmp/shot.png', + overlayRefsRequested: true, + overlayRefsAnnotated: false, + overlayRefSource: 'unavailable', + }); +}); + +test('the union cannot express an annotated capture with no refs', () => { + // The annotated arm carries a non-empty tuple, so this literal is not assignable. + // `@ts-expect-error` fails the build if it ever becomes valid again. + // @ts-expect-error annotated:true requires at least one overlay ref + const contradiction: SnapshotTimeoutEvidence = { + path: '/tmp/shot.png', + overlayRefsRequested: true, + overlayRefSource: 'session-snapshot', + overlayRefsAnnotated: true, + overlayRefs: [], + }; + assert.ok(contradiction); +}); + +test('the union has no stored count that could disagree with the refs', () => { + // The count is derived, so there is no field to set — a mismatched one is not merely wrong, + // it is unwritable. This guards against reintroducing the second source of truth. + const withStoredCount: SnapshotTimeoutEvidence = { + path: '/tmp/shot.png', + overlayRefsRequested: true, + overlayRefSource: 'session-snapshot', + overlayRefsAnnotated: true, + overlayRefs: [overlayRef('e1')], + // @ts-expect-error overlayRefCount is derived, not stored + overlayRefCount: 0, + }; + assert.ok(withStoredCount); +}); + +test('an empty ref list is a capture that was not annotated, not an annotated one', () => { + const none = snapshotTimeoutEvidenceWithOverlayRefs('/tmp/shot.png', []); + const some = snapshotTimeoutEvidenceWithOverlayRefs('/tmp/shot.png', [overlayRef('e1')]); + assert.deepEqual(snapshotTimeoutEvidenceOverlayCounts(none), { + overlayRefCount: 0, + overlayRefsAnnotated: false, + }); + assert.deepEqual(snapshotTimeoutEvidenceOverlayCounts(some), { + overlayRefCount: 1, + overlayRefsAnnotated: true, + }); +}); + +test('a failed annotation keeps the screenshot and names why the refs are missing', () => { + const evidence = snapshotTimeoutEvidenceOverlayFailed('/tmp/shot.png', 'png decode failed'); + assert.deepEqual(evidence, { + path: '/tmp/shot.png', + overlayRefsRequested: true, + overlayRefsAnnotated: false, + overlayRefSource: 'session-snapshot', + overlayAnnotationError: 'png decode failed', + }); +}); diff --git a/packages/contracts/src/snapshot-timeout-evidence.ts b/packages/contracts/src/snapshot-timeout-evidence.ts new file mode 100644 index 0000000000..d45bf0247e --- /dev/null +++ b/packages/contracts/src/snapshot-timeout-evidence.ts @@ -0,0 +1,121 @@ +import type { ScreenshotOverlayRef } from '@agent-device/kernel/snapshot'; + +/** + * The typed `details.androidSnapshotTimeoutScreenshot` payload published when an accessibility + * snapshot times out and the daemon falls back to a screenshot (#1983). + * + * The shape is a discriminated union rather than a bag of optional fields because each arm is a + * different claim about what evidence exists: no capture at all, a capture with no snapshot to + * annotate from, a capture annotated with overlay refs, or a capture whose annotation failed. + * Callers construct it through the builders below so an assembly site cannot publish a fifth, + * undeclared arm. + */ +type CapturedSnapshotTimeoutEvidenceBase = { + path: string; + overlayRefsRequested: true; +}; + +/** At least one ref. Annotation is a claim that something was drawn, so it cannot be empty. */ +export type NonEmptyScreenshotOverlayRefs = readonly [ + ScreenshotOverlayRef, + ...ScreenshotOverlayRef[], +]; + +/** + * No arm stores a ref count. A count beside the refs is a second source of truth that the type + * system cannot keep in step — `{annotated: true, count: 0, refs: [ref]}` would stay assignable — + * so the count is derived from `overlayRefs` by `snapshotTimeoutEvidenceOverlayCounts` instead. + * The arms that carry no refs have nothing to count, which `overlayRefsAnnotated: false` already + * states. + */ +export type SnapshotTimeoutEvidence = + | { + captureFailed: true; + error: string; + } + | (CapturedSnapshotTimeoutEvidenceBase & { + overlayRefSource: 'unavailable'; + overlayRefsAnnotated: false; + }) + | (CapturedSnapshotTimeoutEvidenceBase & { + overlayRefSource: 'session-snapshot'; + overlayRefsAnnotated: true; + overlayRefs: NonEmptyScreenshotOverlayRefs; + }) + | (CapturedSnapshotTimeoutEvidenceBase & { + overlayRefSource: 'session-snapshot'; + overlayRefsAnnotated: false; + overlayRefs: readonly []; + }) + | (CapturedSnapshotTimeoutEvidenceBase & { + overlayRefSource: 'session-snapshot'; + overlayRefsAnnotated: false; + overlayAnnotationError: string; + }); + +/** No screenshot was taken: the evidence path itself failed. */ +export function snapshotTimeoutCaptureFailed(error: string): SnapshotTimeoutEvidence { + return { captureFailed: true, error }; +} + +/** A screenshot exists, but no stored observation was available to derive overlay refs from. */ +export function snapshotTimeoutEvidenceWithoutOverlaySource(path: string): SnapshotTimeoutEvidence { + return { + path, + overlayRefsRequested: true, + overlayRefsAnnotated: false, + overlayRefSource: 'unavailable', + }; +} + +/** + * A screenshot annotated from the stored observation. An empty ref list is not an annotation, and + * the union says so: the annotated arm carries a non-empty tuple, so `annotated: true` with zero + * refs is not a state a caller can build or a reader has to defend against. + */ +export function snapshotTimeoutEvidenceWithOverlayRefs( + path: string, + overlayRefs: readonly ScreenshotOverlayRef[], +): SnapshotTimeoutEvidence { + const base = { path, overlayRefsRequested: true, overlayRefSource: 'session-snapshot' } as const; + return isNonEmptyOverlayRefs(overlayRefs) + ? { ...base, overlayRefsAnnotated: true, overlayRefs } + : { ...base, overlayRefsAnnotated: false, overlayRefs: [] }; +} + +function isNonEmptyOverlayRefs( + refs: readonly ScreenshotOverlayRef[], +): refs is NonEmptyScreenshotOverlayRefs { + return refs.length > 0; +} + +/** A screenshot exists and a stored observation existed, but annotating it threw. */ +export function snapshotTimeoutEvidenceOverlayFailed( + path: string, + overlayAnnotationError: string, +): SnapshotTimeoutEvidence { + return { + path, + overlayRefsRequested: true, + overlayRefsAnnotated: false, + overlayRefSource: 'session-snapshot', + overlayAnnotationError, + }; +} + +/** + * The overlay counts a diagnostic may report, derived from the refs the evidence actually holds. + * This is the only place a count exists, so it cannot disagree with the arm it came from. + */ +export function snapshotTimeoutEvidenceOverlayCounts(evidence: SnapshotTimeoutEvidence): { + overlayRefCount: number | undefined; + overlayRefsAnnotated: boolean | undefined; +} { + if ('captureFailed' in evidence) { + return { overlayRefCount: undefined, overlayRefsAnnotated: undefined }; + } + return { + overlayRefCount: 'overlayRefs' in evidence ? evidence.overlayRefs.length : 0, + overlayRefsAnnotated: evidence.overlayRefsAnnotated, + }; +} diff --git a/scripts/__tests__/test-file-size-ratchet.test.ts b/scripts/__tests__/test-file-size-ratchet.test.ts index b5a0c20dc7..d7d11d0136 100644 --- a/scripts/__tests__/test-file-size-ratchet.test.ts +++ b/scripts/__tests__/test-file-size-ratchet.test.ts @@ -40,7 +40,7 @@ const PINNED_TEST_FILE_LINES: Readonly> = Object.freeze({ 'src/platforms/apple/core/__tests__/runner-session.test.ts': 2001, 'src/utils/__tests__/daemon-client.test.ts': 1910, 'src/utils/__tests__/output.test.ts': 1861, - 'src/platforms/android/__tests__/snapshot.test.ts': 1495, + 'src/platforms/android/__tests__/snapshot.test.ts': 1445, 'src/platforms/apple/core/__tests__/runner-client.test.ts': 1615, 'src/__tests__/client.test.ts': 1598, 'test/integration/provider-scenarios/android-lifecycle.test.ts': 1559, diff --git a/scripts/layering/package-boundaries.test.ts b/scripts/layering/package-boundaries.test.ts index 1c241652f6..2e27769f8d 100644 --- a/scripts/layering/package-boundaries.test.ts +++ b/scripts/layering/package-boundaries.test.ts @@ -121,6 +121,7 @@ const CONTRACT_EXPORTS = [ '@agent-device/contracts/snapshot', '@agent-device/contracts/snapshot-presentation', '@agent-device/contracts/snapshot-runtime', + '@agent-device/contracts/snapshot-timeout-evidence', '@agent-device/contracts/startup-recovery-fence', '@agent-device/contracts/touch-runtime', '@agent-device/contracts/tv-remote', @@ -703,6 +704,14 @@ test('Node resolution enforces the exports map at runtime', () => { contractsReactNativeOverlayResolved.endsWith('packages/contracts/src/react-native-overlay.ts'), contractsReactNativeOverlayResolved, ); + const contractsSnapshotTimeoutEvidenceResolved = import.meta + .resolve('@agent-device/contracts/snapshot-timeout-evidence'); + assert.ok( + contractsSnapshotTimeoutEvidenceResolved.endsWith( + 'packages/contracts/src/snapshot-timeout-evidence.ts', + ), + contractsSnapshotTimeoutEvidenceResolved, + ); const providerWebDriverResolved = import.meta.resolve('@agent-device/provider-webdriver'); assert.ok( providerWebDriverResolved.endsWith('packages/provider-webdriver/src/index.ts'), diff --git a/scripts/layering/session-state.ts b/scripts/layering/session-state.ts index 97702c89b0..be6cf23c92 100644 --- a/scripts/layering/session-state.ts +++ b/scripts/layering/session-state.ts @@ -47,7 +47,7 @@ export const SESSION_STATE_FIELD_OWNERS: Readonly { +/** + * The host-side snapshot facet (`src/snapshot/`) owns presentation, freshness, timeout and + * overlay policy; `src/daemon/` owns the assembly that orders them (#1983, ADR 0004). The + * dependency runs one way only, so a policy stays testable without standing up a session. + * + * The real tree is clean, which is exactly why the positive control below exists: a filter that + * stopped matching would look identical to a boundary being obeyed. + */ +function daemonImportsFromSnapshotFacet( + sources: ReadonlyMap, + workspaceTargets?: ReadonlyMap, +): string[] { + return resolveImportEdges(sources, workspaceTargets) + .filter( + (edge) => edge.file.startsWith('src/snapshot/') && edge.target.startsWith('src/daemon/'), + ) + .map((edge) => `${edge.file}:${edge.line} -> ${edge.target}`); +} + +test('the snapshot facet cannot import daemon assembly', () => { const files = listTrackedTypeScriptFiles(repoRoot); const sources = new Map( files.map((file) => [file, fs.readFileSync(path.join(repoRoot, file), 'utf8')]), ); - const violations = resolveImportEdges(sources, workspaceSpecifierTargets(repoRoot)).filter( - (edge) => - edge.file.startsWith('src/snapshot/snapshot-presentation/') && - edge.target.startsWith('src/daemon/'), - ); assert.deepEqual( - violations.map((edge) => `${edge.file}:${edge.line} -> ${edge.target}`), + daemonImportsFromSnapshotFacet(sources, workspaceSpecifierTargets(repoRoot)), [], - 'the snapshot presentation facet must not depend on daemon assembly', + 'the host-side snapshot facet must not depend on daemon assembly', ); }); + +test('the boundary reports a facet module that reaches back into the daemon', () => { + const sources = new Map([ + ['src/daemon/types.ts', 'export type SessionState = { id: string };\n'], + [ + 'src/snapshot/snapshot-freshness/android.ts', + "import type { SessionState } from '../../daemon/types.ts';\nexport type X = SessionState;\n", + ], + ]); + + assert.deepEqual(daemonImportsFromSnapshotFacet(sources), [ + 'src/snapshot/snapshot-freshness/android.ts:1 -> src/daemon/types.ts', + ]); +}); diff --git a/src/__tests__/eager-closure-budgets.ts b/src/__tests__/eager-closure-budgets.ts index cb32fede77..0ad8b59993 100644 --- a/src/__tests__/eager-closure-budgets.ts +++ b/src/__tests__/eager-closure-budgets.ts @@ -182,6 +182,7 @@ export const FACADE_BUDGETS: Readonly> = Object.freeze({ 'packages/contracts/src/settings.ts': 3, 'packages/contracts/src/snapshot-presentation.ts': 2, 'packages/contracts/src/snapshot-runtime.ts': 3, + 'packages/contracts/src/snapshot-timeout-evidence.ts': 1, 'packages/contracts/src/startup-recovery-fence.ts': 1, 'packages/contracts/src/tv-remote.ts': 3, 'packages/contracts/src/type-text-runtime.ts': 4, diff --git a/src/daemon/__tests__/post-gesture-stabilization-fixtures.ts b/src/daemon/__tests__/post-gesture-stabilization-fixtures.ts index 1b5dce081f..5e42d60333 100644 --- a/src/daemon/__tests__/post-gesture-stabilization-fixtures.ts +++ b/src/daemon/__tests__/post-gesture-stabilization-fixtures.ts @@ -8,7 +8,7 @@ import type { SessionState } from '../types.ts'; * post-gesture-stabilization-verdict.test.ts (the pure verdict/classifier * coverage — split by subject per #1563 review, to stay under the repo's * 500-line test-file tripwire, AGENTS.md), deferred-interaction-outcome.test.ts, - * and android-snapshot-freshness.test.ts. Not a `.test.ts` file, so vitest + * and session-snapshot-freshness.test.ts. Not a `.test.ts` file, so vitest * never tries to run it directly. */ diff --git a/src/daemon/__tests__/session-snapshot-freshness.test.ts b/src/daemon/__tests__/session-snapshot-freshness.test.ts new file mode 100644 index 0000000000..284cbaccaa --- /dev/null +++ b/src/daemon/__tests__/session-snapshot-freshness.test.ts @@ -0,0 +1,67 @@ +import assert from 'node:assert/strict'; +import { afterEach, test, vi } from 'vitest'; +import type { RawSnapshotNode } from '@agent-device/kernel/snapshot'; +import { makeSnapshotState } from '../../__tests__/test-utils/snapshot-builders.ts'; +import { + clearAndroidSnapshotFreshness, + getActiveAndroidSnapshotFreshness, + markAndroidSnapshotFreshness, +} from '../session-snapshot-freshness.ts'; +import { makeSession } from './post-gesture-stabilization-fixtures.ts'; + +afterEach(() => { + vi.useRealTimers(); +}); + +function labeledNodes(count: number, prefix = 'item'): RawSnapshotNode[] { + return Array.from({ length: count }, (_, index) => ({ + index, + type: 'Button', + label: `${prefix}-${index}`, + })); +} + +// --- mark / get / clear lifecycle --- + +test('mark records a route-comparable baseline only from a comparison-safe snapshot', () => { + const session = makeSession('android'); + session.snapshot = makeSnapshotState(labeledNodes(20), { comparisonSafe: true }); + + markAndroidSnapshotFreshness(session, 'click'); + + assert.equal(session.androidSnapshotFreshness?.routeComparable, true); + assert.equal(session.androidSnapshotFreshness?.baselineSignatures?.length, 20); +}); + +test('mark keeps a pruned baseline usable for count comparison but not route comparison', () => { + const session = makeSession('android'); + session.snapshot = makeSnapshotState(labeledNodes(20)); + + markAndroidSnapshotFreshness(session, 'click'); + + assert.equal(session.androidSnapshotFreshness?.routeComparable, false); + assert.equal(session.androidSnapshotFreshness?.baselineSignatures, undefined); + assert.equal(session.androidSnapshotFreshness?.baselineCount, 20); +}); + +test('mark is Android-only and the active window expires', () => { + vi.useFakeTimers(); + const iosSession = makeSession('ios'); + markAndroidSnapshotFreshness(iosSession, 'click'); + assert.equal(iosSession.androidSnapshotFreshness, undefined); + + const session = makeSession('android'); + markAndroidSnapshotFreshness(session, 'click'); + assert.ok(getActiveAndroidSnapshotFreshness(session)); + + vi.advanceTimersByTime(2_501); + assert.equal(getActiveAndroidSnapshotFreshness(session), undefined); + assert.equal(session.androidSnapshotFreshness, undefined); +}); + +test('clear removes an active freshness window', () => { + const session = makeSession('android'); + markAndroidSnapshotFreshness(session, 'click'); + clearAndroidSnapshotFreshness(session); + assert.equal(session.androidSnapshotFreshness, undefined); +}); diff --git a/src/daemon/android-snapshot-freshness.ts b/src/daemon/android-snapshot-freshness.ts deleted file mode 100644 index b961983a39..0000000000 --- a/src/daemon/android-snapshot-freshness.ts +++ /dev/null @@ -1,259 +0,0 @@ -import { - snapshotCaptureAnnotationsFrom, - type SnapshotCaptureAnnotations, -} from '@agent-device/contracts/capture'; -import type { SnapshotState } from '@agent-device/kernel/snapshot'; -import { sleep } from '../utils/timeouts.ts'; -import type { AndroidSnapshotFreshness, SessionState } from './types.ts'; - -export type { AndroidSnapshotFreshness } from './types.ts'; - -// How long after a navigation-sensitive action (press, click, back, open) to consider -// the Android UI hierarchy potentially stale. Android's UIAutomator dump is async -// and can lag behind real transitions by up to ~2 s; 2.5 s gives a comfortable margin -// while avoiding unnecessary retries for steady-state interactions like typing. -const ANDROID_FRESHNESS_WINDOW_MS = 2_500; -const ANDROID_COMPARISON_BASELINE_MAX_AGE_MS = 5_000; - -// Retry suspicious snapshots until this post-action deadline expires. The delay -// sequence stays short in the happy path; the 600 ms tail retry is opportunistic -// and may be skipped when slower devices spend the budget inside each capture. -const ANDROID_FRESHNESS_RETRY_DEADLINE_MS = 1_500; -const ANDROID_FRESHNESS_RETRY_DELAYS_MS = [250, 400, 600] as const; - -export function markAndroidSnapshotFreshness( - session: SessionState, - action: string, - baseline = session.snapshot, -): void { - if (session.device.platform !== 'android') return; - const comparisonBaseline = resolveAndroidComparisonBaseline(session, baseline); - // Route-stuck recovery only makes sense against a baseline captured in a broad, comparable - // shape. Interactive/scoped/depth-limited snapshots are still useful for users, but they are - // too pruned to serve as a reliable "same route vs new route" baseline. - const routeComparable = comparisonBaseline?.comparisonSafe === true; - session.androidSnapshotFreshness = { - action, - markedAt: Date.now(), - baselineCount: (comparisonBaseline ?? baseline)?.nodes.length ?? 0, - baselineSignatures: routeComparable - ? buildSnapshotSignatures(comparisonBaseline?.nodes ?? []) - : undefined, - routeComparable, - }; -} - -function resolveAndroidComparisonBaseline( - session: SessionState, - baseline: SnapshotState | undefined, -): SnapshotState | undefined { - if (baseline?.comparisonSafe === true) return baseline; - const previous = session.lastComparisonSafeSnapshot; - if (!previous || previous.comparisonSafe !== true) return baseline; - return Date.now() - previous.createdAt <= ANDROID_COMPARISON_BASELINE_MAX_AGE_MS - ? previous - : baseline; -} - -export function getActiveAndroidSnapshotFreshness( - session: SessionState | undefined, -): AndroidSnapshotFreshness | undefined { - if (!session || session.device.platform !== 'android') return undefined; - const freshness = session.androidSnapshotFreshness; - if (!freshness) return undefined; - if (Date.now() - freshness.markedAt > ANDROID_FRESHNESS_WINDOW_MS) { - delete session.androidSnapshotFreshness; - return undefined; - } - return freshness; -} - -export function clearAndroidSnapshotFreshness(session: SessionState | undefined): void { - if (!session || session.device.platform !== 'android') return; - delete session.androidSnapshotFreshness; -} - -export function isNavigationSensitiveAction(command: string): boolean { - // Keep this set intentionally narrow. `type`, `fill`, and generic `swipe` happen far more - // often than real route changes, so marking freshness for them would add retry latency to - // common steady-state loops. We only opt in commands that regularly move to a new screen. - return command === 'press' || command === 'click' || command === 'back' || command === 'open'; -} - -/** - * Route signature of an Android snapshot, from the fields the Android backend actually carries. - * The helper serializes no `role`, `selected`, `checked` or `long-clickable` (declared residue, - * #1832), so a signature keying on them would compare constants and claim discrimination it does - * not have. - */ -export function buildSnapshotSignatures(nodes: SnapshotState['nodes']): string[] { - return nodes.map((node) => - [ - node.depth ?? 0, - node.type ?? '', - node.label ?? '', - node.value ?? '', - node.identifier ?? '', - node.enabled === false ? 'disabled' : 'enabled', - node.hittable === true ? 'hittable' : 'not-hittable', - ].join('|'), - ); -} - -// A snapshot whose node count dropped to ≤20% of the previous capture is likely a -// stale or mid-transition dump. The 12-node floor prevents false positives on -// already-tiny trees where fluctuation is normal. -export function isLikelyStaleSnapshotDrop(previousCount: number, currentCount: number): boolean { - if (previousCount < 12) { - return false; - } - return currentCount <= Math.floor(previousCount * 0.2); -} - -export type AndroidFreshnessReason = 'empty-interactive' | 'sharp-drop' | 'stuck-route'; -export type AndroidFreshnessMode = 'default' | 'ref-refresh'; - -/** - * The whole "is this capture suspicious?" decision, in one place with its - * thresholds: which of the three staleness shapes (if any) the attempt - * matches against an active freshness window. `rawNodeCount` is the capture - * backend's pre-filter node count when it disclosed one. - */ -export function getAndroidFreshnessReason( - attempt: { snapshot: SnapshotState; rawNodeCount: number | undefined }, - freshness: AndroidSnapshotFreshness, - options: { interactiveOnly: boolean; mode?: AndroidFreshnessMode }, -): AndroidFreshnessReason | null { - // When interactive-only filtering produces zero visible nodes from ≥12 raw nodes, - // the dump likely captured a transitional frame. The 12-node floor avoids - // false positives on deliberately minimal screens (splash, loading). - if ( - options.interactiveOnly && - attempt.snapshot.nodes.length === 0 && - attempt.rawNodeCount !== undefined && - attempt.rawNodeCount >= 12 - ) { - return 'empty-interactive'; - } - - if (options.mode === 'ref-refresh') { - return null; - } - - if (isLikelyStaleSnapshotDrop(freshness.baselineCount, attempt.snapshot.nodes.length)) { - return !hasMeaningfulSnapshotContent(attempt.snapshot) ? 'sharp-drop' : null; - } - - return freshness.routeComparable && - isNavigationSensitiveAction(freshness.action) && - isLikelySnapshotStuckOnPreviousRoute(freshness.baselineSignatures, attempt.snapshot.nodes) - ? 'stuck-route' - : null; -} - -/** - * The whole freshness recovery: re-capture past suspicious dumps until the - * capture stops matching a staleness shape or the post-action deadline - * expires, then annotate what happened. The freshness window is cleared only - * when a trustworthy capture was seen; a still-suspicious final attempt keeps - * the window so the next capture can try again, and the annotation discloses - * `staleAfterRetries` to the caller. - */ -export async function captureAndroidFreshnessRecoveredAttempt< - T extends { snapshot: SnapshotState; annotations: SnapshotCaptureAnnotations }, ->( - params: { - session: SessionState | undefined; - interactiveOnly: boolean; - androidFreshnessMode?: AndroidFreshnessMode; - capture: () => Promise; - }, - freshness: AndroidSnapshotFreshness, -): Promise { - const classify = (attempt: T) => - getAndroidFreshnessReason( - { snapshot: attempt.snapshot, rawNodeCount: attempt.annotations.analysis?.rawNodeCount }, - freshness, - { interactiveOnly: params.interactiveOnly, mode: params.androidFreshnessMode }, - ); - - let latest = await params.capture(); - let suspiciousReason = classify(latest); - let retryCount = 0; - const retryUntilMs = freshness.markedAt + ANDROID_FRESHNESS_RETRY_DEADLINE_MS; - - for (const delayMs of ANDROID_FRESHNESS_RETRY_DELAYS_MS) { - if (!suspiciousReason) break; - const remainingMs = retryUntilMs - Date.now(); - if (remainingMs <= 0) break; - await sleep(Math.min(delayMs, remainingMs)); - latest = await params.capture(); - retryCount += 1; - suspiciousReason = classify(latest); - } - - if (!suspiciousReason) { - clearAndroidSnapshotFreshness(params.session); - } - - const freshnessAnnotation = - retryCount > 0 || Boolean(suspiciousReason) - ? { - action: freshness.action, - retryCount, - staleAfterRetries: Boolean(suspiciousReason), - reason: suspiciousReason ?? undefined, - } - : undefined; - return { - ...latest, - annotations: { - ...latest.annotations, - ...snapshotCaptureAnnotationsFrom({ freshness: freshnessAnnotation }), - }, - }; -} - -function hasMeaningfulSnapshotContent(snapshot: SnapshotState): boolean { - return snapshot.nodes.some( - (node) => - node.hittable === true || - Boolean(node.label?.trim()) || - Boolean(node.value?.trim()) || - Boolean(node.identifier?.trim()), - ); -} - -export function isLikelySnapshotStuckOnPreviousRoute( - previousSignatures: string[] | undefined, - currentNodes: SnapshotState['nodes'], -): boolean { - if (!previousSignatures || previousSignatures.length === 0) { - return false; - } - const total = Math.max(previousSignatures.length, currentNodes.length); - // Trees smaller than 12 nodes are too small for meaningful route comparison — - // minor UI updates can produce high overlap percentages by coincidence. - if (total < 12) { - return false; - } - const currentSignatures = buildSnapshotSignatures(currentNodes); - const comparableLength = Math.min(previousSignatures.length, currentSignatures.length); - let unchanged = 0; - for (let index = 0; index < comparableLength; index += 1) { - if (previousSignatures[index] === currentSignatures[index]) { - unchanged += 1; - } - } - const additions = Math.max(0, currentSignatures.length - previousSignatures.length); - const removals = Math.max(0, previousSignatures.length - currentSignatures.length); - // Consider the snapshot "stuck" when ≥90% of nodes are identical and the number of - // additions/removals stays within 15% (or at least 3). These thresholds accommodate - // minor dynamic content (clocks, counters) while still detecting genuine route changes. - const toleratedDelta = Math.max(3, Math.floor(total * 0.15)); - return ( - unchanged >= Math.floor(total * 0.9) && - additions <= toleratedDelta && - removals <= toleratedDelta - ); -} diff --git a/src/daemon/android-snapshot-timeout-evidence.ts b/src/daemon/android-snapshot-timeout-evidence.ts index f7f889e371..f2c57249f3 100644 --- a/src/daemon/android-snapshot-timeout-evidence.ts +++ b/src/daemon/android-snapshot-timeout-evidence.ts @@ -3,8 +3,16 @@ import os from 'node:os'; import path from 'node:path'; import type { DaemonResponse, SessionState } from './types.ts'; import { emitDiagnostic } from '../utils/diagnostics.ts'; -import { AppError, normalizeError, type NormalizedError } from '@agent-device/kernel/errors'; -import type { ScreenshotOverlayRef } from '@agent-device/kernel/snapshot'; +import { AppError, normalizeError } from '@agent-device/kernel/errors'; +import { + snapshotTimeoutCaptureFailed, + snapshotTimeoutEvidenceOverlayCounts, + snapshotTimeoutEvidenceOverlayFailed, + snapshotTimeoutEvidenceWithOverlayRefs, + snapshotTimeoutEvidenceWithoutOverlaySource, + type SnapshotTimeoutEvidence, +} from '@agent-device/contracts/snapshot-timeout-evidence'; +import { isAndroidSnapshotTimeoutError } from '../snapshot/snapshot-timeout-policy.ts'; import { contextFromFlags } from './context.ts'; import { annotateScreenshotWithRefs } from './screenshot-overlay.ts'; import { screenshotExecutionFromContext } from './screenshot-runtime.ts'; @@ -13,34 +21,17 @@ import { type ScreenshotRuntimeBindings, } from './screenshot-runtime-binding.ts'; -type CapturedAndroidSnapshotTimeoutEvidenceBase = { - path: string; - overlayRefsRequested: true; -}; - -type AndroidSnapshotTimeoutEvidence = - | { - captureFailed: true; - error: string; - } - | (CapturedAndroidSnapshotTimeoutEvidenceBase & { - overlayRefSource: 'unavailable'; - overlayRefsAnnotated: false; - overlayRefCount: 0; - }) - | (CapturedAndroidSnapshotTimeoutEvidenceBase & { - overlayRefSource: 'session-snapshot'; - overlayRefsAnnotated: boolean; - overlayRefCount: number; - overlayRefs: ScreenshotOverlayRef[]; - }) - | (CapturedAndroidSnapshotTimeoutEvidenceBase & { - overlayRefSource: 'session-snapshot'; - overlayRefsAnnotated: false; - overlayRefCount: 0; - overlayAnnotationError: string; - }); - +/** + * Daemon assembly for the snapshot-timeout evidence path (#1983). + * + * The two things that are not daemon assembly moved out: whether a failure is the + * accessibility-timeout shape is a policy + * (`src/snapshot/snapshot-timeout-policy.ts`), and the published evidence shape is + * vocabulary (`@agent-device/contracts/snapshot-timeout-evidence`, which has its own subpath so + * it stays out of the shared capture facade's eager closure). What remains here is the ordering that + * genuinely needs the daemon: resolving a bound screenshot runtime, writing the artifact, + * annotating it from the stored observation, and emitting the diagnostics. + */ export async function maybeBuildAndroidSnapshotTimeoutFailure( params: { error: unknown; @@ -74,7 +65,7 @@ async function captureAndroidSnapshotTimeoutEvidence( session: SessionState | undefined; device: SessionState['device']; } & ScreenshotRuntimeBindings, -): Promise { +): Promise { try { const capture = await resolveBoundScreenshotRuntime({ device: params.device, @@ -118,9 +109,7 @@ async function captureAndroidSnapshotTimeoutEvidence( phase: 'android_snapshot_timeout_screenshot_captured', data: { path: screenshotPath, - overlayRefCount: 'overlayRefCount' in evidence ? evidence.overlayRefCount : undefined, - overlayRefsAnnotated: - 'overlayRefsAnnotated' in evidence ? evidence.overlayRefsAnnotated : undefined, + ...snapshotTimeoutEvidenceOverlayCounts(evidence), }, }); return evidence; @@ -131,25 +120,16 @@ async function captureAndroidSnapshotTimeoutEvidence( phase: 'android_snapshot_timeout_screenshot_failed', data: { error: normalized.message }, }); - return { - captureFailed: true, - error: normalized.message, - }; + return snapshotTimeoutCaptureFailed(normalized.message); } } async function annotateAndroidSnapshotTimeoutEvidence( screenshotPath: string, session: SessionState | undefined, -): Promise { +): Promise { if (!session?.snapshot) { - return { - path: screenshotPath, - overlayRefsRequested: true, - overlayRefsAnnotated: false, - overlayRefSource: 'unavailable', - overlayRefCount: 0, - }; + return snapshotTimeoutEvidenceWithoutOverlaySource(screenshotPath); } try { @@ -157,14 +137,7 @@ async function annotateAndroidSnapshotTimeoutEvidence( screenshotPath, snapshot: session.snapshot, }); - return { - path: screenshotPath, - overlayRefsRequested: true, - overlayRefsAnnotated: overlayRefs.length > 0, - overlayRefCount: overlayRefs.length, - overlayRefSource: 'session-snapshot', - overlayRefs, - }; + return snapshotTimeoutEvidenceWithOverlayRefs(screenshotPath, overlayRefs); } catch (error) { const normalized = normalizeError(error); emitDiagnostic({ @@ -172,33 +145,6 @@ async function annotateAndroidSnapshotTimeoutEvidence( phase: 'android_snapshot_timeout_screenshot_overlay_failed', data: { path: screenshotPath, error: normalized.message }, }); - return { - path: screenshotPath, - overlayRefsRequested: true, - overlayRefsAnnotated: false, - overlayRefSource: 'session-snapshot', - overlayRefCount: 0, - overlayAnnotationError: normalized.message, - }; + return snapshotTimeoutEvidenceOverlayFailed(screenshotPath, normalized.message); } } - -function isAndroidSnapshotTimeoutError(error: NormalizedError): boolean { - if (error.code !== 'COMMAND_FAILED') return false; - return ( - hasKnownAndroidSnapshotTimeoutMessage(error) || hasHelperTimeoutDetails(error.details?.helper) - ); -} - -function hasKnownAndroidSnapshotTimeoutMessage(error: NormalizedError): boolean { - const text = `${error.message}\n${error.hint ?? ''}`; - return /Android accessibility snapshots can be blocked/i.test(text); -} - -function hasHelperTimeoutDetails(helper: unknown): boolean { - if (!helper || typeof helper !== 'object') return false; - const helperRecord = helper as Record; - const errorType = String(helperRecord.errorType ?? ''); - const message = String(helperRecord.message ?? ''); - return /TimeoutException/i.test(errorType) || /timed out/i.test(message); -} diff --git a/src/daemon/deferred-interaction-outcome.ts b/src/daemon/deferred-interaction-outcome.ts index 2d83119b34..25c6a8fae7 100644 --- a/src/daemon/deferred-interaction-outcome.ts +++ b/src/daemon/deferred-interaction-outcome.ts @@ -7,10 +7,12 @@ import { captureAndroidFreshnessRecoveredAttempt, clearAndroidSnapshotFreshness, getActiveAndroidSnapshotFreshness, - isNavigationSensitiveAction, markAndroidSnapshotFreshness, - type AndroidFreshnessMode, -} from './android-snapshot-freshness.ts'; +} from './session-snapshot-freshness.ts'; +import { + isNavigationSensitiveAction, + type SnapshotFreshnessMode, +} from '../snapshot/snapshot-freshness/index.ts'; import { withGestureNoEffectWarning } from './gesture-no-effect.ts'; import { areInteractionSurfaceSignaturesStable, @@ -151,7 +153,7 @@ type DeferredOutcomeCaptureParams = { logPath: string; /** Whether the capture the verdict rides on was interactive-only filtered. */ interactiveOnly: boolean; - androidFreshnessMode?: AndroidFreshnessMode; + androidFreshnessMode?: SnapshotFreshnessMode; capture: () => Promise; }; diff --git a/src/daemon/handlers/__tests__/find.test.ts b/src/daemon/handlers/__tests__/find.test.ts index a34ab8cf4a..7d1a80a3ab 100644 --- a/src/daemon/handlers/__tests__/find.test.ts +++ b/src/daemon/handlers/__tests__/find.test.ts @@ -3,7 +3,7 @@ import { handleFindCommands } from '../find.ts'; import { handleInteractionCommands } from '../interaction.ts'; import type { CommandFlags } from '@agent-device/contracts/command'; import type { DaemonRequest, DaemonResponse, SessionState } from '../../types.ts'; -import { buildSnapshotSignatures } from '../../android-snapshot-freshness.ts'; +import { buildSnapshotSignatures } from '../../../snapshot/snapshot-freshness/index.ts'; import { makeSessionStore } from '../../../__tests__/test-utils/store-factory.ts'; import { makeIosSession as makeSession, diff --git a/src/daemon/handlers/__tests__/session-open-existing.test.ts b/src/daemon/handlers/__tests__/session-open-existing.test.ts index 4973d7a18f..4a894646ef 100644 --- a/src/daemon/handlers/__tests__/session-open-existing.test.ts +++ b/src/daemon/handlers/__tests__/session-open-existing.test.ts @@ -1,7 +1,7 @@ import { test, expect } from 'vitest'; import * as os from 'node:os'; import * as path from 'node:path'; -import { buildSnapshotSignatures } from '../../android-snapshot-freshness.ts'; +import { buildSnapshotSignatures } from '../../../snapshot/snapshot-freshness/index.ts'; import { AppError } from '@agent-device/kernel/errors'; import { mockLifecycleDispatch as mockDispatch, diff --git a/src/daemon/handlers/__tests__/snapshot-handler.test.ts b/src/daemon/handlers/__tests__/snapshot-handler.test.ts index 44f262f30e..0748eaef2e 100644 --- a/src/daemon/handlers/__tests__/snapshot-handler.test.ts +++ b/src/daemon/handlers/__tests__/snapshot-handler.test.ts @@ -9,7 +9,7 @@ import { setActiveProviderDeviceRuntimes } from '../../../provider-device-runtim import type { ProviderDeviceRuntime } from '@agent-device/contracts/device'; import type { DaemonResponse, SessionState } from '../../types.ts'; import { AppError } from '@agent-device/kernel/errors'; -import { buildSnapshotSignatures } from '../../android-snapshot-freshness.ts'; +import { buildSnapshotSignatures } from '../../../snapshot/snapshot-freshness/index.ts'; import { buildInteractionSurfaceSignature } from '../../interaction-outcome-policy.ts'; import { buildSnapshotPresentationKey } from '@agent-device/kernel/snapshot'; import { snapshotCliOutput } from '../../../commands/capture/output.ts'; @@ -216,7 +216,7 @@ function androidSnapshotTimeoutError(): AppError { { cmd: 'adb', args: ['shell', 'am', 'instrument'], - timeoutMs: 8000, + androidCaptureFailureReason: 'accessibility-timeout', hint: 'Android accessibility snapshots can be blocked by busy or continuously changing app UI. Use screenshot as visual truth after this timeout.', }, ); @@ -240,7 +240,7 @@ function assertAndroidTimeoutEvidencePayload(evidence: unknown) { expect(record.path).toEqual(expect.stringContaining('snapshot-timeout-overlay-refs.png')); expect(fs.existsSync(record.path as string)).toBe(true); expect(record.overlayRefsAnnotated).toBe(true); - expect(record.overlayRefCount).toBe(1); + expect(record.overlayRefs).toHaveLength(1); expect(record.overlayRefs).toEqual([expect.objectContaining({ ref: 'e1', label: 'Continue' })]); } diff --git a/src/daemon/handlers/interaction-touch-android-freshness.ts b/src/daemon/handlers/interaction-touch-android-freshness.ts index 9dad95dd23..42cbe175ea 100644 --- a/src/daemon/handlers/interaction-touch-android-freshness.ts +++ b/src/daemon/handlers/interaction-touch-android-freshness.ts @@ -1,5 +1,5 @@ import { emitDiagnostic } from '../../utils/diagnostics.ts'; -import { getActiveAndroidSnapshotFreshness } from '../android-snapshot-freshness.ts'; +import { getActiveAndroidSnapshotFreshness } from '../session-snapshot-freshness.ts'; import type { SessionState } from '../types.ts'; import type { InteractionHandlerParams } from './interaction-common.ts'; import type { CaptureSnapshotForSession } from './interaction-snapshot.ts'; diff --git a/src/daemon/handlers/snapshot-capture.ts b/src/daemon/handlers/snapshot-capture.ts index 4e7768ca9f..54367ee6b9 100644 --- a/src/daemon/handlers/snapshot-capture.ts +++ b/src/daemon/handlers/snapshot-capture.ts @@ -16,10 +16,8 @@ import { import { resolveRefLabel } from '../../core/snapshot-node-lookup.ts'; import { captureSnapshotWithInteractor } from './snapshot-interactor-capture.ts'; import { buildSnapshotState } from '../snapshot-state.ts'; -import { - clearAndroidSnapshotFreshness, - type AndroidFreshnessMode, -} from '../android-snapshot-freshness.ts'; +import { clearAndroidSnapshotFreshness } from '../session-snapshot-freshness.ts'; +import type { SnapshotFreshnessMode } from '../../snapshot/snapshot-freshness/index.ts'; import { contextFromFlags } from '../context.ts'; import { resolveDeferredInteractionOutcome } from '../deferred-interaction-outcome.ts'; import type { SessionState } from '../types.ts'; @@ -33,7 +31,7 @@ type CaptureSnapshotParams = { outPath?: string; logPath: string; snapshotScope?: string; - androidFreshnessMode?: AndroidFreshnessMode; + androidFreshnessMode?: SnapshotFreshnessMode; signal?: AbortSignal; /** * Request-bound platform capture. Migrated callers inject the selected diff --git a/src/daemon/screenshot-overlay-draw.ts b/src/daemon/screenshot-overlay-draw.ts index e228a5d7e2..0d0b0ee115 100644 --- a/src/daemon/screenshot-overlay-draw.ts +++ b/src/daemon/screenshot-overlay-draw.ts @@ -1,6 +1,6 @@ import type { Rect, ScreenshotOverlayRef } from '@agent-device/kernel/snapshot'; import type { PNG } from '../utils/png.ts'; -import { clamp } from './screenshot-overlay-rects.ts'; +import { clamp } from '../snapshot/screenshot-overlay/rects.ts'; /** * Rasterizing one overlay ref onto a decoded PNG: border, badge, and the bitmap glyphs the badge diff --git a/src/daemon/screenshot-overlay.ts b/src/daemon/screenshot-overlay.ts index feca236320..1c835721b8 100644 --- a/src/daemon/screenshot-overlay.ts +++ b/src/daemon/screenshot-overlay.ts @@ -13,18 +13,19 @@ import { isViewportRootNode, normalizeType, } from '@agent-device/contracts/snapshot'; -import { resolveAndroidOverlaySourceRect } from './screenshot-overlay-android.ts'; +import { + isAndroidUnlabeledClickableSource, + resolveAndroidOverlaySourceRect, +} from '../snapshot/screenshot-overlay/android.ts'; import { drawOverlayRef } from './screenshot-overlay-draw.ts'; -import { clamp, hasPositiveRect, rectArea, rectContains } from './screenshot-overlay-rects.ts'; +import { + clamp, + hasPositiveRect, + rectArea, + rectContains, +} from '../snapshot/screenshot-overlay/rects.ts'; const MAX_OVERLAY_REFS = 24; -const ANDROID_UNLABELED_CLICKABLE_EXCLUDED_TYPES = [ - 'scroll', - 'list', - 'recyclerview', - 'edittext', - 'textfield', -] as const; const ACTIONABLE_ROLE_TYPES = [ 'button', 'link', @@ -184,21 +185,6 @@ function isOverlaySourceNode( return hasTextSignal && isProxyOverlayNode(node); } -function isAndroidUnlabeledClickableSource( - snapshot: SnapshotState, - snapshotBounds: Rect | null, - node: SnapshotNode, -): boolean { - if (snapshot.backend !== 'android') return false; - if (!node.hittable || !hasPositiveRect(node.rect) || isViewportRootNode(node)) return false; - const normalizedType = normalizeType(node.type ?? ''); - if (ANDROID_UNLABELED_CLICKABLE_EXCLUDED_TYPES.some((type) => normalizedType.includes(type))) { - return false; - } - if (snapshotBounds && rectArea(node.rect) > rectArea(snapshotBounds) * 0.25) return false; - return true; -} - function resolveOverlayTarget( nodes: SnapshotState['nodes'], node: SnapshotNode, diff --git a/src/daemon/selector-capture-runtime.ts b/src/daemon/selector-capture-runtime.ts index 48a242f455..5f78cb2e70 100644 --- a/src/daemon/selector-capture-runtime.ts +++ b/src/daemon/selector-capture-runtime.ts @@ -10,7 +10,7 @@ import type { DaemonRequest, SessionState } from './types.ts'; import { SessionStore } from './session-store.ts'; import { captureSnapshot } from './handlers/snapshot-capture.ts'; import { setSessionSnapshot } from './session-snapshot.ts'; -import { getActiveAndroidSnapshotFreshness } from './android-snapshot-freshness.ts'; +import { getActiveAndroidSnapshotFreshness } from './session-snapshot-freshness.ts'; import { isPostGestureStabilizationPending } from './deferred-interaction-outcome.ts'; import type { BoundSelectorCapture } from './selector-capture-binding.ts'; import { buildRuntimeCaptureInput } from './snapshot-runtime-capture-input.ts'; diff --git a/src/daemon/session-snapshot-freshness.ts b/src/daemon/session-snapshot-freshness.ts new file mode 100644 index 0000000000..42b0fd03b6 --- /dev/null +++ b/src/daemon/session-snapshot-freshness.ts @@ -0,0 +1,107 @@ +import type { SnapshotState } from '@agent-device/kernel/snapshot'; +import { + ANDROID_COMPARISON_BASELINE_MAX_AGE_MS, + ANDROID_FRESHNESS_RETRY_BUDGET_MS, + ANDROID_FRESHNESS_RETRY_DELAYS_MS, + ANDROID_FRESHNESS_WINDOW_MS, + androidFreshnessReason, + buildSnapshotSignatures, + captureFreshnessRecoveredAttempt, + type SnapshotFreshnessAttemptShape, + type SnapshotFreshnessMode, + type SnapshotFreshnessWindow, +} from '../snapshot/snapshot-freshness/index.ts'; +import type { SessionState } from './types.ts'; + +/** + * Session binding for the neutral snapshot-freshness facet (#1983). + * + * The staleness policy, its thresholds and the recovery loop live in + * `src/snapshot/snapshot-freshness/`. What stays here is the part that is genuinely daemon + * assembly: reading and retiring the window that hangs off store-owned `SessionState`, and + * choosing the comparison baseline from session snapshot lineage. This module is the declared + * R7 owner of `SessionState.androidSnapshotFreshness`. + */ +export function markAndroidSnapshotFreshness( + session: SessionState, + action: string, + baseline = session.snapshot, +): void { + if (session.device.platform !== 'android') return; + const comparisonBaseline = resolveAndroidComparisonBaseline(session, baseline); + // Route-stuck recovery only makes sense against a baseline captured in a broad, comparable + // shape. Interactive/scoped/depth-limited snapshots are still useful for users, but they are + // too pruned to serve as a reliable "same route vs new route" baseline. + const routeComparable = comparisonBaseline?.comparisonSafe === true; + session.androidSnapshotFreshness = { + action, + markedAt: Date.now(), + baselineCount: (comparisonBaseline ?? baseline)?.nodes.length ?? 0, + baselineSignatures: routeComparable + ? buildSnapshotSignatures(comparisonBaseline?.nodes ?? []) + : undefined, + routeComparable, + }; +} + +function resolveAndroidComparisonBaseline( + session: SessionState, + baseline: SnapshotState | undefined, +): SnapshotState | undefined { + if (baseline?.comparisonSafe === true) return baseline; + const previous = session.lastComparisonSafeSnapshot; + if (!previous || previous.comparisonSafe !== true) return baseline; + return Date.now() - previous.createdAt <= ANDROID_COMPARISON_BASELINE_MAX_AGE_MS + ? previous + : baseline; +} + +export function getActiveAndroidSnapshotFreshness( + session: SessionState | undefined, +): SnapshotFreshnessWindow | undefined { + if (!session || session.device.platform !== 'android') return undefined; + const freshness = session.androidSnapshotFreshness; + if (!freshness) return undefined; + if (Date.now() - freshness.markedAt > ANDROID_FRESHNESS_WINDOW_MS) { + delete session.androidSnapshotFreshness; + return undefined; + } + return freshness; +} + +export function clearAndroidSnapshotFreshness(session: SessionState | undefined): void { + if (!session || session.device.platform !== 'android') return; + delete session.androidSnapshotFreshness; +} + +/** + * Runs the facet's recovery loop with the Android policy and retry schedule bound to it, and + * retires the session window when a trustworthy capture was seen. + */ +export async function captureAndroidFreshnessRecoveredAttempt< + T extends SnapshotFreshnessAttemptShape, +>( + params: { + session: SessionState | undefined; + interactiveOnly: boolean; + androidFreshnessMode?: SnapshotFreshnessMode; + capture: () => Promise; + }, + freshness: SnapshotFreshnessWindow, +): Promise { + return captureFreshnessRecoveredAttempt({ + capture: params.capture, + classify: (attempt) => + androidFreshnessReason( + { snapshot: attempt.snapshot, rawNodeCount: attempt.annotations.analysis?.rawNodeCount }, + freshness, + { interactiveOnly: params.interactiveOnly, mode: params.androidFreshnessMode }, + ), + window: freshness, + retry: { + retryBudgetMs: ANDROID_FRESHNESS_RETRY_BUDGET_MS, + delaysMs: ANDROID_FRESHNESS_RETRY_DELAYS_MS, + }, + onTrustworthyCapture: () => clearAndroidSnapshotFreshness(params.session), + }); +} diff --git a/src/daemon/types.ts b/src/daemon/types.ts index 0eec1ad4fa..b7ac1150ca 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -17,6 +17,7 @@ import type { import type { DeviceInfo, PlatformSelector } from '@agent-device/kernel/device'; import type { Rect, SnapshotState, SnapshotCaptureBackend } from '@agent-device/kernel/snapshot'; import type { ExecBackgroundResult, ExecResult } from '../utils/exec.ts'; +import type { SnapshotFreshnessWindow } from '../snapshot/snapshot-freshness/index.ts'; // Type-only import; erased at runtime. ref-frame.ts imports SessionState from // here, so this back-edge must stay type-only to avoid a runtime cycle. import type { SnapshotDiagnosticsState } from '@agent-device/contracts/capture'; @@ -158,14 +159,6 @@ export type DaemonRequest = Omit Promise; -export type AndroidSnapshotFreshness = { - action: string; - markedAt: number; - baselineCount: number; - baselineSignatures?: string[]; - routeComparable: boolean; -}; - /** * One node's contribution to an interaction-surface signature. Two comparisons * read this, and they need different things from it, which is why both `key` @@ -339,7 +332,7 @@ export type SessionState = { refFrameGeneration?: number; /** Last broad snapshot safe for Android route-freshness comparisons after interactive snapshots. */ lastComparisonSafeSnapshot?: SnapshotState; - androidSnapshotFreshness?: AndroidSnapshotFreshness; + androidSnapshotFreshness?: SnapshotFreshnessWindow; postGestureStabilization?: PostGestureStabilization; pendingInteractionOutcome?: PendingInteractionOutcome; snapshotDiagnostics?: SnapshotDiagnosticsState; diff --git a/src/platforms/android/__tests__/snapshot-capture-failure-reason.test.ts b/src/platforms/android/__tests__/snapshot-capture-failure-reason.test.ts new file mode 100644 index 0000000000..c861d4244d --- /dev/null +++ b/src/platforms/android/__tests__/snapshot-capture-failure-reason.test.ts @@ -0,0 +1,210 @@ +import { afterEach, beforeEach, test, vi } from 'vitest'; +import assert from 'node:assert/strict'; +import type { DeviceInfo } from '@agent-device/kernel/device'; +import { normalizeError } from '@agent-device/kernel/errors'; +import { isAndroidCaptureFailureReason } from '@agent-device/contracts/android-snapshot-quality'; +import { snapshotAndroid } from '../snapshot.ts'; +import { type AndroidAdbExecutor } from '../snapshot-helper.ts'; +import { ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT } from '../../../__tests__/test-utils/android-snapshot-helper.ts'; +import { + isAndroidHelperRuntimeForceStop as isHelperRuntimeReset, + ANDROID_HELPER_INSTALLED_VERSION_PROBE as installedHelperProbe, +} from './snapshot-helper-session.fixtures.ts'; +import { resetAndroidSnapshotHelperInstallCache } from '../snapshot-helper-install.ts'; +import { resetAndroidSnapshotHelperSessions } from '../snapshot-helper-session-lifecycle.ts'; +import { isAndroidSnapshotTimeoutError } from '../../../snapshot/snapshot-timeout-policy.ts'; + +/** + * The Android capture-failure-reason contract, from producer to consumer. + * + * `snapshotAndroid` is the only place that decides an Android capture failed because the + * accessibility hierarchy never arrived, and it publishes that decision as the typed reason + * `accessibility-timeout`. These tests assert both halves in one place: that the producer tags + * exactly the shapes that mean it, and that the daemon's timeout-evidence policy recognizes the + * value it publishes. Split out of `snapshot.test.ts`, which is over the file-size tripwire. + */ + +vi.mock('../../../utils/exec.ts', async (importOriginal) => ({ + ...(await importOriginal()), + runCmd: vi.fn(), +})); + +vi.mock('../adb.ts', async (importOriginal) => ({ + ...(await importOriginal()), + sleep: vi.fn(async () => {}), +})); + +const device: DeviceInfo = { + platform: 'android', + id: 'emulator-5554', + name: 'Pixel', + kind: 'emulator', + booted: true, +}; + +const helperArtifact = ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT; + +beforeEach(() => { + resetAndroidSnapshotHelperInstallCache(); + resetAndroidSnapshotHelperSessions(); +}); + +afterEach(() => { + vi.clearAllMocks(); +}); + +function snapshotAndroidWithHelper(helperAdb: AndroidAdbExecutor) { + return snapshotAndroid(device, { helperAdb, helperArtifact }); +} + +function createHelperAdb( + handlers: Partial>, +): AndroidAdbExecutor { + return async (args, options) => { + if (args.includes('--show-versioncode')) return installedHelperProbe; + if (isHelperRuntimeReset(args)) return { exitCode: 0, stdout: '', stderr: '' }; + const operation = args.includes('instrument') + ? 'instrument' + : args.includes('dumpsys') && args.includes('activity') + ? 'activity' + : undefined; + const handler = operation ? handlers[operation] : undefined; + if (handler) return await handler(args, options); + throw new Error(`unexpected helper adb args: ${args.join(' ')}`); + }; +} + +test('snapshotAndroid preserves structured helper timeout guidance', async () => { + const helperAdb = createHelperAdb({ + instrument: async () => ({ + exitCode: 1, + stdout: [ + 'INSTRUMENTATION_RESULT: agentDeviceProtocol=android-snapshot-helper-v1', + 'INSTRUMENTATION_RESULT: helperApiVersion=1', + 'INSTRUMENTATION_RESULT: ok=false', + 'INSTRUMENTATION_RESULT: outputFormat=uiautomator-xml', + 'INSTRUMENTATION_RESULT: errorType=java.util.concurrent.TimeoutException', + 'INSTRUMENTATION_RESULT: message=Timed out waiting for accessibility root', + 'INSTRUMENTATION_CODE: 1', + ].join('\n'), + stderr: '', + }), + }); + + await assert.rejects( + () => snapshotAndroidWithHelper(helperAdb), + (error) => { + assert.match((error as Error).message, /Timed out waiting for accessibility root/); + assert.match((error as Error).message, /Android snapshot helper failed/); + const details = (error as { details?: Record }).details; + assert.equal( + details?.hint, + 'Android accessibility snapshots can be blocked by busy or continuously changing app UI. Use screenshot as visual truth after this timeout and report the busy UI if it persists.', + ); + // The typed reason is what readers key on; the hint above is prose derived from it. + assert.equal(details?.androidCaptureFailureReason, 'accessibility-timeout'); + assert.equal(isAndroidCaptureFailureReason(details?.androidCaptureFailureReason), true); + // Producer to consumer: the reason this producer publishes is the one the daemon's + // timeout-evidence policy recognizes. Asserted against the real policy so the two cannot + // drift into separate taxonomies. + assert.equal(isAndroidSnapshotTimeoutError(normalizeError(error)), true); + return true; + }, + ); +}); + +test('snapshotAndroid preserves killed helper instrumentation details', async () => { + const helperAdb = createHelperAdb({ + instrument: async () => ({ exitCode: 137, stdout: '', stderr: '' }), + }); + + await assert.rejects( + () => snapshotAndroidWithHelper(helperAdb), + (error) => { + assert.match( + (error as Error).message, + /Android snapshot helper failed before returning parseable output/, + ); + assert.match((error as Error).message, /Android snapshot helper failed/); + const details = (error as { details?: Record }).details; + assert.equal(details?.exitCode, 137); + // A helper killed before it could answer is the same claim to a reader as a structured + // timeout: the hierarchy never arrived. Both shapes publish the one typed reason. + assert.equal(details?.androidCaptureFailureReason, 'accessibility-timeout'); + assert.equal(isAndroidSnapshotTimeoutError(normalizeError(error)), true); + return true; + }, + ); +}); + +function helperFailure(errorType: string, message: string): AndroidAdbExecutor { + return createHelperAdb({ + instrument: async () => ({ + exitCode: 1, + stdout: [ + 'INSTRUMENTATION_RESULT: agentDeviceProtocol=android-snapshot-helper-v1', + 'INSTRUMENTATION_RESULT: helperApiVersion=1', + 'INSTRUMENTATION_RESULT: ok=false', + `INSTRUMENTATION_RESULT: errorType=${errorType}`, + `INSTRUMENTATION_RESULT: message=${message}`, + 'INSTRUMENTATION_CODE: 1', + ].join('\n'), + stderr: '', + }), + }); +} + +async function classifiedReason(helperAdb: AndroidAdbExecutor): Promise<{ + reason: unknown; + recognized: boolean; +}> { + try { + await snapshotAndroidWithHelper(helperAdb); + } catch (error) { + return { + reason: (error as { details?: Record }).details?.androidCaptureFailureReason, + recognized: isAndroidSnapshotTimeoutError(normalizeError(error)), + }; + } + throw new Error('expected the helper capture to reject'); +} + +/** + * The classification is a value, not a message shape. These two are the whole claim: prose can be + * rewritten in any way without moving the typed reason, and prose that reads exactly like a + * timeout does not produce one when the machine-defined `errorType` says otherwise. + */ +test('rewording the helper message preserves the typed timeout reason', async () => { + for (const message of [ + 'Timed out waiting for accessibility root', + 'the accessibility hierarchy never arrived', + '', + 'ROOT_WAIT_EXCEEDED (code 4)', + ]) { + const outcome = await classifiedReason( + helperFailure('java.util.concurrent.TimeoutException', message), + ); + assert.equal(outcome.reason, 'accessibility-timeout', message); + assert.equal(outcome.recognized, true, message); + } +}); + +test('timeout-looking prose without the machine-defined type is not classified', async () => { + for (const [errorType, message] of [ + ['java.lang.IllegalStateException', 'the request timed out waiting for the window'], + ['java.lang.SecurityException', 'Timed out: permission denied'], + ['android.os.DeadObjectException', 'TimeoutException-like failure'], + ]) { + const outcome = await classifiedReason(helperFailure(errorType!, message!)); + assert.equal(outcome.reason, undefined, errorType); + assert.equal(outcome.recognized, false, errorType); + } +}); + +test('snapshotAndroid does not claim a timeout for an ordinary helper failure', async () => { + const outcome = await classifiedReason( + helperFailure('java.lang.SecurityException', 'Permission denied'), + ); + assert.equal(outcome.reason, undefined); + assert.equal(outcome.recognized, false); +}); diff --git a/src/platforms/android/__tests__/snapshot-helper-session.test.ts b/src/platforms/android/__tests__/snapshot-helper-session.test.ts index a5450a47c3..abde725aa7 100644 --- a/src/platforms/android/__tests__/snapshot-helper-session.test.ts +++ b/src/platforms/android/__tests__/snapshot-helper-session.test.ts @@ -7,6 +7,10 @@ import { createSessionProvider, type FakeAndroidProcess, } from './snapshot-helper-session.fixtures.ts'; +import path from 'node:path'; +import { promises as fs } from 'node:fs'; +import { mkdtempForTest } from '../../../__tests__/test-utils/tmp-dir.ts'; +import { withDiagnosticsScope } from '../../../utils/diagnostics.ts'; beforeEach(async () => { await resetAndroidSnapshotHelperSessions(); @@ -191,3 +195,44 @@ test('invalidates and falls back from the helper session after a malformed respo true, ); }); + +/** + * The session fallback diagnostic must read the typed reason its own protocol published, not + * re-compare the helper's error type. A second comparison is a second taxonomy, and it drifts the + * first time either side changes (#1983). + */ +test('the session fallback diagnostic reads the typed timeout reason', async () => { + const tmpDir = await mkdtempForTest('agent-device-android-session-timeout-diag-'); + const logPath = path.join(tmpDir, 'diag.ndjson'); + try { + const processes: FakeAndroidProcess[] = []; + const provider = createSessionProvider({ + calls: [], + processes, + responseMode: 'ui-automation-timeout', + }); + + await withDiagnosticsScope( + { debug: true, logPath, session: 'android-test', requestId: 'req-1', command: 'snapshot' }, + async () => { + const output = await captureAndroidSnapshotWithHelperSession({ + adb: provider.exec, + adbProvider: provider, + deviceKey: 'android:emulator-5554', + }); + assert.equal(output, undefined); + }, + ); + + const log = await fs.readFile(logPath, 'utf8'); + const fallback = log + .split('\n') + .filter(Boolean) + .map((line) => JSON.parse(line) as { phase?: string; data?: Record }) + .find((event) => event.phase === 'android_snapshot_helper_session_fallback'); + assert.ok(fallback, 'expected a session fallback diagnostic'); + assert.equal(fallback.data?.uiAutomationConnectionTimeout, true); + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }); + } +}); diff --git a/src/platforms/android/__tests__/snapshot.test.ts b/src/platforms/android/__tests__/snapshot.test.ts index 1d4a19106d..04f8557627 100644 --- a/src/platforms/android/__tests__/snapshot.test.ts +++ b/src/platforms/android/__tests__/snapshot.test.ts @@ -1074,56 +1074,6 @@ test('snapshotAndroid emits timeout diagnostics when helper capture times out', assert.match(diagnostics, /helper capture timed out/); }); -test('snapshotAndroid preserves structured helper timeout guidance', async () => { - const helperAdb = createHelperAdb({ - instrument: async () => ({ - exitCode: 1, - stdout: [ - 'INSTRUMENTATION_RESULT: agentDeviceProtocol=android-snapshot-helper-v1', - 'INSTRUMENTATION_RESULT: helperApiVersion=1', - 'INSTRUMENTATION_RESULT: ok=false', - 'INSTRUMENTATION_RESULT: outputFormat=uiautomator-xml', - 'INSTRUMENTATION_RESULT: errorType=java.util.concurrent.TimeoutException', - 'INSTRUMENTATION_RESULT: message=Timed out waiting for accessibility root', - 'INSTRUMENTATION_CODE: 1', - ].join('\n'), - stderr: '', - }), - }); - - await assert.rejects( - () => snapshotAndroidWithHelper(helperAdb), - (error) => { - assert.match((error as Error).message, /Timed out waiting for accessibility root/); - assert.match((error as Error).message, /Android snapshot helper failed/); - assert.equal( - (error as { details?: Record }).details?.hint, - 'Android accessibility snapshots can be blocked by busy or continuously changing app UI. Use screenshot as visual truth after this timeout and report the busy UI if it persists.', - ); - return true; - }, - ); -}); - -test('snapshotAndroid preserves killed helper instrumentation details', async () => { - const helperAdb = createHelperAdb({ - instrument: async () => ({ exitCode: 137, stdout: '', stderr: '' }), - }); - - await assert.rejects( - () => snapshotAndroidWithHelper(helperAdb), - (error) => { - assert.match( - (error as Error).message, - /Android snapshot helper failed before returning parseable output/, - ); - assert.match((error as Error).message, /Android snapshot helper failed/); - assert.equal((error as { details?: Record }).details?.exitCode, 137); - return true; - }, - ); -}); - test('snapshotAndroid fails closed after unparseable helper output', async () => { const calls: string[][] = []; const helperAdb: AndroidAdbExecutor = async (args) => { diff --git a/src/platforms/android/snapshot-capture-failure-reason.ts b/src/platforms/android/snapshot-capture-failure-reason.ts new file mode 100644 index 0000000000..04bb5f4a7f --- /dev/null +++ b/src/platforms/android/snapshot-capture-failure-reason.ts @@ -0,0 +1,39 @@ +import type { AndroidCaptureFailureReason } from '@agent-device/contracts/android-snapshot-quality'; + +/** + * Machine-defined signals that mean an Android capture failed because the accessibility + * hierarchy never arrived, mapped to the typed reason readers consume (#1983). + * + * Both signals are values, not prose. `errorType` is the helper's own Java class name, emitted + * as a structured `INSTRUMENTATION_RESULT` field, and 137 is the shell's SIGKILL exit code. The + * classification therefore survives any rewording of helper or wrapper messages, and prose that + * merely reads like a timeout is not classified as one. + */ +const HELPER_TIMEOUT_ERROR_TYPE = 'java.util.concurrent.TimeoutException'; + +/** `am instrument` killed mid-run (128 + SIGKILL): the helper never got to report anything. */ +const INSTRUMENTATION_KILLED_EXIT_CODE = 137; + +/** The reason a helper result's structured `errorType` names, if any. */ +export function androidCaptureFailureReasonFromHelperResult( + helper: Readonly>, +): AndroidCaptureFailureReason | undefined { + return helper.errorType === HELPER_TIMEOUT_ERROR_TYPE ? 'accessibility-timeout' : undefined; +} + +/** The reason an `am instrument` exit code names, if any. */ +export function androidCaptureFailureReasonFromExitCode( + exitCode: number | undefined, +): AndroidCaptureFailureReason | undefined { + return exitCode === INSTRUMENTATION_KILLED_EXIT_CODE ? 'accessibility-timeout' : undefined; +} + +/** + * The typed reason as an error-details fragment, spreadable into an `AppError`'s details. Absent + * rather than `undefined` when there is no reason, so an unclassified failure carries no key. + */ +export function androidCaptureFailureReasonDetail( + reason: AndroidCaptureFailureReason | undefined, +): { androidCaptureFailureReason?: AndroidCaptureFailureReason } { + return reason ? { androidCaptureFailureReason: reason } : {}; +} diff --git a/src/platforms/android/snapshot-helper-capture.ts b/src/platforms/android/snapshot-helper-capture.ts index f5151a11f2..7c7eef446c 100644 --- a/src/platforms/android/snapshot-helper-capture.ts +++ b/src/platforms/android/snapshot-helper-capture.ts @@ -1,4 +1,9 @@ import { AppError } from '@agent-device/kernel/errors'; +import { + androidCaptureFailureReasonDetail, + androidCaptureFailureReasonFromExitCode, + androidCaptureFailureReasonFromHelperResult, +} from './snapshot-capture-failure-reason.ts'; import { execFailureDetails } from '../../utils/exec.ts'; import { parseInstrumentationRecords, @@ -235,6 +240,9 @@ async function readFallbackHelperOutputOrThrow( stdout: result.stdout, stderr: result.stderr, exitCode: result.exitCode, + ...androidCaptureFailureReasonDetail( + androidCaptureFailureReasonFromExitCode(result.exitCode), + ), }, error, ); @@ -369,6 +377,9 @@ function readFinalHelperResult(records: Array>): Record, requestId: str throw new AppError( 'COMMAND_FAILED', headers.message || headers.errorType || 'Android snapshot helper session returned an error', - { helper: headers }, + { + helper: headers, + ...androidCaptureFailureReasonDetail(androidCaptureFailureReasonFromHelperResult(headers)), + }, ); } } diff --git a/src/platforms/android/snapshot-helper-session.ts b/src/platforms/android/snapshot-helper-session.ts index 662e6ce870..4718a61203 100644 --- a/src/platforms/android/snapshot-helper-session.ts +++ b/src/platforms/android/snapshot-helper-session.ts @@ -5,6 +5,7 @@ */ import { AppError } from '@agent-device/kernel/errors'; import { emitDiagnostic } from '../../utils/diagnostics.ts'; +import { readAndroidCaptureFailureReason } from '@agent-device/contracts/android-snapshot-quality'; import type { AndroidSnapshotHelperCaptureOptions, AndroidSnapshotHelperOutput, @@ -74,20 +75,16 @@ async function captureFromAndroidSnapshotHelperSession(params: { data: { deviceKey, reason: error instanceof Error ? error.message : String(error), - uiAutomationConnectionTimeout: isUiAutomationConnectionTimeoutResponse(error), + // The typed reason the session protocol already published, not a second + // comparison against the helper's error type: one owner, one taxonomy (#1983). + uiAutomationConnectionTimeout: + readAndroidCaptureFailureReason(error) === 'accessibility-timeout', }, }); return undefined; } } -function isUiAutomationConnectionTimeoutResponse(error: unknown): boolean { - if (!(error instanceof AppError)) return false; - const helper = error.details?.helper; - if (!helper || typeof helper !== 'object') return false; - return (helper as Record).errorType === 'java.util.concurrent.TimeoutException'; -} - // Touch commands piggyback on a live snapshot session so gestures do not restart instrumentation // (Android permits one UiAutomation owner). They never start a session: without one, callers use // the same helper APK through a one-shot `am instrument` run instead. diff --git a/src/platforms/android/snapshot.ts b/src/platforms/android/snapshot.ts index b06149453f..1384618877 100644 --- a/src/platforms/android/snapshot.ts +++ b/src/platforms/android/snapshot.ts @@ -52,7 +52,11 @@ import { classifyAndroidHelperContent, type AndroidHelperContentRecoveryDecision, } from './snapshot-content-recovery.ts'; -import type { AndroidContentRecoveryReason } from '@agent-device/contracts/platform'; +import type { + AndroidCaptureFailureReason, + AndroidContentRecoveryReason, +} from '@agent-device/contracts/platform'; +import { readAndroidCaptureFailureReason } from '@agent-device/contracts/android-snapshot-quality'; import { resetAndroidSnapshotHelperRuntime, retireAndroidSnapshotHelperAfterContentFailure, @@ -576,6 +580,7 @@ function formatAndroidSnapshotHelperFailureReason(error: unknown): string { function androidSnapshotHelperCaptureError(error: unknown, reason: string): AppError { const normalized = normalizeError(error); + const captureFailureReason = androidCaptureFailureReasonOf(normalized); return new AppError( toAppErrorCode(normalized.code), `Android snapshot helper failed: ${reason}`, @@ -583,17 +588,35 @@ function androidSnapshotHelperCaptureError(error: unknown, reason: string): AppE ...normalized.details, ...liftedWireFields(normalized), androidSnapshotHelperFailureReason: reason, - hint: androidSnapshotHelperCaptureHint(normalized), + ...(captureFailureReason ? { androidCaptureFailureReason: captureFailureReason } : {}), + hint: androidSnapshotHelperCaptureHint(normalized, captureFailureReason), }, error, ); } -function androidSnapshotHelperCaptureHint(normalized: NormalizedError): string { - const busy = - isStructuredHelperTimeout(normalized.details?.helper, normalized.message) || - isKilledHelperInstrumentationFailure(normalized); - if (busy) { +/** + * The typed reason the capture already carries, if its producer named one (#1983). + * + * This rewraps rather than reclassifies. `snapshot-capture-failure-reason.ts` decides at the + * deepest boundary, from machine-defined values — the helper's structured `errorType`, or the + * SIGKILL exit code — so no message shape is consulted here or below. Rewording helper or + * wrapper prose cannot change the reason, and prose that merely reads like a timeout does not + * become one. + */ +function androidCaptureFailureReasonOf( + normalized: NormalizedError, +): AndroidCaptureFailureReason | undefined { + return readAndroidCaptureFailureReason(normalized); +} + +// The prose is derived from the typed reason rather than deciding alongside it, so rewording a +// hint can never change what a reader concludes about the failure. +function androidSnapshotHelperCaptureHint( + normalized: NormalizedError, + captureFailureReason: AndroidCaptureFailureReason | undefined, +): string { + if (captureFailureReason === 'accessibility-timeout') { return 'Android accessibility snapshots can be blocked by busy or continuously changing app UI. Use screenshot as visual truth after this timeout and report the busy UI if it persists.'; } if (normalized.details?.androidSnapshotHelperInstallFailure === true) { @@ -623,29 +646,12 @@ function liftedWireFields(normalized: NormalizedError): Record; -}): boolean { - if (error.details?.exitCode !== 137) return false; - return /Android snapshot helper (failed before returning parseable output|output could not be parsed)/.test( - error.message, - ); -} - function readHelperMessage(helper: unknown): string | undefined { if (!helper || typeof helper !== 'object' || !('message' in helper)) return undefined; const message = String(helper.message).trim(); return message && message !== 'null' ? message : undefined; } -function isStructuredHelperTimeout(helper: unknown, fallbackMessage: string): boolean { - if (!helper || typeof helper !== 'object') return false; - const errorType = 'errorType' in helper ? String(helper.errorType) : ''; - const message = readHelperMessage(helper) ?? fallbackMessage; - return /TimeoutException/.test(errorType) || /timed out/i.test(message); -} - async function resolveAndroidSnapshotHelperArtifact( explicitArtifact?: AndroidSnapshotHelperArtifact, ): Promise<{ artifact?: AndroidSnapshotHelperArtifact; errorReason?: string }> { diff --git a/src/snapshot/__tests__/screenshot-overlay-android.test.ts b/src/snapshot/__tests__/screenshot-overlay-android.test.ts new file mode 100644 index 0000000000..e21f23a0ee --- /dev/null +++ b/src/snapshot/__tests__/screenshot-overlay-android.test.ts @@ -0,0 +1,135 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import type { SnapshotNode } from '@agent-device/kernel/snapshot'; +import { makeSnapshotState } from '../../__tests__/test-utils/snapshot-builders.ts'; +import { + isAndroidUnlabeledClickableSource, + resolveAndroidOverlaySourceRect, +} from '../screenshot-overlay/android.ts'; + +const SCREEN = { x: 0, y: 0, width: 1080, height: 1920 }; + +function androidSnapshot(nodes: Parameters[0]) { + return makeSnapshotState(nodes, { backend: 'android' }); +} + +const never = () => false; +const always = () => true; + +// --- source classification --- + +test('a hittable unlabeled Android node is an overlay source', () => { + const snapshot = androidSnapshot([ + { + index: 0, + type: 'android.view.ViewGroup', + hittable: true, + rect: { x: 0, y: 0, width: 200, height: 120 }, + }, + ]); + assert.equal(isAndroidUnlabeledClickableSource(snapshot, SCREEN, snapshot.nodes[0]!), true); +}); + +test('scroll containers, lists and text fields are regions rather than controls', () => { + for (const type of [ + 'android.widget.ScrollView', + 'androidx.recyclerview.widget.RecyclerView', + 'android.widget.EditText', + ]) { + const snapshot = androidSnapshot([ + { index: 0, type, hittable: true, rect: { x: 0, y: 0, width: 200, height: 120 } }, + ]); + assert.equal( + isAndroidUnlabeledClickableSource(snapshot, SCREEN, snapshot.nodes[0]!), + false, + type, + ); + } +}); + +test('a node covering more than a quarter of the screen reads as layout', () => { + const snapshot = androidSnapshot([ + { + index: 0, + type: 'android.view.ViewGroup', + hittable: true, + rect: { x: 0, y: 0, width: 1080, height: 960 }, + }, + ]); + assert.equal(isAndroidUnlabeledClickableSource(snapshot, SCREEN, snapshot.nodes[0]!), false); + // Without measured bounds there is nothing to call oversized against. + assert.equal(isAndroidUnlabeledClickableSource(snapshot, null, snapshot.nodes[0]!), true); +}); + +test('the policy is inert on a non-Android backend', () => { + const snapshot = makeSnapshotState( + [{ index: 0, type: 'Button', hittable: true, rect: { x: 0, y: 0, width: 200, height: 120 } }], + { backend: 'xctest' }, + ); + assert.equal(isAndroidUnlabeledClickableSource(snapshot, SCREEN, snapshot.nodes[0]!), false); +}); + +// --- source rect balancing --- + +function actionRow(): SnapshotNode[] { + return androidSnapshot([ + // A tall row whose visual content sits high inside it: 40px above, 120px below. + { + index: 0, + type: 'android.view.ViewGroup', + hittable: true, + rect: { x: 0, y: 0, width: 400, height: 300 }, + }, + { + index: 1, + parentIndex: 0, + type: 'android.widget.TextView', + rect: { x: 20, y: 40, width: 200, height: 60 }, + }, + { + index: 2, + parentIndex: 0, + type: 'android.widget.TextView', + rect: { x: 20, y: 110, width: 200, height: 30 }, + }, + ]).nodes; +} + +test('an action row with lopsided padding is balanced around its content', () => { + const nodes = actionRow(); + const rect = resolveAndroidOverlaySourceRect(nodes[0]!, nodes, never, never); + // Content spans y 40..140; the smaller padding (40 top) is mirrored below it. + assert.deepEqual(rect, { x: 0, y: 0, width: 400, height: 180 }); +}); + +test('a node that already carries a role or label keeps its own rect', () => { + const nodes = actionRow(); + assert.equal(resolveAndroidOverlaySourceRect(nodes[0]!, nodes, always, never), null); + assert.equal(resolveAndroidOverlaySourceRect(nodes[0]!, nodes, never, always), null); +}); + +test('a single content child is not enough to infer a row', () => { + const nodes = androidSnapshot([ + { + index: 0, + type: 'android.view.ViewGroup', + hittable: true, + rect: { x: 0, y: 0, width: 400, height: 300 }, + }, + { + index: 1, + parentIndex: 0, + type: 'android.widget.TextView', + rect: { x: 20, y: 40, width: 200, height: 60 }, + }, + ]).nodes; + assert.equal(resolveAndroidOverlaySourceRect(nodes[0]!, nodes, never, never), null); +}); + +test('a non-hittable or frameless target is never rebalanced', () => { + const nodes = actionRow(); + const notHittable = { ...nodes[0]!, hittable: undefined }; + assert.equal(resolveAndroidOverlaySourceRect(notHittable, nodes, never, never), null); + const frameless = { ...nodes[0]!, rect: undefined }; + assert.equal(resolveAndroidOverlaySourceRect(frameless, nodes, never, never), null); +}); diff --git a/src/snapshot/__tests__/snapshot-freshness-recovery.test.ts b/src/snapshot/__tests__/snapshot-freshness-recovery.test.ts new file mode 100644 index 0000000000..f90a49038d --- /dev/null +++ b/src/snapshot/__tests__/snapshot-freshness-recovery.test.ts @@ -0,0 +1,180 @@ +import assert from 'node:assert/strict'; +import { test, vi } from 'vitest'; +import type { SnapshotCaptureAnnotations } from '@agent-device/contracts/capture'; +import { makeSnapshotState } from '../../__tests__/test-utils/snapshot-builders.ts'; +import { captureFreshnessRecoveredAttempt } from '../snapshot-freshness/recovery.ts'; +import type { + SnapshotFreshnessReason, + SnapshotFreshnessWindow, +} from '../snapshot-freshness/types.ts'; + +type Attempt = { + snapshot: ReturnType; + annotations: SnapshotCaptureAnnotations; +}; + +function attempt(): Attempt { + return { snapshot: makeSnapshotState([]), annotations: {} }; +} + +function windowMarkedAt(markedAt: number): SnapshotFreshnessWindow { + return { action: 'click', markedAt, baselineCount: 50, routeComparable: false }; +} + +/** Classifies the first `suspiciousCount` attempts as stale, then reports trustworthy. */ +function classifierFailingFirst(suspiciousCount: number) { + let seen = 0; + return () => { + seen += 1; + return (seen <= suspiciousCount ? 'sharp-drop' : null) as SnapshotFreshnessReason | null; + }; +} + +test('a trustworthy first capture retires the window and annotates nothing', async () => { + let retired = 0; + let captures = 0; + const result = await captureFreshnessRecoveredAttempt({ + capture: async () => { + captures += 1; + return attempt(); + }, + classify: () => null, + window: windowMarkedAt(Date.now()), + retry: { retryBudgetMs: 1_500, delaysMs: [250, 400, 600] }, + onTrustworthyCapture: () => { + retired += 1; + }, + }); + + assert.equal(captures, 1); + assert.equal(retired, 1); + assert.equal(result.annotations.freshness, undefined); +}); + +test('a suspicious capture that recovers discloses the retry count and retires the window', async () => { + vi.useFakeTimers(); + try { + const markedAt = Date.now(); + let retired = 0; + let captures = 0; + const pending = captureFreshnessRecoveredAttempt({ + capture: async () => { + captures += 1; + return attempt(); + }, + classify: classifierFailingFirst(1), + window: windowMarkedAt(markedAt), + retry: { retryBudgetMs: 1_500, delaysMs: [250, 400, 600] }, + onTrustworthyCapture: () => { + retired += 1; + }, + }); + await vi.advanceTimersByTimeAsync(1_500); + const result = await pending; + + assert.equal(captures, 2); + assert.equal(retired, 1); + // `reason` is carried as an explicit undefined when the capture recovered, which is what the + // pre-facet annotation did too — asserted rather than trimmed so a change to it is visible. + assert.deepEqual(result.annotations.freshness, { + action: 'click', + retryCount: 1, + staleAfterRetries: false, + reason: undefined, + }); + } finally { + vi.useRealTimers(); + } +}); + +test('a capture still suspicious after every delay keeps the window and names the reason', async () => { + vi.useFakeTimers(); + try { + const markedAt = Date.now(); + let retired = 0; + let captures = 0; + const pending = captureFreshnessRecoveredAttempt({ + capture: async () => { + captures += 1; + return attempt(); + }, + classify: () => 'stuck-route' as const, + window: windowMarkedAt(markedAt), + retry: { retryBudgetMs: 1_500, delaysMs: [250, 400, 600] }, + onTrustworthyCapture: () => { + retired += 1; + }, + }); + await vi.advanceTimersByTimeAsync(1_500); + const result = await pending; + + // The initial capture plus all three delays fit inside the 1.5 s budget. + assert.equal(captures, 4); + assert.equal(retired, 0, 'a still-suspicious capture must leave the window standing'); + assert.deepEqual(result.annotations.freshness, { + action: 'click', + retryCount: 3, + staleAfterRetries: true, + reason: 'stuck-route', + }); + } finally { + vi.useRealTimers(); + } +}); + +/** + * The invariant: the deadline is derived from the window's `markedAt` plus the budget, never from + * when the loop happened to start. A window whose budget was already spent before the loop ran + * therefore gets its one capture and no retries. + */ +test('a budget already spent before the loop started runs the capture once and no retries', async () => { + let captures = 0; + const markedAt = Date.now() - 10_000; + const result = await captureFreshnessRecoveredAttempt({ + capture: async () => { + captures += 1; + return attempt(); + }, + classify: () => 'sharp-drop' as const, + window: windowMarkedAt(markedAt), + retry: { retryBudgetMs: 1_500, delaysMs: [250, 400, 600] }, + }); + + assert.equal(captures, 1); + assert.deepEqual(result.annotations.freshness, { + action: 'click', + retryCount: 0, + staleAfterRetries: true, + reason: 'sharp-drop', + }); +}); + +test('the same budget retries or not depending only on how old the window is', async () => { + vi.useFakeTimers(); + try { + const now = Date.now(); + async function retriesFor(markedAt: number): Promise { + let captures = 0; + const pending = captureFreshnessRecoveredAttempt({ + capture: async () => { + captures += 1; + return attempt(); + }, + classify: () => 'sharp-drop' as const, + window: windowMarkedAt(markedAt), + retry: { retryBudgetMs: 1_500, delaysMs: [250, 400, 600] }, + }); + await vi.advanceTimersByTimeAsync(2_000); + await pending; + return captures - 1; + } + + // Identical budget, identical delays — only `markedAt` differs, and that alone decides + // whether any retry fits. A loop that measured its budget from its own start would return + // the same count for both. + assert.equal(await retriesFor(now), 3); + assert.equal(await retriesFor(now - 1_500), 0); + } finally { + vi.useRealTimers(); + } +}); diff --git a/src/daemon/__tests__/android-snapshot-freshness.test.ts b/src/snapshot/__tests__/snapshot-freshness.test.ts similarity index 58% rename from src/daemon/__tests__/android-snapshot-freshness.test.ts rename to src/snapshot/__tests__/snapshot-freshness.test.ts index e31496503e..3510714bd4 100644 --- a/src/daemon/__tests__/android-snapshot-freshness.test.ts +++ b/src/snapshot/__tests__/snapshot-freshness.test.ts @@ -1,22 +1,14 @@ import assert from 'node:assert/strict'; -import { afterEach, test, vi } from 'vitest'; +import { test } from 'vitest'; import type { RawSnapshotNode } from '@agent-device/kernel/snapshot'; import { makeSnapshotState } from '../../__tests__/test-utils/snapshot-builders.ts'; import { + androidFreshnessReason, buildSnapshotSignatures, - clearAndroidSnapshotFreshness, - getActiveAndroidSnapshotFreshness, - getAndroidFreshnessReason, isLikelySnapshotStuckOnPreviousRoute, isLikelyStaleSnapshotDrop, - markAndroidSnapshotFreshness, - type AndroidSnapshotFreshness, -} from '../android-snapshot-freshness.ts'; -import { makeSession } from './post-gesture-stabilization-fixtures.ts'; - -afterEach(() => { - vi.useRealTimers(); -}); +} from '../snapshot-freshness/android.ts'; +import type { SnapshotFreshnessWindow } from '../snapshot-freshness/index.ts'; function labeledNodes(count: number, prefix = 'item'): RawSnapshotNode[] { return Array.from({ length: count }, (_, index) => ({ @@ -30,9 +22,9 @@ function anonymousNodes(count: number): RawSnapshotNode[] { return Array.from({ length: count }, (_, index) => ({ index, type: 'View' })); } -function freshnessRecord( - overrides: Partial = {}, -): AndroidSnapshotFreshness { +function freshnessWindow( + overrides: Partial = {}, +): SnapshotFreshnessWindow { return { action: 'click', markedAt: Date.now(), @@ -42,26 +34,26 @@ function freshnessRecord( }; } -// --- getAndroidFreshnessReason: the whole three-reason classification --- +// --- androidFreshnessReason: the whole three-reason classification --- test('empty interactive capture over a dozen raw nodes reads as a transitional frame', () => { - const reason = getAndroidFreshnessReason( + const reason = androidFreshnessReason( { snapshot: makeSnapshotState([]), rawNodeCount: 12 }, - freshnessRecord(), + freshnessWindow(), { interactiveOnly: true }, ); assert.equal(reason, 'empty-interactive'); }); test('empty interactive capture stays trusted below the raw-node floor or without disclosure', () => { - const belowFloor = getAndroidFreshnessReason( + const belowFloor = androidFreshnessReason( { snapshot: makeSnapshotState([]), rawNodeCount: 11 }, - freshnessRecord({ baselineCount: 0 }), + freshnessWindow({ baselineCount: 0 }), { interactiveOnly: true }, ); - const undisclosed = getAndroidFreshnessReason( + const undisclosed = androidFreshnessReason( { snapshot: makeSnapshotState([]), rawNodeCount: undefined }, - freshnessRecord({ baselineCount: 0 }), + freshnessWindow({ baselineCount: 0 }), { interactiveOnly: true }, ); assert.equal(belowFloor, null); @@ -69,27 +61,27 @@ test('empty interactive capture stays trusted below the raw-node floor or withou }); test('ref-refresh mode only ever reports the empty-interactive shape', () => { - const suppressed = getAndroidFreshnessReason( + const suppressed = androidFreshnessReason( { snapshot: makeSnapshotState(anonymousNodes(3)), rawNodeCount: undefined }, - freshnessRecord({ baselineCount: 50 }), + freshnessWindow({ baselineCount: 50 }), { interactiveOnly: false, mode: 'ref-refresh' }, ); assert.equal(suppressed, null); }); test('a sharp node-count drop with no meaningful content reads as stale', () => { - const reason = getAndroidFreshnessReason( + const reason = androidFreshnessReason( { snapshot: makeSnapshotState(anonymousNodes(3)), rawNodeCount: undefined }, - freshnessRecord({ baselineCount: 50 }), + freshnessWindow({ baselineCount: 50 }), { interactiveOnly: false }, ); assert.equal(reason, 'sharp-drop'); }); test('a sharp drop onto a screen with real content is trusted (deliberately minimal screens)', () => { - const reason = getAndroidFreshnessReason( + const reason = androidFreshnessReason( { snapshot: makeSnapshotState(labeledNodes(3)), rawNodeCount: undefined }, - freshnessRecord({ baselineCount: 50 }), + freshnessWindow({ baselineCount: 50 }), { interactiveOnly: false }, ); assert.equal(reason, null); @@ -98,9 +90,9 @@ test('a sharp drop onto a screen with real content is trusted (deliberately mini test('a near-identical tree after a navigation-sensitive action reads as stuck on the previous route', () => { const nodes = labeledNodes(20); const baseline = makeSnapshotState(nodes); - const reason = getAndroidFreshnessReason( + const reason = androidFreshnessReason( { snapshot: makeSnapshotState(nodes), rawNodeCount: undefined }, - freshnessRecord({ + freshnessWindow({ baselineCount: baseline.nodes.length, baselineSignatures: buildSnapshotSignatures(baseline.nodes), routeComparable: true, @@ -113,14 +105,14 @@ test('a near-identical tree after a navigation-sensitive action reads as stuck o test('stuck-route never fires without a route-comparable baseline or for a non-navigation action', () => { const nodes = labeledNodes(20); const signatures = buildSnapshotSignatures(makeSnapshotState(nodes).nodes); - const notComparable = getAndroidFreshnessReason( + const notComparable = androidFreshnessReason( { snapshot: makeSnapshotState(nodes), rawNodeCount: undefined }, - freshnessRecord({ baselineCount: 20, baselineSignatures: signatures, routeComparable: false }), + freshnessWindow({ baselineCount: 20, baselineSignatures: signatures, routeComparable: false }), { interactiveOnly: false }, ); - const steadyStateAction = getAndroidFreshnessReason( + const steadyStateAction = androidFreshnessReason( { snapshot: makeSnapshotState(nodes), rawNodeCount: undefined }, - freshnessRecord({ + freshnessWindow({ action: 'type', baselineCount: 20, baselineSignatures: signatures, @@ -134,9 +126,9 @@ test('stuck-route never fires without a route-comparable baseline or for a non-n test('a genuinely new route is trusted', () => { const baseline = makeSnapshotState(labeledNodes(20, 'catalog')); - const reason = getAndroidFreshnessReason( + const reason = androidFreshnessReason( { snapshot: makeSnapshotState(labeledNodes(20, 'checkout')), rawNodeCount: undefined }, - freshnessRecord({ + freshnessWindow({ baselineCount: baseline.nodes.length, baselineSignatures: buildSnapshotSignatures(baseline.nodes), routeComparable: true, @@ -161,48 +153,3 @@ test('isLikelySnapshotStuckOnPreviousRoute ignores missing baselines and tiny tr const tiny = makeSnapshotState(labeledNodes(5)).nodes; assert.equal(isLikelySnapshotStuckOnPreviousRoute(buildSnapshotSignatures(tiny), tiny), false); }); - -// --- mark / get / clear lifecycle --- - -test('mark records a route-comparable baseline only from a comparison-safe snapshot', () => { - const session = makeSession('android'); - session.snapshot = makeSnapshotState(labeledNodes(20), { comparisonSafe: true }); - - markAndroidSnapshotFreshness(session, 'click'); - - assert.equal(session.androidSnapshotFreshness?.routeComparable, true); - assert.equal(session.androidSnapshotFreshness?.baselineSignatures?.length, 20); -}); - -test('mark keeps a pruned baseline usable for count comparison but not route comparison', () => { - const session = makeSession('android'); - session.snapshot = makeSnapshotState(labeledNodes(20)); - - markAndroidSnapshotFreshness(session, 'click'); - - assert.equal(session.androidSnapshotFreshness?.routeComparable, false); - assert.equal(session.androidSnapshotFreshness?.baselineSignatures, undefined); - assert.equal(session.androidSnapshotFreshness?.baselineCount, 20); -}); - -test('mark is Android-only and the active window expires', () => { - vi.useFakeTimers(); - const iosSession = makeSession('ios'); - markAndroidSnapshotFreshness(iosSession, 'click'); - assert.equal(iosSession.androidSnapshotFreshness, undefined); - - const session = makeSession('android'); - markAndroidSnapshotFreshness(session, 'click'); - assert.ok(getActiveAndroidSnapshotFreshness(session)); - - vi.advanceTimersByTime(2_501); - assert.equal(getActiveAndroidSnapshotFreshness(session), undefined); - assert.equal(session.androidSnapshotFreshness, undefined); -}); - -test('clear removes an active freshness window', () => { - const session = makeSession('android'); - markAndroidSnapshotFreshness(session, 'click'); - clearAndroidSnapshotFreshness(session); - assert.equal(session.androidSnapshotFreshness, undefined); -}); diff --git a/src/snapshot/__tests__/snapshot-timeout-policy.test.ts b/src/snapshot/__tests__/snapshot-timeout-policy.test.ts new file mode 100644 index 0000000000..52dee64372 --- /dev/null +++ b/src/snapshot/__tests__/snapshot-timeout-policy.test.ts @@ -0,0 +1,55 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import { AppError, normalizeError } from '@agent-device/kernel/errors'; +import { isAndroidSnapshotTimeoutError } from '../snapshot-timeout-policy.ts'; + +function failure(details: Record = {}) { + return normalizeError(new AppError('COMMAND_FAILED', 'Android snapshot helper failed', details)); +} + +test('the typed reason published by the producer is what the policy recognizes', () => { + assert.equal( + isAndroidSnapshotTimeoutError( + failure({ androidCaptureFailureReason: 'accessibility-timeout' }), + ), + true, + ); +}); + +/** + * The prose is human-facing and may be reworded; it carries no decision. A failure that reads + * exactly like a timeout but was never classified as one by its producer is not a timeout here. + */ +test('hint and helper prose alone are not timeout evidence', () => { + const hintOnly = failure({ + hint: 'Android accessibility snapshots can be blocked by a foreground overlay.', + }); + const helperProseOnly = failure({ + helper: { errorType: 'java.util.concurrent.TimeoutException', message: 'timed out' }, + }); + assert.equal(isAndroidSnapshotTimeoutError(hintOnly), false); + assert.equal(isAndroidSnapshotTimeoutError(helperProseOnly), false); +}); + +test('an unclassified or differently classified failure is not a timeout', () => { + assert.equal(isAndroidSnapshotTimeoutError(failure()), false); + assert.equal( + isAndroidSnapshotTimeoutError(failure({ androidCaptureFailureReason: 'something-else' })), + false, + ); + assert.equal( + isAndroidSnapshotTimeoutError( + failure({ androidSnapshotHelperFailureReason: 'system-window-only' }), + ), + false, + ); +}); + +test('the reason is read regardless of the error code its producer chose', () => { + const unsupported = normalizeError( + new AppError('UNSUPPORTED_OPERATION', 'helper unavailable', { + androidCaptureFailureReason: 'accessibility-timeout', + }), + ); + assert.equal(isAndroidSnapshotTimeoutError(unsupported), true); +}); diff --git a/src/daemon/screenshot-overlay-android.ts b/src/snapshot/screenshot-overlay/android.ts similarity index 60% rename from src/daemon/screenshot-overlay-android.ts rename to src/snapshot/screenshot-overlay/android.ts index 4642765874..4ac877886e 100644 --- a/src/daemon/screenshot-overlay-android.ts +++ b/src/snapshot/screenshot-overlay/android.ts @@ -1,6 +1,45 @@ -import type { Rect, SnapshotNode } from '@agent-device/kernel/snapshot'; -import { normalizeType } from '@agent-device/contracts/snapshot'; -import { hasPositiveRect, rectContains, unionRects } from './screenshot-overlay-rects.ts'; +import type { Rect, SnapshotNode, SnapshotState } from '@agent-device/kernel/snapshot'; +import { isViewportRootNode, normalizeType } from '@agent-device/contracts/snapshot'; +import { hasPositiveRect, rectArea, rectContains, unionRects } from './rects.ts'; + +/** + * Android overlay policy (#1983): which Android nodes earn an overlay ref, and what rectangle + * an overlay for one of them should actually cover. + * + * Android reports whole clickable rows as hittable with no role and no label, so both questions + * need Android-specific answers that the shared ranking pass must not have to know about. The + * daemon keeps the ranking, projection and artifact assembly; the classification lives here. + */ + +// A hittable Android node with none of these types is a plausible unlabeled control. Scroll +// containers, lists and text fields are hittable too, but an overlay over one of them marks a +// region rather than a control. +const ANDROID_UNLABELED_CLICKABLE_EXCLUDED_TYPES = [ + 'scroll', + 'list', + 'recyclerview', + 'edittext', + 'textfield', +] as const; + +/** + * Whether an Android node qualifies as an unlabeled clickable overlay source. A node larger + * than a quarter of the snapshot bounds is treated as layout, not as a control. + */ +export function isAndroidUnlabeledClickableSource( + snapshot: SnapshotState, + snapshotBounds: Rect | null, + node: SnapshotNode, +): boolean { + if (snapshot.backend !== 'android') return false; + if (!node.hittable || !hasPositiveRect(node.rect) || isViewportRootNode(node)) return false; + const normalizedType = normalizeType(node.type ?? ''); + if (ANDROID_UNLABELED_CLICKABLE_EXCLUDED_TYPES.some((type) => normalizedType.includes(type))) { + return false; + } + if (snapshotBounds && rectArea(node.rect) > rectArea(snapshotBounds) * 0.25) return false; + return true; +} export function resolveAndroidOverlaySourceRect( target: SnapshotNode, diff --git a/src/daemon/screenshot-overlay-rects.ts b/src/snapshot/screenshot-overlay/rects.ts similarity index 100% rename from src/daemon/screenshot-overlay-rects.ts rename to src/snapshot/screenshot-overlay/rects.ts diff --git a/src/snapshot/snapshot-freshness/android.ts b/src/snapshot/snapshot-freshness/android.ts new file mode 100644 index 0000000000..33e83726c6 --- /dev/null +++ b/src/snapshot/snapshot-freshness/android.ts @@ -0,0 +1,139 @@ +import type { SnapshotState } from '@agent-device/kernel/snapshot'; +import type { + SnapshotFreshnessAttempt, + SnapshotFreshnessReason, + SnapshotFreshnessMode, + SnapshotFreshnessWindow, +} from './types.ts'; + +// How long after a navigation-sensitive action (press, click, back, open) to consider +// the Android UI hierarchy potentially stale. Android's UIAutomator dump is async +// and can lag behind real transitions by up to ~2 s; 2.5 s gives a comfortable margin +// while avoiding unnecessary retries for steady-state interactions like typing. +export const ANDROID_FRESHNESS_WINDOW_MS = 2_500; +export const ANDROID_COMPARISON_BASELINE_MAX_AGE_MS = 5_000; + +// Retry suspicious snapshots until this post-action deadline expires. The delay +// sequence stays short in the happy path; the 600 ms tail retry is opportunistic +// and may be skipped when slower devices spend the budget inside each capture. +export const ANDROID_FRESHNESS_RETRY_BUDGET_MS = 1_500; +export const ANDROID_FRESHNESS_RETRY_DELAYS_MS = [250, 400, 600] as const; + +export function isNavigationSensitiveAction(command: string): boolean { + // Keep this set intentionally narrow. `type`, `fill`, and generic `swipe` happen far more + // often than real route changes, so marking freshness for them would add retry latency to + // common steady-state loops. We only opt in commands that regularly move to a new screen. + return command === 'press' || command === 'click' || command === 'back' || command === 'open'; +} + +/** + * Route signature of an Android snapshot, from the fields the Android backend actually carries. + * The helper serializes no `role`, `selected`, `checked` or `long-clickable` (declared residue, + * #1832), so a signature keying on them would compare constants and claim discrimination it does + * not have. + */ +export function buildSnapshotSignatures(nodes: SnapshotState['nodes']): string[] { + return nodes.map((node) => + [ + node.depth ?? 0, + node.type ?? '', + node.label ?? '', + node.value ?? '', + node.identifier ?? '', + node.enabled === false ? 'disabled' : 'enabled', + node.hittable === true ? 'hittable' : 'not-hittable', + ].join('|'), + ); +} + +// A snapshot whose node count dropped to ≤20% of the previous capture is likely a +// stale or mid-transition dump. The 12-node floor prevents false positives on +// already-tiny trees where fluctuation is normal. +export function isLikelyStaleSnapshotDrop(previousCount: number, currentCount: number): boolean { + if (previousCount < 12) { + return false; + } + return currentCount <= Math.floor(previousCount * 0.2); +} + +/** + * The whole "is this capture suspicious?" decision, in one place with its + * thresholds: which of the three staleness shapes (if any) the attempt + * matches against an active freshness window. `rawNodeCount` is the capture + * backend's pre-filter node count when it disclosed one. + */ +export function androidFreshnessReason( + attempt: SnapshotFreshnessAttempt, + window: SnapshotFreshnessWindow, + options: { interactiveOnly: boolean; mode?: SnapshotFreshnessMode }, +): SnapshotFreshnessReason | null { + // When interactive-only filtering produces zero visible nodes from ≥12 raw nodes, + // the dump likely captured a transitional frame. The 12-node floor avoids + // false positives on deliberately minimal screens (splash, loading). + if ( + options.interactiveOnly && + attempt.snapshot.nodes.length === 0 && + attempt.rawNodeCount !== undefined && + attempt.rawNodeCount >= 12 + ) { + return 'empty-interactive'; + } + + if (options.mode === 'ref-refresh') { + return null; + } + + if (isLikelyStaleSnapshotDrop(window.baselineCount, attempt.snapshot.nodes.length)) { + return !hasMeaningfulSnapshotContent(attempt.snapshot) ? 'sharp-drop' : null; + } + + return window.routeComparable && + isNavigationSensitiveAction(window.action) && + isLikelySnapshotStuckOnPreviousRoute(window.baselineSignatures, attempt.snapshot.nodes) + ? 'stuck-route' + : null; +} + +function hasMeaningfulSnapshotContent(snapshot: SnapshotState): boolean { + return snapshot.nodes.some( + (node) => + node.hittable === true || + Boolean(node.label?.trim()) || + Boolean(node.value?.trim()) || + Boolean(node.identifier?.trim()), + ); +} + +export function isLikelySnapshotStuckOnPreviousRoute( + previousSignatures: string[] | undefined, + currentNodes: SnapshotState['nodes'], +): boolean { + if (!previousSignatures || previousSignatures.length === 0) { + return false; + } + const total = Math.max(previousSignatures.length, currentNodes.length); + // Trees smaller than 12 nodes are too small for meaningful route comparison — + // minor UI updates can produce high overlap percentages by coincidence. + if (total < 12) { + return false; + } + const currentSignatures = buildSnapshotSignatures(currentNodes); + const comparableLength = Math.min(previousSignatures.length, currentSignatures.length); + let unchanged = 0; + for (let index = 0; index < comparableLength; index += 1) { + if (previousSignatures[index] === currentSignatures[index]) { + unchanged += 1; + } + } + const additions = Math.max(0, currentSignatures.length - previousSignatures.length); + const removals = Math.max(0, previousSignatures.length - currentSignatures.length); + // Consider the snapshot "stuck" when ≥90% of nodes are identical and the number of + // additions/removals stays within 15% (or at least 3). These thresholds accommodate + // minor dynamic content (clocks, counters) while still detecting genuine route changes. + const toleratedDelta = Math.max(3, Math.floor(total * 0.15)); + return ( + unchanged >= Math.floor(total * 0.9) && + additions <= toleratedDelta && + removals <= toleratedDelta + ); +} diff --git a/src/snapshot/snapshot-freshness/index.ts b/src/snapshot/snapshot-freshness/index.ts new file mode 100644 index 0000000000..0837ecb5d5 --- /dev/null +++ b/src/snapshot/snapshot-freshness/index.ts @@ -0,0 +1,14 @@ +export { + ANDROID_COMPARISON_BASELINE_MAX_AGE_MS, + ANDROID_FRESHNESS_RETRY_BUDGET_MS, + ANDROID_FRESHNESS_RETRY_DELAYS_MS, + ANDROID_FRESHNESS_WINDOW_MS, + androidFreshnessReason, + buildSnapshotSignatures, + isNavigationSensitiveAction, +} from './android.ts'; +export { + captureFreshnessRecoveredAttempt, + type SnapshotFreshnessAttemptShape, +} from './recovery.ts'; +export type { SnapshotFreshnessMode, SnapshotFreshnessWindow } from './types.ts'; diff --git a/src/snapshot/snapshot-freshness/recovery.ts b/src/snapshot/snapshot-freshness/recovery.ts new file mode 100644 index 0000000000..9e8414c632 --- /dev/null +++ b/src/snapshot/snapshot-freshness/recovery.ts @@ -0,0 +1,83 @@ +import { + snapshotCaptureAnnotationsFrom, + type SnapshotCaptureAnnotations, +} from '@agent-device/contracts/capture'; +import type { SnapshotState } from '@agent-device/kernel/snapshot'; +import { sleep } from '../../utils/timeouts.ts'; +import type { SnapshotFreshnessReason, SnapshotFreshnessWindow } from './types.ts'; + +/** + * How long retries may continue, and what to wait between them. Both are policy inputs rather + * than constants here, because "how long may a backend lag behind a real transition" is a + * property of the acquisition backend, not of the recovery loop. + * + * Both fields are durations. The loop derives the actual deadline from the window's `markedAt` + * itself, so the budget is always spent from the action rather than from whenever the first + * capture happened to return, and a caller has no absolute instant it could get wrong. + */ +export type SnapshotFreshnessRetrySchedule = { + retryBudgetMs: number; + delaysMs: readonly number[]; +}; + +export type SnapshotFreshnessAttemptShape = { + snapshot: SnapshotState; + annotations: SnapshotCaptureAnnotations; +}; + +/** + * The whole freshness recovery: re-capture past suspicious dumps until the + * capture stops matching a staleness shape or the post-action deadline + * expires, then annotate what happened. `onTrustworthyCapture` fires only when + * a trustworthy capture was seen, so the caller can retire its window; a + * still-suspicious final attempt leaves the window standing so the next capture + * can try again, and the annotation discloses `staleAfterRetries` to the caller. + */ +export async function captureFreshnessRecoveredAttempt< + T extends SnapshotFreshnessAttemptShape, +>(params: { + capture: () => Promise; + classify: (attempt: T) => SnapshotFreshnessReason | null; + window: SnapshotFreshnessWindow; + retry: SnapshotFreshnessRetrySchedule; + onTrustworthyCapture?: () => void; +}): Promise { + // Derived here, never accepted from the caller: the budget runs from the action that opened + // the window, so time already spent inside the first capture is time the retries do not get. + const retryUntilMs = params.window.markedAt + params.retry.retryBudgetMs; + + let latest = await params.capture(); + let suspiciousReason = params.classify(latest); + let retryCount = 0; + + for (const delayMs of params.retry.delaysMs) { + if (!suspiciousReason) break; + const remainingMs = retryUntilMs - Date.now(); + if (remainingMs <= 0) break; + await sleep(Math.min(delayMs, remainingMs)); + latest = await params.capture(); + retryCount += 1; + suspiciousReason = params.classify(latest); + } + + if (!suspiciousReason) { + params.onTrustworthyCapture?.(); + } + + const freshnessAnnotation = + retryCount > 0 || Boolean(suspiciousReason) + ? { + action: params.window.action, + retryCount, + staleAfterRetries: Boolean(suspiciousReason), + reason: suspiciousReason ?? undefined, + } + : undefined; + return { + ...latest, + annotations: { + ...latest.annotations, + ...snapshotCaptureAnnotationsFrom({ freshness: freshnessAnnotation }), + }, + }; +} diff --git a/src/snapshot/snapshot-freshness/types.ts b/src/snapshot/snapshot-freshness/types.ts new file mode 100644 index 0000000000..3b7fb6609d --- /dev/null +++ b/src/snapshot/snapshot-freshness/types.ts @@ -0,0 +1,37 @@ +import type { SnapshotState } from '@agent-device/kernel/snapshot'; + +/** + * What a navigation-sensitive action recorded about the tree it was dispatched against, so a + * later capture can be judged against it. + * + * The window carries no platform vocabulary: it is a marked instant, a baseline size, and an + * optional route signature. Which shapes count as suspicious is decided by the `classify` + * callback the recovery loop is bound with, not by the window, so a second backend with an async + * hierarchy dump can reuse the window and the loop without inheriting Android's thresholds. + */ +export type SnapshotFreshnessWindow = { + action: string; + markedAt: number; + baselineCount: number; + baselineSignatures?: string[]; + routeComparable: boolean; +}; + +/** Which staleness shape a capture matched, or `null` when the capture looked trustworthy. */ +export type SnapshotFreshnessReason = 'empty-interactive' | 'sharp-drop' | 'stuck-route'; + +/** + * `ref-refresh` re-reads a known tree to refresh refs rather than to observe a transition, so it + * opts out of the route-comparison shapes while keeping the empty-interactive guard. + */ +export type SnapshotFreshnessMode = 'default' | 'ref-refresh'; + +/** + * One capture judged against a window. `rawNodeCount` is the acquisition backend's pre-filter + * node count when it disclosed one — absent means the backend proved nothing about how much it + * filtered away. + */ +export type SnapshotFreshnessAttempt = { + snapshot: SnapshotState; + rawNodeCount: number | undefined; +}; diff --git a/src/snapshot/snapshot-timeout-policy.ts b/src/snapshot/snapshot-timeout-policy.ts new file mode 100644 index 0000000000..ee8d50442b --- /dev/null +++ b/src/snapshot/snapshot-timeout-policy.ts @@ -0,0 +1,15 @@ +import type { NormalizedError } from '@agent-device/kernel/errors'; +import { readAndroidCaptureFailureReason } from '@agent-device/contracts/android-snapshot-quality'; + +/** + * Whether a failed Android snapshot is the accessibility-timeout shape that the screenshot + * evidence path exists for (#1983). + * + * This reads the typed reason the Android platform boundary published when it classified the + * failure; it does not re-derive that classification. The producer is the only place that knows + * whether the helper answered with a structured timeout or was killed before it could answer, and + * a second reader sniffing hint prose would drift from it the first time the prose is reworded. + */ +export function isAndroidSnapshotTimeoutError(error: NormalizedError): boolean { + return readAndroidCaptureFailureReason(error) === 'accessibility-timeout'; +}