fix(coding): the single-flight claim covered 1 of 3 paths that start a Pilot (#208)

#208 said "one driver per engine". It put the claim on
`POST /sessions/:id/run` and nowhere else. There are THREE paths that start a
Pilot against a live session, and the other two were wide open:

  routes/coding.ts:1332   /sessions/:id/run          ✅ claimed
  routes/coding.ts:1008   the `drive_claude` tool    ❌
  lib/loop-drivers.ts     owner Loop + delegate_goal ❌

So a Lead delegating a goal, or the owner pressing Loop, could put a SECOND
Pilot on a session a first was already driving — the two interleaving
`tmux send-keys` into one pane, each reasoning over a terminal the other is
writing to. That is exactly the failure the claim exists to prevent, and it was
reachable by the two most ordinary actions in the product. The route-only fix was
a sixth of a guarantee.

(The other three `CODING_SESSION.create` sites are `mode: "watch"` finish-watchers
— passive, they send no instructions, so they correctly take no claim.)

Both paths now claim BEFORE anything observable is written. Order matters: a claim
checked after the run row exists strands an `agent_loop_runs` row no workflow will
close (the #207C failure), and a refusal after the board write announces work that
never started. Each threads its `driverId` into the workflow so the existing
release path (`endSession`, #206) frees it.

Also removed `delegateToPilot` — 94 lines made unreachable by #210's driver table
and still carrying its own unclaimed `CODING_SESSION.create`. Dead code that
starts workflows is worse than dead code.

And raised the remaining browser-runner timeouts from 60s to 120s. `beforeEach`
launches a fresh real Chrome per test — eight per file — so a launch alone can eat
most of a 60s budget under load; two different tests there timed out in one
full-suite run and the file passed 8/8 in 27s in isolation right after. The
heaviest test was already bumped for precisely this; the rest were left to flake.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: serge-the-dev

packages/browser-runner/src/runner-browser.test.ts

@@ -76,16 +76,19 @@ describe("LocalRunner brain-driven browser endpoints", () => {
 		expect(sub.resume?.filename).toContain("resume");
 		expect(sub.resume?.size).toBeGreaterThan(0);
 		expect(result.url).toContain("/success/");
-		// 120s, not the file's usual 60s: this is the heaviest test here (8 sequential
-		// real-browser actions, ~22s on a dev machine) and it repeatedly flaked at 60s
-		// on CI's slower runners while passing on every rerun.
+		// 120s across this whole file, not 60s. `beforeEach` builds a new LocalRunner and launches
+		// a FRESH real Chrome for every test — eight launches per file — so under a loaded machine
+		// (a parallel vitest pool, or an e2e run alongside) a launch alone can eat most of a 60s
+		// budget. Two different tests here timed out at 60s in one full-suite run and the whole
+		// file passed 8/8 in 27s immediately after, in isolation. The heaviest test was already
+		// bumped for exactly this; the rest were left to flake.
 	}, 120_000);
 
 	it("snapshot reports a CAPTCHA challenge so the brain can hand off", async () => {
 		await runner.browserAct({ action: "navigate", url: `${server.jobUrl}?challenge=1` });
 		const snap = await runner.browserSnapshot();
 		expect(snap.challenge).toBe("cloudflare-turnstile");
-	}, 60_000);
+	}, 120_000);
 
 	it("does NOT hand off for an invisible reCAPTCHA badge, but DOES for a visible checkbox", async () => {
 		// The Dayforce false-positive: an invisible v3 badge must be ignored.
@@ -94,7 +97,7 @@ describe("LocalRunner brain-driven browser endpoints", () => {
 		// A visible "I'm not a robot" checkbox IS a real challenge.
 		await runner.browserAct({ action: "navigate", url: `${server.jobUrl}?recaptcha=1` });
 		expect((await runner.browserSnapshot()).challenge).toBe("recaptcha");
-	}, 60_000);
+	}, 120_000);
 
 	it("agent handoff lifecycle: same-session pause → solved → resume → complete", async () => {
 		// Agent-driven task: created running, never auto-executed by the runner.
@@ -123,7 +126,7 @@ describe("LocalRunner brain-driven browser endpoints", () => {
 
 		await runner.browserComplete(task.id, "submitted", "Application received");
 		expect(runner.store.getTask(task.id)?.status).toBe("completed");
-	}, 60_000);
+	}, 120_000);
 
 	it("auto-handles a native dialog so the brain isn't wedged (real ATS: alert after résumé upload)", async () => {
 		// A page that pops a native alert — exactly what Coles does after résumé upload.
@@ -138,7 +141,7 @@ describe("LocalRunner brain-driven browser endpoints", () => {
 		const after = await runner.browserAct({ action: "key", key: "Enter" });
 		expect(after.ok).toBe(true);
 		expect(after.feedback ?? "").toMatch(/dialog was accepted/i);
-	}, 60_000);
+	}, 120_000);
 
 	it("reads back a masked field's real value so the brain doesn't oscillate", async () => {
 		// A phone field that transforms input like a real intl mask: "0404…" → "+61404…".
@@ -149,7 +152,7 @@ describe("LocalRunner brain-driven browser endpoints", () => {
 		// The runner reports what the field ACTUALLY holds now (the transformed value).
 		expect(res.feedback ?? "").toMatch(/now reads/i);
 		expect(res.feedback ?? "").toContain("+61404453580");
-	}, 60_000);
+	}, 120_000);
 
 	it("upload falls back to setInputFiles when the target isn't the chooser trigger (Ashby drop-zone)", async () => {
 		// Ashby: the labeled 'Resume' control is a drop-zone/button that does NOT open a
@@ -161,13 +164,13 @@ describe("LocalRunner brain-driven browser endpoints", () => {
 		await runner.browserAct({ action: "upload", ref: ref(snap.snapshot, "button", "Resume"), name: "Resume" }, resume());
 		// The input's onchange set the title to FILE:<name> → the file really attached.
 		expect((await runner.browserSnapshot()).title).toContain("FILE:");
-	}, 60_000);
+	}, 120_000);
 
 	it("a failed action (bad ref) throws so the workflow surfaces it to the brain", async () => {
 		await runner.browserAct({ action: "navigate", url: server.jobUrl });
 		await runner.browserSnapshot();
 		// A ref that doesn't exist must not silently succeed — it throws (→ the
 		// workflow maps it to `error`, driving the brain's self-correction).
 		await expect(runner.browserAct({ action: "click", ref: "e99999", name: "ghost" })).rejects.toThrow();
-	}, 60_000);
+	}, 120_000);
 });

