Skip to content

Commit ab596f8

Browse files
test(cli): the shared-name install-local jobs test lets each install run to the setup deadline, not a fixed 120 s (#25547)
<!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuDWJsBGQpadCH8JR2xZQ5re)_ Fixes #25499 Clause-②: no Metadata-protocol: none `packages/cli/test/package-install-local-jobs-shared-name.integration.test.ts` gave each of its three `os package install` children a fixed 120 s SIGKILL. That number was set on an idle machine. Under a full `pnpm test` on a 4 vCPU box, the first install outlived it, and 6 of 9 assertions went red against correct code. The `beforeAll` still had more than 390 s of its 600 s budget left. Test-only. `skip-changeset`: the file is under `packages/cli/test/`, which `@objectstack/cli`'s `files[]` does not ship. ## What changed (one file) - **An install has no budget of its own.** `beforeAll` takes one deadline when it starts: `SETUP_BUDGET_MS` (the hook's existing timeout, `2 * BOOT_TIMEOUT_MS + 12 * RUN_WAIT_MS`, unchanged at 600 s) less a 10 s `KILL_REPORT_RESERVE_MS`. Each install may run until that deadline. On a slow box, the installs use the time the boots and run waits did not need. No number went up. - **A kill names the install.** On the deadline the install's whole process group is SIGKILLed: the child now spawns `detached`, so the node process that `tsx` starts is killed too, and that process holds the pipes `close` waits on. The promise then **rejects**, naming the install, its wall time and the budget, with both transcripts. So the red is that message, not a bare `exit null` and not vitest's `Hook timed out`, which names no step. - **The wall time is kept as evidence and is not an input.** `InstallRun` now carries `elapsedMs`, and the first `it` puts it in its failure message. Why there is no constant and no load factor: `run-dev-unbuilt-workspace.e2e.test.ts` in this package records two ceilings that failed this way, a fixed one and one scaled from a calibration. A bigger constant fails again on a busier box. The shared spawner helpers in `test/helpers/serve-process.ts` export no install budget and no load-aware ceiling (their only child budget is `runServe`'s inline 180 s default). So this change adds no export to them, and the seven sibling `package-install-local-*` files keep their own copy of `packageInstall()` (see Acceptance notes). ## Verification (head `02097ac15b`, 4 vCPU container, every run under `scripts/pm/os-verify-lock.sh`) | leg | result | |---|---| | the file alone (pure tree) | `Test Files 1 passed`, `Tests 9 passed`, 87.10 s | | per-install wall time, alone (one-off logging mutation, restored) | alpha 11105 ms, beta 10159 ms, gamma 11483 ms, all exit 0 | | per-install wall time inside the whole cli `integration` tier, `VITEST_MAX_WORKERS=3` (same mutation) | alpha 14198 ms, beta 13516 ms, gamma 15747 ms, all exit 0; tier `Test Files 171 passed, 1 skipped (172)`, `Tests 1469 passed, 9 skipped`, 2498.96 s | | ablation: bogus artifact path (`./dist/no-such-artifact.json`) | red: `all three packages install` fails with `alpha (9838 ms): … Cannot read artifact: ENOENT`, exit 1 against expected 0. 9/9 red, no hook timeout | | ablation: budget spent (`deadline = Date.now() + 1`) | red in 16.96 s: `beforeAll` rejects with `os package install in …/shared_alpha was SIGKILLed after 7 ms: the setup budget (600000 ms for the whole beforeAll, less a 10000 ms reserve) was spent.` The file fails and its 9 tests are skipped. No `Hook timed out` | The mutations went through `scripts/ablation-replace.mjs`, in WRAP mode, with an EXIT/INT/TERM trap. After the last leg the file's blob equals HEAD (`bc1e7d9b3a…`) and `git diff HEAD` is empty. My first attempt at the logging mutation did nothing: its anchor was a substring of its replacement, so the tool refused it (anchor count 1 -> 1). It was re-run with an anchor that is not. **NOT MEASURED: a full `pnpm test` on this box.** The loaded reading above is the cli integration tier at three workers, not the turbo-wide run from the issue. The installs there took 14-16 s, against more than 120 s in the issue's run. So this box's load never came near the old ceiling, and the fix's tolerance of a busier box is argued from the design, not measured. The local gates are listed in the report on #25499. `pnpm lint` and the cli `typecheck` / `unit` tier were run here. ## Acceptance notes - The same fixed `setTimeout(() => child.kill('SIGKILL'), 120_000)` install ceiling is in six sibling files: `package-install-local-{jobs,jobs-pull,hooks,handlers,boot-steps,uninstall-cleanups}.integration.test.ts`. Not changed here, because the claim's file surface is this one file. No red is on record for them. carrier: none. - In this file, `bootStart`'s fixed `BOOT_TIMEOUT_MS` still does not read the shared deadline. If installs used most of the budget, a hung second boot would show as `Hook timed out` instead of a named error. Not observed. carrier: none. --- _Generated by [Claude Code](https://claude.ai/code/session_01CxBKycaiUpA1yGx1XyoGqX)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01CxBKycaiUpA1yGx1XyoGqX Co-authored-by: Claude <noreply@anthropic.com>
1 parent 673ebfd commit ab596f8

