Skip to content

Commit 6e3134b

Browse files
thymikeethymikee
committed
fix: preserve publish errors during filesystem faults
1 parent 0dfaacb commit 6e3134b

5 files changed

Lines changed: 102 additions & 38 deletions

File tree

‎src/daemon/__tests__/filesystem-boundary-faults.test.ts‎

Lines changed: 52 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,23 @@ type FaultInjection = { wasInjected: () => boolean };
3030

3131
const FILESYSTEM_ERRNOS: readonly FilesystemErrno[] = ['EIO', 'ENOSPC', 'EMFILE'];
3232
const FAULT_POINTS: readonly FaultPoint[] = ['write', 'rename'];
33+
const REQUIRED_DAEMON_MATRIX_MODULES = [
34+
'durable-capture-resource-store/screen-recording',
35+
'durable-capture-resource-store/app-log',
36+
'device-claims',
37+
'session-script-writer',
38+
'daemon-shutdown-report',
39+
'session-store',
40+
] as const;
41+
const DURABLE_RESOURCE_CONFIGS = [
42+
{
43+
resourceKind: 'screen-recording',
44+
fileName: 'screen-recording.resource.json',
45+
displayName: 'Screen recording',
46+
},
47+
{ resourceKind: 'app-log', fileName: 'app-log.resource.json', displayName: 'App-log' },
48+
] as const;
49+
const registeredDaemonMatrixRows: string[] = [];
3350

