Fix startup crash when a stuck workdir lock survives an interrupt (#596)

* Report a stuck workdir lock as a clear error instead of an unhandled rejection

close() on the agent package releases its workdir lock only after
reactor.abort()/sendQueue.drain() and the shutdown-complete race finish. A
throw partway through (most likely right when an operator interrupts
mid-inference, exactly when those paths are stressed) leaves the lock held
forever in-process: the agent is already marked closed, so retrying close()
is a silent no-op that can never release it. The next buildAgent() for that
workdir then throws AgentContextLockError, and reloadIfIdle's rebuild had no
try/catch around it, so the throw escaped as an unhandled rejection and
crashed the process.

Route every rebuild site (interrupt, reload, session rotation) through a
shared close-then-check helper: a failed close now short-circuits the
rebuild instead of attempting a second, doomed acquisition, and the failure
surfaces as a plain-language caught error.

* Document rotation's exemption and prove the fix through the real queue

Session rotation was never routed through closeAgentForRebuild: it mints a
fresh sessionId/workdir before rebuilding, so a leaked lock on the old
workdir can never be re-acquired there. Write that reasoning down at the
call site and next to closeAgentForRebuild's doc comment, since the
asymmetry across the three rebuild sites needs an explanation the next
reader can find.

Replace the helper-only regression test with one that drives the real
session-operation-queue the same way reloadIfIdle actually calls it (void
enqueue(...), no awaited return value) and asserts, via a real
process.on("unhandledRejection") listener, that the rejection is contained
and surfaces through fatalBuildError instead of escaping. reloadIfIdle
itself can't be reached in isolation without standing up the full TUI
runner; that's noted at the test.

Author: TheGreatAxios

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.