From 1b811dfa2845036940deab032a736a5072cb5398 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Mon, 17 Aug 2026 20:50:24 +0000 Subject: [PATCH] make plan mode reachable in chat opusplan has been in the picker since 57bbcca9 and did nothing: it means 'Opus while planning, Sonnet at rest', and claude-manager hardcoded permissionMode: 'bypassPermissions', so a session never entered plan mode and opusplan rested forever on Sonnet. verified against a real claude first (SDK 0.3.233), and one finding changed the design: approving ExitPlanMode drops the harness into 'default', NOT back into bypassPermissions, and the agent carries on inside the same turn. so the restore is fired the moment approval is given rather than at turn end, and canUseTool allows everything that is not ExitPlanMode - for a few hundred ms it is the only thing between the agent and its tools, and a default-deny there would jail an agent the platform deliberately does not jail. canUseTool is passed always. under bypassPermissions the SDK shadows it entirely (CLAUDE_SDK_CAN_USE_TOOL_SHADOWED) so it costs a log line, and since it is a query() option read once, passing it unconditionally is what makes the mode toggleable on a live conversation instead of only at session start. sticky per session on chat_session_models.plan_mode, beside the model because they are one decision about what a turn costs. boolean rather than the PermissionMode string: officer exposes two of the five, and a text column would accept three the sidecar cannot handle. plan approval is the only place the agent blocks on officer - a canUseTool promise held in the sidecar while the plan round-trips to a browser. every path that ends a session or a turn settles it as refused, plus a 20m timeout, because a leaked entry is a hung turn rather than a stale value. not auto-entered for opusplan: the CLI does not either. the toggle says so instead - 'opusplan rests on Sonnet, turn on Plan for Opus'. Co-Authored-By: Claude Opus 5 --- .../Screens/Dashboard/ChatHistory/index.tsx | 1 + .../src/chat-session-models/index.ts | 1 + .../src/chat-session-models/queries.ts | 48 ++++- .../src/chat-session-models/schema.ts | 22 +- src/servers/api/chat/chat.ts | 19 +- src/servers/api/chat/types.ts | 65 +++++- src/servers/api/chat/websocket.ts | 115 ++++++++++- src/servers/sidecar-registry.ts | 17 ++ src/servers/sidecar/claude/claude-manager.ts | 195 +++++++++++++++++- .../sidecar/claude/turn-stream.test.ts | 43 ++++ src/servers/sidecar/claude/turn-stream.ts | 37 ++++ src/servers/sidecar/claude/user-instance.ts | 12 ++ src/servers/sidecar/protocol.ts | 30 +++ .../Chat/EmbeddableChat/useEmbeddableChat.ts | 6 + .../src/apps/Chat/components/InputArea.tsx | 4 + .../apps/Chat/components/MessageBubble.tsx | 137 +++++++++++- .../src/apps/Chat/components/MessageList.tsx | 2 + .../apps/Chat/components/ModelSelector.tsx | 44 +++- .../src/apps/Chat/components/turn-rows.ts | 12 +- .../officerdev/src/apps/Chat/types.ts | 32 ++- .../src/apps/ChatHistory/ChatDetailPanel.tsx | 8 +- .../src/apps/ChatHistory/SessionRow.tsx | 19 +- .../officerdev/src/hooks/useChat.ts | 96 +++++++++ src/workspaces/state/src/useClaudeSessions.ts | 9 + 24 files changed, 946 insertions(+), 28 deletions(-) diff --git a/src/apps/officer-web/Screens/Dashboard/ChatHistory/index.tsx b/src/apps/officer-web/Screens/Dashboard/ChatHistory/index.tsx index 2d6f47d0..bcee9383 100644 --- a/src/apps/officer-web/Screens/Dashboard/ChatHistory/index.tsx +++ b/src/apps/officer-web/Screens/Dashboard/ChatHistory/index.tsx @@ -108,6 +108,7 @@ export const SessionListPage = ({ isNew }: SessionListPageProps) => { // stored row, falling back to a family guess. Null means "unknown", and the picker must then // leave the selection alone rather than guess — see ModelSelector. model: detail.officerModel ?? undefined, + planMode: detail.planMode, resumeSessionId: sessionId, initialMessages: detail.messages as unknown as NonNullable['initialMessages'], total: detail.total, diff --git a/src/databases/officer_db/src/chat-session-models/index.ts b/src/databases/officer_db/src/chat-session-models/index.ts index 94f3cc8c..6f89ce57 100644 --- a/src/databases/officer_db/src/chat-session-models/index.ts +++ b/src/databases/officer_db/src/chat-session-models/index.ts @@ -1,5 +1,6 @@ export { setChatSessionModel, + setChatSessionPlanMode, getChatSessionModel, getChatSessionModels, seedMissingChatSessionModels, diff --git a/src/databases/officer_db/src/chat-session-models/queries.ts b/src/databases/officer_db/src/chat-session-models/queries.ts index 6f40cf46..df20ef4a 100644 --- a/src/databases/officer_db/src/chat-session-models/queries.ts +++ b/src/databases/officer_db/src/chat-session-models/queries.ts @@ -10,24 +10,46 @@ import { chatSessionModels } from './schema'; * the agent that answered. That is the same distinction the per-message attribution makes in `websocket.ts`, * and it is the reason this takes a plain string rather than reading the client's message itself. */ -export async function setChatSessionModel(sessionId: string, model: string): Promise { +export async function setChatSessionModel(sessionId: string, model: string, planMode?: boolean): Promise { await db .insert(chatSessionModels) - .values({ sessionId, model }) + .values({ sessionId, model, ...(planMode === undefined ? {} : { planMode }) }) .onConflictDoUpdate({ target: chatSessionModels.sessionId, - set: { model, updatedAt: new Date() }, + // `planMode` is written only when the caller has an opinion. A turn that carries no plan-mode flag + // must not silently switch a planning session back off — the same rule the model half follows, where + // a bare harness id leaves the stored tier alone rather than overwriting it with a non-answer. + set: { model, ...(planMode === undefined ? {} : { planMode }), updatedAt: new Date() }, }); } -/** One session's stored model, or null when nothing has recorded one yet. */ -export async function getChatSessionModel(sessionId: string): Promise { +/** + * Turn plan mode on or off for a session that already has a row — UPDATE, deliberately not an upsert. + * + * The toggle can be flipped on a conversation that has never taken a turn, and such a session has no row + * yet and no model to invent for one (`model` is NOT NULL, and guessing it here would write a tier the user + * never picked). Rather than fabricate a row, this no-ops: the client holds the toggle in its own state and + * sends it with the next turn, and `setChatSessionModel` writes both fields together at the point where the + * model is finally known. + * + * So the two writers divide cleanly — this one is for a session with history, that one for a session + * acquiring it. + */ +export async function setChatSessionPlanMode(sessionId: string, planMode: boolean): Promise { + await db + .update(chatSessionModels) + .set({ planMode, updatedAt: new Date() }) + .where(inArray(chatSessionModels.sessionId, [sessionId])); +} + +/** One session's stored model and plan mode, or null when nothing has recorded one yet. */ +export async function getChatSessionModel(sessionId: string): Promise<{ model: string; planMode: boolean } | null> { const [row] = await db - .select({ model: chatSessionModels.model }) + .select({ model: chatSessionModels.model, planMode: chatSessionModels.planMode }) .from(chatSessionModels) .where(inArray(chatSessionModels.sessionId, [sessionId])) .limit(1); - return row?.model ?? null; + return row ?? null; } /** @@ -37,13 +59,19 @@ export async function getChatSessionModel(sessionId: string): Promise> { +export async function getChatSessionModels( + sessionIds: string[], +): Promise> { if (!sessionIds.length) return new Map(); const rows = await db - .select({ sessionId: chatSessionModels.sessionId, model: chatSessionModels.model }) + .select({ + sessionId: chatSessionModels.sessionId, + model: chatSessionModels.model, + planMode: chatSessionModels.planMode, + }) .from(chatSessionModels) .where(inArray(chatSessionModels.sessionId, sessionIds)); - return new Map(rows.map((r) => [r.sessionId, r.model])); + return new Map(rows.map((r) => [r.sessionId, { model: r.model, planMode: r.planMode }])); } /** diff --git a/src/databases/officer_db/src/chat-session-models/schema.ts b/src/databases/officer_db/src/chat-session-models/schema.ts index e9096350..6600caee 100644 --- a/src/databases/officer_db/src/chat-session-models/schema.ts +++ b/src/databases/officer_db/src/chat-session-models/schema.ts @@ -1,4 +1,4 @@ -import { pgTable, text, timestamp } from 'drizzle-orm/pg-core'; +import { boolean, pgTable, text, timestamp } from 'drizzle-orm/pg-core'; /** * Which model each chat session is using — the one fact about a session the transcript cannot answer. @@ -49,5 +49,25 @@ export const chatSessionModels = pgTable('chat_session_models', { * new model must never be a schema migration. */ model: text('model').notNull(), + /** + * Whether this conversation is in PLAN mode — the second thing about a session the transcript cannot say. + * + * It belongs beside the model rather than in a table of its own because it is the same kind of fact and + * answers the same question. `claude-code/opusplan` means "Opus while planning, Sonnet at rest", so the + * tier a session is actually paying for is not `model` alone — it is `model` AND this. Split across two + * tables, every read that wanted one would want the other. + * + * ── Why a boolean and not the PermissionMode string ── + * + * The SDK has five (`default`, `acceptEdits`, `bypassPermissions`, `plan`, `dontAsk`) and Officer exposes + * exactly two of them: plan, and the `bypassPermissions` the platform has always run on. A text column + * would accept the other three from any writer, and the sidecar has no handling for them — an + * `acceptEdits` here would restore as a mode nothing in the UI can show or turn off. A boolean cannot hold + * a value the system does not implement. + * + * `false` is the default in every sense: it is what every existing row means, and it is the unsandboxed + * behaviour CLAUDE.md requires the platform to keep as its baseline. + */ + planMode: boolean('plan_mode').notNull().default(false), updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(), }); diff --git a/src/servers/api/chat/chat.ts b/src/servers/api/chat/chat.ts index f84eab6c..a7f8b2f6 100644 --- a/src/servers/api/chat/chat.ts +++ b/src/servers/api/chat/chat.ts @@ -136,10 +136,18 @@ chatRouter.get('/sessions', async (ctx) => { than guessed" rule the picker follows. Never fatal: a database that will not answer this must not take the conversation list down with it. */ - const models = await getChatSessionModels(sessions.map((s) => s.id)).catch(() => new Map()); + const models = await getChatSessionModels(sessions.map((s) => s.id)).catch( + () => new Map(), + ); return ctx.json({ - sessions: sessions.map((s) => ({ ...s, model: models.get(s.id) ?? null })), + sessions: sessions.map((s) => { + const stored = models.get(s.id); + // Spelled out field by field rather than spread: this row goes onto the wire untyped (`ctx.json`), + // so widening the stored shape from a string to a record would have silently shipped an OBJECT as + // `model` and broken every consumer that calls `.split('/')` on it — with nothing to catch it. + return { ...s, model: stored?.model ?? null, planMode: stored?.planMode ?? false }; + }), }); }); @@ -191,14 +199,17 @@ chatRouter.get('/sessions/:id', async (ctx) => { not taken a turn since `chat_session_models` landed. Null when neither can answer — the client shows no model rather than guessing, which is the whole defect being fixed. */ - const officerModel = - (await getChatSessionModel(id).catch(() => null)) ?? officerModelForApiModel(detail.model) ?? null; + const stored = await getChatSessionModel(id).catch(() => null); + const officerModel = stored?.model ?? officerModelForApiModel(detail.model) ?? null; return ctx.json({ ...detail, // `detail.model` stays on the payload untouched — it is the honest record of what the API served, and // the detail view may want it. `officerModel` is the separate question of what to select in the picker. officerModel, + // Whether this conversation is planning, so the composer's toggle comes back in the state the user left + // it. Only a stored row can answer — plan mode leaves no trace in the transcript at all. + planMode: stored?.planMode ?? false, messages, total, offset: windowed ? start : 0, diff --git a/src/servers/api/chat/types.ts b/src/servers/api/chat/types.ts index 15787cc0..0845f4ba 100644 --- a/src/servers/api/chat/types.ts +++ b/src/servers/api/chat/types.ts @@ -70,6 +70,12 @@ export type ClientMessage = context?: string; contextId?: string; resumeSummary?: string; + /** + * Run this turn in plan mode. Carried on the turn as well as settable on its own (`plan:mode` below), + * because a brand-new conversation has no live session to toggle and no stored row to update — its + * first turn is the first moment the preference can be acted on OR written down. + */ + planMode?: boolean; } | { type: 'resume'; @@ -111,6 +117,35 @@ export type ClientMessage = * it is the one claim worth checking against the agent — see `endTurnIfAgentIsGone`. */ generating?: boolean; + } + | { + // Flip plan mode between turns — the moment the toggle is actually used, since you decide to plan + // while reading the last answer rather than mid-generation. + type: 'plan:mode'; + enabled: boolean; + /** + * The transcript uuid, so the preference can be made sticky right away. + * + * Sent by the client because the client is the only party that reliably has it between turns: it is in + * the address bar, while officer's `UserSession` never holds it (the id is the harness's, and officer + * learns it per-turn through `session:claude`). Absent on a conversation that has not taken a turn + * yet — there is no row to update and nothing to key one on, so the toggle simply rides along on the + * first turn instead. + */ + claudeSessionId?: string; + } + | { + /** + * Approve or reject a plan the agent is blocked on. + * + * `feedback` on a rejection is sent to the agent as the denial message, which the harness turns into + * "Please revise your plan based on the feedback and call ExitPlanMode again" — so rejecting with a + * reason is a revision request, not an abort. + */ + type: 'plan:decide'; + requestId: string; + approve: boolean; + feedback?: string; }; // The `Task` tool's id, when this piece of output came from a subagent rather than the agent you are @@ -246,7 +281,24 @@ export type ServerMessage = `ok: false` is a switch the harness refused. It is not an error message — the turn still ran and still answered, as `from` — so it says what actually happened rather than what was asked for. */ - | { type: 'model:switched'; from: string | null; to: string; ok: boolean; error?: string }; + | { type: 'model:switched'; from: string | null; to: string; ok: boolean; error?: string } + /* + Plan mode, in three messages. + + `plan:mode` is the toggle landing — durable for the same reason `model:switched` is, because "the agent + stopped being able to edit things here" explains a stretch of transcript that otherwise reads as an agent + that inexplicably refused to do the work. `ok: false` is a mode change the harness rejected. + + `plan:proposed` is the agent asking to leave plan mode, and it is the one message in this file that the + AGENT IS BLOCKED ON: a `canUseTool` promise inside the sidecar stays pending until a decision comes back + with this `requestId`. Everything else here is a notification. + + `plan:decided` records the answer. Both halves are durable so a refresh mid-plan replays the card and its + outcome instead of showing a conversation that stops dead at a plan nobody appears to have answered. + */ + | { type: 'plan:mode'; enabled: boolean; ok: boolean; error?: string } + | { type: 'plan:proposed'; requestId: string; plan: string } + | { type: 'plan:decided'; requestId: string; approved: boolean; feedback?: string }; // The turn-output subset of ServerMessage — everything the agent sidecar produces on its own. The // remaining members (session:init, sync:messages, disconnected, connection-level errors) are officer's: @@ -269,7 +321,10 @@ export type TurnMessageType = | 'task:notification' | 'compact:start' | 'compact:done' - | 'model:switched'; + | 'model:switched' + | 'plan:mode' + | 'plan:proposed' + | 'plan:decided'; export type TurnMessage = Extract & { prevSeq?: number }; @@ -305,7 +360,11 @@ export type ChatEvent = | { type: 'compact:start'; trigger: 'manual' | 'auto' } | { type: 'compact:done'; trigger: 'manual' | 'auto'; preTokens: number; durationMs?: number } // Emitted by `applyModelChange` when a live session moves onto a different tier — see claude-manager. - | { type: 'model:switched'; from: string | null; to: string; ok: boolean; error?: string }; + | { type: 'model:switched'; from: string | null; to: string; ok: boolean; error?: string } + // Plan mode — see the matching ServerMessage members for what each one means. + | { type: 'plan:mode'; enabled: boolean; ok: boolean; error?: string } + | { type: 'plan:proposed'; requestId: string; plan: string } + | { type: 'plan:decided'; requestId: string; approved: boolean; feedback?: string }; export type UserSession = { sessionId: string; diff --git a/src/servers/api/chat/websocket.ts b/src/servers/api/chat/websocket.ts index 3310591c..01aac869 100644 --- a/src/servers/api/chat/websocket.ts +++ b/src/servers/api/chat/websocket.ts @@ -24,6 +24,7 @@ import { appendChatEvent, getUserById, setChatSessionModel, + setChatSessionPlanMode, } from 'officerdb'; import { resolveHomeDir } from '@@/user-home'; import { claudeLoginState } from '@@/os-user-claude'; @@ -218,6 +219,10 @@ export function message(ws: ServerWebSocket, raw: string | Buffer): void await handleResumeCursor(ws, clientMsg); } else if (clientMsg.type === 'attach') { await handleAttach(ws, clientMsg); + } else if (clientMsg.type === 'plan:mode') { + await handlePlanMode(ws, clientMsg); + } else if (clientMsg.type === 'plan:decide') { + handlePlanDecision(ws, clientMsg); } } catch (err) { logger.error('Error handling WebSocket message', { email: ws.data.email, error: String(err) }); @@ -318,7 +323,7 @@ function foldIntoSession(session: UserSession, msg: TurnMessage, model: string): } // Shared by both harnesses now: whichever sidecar ran the turn has already translated and committed it. -function createMessageHandler(sessionId: string, model: string, resumeSessionId?: string) { +function createMessageHandler(sessionId: string, model: string, resumeSessionId?: string, planMode?: boolean) { /* The model messages are ATTRIBUTED to, which is not always the model the turn asked for. @@ -353,6 +358,20 @@ function createMessageHandler(sessionId: string, model: string, resumeSessionId? */ let transcriptId: string | null = resumeSessionId ?? null; + /* + Plan mode, tracked exactly like `effective` above and for the same reason. + + What the turn ASKED for is `planMode`; what the harness is actually on is whatever its `plan:mode` + messages last said, including the one `planGate` emits when a plan is approved and the session drops back + out of planning on its own. Storing the request would leave a session marked as planning long after it + stopped — and since this is the value the composer restores from, that is a toggle that comes back on by + itself. + + Undefined until something says otherwise, so a client that never mentions plan mode writes nothing rather + than writing `false` over a session that is deliberately planning. + */ + let effectivePlanMode: boolean | undefined = planMode; + /** * Write the model down, once both halves are known. * @@ -378,7 +397,7 @@ function createMessageHandler(sessionId: string, model: string, resumeSessionId? answer alone rather than to overwrite it with a non-answer. */ if (!effective.includes('/')) return; - void setChatSessionModel(transcriptId, effective).catch((err: unknown) => { + void setChatSessionModel(transcriptId, effective, effectivePlanMode).catch((err: unknown) => { logger.warn('Could not record the session model', { sessionId, transcriptId, @@ -402,6 +421,13 @@ function createMessageHandler(sessionId: string, model: string, resumeSessionId? record(); } + // Follow the harness, never the request — a refused switch (`ok: false`) leaves the session where it was, + // and recording the attempt would restore a mode the agent is not in. + if (msg.type === 'plan:mode' && msg.ok) { + effectivePlanMode = msg.enabled; + record(); + } + // The transcript id arrives at the START of a turn, and again on the closing `result` for a session // whose id was assigned mid-turn. Recording on both means a brand-new conversation gets its model // written on its very first turn rather than on its second. @@ -583,6 +609,7 @@ async function handleClaudeCodeChat( cwdRoot?: string; images?: PromptImage[]; resumeSessionId?: string; + planMode?: boolean; }, effectivePrompt: string, ): Promise { @@ -659,7 +686,7 @@ async function handleClaudeCodeChat( // The resume id is passed so a tier switch on a RESUMED conversation is recorded when it happens rather // than when the turn ends — see the seed in `createMessageHandler`. - const onMessage = createMessageHandler(sessionId, model, msg.resumeSessionId); + const onMessage = createMessageHandler(sessionId, model, msg.resumeSessionId, msg.planMode); /** * One description of the turn, used by both paths below. @@ -693,6 +720,9 @@ async function handleClaudeCodeChat( // Never from the client: whose account a turn runs as is resolved here and nowhere else. member: identity.kind === 'member' ? identity.run : undefined, resumeSessionId: msg.resumeSessionId, + // Undefined means "no opinion" and leaves a live session's mode alone — see ClaudeSpawnStreamingParams. + // Only the composer's toggle sets it, so an older client sends nothing and nothing changes. + planMode: msg.planMode, }; try { @@ -859,6 +889,85 @@ async function handleResume( } } +/** + * Flip plan mode between turns. + * + * Two independent effects, and neither implies the other: + * + * - the LIVE session changes mode, if there is one. There often is not: the sidecar collects a session + * after thirty minutes idle, and reading an answer for half an hour before deciding to plan is an + * entirely ordinary thing to do. Nothing is wrong in that case — the next turn carries `planMode` and + * `createSession` opens in the right mode. + * - the PREFERENCE is stored, so a refresh restores the toggle. This is the half that makes the feature + * survive the page, and it is why the sticky column exists. + * + * Claude-only. OpenCode has no plan mode and no permission-mode control request, so a toggle there would be + * a switch that does nothing; the client hides it, and this refuses it rather than trusting that. + */ +async function handlePlanMode( + ws: ServerWebSocket, + msg: { enabled: boolean; claudeSessionId?: string }, +): Promise { + const sessionId = wsToSessionMap.get(ws); + const session = sessionId ? sessionManager.getSession(sessionId) : null; + + if (session && !isClaudeModel(session.model)) { + sendToClient(ws, { + type: 'plan:mode', + enabled: msg.enabled, + ok: false, + error: 'Plan mode is a Claude Code feature. OpenCode has no equivalent.', + }); + return; + } + + if (sessionId) sidecar.setClaudePlanMode(sessionId, ws.data.userId, msg.enabled); + + if (msg.claudeSessionId) { + try { + await setChatSessionPlanMode(msg.claudeSessionId, msg.enabled); + } catch (err) { + // Bookkeeping, like the model record: worth a log, never worth failing the toggle the user just made. + logger.warn('Could not store the session plan mode', { + claudeSessionId: msg.claudeSessionId, + error: err instanceof Error ? err.message : String(err), + }); + } + } + + /* + Echoed immediately rather than waiting for the sidecar. + + A live session will emit its own `plan:mode` through the durable log a moment later and the two agree — + but a session that is NOT live emits nothing at all, and without this the toggle would sit there having + visibly done nothing on exactly the conversations where using it is most natural. + */ + sendToClient(ws, { type: 'plan:mode', enabled: msg.enabled, ok: true }); +} + +/** + * Relay the user's plan decision to the agent that is blocked on it. + * + * Deliberately silent about the outcome: the sidecar answers through the session log with `plan:decided`, + * which reaches every socket watching this conversation rather than only the one that clicked. Two panes on + * the same chat is an ordinary setup here, and a plan resolved in one of them must not stay pending in the + * other. + */ +function handlePlanDecision( + ws: ServerWebSocket, + msg: { requestId: string; approve: boolean; feedback?: string }, +): void { + const sessionId = wsToSessionMap.get(ws); + if (!sessionId) return; + sidecar.decideClaudePlan({ + sessionKey: sessionId, + userId: ws.data.userId, + requestId: msg.requestId, + approve: msg.approve, + feedback: msg.feedback, + }); +} + async function handleStop(ws: ServerWebSocket): Promise { const sessionId = wsToSessionMap.get(ws); diff --git a/src/servers/sidecar-registry.ts b/src/servers/sidecar-registry.ts index 49304d2a..12fa1da7 100644 --- a/src/servers/sidecar-registry.ts +++ b/src/servers/sidecar-registry.ts @@ -297,6 +297,23 @@ export function interruptClaude(sessionKey: string, userId: number): void { sendFire('claude', { type: 'claude:interrupt', id: nextId(), sessionKey, userId }); } +// Plan mode, both directions. Fire-and-forget on purpose: the agent's answer to either one comes back +// through the session log as a `plan:mode` / `plan:decided` message, which is durable, ordered with the +// rest of the turn, and delivered to every socket watching — none of which a command reply would be. +export function setClaudePlanMode(sessionKey: string, userId: number, enabled: boolean): void { + sendFire('claude', { type: 'claude:set-plan-mode', id: nextId(), sessionKey, userId, enabled }); +} + +export function decideClaudePlan(params: { + sessionKey: string; + userId: number; + requestId: string; + approve: boolean; + feedback?: string; +}): void { + sendFire('claude', { type: 'claude:plan-decision', id: nextId(), ...params }); +} + /** * Is the agent still running a turn for this session? * diff --git a/src/servers/sidecar/claude/claude-manager.ts b/src/servers/sidecar/claude/claude-manager.ts index 0e238fe6..863889f0 100644 --- a/src/servers/sidecar/claude/claude-manager.ts +++ b/src/servers/sidecar/claude/claude-manager.ts @@ -230,6 +230,28 @@ type PersistentSession = { * and answered as another. */ model?: string; + /** + * Whether this session is currently in plan mode. + * + * Same shape and same reason as `model` above: `permissionMode` is a `query()` option read once, so this + * is the only record of what the live session is actually on, and a change has to go through a control + * request rather than the next turn's options. + * + * It is also what `canUseTool` reads to decide whether an `ExitPlanMode` is a real decision or a formality + * — see `planGate`. + */ + planMode: boolean; + /** + * Plans waiting on a human, keyed by request id. + * + * Each entry is a `canUseTool` promise the AGENT IS BLOCKED ON, so this map is the one piece of session + * state whose leak is a hang rather than a stale value: an entry that is never resolved is a turn that + * never ends. Everything that can end a session resolves these first — see `settleAllPlans`. + */ + pendingPlans: Map< + string, + { resolve: (decision: { approve: boolean; feedback?: string }) => void; timer: ReturnType } + >; /** The tail of the child's stderr, so its exit can carry a reason instead of just a number. */ stderrTail: string; /** Whether the child ever produced a single message. False plus a non-zero exit means it never started. */ @@ -467,6 +489,8 @@ function createSession(params: ClaudeSpawnStreamingParams, onEvent: (event: Chat emit: onEvent, isGenerating: false, pendingTasks: new Set(), + planMode: params.planMode === true, + pendingPlans: new Map(), interrupted: false, stderrTail: '', sawOutput: false, @@ -502,8 +526,30 @@ function createSession(params: ClaudeSpawnStreamingParams, onEvent: (event: Chat // HOST_HOME is this process's home — the owner's. Defaulting a member's turn to it would start them in // a directory they cannot read, and the failure would look like a broken agent rather than a wrong cwd. cwd: params.cwd ?? params.member?.home ?? HOST_HOME, - permissionMode: 'bypassPermissions', + // `bypassPermissions` unless the conversation opened in plan mode. CLAUDE.md is explicit that the + // unsandboxed default is deliberate — this is the owner's own machine — so plan mode is the only + // thing that may ever move it, and it moves back the moment a plan is approved. + permissionMode: session.planMode ? 'plan' : 'bypassPermissions', allowDangerouslySkipPermissions: true, + /** + * Passed ALWAYS, and shadowed almost always. + * + * Under `bypassPermissions` the SDK never consults this and says so once at startup + * (`CLAUDE_SDK_CAN_USE_TOOL_SHADOWED`) — verified 2026-08-17: a bypassing session made a file edit with + * zero calls here. But `canUseTool` is a `query()` option read once at session creation, so a session + * that did not pass it can never gate anything later. Passing it unconditionally is what makes plan + * mode toggleable on a LIVE conversation instead of only choosable when one starts, and it costs a + * log line. + * + * It allows everything that is not `ExitPlanMode`, and that is not laziness. Approving a plan drops + * the harness into `default` — not back into `bypassPermissions` — so for the rest of that turn this + * callback is the only thing between the agent and its tools. A default-deny here would jail an agent + * the platform deliberately does not jail. + */ + canUseTool: async (toolName, toolInput) => { + if (toolName !== 'ExitPlanMode' || !session.planMode) return { behavior: 'allow', updatedInput: toolInput }; + return planGate(session, toolInput); + }, includePartialMessages: true, // The only warning that compaction is about to happen. Everything else the harness does narrates // itself through the message stream; compaction goes silent for as long as it takes — 2.5 minutes @@ -649,6 +695,9 @@ function createSession(params: ClaudeSpawnStreamingParams, onEvent: (event: Chat } finally { if (session.idleTimer) clearTimeout(session.idleTimer); if (session.stallTimer) clearTimeout(session.stallTimer); + // Before the session leaves the map, so nothing is left holding a promise whose resolver has just + // become unreachable. + settleAllPlans(session, 'The session ended before the plan was answered.'); sessions.delete(sessionKey); } })(); @@ -674,6 +723,7 @@ export async function spawnClaudeStreaming( if (session) { session.emit = onEvent; // adopt the latest emitter (equivalent across turns; keeps events flowing) await applyModelChange(session, params.model); + await applyPlanModeChange(session, params.planMode); } else { session = createSession(params, onEvent); } @@ -715,6 +765,145 @@ async function applyModelChange(session: PersistentSession, model: string | unde } } +/* + How long a plan may sit unanswered before it is refused on the user's behalf. + + This is a deadlock timer, not a UX one. `canUseTool` gates the turn: while a plan is pending the agent is + blocked, `isGenerating` stays true, and no further events are emitted — which also means the stall + watchdog above sees a silent turn and would eventually tear the whole session down. Refusing after twenty + minutes gives a plan a realistic reading time while guaranteeing the turn ends by a route that keeps the + conversation, since a denial is something the agent can answer. +*/ +const PLAN_DECISION_TIMEOUT_MS = 20 * 60 * 1000; + +/** + * Hold `ExitPlanMode` open while a human decides. + * + * The one place in this file where the agent waits on Officer rather than the other way round. The promise + * returned here is the harness's permission answer, so every path out of it must resolve — a throw or a + * forgotten branch is a wedged turn, and the symptom would be a chat that shows a plan and then nothing. + */ +async function planGate( + session: PersistentSession, + toolInput: Record, +): Promise<{ behavior: 'allow'; updatedInput: Record } | { behavior: 'deny'; message: string }> { + const plan = typeof toolInput.plan === 'string' ? toolInput.plan : ''; + const requestId = `plan_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`; + + const decision = await new Promise<{ approve: boolean; feedback?: string }>((resolve) => { + const timer = setTimeout(() => { + session.pendingPlans.delete(requestId); + console.warn(`[claude:plan:${session.sessionKey}] no decision in 20m — refusing on the user's behalf`); + resolve({ approve: false, feedback: 'No decision was made in time. The plan was not approved.' }); + }, PLAN_DECISION_TIMEOUT_MS); + + session.pendingPlans.set(requestId, { resolve, timer }); + session.emit({ type: 'plan:proposed', requestId, plan }); + }); + + session.emit({ type: 'plan:decided', requestId, approved: decision.approve, feedback: decision.feedback }); + + if (!decision.approve) { + // Denial is the REVISION path, not an abort: the harness feeds this string back and asks the agent to + // call ExitPlanMode again. So the message is the feedback, and a bare refusal still needs to say + // something the agent can act on. + return { + behavior: 'deny', + message: decision.feedback?.trim() || 'The plan was rejected. Revise it and ask again.', + }; + } + + /* + Approval leaves plan mode — but into `default`, NOT back into `bypassPermissions` (verified 2026-08-17). + The agent carries straight on inside this same turn, so the restore is fired here rather than at turn end. + + Deliberately NOT awaited, and the ordering is the reason: the harness is waiting on the promise this + function returns, so awaiting a control request before answering it invites a deadlock. Any tool call + that lands in the gap is allowed by the callback above, which makes the window a matter of overhead + rather than correctness. + */ + session.planMode = false; + void session.query.setPermissionMode('bypassPermissions').catch((err: unknown) => { + console.error(`[claude:plan:${session.sessionKey}] could not restore bypassPermissions: ${String(err)}`); + }); + session.emit({ type: 'plan:mode', enabled: false, ok: true }); + + return { behavior: 'allow', updatedInput: toolInput }; +} + +/** + * Resolve every outstanding plan as refused. + * + * Called from every path that ends a session or a turn. A pending plan is a promise the agent is blocked + * on, so leaving one behind when the session goes away is the difference between a turn that ends and a + * turn that hangs until the stall watchdog notices. + */ +function settleAllPlans(session: PersistentSession, reason: string): void { + for (const [requestId, pending] of session.pendingPlans) { + clearTimeout(pending.timer); + session.pendingPlans.delete(requestId); + pending.resolve({ approve: false, feedback: reason }); + } +} + +/** + * The user's answer to a pending plan. Returns false when there is nothing waiting under that id. + * + * A miss is ordinary rather than exceptional: a client that reconnects and replays its last action can + * answer a plan that has already timed out, and the honest response is to ignore it — the alternative is + * resolving a promise that no longer exists, or worse, a newer plan that happens to be pending. + */ +export function decideClaudePlan( + sessionKey: string, + userId: number, + requestId: string, + approve: boolean, + feedback?: string, +): boolean { + const session = ownedSession(sessionKey, userId); + const pending = session?.pendingPlans.get(requestId); + if (!session || !pending) return false; + clearTimeout(pending.timer); + session.pendingPlans.delete(requestId); + pending.resolve({ approve, feedback }); + return true; +} + +/** + * Turn plan mode on or off on a live session — the same control-request shape as `applyModelChange`. + * + * Reachable two ways, because the toggle is used at two different moments: on the turn itself (a new + * conversation whose first turn should plan) and on its own between turns (the common case — you decide to + * plan while reading the last answer, when there is no turn to carry the flag). + * + * A refusal is reported and swallowed for the same reason a model refusal is: the conversation is worth more + * than the mode, so the session stays on what it was and says so rather than losing the turn. + */ +async function applyPlanModeChange(session: PersistentSession, enabled: boolean | undefined): Promise { + if (enabled === undefined || enabled === session.planMode) return; + + try { + await session.query.setPermissionMode(enabled ? 'plan' : 'bypassPermissions'); + session.planMode = enabled; + console.log(`[claude:plan:${session.sessionKey}] plan mode ${enabled ? 'on' : 'off'}`); + session.emit({ type: 'plan:mode', enabled, ok: true }); + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + console.error(`[claude:plan:${session.sessionKey}] refused plan mode ${enabled ? 'on' : 'off'}: ${reason}`); + // `session.planMode` is deliberately untouched: believing a switch that failed would put the gate above + // out of step with the harness, and the visible symptom of that is an ExitPlanMode nobody is asked about. + session.emit({ type: 'plan:mode', enabled, ok: false, error: reason }); + } +} + +/** Flip plan mode on a live session from outside a turn. Returns false when no such session is live. */ +export async function setClaudePlanMode(sessionKey: string, userId: number, enabled: boolean): Promise { + const session = ownedSession(sessionKey, userId); + if (!session) return false; + await applyPlanModeChange(session, enabled); + return true; +} + /** Interrupt the current turn but KEEP the session alive (the "stop" button). */ export async function interruptClaudeSession(sessionKey: string, userId: number): Promise { const session = ownedSession(sessionKey, userId); @@ -722,6 +911,9 @@ export async function interruptClaudeSession(sessionKey: string, userId: number) // Set before the await: the failed `result` can arrive while interrupt() is still resolving, and the // consumer loop reads this flag to tell a stop from a fault. session.interrupted = true; + // Stop cannot reach a turn that is blocked on a plan — the agent is not running, it is waiting on us — + // so the promise has to be released for the interrupt to have anything to interrupt. + settleAllPlans(session, 'The turn was stopped before the plan was answered.'); try { await session.query.interrupt(); session.isGenerating = false; @@ -740,6 +932,7 @@ export function killClaudeSession(sessionKey: string, userId: number): boolean { if (!session) return false; if (session.idleTimer) clearTimeout(session.idleTimer); if (session.stallTimer) clearTimeout(session.stallTimer); + settleAllPlans(session, 'The session was closed before the plan was answered.'); try { session.abort.abort(); } catch { diff --git a/src/servers/sidecar/claude/turn-stream.test.ts b/src/servers/sidecar/claude/turn-stream.test.ts index 0efa2188..0e11e595 100644 --- a/src/servers/sidecar/claude/turn-stream.test.ts +++ b/src/servers/sidecar/claude/turn-stream.test.ts @@ -186,6 +186,49 @@ describe('createTurnStream', () => { ]); }); + /* + Plan mode's three events, and the one property that is not obvious. + + A plan is the only DURABLE row the transcript needs in order to stay honest after a refresh: the card is + interactive, so a plan that replayed without its decision would come back offering to approve something + that was approved an hour ago. Both halves therefore have to survive, and `plan:proposed` has to land + after the agent's introduction to it rather than inside the sentence. + */ + test('a plan is durable and lands after the words that introduce it', () => { + const { durable } = run([ + { type: 'delta', text: 'Here is what I propose:' }, + { type: 'plan:proposed', requestId: 'plan_1', plan: '# Plan\n\nDo the thing.' }, + ]); + expect(durable).toEqual([ + { type: 'assistant:text', text: 'Here is what I propose:' }, + { type: 'plan:proposed', requestId: 'plan_1', plan: '# Plan\n\nDo the thing.' }, + ]); + }); + + test('a decision is durable and carries its feedback, so a rejected plan replays with its reason', () => { + // The feedback IS what the agent was told to revise against, so the pair only makes sense together. + const { durable } = run([{ type: 'plan:decided', requestId: 'plan_1', approved: false, feedback: 'too broad' }]); + expect(durable).toEqual([{ type: 'plan:decided', requestId: 'plan_1', approved: false, feedback: 'too broad' }]); + }); + + test('plan mode going on or off is a durable boundary, like a model switch', () => { + // Same reasoning as the model marker: without it, a stretch where the agent researched and refused to + // edit anything reads as an agent that lost its nerve. + const { durable } = run([ + { type: 'delta', text: 'ok' }, + { type: 'plan:mode', enabled: true, ok: true }, + ]); + expect(durable).toEqual([ + { type: 'assistant:text', text: 'ok' }, + { type: 'plan:mode', enabled: true, ok: true }, + ]); + }); + + test('a refused plan-mode change is recorded with its reason', () => { + const { durable } = run([{ type: 'plan:mode', enabled: true, ok: false, error: 'nope' }]); + expect(durable).toEqual([{ type: 'plan:mode', enabled: true, ok: false, error: 'nope' }]); + }); + test('a refused switch is recorded too, with its reason', () => { // Silence here would be the worst outcome: the turn answers as the OLD model while the picker shows the // new one, which is the exact mismatch this whole change exists to remove. diff --git a/src/servers/sidecar/claude/turn-stream.ts b/src/servers/sidecar/claude/turn-stream.ts index 58fc7621..6402693d 100644 --- a/src/servers/sidecar/claude/turn-stream.ts +++ b/src/servers/sidecar/claude/turn-stream.ts @@ -150,6 +150,43 @@ export function createTurnStream(sessionId: string): TurnStream { }, ]; + case 'plan:mode': + // Flushed like `model:switched`, and for the same reason: the marker's job is to say WHERE in the + // conversation the rules changed, which is worthless if it lands mid-paragraph. + return [ + ...flushAll(), + { + msg: { + type: 'plan:mode', + enabled: event.enabled, + ok: event.ok, + ...(event.error ? { error: event.error } : {}), + }, + durable: true, + }, + ]; + + case 'plan:proposed': + // Flushed because the agent's closing words before a plan are its introduction to it, and a card + // spliced into an unfinished sentence reads as an interruption rather than a conclusion. + return [ + ...flushAll(), + { msg: { type: 'plan:proposed', requestId: event.requestId, plan: event.plan }, durable: true }, + ]; + + case 'plan:decided': + return [ + { + msg: { + type: 'plan:decided', + requestId: event.requestId, + approved: event.approved, + ...(event.feedback ? { feedback: event.feedback } : {}), + }, + durable: true, + }, + ]; + case 'compact:done': return [ { diff --git a/src/servers/sidecar/claude/user-instance.ts b/src/servers/sidecar/claude/user-instance.ts index 10978efd..9cbaa1df 100644 --- a/src/servers/sidecar/claude/user-instance.ts +++ b/src/servers/sidecar/claude/user-instance.ts @@ -297,6 +297,18 @@ async function handleCommand(cmd: SidecarCommand, reply: ReplyFn) { reply({ type: 'claude:interrupted', id: cmd.id }); break; + // Both plan commands answer through the SESSION LOG rather than with a reply, and that is the point: + // what the user needs to see is `plan:mode` / `plan:decided` arriving in the conversation, durably and + // in order with everything else. A command reply would be a second, weaker channel saying the same + // thing to one socket — and officer sends both of these fire-and-forget, so nothing is waiting on one. + case 'claude:set-plan-mode': + await claudeManager.setClaudePlanMode(cmd.sessionKey, cmd.userId, cmd.enabled); + break; + + case 'claude:plan-decision': + claudeManager.decideClaudePlan(cmd.sessionKey, cmd.userId, cmd.requestId, cmd.approve, cmd.feedback); + break; + case 'claude:list': reply({ type: 'claude:sessions', id: cmd.id, sessions: claudeManager.listSessions(cmd.userId) }); break; diff --git a/src/servers/sidecar/protocol.ts b/src/servers/sidecar/protocol.ts index 6a932122..cda3da5e 100644 --- a/src/servers/sidecar/protocol.ts +++ b/src/servers/sidecar/protocol.ts @@ -59,6 +59,27 @@ export type SidecarCommand = // 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; userId: number } + // Turn plan mode on or off on a session that is already live, without waiting for the next turn. The + // toggle is meant to be usable while reading the agent's last answer, which is exactly when no turn is + // in flight to carry it. + | { type: 'claude:set-plan-mode'; id: string; sessionKey: string; userId: number; enabled: boolean } + /** + * The user's answer to a plan the agent asked to exit with. + * + * This is the only command that resolves a promise the AGENT is blocked on: `canUseTool` is holding + * `ExitPlanMode` open inside the sidecar while this round-trips to a browser and back. `requestId` + * identifies which one — a session can only have a single plan outstanding today, but keying on the id + * means a stale answer from a reconnected client cannot resolve a newer plan. + */ + | { + type: 'claude:plan-decision'; + id: string; + sessionKey: string; + userId: number; + requestId: string; + approve: boolean; + feedback?: string; + } // Which session key owns this transcript? The map lives on the agent's disk, so only it can answer — // see `findClaudeSessionKey` in sidecar-registry, and `attach` in the chat socket for why it is asked. | { type: 'claude:find-session'; id: string; claudeSessionId: string; userId: number } @@ -187,6 +208,15 @@ export type ClaudeSpawnStreamingParams = { cwd?: string; model?: string; resumeSessionId?: string; // resume this Claude session uuid (from the /chat session list) + /** + * Run this turn in PLAN mode — research and propose, execute nothing until the plan is approved. + * + * Carried on the turn for the same reason `model` is: both are `query()` options read once at session + * creation, so both need a control request to change on a live session, and the turn is where the client's + * current intent arrives. Absent means "no opinion" and leaves a live session's mode alone; an explicit + * `false` turns planning off. + */ + planMode?: boolean; // Whether turn output should be committed to chat_session_events (default true). A chat session wants // it — that is what survives an officer restart. A pipeline step does not: its sessionKey is a throwaway // uuid no browser will ever replay, and the job's own event log is its record. diff --git a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts index 88242e4d..1f8bdaa5 100644 --- a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts +++ b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts @@ -74,8 +74,11 @@ export function useEmbeddableChat(params: UseEmbeddableChatParams, onMessageComp selectedModel, hasStarted, thinkingLevel, + planMode, setSelectedModel, setThinkingLevel, + setPlanMode, + decidePlan, sendPrompt, stopGeneration: stopTurn, loadOlder, @@ -572,8 +575,11 @@ export function useEmbeddableChat(params: UseEmbeddableChatParams, onMessageComp selectedModel, hasStarted, thinkingLevel, + planMode, setSelectedModel, setThinkingLevel, + setPlanMode, + decidePlan, sendPrompt, stopGeneration, input, diff --git a/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx b/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx index e78d3a8a..351baae2 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx @@ -41,6 +41,8 @@ export const InputArea = ({ manager }: InputAreaProps) => { appendToInput, thinkingLevel, setThinkingLevel, + planMode, + setPlanMode, queued, sendQueuedNow, unqueue, @@ -162,6 +164,8 @@ export const InputArea = ({ manager }: InputAreaProps) => { thinkingLevel={thinkingLevel} onThinkingChange={setThinkingLevel} onBackgroundJob={() => setJobDialogOpen(true)} + planMode={planMode} + onPlanModeChange={setPlanMode} /> diff --git a/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx b/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx index 67ebdac4..25bf98e8 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx @@ -16,6 +16,7 @@ import { RotateCcw, FoldVertical, ArrowLeftRight, + ClipboardList, } from 'lucide-react'; import { toast } from '@/components/ui/sonner'; import type { Tone } from '@/components/Data'; @@ -120,11 +121,13 @@ type MessageBubbleProps = { message: ChatMessage; onAnswer?: (text: string) => void; onRetry?: (prompt: string) => void; + /** Answer a plan the agent is parked on. Absent in read-only views, where the card renders without buttons. */ + onDecidePlan?: (requestId: string, approve: boolean, feedback?: string) => void; /** Set for rows in the turn you are watching; tool rows start expanded and stay that way until it folds. */ defaultOpen?: boolean; }; -export const MessageBubble = ({ message, onAnswer, onRetry, defaultOpen }: MessageBubbleProps) => { +export const MessageBubble = ({ message, onAnswer, onRetry, onDecidePlan, defaultOpen }: MessageBubbleProps) => { switch (message.role) { case 'user': { const rawText = typeof message.text === 'string' ? message.text : ''; @@ -215,6 +218,12 @@ export const MessageBubble = ({ message, onAnswer, onRetry, defaultOpen }: Messa case 'model': return ; + case 'planmode': + return ; + + case 'plan': + return ; + case 'interrupted': return (
@@ -382,6 +391,132 @@ const ModelSwitch = ({ message }: { message: Extract }) => ( +
+
+ + + {message.ok ? ( + message.enabled ? ( + 'Plan mode on — research only' + ) : ( + 'Plan mode off' + ) + ) : ( + <> + Plan mode unchanged + · {message.error ?? 'the agent refused'} + + )} + +
+
+); + +/** + * A plan, with the two buttons the agent is waiting on. + * + * The only blocking row in the transcript: while `approved` is undefined there is a real `canUseTool` + * promise parked in the sidecar, so this card is not a summary of something that happened — it is the thing + * that has to happen next. It is styled as a card rather than a rule for exactly that reason; every other + * marker here is scenery, and this one is a question. + * + * Rejection takes a reason and sends it, because the harness treats a denial message as revision feedback + * ("revise your plan and call ExitPlanMode again") rather than as an abort. So the useful rejection is a + * sentence, and the box is offered before the button rather than after it. + */ +const PlanCard = ({ + message, + onDecide, +}: { + message: Extract; + onDecide?: (requestId: string, approve: boolean, feedback?: string) => void; +}) => { + const [feedback, setFeedback] = useState(''); + const [rejecting, setRejecting] = useState(false); + const decided = message.approved !== undefined; + + return ( +
+
+
+ + {message.approved === undefined + ? 'Plan — awaiting your approval' + : message.approved + ? 'Plan approved' + : 'Plan rejected'} +
+ +
+ + {message.plan} + +
+ + {/* The reason a rejected plan was rejected, kept on the card. It is what the agent was told, so the + next plan is a response to it and the pair only makes sense read together. */} + {decided && message.feedback && ( +
{message.feedback}
+ )} + + {!decided && onDecide && ( +
+ {rejecting && ( +