workers/api/src/lib/delegate-instance.ts

@@ -23,9 +23,6 @@ import { DEFAULT_LOOP_DRIVER, loopDriverFor } from "./loop-drivers.js";
 import { capabilitiesForInstance } from "./agent-capabilities.js";
 import { sanitizeMaxIterations } from "./agent-loop.js";
 import { logEvent } from "./events.js";
-import { agentCapabilities } from "./agent-capabilities.js";
-import { getActiveSessionForRepo, listRepos } from "./coding-store.js";
-import { delegationTaskRecord } from "./delegation.js";
 import type { Env } from "../types.js";
 
 export interface DelegateInstanceInput {
@@ -144,98 +141,3 @@ export async function delegateToInstance(env: Env, input: DelegateInstanceInput)
 
 	return { ok: true, runId, budgetId, depth };
 }
-
-/**
- * Delegate to a coding agent's Pilot — the durable loop that drives a real CLI.
- *
- * Mirrors what the hardcoded Overseer does, with the repo resolved from the subordinate rather
- * than named by the caller: a Repo Coder owns exactly one repo, so the supervisor should not have
- * to know which. Refusals are explicit, because the alternative — a board card that looks
- * delegated and never moves — is the failure #154 was written to kill.
- */
-async function delegateToPilot(
-	env: Env,
-	input: DelegateInstanceInput,
-	depth: number,
-	budgetId: string,
-	objective: string,
-): Promise<DelegateInstanceResult> {
-	const repos = await listRepos(env, input.subordinateInstanceId, input.userId).catch(() => []);
-	const repo = repos[0];
-	if (!repo) {
-		return {
-			ok: false,
-			status: 409,
-			error: "That coding agent has no repository yet — add one on its Coding tab first.",
-		};
-	}
-	const session = await getActiveSessionForRepo(env, input.subordinateInstanceId, input.userId, repo.id);
-	if (!session) {
-		return {
-			ok: false,
-			status: 409,
-			error: `${repo.name} has no live coding session — start one on its Coding tab (and run \`pags up\`), then delegate again.`,
-		};
-	}
-
-	// Observable board task, so a delegated goal is trackable rather than buried in one repo's
-	// thread. The Pilot flips it to completed/failed at its terminal state.
-	const taskId = `deleg-${crypto.randomUUID()}`;
-
-	// AND a loop-run row, so `check_delegation` answers for BOTH delegation kinds. Without it
-	// the supervisor is handed a run id, told to track it, and gets "no run with that id" —
-	// the tool's own instructions would be a lie.
-	const runId = crypto.randomUUID();
-	await createLoopRun(env, {
-		runId,
-		userId: input.userId,
-		instanceId: input.subordinateInstanceId,
-		objective,
-		maxIterations: sanitizeMaxIterations(input.maxIterations),
-		budgetId,
-		startedAt: Date.now(),
-	}).catch(() => undefined);
-	await env.DB.prepare(
-		`INSERT INTO instance_runtime_tasks (id, instance_id, user_id, type, status, payload, created_at, updated_at)
-		 VALUES (?1, ?2, ?3, 'delegation', 'running', ?4, datetime('now'), datetime('now'))`,
-	)
-		.bind(
-			taskId,
-			input.subordinateInstanceId,
-			input.userId,
-			JSON.stringify(delegationTaskRecord({ id: taskId, targetLabel: repo.name, objective, status: "running", now: new Date().toISOString() })),
-		)
-		.run()
-		.catch(() => undefined);
-
-	await env.CODING_SESSION.create({
-		params: {
-			instanceId: input.subordinateInstanceId,
-			userId: input.userId,
-			sessionId: session.id,
-			repoId: repo.id,
-			runnerNode: session.runnerNode ?? null,
-			cloneUrl: repo.cloneUrl ?? undefined,
-			branch: repo.branch || undefined,
-			goal: { objective, repo: repo.name, clientType: session.clientType },
-			boardTaskId: taskId,
-			// #184: a DELEGATED coding run draws on the tree's pool. A human driving the same
-			// Pilot from the Coding tab passes no budget and stays unmetered, as before.
-			budgetId,
-			depth,
-			loopRunId: runId,
-		},
-	});
-
-	await logEvent(env, {
-		source: "coding",
-		event: "delegate",
-		message: `${input.supervisorInstanceId} → ${repo.name}: ${objective.slice(0, 160)}`,
-		userId: input.userId,
-		instanceId: input.supervisorInstanceId,
-		traceId: input.parentTraceId ?? taskId,
-		context: { taskId, depth, budgetId, subordinate: input.subordinateInstanceId },
-	}).catch(() => undefined);
-
-	return { ok: true, runId, budgetId, depth };
-}

