Skip to content

Commit d22edfe

Browse files
committed
refactor(daemon): converge the marker helpers and shared fixture waits
1 parent d4b27ba commit d22edfe

10 files changed

Lines changed: 98 additions & 154 deletions

File tree

‎.github/workflows/ios.yml‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -171,17 +171,11 @@ jobs:
171171
pnpm clean:daemon
172172
# The public-SDK record is the cold-startup evidence; the CLI timer only wraps attach.
173173
node --input-type=module -e '
174-
import { performance } from "node:perf_hooks";
174+
const { performance } = await import("node:perf_hooks");
175175
const { ensureDaemon } = await import("./dist/src/index.js");
176-
const stateDir = process.env.AGENT_DEVICE_STATE_DIR;
177-
const startedAt = performance.now();
178-
const connection = await ensureDaemon({ stateDir });
179-
console.log(JSON.stringify({
180-
phase: "daemon_cold_startup",
181-
durationMs: Math.round(performance.now() - startedAt),
182-
started: connection.started,
183-
pid: connection.daemon.pid,
184-
}));
176+
const t0 = performance.now();
177+
const c = await ensureDaemon({ stateDir: process.env.AGENT_DEVICE_STATE_DIR });
178+
console.log(JSON.stringify({ phase: "daemon_cold_startup", started: c.started, pid: c.daemon.pid, durationMs: Math.round(performance.now() - t0) }));
185179
'
186180
node bin/agent-device.mjs prepare ios-runner --platform ios --udid "${{ steps.ios-simulator.outputs.simulator-udid }}" --timeout "$AGENT_DEVICE_IOS_PREPARE_TIMEOUT_MS" --json
187181

