diff --git a/src/servers/api/chat/chat.ts b/src/servers/api/chat/chat.ts index e3b8e3a5..78e09a59 100644 --- a/src/servers/api/chat/chat.ts +++ b/src/servers/api/chat/chat.ts @@ -296,7 +296,9 @@ chatRouter.post('/tasks/:id/restart', async (ctx) => { const result = await restartBackgroundTask(who, taskId, (code: number) => { if (!body.sessionId) return; - emitToSession(body.sessionId, { + // Fire-and-forget because the exit arrives long after this request answered; `emitToSession` swallows + // its own failures, so there is nothing here to await for. + void emitToSession(body.sessionId, { type: 'task:notification', taskId, status: code === 0 ? 'completed' : 'failed', @@ -306,8 +308,14 @@ chatRouter.post('/tasks/:id/restart', async (ctx) => { // Announced only on success, and only after the process exists — a `task:started` for something that // failed to spawn would leave a chip running forever with nothing behind it. + // + // Awaited, so the event is in the durable log before the client is told the restart worked. The tray is + // rebuilt from that log on every refresh, and a client that reloaded in the gap would have folded the + // ORIGINAL task's `task:notification` as the last word and shown nothing — which is precisely the bug + // this replaced: a live dev server with no chip and no way to reach it, since every task route needs an + // id the browser had just thrown away. if (result.ok && body.sessionId) { - emitToSession(body.sessionId, { + await emitToSession(body.sessionId, { type: 'task:started', taskId, description: result.command?.split('\n')[0]?.slice(0, 80) ?? 'restarted job', diff --git a/src/servers/api/chat/websocket.ts b/src/servers/api/chat/websocket.ts index 7b70990c..0fd39419 100644 --- a/src/servers/api/chat/websocket.ts +++ b/src/servers/api/chat/websocket.ts @@ -174,7 +174,8 @@ function sendToClient(ws: ServerWebSocket | null, msg: ServerMessage, se } } -// Nothing in officer writes to chat_session_events any more. Both harnesses commit their own turn +// Officer writes to chat_session_events only for events it ORIGINATES — `cut-off` and the restart +// button's task pair (`emitToSession`). Both harnesses commit their own turn // output in the sidecar that produced it (`sidecar/claude/session-log.ts`), which is the whole point: // the durable record does not travel over the socket between the two processes, so officer can restart // mid-turn without losing it. Officer reads the table on `resume` (getChatEventsSince) and relays what @@ -320,21 +321,48 @@ function createMessageHandler(sessionId: string, model: string) { } /** - * Push an event into a live session from OUTSIDE a turn. + * Push an event into a session from OUTSIDE a turn, durably. * * The restart button is the only caller: the platform respawns a process the harness is no longer watching, * so nothing else would emit the `task:started` / `task:notification` pair that the tray, the log view and - * every reconnect are built on. Folded into the session first and then sent, in that order, so a browser - * that reconnects a second later sees the same thing as one that was already listening. + * every reconnect are built on. * - * Returns false when the session is not live — a restart is still legitimate then, it simply has nobody to - * tell. + * ── Why this writes to the log, against the note above ── + * + * It used to only fold and send, and that made the tray a fact about one browser tab. A refresh rebuilds + * the tray by folding the durable log — `task:started` minus `task:notification`, see + * `collectRunningTasks` — so a restart announced only over sockets left the log reading "started, then + * failed" while the process it described was still up and serving. The pill vanished on refresh and the + * job became uncontrollable from the UI: every task route takes an id, and the id existed nowhere else. + * + * `cut-off` is the same exception for the same reason, and the rule behind both is narrower than "officer + * does not write here": officer writes events it ORIGINATES, because no sidecar will. Turn output still + * belongs to the harness that produced it. + * + * ── The append is unconditional ── + * + * It happens whether or not a session is live in memory, and that is the half that was silently missing: + * a platform restart drops the in-memory session while the job keeps running, and the old early return + * meant the announcement went nowhere and was never written down either. Persistence does not need a + * listener. Delivery does, and only delivery is conditional. + * + * Returns whether it reached a live socket — false is ordinary, not an error. */ -export function emitToSession(sessionId: string, msg: TurnMessage): boolean { +export async function emitToSession(sessionId: string, msg: TurnMessage): Promise { + let seq: number | undefined; + try { + seq = await appendChatEvent(sessionId, msg); + } catch (err) { + // Still tell whoever is listening. A pill that cannot survive a refresh beats no pill at all, and the + // process is running either way. + logger.error('Failed to persist an out-of-turn event', { sessionId, type: msg.type, error: String(err) }); + } + const session = sessionManager.getSession(sessionId); if (!session) return false; foldIntoSession(session, msg, session.meta.model ?? ''); - for (const socket of session.sockets) sendToClient(socket as ServerWebSocket, msg); + // The seq is what a client records as its replay cursor, so it must be the one the log assigned. + for (const socket of session.sockets) sendToClient(socket as ServerWebSocket, msg, seq); return true; }