Separate Pi retry policy from lifecycle process coverage

plugins/provider-pi/src/bridge/bridge.lifecycle.test.ts

@@ -133,22 +133,21 @@ it("discard ends the child and removes the session file", async () => {
 }, 90_000);
 
 it("a failed construction leaves no child", async () => {
-  // A provider the catalog does not know passes the up-front check (pi may
-  // serve providers the catalog omits); the fake then starts on its default
-  // model, the bridge sees the mismatch after get_state, retries the spawn
-  // through the transient-auth window, and fails.
+  // Retry policy has deterministic unit coverage. This process-level case
+  // needs one real child to prove the bridge kills a failed construction.
+  vi.stubEnv("FAKE_PI_EXIT_BEFORE_FIRST_RESPONSE", "1");
   const response = await harness.request((nextId += 1), "thread/start", {
     threadId: "thr_lc_failed",
     cwd: harness.workspaceDir,
     instructionMode: "append",
-    options: { ...FULL_PERMISSION_OPTIONS, model: "other-provider/no-such-model" },
+    options: FULL_PERMISSION_OPTIONS,
   });
   expect(response.error).toMatchObject({
-    message: expect.stringContaining('did not start with model "other-provider/no-such-model"'),
+    message: expect.stringContaining("pi exited"),
   });
   const log = harness.readProcessLog();
-  expect(log.spawned.length).toBeGreaterThan(1);
-  await expectEveryChildGone(log.spawned.length);
+  expect(log.spawned).toHaveLength(1);
+  await expectEveryChildGone(1);
 }, 90_000);
 
 it("the fork helper child exits once the fork is done", async () => {
@@ -212,7 +211,7 @@ async function expectScratchFilesGone(): Promise<void> {
   }
 }
 
-it("a child's tool and prompt files go with the child, for a released thread and for every attempt of a failed construction", async () => {
+it("a child's tool and prompt files go with the child after release and failed construction", async () => {
   // The bridge process outlives every pi child it spawns, so a file written
   // for one child must not wait for the bridge's own temp dir to be removed.
   await harness.startThread("thr_lc_scratch", {
@@ -234,18 +233,19 @@ it("a child's tool and prompt files go with the child, for a released thread and
   await expectEveryChildGone(1);
   await expectScratchFilesGone();
 
-  // Every retry of the transient-auth window wrote its own set; each went
-  // with its detached child.
+  // A construction that exits before readiness still owns the files the
+  // bridge prepared for it, and its exit removes them.
+  vi.stubEnv("FAKE_PI_EXIT_BEFORE_FIRST_RESPONSE", "1");
   const failed = await harness.request((nextId += 1), "thread/start", {
     threadId: "thr_lc_scratch_failed",
     cwd: harness.workspaceDir,
     instructionMode: "append",
-    options: { ...FULL_PERMISSION_OPTIONS, instructions: "be brief", model: "other-provider/no-such-model" },
+    options: { ...FULL_PERMISSION_OPTIONS, instructions: "be brief" },
   });
   expect(failed.error).toBeDefined();
   const log = harness.readProcessLog();
-  expect(log.spawned.length).toBeGreaterThan(2);
-  await expectEveryChildGone(log.spawned.length);
+  expect(log.spawned).toHaveLength(2);
+  await expectEveryChildGone(2);
   await expectScratchFilesGone();
 }, 60_000);
 

plugins/provider-pi/src/bridge/fake-pi-rpc.mjs

@@ -48,6 +48,8 @@
  * - Fault knobs for the bridge's own tests: FAKE_PI_SPAWN_COUNTER_FILE counts
  *   spawns across processes and FAKE_PI_MISMATCH_FIRST_SPAWN=1 makes only the
  *   first spawn ignore `--model` (a transient model mismatch);
+ *   FAKE_PI_EXIT_BEFORE_FIRST_RESPONSE=1 exits after recording the spawn but
+ *   before importing the extension or reading a command;
  *   FAKE_PI_NO_SESSION_START=1 never emits session_start to the extension (so
  *   no `ready`); FAKE_PI_DROP_STEER_AT_END=1 ends a run with a queued steer
  *   still queued; FAKE_PI_STREAMING_AFTER_END=1 reports isStreaming after a
@@ -134,6 +136,9 @@ process.on("SIGTERM", () => {
   if (hangOnClose) return;
   exit();
 });
+if (process.env.FAKE_PI_EXIT_BEFORE_FIRST_RESPONSE === "1") {
+  exit();
+}
 
 const MODELS = [
   {

plugins/provider-pi/src/bridge/rpc-session.retry.test.ts

@@ -0,0 +1,28 @@
+import { expect, it, vi } from "vitest";
+import { runPiTransientAuthConstruction } from "./rpc-session.js";
+
+it("exhausts the initial construction and eight transient-auth retries deterministically", async () => {
+  const errors = Array.from(
+    { length: 9 },
+    (_, index) => new Error(`mismatch ${index + 1}`),
+  );
+  const attempt = vi.fn(async () => ({
+    ok: false as const,
+    error: errors[attempt.mock.calls.length - 1]!,
+  }));
+  const discardFailedAttempt = vi.fn();
+  const waitBeforeRetry = vi.fn(async () => undefined);
+
+  await expect(
+    runPiTransientAuthConstruction({
+      attempt,
+      discardFailedAttempt,
+      isClosed: () => false,
+      waitBeforeRetry,
+    }),
+  ).rejects.toBe(errors[8]);
+
+  expect(attempt).toHaveBeenCalledTimes(9);
+  expect(discardFailedAttempt).toHaveBeenCalledTimes(8);
+  expect(waitBeforeRetry).toHaveBeenCalledTimes(8);
+});

plugins/provider-pi/src/bridge/rpc-session.ts

@@ -119,6 +119,38 @@ const CHANNEL_REQUEST_TIMEOUT_MS = 30_000;
 /** How long an `agent_end` waits for the extension's in-process leaf report. */
 const AGENT_END_LEAF_TIMEOUT_MS = 5_000;
 
+type PiSessionConstructionOutcome = { ok: true } | { ok: false; error: Error };
+
+function waitForPiTransientAuthRetry(): Promise<void> {
+  return new Promise((resolve) =>
+    setTimeout(resolve, PI_TRANSIENT_AUTH_RETRY_DELAY_MS),
+  );
+}
+
+/**
+ * Retry the transient model-resolution window without coupling its policy to
+ * child construction. Tests inject synchronous attempts and waits; production
+ * supplies real child construction and teardown.
+ */
+export async function runPiTransientAuthConstruction(args: {
+  attempt: () => Promise<PiSessionConstructionOutcome>;
+  discardFailedAttempt: () => void;
+  isClosed: () => boolean;
+  waitBeforeRetry: () => Promise<void>;
+}): Promise<void> {
+  for (let attempt = 0; ; attempt += 1) {
+    const outcome = await args.attempt();
+    if (outcome.ok) {
+      return;
+    }
+    if (attempt >= PI_TRANSIENT_AUTH_MAX_RETRIES || args.isClosed()) {
+      throw outcome.error;
+    }
+    args.discardFailedAttempt();
+    await args.waitBeforeRetry();
+  }
+}
+
 export interface PiRpcSessionState {
   model?: { provider?: string; id?: string; contextWindow?: number };
   thinkingLevel?: string;
@@ -198,30 +230,21 @@ export class PiRpcSession {
    * resolved its model): respawn a few times before failing.
    */
   async start(): Promise<void> {
-    for (let attempt = 0; ; attempt += 1) {
-      const outcome = await this.spawnAndVerify();
-      if (outcome.ok) {
-        return;
-      }
-      if (attempt >= PI_TRANSIENT_AUTH_MAX_RETRIES || this.closed) {
-        throw outcome.error;
-      }
-      // The failed attempt's child is detached before it is killed: its exit
-      // (async, possibly after the next attempt spawned) must neither report
-      // a session error for a thread still under construction nor touch the
-      // next child's readiness, leaf waiter, or processing state.
-      const failed = this.child;
-      this.child = undefined;
-      failed?.kill();
-      await new Promise((resolve) =>
-        setTimeout(resolve, PI_TRANSIENT_AUTH_RETRY_DELAY_MS),
-      );
-    }
+    await runPiTransientAuthConstruction({
+      attempt: () => this.spawnAndVerify(),
+      discardFailedAttempt: () => {
+        // A retried child is detached before it is killed: its late lines and
+        // exit must not touch the next child's state or report a session error.
+        const failed = this.child;
+        this.child = undefined;
+        failed?.kill();
+      },
+      isClosed: () => this.closed,
+      waitBeforeRetry: waitForPiTransientAuthRetry,
+    });
   }
 
-  private async spawnAndVerify(): Promise<
-    { ok: true } | { ok: false; error: Error }
-  > {
+  private async spawnAndVerify(): Promise<PiSessionConstructionOutcome> {
     const toolsFilePath = join(
       this.options.scratchDir,
       `pi-tools-${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}.json`,