Skip to content

Commit f85f322

Browse files
thymikeethymikee
committed
fix(android): retire completed recording evidence after pid reuse (#2476)
A reused emulator can reassign a recorded screenrecord pid to an unrelated process. The transport proves that replacement with `ownership-lost`, but completed-evidence retirement and reattach accepted only `missing`, so they treated proven termination like an uncertain live recorder: `record start` refused forever on the retained marker, and `record stop` could not return the already-finalized completion. Classify the declared ownership observations once, in the contract that declares them, and ask that question instead of comparing to `missing`. Retirement still refuses a live or unreadable recorder and never signals a pid it proved is not its own.
1 parent 47b1cae commit f85f322

11 files changed

Lines changed: 381 additions & 13 deletions

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,14 @@
22

33
## Unreleased
44

5+
- Fixed: Android `record start` no longer refuses to begin after a reused emulator reassigned the
6+
previous recorder's pid. A completed recording's native marker is retired only once its recorder is
7+
proven gone, but only an absent pid counted as proof — a pid that now names an unrelated process,
8+
or a recorder that exited and waits to be reaped, did not. `record start` then failed every later
9+
attempt with `Android screenrecord completed evidence cannot be safely retired`, and `record stop`
10+
could not return an already-finalized recording. Proven termination now retires the marker and
11+
returns the stored completion; a live or unreadable recorder still blocks, and the unrelated
12+
process is never signalled (#2476).
513
- Fixed: iOS Simulator snapshots of Safari and of apps with a `WKWebView` stopped showing the
614
page in 0.21.0 — chrome plus empty `[webview]` nodes, no links, text, or form fields, so no ref
715
could reach the page (#2484). The host AX bridge that 0.21.0 made the Simulator's snapshot source

‎packages/contracts/src/screen-recording-runtime-host.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { expect, test } from 'vitest';
2+
import {
3+
type AndroidScreenRecordingProcessOwnership,
4+
provesAndroidScreenRecordTermination,
5+
} from './screen-recording-runtime-host.ts';
6+
7+
const TERMINATION_PROOF: readonly (readonly [AndroidScreenRecordingProcessOwnership, boolean])[] = [
8+
['missing', true],
9+
['ownership-lost', true],
10+
['owned-alive', false],
11+
['uncertain', false],
12+
];
13+
14+
test.each(TERMINATION_PROOF)(
15+
'observation %s proves recorder termination as %s',
16+
(ownership, expected) => {
17+
expect(provesAndroidScreenRecordTermination(ownership)).toBe(expected);
18+
},
19+
);

‎packages/contracts/src/screen-recording-runtime-host.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,27 @@ export type AndroidScreenRecordingProcessOwnership =
100100
| 'ownership-lost'
101101
| 'uncertain';
102102

103+
/**
104+
* Whether an observation proves that the process named by the inspected identity is gone.
105+
*
106+
* `ownership-lost` is proof rather than doubt: the pid is present, yet the identity metadata that
107+
* was readable there named something else — a reassigned pid, or an exited task whose command line
108+
* is already gone — so the recorded process can no longer write its artifact. `uncertain` reads
109+
* nothing conclusive and proves nothing, and a matching identity is provably still running.
110+
*/
111+
export function provesAndroidScreenRecordTermination(
112+
ownership: AndroidScreenRecordingProcessOwnership,
113+
): boolean {
114+
switch (ownership) {
115+
case 'missing':
116+
case 'ownership-lost':
117+
return true;
118+
case 'owned-alive':
119+
case 'uncertain':
120+
return false;
121+
}
122+
}
123+
103124
export type AndroidScreenRecordingStopOutcome =
104125
| 'stopped'
105126
| 'already-missing'

‎packages/platform-android/src/recording/launch.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { DeviceInfo } from '@agent-device/kernel/device';
22
import type { PlatformRuntimeHost } from '@agent-device/contracts/platform-runtime-operations';
33
import type { ScreenRecordingStartInput } from '@agent-device/contracts/screen-recording-runtime';
4+
import { provesAndroidScreenRecordTermination } from '@agent-device/contracts/screen-recording-runtime-host';
45
import {
56
cleanupChunks,
67
AndroidScreenRecordingStartRollbackUnconfirmed,
@@ -167,7 +168,7 @@ async function retireCompletedEvidence(
167168
remotePath: chunk.remotePath,
168169
startTime: chunk.remoteStartTime,
169170
});
170-
if (state !== 'missing')
171+
if (!provesAndroidScreenRecordTermination(state))
171172
throw new Error('Android screenrecord completed evidence cannot be safely retired');
172173
}
173174
await cleanupChunks(transport, evidence.chunks);

‎packages/platform-android/src/recording/recovery.test.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,56 @@ test('retains completed evidence while an exact persisted recorder identity rema
135135
expect(JSON.parse(manifest)).toHaveProperty('completion');
136136
});
137137

138+
test('terminalizes completed evidence whose recorder pid was reassigned, touching nothing', async () => {
139+
let manifest = '';
140+
const removals: string[] = [];
141+
const runtime = await start({
142+
writeManifest: async ({ contents }: { contents: string }) => {
143+
manifest = contents;
144+
},
145+
readManifest: async () =>
146+
manifest ? { status: 'read' as const, contents: manifest } : { status: 'missing' as const },
147+
inspect: async () => 'ownership-lost' as const,
148+
remove: async (remotePath: string) => {
149+
removals.push(`artifact:${remotePath}`);
150+
return true;
151+
},
152+
removeManifest: async (manifestPath: string) => {
153+
removals.push(`marker:${manifestPath}`);
154+
return true;
155+
},
156+
});
157+
const started = await runtime.screenRecordingStart(recordingInput());
158+
const native = JSON.parse(manifest);
159+
manifest = JSON.stringify({
160+
...native,
161+
completion: {
162+
backend: 'adb screenrecord',
163+
outPath: native.outputPath,
164+
startedAt: native.startedAt,
165+
completedAt: native.startedAt + 1,
166+
scope: native.scope,
167+
showTouches: native.showTouches,
168+
recordOnlySession: native.recordOnlySession,
169+
},
170+
});
171+
172+
await expect(runtime.screenRecordingReattach({ envelope: started.envelope })).resolves.toEqual({
173+
status: 'completed',
174+
result: {
175+
backend: 'adb screenrecord',
176+
outPath: native.outputPath,
177+
startedAt: native.startedAt,
178+
completedAt: native.startedAt + 1,
179+
scope: native.scope,
180+
showTouches: native.showTouches,
181+
recordOnlySession: native.recordOnlySession,
182+
},
183+
});
184+
expect(removals).toEqual([]);
185+
expect(JSON.parse(manifest)).toHaveProperty('completion');
186+
});
187+
138188
test('makes matching pending evidence cleanup-eligible and stops discovered exact recorder pids', async () => {
139189
let manifest = '';
140190
const removed: string[] = [];

‎packages/platform-android/src/recording/recovery.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type {
44
ScreenRecordingRuntimeOperations,
55
ScreenRecordingStartInput,
66
} from '@agent-device/contracts/screen-recording-runtime';
7+
import { provesAndroidScreenRecordTermination } from '@agent-device/contracts/screen-recording-runtime-host';
78
import { createScreenRecordingLiveHandle } from '@agent-device/capture-kit';
89
import {
910
androidScreenRecordingDescriptorCodec,
@@ -192,11 +193,13 @@ async function completedEvidenceIsTerminal(transport: Transport, evidence: Nativ
192193
try {
193194
for (const chunk of evidence.chunks) {
194195
if (
195-
(await transport.inspect({
196-
pid: chunk.remotePid,
197-
remotePath: chunk.remotePath,
198-
startTime: chunk.remoteStartTime,
199-
})) !== 'missing'
196+
!provesAndroidScreenRecordTermination(
197+
await transport.inspect({
198+
pid: chunk.remotePid,
199+
remotePath: chunk.remotePath,
200+
startTime: chunk.remoteStartTime,
201+
}),
202+
)
200203
)
201204
return false;
202205
}

‎packages/platform-android/src/recording/start-reconciliation.test.ts‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,88 @@ test('reconciles coherent completed evidence before output preparation or launch
5656
await started.pendingHandle.transfer().forceCleanup();
5757
});
5858

59+
test('retires completed evidence whose recorder pid was reassigned, signaling nothing', async () => {
60+
let marker = JSON.stringify(completedEvidence());
61+
const calls: string[] = [];
62+
const runtime = await start({
63+
readManifest: async (path: string) =>
64+
path.startsWith('/sdcard') && marker
65+
? { status: 'read' as const, contents: marker }
66+
: { status: 'missing' as const },
67+
inspect: async () => 'ownership-lost' as const,
68+
stop: async ({ pid }: { pid: string }) => {
69+
calls.push(`signal:${pid}`);
70+
return 'already-missing' as const;
71+
},
72+
remove: async (path: string) => {
73+
calls.push(`artifact:${path}`);
74+
return true;
75+
},
76+
removeManifest: async () => {
77+
calls.push('manifest');
78+
marker = '';
79+
return true;
80+
},
81+
outputs: {
82+
prepare: async () => {
83+
calls.push('prepare');
84+
},
85+
},
86+
start: async () => {
87+
calls.push('launch');
88+
return recordingProcess('77');
89+
},
90+
});
91+
const started = await runtime.screenRecordingStart(newInput());
92+
expect(calls).toEqual([
93+
'artifact:/sdcard/agent-device-recording-1.mp4',
94+
'manifest',
95+
'prepare',
96+
'launch',
97+
]);
98+
await started.pendingHandle.transfer().forceCleanup();
99+
});
100+
101+
test.each([
102+
['an unconfirmed recorder identity', 'uncertain'],
103+
['a live recorder', 'owned-alive'],
104+
])('refuses completed evidence named by %s', async (_name, ownership) => {
105+
const marker = JSON.stringify(completedEvidence());
106+
const calls: string[] = [];
107+
const runtime = await start({
108+
readManifest: async (path: string) =>
109+
path.startsWith('/sdcard')
110+
? { status: 'read' as const, contents: marker }
111+
: { status: 'missing' as const },
112+
inspect: async () => ownership,
113+
stop: async ({ pid }: { pid: string }) => {
114+
calls.push(`signal:${pid}`);
115+
return 'already-missing' as const;
116+
},
117+
remove: async () => {
118+
calls.push('artifact');
119+
return true;
120+
},
121+
removeManifest: async () => {
122+
calls.push('manifest');
123+
return true;
124+
},
125+
outputs: {
126+
prepare: async () => {
127+
calls.push('prepare');
128+
},
129+
},
130+
start: async () => {
131+
calls.push('launch');
132+
return recordingProcess('77');
133+
},
134+
});
135+
await expect(runtime.screenRecordingStart(newInput())).rejects.toThrow(
136+
'cannot be safely retired',
137+
);
138+
expect(calls).toEqual([]);
139+
});
140+
59141
test('retirement failure blocks launch and can succeed on a later retry', async () => {
60142
let marker = JSON.stringify(completedEvidence());
61143
let allowRemoval = false;

‎src/platform-runtime-screen-recording-android-host.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,41 @@ test('retains an interrupted empty-stderr manifest probe as unavailable', async
196196
}
197197
});
198198

199+
test('proves termination for a reassigned pid and an exited task, and never for an unreadable one', async () => {
200+
const commands: string[] = [];
201+
let identity: { stat: string; cmdline: string } = {
202+
stat: procStat(4004, '5432'),
203+
cmdline: ['/system/bin/servicemanager', ''].join('\0'),
204+
};
205+
await withAndroidAdbProvider(
206+
{
207+
exec: async (args) => {
208+
const command = args[1] ?? '';
209+
commands.push(command);
210+
if (command.endsWith('/stat')) return result(identity.stat);
211+
if (command.endsWith('/cmdline')) return result(identity.cmdline);
212+
return result('');
213+
},
214+
},
215+
{ serial: android.id },
216+
async () => {
217+
const transport = await createAndroidScreenRecordingTransport(android);
218+
const recorded = { pid: '4004', remotePath: '/sdcard/capture.mp4', startTime: '3766' };
219+
await expect(transport.inspect(recorded)).resolves.toBe('ownership-lost');
220+
identity = {
221+
stat: procStat(4004, '3766'),
222+
cmdline: ['/system/bin/screenrecord', '--bit-rate', '8000000', '/other.mp4', ''].join('\0'),
223+
};
224+
await expect(transport.inspect(recorded)).resolves.toBe('ownership-lost');
225+
identity = { stat: procStat(4004, '3766'), cmdline: '' };
226+
await expect(transport.inspect(recorded)).resolves.toBe('ownership-lost');
227+
identity = { stat: procStat(4004, '3766'), cmdline: '' };
228+
await expect(transport.stop(recorded)).resolves.toBe('ownership-lost');
229+
expect(commands.some((command) => command.startsWith('kill '))).toBe(false);
230+
},
231+
);
232+
});
233+
199234
function procStat(pid: number, startTime: string): string {
200235
return `${pid} (screenrecord) S ${Array.from({ length: 18 }, () => '0').join(' ')} ${startTime}`;
201236
}

‎src/platform-runtime-screen-recording-android-host.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import path from 'node:path';
22
import type {
33
AndroidScreenRecordingProcessIdentity,
4+
AndroidScreenRecordingProcessOwnership,
45
AndroidScreenRecordingTransport,
56
} from '@agent-device/contracts/screen-recording-runtime-host';
67
import type { DeviceInfo } from '@agent-device/kernel/device';
@@ -140,7 +141,7 @@ async function inspectAndroidScreenRecordingProcess(
140141
signal?: AbortSignal,
141142
): Promise<
142143
Readonly<{
143-
status: 'missing' | 'owned-alive' | 'ownership-lost' | 'uncertain';
144+
status: AndroidScreenRecordingProcessOwnership;
144145
process?: AndroidScreenRecordingProcessIdentity;
145146
}>
146147
> {

0 commit comments

Comments
 (0)