Fix startup crash when a stuck workdir lock survives an interrupt

CHANGELOG.md

@@ -29,6 +29,15 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
   the subtree-scoping rule for those is written and tested but not yet wired to
   a live call site. `task()` is unchanged and still the only spawn verb.
 
+### Fixed
+
+- **Interrupting a turn no longer risks a startup crash.** If an interrupt hit
+  the agent mid-teardown, a failed close could leave its in-process workdir
+  lock stuck held, and the immediate rebuild threw "an agent is already open"
+  as an unhandled rejection. A failed close now short-circuits the rebuild
+  with a clear, catchable error instead of retrying a doomed second
+  acquisition.
+
 ## [0.2.107] - 2026-08-24
 
 ### Agent

src/tui/runner.ts

@@ -6,6 +6,7 @@ import {
   defineTool,
   createDirectorRegistry,
   defineDirector,
+  AgentContextLockError,
   type Agent,
 } from "@intx/agent";
 import { noopAuditStore, permissiveAuthorize } from "@intx/agent/testing";
@@ -283,6 +284,46 @@ export function resumeTranscriptLoadErrorBlock(err: unknown): {
   return { type: "error", message: `Could not load prior session transcript: ${message}` };
 }
 
+// The agent package releases its workdir lock at the very end of close(),
+// after reactor.abort()/sendQueue.drain() and the shutdown-complete race have
+// all run. If any of that throws (most likely right when an operator
+// interrupts mid-inference, which is exactly when those paths are under
+// stress), the lock is never released — and because the agent is already
+// marked closed internally, retrying close() is a silent no-op that can
+// never release it either. Every rebuild site that reuses the *same* workdir
+// (interrupt, reloadIfIdle) must treat that as fatal for the current rebuild
+// instead of calling buildAgent() again: a second createAgent() for the same
+// workdir is then guaranteed to throw AgentContextLockError for a lock
+// nothing will ever free, which is the "agent already open" crash. Session
+// rotation (newSession) is the one rebuild site that does NOT route through
+// this helper: it always points buildAgent() at a freshly minted workdir
+// before rebuilding, so a leaked lock on the old workdir can never be
+// re-acquired there — see the comment at its close() call for why.
+export async function closeAgentForRebuild(agent: Agent, context: string): Promise<boolean> {
+  try {
+    await agent.close();
+    return true;
+  } catch (err) {
+    tuiLogger.debug(`agent.close during ${context} teardown failed: {error}`, {
+      error: err instanceof Error ? err.message : String(err),
+    });
+    return false;
+  }
+}
+
+// Every rebuild site funnels its failure (a lock left held by a failed
+// close, or any other buildAgent failure) through here so it surfaces as a
+// plain-language, caught error rather than an unhandled rejection.
+export function agentRebuildFailure(err: unknown): Error {
+  return err instanceof AgentContextLockError
+    ? new Error(
+        "Could not start a new agent: the previous one did not shut down cleanly. Restart Corbits to continue.",
+      )
+    : err instanceof Error
+      ? err
+      : new Error(String(err));
+}
+
 export interface ResumeSeed {
   turnsUsed: number;
   mcpServers: ConnectedMcpServer[];
@@ -1646,21 +1687,25 @@ export async function runTUI(initialConfig: Config): Promise<number> {
       if (!pendingReload || inFlight > 0) return;
       pendingReload = false;
       void enqueueOp(async () => {
-        const old = currentAgent;
-        await old.close().catch((err: unknown) => {
-          tuiLogger.debug("agent.close during reload teardown failed: {error}", {
-            error: err instanceof Error ? err.message : String(err),
-          });
-        });
-        await streamPromise.catch((err: unknown) => {
-          tuiLogger.debug("stream drain during reload teardown failed: {error}", {
-            error: err instanceof Error ? err.message : String(err),
+        try {
+          const old = currentAgent;
+          const closedCleanly = await closeAgentForRebuild(old, "reload");
+          await streamPromise.catch((err: unknown) => {
+            tuiLogger.debug("stream drain during reload teardown failed: {error}", {
+              error: err instanceof Error ? err.message : String(err),
+            });
           });
-        });
-        currentAgent = await buildAgent();
-        streamPromise = consumeStream(currentAgent.stream(), streamSink);
-        // The rebuild made a fresh director; re-attach the active workflow.
-        workflowController.reattach();
+          if (!closedCleanly) {
+            throw new AgentContextLockError(workdir);
+          }
+          currentAgent = await buildAgent();
+          streamPromise = consumeStream(currentAgent.stream(), streamSink);
+          // The rebuild made a fresh director; re-attach the active workflow.
+          workflowController.reattach();
+        } catch (err) {
+          recordRunError(err);
+          fatalBuildError = agentRebuildFailure(err);
+        }
       });
     };
 
@@ -1812,24 +1857,23 @@ export async function runTUI(initialConfig: Config): Promise<number> {
           // and salvages the buffer before that teardown, so it is never lost
           // or misattributed to the rebuilt agent's next cycle.
           await cycleRecorder.dispose("interrupted");
-          await currentAgent.close().catch((err: unknown) => {
-            tuiLogger.debug("agent.close during interrupt teardown failed: {error}", {
-              error: err instanceof Error ? err.message : String(err),
-            });
-          });
+          const closedCleanly = await closeAgentForRebuild(currentAgent, "interrupt");
           await streamPromise.catch((err: unknown) => {
             tuiLogger.debug("stream drain during interrupt teardown failed: {error}", {
               error: err instanceof Error ? err.message : String(err),
             });
           });
+          if (!closedCleanly) {
+            throw new AgentContextLockError(workdir);
+          }
           currentAgent = await buildAgent();
           cycleRecorder.reset();
           streamPromise = consumeStream(currentAgent.stream(), streamSink);
           workflowController.reattach();
           fatalBuildError = null;
         } catch (err) {
           recordRunError(err);
-          fatalBuildError = err instanceof Error ? err : new Error(String(err));
+          fatalBuildError = agentRebuildFailure(err);
         }
       });
     };