1 file changed

Lines changed: 61 additions & 10 deletions

File tree

‎packages/cli/test/package-install-local-jobs-shared-name.integration.test.ts‎

Lines changed: 61 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -234,23 +234,74 @@ async function authenticate(live: LiveStart): Promise<string> {
234234
return token;
235235
}
236236

237+
/**
238+
* The `beforeAll` budget, and the ONE number an install's ceiling is read from.
239+
*
240+
* An install had a ceiling of its own, a fixed 120 s, set on an idle machine.
241+
* The file passes alone in about 76 s, all three installs included. Under a full
242+
* `pnpm test` on a 4 vCPU box the first install outlived it. It was SIGKILLed
243+
* with more than 390 s of the hook's 600 s left (the whole file took 209 s),
244+
* and six assertions went red against correct code. A bigger constant fails
245+
* the same way on a busier box. A ceiling scaled from a calibration fails too
246+
* (`run-dev-unbuilt-workspace.e2e.test.ts` records both attempts).
247+
*
248+
* So an install has no budget of its own. It may run until this phase's
249+
* deadline, the hook's start plus this budget, minus the reserve below. A slow
250+
* box spends the slack the boots and the run waits did not use. An install
251+
* that hangs is still killed, and the kill names the install.
252+
*/
253+
const SETUP_BUDGET_MS = 2 * BOOT_TIMEOUT_MS + 12 * RUN_WAIT_MS;
254+
/**
255+
* What a killed install leaves the hook, before vitest's own hook timeout.
256+
* The group kill, the pipes closing and the throw must land inside it. Past
257+
* that, the red is `Hook timed out`, which names no step.
258+
*/
259+
const KILL_REPORT_RESERVE_MS = 10_000;
260+
261+
interface InstallRun { exit: number | null; output: string; elapsedMs: number }
262+
237263
/**
238264
* `os package install ./dist/objectstack.json` against the running runtime.
239265
* ⛔ Asynchronous on purpose — see the #21321 sibling: a `spawnSync` stops this
240266
* process draining the server's pipes for the whole install.
267+
*
268+
* The child runs until `deadline` (see `SETUP_BUDGET_MS`). It gets its own
269+
* process group, so the kill reaches the node process `tsx` starts as well.
270+
* That process holds the pipes, and `close` would wait for it. A kill REJECTS,
271+
* naming the install, the wall time and the budget: the rest of the hook could
272+
* only run into vitest's hook timeout. An install that exits by itself
273+
* resolves, whatever its code, and the first `it` judges it.
241274
*/
242-
function packageInstall(appDir: string, live: LiveStart): Promise<{ exit: number | null; output: string }> {
243-
return new Promise((done) => {
275+
function packageInstall(appDir: string, live: LiveStart, deadline: number): Promise<InstallRun> {
276+
return new Promise((done, fail) => {
277+
const started = Date.now();
244278
const child = spawn(TSX, [CLI, 'package', 'install', './dist/objectstack.json', '--runtime', live.base, '--email', EMAIL, '--password', PASSWORD], {
245279
cwd: appDir,
246280
env: childEnv({ NO_COLOR: '1' }),
247281
stdio: ['ignore', 'pipe', 'pipe'],
282+
detached: true,
248283
});
249284
let output = '';
285+
let killed = false;
250286
child.stdout?.on('data', (d) => { output += String(d); });
251287
child.stderr?.on('data', (d) => { output += String(d); });
252-
const timer = setTimeout(() => child.kill('SIGKILL'), 120_000);
253-
child.on('close', (code) => { clearTimeout(timer); done({ exit: code, output }); });
288+
const timer = setTimeout(() => {
289+
killed = true;
290+
try { process.kill(-child.pid!, 'SIGKILL'); } catch { /* group already gone */ }
291+
}, Math.max(0, deadline - started));
292+
child.on('close', (code) => {
293+
clearTimeout(timer);
294+
const elapsedMs = Date.now() - started;
295+
if (!killed) {
296+
done({ exit: code, output, elapsedMs });
297+
return;
298+
}
299+
fail(new Error(
300+
`os package install in ${appDir} was SIGKILLed after ${elapsedMs} ms: the setup budget ` +
301+
`(${SETUP_BUDGET_MS} ms for the whole beforeAll, less a ${KILL_REPORT_RESERVE_MS} ms reserve) was spent.` +
302+
`\n--- install output ---\n${output.slice(-3000)}\n--- runtime output ---\n${live.output().slice(-3000)}`,
303+
));
304+
});
254305
});
255306
}
256307

@@ -314,7 +365,6 @@ function sharedNames(rows: any[], key: 'name' | 'job_name'): string[] {
314365
return [...new Set(rows.map((r) => String(r?.[key])).filter((n) => n.endsWith(SHARED_JOB)))].sort();
315366
}
316367

317-
interface InstallRun { exit: number | null; output: string }
318368
const readings: {
319369
installs: Record<string, InstallRun>;
320370
alphaAlone?: { answer: Answer; floor: number };
@@ -336,6 +386,7 @@ const readings: {
336386
} = { installs: {}, output: {} };
337387

338388
beforeAll(async () => {
389+
const deadline = Date.now() + SETUP_BUDGET_MS - KILL_REPORT_RESERVE_MS;
339390
const root = mkdtempSync(join(tmpdir(), 'install-local-jobs-shared-'));
340391
dirs.push(root);
341392
const appDir = (pkg: Pkg) => {
@@ -359,15 +410,15 @@ beforeAll(async () => {
359410
const token = await authenticate(first);
360411

361412
// ALPHA alone: a single-package runtime.
362-
readings.installs.alpha = await packageInstall(dirOf.alpha, first);
413+
readings.installs.alpha = await packageInstall(dirOf.alpha, first, deadline);
363414
readings.alphaAlone = await awaitRuns(first, token, ALPHA, 0);
364415
readings.alphaAloneCatalogue = await catalogue(first, token);
365416

366417
// BETA and GAMMA declare the same job name. The floor for ALPHA is taken
367418
// AFTER both installs answered, so only a run that happens once they are in
368419
// lifts it.
369-
readings.installs.beta = await packageInstall(dirOf.beta, first);
370-
readings.installs.gamma = await packageInstall(dirOf.gamma, first);
420+
readings.installs.beta = await packageInstall(dirOf.beta, first, deadline);
421+
readings.installs.gamma = await packageInstall(dirOf.gamma, first, deadline);
371422
await sleep(SETTLE_MS);
372423
readings.alphaAfterOthers = await awaitRuns(first, token, ALPHA, await countOf(first, token, ALPHA));
373424
readings.betaRuns = await awaitRuns(first, token, BETA, 0);
@@ -394,7 +445,7 @@ beforeAll(async () => {
394445
readings.gammaAfterAlphaGone = await awaitRuns(second, token2, GAMMA, await countOf(second, token2, GAMMA));
395446
readings.output.restart = second.output();
396447
await stopGroup(second.child);
397-
}, 2 * BOOT_TIMEOUT_MS + 12 * RUN_WAIT_MS);
448+
}, SETUP_BUDGET_MS);
398449

399450
afterAll(async () => {
400451
for (const child of groups) await stopGroup(child);
@@ -406,7 +457,7 @@ const transcript = (phase: string) => `\n--- ${phase} output ---\n${(readings.ou
406457
describe('#21602: two packages declaring the same job name both run, and an uninstall stops only its own', () => {
407458
it('all three packages install (exit 0)', () => {
408459
for (const [name, run] of Object.entries(readings.installs)) {
409-
expect(run.exit, `${name}: ${run.output}`).toBe(0);
460+
expect(run.exit, `${name} (${run.elapsedMs} ms): ${run.output}`).toBe(0);
410461
expect(run.output, name).toMatch(/Package installed into the running kernel/);
411462
}
412463
});

0 commit comments

Comments
 (0)