Skip to content

Commit e168409

Browse files
authored
fix: complete request cancellation propagation (#1709)
* fix: route duration waits through cancellation-aware sleep AI-assisted implementation. The code and validation evidence were reviewed before submission. * refactor: expose shared cancellation-aware wait sleep AI-assisted implementation. The code and validation evidence were reviewed before submission. * fix: forward request cancellation through snapshot runtime AI-assisted implementation. The code and validation evidence were reviewed before submission. * test: cover duration wait cancellation authorities AI-assisted implementation. The code and validation evidence were reviewed before submission. * test: cover snapshot request cancellation propagation AI-assisted implementation. The code and validation evidence were reviewed before submission. * style: normalize wait cancellation test ending AI-assisted cleanup. The remote content was compared byte-for-byte with the reviewed local test file. * style: normalize snapshot cancellation test ending AI-assisted cleanup. The remote content was compared byte-for-byte with the reviewed local test file.
1 parent c7242f8 commit e168409

5 files changed

Lines changed: 154 additions & 9 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import assert from 'node:assert/strict';
2+
import { test } from 'vitest';
3+
import type { AgentDeviceBackend } from '../../../backend.ts';
4+
import { createLocalArtifactAdapter } from '../../../io.ts';
5+
import {
6+
createAgentDevice,
7+
createMemorySessionStore,
8+
localCommandPolicy,
9+
} from '../../../runtime.ts';
10+
11+
type Deferred = {
12+
promise: Promise<void>;
13+
resolve: () => void;
14+
};
15+
16+
function deferred(): Deferred {
17+
let resolve!: () => void;
18+
const promise = new Promise<void>((done) => {
19+
resolve = done;
20+
});
21+
return { promise, resolve };
22+
}
23+
24+
for (const authority of ['runtime', 'command'] as const) {
25+
test(`duration wait observes ${authority} cancellation`, async () => {
26+
const controller = new AbortController();
27+
const sleepStarted = deferred();
28+
const releaseSleep = deferred();
29+
const reason = new Error(`${authority} wait canceled`);
30+
const device = createAgentDevice({
31+
backend: { platform: 'ios' } satisfies AgentDeviceBackend,
32+
artifacts: createLocalArtifactAdapter(),
33+
sessions: createMemorySessionStore(),
34+
policy: localCommandPolicy(),
35+
clock: {
36+
now: () => 0,
37+
sleep: async () => {
38+
sleepStarted.resolve();
39+
await releaseSleep.promise;
40+
},
41+
},
42+
...(authority === 'runtime' ? { signal: controller.signal } : {}),
43+
});
44+
45+
const waiting = device.selectors.wait({
46+
target: { kind: 'sleep', durationMs: 1_000 },
47+
...(authority === 'command' ? { signal: controller.signal } : {}),
48+
});
49+
await sleepStarted.promise;
50+
controller.abort(reason);
51+
releaseSleep.resolve();
52+
53+
await assert.rejects(waiting, (error) => error === reason);
54+
});
55+
}

src/commands/interaction/runtime/selector-wait.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { findNodeByLabel, resolveRefLabel } from './selector-read-utils.ts';
2525
import {
2626
createWaitPolling,
2727
DEFAULT_WAIT_TIMEOUT_MS,
28+
sleepWithWaitCancellation,
2829
type WaitPollDeadline,
2930
waitTimeoutError,
3031
} from './wait-polling.ts';
@@ -178,7 +179,7 @@ export function createSelectorWaitCommands<Runtime extends SelectorWaitRuntime>(
178179
options: WaitCommandOptions,
179180
): Promise<WaitCommandResult> => {
180181
if (options.target.kind === 'sleep') {
181-
await sleep(runtime, options.target.durationMs);
182+
await sleepWithWaitCancellation(runtime, options, options.target.durationMs);
182183
return { kind: 'sleep', waitedMs: options.target.durationMs };
183184
}
184185
if (options.target.kind === 'ref') {
@@ -447,8 +448,3 @@ function backendContext(
447448
metadata: options.metadata,
448449
};
449450
}
450-
451-
async function sleep(runtime: SelectorWaitRuntime, durationMs: number): Promise<void> {
452-
if (runtime.clock) await runtime.clock.sleep(durationMs);
453-
else await new Promise((resolve) => setTimeout(resolve, durationMs));
454-
}

src/commands/interaction/runtime/wait-polling.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,11 @@ export function createWaitPolling(
8787
}),
8888
rethrowIfNeverReadable: unreadable.rethrowIfNeverReadable,
8989
sleepUntilNextPoll: async () =>
90-
await sleepWithinWait(runtime, options, Math.min(WAIT_POLL_INTERVAL_MS, remainingMs())),
90+
await sleepWithWaitCancellation(
91+
runtime,
92+
options,
93+
Math.min(WAIT_POLL_INTERVAL_MS, remainingMs()),
94+
),
9195
timeoutMs,
9296
waitedMs: () => now(runtime) - startedAtMs,
9397
};
@@ -162,7 +166,7 @@ function now(runtime: WaitPollingRuntime): number {
162166
return runtime.clock?.now() ?? Date.now();
163167
}
164168

