Skip to content

Commit 2fc31c2

Browse files
committed
fix: harden owned child cleanup identities
1 parent 8d4a8dd commit 2fc31c2

4 files changed

Lines changed: 87 additions & 14 deletions

src/platform-runtime-screen-recording-apple-simulator-host.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,25 @@ test('post-publication abort does not kill the adopted process', async () => {
155155
await expect(process.wait).resolves.toMatchObject({ exitCode: 0 });
156156
});
157157

158+
test('publishes the post-exec simulator identity after readiness', async () => {
159+
const root = mkdtempForTestSync('agent-device-recording-post-exec-');
160+
const outputPath = path.join(root, 'capture.mp4');
161+
const running = background(47, `xcrun simctl io ${simulator.id} recordVideo ${outputPath}`);
162+
const postExecCommand = `/Library/Developer/PrivateFrameworks/CoreSimulator.framework/Versions/A/Resources/bin/simctl io ${simulator.id} recordVideo ${outputPath}`;
163+
setTimeout(() => {
164+
processes.commands.set(47, postExecCommand);
165+
fs.writeFileSync(outputPath, 'recording');
166+
}, 25);
167+
168+
const process = await withTransport(
169+
running.process,
170+
async () => await startAppleSimulatorRecording(simulator, outputPath),
171+
);
172+
173+
expect(process.markers?.[0]).toMatchObject({ pid: 47, command: postExecCommand });
174+
await expect(process.terminate()).resolves.toBeUndefined();
175+
});
176+
158177
test('stops a locally launched simulator recorder after xcrun execs the simctl binary', async () => {
159178
const root = mkdtempForTestSync('agent-device-recording-xcrun-exec-');
160179
const outputPath = path.join(root, 'capture.mp4');

src/platform-runtime-screen-recording-apple-simulator-host.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,12 @@ export async function startAppleSimulatorRecording(
7474
}
7575
try {
7676
await waitForReadiness(outputPath, background.wait, signal);
77-
const markers = await resolveManagedProcessTree(rootMarker);
77+
// `runCmdBackground('xcrun', ...)` may briefly expose its shell wrapper before that
78+
// process execs CoreSimulator's `simctl`. Read the root identity again after the output
79+
// proves the recorder is ready so the durable descriptor and generic startup reaper share
80+
// the stable post-exec identity already used by the live Apple matcher.
81+
const postExecRootMarker = (await resolveManagedProcessIdentity(rootMarker.pid)) ?? rootMarker;
82+
const markers = await resolveManagedProcessTree(postExecRootMarker);
7883
return createAppleSimulatorProcess(background, markers);
7984
} catch (error) {
8085
await terminateManagedProcessSet(

src/platforms/web/agent-browser-provider.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,35 @@ test('agent-browser close failure does not clear the daemon-owned record', async
168168
expect(ownedProcessRecords.clear).not.toHaveBeenCalled();
169169
});
170170

171+
test('agent-browser semantic close failure does not clear the daemon-owned record', async () => {
172+
const ownedProcessRecords: OwnedProcessRecordStore = {
173+
replace: vi.fn(),
174+
clear: vi.fn(),
175+
read: vi.fn(() => []),
176+
};
177+
178+
await withManagedAgentBrowserProvider(
179+
{
180+
session: 'web-session',
181+
openWebSessionNames: () => ['web-session'],
182+
ownedProcessRecords,
183+
},
184+
async (provider) => {
185+
await withCommandExecutorOverride(
186+
async () =>
187+
jsonResult({
188+
success: false,
189+
code: 'CLOSE_FAILED',
190+
error: 'browser stayed open',
191+
}),
192+
async () => await assert.rejects(async () => await provider.close()),
193+
);
194+
},
195+
);
196+
197+
expect(ownedProcessRecords.clear).not.toHaveBeenCalled();
198+
});
199+
171200
test('agent-browser provider ignores provider-startup cleanup failures', async () => {
172201
const calls: AgentBrowserCall[] = [];
173202
mockProviderStartupCleanup.mockRejectedValue(new Error('ps failed'));

src/platforms/web/agent-browser-provider.ts

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ import {
2828
import type { OwnedProcessRecordStore } from '../../utils/owned-process-record.ts';
2929

3030
const AGENT_BROWSER = 'agent-browser';
31-
// Exported so WEB_BROWSER_SESSION_TEARDOWN_BUDGET_MS (daemon/server/daemon-runtime.ts) can be
32-
// pinned to it instead of drifting out of sync with a copied number.
31+
// Exported so daemon shutdown can pin its web-close budget to the ceiling enforced for one
32+
// `agent-browser` CLI call instead of copying a number that can drift.
3333
export const AGENT_BROWSER_TIMEOUT_MS = 30_000;
3434
const AGENT_BROWSER_DOCTOR_HINT =
3535
'Run `agent-device web setup` to install the managed web backend.';
@@ -238,24 +238,28 @@ async function runAgentBrowserJson(
238238
): Promise<unknown> {
239239
const { session, options, signal } = params;
240240
const cliArgs = [...args, '--json', ...(session ? ['--session', session] : [])];
241-
const result = await runAgentBrowserCommand(cliArgs, options, signal);
242-
const parsed = parseAgentBrowserJson(result.stdout, result.stderr, cliArgs, result.exitCode);
243-
return unwrapAgentBrowserJson(parsed, result, cliArgs);
241+
return await runAgentBrowserCommand(
242+
cliArgs,
243+
options,
244+
(result) => {
245+
const parsed = parseAgentBrowserJson(result.stdout, result.stderr, cliArgs, result.exitCode);
246+
return unwrapAgentBrowserJson(parsed, result, cliArgs);
247+
},
248+
signal,
249+
);
244250
}
245251

246252
async function runAgentBrowserCommand(
247253
cliArgs: string[],
248254
options: AgentBrowserProviderOptions,
255+
interpret: (result: { stdout: string; stderr: string; exitCode: number }) => unknown,
249256
signal?: AbortSignal,
250-
): Promise<{
251-
stdout: string;
252-
stderr: string;
253-
exitCode: number;
254-
}> {
257+
): Promise<unknown> {
255258
let stdout = '';
256259
let stderr = '';
257260
let exitCode = 0;
258261
let commandCompleted = false;
262+
let semanticSuccess = false;
259263
const status = getManagedAgentBrowserStatus({ stateDir: options.stateDir });
260264
try {
261265
await cleanupProviderStartupOrphans(options);
@@ -271,24 +275,39 @@ async function runAgentBrowserCommand(
271275
exitCode = result.exitCode;
272276
commandCompleted = true;
273277
} catch (error) {
278+
await finalizeAgentBrowserProcessRecord({
279+
cliArgs,
280+
commandCompleted,
281+
exitCode,
282+
semanticSuccess,
283+
options,
284+
status,
285+
});
274286
throw mapAgentBrowserRunError(error, cliArgs);
287+
}
288+
289+
try {
290+
const result = { stdout, stderr, exitCode };
291+
const output = interpret(result);
292+
semanticSuccess = true;
293+
return output;
275294
} finally {
276295
await finalizeAgentBrowserProcessRecord({
277296
cliArgs,
278297
commandCompleted,
279298
exitCode,
299+
semanticSuccess,
280300
options,
281301
status,
282302
});
283303
}
284-
285-
return { stdout, stderr, exitCode };
286304
}
287305

288306
async function finalizeAgentBrowserProcessRecord(params: {
289307
cliArgs: string[];
290308
commandCompleted: boolean;
291309
exitCode: number;
310+
semanticSuccess: boolean;
292311
options: AgentBrowserProviderOptions;
293312
status: ReturnType<typeof getManagedAgentBrowserStatus>;
294313
}): Promise<void> {
@@ -315,12 +334,13 @@ async function finalizeAgentBrowserProcessRecord(params: {
315334
function canClearAgentBrowserRecord(
316335
params: Pick<
317336
Parameters<typeof finalizeAgentBrowserProcessRecord>[0],
318-
'cliArgs' | 'commandCompleted' | 'exitCode'
337+
'cliArgs' | 'commandCompleted' | 'exitCode' | 'semanticSuccess'
319338
>,
320339
otherOpenSessionCount: number,
321340
): boolean {
322341
return (
323342
params.commandCompleted &&
343+
params.semanticSuccess &&
324344
params.cliArgs[0] === 'close' &&
325345
params.exitCode === 0 &&
326346
otherOpenSessionCount === 0

0 commit comments

Comments
 (0)