end a turn whose agent has gone
restarting officer-agent takes every persistent session with it and nothing downstream notices: the browser's socket is healthy, officer's subscription is a bus filter, and there is simply never another event. the spinner ran forever and a refresh didn't help, because the transcript has no ending to read. keyed off the agent *registering*, not disconnecting — a disconnect fires on every `pm2 restart officer`, when the turn is fine. a registration socket dies with its process, so an agent appearing on it is a new one. covers the sitting tab; the reconnect path covers the rest, with the client now sending its belief that a turn is in flight and officer checking it against the agent over a new claude:is-generating. the check fails toward alive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -522,6 +522,48 @@ ends.
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## 19. The turn that never ends
|
||||||
|
|
||||||
|
**The symptom you described:** the last thing on screen is a tool call, the spinner runs forever, and
|
||||||
|
refreshing puts you back on the same dead conversation. It looked like a cancel you didn't press.
|
||||||
|
|
||||||
|
**What actually happens.** It is the agent sidecar restarting. `officer-agent` holds the persistent
|
||||||
|
`query()` for each session, so when that process goes, so does every turn it was running — and nothing
|
||||||
|
downstream finds out. The browser's socket is to `officer`, which is fine; officer's subscription is an
|
||||||
|
event-bus filter, which is also fine; there is simply never another event. The `pm2` logs make the two
|
||||||
|
cases plain: `[sidecar] disconnected from API server` with no `SIGINT` beside it is officer restarting
|
||||||
|
underneath a healthy agent (turn survives — that's the documented design), while `[agent] SIGINT
|
||||||
|
received` is the case that kills turns.
|
||||||
|
|
||||||
|
So the fix could not key off the sidecar _disconnecting_ — that fires on every `pm2 restart officer`,
|
||||||
|
when nothing is wrong. It keys off a **registration**: a registration socket lives and dies with its
|
||||||
|
process, so an agent appearing on it is an agent that has just started, and anything it was mid-turn on
|
||||||
|
is gone.
|
||||||
|
|
||||||
|
Two paths, because the tab can be in two states:
|
||||||
|
|
||||||
|
- **The tab is sitting there with a live socket.** The new agent registers, and every session officer
|
||||||
|
still believes is generating gets checked and ended. On a freshly-restarted _officer_ this loop is
|
||||||
|
empty — no sessions yet — which is right, because that case belongs to the other path.
|
||||||
|
- **The tab reconnects** (socket blip, or a refresh, or officer itself restarted). The client now sends
|
||||||
|
`generating` in its resume handshake — its belief that a turn is in flight. Officer can't confirm that
|
||||||
|
from its own memory, which died with the process, so it asks the agent over a new `claude:is-generating`
|
||||||
|
command. The agent is the only party that knows.
|
||||||
|
|
||||||
|
Either way you get a line saying the agent restarted and the turn was cut off, and it's written to
|
||||||
|
`chat_session_events` — so a reload afterwards shows the same explanation rather than a conversation that
|
||||||
|
trails off mid-tool-call.
|
||||||
|
|
||||||
|
The liveness check **fails toward alive**: a timeout, or no answer, is read as "still running". Telling
|
||||||
|
you a turn died while it is quietly typing would be a worse lie than a spinner that stays up a bit
|
||||||
|
longer. Only a registered agent answering "no", or no agent at all, counts as dead. OpenCode sessions are
|
||||||
|
left alone — that harness runs a turn per invocation and has no equivalent question.
|
||||||
|
|
||||||
|
**Not verified:** the browser, and the restart itself. The mechanism is reasoned from the code plus the
|
||||||
|
`pm2` logs that pinned the cause; typecheck and the full 365-test suite are clean.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## Things noticed and deliberately left alone
|
## Things noticed and deliberately left alone
|
||||||
|
|
||||||
- **`useChatWebSocket` silently ignores unparseable frames.** That one is intentional and the comment
|
- **`useChatWebSocket` silently ignores unparseable frames.** That one is intentional and the comment
|
||||||
|
|||||||
@@ -87,6 +87,11 @@ export type ClientMessage =
|
|||||||
// client is the only party that still remembers them — officer's copy died with the process.
|
// client is the only party that still remembers them — officer's copy died with the process.
|
||||||
model?: string;
|
model?: string;
|
||||||
cwd?: string;
|
cwd?: string;
|
||||||
|
/**
|
||||||
|
* The client still shows a turn in flight. Officer cannot know this on its own after a restart, and
|
||||||
|
* it is the one claim worth checking against the agent — see `endTurnIfAgentIsGone`.
|
||||||
|
*/
|
||||||
|
generating?: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
// The `Task` tool's id, when this piece of output came from a subagent rather than the agent you are
|
// The `Task` tool's id, when this piece of output came from a subagent rather than the agent you are
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ import { ensureGeneralChatSessionsCwd } from './claude-sessions';
|
|||||||
import * as sidecar from '@@/sidecar-registry';
|
import * as sidecar from '@@/sidecar-registry';
|
||||||
import { join } from 'path';
|
import { join } from 'path';
|
||||||
import { getOwnerHomeDir, getEmailAccountsDir } from '../../../servers/data-path';
|
import { getOwnerHomeDir, getEmailAccountsDir } from '../../../servers/data-path';
|
||||||
import { getUserSettings, getEmailAccounts, getChatEventsSince } from 'officerdb';
|
import { getUserSettings, getEmailAccounts, getChatEventsSince, appendChatEvent } from 'officerdb';
|
||||||
import { mkdirSync } from 'node:fs';
|
import { mkdirSync } from 'node:fs';
|
||||||
import { logger } from './logger';
|
import { logger } from './logger';
|
||||||
|
|
||||||
@@ -579,11 +579,12 @@ function adoptOrphanedSession(ws: ServerWebSocket<WSData>, sessionId: string, mo
|
|||||||
// task:notification. attachWs cancels the pending idle-GC.
|
// task:notification. attachWs cancels the pending idle-GC.
|
||||||
async function handleResumeCursor(
|
async function handleResumeCursor(
|
||||||
ws: ServerWebSocket<WSData>,
|
ws: ServerWebSocket<WSData>,
|
||||||
msg: { sessionId: string; cursor: number; model?: string; cwd?: string },
|
msg: { sessionId: string; cursor: number; model?: string; cwd?: string; generating?: boolean },
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
const { sessionId, cursor } = msg;
|
const { sessionId, cursor } = msg;
|
||||||
|
const model = msg.model || DEFAULT_MODEL;
|
||||||
if (!sessionManager.getSession(sessionId)) {
|
if (!sessionManager.getSession(sessionId)) {
|
||||||
adoptOrphanedSession(ws, sessionId, msg.model || DEFAULT_MODEL, msg.cwd ?? '');
|
adoptOrphanedSession(ws, sessionId, model, msg.cwd ?? '');
|
||||||
}
|
}
|
||||||
sessionManager.attachWs(sessionId, ws);
|
sessionManager.attachWs(sessionId, ws);
|
||||||
wsToSessionMap.set(ws as any, sessionId);
|
wsToSessionMap.set(ws as any, sessionId);
|
||||||
@@ -595,8 +596,58 @@ async function handleResumeCursor(
|
|||||||
} catch (err) {
|
} catch (err) {
|
||||||
logger.error('Failed to replay chat events on resume-cursor', { sessionId, error: String(err) });
|
logger.error('Failed to replay chat events on resume-cursor', { sessionId, error: String(err) });
|
||||||
}
|
}
|
||||||
|
if (msg.generating) await endTurnIfAgentIsGone(ws, sessionId, model);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The client came back still believing a turn is running. Check whether it is, and if it isn't, say so.
|
||||||
|
*
|
||||||
|
* A restart of the agent sidecar takes its persistent sessions with it, and nothing downstream notices:
|
||||||
|
* the turn simply stops emitting. The browser's socket is fine, the conversation looks alive, and the
|
||||||
|
* spinner runs forever — a refresh doesn't help either, because there is no ending in the transcript to
|
||||||
|
* read. This is the one moment we can catch it, so the answer is written durably: a reload after this
|
||||||
|
* shows the same explanation rather than a conversation that trails off mid-tool-call.
|
||||||
|
*
|
||||||
|
* Only the claude harness is asked. OpenCode runs a turn per invocation and has no equivalent question,
|
||||||
|
* so its sessions are left alone rather than guessed at.
|
||||||
|
*/
|
||||||
|
async function endTurnIfAgentIsGone(
|
||||||
|
ws: ServerWebSocket<WSData> | null,
|
||||||
|
sessionId: string,
|
||||||
|
model: string,
|
||||||
|
): Promise<void> {
|
||||||
|
if (!isClaudeModel(model)) return;
|
||||||
|
if (await sidecar.isClaudeGenerating(sessionId)) return;
|
||||||
|
|
||||||
|
const session = sessionManager.getSession(sessionId);
|
||||||
|
if (session) session.isGenerating = false;
|
||||||
|
|
||||||
|
const event: ServerMessage = {
|
||||||
|
type: 'error',
|
||||||
|
message: 'The agent restarted while this turn was running, so it was cut off. The conversation is intact.',
|
||||||
|
};
|
||||||
|
try {
|
||||||
|
const seq = await appendChatEvent(sessionId, event);
|
||||||
|
sendToClient(ws, event, seq);
|
||||||
|
} catch (err) {
|
||||||
|
// Still tell this client — an un-replayable explanation beats a spinner that never stops.
|
||||||
|
logger.error('Failed to persist cut-off notice', { sessionId, error: String(err) });
|
||||||
|
sendToClient(ws, event);
|
||||||
|
}
|
||||||
|
logger.info('Ended a turn whose agent had gone', { sessionId });
|
||||||
|
}
|
||||||
|
|
||||||
|
// The other half of the same problem: the agent restarts while the browser sits there with a healthy
|
||||||
|
// socket, so nothing ever reconnects and nothing ever asks. A fresh agent process means every turn we
|
||||||
|
// still believe is running belongs to a process that no longer exists. On a fresh officer this loop is
|
||||||
|
// empty — it has no sessions yet — which is exactly right, because that case is the reconnect's to catch.
|
||||||
|
sidecar.onClaudeSidecarStarted(() => {
|
||||||
|
for (const session of sessionManager.getAllSessions()) {
|
||||||
|
if (!session.isGenerating) continue;
|
||||||
|
void endTurnIfAgentIsGone(session.ws as ServerWebSocket<WSData> | null, session.sessionId, session.model);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
export const chatWebsocket = {
|
export const chatWebsocket = {
|
||||||
open,
|
open,
|
||||||
message,
|
message,
|
||||||
|
|||||||
@@ -65,9 +65,31 @@ export function registerSidecar(ws: ServerWebSocket<any>, registration: SidecarR
|
|||||||
console.log(
|
console.log(
|
||||||
`[registry] registered sidecar "${registration.name}" (id=${id}, capabilities=[${registration.capabilities.join(', ')}])`,
|
`[registry] registered sidecar "${registration.name}" (id=${id}, capabilities=[${registration.capabilities.join(', ')}])`,
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// A registration socket lives and dies with its process, so an agent showing up here is an agent that
|
||||||
|
// has just started — and whatever it was running before is gone. Announced rather than inferred from
|
||||||
|
// the *disconnect*, which is the wrong signal entirely: every `pm2 restart officer` drops these sockets
|
||||||
|
// while the sidecars, and their turns, carry on perfectly well.
|
||||||
|
if (registration.capabilities.includes('claude')) {
|
||||||
|
for (const handler of claudeRestartHandlers) {
|
||||||
|
try {
|
||||||
|
handler();
|
||||||
|
} catch (err) {
|
||||||
|
console.error('[registry] claude-restart handler failed', err);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
return id;
|
return id;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const claudeRestartHandlers = new Set<() => void>();
|
||||||
|
|
||||||
|
/** Notified when the agent sidecar registers — i.e. when a new agent process has come up. */
|
||||||
|
export function onClaudeSidecarStarted(handler: () => void): () => void {
|
||||||
|
claudeRestartHandlers.add(handler);
|
||||||
|
return () => claudeRestartHandlers.delete(handler);
|
||||||
|
}
|
||||||
|
|
||||||
export function unregisterSidecar(id: string): void {
|
export function unregisterSidecar(id: string): void {
|
||||||
const sc = sidecars.get(id);
|
const sc = sidecars.get(id);
|
||||||
if (!sc) return;
|
if (!sc) return;
|
||||||
@@ -273,6 +295,29 @@ export function interruptClaude(sessionKey: string): void {
|
|||||||
sendFire('claude', { type: 'claude:interrupt', id: nextId(), sessionKey });
|
sendFire('claude', { type: 'claude:interrupt', id: nextId(), sessionKey });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Is the agent still running a turn for this session?
|
||||||
|
*
|
||||||
|
* Asked of the sidecar because it is the only party that can answer: officer's own memory of a turn dies
|
||||||
|
* with `pm2 restart officer` while the turn itself carries on, so "we don't remember it" is not evidence
|
||||||
|
* of anything. Used on reconnect to tell those two apart — a turn that outlived a restart from one whose
|
||||||
|
* process is gone and is never going to produce another token.
|
||||||
|
*
|
||||||
|
* Fails *toward alive*: no answer means we don't know, and wrongly telling someone their turn died while
|
||||||
|
* it is still typing is worse than leaving a spinner up a little longer. Only a registered agent that
|
||||||
|
* says "no", or no agent at all, counts as dead.
|
||||||
|
*/
|
||||||
|
export async function isClaudeGenerating(sessionKey: string): Promise<boolean> {
|
||||||
|
const sc = findSidecarByCapability('claude');
|
||||||
|
if (!sc) return false;
|
||||||
|
try {
|
||||||
|
const res = await sendCommandToSidecar(sc, { type: 'claude:is-generating', id: nextId(), sessionKey });
|
||||||
|
return res.type === 'claude:generating' ? res.generating : true;
|
||||||
|
} catch {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export function clearClaudeSession(sessionKey: string): void {
|
export function clearClaudeSession(sessionKey: string): void {
|
||||||
sendFire('claude', { type: 'claude:clear-session', id: nextId(), sessionKey });
|
sendFire('claude', { type: 'claude:clear-session', id: nextId(), sessionKey });
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -379,3 +379,15 @@ export function clearSession(sessionKey: string): void {
|
|||||||
export function getActiveSessionKeys(): string[] {
|
export function getActiveSessionKeys(): string[] {
|
||||||
return Array.from(sessions.keys());
|
return Array.from(sessions.keys());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Is a turn actually in flight for this session, right now, in this process?
|
||||||
|
*
|
||||||
|
* This is the ground truth a reconnecting browser has no way to work out for itself. Officer's own view
|
||||||
|
* dies with `pm2 restart officer` while the turn keeps running here, so "officer doesn't remember" means
|
||||||
|
* nothing — and if *this* process was the one that restarted, the session is simply absent and the turn
|
||||||
|
* it was running is gone, however alive the client still believes it to be.
|
||||||
|
*/
|
||||||
|
export function isSessionGenerating(sessionKey: string): boolean {
|
||||||
|
return sessions.get(sessionKey)?.isGenerating ?? false;
|
||||||
|
}
|
||||||
|
|||||||
@@ -190,6 +190,10 @@ async function handleCommand(cmd: SidecarCommand, reply: ReplyFn) {
|
|||||||
reply({ type: 'claude:interrupted', id: cmd.id });
|
reply({ type: 'claude:interrupted', id: cmd.id });
|
||||||
break;
|
break;
|
||||||
|
|
||||||
|
case 'claude:is-generating':
|
||||||
|
reply({ type: 'claude:generating', id: cmd.id, generating: claudeManager.isSessionGenerating(cmd.sessionKey) });
|
||||||
|
break;
|
||||||
|
|
||||||
case 'claude:clear-session':
|
case 'claude:clear-session':
|
||||||
claudeManager.clearSession(cmd.sessionKey);
|
claudeManager.clearSession(cmd.sessionKey);
|
||||||
sessionLog.drop(cmd.sessionKey);
|
sessionLog.drop(cmd.sessionKey);
|
||||||
|
|||||||
@@ -17,6 +17,9 @@ export type SidecarCommand =
|
|||||||
| { type: 'claude:kill'; id: string; sessionKey: string }
|
| { type: 'claude:kill'; id: string; sessionKey: string }
|
||||||
| { type: 'claude:interrupt'; id: string; sessionKey: string }
|
| { type: 'claude:interrupt'; id: string; sessionKey: string }
|
||||||
| { type: 'claude:clear-session'; id: string; sessionKey: string }
|
| { type: 'claude:clear-session'; id: string; sessionKey: string }
|
||||||
|
// Is a turn still running for this session? Only the process that owns the session can say, which is
|
||||||
|
// exactly why it is asked over the wire — see `isClaudeGenerating` in sidecar-registry.
|
||||||
|
| { type: 'claude:is-generating'; id: string; sessionKey: string }
|
||||||
// OpenCode — drive a turn via `opencode run … --format json` (tools re-anchored to cwd via --dir)
|
// OpenCode — drive a turn via `opencode run … --format json` (tools re-anchored to cwd via --dir)
|
||||||
| { type: 'opencode:run-streaming'; id: string; params: OpenCodeRunParams }
|
| { type: 'opencode:run-streaming'; id: string; params: OpenCodeRunParams }
|
||||||
| { type: 'opencode:kill'; id: string; sessionKey: string }
|
| { type: 'opencode:kill'; id: string; sessionKey: string }
|
||||||
@@ -43,6 +46,7 @@ export type SidecarEvent =
|
|||||||
| { type: 'claude:killed'; id: string }
|
| { type: 'claude:killed'; id: string }
|
||||||
| { type: 'claude:interrupted'; id: string }
|
| { type: 'claude:interrupted'; id: string }
|
||||||
| { type: 'claude:session-cleared'; id: string }
|
| { type: 'claude:session-cleared'; id: string }
|
||||||
|
| { type: 'claude:generating'; id: string; generating: boolean }
|
||||||
// VNC
|
// VNC
|
||||||
| { type: 'vnc:started'; id: string; port: number; display: number }
|
| { type: 'vnc:started'; id: string; port: number; display: number }
|
||||||
| { type: 'vnc:password'; id: string; password: string }
|
| { type: 'vnc:password'; id: string; password: string }
|
||||||
|
|||||||
@@ -93,6 +93,9 @@ export function useChat(initialSessionId?: string, initialModel?: string | null,
|
|||||||
const onTurnCompleteRef = useRef(onTurnComplete);
|
const onTurnCompleteRef = useRef(onTurnComplete);
|
||||||
onTurnCompleteRef.current = onTurnComplete;
|
onTurnCompleteRef.current = onTurnComplete;
|
||||||
const resumeSummaryRef = useRef<string | undefined>(initialResumeSummary);
|
const resumeSummaryRef = useRef<string | undefined>(initialResumeSummary);
|
||||||
|
// Mirror for the reconnect handshake, which runs from a stable callback and cannot read state.
|
||||||
|
const isGeneratingRef = useRef(false);
|
||||||
|
isGeneratingRef.current = isGenerating;
|
||||||
|
|
||||||
const client = useClient();
|
const client = useClient();
|
||||||
|
|
||||||
@@ -393,6 +396,10 @@ export function useChat(initialSessionId?: string, initialModel?: string | null,
|
|||||||
cursor: cursorRef.current,
|
cursor: cursorRef.current,
|
||||||
...(modelRef.current ? { model: modelRef.current } : {}),
|
...(modelRef.current ? { model: modelRef.current } : {}),
|
||||||
...(cwdRef.current ? { cwd: cwdRef.current } : {}),
|
...(cwdRef.current ? { cwd: cwdRef.current } : {}),
|
||||||
|
// Whether we still think a turn is running. If the agent restarted under us there is nothing left
|
||||||
|
// to produce output, and this is the only moment anyone can notice — the socket is healthy, the
|
||||||
|
// transcript just stops. The server checks the claim with the agent and answers if it's false.
|
||||||
|
generating: isGeneratingRef.current,
|
||||||
});
|
});
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user