fix(test): unstick Smithery probe lifecycle and pin DAEMON_GENERATION 54

packages/coding-agent/CHANGELOG.md

@@ -11,6 +11,8 @@
 
 ### Fixed
 
+- `notifications-topic-registry.test.ts` pins `DAEMON_GENERATION` at 54 after #3965 (was stale at 53; Dev CI run 31133356543).
+- `smithery-env-trust.test.ts` no longer awaits hung probe pipes past a hard per-attempt deadline: minimal child env, `stdin: "ignore"`, SIGKILL + settled race, and up to 2 timeout-only retries. Fixes the inherited-config case that hit exactly 60001ms on Dev CI run 31133356543 after kill-at-45s left `Promise.all` on stdout/stderr unresolved. Assertions unchanged.
 - A Telegram notification daemon that dies without a clean shutdown no longer keeps advertising itself as the ready owner. Ownership was surrendered only by `releaseDaemonOwnership`, which runs after a fully quiesced and fully persisted shutdown; an uncaught error, a failed final topic-registry persist, or a signal left `ownershipPhase: "ready"` and a matching ownership lock on disk, and every later reader attached to a process that no longer existed. Observed in the field: a daemon wrote one heartbeat 559 ms after readiness, died on an uncaught `shared topic authority unavailable` error, and was still recorded as ready eight hours and seventeen crashes later, with no notification delivered in between. The daemon-internal entrypoint now records `stoppedAt` on the way out — from its own `finally` and from a `postmortem` hook that also covers the fatal paths that call `process.exit()` without unwinding — fenced on full owner identity so a successor's state is never touched, and leaving the ownership lock for the existing reclaim path to adjudicate. `isFreshLiveOwner` already treats `stoppedAt` as disqualifying, so recovery no longer depends on a pid liveness check that a recycled pid can defeat.
 - `smithery-env-trust.test.ts` warms the Bun probe child in `beforeAll` and kills stalled spawns at a 45s budget so the first case no longer absorbs cold-start compile cost into its per-test timeout under shard contention (observed 60001ms timeout after the 60s cap on #3969 exact-head CI). Assertions unchanged.
 - `smithery-env-trust.test.ts` raises the per-test child-process timeout from 30s to 60s so CI contention cannot fail at the previous 30s cap (Dev CI run 31128319216 timed out at 30004ms).

packages/coding-agent/test/notifications-topic-registry.test.ts

@@ -562,8 +562,9 @@ test("preserves a no-provenance endpoint claim before a held create can stage it
 	await creating;
 	expect(reg.endpointAuthority(binding)).toEqual({ state: "unique", sessionId: "B" });
 });