@@ -1861,6 +1905,15 @@ export async function runTUI(initialConfig: Config): Promise<number> {
           // settles, and a dead cycle's partial must land in the session that
           // produced it, not the fresh one.
           await cycleRecorder.dispose("rotation");
+          // Deliberately not routed through closeAgentForRebuild/
+          // agentRebuildFailure (unlike interrupt and reloadIfIdle, CL-5753):
+          // rotation mints a fresh sessionId/workdir below before calling
+          // buildAgent(), so even a close() that leaks the old workdir's lock
+          // (see closeAgentForRebuild's doc comment) can never cause a second
+          // acquisition on that same workdir — buildAgent() always targets
+          // the new, unlocked directory. The old lock still leaks for the
+          // rest of the process, but nothing ever tries to re-acquire it, so
+          // there is no crash to guard against here.
           await currentAgent.close().catch((err: unknown) => {
             tuiLogger.debug("agent.close during session-rotation teardown failed: {error}", {
               error: err instanceof Error ? err.message : String(err),

tests/unit/tui/runner.test.ts

@@ -1,11 +1,15 @@
 import { test, expect } from "bun:test";
 import { EventEmitter } from "node:events";
+import { AgentContextLockError, type Agent } from "@intx/agent";
 import {
+  agentRebuildFailure,
+  closeAgentForRebuild,
   createTUIEventEmitter,
   getTUIRunSummaryStatus,
   loadLocalSettingsWriteBase,
   resumeTranscriptLoadErrorBlock,
 } from "../../../src/tui/runner.js";
+import { createSessionOperationQueue } from "../../../src/tui/session-operation-queue.js";
 import { createRunSink } from "../../../src/session/run-sink.js";
 
 test("createTUIEventEmitter returns an EventEmitter", () => {
@@ -97,3 +101,134 @@ test("rotation resets run-sink so a new session starts from a clean state", () =
   expect(runSink.getStatus()).toBe("done");
   expect(collectorAfterReset.getTurns()).toHaveLength(0);
 });
+
+// CL-5753: an interrupt can hit close() while reactor.abort()/sendQueue.drain()
+// are mid-teardown, throwing before @intx/agent's close() ever reaches
+// lock.release(). Once that happens the agent is already marked closed, so a
+// retried close() is a silent no-op that can never free the lock either — the
+// workdir's lock is stuck held for the rest of the process. The next
+// buildAgent() for that same workdir is then guaranteed to throw
+// AgentContextLockError ("an agent is already open for workdir: ..."), which
+// is the crash from the ticket. These tests cover the two functions the
+// runner now routes every rebuild through so that failure is reported in
+// plain language rather than escaping as an unhandled rejection.
+function stubAgent(closeImpl: () => Promise<void>): Agent {
+  return { close: closeImpl } as unknown as Agent;
+}
+
+test("closeAgentForRebuild reports a failed close without throwing", async () => {
+  const agent = stubAgent(() => Promise.reject(new AgentContextLockError("/tmp/workdir")));
+  const closedCleanly = await closeAgentForRebuild(agent, "interrupt");
+  expect(closedCleanly).toBe(false);
+});
+
+test("closeAgentForRebuild reports success when close() resolves", async () => {
+  const agent = stubAgent(() => Promise.resolve());
+  const closedCleanly = await closeAgentForRebuild(agent, "interrupt");
+  expect(closedCleanly).toBe(true);
+});
+
+test("agentRebuildFailure turns a stale-lock AgentContextLockError into a plain-language message", () => {
+  // Simulates the second acquisition throwing after a failed close left the
+  // lock held: buildAgent() surfaces AgentContextLockError, which must not
+  // reach the caller as a raw stack trace.
+  const err = agentRebuildFailure(new AgentContextLockError("/tmp/workdir"));
+  expect(err.message).not.toContain("already open");
+  expect(err.message).toMatch(/restart/i);
+});
+
+test("agentRebuildFailure passes other errors through unchanged", () => {
+  const original = new Error("network unreachable");
+  expect(agentRebuildFailure(original)).toBe(original);
+});
+
+test("a failed close followed by a lock error never surfaces as a raw AgentContextLockError", async () => {
+  // End-to-end shape of the fix: close() throws (lock leaked in-process),
+  // the rebuild site short-circuits instead of calling buildAgent() again,
+  // and the resulting error is the plain-language one — never the raw
+  // AgentContextLockError a bare `throw` would have produced.
+  const agent = stubAgent(() => Promise.reject(new AgentContextLockError("/tmp/workdir")));
+  let rebuildError: Error | null = null;
+  try {
+    const closedCleanly = await closeAgentForRebuild(agent, "interrupt");
+    if (!closedCleanly) {
+      throw new AgentContextLockError("/tmp/workdir");
+    }
+  } catch (err) {
+    rebuildError = agentRebuildFailure(err);
+  }
+  expect(rebuildError).not.toBeNull();
+  expect(rebuildError).not.toBeInstanceOf(AgentContextLockError);
+  expect(rebuildError!.message).toMatch(/restart/i);
+});
+
+// reloadIfIdle itself is a closure captured inside runTUI's single ~2500-line
+// scope (currentAgent, buildAgent, streamPromise, workflowController,
+// pendingReload/inFlight, fatalBuildError, etc. are all local variables of
+// that function), with no seam to construct or call it in isolation short of
+// standing up the full TUI runner — provider config, plugin discovery, MCP
+// wiring, and a real OpenTUI host. That is out of scope for this fix; it
+// would be its own extraction. What can be driven directly, and is exactly
+// the failure this bug reports, is the real `session-operation-queue.ts`
+// queue exercised the same way every rebuild site uses it: `void
+// enqueueOp(async () => { try { ... } catch (err) { fatalBuildError = ... } })`.
+// `enqueue` is `tail = tail.then(op, op); return tail;` — if `op` rejects and
+// nothing internally catches it, that returned promise is the only thing
+// that ever observes the rejection, and `void` discards it, which is
+// precisely how the unhandled rejection in the ticket escaped.
+test("a rejecting reload op through the real session-operation-queue never triggers an unhandled rejection", async () => {
+  const { enqueue, awaitTail } = createSessionOperationQueue();
+  const agent = stubAgent(() => Promise.reject(new AgentContextLockError("/tmp/workdir")));
+
+  let unhandled: unknown = null;
+  const onUnhandledRejection = (reason: unknown): void => {
+    unhandled = reason;
+  };
+  process.on("unhandledRejection", onUnhandledRejection);
+
+  let fatalBuildError: Error | null = null;
+  try {
+    // Mirrors reloadIfIdle's body verbatim: close the current agent through
+    // closeAgentForRebuild, skip buildAgent() and throw instead of
+    // re-acquiring on a failed close, and land any failure in
+    // fatalBuildError via agentRebuildFailure — all behind `void enqueueOp`,
+    // exactly as the runner calls it.
+    void enqueue(async () => {
+      try {
+        const closedCleanly = await closeAgentForRebuild(agent, "reload");
+        if (!closedCleanly) {
+          throw new AgentContextLockError("/tmp/workdir");
+        }
+      } catch (err) {
+        fatalBuildError = agentRebuildFailure(err);
+      }
+    });
+
+    await awaitTail();
+    // Give any unhandled rejection queued by the engine a chance to fire
+    // before asserting its absence — it lands on a later microtask/macrotask
+    // than the awaited queue settlement.
+    await new Promise((resolve) => setTimeout(resolve, 0));
+  } finally {
+    process.off("unhandledRejection", onUnhandledRejection);
+  }
+
+  expect(unhandled).toBeNull();
+  expect(fatalBuildError).not.toBeNull();
+  expect(fatalBuildError).not.toBeInstanceOf(AgentContextLockError);
+  expect(fatalBuildError!.message).toMatch(/restart/i);
+});
+
+// A true negative control (reproducing reloadIfIdle's pre-fix shape — no
+// try/catch around the queued op — and asserting the rejection escapes) was
+// attempted here and deliberately removed: bun:test installs its own
+// `unhandledRejection` listener that fails whichever test is running the
+// instant one fires, regardless of what that test asserts, so a test
+// designed to prove an unhandled rejection *does* escape cannot pass in this
+// harness — it is intercepted before the assertion runs. That interception
+// is itself the strongest available evidence for the bug this fix removes:
+// the pre-fix `reloadIfIdle` body run through this exact harness fails the
+// suite outright (confirmed manually while writing this test), rather than
+// failing a single assertion. The test above is the harness-compatible half
+// of that pair: same real queue, same real helpers, proving the fixed shape
+// produces no such failure.