Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/adr/0019-request-bound-platform-runtime.md
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,14 @@ set to read facts — is retired for new units: they may not introduce admission
an already-migrated handler onto facts inspection changes production admission behavior; it is its
own independently reviewed unit with parity evidence, never a side effect of a structural change.

One second bind per request is permitted, and is not the retired idiom: a post-action observation
(`--settle`) whose action use carries no capture admits the `capture-only` plan for its own reads
(#3510). It is not an admission-only bind — it never reads facts through a binding, admits through
the facts probe, and binds only on admission — and where the action's use already required
`captureSnapshot` (`scroll top|bottom`, `scroll --until`) it projects the request's cached device
binding rather than acquiring a second one. It stays capture-only: a second bind that carries a
mutating operation is the execution use the plan should have selected.

Use declarations share one neutral `defineUse`; per-domain currying wrappers add a module per
domain for no information and are not added. Required-only declarations are the default. Declaring
a preferred operation requires a recorded measurement of the fast path's benefit in the unit
Expand Down
1 change: 1 addition & 0 deletions scripts/layering/daemon-layer-manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
"src/daemon/session-state.ts",
"src/daemon/session-store.ts",
"src/daemon/session-tenant-scope.ts",
"src/daemon/settle-observation-capture.ts",
"src/daemon/snapshot-capture.ts",
"src/daemon/snapshot-interactor-capture.ts",
"src/daemon/snapshot-runtime-binding.ts",
Expand Down
143 changes: 115 additions & 28 deletions src/daemon/__tests__/generic-settle.test.ts

Large diffs are not rendered by default.

32 changes: 22 additions & 10 deletions src/daemon/__tests__/ios-comparison-key-fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,41 @@ import {
createIosSnapshotRequest,
iosSnapshotComparisonIdentityKey,
} from '@agent-device/capture-kit/ios-snapshot-planning';
import type { IosSnapshotComparisonIdentity } from '@agent-device/contracts/ios-snapshot';
import { IOS_SYSTEM_SURFACE_HOSTS } from '@agent-device/contracts/ios-system-surface';

/**
* The comparison keys the iOS snapshot route stamps on XCTest captures, built through the production
* key function: an app capture is lineaged to the app, a capture of an in-place system surface (a web
* sign-in sheet) to its HOST, because that is the surface it describes (#2438). Regressions that rely
* on a cross-surface pair being incomparable build both keys here, so they fail if the construction
* ever stops distinguishing them.
* The comparison identity the iOS snapshot route stamps on a capture it served from the XCTest
* runner, built through the production key function: an app capture is lineaged to the app, a
* capture of an in-place system surface (a web sign-in sheet) to its HOST, because that is the
* surface it describes (#2438). Regressions that rely on a cross-surface pair being incomparable
* build both here, so they fail if the construction ever stops distinguishing them. A test that
* needs both halves of one capture — a stored baseline keyed from it, and a capture result carrying
* it — takes the identity rather than rewriting the shape (#3510).
*/
export function appCaptureComparisonIdentity(
deviceId: string,
appBundleId: string,
): IosSnapshotComparisonIdentity {
return runnerComparisonIdentity(`${deviceId}:${appBundleId}`);
}

export function appCaptureComparisonKey(deviceId: string, appBundleId: string): string {
return runnerComparisonKey(`${deviceId}:${appBundleId}`);
return iosSnapshotComparisonIdentityKey(appCaptureComparisonIdentity(deviceId, appBundleId));
}

export function systemSurfaceCaptureComparisonKey(deviceId: string): string {
return runnerComparisonKey(`${deviceId}:${IOS_SYSTEM_SURFACE_HOSTS[0]!.bundleId}`);
return iosSnapshotComparisonIdentityKey(
runnerComparisonIdentity(`${deviceId}:${IOS_SYSTEM_SURFACE_HOSTS[0]!.bundleId}`),
);
}

function runnerComparisonKey(targetId: string): string {
return iosSnapshotComparisonIdentityKey({
function runnerComparisonIdentity(targetId: string): IosSnapshotComparisonIdentity {
return {
producer: 'apple-runner',
intent: 'full',
lineage: { targetId },
presentationKey: buildIosSnapshotPresentationKey(createIosSnapshotRequest()),
residue: [{ kind: 'fallback-source', producer: 'apple-runner' }],
});
};
}
137 changes: 137 additions & 0 deletions src/daemon/__tests__/settle-observation-capture.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import { beforeEach, expect, test, vi } from 'vitest';
import { IOS_SIMULATOR } from '@agent-device/test-kit/device-fixtures';
import { makeIosSession } from './support/session-factories.ts';
import { daemonPlatformServicesFixture } from './platform-services-fixture.ts';
import { selectorCaptureFixture } from './selector-capture-fixture.ts';
import { resolveSettleObservationCapture } from '../settle-observation-capture.ts';
import { emitDiagnostic } from '@agent-device/host-kit/diagnostics';

vi.mock('@agent-device/host-kit/diagnostics', async (importOriginal) => {
const actual = await importOriginal<typeof import('@agent-device/host-kit/diagnostics')>();
return { ...actual, emitDiagnostic: vi.fn(actual.emitDiagnostic) };
});

// #3510: the capture a `--settle` observation polls on a route whose action runtime use carries
// none. The guarantee it owes is producer identity with the command the caller runs next; the one it
// must not break is that settling never fails the action it follows. These pin which plan gets
// admitted, how often, and that every way an observation capture goes away degrades to no capture
// while disclosing its typed reason.

const unavailablePhase = ['settle_observation_capture_unavailable'];

/** Level included: a degrade the caller cannot see must not be info-only, which success drops. */
function disclosures(): { level: string | undefined; data: Record<string, unknown> }[] {
return vi
.mocked(emitDiagnostic)
.mock.calls.flatMap(([event]) =>
event.phase === unavailablePhase[0] && event.data
? [{ level: event.level, data: event.data }]
: [],
);
}

beforeEach(() => {
vi.mocked(emitDiagnostic).mockClear();
});

test('an available plan inspects once and binds once for the whole observation', async () => {
const fixture = selectorCaptureFixture();

const capture = await resolveSettleObservationCapture({
command: 'back',
session: makeIosSession('settle', { appBundleId: 'com.example.app' }),
inspectFacts: fixture.inspectFacts,
platformServices: daemonPlatformServicesFixture(),
bindDevice: fixture.bindDevice,
});
if (!capture) throw new Error('expected an admitted capture');

expect(fixture.inspections).toEqual([IOS_SIMULATOR]);
expect(fixture.binds).toEqual([IOS_SIMULATOR]);

await capture({});
await capture({});
expect(fixture.binds).toEqual([IOS_SIMULATOR]);
expect(fixture.captures).toHaveLength(2);
});

// One rule over the two plan cells: an owner that refuses the observation's plan keeps the loop on
// its own snapshot path, and the refusal is disclosed with the typed fact rather than passing as a
// silently unchanged observation. The first row is the Apple no-tracked-app cell, where the owner's
// own `snapshot` refuses for the same declared reason.
test.each([
{
plan: 'without-active-app',
refusal: { withoutActiveApp: { available: false, reason: 'owner-capability-missing' } },
session: () => makeIosSession('no-app'),
operation: 'captureSnapshotWithoutActiveApp',
reason: 'owner-capability-missing',
},
{
plan: 'active-app',
refusal: { capture: { available: false, reason: 'unsupported-platform-leaf' } },
session: () => makeIosSession('settle', { appBundleId: 'com.example.app' }),
operation: 'captureSnapshot',
reason: 'unsupported-platform-leaf',
},
] as const)(
'a $plan plan the owner refuses binds nothing and discloses its reason',
async ({ refusal, session, operation, reason }) => {
const fixture = selectorCaptureFixture(refusal);

const capture = await resolveSettleObservationCapture({
command: 'back',
session: session(),
inspectFacts: fixture.inspectFacts,
platformServices: daemonPlatformServicesFixture(),
bindDevice: fixture.bindDevice,
});

expect(capture).toBeUndefined();
expect(fixture.binds).toEqual([]);
expect(disclosures()).toEqual([
{ level: 'warn', data: { command: 'back', outcome: 'refused', operation, reason } },
]);
},
);

// The two ways the owner's own machinery can fail the observation after a valid plan. Both are
// best-effort by contract: the command the loop follows must still run, and each is disclosed at
// `warn` — an info-only record is dropped on the success path, and these cells answer `ok: true`.
test.each([
// Apple's inspection reads the host's screen-recording state and re-throws on a cancelled probe.
['admission-probe-failed', Object.assign(new Error('probe cancelled'), { name: 'AbortError' })],
['bind-failed', Object.assign(new Error('gateway closed'), { name: 'AppError' })],
] as const)(
'a %s costs the action nothing and discloses its own failure',
async (outcome, failure) => {
const fixture = selectorCaptureFixture();
const failing =
outcome === 'admission-probe-failed'
? {
inspectFacts: async () => {
throw failure;
},
bindDevice: fixture.bindDevice,
}
: {
inspectFacts: fixture.inspectFacts,
bindDevice: async () => {
throw failure;
},
};

const capture = await resolveSettleObservationCapture({
command: 'back',
session: makeIosSession('settle', { appBundleId: 'com.example.app' }),
platformServices: daemonPlatformServicesFixture(),
...failing,
});

expect(capture).toBeUndefined();
expect(fixture.binds).toEqual([]);
expect(disclosures()).toEqual([
{ level: 'warn', data: { command: 'back', outcome, errorName: failure.name } },
]);
},
);
20 changes: 19 additions & 1 deletion src/daemon/generic-settle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ import {
settleFlagGuardResponse,
} from './interaction/index.ts';
import type { BoundContextFromFlags } from './context.ts';
import type { RuntimeAdmissionBindings } from './request-runtime-binding.ts';
import type { DaemonPlatformServices } from './platform-services.ts';
import { resolveSettleObservationCapture } from './settle-observation-capture.ts';
import { issueSettleRefs } from './session-snapshot.ts';
import type { SessionStore } from './session-store.ts';
import type { DaemonRequest, DaemonResponse } from './daemon-request.ts';
Expand Down Expand Up @@ -66,7 +68,7 @@ type GenericSettleContext = {
sessionStore: SessionStore;
contextFromFlags: BoundContextFromFlags;
platformServices: DaemonPlatformServices;
};
} & RuntimeAdmissionBindings;

/**
* Rejects an orphaned `--settle-quiet` up front (the shared flag grammar), and
Expand Down Expand Up @@ -120,10 +122,25 @@ async function observeSettled(
return refsGeneration === undefined ? observation : { ...observation, refsGeneration };
}

/**
* Resolves the observation's capture before the runtime is constructed, so the loop reads the screen
* through the same producer the caller's next command reads it through (#3510). `back` and

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This overstates the guarantee: admission or binding failure can leave the settle loop with its prior observation instead of reading through that producer. Qualify the guarantee to cases where admission and binding succeed.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/daemon/generic-settle.ts, line 127:

<comment>This overstates the guarantee: admission or binding failure can leave the settle loop with its prior observation instead of reading through that producer. Qualify the guarantee to cases where admission and binding succeed.</comment>

<file context>
@@ -124,15 +124,11 @@ async function observeSettled(
- * admit-and-bind for one operation, served from the gateway's binding cache rather than a second
- * device acquisition, because the observation runtime is built here and holds no reference to the
- * leaf execution's own binding.
+ * through the same producer the caller's next command reads it through (#3510). `back` and
+ * `scroll <direction>` are the cells that need it: their action use is the movement alone.
+ * `scroll top|bottom` and `scroll --until` already require `captureSnapshot` for their between-pass
</file context>
Suggested change
* through the same producer the caller's next command reads it through (#3510). `back` and
* through the request-bound producer when admission and binding succeed (#3510). `back` and

* `scroll <direction>` are the cells that need it: their action use is the movement alone.
* `scroll top|bottom` and `scroll --until` already require `captureSnapshot` for their between-pass
* verification; there this is the request's second admit-and-bind for one operation, served from the
* gateway's binding cache rather than a second device acquisition.
*/
async function createGenericSettleRuntime(
context: GenericSettleContext,
): Promise<Awaited<ReturnType<typeof createInteractionRuntime>> | undefined> {
try {
const boundCapture = await resolveSettleObservationCapture({
command: context.req.command,
session: context.session,
inspectFacts: context.inspectFacts,
platformServices: context.platformServices,
bindDevice: context.bindDevice,
});
return await createInteractionRuntime({
createCommandSurface: context.createCommandSurface,
req: context.req,
Expand All @@ -134,6 +151,7 @@ async function createGenericSettleRuntime(
contextFromFlags: context.contextFromFlags,
platformServices: context.platformServices,
captureSnapshotForSession,
settleObservationCapture: boundCapture,
});
} catch {
return undefined;
Expand Down
12 changes: 7 additions & 5 deletions src/daemon/interaction/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type {
CaptureSnapshotForSession,
FindRouteInput,
InteractionRouteInput,
SettleObservationCaptureInput,
} from './internal/types.ts';

export type { FindRouteInput, InteractionRouteInput } from './internal/types.ts';
Expand Down Expand Up @@ -67,11 +68,12 @@ export const captureSnapshotForSession: CaptureSnapshotForSession = async (
};

export async function createInteractionRuntime(
params: InteractionRouteInput & {
pairedGestureViewport?: Rect;
touchExecutor?: import('../touch-runtime.ts').BoundTouchExecutor;
gestures?: import('../gesture-runtime.ts').BoundGestureExecutor;
},
params: SettleObservationCaptureInput &
InteractionRouteInput & {
pairedGestureViewport?: Rect;
touchExecutor?: import('../touch-runtime.ts').BoundTouchExecutor;
gestures?: import('../gesture-runtime.ts').BoundGestureExecutor;
},
) {
return await createInteractionRuntimeForRoute({
...bindInteractionSession(params),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export function snapshotPayload(
backend: 'tree' | 'queries' | 'private-ax' = 'tree',
) {
return {
backend: 'xctest',
backend: 'xctest' as const,
producer: 'apple-runner' as const,
nodes,
quality: { state: 'healthy' as const, backend },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import fs from 'node:fs';
import path from 'node:path';
import { AppError } from '@agent-device/kernel/errors';
import { buildSnapshotPresentationKey } from '@agent-device/kernel/snapshot';
import { buildSnapshotState } from '@agent-device/capture-kit/snapshot-state';
import { handleInteractionCommands } from '../../index.ts';
import {
makeIosSession,
Expand All @@ -22,6 +23,7 @@ import {
snapshotPayload,
} from './interaction-ios-tap-outcome-fixtures.ts';
import {
appCaptureComparisonIdentity,
appCaptureComparisonKey,
systemSurfaceCaptureComparisonKey,
} from '../../../__tests__/ios-comparison-key-fixture.ts';
Expand Down Expand Up @@ -294,6 +296,80 @@ test('a changed capture from a different iOS backend keeps the tap failure', asy
expect(sessionStore.get(sessionName)?.actions).toHaveLength(0);
});

/**
* #3510: `--settle` on a coordinate press binds the request's own capture, and the pre-action
* baseline the settle plan takes goes through it. The corroboration probe is an observation too, so
* it answers through the same capture AND under the same backend pin: an unkeyed baseline next to a
* keyed capture, or a keyed baseline next to an unkeyed one, is an identity mismatch that would
* report a tap that landed as a recorded failure.
*
* A private-AX baseline is the unkeyed half of that pair: a pinned capture leaves the planned route,
* and an app capture off that route carries no identity (#2438). Dropping the pin whenever a bound
* capture exists would answer it from the bridge and refuse the rescue the pin exists to earn.
*/
test.each([
{
name: 'a request-bound bridge capture corroborates its own keyed baseline',
baseline: () =>
buildSnapshotState(
{ ...snapshotPayload(profileNodes), comparisonIdentity: identity() },
{ snapshotInteractiveOnly: false },
),
after: () => ({ ...snapshotPayload(imageViewerNodes), comparisonIdentity: identity() }),
expectedPreferredBackend: undefined,
},
{
name: 'a private-AX baseline pins the backend of the request-bound capture',
baseline: () =>
buildSnapshotState(snapshotPayload(profileNodes, 'private-ax'), {
snapshotInteractiveOnly: false,
}),
// A pinned capture leaves the planned route, so the runner answers and stamps no identity.
after: () => snapshotPayload(imageViewerNodes, 'private-ax'),
expectedPreferredBackend: 'private-ax',
},
] as const)(
'the corroboration probe answers $name',
async ({ baseline, after, expectedPreferredBackend }) => {
const sessionName = 'ios-corroboration-bound-capture';
const sessionStore = makeSessionStore();
sessionStore.publish(
sessionName,
makeIosSession(sessionName, { appBundleId: 'com.example.app', snapshot: baseline() }),
);
const askedWith: Record<string, unknown>[] = [];
const boundCapture = vi.fn(async () => after());

const corroboration = await corroborateIosTapFailure({
error: new AppError('XCTEST_RECORDED_FAILURE', 'tap failed'),
command: 'click',
requestId: undefined,
flags: {},
ref: sessionStore.lookup(sessionName)!,
sessionStore,
contextFromFlags,
// Mirrors the real seam: one capture ask, answered by the bound capture under the pin.
captureSnapshotForSession: async (_ref, _flags, _store, _context, options) => {
askedWith.push(options as unknown as Record<string, unknown>);
return buildSnapshotState(await options.boundCapture!([] as never), {
snapshotInteractiveOnly: options.interactiveOnly,
});
},
boundCapture: boundCapture as unknown as Parameters<
typeof corroborateIosTapFailure
>[0]['boundCapture'],
});

expect(boundCapture).toHaveBeenCalledTimes(1);
expect(askedWith[0]?.preferredBackend).toBe(expectedPreferredBackend);
expect(corroboration?.warning).toMatch(/post-action accessibility capture changed/);
},
);

function identity() {
return appCaptureComparisonIdentity(IOS_SIMULATOR.id, 'com.example.app');
}

test('a producer or generation switch cannot corroborate a failed tap', async () => {
const sessionName = 'ios-comparison-identity-mismatch';
const sessionStore = makeSessionStore();
Expand Down
Loading
Loading