From e7ec7ba4f52d6ee2c64c976042c1c22cb69e7a6e Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Wed, 26 Aug 2026 01:14:10 +0800 Subject: [PATCH] fix: reconcile stale session statuses on boot; name the tool blocking a send MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes for the same confusing experience — returning to a session and being told it is busy or blocked, with nothing to act on. 1. Non-terminal statuses survived daemon restarts. `thinking` / `tool_running` / `waiting_approval` all describe work owned by a LIVE process: the provider's turn loop, and for approvals the in-memory `#pendingApprovals` resolvers. None of it survives a restart, and a resumed Session starts `idle` (`#status` is initialised, never restored from the row). The row was never corrected, so the session list advertised work that no longer existed. Observed on a live daemon: rows stuck in `tool_running` and `waiting_approval` for 11-19 days across many restarts. `Store.reconcileStaleSessionStatuses()` clears them at boot, before resumeSessions, so rows and live sessions agree from the first broadcast. `error` is deliberately preserved — a terminal state a human may still want to see, not an artifact of the process dying. 2. The mid-turn send rejection named nothing. Sending while an approval is pending correctly throws (falling through would close the live turn queue and auto-deny the approval), but the message only said "A tool approval is pending" — which tells the user something is waiting, not what, and offers no way to act. After stepping away it reads as the session having gone silent. It now names the tool, via the same maps the approval bar already uses (`#approvalIdToMessageId` -> `#toolCallMessages`), so there is no extra bookkeeping to keep in sync, and states plainly that the message was saved. Falls back to the old phrasing when no name resolves: the status is authoritative, the label is best-effort, and a missing name must never turn a clear refusal into a crash. Co-Authored-By: Claude Opus 5 (1M context) --- src/daemon/session-manager.ts | 14 +++++ src/daemon/session.ts | 27 ++++++++- src/daemon/store.ts | 27 +++++++++ src/tests/stale-status-reconcile.test.ts | 74 ++++++++++++++++++++++++ 4 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 src/tests/stale-status-reconcile.test.ts diff --git a/src/daemon/session-manager.ts b/src/daemon/session-manager.ts index a54ab22..a016567 100644 --- a/src/daemon/session-manager.ts +++ b/src/daemon/session-manager.ts @@ -572,6 +572,20 @@ export class SessionManager { * Rebuilds in-memory session objects and scrollback buffers. */ async resumeSessions(): Promise { + // Correct statuses stranded by a daemon that stopped mid-turn. A resumed + // Session always starts `idle` in memory, so a row still claiming + // `thinking` / `tool_running` / `waiting_approval` is pure staleness — it + // made the session list show work that no longer exists (and, for + // `waiting_approval`, an approval whose resolver died with the process). + // Done BEFORE resume so rows and live sessions agree from the first + // broadcast. + const corrected = this.#store.reconcileStaleSessionStatuses(); + if (corrected > 0) { + console.log( + `[codeoid] reconciled ${corrected} stale session status(es) left by a previous run`, + ); + } + // Reload the durable conductor identity first (design R2): the persisted // {identityId, wimseUri, apiKey} row is reused instead of re-registering, // so the conductor keeps ONE stable WIMSE URI across daemon restarts. diff --git a/src/daemon/session.ts b/src/daemon/session.ts index 17e3b50..de65a6a 100644 --- a/src/daemon/session.ts +++ b/src/daemon/session.ts @@ -1839,7 +1839,7 @@ export class Session { // the message is already persisted, the user re-sends after deciding. if (this.#status === "waiting_approval") { throw new Error( - "A tool approval is pending — approve or deny it before sending (this backend can't queue mid-turn).", + `${this.#describePendingApprovals()} — approve or deny it before sending (this backend can't queue mid-turn). Your message was saved; send again after deciding.`, ); } @@ -3334,6 +3334,31 @@ export class Session { // ── Internals ───────────────────────────────────────────────────────── + /** + * Human-readable summary of what is currently blocking a send. + * + * The old rejection just said "A tool approval is pending", which told the + * user that *something* was waiting but not what, and gave them nothing to + * act on — so a message sent after stepping away read as the session simply + * going silent. Tool names come from the same maps the approval bar uses + * (`#approvalIdToMessageId` → `#toolCallMessages`), so no extra bookkeeping. + * + * Falls back to the generic phrasing when the maps have no entry — the + * status is authoritative, the label is best-effort, and a missing name must + * never turn a clear refusal into a crash. + */ + #describePendingApprovals(): string { + const names: string[] = []; + for (const approvalId of this.#pendingApprovals.keys()) { + const msgId = this.#approvalIdToMessageId.get(approvalId); + const name = msgId ? this.#toolCallMessages.get(msgId)?.tool?.name : undefined; + if (name) names.push(name); + } + if (names.length === 0) return "A tool approval is pending"; + if (names.length === 1) return `The tool \`${names[0]}\` is waiting for approval`; + return `${names.length} tool approvals are pending (${names.join(", ")})`; + } + #waitForApproval( approvalId: string, ): Promise<{ approved: boolean; updatedInput?: Record }> { diff --git a/src/daemon/store.ts b/src/daemon/store.ts index fb57e0c..76e15aa 100644 --- a/src/daemon/store.ts +++ b/src/daemon/store.ts @@ -719,6 +719,33 @@ export class Store { .run(status, id); } + /** + * Reconcile non-terminal statuses left behind by a daemon that stopped + * mid-turn. Returns the number of rows corrected. + * + * `thinking` / `tool_running` / `waiting_approval` all describe work owned by + * a LIVE process: the provider's turn loop, and for approvals the in-memory + * `#pendingApprovals` resolvers. None of that survives a restart — a resumed + * Session starts at `idle` (`#status` is initialised, never restored) — but + * the row was never corrected, so `codeoid ls` and the web UI kept showing + * sessions as busy indefinitely. Observed on a live daemon: rows stuck in + * `tool_running` and `waiting_approval` for 11–19 days across restarts. + * + * `error` is deliberately preserved: it is a terminal state a human may still + * want to see, not an artifact of the process dying. + * + * Call once at boot, BEFORE resumeSessions, so resumed sessions and their + * rows agree from the first broadcast. + */ + reconcileStaleSessionStatuses(): number { + return this.#db + .prepare( + `UPDATE sessions SET status = 'idle' + WHERE status IN ('thinking', 'tool_running', 'waiting_approval')`, + ) + .run().changes; + } + /** * Rename a session (`session.rename`). The `name` column is otherwise * written ONLY by createSession's INSERT OR REPLACE, so without this a diff --git a/src/tests/stale-status-reconcile.test.ts b/src/tests/stale-status-reconcile.test.ts new file mode 100644 index 0000000..2843169 --- /dev/null +++ b/src/tests/stale-status-reconcile.test.ts @@ -0,0 +1,74 @@ +import { describe, test, expect, beforeEach, afterEach } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { Store } from "../daemon/store.js"; +import type { SessionStatus } from "../protocol/types.js"; + +/** + * `thinking` / `tool_running` / `waiting_approval` all describe work owned by a + * LIVE process — the provider's turn loop, and for approvals the in-memory + * resolver map. None of it survives a restart, and a resumed Session starts at + * `idle` (its `#status` field is initialised, never restored from the row). The + * rows were nonetheless left untouched, so the session list kept advertising + * work that no longer existed — observed stuck for 11-19 days across restarts + * on a live daemon. + */ +describe("reconcileStaleSessionStatuses", () => { + let dir: string; + let store: Store; + + const seed = (id: string, status: SessionStatus) => + store.createSession({ + id, + name: `s-${id}`, + workdir: "/tmp", + status, + createdBy: "test", + accountId: "acct", + projectId: "proj", + } as never); + + const statusOf = (id: string) => store.getSession(id)?.status; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "codeoid-reconcile-")); + store = new Store(join(dir, "codeoid.db")); + }); + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + test("clears every non-terminal status left by a dead process", () => { + seed("a", "thinking"); + seed("b", "tool_running"); + seed("c", "waiting_approval"); + + expect(store.reconcileStaleSessionStatuses()).toBe(3); + for (const id of ["a", "b", "c"]) expect(statusOf(id)).toBe("idle"); + }); + + test("preserves `error` — a terminal state a human may still want to see", () => { + seed("err", "error"); + seed("busy", "thinking"); + + // Only the live-work row is corrected. + expect(store.reconcileStaleSessionStatuses()).toBe(1); + expect(statusOf("err")).toBe("error"); + expect(statusOf("busy")).toBe("idle"); + }); + + test("leaves already-idle rows untouched and reports zero", () => { + seed("calm", "idle"); + expect(store.reconcileStaleSessionStatuses()).toBe(0); + expect(statusOf("calm")).toBe("idle"); + }); + + test("is idempotent — a second boot corrects nothing", () => { + seed("a", "waiting_approval"); + expect(store.reconcileStaleSessionStatuses()).toBe(1); + expect(store.reconcileStaleSessionStatuses()).toBe(0); + expect(statusOf("a")).toBe("idle"); + }); +});