‎packages/daemon-contracts/src/daemon-registration-owner.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ import {
4747
import { sweepDeadDaemonStartupPhasesFiles } from './daemon-startup-phases.ts';
4848

4949
export const DAEMON_STARTUP_EXIT_CODES = Object.freeze({ busy: 75, unproven: 78 });
50-
5150
export type DaemonRegistrationFields = Readonly<{
5251
socketPort?: number;
5352
httpPort?: number;

‎packages/daemon-contracts/src/daemon-startup-phases.test.ts‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import path from 'node:path';
55
import { expect, test } from 'vitest';
66
import { tryAcquireDaemonRegistration } from '@agent-device/daemon-contracts/daemon-registration-owner';
77
import {
8-
claimDaemonStartupPhasesFile,
98
readDaemonStartupPhaseMarkers,
109
recordDaemonStartupMarker,
1110
removeDaemonStartupPhasesFile,
@@ -50,19 +49,19 @@ test('marker reading ignores unknown phases and unparsable lines', () => {
5049
});
5150

5251
test('the producer lifetime claim empties a same-pid leftover', async () => {
53-
// Pid reuse after both the old client and daemon died before either cleanup: the entry claims
52+
// Pid reuse after both the old client and daemon died before either cleanup: the entry removes
5453
// its record before its first marker; this child repeats that producer sequence itself.
5554
const stateDir = await mkdtempForTest('agent-device-markers-');
5655
writeFileSync(path.join(stateDir, 'package.json'), '{"type":"module"}');
5756
const entry = path.join(stateDir, 'claim-producer.js');
5857
const registrationUrl = new URL('./daemon-startup-phases.ts', import.meta.url).href;
5958
writeFileSync(
6059
entry,
61-
`import { claimDaemonStartupPhasesFile, recordDaemonStartupMarker, resolveDaemonStartupPhasesPath } from ${JSON.stringify(registrationUrl)};
60+
`import { removeDaemonStartupPhasesFile, recordDaemonStartupMarker, resolveDaemonStartupPhasesPath } from ${JSON.stringify(registrationUrl)};
6261
const file = resolveDaemonStartupPhasesPath(${JSON.stringify(stateDir)}, process.pid);
6362
recordDaemonStartupMarker(file, 'listening');
6463
recordDaemonStartupMarker(file, 'registered');
65-
claimDaemonStartupPhasesFile(file);
64+
removeDaemonStartupPhasesFile(file);
6665
recordDaemonStartupMarker(file, 'modules-loaded');
6766
`,
6867
);
@@ -76,13 +75,19 @@ recordDaemonStartupMarker(file, 'modules-loaded');
7675
expect(phases).toEqual({ 'modules-loaded': expect.any(Number) });
7776
});
7877

79-
test('a refused claim reset reports through the callback instead of throwing', async () => {
78+
test.each([
79+
['remove', (p: string, cb: (e: unknown) => void) => removeDaemonStartupPhasesFile(p, cb)],
80+
[
81+
'record',
82+
(p: string, cb: (e: unknown) => void) => recordDaemonStartupMarker(p, 'modules-loaded', cb),
83+
],
84+
] as const)('a refused %s reports through the callback instead of throwing', async (_name, run) => {
8085
const stateDir = await mkdtempForTest('agent-device-markers-');
81-
const markerPath = resolveDaemonStartupPhasesPath(stateDir, 778);
86+
const markerPath = resolveDaemonStartupPhasesPath(stateDir, 777);
8287
mkdirSync(markerPath, { recursive: true });
8388
const reported: unknown[] = [];
8489
expect(() =>
85-
claimDaemonStartupPhasesFile(markerPath, (error) => {
90+
run(markerPath, (error) => {
8691
reported.push(error);
8792
}),
8893
).not.toThrow();
@@ -121,20 +126,6 @@ test('removeDaemonStartupPhasesFile removes a file and tolerates one already gon
121126
expect(() => removeDaemonStartupPhasesFile(file)).not.toThrow();
122127
});
123128

124-
test('a marker write failure reports through the callback, keeping startup running', async () => {
125-
const stateDir = await mkdtempForTest('agent-device-markers-');
126-
const markerPath = resolveDaemonStartupPhasesPath(stateDir, 777);
127-
mkdirSync(markerPath, { recursive: true });
128-
const reported: unknown[] = [];
129-
expect(() =>
130-
recordDaemonStartupMarker(markerPath, 'modules-loaded', (error) => {
131-
reported.push(error);
132-
}),
133-
).not.toThrow();
134-
expect(reported).toHaveLength(1);
135-
expect(reported[0]).toHaveProperty('message');
136-
});
137-
138129
test('a crash-replay append keeps the file valid JSONL', async () => {
139130
const stateDir = await mkdtempForTest('agent-device-markers-');
140131
const markerPath = resolveDaemonStartupPhasesPath(stateDir, 999);

‎packages/daemon-contracts/src/daemon-startup-phases.ts‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import { isProcessAlive } from '@agent-device/host-kit/process';
55

66
/**
77
* Startup phase markers: a per-launch record, named for the writing pid, of how far that startup
8-
* got, so a waiting client tells "slow but progressing" from "stuck" without a timestamp and with
9-
* no heartbeat. The writer removes it on completion, the launching client after a joined exit, and
10-
* each new registration owner sweeps the files of provably dead pids.
8+
* got, so a waiting client tells "slow but progressing" from "stuck" without a heartbeat. The
9+
* writer removes it on completion, the client after a joined exit, and each new registration
10+
* owner sweeps the files of provably dead pids.
1111
*/
1212
const DAEMON_STARTUP_PHASES = Object.freeze([
1313
'modules-loaded',
@@ -66,11 +66,11 @@ function endsWithoutNewline(file: string): boolean {
6666
}
6767

6868
/**
69-
* Empties this launch's own record before its first marker: a per-pid name identifies a writer,
70-
* not a lifetime. No caller resets a file named for another pid; a refused reset never stops the
71-
* startup because the reset ran first when it could run.
69+
* Removes this launch's record: before its first marker (a per-pid name identifies a writer, not
70+
* a lifetime, so the entry claims its file first), and after a finished startup or joined exit.
71+
* A refused removal only reports through `reportFailure`; it never stops a startup.
7272
*/
73-
export function claimDaemonStartupPhasesFile(
73+
export function removeDaemonStartupPhasesFile(
7474
startupPhasesPath: string,
7575
reportFailure?: (error: NormalizedError) => void,
7676
): void {
@@ -81,13 +81,6 @@ export function claimDaemonStartupPhasesFile(
8181
}
8282
}
8383

84-
/** Removes a marker file whose writer is provably gone: a finished startup or a joined exit. */
85-
export function removeDaemonStartupPhasesFile(startupPhasesPath: string): void {
86-
try {
87-
fs.rmSync(startupPhasesPath, { force: true });
88-
} catch {}
89-
}
90-
9184
/** Sweeps marker files of provably dead (or unnameable) pids; a live pid's file is kept. */
9285
export function sweepDeadDaemonStartupPhasesFiles(stateDir: string): void {
9386
let entries: string[];

‎scripts/__tests__/eager-closure-budgets.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ export const APPROVED_CLOSURE_ADDITIONS: readonly Readonly<{
156156
module: 'packages/daemon-contracts/src/daemon-startup-phases.ts',
157157
issue: '#3782',
158158
reason:
159-
'The startup phase record owner the #3791 review asked for (its own file/format/writer/reader/sweep beside daemon-shutdown-report). The daemon entry records its earliest `modules-loaded` marker at module load, so the owner must be eager there; the entry pays for the record it writes first.',
159+
'The startup phase record owner the #3791 review asked for. The daemon entry records its earliest `modules-loaded` marker at module load, so the owner must be eager there; the entry pays for the record it writes first.',
160160
owner: 'thymikee',
161161
},
162162
]);

‎src/__tests__/test-utils/registered-daemon-fixture.test.ts‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
launchRegisteredDaemonFixtureArgs,
1212
registeredDaemonFixtureArgs,
1313
spawnRegisteredDaemonFixture,
14+
waitForFixtureFile,
1415
waitForRegisteredDaemonFixture,
1516
finishRegisteredDaemonFixture,
1617
} from './registered-daemon-fixture.ts';
@@ -36,21 +37,12 @@ test('args built before any launch keep their own entry script under a concurren
3637
assert.notEqual(firstArgs[1], secondArgs[1]);
3738
const first = launchRegisteredDaemonFixtureArgs(paths, firstArgs, { stdio: 'ignore' });
3839
try {
39-
const held = path.join(paths.baseDir, `registration-held-${first.pid}`);
40-
for (let attempt = 0; !fs.existsSync(held); attempt += 1) {
41-
assert.ok(attempt < 400, 'child 1 never acquired the registration');
42-
await new Promise((resolve) => setTimeout(resolve, 10));
43-
}
40+
await waitForFixtureFile(path.join(paths.baseDir, `registration-held-${first.pid}`));
4441
const second = launchRegisteredDaemonFixtureArgs(paths, secondArgs, { stdio: 'ignore' });
4542
assert.equal((await second.exited).exitCode, DAEMON_STARTUP_EXIT_CODES.busy);
46-
const ownRecord = resolveDaemonStartupPhasesPath(paths.baseDir, first.pid);
47-
const text = fs.readFileSync(ownRecord, 'utf8');
43+
const text = fs.readFileSync(resolveDaemonStartupPhasesPath(paths.baseDir, first.pid), 'utf8');
4844
assert.match(text, /modules-loaded/);
49-
assert.doesNotMatch(
50-
text,
51-
/lock-acquired/,
52-
"child 1 must run the script built for it, not the second build's switches",
53-
);
45+
assert.doesNotMatch(text, /lock-acquired/, 'child 1 must run the script built for it');
5446
} finally {
5547
await finishRegisteredDaemonFixture(paths.baseDir);
5648
}

‎src/__tests__/test-utils/registered-daemon-fixture.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ export function registeredDaemonFixtureArgs(
5252
`import fs from 'node:fs';
5353
import path from 'node:path';
5454
import { DAEMON_STARTUP_EXIT_CODES, tryAcquireDaemonRegistration } from ${JSON.stringify(registrationUrl)};
55-
import { claimDaemonStartupPhasesFile, recordDaemonStartupMarker, resolveDaemonStartupPhasesPath } from ${JSON.stringify(phasesUrl)};
55+
import { removeDaemonStartupPhasesFile, recordDaemonStartupMarker, resolveDaemonStartupPhasesPath } from ${JSON.stringify(phasesUrl)};
5656
const paths = ${JSON.stringify(paths)};
5757
const barrier = ${JSON.stringify(acquisitionBarrier)};
5858
const emitBefore = ${fixtureOptions?.emitStartupMarkerBeforeAcquisition ? 'true' : 'false'};
@@ -61,11 +61,7 @@ if (barrier) {
6161
fs.writeFileSync(barrier + '.ready-' + process.pid, 'ready');
6262
while (!fs.existsSync(barrier)) await new Promise(resolve => setTimeout(resolve, 10));
6363
}
64-
if (emitBefore) {
65-
const ownRecord = resolveDaemonStartupPhasesPath(paths.baseDir, process.pid);
66-
claimDaemonStartupPhasesFile(ownRecord);
67-
recordDaemonStartupMarker(ownRecord, 'modules-loaded');
68-
}
64+
if (emitBefore) { const own = resolveDaemonStartupPhasesPath(paths.baseDir, process.pid); removeDaemonStartupPhasesFile(own); recordDaemonStartupMarker(own, 'modules-loaded'); }
6965
const acquired = await tryAcquireDaemonRegistration(paths);
7066
if (acquired.status !== 'acquired') process.exit(DAEMON_STARTUP_EXIT_CODES[acquired.status]);
7167
if (emitAfter)
@@ -117,6 +113,14 @@ export function spawnRegisteredDaemonFixture(
117113
);
118114
}
119115

116+
/** Waits up to 4 s for a fixture child to publish `file`; one shared wait for every probe. */
117+
export async function waitForFixtureFile(file: string): Promise<void> {
118+
for (let attempt = 0; !fs.existsSync(file); attempt += 1) {
119+
assert.ok(attempt < 400, `fixture did not publish ${file}`);
120+
await new Promise((resolve) => setTimeout(resolve, 10));
121+
}
122+
}
123+
120124
export async function waitForRegisteredDaemonFixture(
121125
paths: DaemonPaths,
122126
child: ReturnType<typeof runCmdDetachedMonitored>,

0 commit comments

Comments
 (0)