Skip to content

Commit 2034d9d

Browse files
committed
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.
1 parent ea84995 commit 2034d9d

3 files changed

Lines changed: 132 additions & 20 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,15 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
2929
the subtree-scoping rule for those is written and tested but not yet wired to
3030
a live call site. `task()` is unchanged and still the only spawn verb.
3131

32+
### Fixed
33+
34+
- **Interrupting a turn no longer risks a startup crash.** If an interrupt hit
35+
the agent mid-teardown, a failed close could leave its in-process workdir
36+
lock stuck held, and the immediate rebuild threw "an agent is already open"
37+
as an unhandled rejection. A failed close now short-circuits the rebuild
38+
with a clear, catchable error instead of retrying a doomed second
39+
acquisition.
40+
3241
## [0.2.107] - 2026-08-24
3342

3443
### Agent

‎src/tui/runner.ts‎

Lines changed: 60 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
defineTool,
77
createDirectorRegistry,
88
defineDirector,
9+
AgentContextLockError,
910
type Agent,
1011
} from "@intx/agent";
1112
import { noopAuditStore, permissiveAuthorize } from "@intx/agent/testing";
@@ -283,6 +284,42 @@ export function resumeTranscriptLoadErrorBlock(err: unknown): {
283284
return { type: "error", message: `Could not load prior session transcript: ${message}` };
284285
}
285286

287+
// The agent package releases its workdir lock at the very end of close(),
288+
// after reactor.abort()/sendQueue.drain() and the shutdown-complete race have
289+
// all run. If any of that throws (most likely right when an operator
290+
// interrupts mid-inference, which is exactly when those paths are under
291+
// stress), the lock is never released — and because the agent is already
292+
// marked closed internally, retrying close() is a silent no-op that can
293+
// never release it either. Every rebuild site must treat that as fatal for
294+
// the current rebuild instead of calling buildAgent() again: a second
295+
// createAgent() for the same workdir is then guaranteed to throw
296+
// AgentContextLockError for a lock nothing will ever free, which is the
297+
// "agent already open" crash.
298+
export async function closeAgentForRebuild(agent: Agent, context: string): Promise<boolean> {
299+
try {
300+
await agent.close();
301+
return true;
302+
} catch (err) {
303+
tuiLogger.debug(`agent.close during ${context} teardown failed: {error}`, {
304+
error: err instanceof Error ? err.message : String(err),
305+
});
306+
return false;
307+
}
308+
}
309+
310+
// Every rebuild site funnels its failure (a lock left held by a failed
311+
// close, or any other buildAgent failure) through here so it surfaces as a
312+
// plain-language, caught error rather than an unhandled rejection.
313+
export function agentRebuildFailure(err: unknown): Error {
314+
return err instanceof AgentContextLockError
315+
? new Error(
316+
"Could not start a new agent: the previous one did not shut down cleanly. Restart Corbits to continue.",
317+
)
318+
: err instanceof Error
319+
? err
320+
: new Error(String(err));
321+
}
322+
286323
export interface ResumeSeed {
287324
turnsUsed: number;
288325
mcpServers: ConnectedMcpServer[];
@@ -1646,21 +1683,25 @@ export async function runTUI(initialConfig: Config): Promise<number> {
16461683
if (!pendingReload || inFlight > 0) return;
16471684
pendingReload = false;
16481685
void enqueueOp(async () => {
1649-
const old = currentAgent;
1650-
await old.close().catch((err: unknown) => {
1651-
tuiLogger.debug("agent.close during reload teardown failed: {error}", {
1652-
error: err instanceof Error ? err.message : String(err),
1653-
});
1654-
});
1655-
await streamPromise.catch((err: unknown) => {
1656-
tuiLogger.debug("stream drain during reload teardown failed: {error}", {
1657-
error: err instanceof Error ? err.message : String(err),
1686+
try {
1687+
const old = currentAgent;
1688+
const closedCleanly = await closeAgentForRebuild(old, "reload");
1689+
await streamPromise.catch((err: unknown) => {
1690+
tuiLogger.debug("stream drain during reload teardown failed: {error}", {
1691+
error: err instanceof Error ? err.message : String(err),
1692+
});
16581693
});
1659-
});
1660-
currentAgent = await buildAgent();
1661-
streamPromise = consumeStream(currentAgent.stream(), streamSink);
1662-
// The rebuild made a fresh director; re-attach the active workflow.
1663-
workflowController.reattach();
1694+
if (!closedCleanly) {
1695+
throw new AgentContextLockError(workdir);
1696+
}
1697+
currentAgent = await buildAgent();
1698+
streamPromise = consumeStream(currentAgent.stream(), streamSink);
1699+
// The rebuild made a fresh director; re-attach the active workflow.
1700+
workflowController.reattach();
1701+
} catch (err) {
1702+
recordRunError(err);
1703+
fatalBuildError = agentRebuildFailure(err);
1704+
}
16641705
});
16651706
};
16661707

@@ -1812,24 +1853,23 @@ export async function runTUI(initialConfig: Config): Promise<number> {
18121853
// and salvages the buffer before that teardown, so it is never lost
18131854
// or misattributed to the rebuilt agent's next cycle.
18141855
await cycleRecorder.dispose("interrupted");
1815-
await currentAgent.close().catch((err: unknown) => {
1816-
tuiLogger.debug("agent.close during interrupt teardown failed: {error}", {
1817-
error: err instanceof Error ? err.message : String(err),
1818-
});
1819-
});
1856+
const closedCleanly = await closeAgentForRebuild(currentAgent, "interrupt");
18201857
await streamPromise.catch((err: unknown) => {
18211858
tuiLogger.debug("stream drain during interrupt teardown failed: {error}", {
18221859
error: err instanceof Error ? err.message : String(err),
18231860
});
18241861
});
1862+
if (!closedCleanly) {
1863+
throw new AgentContextLockError(workdir);
1864+
}
18251865
currentAgent = await buildAgent();
18261866
cycleRecorder.reset();
18271867
streamPromise = consumeStream(currentAgent.stream(), streamSink);
18281868
workflowController.reattach();
18291869
fatalBuildError = null;
18301870
} catch (err) {
18311871
recordRunError(err);
1832-
fatalBuildError = err instanceof Error ? err : new Error(String(err));
1872+
fatalBuildError = agentRebuildFailure(err);
18331873
}
18341874
});
18351875
};