workers/api/src/lib/loop-drivers.test.ts

@@ -36,7 +36,7 @@ describe("loopDriverFor — the ONE Loop dispatches on what the agent DECLARES (
 });
 
 /** Env stub recording the writes + which Workflow binding was created. */
-function stubEnv(opts: { repos?: unknown[]; session?: unknown } = {}) {
+function stubEnv(opts: { repos?: unknown[]; session?: unknown; claimTaken?: boolean } = {}) {
 	const sql: string[] = [];
 	const created: Array<{ binding: string; params: Record<string, unknown> }> = [];
 	const wf = (binding: string) => ({ create: vi.fn(async (a: { params: Record<string, unknown> }) => { created.push({ binding, params: a.params }); return { id: "wf" }; }) });
@@ -47,7 +47,8 @@ function stubEnv(opts: { repos?: unknown[]; session?: unknown } = {}) {
 				return {
 					bind() {
 						return {
-							async run() { return { meta: { changes: 1 } }; },
+							// A claim whose predicate matched nothing = somebody else holds it.
+							async run() { return { meta: { changes: opts.claimTaken && q.includes("driver_id") ? 0 : 1 } }; },
 							async all() { return { results: opts.repos ?? [] }; },
 							async first() { return opts.session ?? null; },
 						};
@@ -127,3 +128,47 @@ describe('the "Delegated:" board card belongs to a supervisor, not to the owner'
 		expect(String(created[0].params.boardTaskId)).toMatch(/^deleg-/);
 	});
 });
+
+describe("one driver per engine — every path that starts a Pilot claims it (#208 hole)", () => {
+	const codingEnv = (claimTaken = false) => stubEnv({
+		repos: [{ id: "r1", name: "fws/platform" }],
+		session: { id: "s1", client_type: "claude" },
+		claimTaken,
+	});
+
+	it("claims the session before creating the workflow", async () => {
+		// #208 put the claim on `/sessions/:id/run` ONLY. This path — the owner's Loop button and
+		// every `delegate_goal` to a coding agent — reaches the same tmux pane and was left open,
+		// so two Pilots could interleave `send-keys` into one terminal, each reasoning over output
+		// the other was writing.
+		const { env, sql, created } = codingEnv();
+		const out = await loopDriverFor(caps("CODING_SESSION")).start({ env, ...base });
+		expect(out.ok).toBe(true);
+		expect(sql.some((q) => q.includes("driver_id") && q.includes("UPDATE coding_sessions"))).toBe(true);
+		// The Pilot must carry the claim, or nothing releases it when the run ends.
+		expect(created[0].params.driverId).toBeTruthy();
+	});
+
+	it("refuses — and starts NOTHING — when another driver already holds the session", async () => {
+		const { env, sql, created } = codingEnv(true);
+		const out = await loopDriverFor(caps("CODING_SESSION")).start({ env, ...base, delegated: true });
+		expect(out).toMatchObject({ ok: false, status: 409 });
+		if (!out.ok) expect(out.error).toMatch(/already being worked on/i);
+		// No workflow, no loop-run row, and no "Delegated:" card announcing work that never began.
+		expect(created).toHaveLength(0);
+		expect(sql.some((q) => q.includes("INSERT INTO agent_loop_runs"))).toBe(false);
+		expect(sql.some((q) => q.includes("'delegation'"))).toBe(false);
+	});
+
+	it("claims BEFORE opening the run row, so a refusal leaves no orphan", async () => {
+		// Order matters: a claim checked after the row is written leaves an `agent_loop_runs` row
+		// that no workflow will ever close — the stranded-row failure #207C exists to sweep up.
+		const { env, sql } = codingEnv();
+		await loopDriverFor(caps("CODING_SESSION")).start({ env, ...base });
+		const claimAt = sql.findIndex((q) => q.includes("driver_id") && q.includes("UPDATE coding_sessions"));
+		const runAt = sql.findIndex((q) => q.includes("INSERT INTO agent_loop_runs"));
+		expect(claimAt).toBeGreaterThanOrEqual(0);
+		expect(claimAt).toBeLessThan(runAt);
+	});
+});
+

workers/api/src/lib/loop-drivers.ts

@@ -21,7 +21,7 @@
 import { createLoopRun } from "./agent-loop-store.js";
 import { sanitizeMaxIterations } from "./agent-loop.js";
 import { delegationTaskRecord } from "./delegation.js";
-import { getActiveSessionForRepo, listRepos } from "./coding-store.js";
+import { claimSessionDriver, getActiveSessionForRepo, listRepos } from "./coding-store.js";
 import type { AgentCapabilities } from "./agent-capabilities.js";
 import type { Env } from "../types.js";
 
@@ -115,6 +115,20 @@ const codingDriver: LoopDriver = {
 			};
 		}
 
+		// Single-flight, same as `/sessions/:id/run` (#208). Without it a Lead delegating a goal —
+		// or the owner pressing Loop — starts a SECOND Pilot on a session that is already being
+		// driven, and the two interleave `tmux send-keys` into the same pane, each reasoning over
+		// a terminal the other is writing to. #208 added the claim to the route only; these paths
+		// reach the same engine and were left open, so the guarantee was a sixth of a guarantee.
+		const driverId = crypto.randomUUID();
+		if (!(await claimSessionDriver(env, instanceId, userId, session.id, driverId))) {
+			return {
+				ok: false,
+				status: 409,
+				error: `${repo.name} is already being worked on — wait for the current run to finish, or stop it first.`,
+			};
+		}
+
 		const runId = crypto.randomUUID();
 		const maxIterations = sanitizeMaxIterations(input.maxIterations);
 		await createLoopRun(env, {
@@ -158,6 +172,7 @@ const codingDriver: LoopDriver = {
 				budgetId: input.budgetId,
 				depth: input.depth,
 				loopRunId: runId,
+				driverId,
 			},
 		});
 		return { ok: true, runId, driver: codingDriver.id };

workers/api/src/routes/coding.ts

@@ -991,6 +991,16 @@ async function delegateToTarget(
 		return { ok: false, reply: `${targetLabel} has no live session — open it (or tap Start) first, then I can drive it.` };
 	}
 
+	// Single-flight BEFORE anything observable is written (#208). This is the third path that
+	// starts a Pilot on a real session, and it was the one still left open: the claim went on
+	// `/sessions/:id/run` only, so `drive_claude` could put a SECOND Pilot on a pane a first was
+	// already typing into. Claimed first so a refusal doesn't leave a board card + trace event
+	// announcing work that never started.
+	const driverId = crypto.randomUUID();
+	if (!(await claimSessionDriver(c.env, instanceId, uid, session.id, driverId))) {
+		return { ok: false, reply: `${targetLabel} is already being worked on — let the current run finish, or stop it first.` };
+	}
+
 	const taskId = `deleg-${crypto.randomUUID()}`;
 	const now = new Date().toISOString();
 	const label = objective.length > 120 ? `${objective.slice(0, 117)}…` : objective;
@@ -1010,6 +1020,7 @@ async function delegateToTarget(
 			instanceId, userId: uid, sessionId: session.id, repoId: repo.id,
 			runnerNode: session.runnerNode ?? null, cloneUrl: repo.cloneUrl ?? undefined,
 			branch: repo.branch || undefined, token: token ?? undefined, goal, boardTaskId: taskId,
+			driverId,
 		},
 	});
 	return { ok: true, taskId, label: targetLabel, reply: `On it — delegated to ${repo.name}; track it on the board.` };