Skip to content

Commit ca2de4d

Browse files
thymikeeclaude
andcommitted
fix(revyl): scale a rotated iOS capture by its longer side, and drop the dead run cancel
A screenshot's source density came from the width alone, which a rotated app interface swaps. The run-cancel fallback in stopRevylSession was unreachable, because every rented session carries the issued session id, so the cancel call, its type and its fixture route go. The shared reader also treats a half-filled screen box as invalid and reads role_description as scalar text. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 86c4c7f commit ca2de4d

11 files changed

Lines changed: 50 additions & 76 deletions

File tree

‎packages/capture-kit/src/ios-ax-tree.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,12 @@ test('without a usable root the device screen is the viewport, and a bad screen
9898
),
9999
).toEqual({ kind: 'derived', rect: { x: 0, y: 0, width: 320, height: 240 } });
100100
});
101+
102+
test('a screen box with one dimension is invalid, and a null role description is omitted', () => {
103+
expect(resolveIosAxTreeViewport([{ type: 'Application' }], { width: 440 })).toEqual({
104+
kind: 'missing',
105+
reason: 'invalid',
106+
});
107+
const [node] = flattenIosTree({ type: 'Button', role_description: null });
108+
expect(node?.roleDescription).toBeUndefined();
109+
});

‎packages/capture-kit/src/ios-ax-tree.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export type IosTreeNode = {
2525
role?: string;
2626
subrole?: string | null;
2727
selected?: boolean;
28-
role_description?: string;
28+
role_description?: IosTextAttribute;
2929
hittable?: boolean;
3030
traits?: string[];
3131
pid?: number;
@@ -65,7 +65,7 @@ function mapIosNode(
6565
selected: readIosNodeSelected(node),
6666
password: node.subrole === 'AXSecureTextField' ? true : undefined,
6767
heading: node.traits?.includes('Header') === true ? true : undefined,
68-
roleDescription: node.role_description,
68+
roleDescription: scalarText(node.role_description),
6969
hittable: node.hittable,
7070
depth: options.depth,
7171
parentIndex: options.parentIndex,
@@ -155,7 +155,7 @@ export function resolveIosAxTreeViewport(
155155
(Array.isArray(tree) ? tree : [tree]).map(viewportRoot),
156156
);
157157
if (evidence?.kind === 'reported') return { kind: 'derived', rect: evidence.rect };
158-
if (screen?.width !== undefined && screen.height !== undefined) {
158+
if (screen !== undefined && (screen.width !== undefined || screen.height !== undefined)) {
159159
return isFiniteNumber(screen.width) &&
160160
isFiniteNumber(screen.height) &&
161161
screen.width > 0 &&

‎packages/provider-revyl/src/interactor.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,24 @@ test('an iOS screenshot is rescaled from its native scale to the requested densi
520520
assert.deepEqual(await readPngSize(path.join(directory, 'native.png')), { width: 6, height: 6 });
521521
});
522522

523+
test('an iOS screenshot of an interface rotated since health keeps its aspect at the right scale', async () => {
524+
// health read a 2x4-point portrait screen; the app has since rotated, so the 3x capture is 12x6.
525+
const { interactor } = await liveInteractor('ios', {
526+
screen: { width: 2, height: 4 },
527+
respond: {
528+
'GET /api/v1/execution/device-proxy/run-1/screenshot': () => ({
529+
contentType: 'image/png',
530+
body: [...PNG.sync.write(new PNG({ width: 12, height: 6 }))],
531+
}),
532+
},
533+
});
534+
const outPath = path.join(await mkdtempForTest('agent-device-revyl-shot-rotated-'), 'shot.png');
535+
536+
await interactor.screenshot(outPath);
537+
538+
assert.deepEqual(await readPngSize(outPath), { width: 4, height: 2 });
539+
});
540+
523541
test('two captures of one path each keep their own temporary file', async () => {
524542
// Both writes land in the same directory with the same base name. If they shared a temporary sibling, the
525543
// first rename would move it away and the second would fail with ENOENT on a file that is no longer there.

‎packages/provider-revyl/src/interactor.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ export function createRevylInteractor(session: RevylSession, signal?: AbortSigna
369369
/** Rescales an iOS capture from the density its point size implies to the one the caller asked for. */
370370
async function normalizeRevylIosDensity(
371371
file: string,
372-
screen: { width: number } | undefined,
372+
screen: { width: number; height: number } | undefined,
373373
pixelDensity: number | undefined,
374374
): Promise<void> {
375375
// The host reports the requested density for every iOS simulator capture, so an image whose source
@@ -388,7 +388,9 @@ async function normalizeRevylIosDensity(
388388
import('@agent-device/capture-kit/screenshot-density'),
389389
]);
390390
const size = await readPngSize(file);
391-
const target = computeDensityScaledScreenshotSize(size, size.width / screen.width, pixelDensity);
391+
// The longer sides match whichever way the app has rotated its interface since `health` was read.
392+
const sourceDensity = Math.max(size.width, size.height) / Math.max(screen.width, screen.height);
393+
const target = computeDensityScaledScreenshotSize(size, sourceDensity, pixelDensity);
392394
if (target) await resizePngFile(file, target.width, target.height);
393395
}
394396

‎packages/provider-revyl/src/revyl-backend.fixtures.ts‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,6 @@ export async function startRevylFixtureBackend(
9797
let health503Polls = 0;
9898
/** The platform and session id of the last `start_device`, which is what later answers report. */
9999
let started: { platform: string; sessionId: string } | undefined;
100-
/** The session a stop or a run cancel ends: the id in a session stop, the started run's for a cancel. */
101-
const stoppedSessionIn = (url: string): string | undefined =>
102-
url.startsWith('/api/v1/execution/device/status/cancel/')
103-
? started?.sessionId
104-
: sessionIdIn(url);
105100
/**
106101
* The sessions Revyl would hold right now, by id. A session is made when its `start_device` is
107102
* *answered* — with a start delay, later than the request — and a confirmed `/stop` for that id ends
@@ -140,7 +135,7 @@ export async function startRevylFixtureBackend(
140135
answered.push(`${method} ${url}`);
141136
if (response.statusCode !== 200 || !lifecycleAccepted(sent?.body)) return;
142137
if (url.endsWith('/start_device') && started) openSessions.add(started.sessionId);
143-
const stoppedId = stoppedSessionIn(url);
138+
const stoppedId = sessionIdIn(url);
144139
if (stoppedId !== undefined) openSessions.delete(stoppedId);
145140
});
146141
const override = overrides.get(`${method} ${url}`);
@@ -181,7 +176,7 @@ export async function startRevylFixtureBackend(
181176
stop: () => {
182177
// With `sessionExistsOnceStarted`, a stop that arrives before the start has answered names a
183178
// session Revyl never made. The caller must not read that 404 as a release it never got.
184-
const stopping = stoppedSessionIn(url);
179+
const stopping = sessionIdIn(url);
185180
if (options.sessionExistsOnceStarted && (!stopping || !openSessions.has(stopping))) {
186181
return { status: 404, body: { detail: 'no such session', error: 'Not Found' } };
187182
}
@@ -306,11 +301,10 @@ function fixtureRoute(url: string): string {
306301
return 'action';
307302
}
308303
if (isDeviceSessionRoute(url) && url.endsWith('/stop')) return 'stop';
309-
if (url.startsWith('/api/v1/execution/device/status/cancel/')) return 'stop';
310304
return 'unknown';
311305
}
312306

313-
/** A `/stop` or run cancel ends its session exactly when the client accepts the answer. */
307+
/** A `/stop` ends its session exactly when the client accepts the answer. */
314308
function lifecycleAccepted(body: unknown): boolean {
315309
if (body === null || typeof body !== 'object') return false;
316310
const verdict = body as { success?: unknown; request_accepted?: unknown };

‎packages/provider-revyl/src/revyl-client.test.ts‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,6 @@ test('reads the typed failure one status names, on the route that answered', asy
106106
code: 'SESSION_NOT_FOUND',
107107
call: async (client) => await client.stopDeviceSession('session-1'),
108108
},
109-
{
110-
status: 410,
111-
reason: 'provider-session-gone',
112-
code: 'SESSION_NOT_FOUND',
113-
call: async (client) => await client.cancelDeviceRun('run-1'),
114-
},
115109
{
116110
// The same status on a route that names no session is a request the backend did not route.
117111
status: 404,
@@ -236,15 +230,6 @@ test('a lifecycle stop that Revyl answers with a refusal is still a refusal', as
236230
async () => await unaccepted.stopDeviceSession('session-1'),
237231
(error: unknown) => error instanceof AppError && error.details?.reason === 'provider-refused',
238232
);
239-
240-
const canceled = clientResponding(() => jsonResponse({ success: false }));
241-
await assert.rejects(
242-
async () => await canceled.cancelDeviceRun('run-1'),
243-
(error: unknown) =>
244-
error instanceof AppError &&
245-
error.details?.reason === 'provider-refused' &&
246-
error.details?.operation === 'cancel',
247-
);
248233
});
249234

250235
test('a confirmed lifecycle stop reports the session Revyl ended', async () => {

‎packages/provider-revyl/src/revyl-client.ts‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import type {
99
RevylWorkerActionResponse,
1010
RevylWorkerConnection,
1111
RevylWorkerHealth,
12-
RevylWorkflowCancelResponse,
13-
RevylWorkflowLifecycleResponse,
1412
} from './revyl-types.ts';
1513

1614
export const REVYL_DEFAULT_BACKEND_URL = 'https://backend.revyl.ai';
@@ -50,7 +48,7 @@ type RevylRequest = Readonly<{
5048
timeoutMs?: number;
5149
/**
5250
* Defaults to true for GET and false for POST. A POST opts in only when Revyl documents it as
53-
* idempotent, which `/stop` and `/status/cancel` are; a device action is never replayed.
51+
* idempotent, which `/stop` is; a device action is never replayed.
5452
*/
5553
retry?: boolean;
5654
/** A device-proxy action carries the agent header; an execution-API call does not. */
@@ -84,8 +82,6 @@ export type RevylClient = Readonly<{
8482
signal?: AbortSignal,
8583
): Promise<RevylWorkerActionResponse>;
8684
stopDeviceSession(sessionId: string): Promise<RevylDeviceSessionStopResponse>;
87-
/** The fallback for a session whose start never returned an id: cancel the run itself. */
88-
cancelDeviceRun(workflowRunId: string): Promise<void>;
8985
/** Cheap enough to run on every `connect`: it authenticates without starting a device. */
9086
verifyCredentials(signal?: AbortSignal): Promise<void>;
9187
}>;
@@ -204,15 +200,6 @@ export function createRevylClient(options: RevylClientOptions): RevylClient {
204200
assertLifecycleAccepted('stop', response);
205201
return response;
206202
},
207-
async cancelDeviceRun(workflowRunId) {
208-
const response = await json<RevylWorkflowCancelResponse>({
209-
method: 'POST',
210-
path: `/api/v1/execution/device/status/cancel/${encodeURIComponent(workflowRunId)}`,
211-
retry: true,
212-
route: 'session',
213-
});
214-
assertLifecycleAccepted('cancel', response);
215-
},
216203
async verifyCredentials(signal) {
217204
await json<unknown>({
218205
method: 'GET',
@@ -319,21 +306,19 @@ function assertSuccess(request: RevylRequest, response: RevylResponse): void {
319306
}
320307

321308
/**
322-
* Revyl's `/stop` and `/status/cancel` answers name the verdict they reached, and a 200 that says
323-
* `success: false` or `request_accepted: false` is a refusal the caller still has to act on: the
324-
* session is rented either way.
309+
* Revyl's `/stop` answer names the verdict it reached, and a 200 that says `success: false` or
310+
* `request_accepted: false` is a refusal the caller still has to act on: the session is rented
311+
* either way.
325312
*/
326313
function assertLifecycleAccepted(
327-
operation: 'stop' | 'cancel',
328-
response: RevylWorkflowLifecycleResponse,
314+
operation: 'stop',
315+
response: RevylDeviceSessionStopResponse,
329316
): void {
330317
if (response.success === true && (response.request_accepted ?? true) === true) return;
331318
const message = typeof response.message === 'string' ? response.message : undefined;
332319
throw new AppError(
333320
'COMMAND_FAILED',
334-
`Revyl did not ${operation === 'stop' ? 'stop the session' : 'cancel the run'}${
335-
message === undefined ? '' : `: ${message}`
336-
}`,
321+
`Revyl did not stop the session${message === undefined ? '' : `: ${message}`}`,
337322
{
338323
reason: 'provider-refused',
339324
operation,

‎packages/provider-revyl/src/revyl-session.ts‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -479,19 +479,11 @@ async function sleepBefore(deadline: RevylDeadline, signal?: AbortSignal): Promi
479479
}
480480

481481
/**
482-
* Ends one Revyl session. `/stop` is idempotent, so a repeat release is safe, and a session with no
483-
* run to route to is ended by its session id — the same pair of ends Revyl's own clients take.
482+
* Ends one Revyl session by the id this runtime issued for it. `/stop` is idempotent, so a repeat
483+
* release is safe; every rented session carries that id, so no run-level cancel is needed.
484484
*/
485-
export async function stopRevylSession(
486-
client: RevylClient,
487-
runId: string | undefined,
488-
sessionId: string | undefined,
489-
): Promise<void> {
490-
if (sessionId) {
491-
await client.stopDeviceSession(sessionId);
492-
return;
493-
}
494-
if (runId) await client.cancelDeviceRun(runId);
485+
export async function stopRevylSession(client: RevylClient, sessionId: string): Promise<void> {
486+
await client.stopDeviceSession(sessionId);
495487
}
496488

497489
function readRevylScreenSize(health: RevylWorkerHealth): RevylScreenSize | undefined {

‎packages/provider-revyl/src/revyl-types.ts‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -53,17 +53,6 @@ export type RevylDeviceSessionStopResponse = Readonly<{
5353
message?: string;
5454
}>;
5555

56-
/** Revyl's answer to cancelling a run: the same verdict spelling, without the acceptance field. */
57-
export type RevylWorkflowCancelResponse = Readonly<{
58-
success?: boolean;
59-
message?: string;
60-
task_id?: string;
61-
}>;
62-
63-
/** What either lifecycle answer has to say for the runtime to treat the end as done. */
64-
export type RevylWorkflowLifecycleResponse = RevylDeviceSessionStopResponse &
65-
RevylWorkflowCancelResponse;
66-
6756
/**
6857
* A device-proxy action result. `install` reports its verdict as `Success`, capitalized, where
6958
* every other action uses `success`; a refusal arrives in a 200 body rather than a status code.

‎packages/provider-revyl/src/runtime.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ afterEach(async () => {
2222

2323
/** Stops the fixture recorded, which is what every lifecycle assertion reads. */
2424
function stops(fixture: RevylFixtureBackend): string[] {
25-
return fixture.paths().filter((path) => path.includes('/stop') || path.includes('/cancel/'));
25+
return fixture.paths().filter((path) => path.includes('/stop'));
2626
}
2727

2828
test('a lease start waits for the worker and the device, and reports the rented device', async () => {

0 commit comments

Comments
 (0)