165-
async function sleepWithinWait(
169+
export async function sleepWithWaitCancellation(
166170
runtime: WaitPollingRuntime,
167171
options: WaitPollingOptions,
168172
durationMs: number,
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import { afterEach, expect, test, vi } from 'vitest';
2+
import { makeAndroidSession } from '../../__tests__/test-utils/session-factories.ts';
3+
import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts';
4+
import {
5+
clearRequestAbortRegistration,
6+
markRequestCanceled,
7+
registerRequestAbort,
8+
} from '../../request/cancel.ts';
9+
import { dispatchSnapshotDiffViaRuntime, dispatchSnapshotViaRuntime } from '../snapshot-runtime.ts';
10+
11+
const dispatchCommandMock = vi.hoisted(() => vi.fn());
12+
13+
vi.mock('../../core/dispatch.ts', async (importOriginal) => {
14+
const actual = await importOriginal<typeof import('../../core/dispatch.ts')>();
15+
return {
16+
...actual,
17+
dispatchCommand: dispatchCommandMock,
18+
};
19+
});
20+
21+
type Deferred = {
22+
promise: Promise<void>;
23+
resolve: () => void;
24+
};
25+
26+
function deferred(): Deferred {
27+
let resolve!: () => void;
28+
const promise = new Promise<void>((done) => {
29+
resolve = done;
30+
});
31+
return { promise, resolve };
32+
}
33+
34+
afterEach(() => {
35+
dispatchCommandMock.mockReset();
36+
});
37+
38+
for (const command of ['snapshot', 'diff snapshot'] as const) {
39+
test(`${command} forwards request cancellation into snapshot dispatch`, async () => {
40+
const sessionName = 'default';
41+
const sessionStore = makeSessionStore('agent-device-snapshot-cancellation-');
42+
sessionStore.set(sessionName, makeAndroidSession(sessionName));
43+
const requestId = `snapshot-cancellation-${command.replace(' ', '-')}`;
44+
const registration = registerRequestAbort(requestId);
45+
if (!registration) throw new Error('expected request abort registration');
46+
const dispatchEntered = deferred();
47+
const releaseDispatch = deferred();
48+
let observedSignal: AbortSignal | undefined;
49+
50+
dispatchCommandMock.mockImplementation(async (...args: unknown[]) => {
51+
const context = args[4] as { signal?: AbortSignal } | undefined;
52+
observedSignal = context?.signal;
53+
dispatchEntered.resolve();
54+
await releaseDispatch.promise;
55+
context?.signal?.throwIfAborted();
56+
return { nodes: [], truncated: false, backend: 'uiautomator' };
57+
});
58+
59+
try {
60+
const input = {
61+
req: {
62+
command: command === 'snapshot' ? 'snapshot' : 'diff',
63+
positionals: command === 'snapshot' ? [] : ['snapshot'],
64+
token: 't',
65+
session: sessionName,
66+
meta: { requestId },
67+
},
68+
sessionName,
69+
logPath: '/tmp/agent-device-snapshot-cancellation.log',
70+
sessionStore,
71+
};
72+
const running =
73+
command === 'snapshot'
74+
? dispatchSnapshotViaRuntime(input)
75+
: dispatchSnapshotDiffViaRuntime(input);
76+
await dispatchEntered.promise;
77+
markRequestCanceled(requestId);
78+
releaseDispatch.resolve();
79+
80+
expect(observedSignal).toBe(registration.controller.signal);
81+
await expect(running).rejects.toBe(registration.controller.signal.reason);
82+
} finally {
83+
releaseDispatch.resolve();
84+
clearRequestAbortRegistration(registration);
85+
}
86+
});
87+
}

src/daemon/snapshot-runtime.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
} from './snapshot-quality-latch.ts';
2828
import { createDaemonRuntimePolicy } from './runtime-policy.ts';
2929
import { createDaemonRuntimeSessionStore } from './runtime-session.ts';
30+
import { getRequestSignal } from '../request/cancel.ts';
3031
import { isInteractiveObservation } from './session-action-recorder.ts';
3132
import { setSnapshotLineage } from './session-snapshot.ts';
3233
import { SessionStore } from './session-store.ts';
@@ -284,6 +285,7 @@ function createSnapshotRuntime(params: {
284285
capturedQuality: params.capturedQuality,
285286
}),
286287
...createDaemonRuntimePolicy('snapshot'),
288+
signal: getRequestSignal(req.meta?.requestId),
287289
sessions: createDaemonRuntimeSessionStore({
288290
sessionName,
289291
getSession: () => sessionStore.get(sessionName),
@@ -394,14 +396,15 @@ function createDaemonSnapshotBackend(params: {
394396
const { req, logPath, session, device, snapshotScope } = params;
395397
return {
396398
platform: publicPlatformString(device),
397-
captureSnapshot: async (_context, options): Promise<BackendSnapshotResult> => {
399+
captureSnapshot: async (context, options): Promise<BackendSnapshotResult> => {
398400
const capture = await captureSnapshot({
399401
device,
400402
session,
401403
flags: req.flags,
402404
outPath: options?.outPath ?? req.flags?.out,
403405
logPath,
404406
snapshotScope,
407+
signal: context.signal,
405408
});
406409
const annotations = snapshotCaptureAnnotationsFrom(capture);
407410
// Feed the latch seam the capture's own verdict: the stored session

0 commit comments

Comments
 (0)