-test("publishes generation 53 at serving epoch 5", () => {
-	expect(DAEMON_GENERATION).toBe(53);
+test("publishes generation 54 at serving epoch 5", () => {
+	// Generation 54: owner stoppedAt postmortem for unclean daemon death (#3965).
+	expect(DAEMON_GENERATION).toBe(54);
 	expect(SERVING_EPOCH).toBe(5);
 });
 test("archives pending topics into retained inactive records", async () => {

packages/coding-agent/test/smithery-env-trust.test.ts

@@ -46,51 +46,122 @@ afterEach(() => {
 	for (const dir of tempDirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true });
 });
 
-// Per-spawn budget for the probe child. After a suite warmup, healthy spawns
-// finish in ~300ms; under extreme shard contention they may take a few seconds.
-// Kill rather than wait for the outer it() timeout so a stalled child cannot
-// pin the suite for the full 60s and leak pipes.
-const PROBE_SPAWN_BUDGET_MS = 45_000;
+/**
+ * Per-attempt budget for one probe child.
+ *
+ * Healthy spawns finish in ~300–500ms after suite warmup. Under CI shard
+ * contention a child can stall during Bun startup; the previous harness awaited
+ * `stdout`/`stderr`/`exited` as a single `Promise.all` and only `kill()`ed on a
+ * timer. When the kill did not close the pipes, `Promise.all` never settled and
+ * the outer `it(..., 60_000)` failed at ~60001ms (Dev CI run 31133356543 —
+ * "still honors inherited Smithery configuration") even though the timer had
+ * fired at 45s. Race the attempt deadline so a stalled child cannot pin the
+ * suite past this budget, and SIGKILL so the process actually dies.
+ */
+const PROBE_SPAWN_BUDGET_MS = 15_000;
+/** Contention recovery: retry only spawn-lifecycle timeouts, never assertion failures. */
+const PROBE_SPAWN_RETRIES = 2;
 
-async function resolveIn(cwd: string, overrides: Record<string, string> = {}): Promise<Resolved> {
-	const env: Record<string, string> = {};
-	for (const [key, value] of Object.entries(process.env)) {
-		if (value !== undefined) env[key] = value;
-	}
-	for (const key of KEYS) delete env[key];
-	// `$credentialEnv` also consults the agent `.env`, the GJC config `.env`,
-	// `~/.env` and the login shell rc files; keep all of them neutral.
-	env.HOME = tempDir();
-	env.GJC_CODING_AGENT_DIR = tempDir();
+function isProbeTimeout(error: unknown): boolean {
+	return error instanceof Error && error.message.startsWith("probe timed out after ");
+}
+
+function buildProbeEnv(overrides: Record<string, string>): Record<string, string> {
+	// Minimal env — copying the full parent process.env under GitHub Actions can
+	// be multi-kilobyte and has been observed to correlate with stalled first-byte
+	// child startup under shard pressure. The probe only needs PATH + neutral
+	// home/config roots + the Smithery keys under test.
+	const env: Record<string, string> = {
+		PATH: process.env.PATH ?? "/usr/bin:/bin",
+		HOME: tempDir(),
+		GJC_CODING_AGENT_DIR: tempDir(),
+		TMPDIR: process.env.TMPDIR ?? os.tmpdir(),
+		LANG: process.env.LANG ?? "C",
+	};
+	// Do not forward BUN_OPTIONS / NODE_OPTIONS — test-runner flags can stall
+	// child startup under contention when combined with a cold module graph.
 	Object.assign(env, overrides);
+	return env;
+}
+
+async function resolveInOnce(cwd: string, overrides: Record<string, string> = {}): Promise<Resolved> {
+	const env = buildProbeEnv(overrides);
+	const proc = Bun.spawn([process.execPath, PROBE], {
+		cwd,
+		env,
+		stdout: "pipe",
+		stderr: "pipe",
+		stdin: "ignore",
+	});
+
+	const { promise, resolve, reject } = Promise.withResolvers<Resolved>();
+	let settled = false;
+	const settle = (fn: () => void): void => {
+		if (settled) return;
+		settled = true;
+		fn();
+	};
 
-	const proc = Bun.spawn([process.execPath, PROBE], { cwd, env, stdout: "pipe", stderr: "pipe" });
-	let timedOut = false;
 	const timer = setTimeout(() => {
-		timedOut = true;
 		try {
-			proc.kill();
+			proc.kill(9);
 		} catch {
 			// already exited
 		}
+		settle(() => {
+			reject(new Error(`probe timed out after ${PROBE_SPAWN_BUDGET_MS}ms and was killed`));
+		});
 	}, PROBE_SPAWN_BUDGET_MS);
-	try {
-		const [stdout, stderr, exitCode] = await Promise.all([
-			new Response(proc.stdout).text(),
-			new Response(proc.stderr).text(),
-			proc.exited,
-		]);
-		if (timedOut) {
-			throw new Error(
-				`probe timed out after ${PROBE_SPAWN_BUDGET_MS}ms and was killed` +
-					(stderr.trim() ? `: ${stderr.trim()}` : ""),
-			);
+
+	void (async () => {
+		try {
+			const [stdout, stderr, exitCode] = await Promise.all([
+				new Response(proc.stdout).text(),
+				new Response(proc.stderr).text(),
+				proc.exited,
+			]);
+			if (settled) return;
+			if (exitCode !== 0) {
+				settle(() => {
+					reject(new Error(`probe failed (${exitCode}): ${stderr}`));
+				});
+				return;
+			}
+			settle(() => {
+				resolve(JSON.parse(stdout.trim()) as Resolved);
+			});
+		} catch (error) {
+			settle(() => {
+				reject(error instanceof Error ? error : new Error(String(error)));
+			});
 		}
-		if (exitCode !== 0) throw new Error(`probe failed (${exitCode}): ${stderr}`);
-		return JSON.parse(stdout.trim()) as Resolved;
+	})();
+
+	try {
+		return await promise;
 	} finally {
 		clearTimeout(timer);
+		if (!settled) {
+			try {
+				proc.kill(9);
+			} catch {
+				// ignore
+			}
+		}
+	}
+}
+
+async function resolveIn(cwd: string, overrides: Record<string, string> = {}): Promise<Resolved> {
+	let lastTimeout: Error | undefined;
+	for (let attempt = 0; attempt <= PROBE_SPAWN_RETRIES; attempt++) {
+		try {
+			return await resolveInOnce(cwd, overrides);
+		} catch (error) {
+			if (!isProbeTimeout(error) || attempt === PROBE_SPAWN_RETRIES) throw error;
+			lastTimeout = error instanceof Error ? error : new Error(String(error));
+		}
 	}
+	throw lastTimeout ?? new Error("probe failed after retries");
 }
 
 const PLANTED = [
@@ -102,15 +173,13 @@ const PLANTED = [
 describe("Smithery env trust boundary", () => {
 	// Cold-start the probe module graph outside per-test budgets. Under CI shard
 	// contention the first Bun child can spend tens of seconds compiling the
-	// probe + env stack; later spawns then complete in ~300ms. Without a warmup,
-	// the first it() absorbs cold-start into its 60s budget and flakes (observed
-	// 60001ms on #3969 exact-head after the 60s bump). beforeAll is the isolation
-	// fix; 120s matches other child-process suite budgets and is not a third
-	// blind per-test timeout bump.
+	// probe + env stack; later spawns then complete in ~300ms.
 	beforeAll(async () => {
 		await resolveIn(projectDir());
 	}, 120_000);
 
+	// Outer budgets stay generous enough for retries (3 × 15s) but no longer
+	// depend on a single hung Promise.all surviving until 60s.
 	it("uses the built-in endpoints and no key by default", async () => {
 		const resolved = await resolveIn(projectDir());
 		expect(resolved.url).toBe("https://smithery.ai");