From 96ceb3aca6a629eb16e1f14f1397d62a686ce5d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 4 Aug 2026 01:36:35 +0000 Subject: [PATCH] delete the claude-done hook and its unauthenticated endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chain was: a Stop hook in Claude's settings curls POST /api/hooks/claude-done, the platform POSTs /_officer/panel-refresh to the pty sidecar, the sidecar sends a `panel-refresh` frame to every attached terminal, and the Claude Code panel bumps `preview:refresh` and `files:refresh-signal`. It has never fired. generateClaudeSettings writes settings.json into the MANAGED home under DATA_PATH, but HOME_DIR points terminals at the owner's real login home — which is where Claude reads its settings from. Verified on this machine: no claude-done hook exists in ~/.claude/settings.json, and DATA_PATH/*/home/.claude does not exist at all. Deleting rather than repairing it, because the Chat panel already does exactly this job from onTurnComplete — in-process, conditioned on the turn having made tool calls, with no hook, no HTTP round trip, and no endpoint. The chat UI is where agent work happens; the terminal TUI is not the destination. Also removes /api/hooks/claude-done, which was mounted above protectedRouter and so was the one unauthenticated write-ish endpoint on the API surface. Co-Authored-By: Claude Opus 5 --- docs/claude-sidecar-isolation.md | 13 +++++++--- src/servers/api/terminal/sidecar-server.ts | 4 +-- src/servers/api/terminal/websocket.ts | 11 +------- src/servers/generate-container-context.ts | 23 +++++----------- src/servers/hono.ts | 9 ------- src/servers/sidecar/pty/server.mjs | 8 +----- src/servers/sidecar/pty/sessions.mjs | 5 ---- .../apps/Terminal/CommandTerminalWrapper.tsx | 9 ++++--- .../officerdev/src/apps/Terminal/Terminal.tsx | 6 ----- .../officerdev/src/apps/Terminal/index.tsx | 26 +++++-------------- 10 files changed, 31 insertions(+), 83 deletions(-) diff --git a/docs/claude-sidecar-isolation.md b/docs/claude-sidecar-isolation.md index e53b95d4..f4c0e518 100644 --- a/docs/claude-sidecar-isolation.md +++ b/docs/claude-sidecar-isolation.md @@ -255,10 +255,15 @@ Now the structural fix, which is also most of the Pass 1 claude findings: The rest of the Pass 1 claude findings, now unblocked: the transcript reader (`chat/claude-sessions.ts`, 361 lines), the turn-loop state machine (`chat/websocket.ts:168-312`), session CRUD (`chat/chat.ts`), the model catalogue (`chat/list-models.ts:5-9`), CLI install/auth -(`server-settings/claude-code.ts`), and `generate-container-context.ts`'s settings writer. Also point -the `Stop` hook at the sidecar instead of `http://localhost:5000/api/hooks/claude-done` -(`generate-container-context.ts:144-153`, `hono.ts:79-86`) — as written, that hook silently fails -during exactly the restart window we care about. +(`server-settings/claude-code.ts`), and `generate-container-context.ts`'s settings writer. + +The `Stop` hook that used to be part of this stage is gone rather than moved. It curled +`http://localhost:5000/api/hooks/claude-done` so the file browser would refresh when the terminal's +Claude finished a turn, and it had never once fired: the writer installs it into the managed home under +`DATA_PATH`, while `HOME_DIR` points terminals at the owner's real login home, which is where Claude +reads its settings from. The whole chain — unauthenticated endpoint, sidecar `POST /_officer/panel-refresh`, +`panel-refresh` frame, `onPanelRefresh` prop — was deleted on 2026-08-04. The Chat panel already does the +same job from `onTurnComplete`, in-process, with no hook and no HTTP round trip. ### Stage 5 — the harder question: surviving a *sidecar* restart diff --git a/src/servers/api/terminal/sidecar-server.ts b/src/servers/api/terminal/sidecar-server.ts index 8e500838..ba78901d 100644 --- a/src/servers/api/terminal/sidecar-server.ts +++ b/src/servers/api/terminal/sidecar-server.ts @@ -1,8 +1,8 @@ import { createSidecarProxy } from '../../sidecar/create-proxy'; // `/api/terminal/*` is a plain auth-and-forward proxy onto the pty sidecar's own listener, like every -// other HTTP sidecar. The sidecar owns `/_officer/sessions` (list), `DELETE /_officer/sessions/:id` (kill) -// and `POST /_officer/panel-refresh`; the platform knows none of those contracts, only where to send them. +// other HTTP sidecar. The sidecar owns `/_officer/sessions` (list) and `DELETE /_officer/sessions/:id` +// (kill); the platform knows neither contract, only where to send them. // // `/api/terminal/ws` does NOT come through here — Bun's route table takes it first and hands it to the // byte relay in `websocket.ts`. diff --git a/src/servers/api/terminal/websocket.ts b/src/servers/api/terminal/websocket.ts index 491c35ab..5d7a4f93 100644 --- a/src/servers/api/terminal/websocket.ts +++ b/src/servers/api/terminal/websocket.ts @@ -1,5 +1,5 @@ import type { ServerWebSocket } from 'bun'; -import { getTerminalWsUrl, getTerminalHttpUrl } from './sidecar-server'; +import { getTerminalWsUrl } from './sidecar-server'; // Byte relay between the browser and the pty sidecar's own socket. // @@ -93,12 +93,3 @@ export const terminalWebsocket = { drain() {}, }; - -// The claude-done hook asks attached terminals to refresh their panel. The sidecar holds those sockets -// now, so this is a POST to it rather than a loop over sockets officer used to own. Fire-and-forget: a -// missed refresh is a stale panel, not a failure worth surfacing. -export const broadcastPanelRefresh = (_email: string): void => { - const base = getTerminalHttpUrl(); - if (!base) return; - fetch(`${base}/_officer/panel-refresh`, { method: 'POST' }).catch(() => {}); -}; diff --git a/src/servers/generate-container-context.ts b/src/servers/generate-container-context.ts index 4c624419..60a09c4f 100644 --- a/src/servers/generate-container-context.ts +++ b/src/servers/generate-container-context.ts @@ -2,11 +2,8 @@ import { existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from import { join } from 'node:path'; import { itemsDir, getHomeDir, DATA_PATH } from '@@/data-path'; -type HookEntry = { type: string; command: string }; -type HookRule = { matcher?: Record; hooks: HookEntry[] }; -type ClaudeSettings = Record & { - hooks?: Record; -}; +// Whatever the host's settings.json holds is preserved verbatim; we only add the two permission keys below. +type ClaudeSettings = Record; type FrontmatterEntry = { name: string; description: string }; @@ -141,17 +138,11 @@ export function generateClaudeSettings(email: string, username?: string): string // no host settings } - const hookCommand = `curl -s -X POST http://localhost:5000/api/hooks/claude-done -H 'Content-Type: application/json' -d '{"email":"${email}"}'`; - const hooks = settings.hooks ?? {}; - const stopRules = hooks.Stop ?? []; - const hasOurHook = stopRules.some((rule) => rule.hooks?.some((h) => h.command?.includes('/api/hooks/claude-done'))); - - if (!hasOurHook) { - stopRules.push({ hooks: [{ type: 'command', command: hookCommand }] }); - } - - hooks.Stop = stopRules; - settings.hooks = hooks; + // A Stop hook used to be injected here, curling /api/hooks/claude-done so the file browser would refresh + // when the terminal's Claude finished. It never fired: this writes to the MANAGED home under DATA_PATH, + // while HOME_DIR points terminals at the owner's real login home, where Claude actually reads its + // settings. The chat UI — the destination for agent work — refreshes those panels itself from + // onTurnComplete, with no hook, no HTTP round trip and no unauthenticated endpoint. settings.defaultMode = 'bypassPermissions'; settings.skipDangerousModePermissionPrompt = true; diff --git a/src/servers/hono.ts b/src/servers/hono.ts index 5116b930..a5e2b4a0 100644 --- a/src/servers/hono.ts +++ b/src/servers/hono.ts @@ -45,7 +45,6 @@ import { desktopRouter } from './api/desktop/rest'; import { bugReportRouter } from './api/bug-report/bug-report'; import { chatRouter } from './api/chat/chat'; import { pipelineJobsRouter } from './api/tasks/pipeline-jobs-routes'; -import { broadcastPanelRefresh } from './api/terminal/websocket'; import { CustomError } from './custom-errors'; import { userMiddleware, bodyParser, isOriginAllowed, originScopeMiddleware } from './_middlewares'; import { isMusicOriginExempt } from './_middlewares/origin-validation'; @@ -84,14 +83,6 @@ honoServer.route('/api/waitlist', waitlistRouter); // notifications WebSocket is upgraded at the serve level (server.tsx). honoServer.route('/api/vault', vaultRouter); honoServer.get('/api/integrations/google/callback', googleCallbackHandler); -honoServer.post('/api/hooks/claude-done', async (ctx) => { - const body = await ctx.req.json().catch(() => null); - const email = (body as Record | null)?.email; - if (typeof email === 'string' && email.includes('@')) { - broadcastPanelRefresh(email); - } - return ctx.json({ ok: true }); -}); const protectedRouter = createRouter(); protectedRouter.use(bodyParser()); diff --git a/src/servers/sidecar/pty/server.mjs b/src/servers/sidecar/pty/server.mjs index c59de87c..cf640952 100644 --- a/src/servers/sidecar/pty/server.mjs +++ b/src/servers/sidecar/pty/server.mjs @@ -7,7 +7,7 @@ import * as store from './sessions.mjs'; // browser and relays bytes here without reading them. // // The socket protocol is the one the browser already speaks, unchanged: {input,resize} in, -// {output,replay,exit,panel-refresh} out. That is deliberate — the frontend did not have to move for the +// {output,replay,exit} out. That is deliberate — the frontend did not have to move for the // transport to. const wsSend = (socket) => ({ @@ -37,12 +37,6 @@ export function startServer() { return json(res, killed ? 200 : 404, { ok: killed }); } - // The claude-done hook: tell attached terminals to refresh their panel. - if (url.pathname === '/_officer/panel-refresh' && req.method === 'POST') { - store.broadcastPanelRefresh(); - return json(res, 200, { ok: true }); - } - json(res, 404, { error: 'not found' }); }); diff --git a/src/servers/sidecar/pty/sessions.mjs b/src/servers/sidecar/pty/sessions.mjs index aaaf46f7..4935175f 100644 --- a/src/servers/sidecar/pty/sessions.mjs +++ b/src/servers/sidecar/pty/sessions.mjs @@ -170,11 +170,6 @@ export function list() { })); } -/** Tell every attached client to refresh its panel — fired by the platform's claude-done hook. */ -export function broadcastPanelRefresh() { - for (const session of sessions.values()) broadcast(session, { type: 'panel-refresh' }); -} - export function killAll() { for (const session of sessions.values()) { try { diff --git a/src/workspaces/officerdev/src/apps/Terminal/CommandTerminalWrapper.tsx b/src/workspaces/officerdev/src/apps/Terminal/CommandTerminalWrapper.tsx index 68228d0f..0a227e2b 100644 --- a/src/workspaces/officerdev/src/apps/Terminal/CommandTerminalWrapper.tsx +++ b/src/workspaces/officerdev/src/apps/Terminal/CommandTerminalWrapper.tsx @@ -11,13 +11,15 @@ type CommandTerminalWrapperProps = { panelId: string; command: string; statePrefix: string; - onPanelRefresh?: () => void; }; -export const CommandTerminalWrapper = ({ panelId, command, statePrefix, onPanelRefresh }: CommandTerminalWrapperProps) => { +export const CommandTerminalWrapper = ({ panelId, command, statePrefix }: CommandTerminalWrapperProps) => { const { dashboardId, cwd } = useWorkspace(); const stateKey = dashboardId ? `ws-${statePrefix}-${dashboardId}` : `ws-${statePrefix}-default`; - const { value: terminals, setValue: setTerminals } = useDashboardState>(stateKey, EMPTY_TERMINALS); + const { value: terminals, setValue: setTerminals } = useDashboardState>( + stateKey, + EMPTY_TERMINALS, + ); const sessionId = terminals[panelId]; useEffect(() => { @@ -43,7 +45,6 @@ export const CommandTerminalWrapper = ({ panelId, command, statePrefix, onPanelR sessionId={sessionId} cwd={cwd} initialInput={fullCommand} - onPanelRefresh={onPanelRefresh} onConnectionChange={onConnectionChange} /> ); diff --git a/src/workspaces/officerdev/src/apps/Terminal/Terminal.tsx b/src/workspaces/officerdev/src/apps/Terminal/Terminal.tsx index 7c0e0716..aa829e19 100644 --- a/src/workspaces/officerdev/src/apps/Terminal/Terminal.tsx +++ b/src/workspaces/officerdev/src/apps/Terminal/Terminal.tsx @@ -38,7 +38,6 @@ export type TerminalViewProps = { onExit?: () => void; onCommandDone?: (exitCode: number, output: string) => void; onDisconnect?: () => void; - onPanelRefresh?: () => void; onConnectionChange?: (state: TerminalConnectionState) => void; /** The shell's own title (OSC 0/2) — what's actually running, for the panel header. */ onTitleChange?: (title: string) => void; @@ -89,7 +88,6 @@ export const TerminalView = ({ onExit, onCommandDone, onDisconnect, - onPanelRefresh, onConnectionChange, onTitleChange, onBell, @@ -105,7 +103,6 @@ export const TerminalView = ({ const onExitRef = useRef(onExit); const onCommandDoneRef = useRef(onCommandDone); const onDisconnectRef = useRef(onDisconnect); - const onPanelRefreshRef = useRef(onPanelRefresh); const onConnectionChangeRef = useRef(onConnectionChange); const commandRef = useRef(command); const initialInputRef = useRef(initialInput); @@ -114,7 +111,6 @@ export const TerminalView = ({ onExitRef.current = onExit; onCommandDoneRef.current = onCommandDone; onDisconnectRef.current = onDisconnect; - onPanelRefreshRef.current = onPanelRefresh; onConnectionChangeRef.current = onConnectionChange; commandRef.current = command; initialInputRef.current = initialInput; @@ -295,8 +291,6 @@ export const TerminalView = ({ onExitRef.current?.(); } else if (msg.type === 'detached') { term.write('\r\n[Session taken over]\r\n'); - } else if (msg.type === 'panel-refresh') { - onPanelRefreshRef.current?.(); } } catch { // ignore diff --git a/src/workspaces/officerdev/src/apps/Terminal/index.tsx b/src/workspaces/officerdev/src/apps/Terminal/index.tsx index 81dbf3f6..4fbfd573 100644 --- a/src/workspaces/officerdev/src/apps/Terminal/index.tsx +++ b/src/workspaces/officerdev/src/apps/Terminal/index.tsx @@ -1,7 +1,5 @@ -import { useCallback } from 'react'; import type { AppRegistryMeta } from '../../AppRegistry'; import { TerminalSquare, Monitor, Columns2, PenLine, Sparkles, ListTree } from 'lucide-react'; -import { usePanelChannel } from 'hooks/usePanelChannel'; import { TerminalWrapper } from './TerminalWrapper'; import { HostTerminalWrapper } from './HostTerminalWrapper'; import { CommandTerminalWrapper } from './CommandTerminalWrapper'; @@ -29,24 +27,12 @@ const NvimWrapper = ({ panelId }: { panelId: string }) => ( ); -const ClaudeCodeWrapper = ({ panelId }: { panelId: string }) => { - const [, setPreviewRefresh] = usePanelChannel('preview:refresh', 0); - const [, setFilesRefresh] = usePanelChannel('files:refresh-signal', 0); - - const onPanelRefresh = useCallback(() => { - setPreviewRefresh(Date.now()); - setFilesRefresh(Date.now()); - }, [setPreviewRefresh, setFilesRefresh]); - - return ( - - ); -}; +// No turn-complete refresh here. Claude's Stop hook was supposed to drive one, but it was written into the +// managed home under DATA_PATH while HOME_DIR points terminals at the owner's real login home — so it was +// never installed and never fired. The Chat panel does the same job from onTurnComplete, in-process. +const ClaudeCodeWrapper = ({ panelId }: { panelId: string }) => ( + +); export const appRegistryMetas: AppRegistryMeta[] = [ {