‎tests/unit/tui/runner.test.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { test, expect } from "bun:test";
22
import { EventEmitter } from "node:events";
3+
import { AgentContextLockError, type Agent } from "@intx/agent";
34
import {
5+
agentRebuildFailure,
6+
closeAgentForRebuild,
47
createTUIEventEmitter,
58
getTUIRunSummaryStatus,
69
loadLocalSettingsWriteBase,
@@ -97,3 +100,63 @@ test("rotation resets run-sink so a new session starts from a clean state", () =
97100
expect(runSink.getStatus()).toBe("done");
98101
expect(collectorAfterReset.getTurns()).toHaveLength(0);
99102
});
103+
104+
// CL-5753: an interrupt can hit close() while reactor.abort()/sendQueue.drain()
105+
// are mid-teardown, throwing before @intx/agent's close() ever reaches
106+
// lock.release(). Once that happens the agent is already marked closed, so a
107+
// retried close() is a silent no-op that can never free the lock either — the
108+
// workdir's lock is stuck held for the rest of the process. The next
109+
// buildAgent() for that same workdir is then guaranteed to throw
110+
// AgentContextLockError ("an agent is already open for workdir: ..."), which
111+
// is the crash from the ticket. These tests cover the two functions the
112+
// runner now routes every rebuild through so that failure is reported in
113+
// plain language rather than escaping as an unhandled rejection.
114+
function stubAgent(closeImpl: () => Promise<void>): Agent {
115+
return { close: closeImpl } as unknown as Agent;
116+
}
117+
118+
test("closeAgentForRebuild reports a failed close without throwing", async () => {
119+
const agent = stubAgent(() => Promise.reject(new AgentContextLockError("/tmp/workdir")));
120+
const closedCleanly = await closeAgentForRebuild(agent, "interrupt");
121+
expect(closedCleanly).toBe(false);
122+
});
123+
124+
test("closeAgentForRebuild reports success when close() resolves", async () => {
125+
const agent = stubAgent(() => Promise.resolve());
126+
const closedCleanly = await closeAgentForRebuild(agent, "interrupt");
127+
expect(closedCleanly).toBe(true);
128+
});
129+
130+
test("agentRebuildFailure turns a stale-lock AgentContextLockError into a plain-language message", () => {
131+
// Simulates the second acquisition throwing after a failed close left the
132+
// lock held: buildAgent() surfaces AgentContextLockError, which must not
133+
// reach the caller as a raw stack trace.
134+
const err = agentRebuildFailure(new AgentContextLockError("/tmp/workdir"));
135+
expect(err.message).not.toContain("already open");
136+
expect(err.message).toMatch(/restart/i);
137+
});
138+
139+
test("agentRebuildFailure passes other errors through unchanged", () => {
140+
const original = new Error("network unreachable");
141+
expect(agentRebuildFailure(original)).toBe(original);
142+
});
143+
144+
test("a failed close followed by a lock error never surfaces as a raw AgentContextLockError", async () => {
145+
// End-to-end shape of the fix: close() throws (lock leaked in-process),
146+
// the rebuild site short-circuits instead of calling buildAgent() again,
147+
// and the resulting error is the plain-language one — never the raw
148+
// AgentContextLockError a bare `throw` would have produced.
149+
const agent = stubAgent(() => Promise.reject(new AgentContextLockError("/tmp/workdir")));
150+
let rebuildError: Error | null = null;
151+
try {
152+
const closedCleanly = await closeAgentForRebuild(agent, "interrupt");
153+
if (!closedCleanly) {
154+
throw new AgentContextLockError("/tmp/workdir");
155+
}
156+
} catch (err) {
157+
rebuildError = agentRebuildFailure(err);
158+
}
159+
expect(rebuildError).not.toBeNull();
160+
expect(rebuildError).not.toBeInstanceOf(AgentContextLockError);
161+
expect(rebuildError!.message).toMatch(/restart/i);
162+
});

0 commit comments

Comments
 (0)