3451
type BoundaryFixture = {
3552
targetPath: string;
@@ -44,25 +61,23 @@ afterEach(() => {
4461
delete process.env.AGENT_DEVICE_CLAIMS_DIR;
4562
});
4663

47-
registerFilesystemMatrix('durable-capture-resource-store', (root) => {
48-
const store = createDurableCaptureResourceStore({
49-
resourceKind: 'screen-recording',
50-
fileName: 'screen-recording.resource.json',
51-
displayName: 'Screen recording',
52-
});
53-
const targetPath = store.resolvePath(path.join(root, 'sessions', 'default'));
64+
for (const config of DURABLE_RESOURCE_CONFIGS) {
65+
registerFilesystemMatrix(`durable-capture-resource-store/${config.resourceKind}`, (root) => {
66+
const store = createDurableCaptureResourceStore(config);
67+
const targetPath = store.resolvePath(path.join(root, 'sessions', 'default'));
5468

55-
return {
56-
targetPath,
57-
// The durable store opens the temporary path and writes through its file
58-
// descriptor, so the write-side fault is identified by the descriptor.
59-
// This fixture performs no other writes after the fault is installed.
60-
matchesPublishPath: (value) =>
61-
typeof value === 'number' || matchesTargetOrTemporary(value, targetPath),
62-
run: async () => store.write(targetPath, resourceEnvelope('filesystem-fault')),
63-
expected: 'throw',
64-
};
65-
});
69+
return {
70+
targetPath,
71+
// The durable store opens the temporary path and writes through its file
72+
// descriptor, so the write-side fault is identified by the descriptor.
73+
matchesPublishPath: (value) =>
74+
typeof value === 'number' || matchesTargetOrTemporary(value, targetPath),
75+
run: async () =>
76+
store.write(targetPath, resourceEnvelope(config.resourceKind, 'filesystem-fault')),
77+
expected: 'throw',
78+
};
79+
});
80+
}
6681

6782
registerFilesystemMatrix('device-claims', (root) => {
6883
const device: DeviceInfo = {
@@ -153,14 +168,26 @@ registerFilesystemMatrix('session-store', (root) => {
153168
};
154169
});
155170

171+
test('filesystem errno matrix declares every scoped daemon row', () => {
172+
const expectedRows = REQUIRED_DAEMON_MATRIX_MODULES.flatMap((moduleName) =>
173+
FAULT_POINTS.flatMap((faultPoint) =>
174+
FILESYSTEM_ERRNOS.map((errno) => `${moduleName}:${faultPoint}:${errno}`),
175+
),
176+
);
177+
assert.deepEqual([...registeredDaemonMatrixRows].sort(), expectedRows.sort());
178+
});
179+
156180
function registerFilesystemMatrix(
157181
moduleName: string,
158182
createFixture: (root: string) => BoundaryFixture,
159183
): void {
160184
for (const faultPoint of FAULT_POINTS) {
161185
for (const errno of FILESYSTEM_ERRNOS) {
186+
registeredDaemonMatrixRows.push(`${moduleName}:${faultPoint}:${errno}`);
162187
test(`${moduleName} ${faultPoint} ${errno} leaves no unpublished temporary file`, async () => {
163-
const root = mkdtempForTestSync(`agent-device-${moduleName}-boundary-`);
188+
const root = mkdtempForTestSync(
189+
`agent-device-${moduleName.replaceAll('/', '-')}-boundary-`,
190+
);
164191
const fixture = createFixture(root);
165192
const fault = installFilesystemFault(faultPoint, errno, fixture.matchesPublishPath);
166193

@@ -199,6 +226,7 @@ function installFilesystemFault(
199226
vi.spyOn(fs, 'writeFileSync').mockImplementation((...args) => {
200227
if (matchesPublishPath(args[0])) {
201228
injected = true;
229+
Reflect.apply(originalWriteFileSync, fs, args);
202230
throw failure;
203231
}
204232
return Reflect.apply(originalWriteFileSync, fs, args);
@@ -251,9 +279,12 @@ function listTemporaryFiles(root: string): string[] {
251279
}
252280
}
253281

254-
function resourceEnvelope(sessionId: string) {
282+
function resourceEnvelope<ResourceKind extends 'screen-recording' | 'app-log'>(
283+
resourceKind: ResourceKind,
284+
sessionId: string,
285+
) {
255286
return createDurableResourceEnvelope({
256-
resourceKind: 'screen-recording',
287+
resourceKind,
257288
sessionId,
258289
device: { id: 'emulator-5554', family: 'android', kind: 'emulator' },
259290
owner: localRuntimeOwner('android'),

‎src/daemon/client/__tests__/boundary-fault-acceptance.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,18 @@ import type { DaemonRequest } from '../../types.ts';
99
import { sendRequest } from '../daemon-client-transport.ts';
1010

1111
const PROCESS_DEATH_COMMANDS = ['open', 'close'] as const;
12+
const REQUIRED_PROCESS_DEATH_COMMANDS = ['open', 'close'] as const;
13+
14+
test('acceptance matrix declares both process-death lifecycle commands', () => {
15+
assert.deepEqual([...PROCESS_DEATH_COMMANDS].sort(), [...REQUIRED_PROCESS_DEATH_COMMANDS].sort());
16+
});
1217

1318
test.each(PROCESS_DEATH_COMMANDS)(
1419
'acceptance row: process death after %s dispatch is a bounded, non-replayed request',
1520
async (command) => {
21+
// This acceptance row deliberately stops at the transport boundary: a
22+
// dead daemon is observed as a closed response socket. Lifecycle teardown
23+
// and process-tree cleanup belong to the deferred full fault matrix.
1624
const endpoint = await startEndpointThatDiesAfterRequest();
1725
const baseDir = mkdtempForTestSync(`agent-device-process-death-${command}-`);
1826
const statePaths = daemonPaths(baseDir);
@@ -31,6 +39,8 @@ test.each(PROCESS_DEATH_COMMANDS)(
3139
assert.ok(error instanceof AppError);
3240
assert.equal(error.code, 'COMMAND_FAILED');
3341
assert.match(error.message, /communicate with daemon/i);
42+
assert.equal(error.details?.requestId, request.meta?.requestId);
43+
assert.match(String(error.details?.hint), /Retry command/i);
3444
return true;
3545
},
3646
);

‎src/daemon/session-script-writer.ts‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -286,27 +286,31 @@ function publishHealedScriptAtomically(params: {
286286
dir,
287287
`.${path.basename(scriptPath)}.${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}.tmp`,
288288
);
289-
fs.writeFileSync(tempPath, script);
290289
try {
291-
if (force) {
292-
fs.renameSync(tempPath, scriptPath);
293-
return;
290+
fs.writeFileSync(tempPath, script);
291+
try {
292+
if (force) {
293+
fs.renameSync(tempPath, scriptPath);
294+
return;
295+
}
296+
// Atomic create-exclusive: EEXIST iff a file already sits at scriptPath.
297+
fs.linkSync(tempPath, scriptPath);
298+
} catch (error) {
299+
if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error;
300+
throw new AppError(
301+
'COMMAND_FAILED',
302+
`A file already exists at ${scriptPath}; remove it, pass replay --save-script=<other-path>, or pass --force/--overwrite to replace it.`,
303+
{ reason: 'script_target_exists', path: scriptPath },
304+
);
294305
}
295-
// Atomic create-exclusive: EEXIST iff a file already sits at scriptPath.
296-
fs.linkSync(tempPath, scriptPath);
297-
} catch (error) {
298-
if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error;
299-
throw new AppError(
300-
'COMMAND_FAILED',
301-
`A file already exists at ${scriptPath}; remove it, pass replay --save-script=<other-path>, or pass --force/--overwrite to replace it.`,
302-
{ reason: 'script_target_exists', path: scriptPath },
303-
);
304306
} finally {
305307
// linkSync leaves the temp hard-link behind on success; an error leaves
306308
// it too — always clean up whatever of our own temp remains. A `force`
307309
// rename already consumed tempPath (it no longer exists at this path),
308310
// so this is a harmless no-op in that branch.
309-
fs.rmSync(tempPath, { force: true });
311+
try {
312+
fs.rmSync(tempPath, { force: true });
313+
} catch {}
310314
}
311315
}
312316

‎src/platforms/apple/core/__tests__/runner-lease-filesystem-boundary-faults.test.ts‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ type FaultInjection = { wasInjected: () => boolean };
1212

1313
const FILESYSTEM_ERRNOS: readonly FilesystemErrno[] = ['EIO', 'ENOSPC', 'EMFILE'];
1414
const FAULT_POINTS: readonly FaultPoint[] = ['write', 'rename'];
15+
const REQUIRED_RUNNER_LEASE_MODULES = ['runner-lease'] as const;
1516
const DEVICE_ID = 'filesystem-fault-runner';
16-
17-
let leaseRoot: string;
17+
const registeredRunnerLeaseRows: string[] = [];
1818

1919
afterEach(() => {
2020
vi.restoreAllMocks();
@@ -23,8 +23,9 @@ afterEach(() => {
2323

2424
for (const faultPoint of FAULT_POINTS) {
2525
for (const errno of FILESYSTEM_ERRNOS) {
26+
registeredRunnerLeaseRows.push(`runner-lease:${faultPoint}:${errno}`);
2627
test(`runner lease ${faultPoint} ${errno} leaves no unpublished temporary file`, () => {
27-
leaseRoot = mkdtempForTestSync(`agent-device-runner-lease-boundary-`);
28+
const leaseRoot = mkdtempForTestSync(`agent-device-runner-lease-boundary-`);
2829
process.env.AGENT_DEVICE_IOS_RUNNER_LEASE_DIR = leaseRoot;
2930
const targetPath = path.join(leaseRoot, `${DEVICE_ID}.json`);
3031
const lease = makeRunnerLease({
@@ -49,6 +50,15 @@ for (const faultPoint of FAULT_POINTS) {
4950
}
5051
}
5152

53+
test('filesystem errno matrix declares every runner-lease row', () => {
54+
const expectedRows = REQUIRED_RUNNER_LEASE_MODULES.flatMap((moduleName) =>
55+
FAULT_POINTS.flatMap((faultPoint) =>
56+
FILESYSTEM_ERRNOS.map((errno) => `${moduleName}:${faultPoint}:${errno}`),
57+
),
58+
);
59+
assert.deepEqual([...registeredRunnerLeaseRows].sort(), expectedRows.sort());
60+
});
61+
5262
function installFilesystemFault(
5363
faultPoint: FaultPoint,
5464
errno: FilesystemErrno,
@@ -72,6 +82,7 @@ function installFilesystemFault(
7282
vi.spyOn(fs, 'writeFileSync').mockImplementation((...args) => {
7383
if (matchesTarget(args[0])) {
7484
injected = true;
85+
Reflect.apply(originalWriteFileSync, fs, args);
7586
throw failure;
7687
}
7788
return Reflect.apply(originalWriteFileSync, fs, args);

‎src/platforms/apple/core/__tests__/runner-recovery-wiring.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ const LOST_RESPONSE_MUTATION_ROWS = [
5353
request: { command: 'type', text: 'hello', textEntryMode: 'replace' },
5454
},
5555
] as const;
56+
const REQUIRED_LOST_RESPONSE_ACCEPTANCE_COMMANDS = ['press', 'fill'] as const;
5657

5758
afterEach(async () => {
5859
await server?.close();
@@ -76,6 +77,13 @@ function seedSession(port: number): RunnerSession {
7677
return session;
7778
}
7879

80+
test('acceptance matrix declares both lost-response mutation commands', () => {
81+
assert.deepEqual(
82+
LOST_RESPONSE_MUTATION_ROWS.map((row) => row.acceptanceCommand).sort(),
83+
[...REQUIRED_LOST_RESPONSE_ACCEPTANCE_COMMANDS].sort(),
84+
);
85+
});
86+
7987
test.each(LOST_RESPONSE_MUTATION_ROWS)(
8088
'acceptance row: lost-response-after-mutation for $acceptanceCommand does not replay the mutation',
8189
async ({ runnerCommand, request }) => {

0 commit comments

Comments
 (0)