From 0c2ff88c8881f7ff6e8602008d733aed92b38a0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sat, 22 Aug 2026 04:29:35 +0000 Subject: [PATCH] a commander is a commander wherever it is opened MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixing a regression I introduced last night. Commander rows in the Live list were changed to open in /chat rather than navigating to Command HQ — right, and it cost the composer its commander controls, because `isCommander` was computed from the ROUTE: `manager.context === 'command-hq'`. So the same commander, rendered on a different screen, silently lost "exit and hand back, then re-arm" — the only fire behaviour a watcher wants, and the one the dialog makes default inside a commander. GET /chat/sessions/:id now reports isCommander, derived from the session's own cwd by the same isCommandersDir already used by the sessions list and /chat/live. The answer travels with the conversation. The composer reads one value, manager.isCommander, true when EITHER source says so — and both are needed. The screen knows when Command HQ starts a commander, before a session or a transcript exists for any route to report on. The session knows once it does, wherever it is opened. Screen-only was this bug; session-only would have broken the brand-new composer, which is where the whole thing starts. Verified on a real commander rather than by reading: 172cdd8c resolves cwd=…/commanders and isCommander=true. Co-Authored-By: Claude Opus 5 --- TODO.md | 4 ++-- src/servers/api/chat/chat.ts | 10 ++++++++++ .../officerdev/src/apps/Chat/ChatPanelWrapper.tsx | 6 ++++++ .../apps/Chat/EmbeddableChat/EmbeddableChat.tsx | 8 ++++++++ .../apps/Chat/EmbeddableChat/useEmbeddableChat.ts | 11 +++++++++++ .../src/apps/Chat/components/InputArea.tsx | 6 +++++- src/workspaces/state/src/useClaudeSessions.ts | 8 ++++++++ ...ommander-opened-from-chat-loses-its-context.md | 15 ++++++++++++++- 8 files changed, 64 insertions(+), 4 deletions(-) diff --git a/TODO.md b/TODO.md index dddf046b..9d874cc6 100644 --- a/TODO.md +++ b/TODO.md @@ -20,7 +20,7 @@ the owner's OS user and can never be granted. Indirection there really is accide ## Themes -**86 open, 23 closed**, across 29 themes. +**85 open, 24 closed**, across 29 themes. | Theme | Open | Closed | | --- | --- | --- | @@ -35,7 +35,7 @@ the owner's OS user and can never be granted. Indirection there really is accide | [Fleet — one command across every server](todo/fleet/) | 5 | 2 | | [Headscale](todo/headscale/) | 0 | 5 | | [Infra (alpha)](todo/infra/) | 6 | 0 | -| [Known bugs](todo/known-bugs/) | 4 | 1 | +| [Known bugs](todo/known-bugs/) | 3 | 2 | | [Mobile apps](todo/mobile-apps/) | 3 | 0 | | [Multi-user](todo/multi-user/) | 17 | 4 | | [Notifications](todo/notifications/) | 1 | 0 | diff --git a/src/servers/api/chat/chat.ts b/src/servers/api/chat/chat.ts index 9345d179..0333ebe7 100644 --- a/src/servers/api/chat/chat.ts +++ b/src/servers/api/chat/chat.ts @@ -257,6 +257,16 @@ chatRouter.get('/sessions/:id', async (ctx) => { offset: windowed ? start : 0, title: context?.title ?? null, partCount: context?.partCount ?? 1, + /* + Whether this conversation is a commander, derived from ITS OWN cwd rather than from the screen asking. + + The composer offers "exit and hand back, then re-arm" — the only fire behaviour a watcher wants — and + decided whether to by looking at the route it was rendered on. So the same commander, opened from + /chat instead of Command HQ, silently lost the control that is the whole point of it. A commander is a + commander wherever it is rendered, and `detail.cwd` is the transcript's own directory, so this answer + travels with the conversation. + */ + isCommander: isCommandersDir(who.email, detail.cwd), }); }); diff --git a/src/workspaces/officerdev/src/apps/Chat/ChatPanelWrapper.tsx b/src/workspaces/officerdev/src/apps/Chat/ChatPanelWrapper.tsx index b7c8e714..5cbe7536 100644 --- a/src/workspaces/officerdev/src/apps/Chat/ChatPanelWrapper.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/ChatPanelWrapper.tsx @@ -19,6 +19,8 @@ type ResumeState = { messages: ChatMessage[]; total: number; offset: number; + /** Carried so the composer offers a commander's own controls here, not only in Command HQ. */ + isCommander: boolean; }; // Transcripts get extremely long, so only the tail is loaded up front; the chat pages older messages in @@ -104,6 +106,9 @@ const ChatPanelInner = ({ promptPrefix={promptPrefix} panelId={panelId} sessionId={sessionKey ?? undefined} + // From the SESSION, so a commander opened here gets a commander's composer. `chatContext` may also + // set it via `context`; either being true is enough — see `manager.isCommander`. + isCommander={resume?.isCommander === true} {...chatContext} /> @@ -221,6 +226,7 @@ export const ChatPanelWrapper = ({ panelId, promptPrefix }: ChatPanelWrapperProp messages: detail.messages as unknown as ChatMessage[], total: detail.total, offset: detail.offset, + isCommander: detail.isCommander === true, }); } catch { // A stale id — the transcript was deleted or pruned from under us. Open a NEW conversation rather diff --git a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/EmbeddableChat.tsx b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/EmbeddableChat.tsx index 67867b5c..3207af75 100644 --- a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/EmbeddableChat.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/EmbeddableChat.tsx @@ -23,6 +23,14 @@ type EmbeddableChatProps = { chat?: UseChatType; context?: string; contextId?: string; + /** + * This SESSION is a commander, as reported by `GET /chat/sessions/:id`. + * + * Separate from `context`, which says which SCREEN is rendering. Both can make the composer offer the + * handoff watcher and only this one is true of the conversation wherever it is opened — which is the bug: + * a commander opened in /chat had `context: 'chat'` and silently lost the control. + */ + isCommander?: boolean; onMessageComplete?: () => void; }; diff --git a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts index 8d4b18dc..f4737538 100644 --- a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts +++ b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts @@ -49,6 +49,8 @@ type UseEmbeddableChatParams = { chat?: UseChatType; context?: string; contextId?: string; + /** This session is a commander, per `GET /chat/sessions/:id`. See the note where it is surfaced. */ + isCommander?: boolean; }; /** @@ -611,6 +613,15 @@ export function useEmbeddableChat(params: UseEmbeddableChatParams, onMessageComp // place a `handoff` watcher can be promised — see BackgroundJobDialog — and it was invisible to the // composer, so the option was never rendered anywhere at all. context: params.context, + /* + One answer to "is this a commander", from either of the two things that can know. + + The SCREEN knows when Command HQ starts one, before the session exists and before any route could + report on it. The SESSION knows once it has a transcript, wherever it is opened. Neither alone is + enough: screen-only was the original bug (a commander in /chat lost its controls) and session-only + would lose the brand-new composer, which has no session yet. + */ + isCommander: params.context === 'command-hq' || params.isCommander === true, messages, historicalCount, streamingText, diff --git a/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx b/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx index 4d319da1..913214e1 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx @@ -199,12 +199,16 @@ export const InputArea = ({ manager }: InputAreaProps) => { `handoff` — exit, act, re-arm — is only offered inside a Command HQ commander, because it is a promise the platform has to keep rather than only a sentence in the prompt. + Read from the MANAGER rather than recomputed from the route, which is what it was: a commander + opened from /chat reported `context: 'chat'` and lost the option, having gained it two hours + earlier. `manager.isCommander` is true when either the screen or the session says so. + This prop was never passed by anybody, so `isCommander` defaulted to false everywhere and the option was rendered NOWHERE — including in a commander, where the dialog's own code makes it the default. The whole mechanism existed: the fire behaviour, the contract, `armWatcher` reaching the server. Only the radio button was missing, so the feature looked unbuilt. */ - isCommander={manager.context === 'command-hq'} + isCommander={manager.isCommander} onSubmit={(intent, preamble, armWatcher) => { // Claimed by the next job to start, which is how the preview tells this one apart from the // background jobs an agent starts on its own. diff --git a/src/workspaces/state/src/useClaudeSessions.ts b/src/workspaces/state/src/useClaudeSessions.ts index a3bbc004..ec6d486a 100644 --- a/src/workspaces/state/src/useClaudeSessions.ts +++ b/src/workspaces/state/src/useClaudeSessions.ts @@ -96,6 +96,14 @@ export type ClaudeSessionDetail = { title?: string | null; /** Transcripts spliced into `messages` — `> 1` when this is a merged `/clear` chain. */ partCount?: number; + /** + * A commander rather than an ordinary conversation, derived server-side from this session's own cwd. + * + * It travels with the SESSION and not with the screen, which is the whole point. The composer decides + * whether to offer "exit and hand back, then re-arm" — the only fire behaviour a watcher wants — and used + * to decide it from the route, so the same commander opened in /chat quietly lost it. + */ + isCommander?: boolean; }; export type ClaudePwd = { cwd: string; sessionCount: number; updatedAt: string; isDefault: boolean }; diff --git a/todo/known-bugs/a-commander-opened-from-chat-loses-its-context.md b/todo/known-bugs/a-commander-opened-from-chat-loses-its-context.md index fb731adc..91bfba72 100644 --- a/todo/known-bugs/a-commander-opened-from-chat-loses-its-context.md +++ b/todo/known-bugs/a-commander-opened-from-chat-loses-its-context.md @@ -2,7 +2,7 @@ id: known-bugs/a-commander-opened-from-chat-loses-its-context title: 'A commander opened from /chat loses its commander controls' theme: known-bugs -status: open +status: done created: 2026-08-21 date_source: stated order: 4 @@ -28,3 +28,16 @@ closes this and makes the same thing work in any future surface that renders a c Related: the same `context` name means two different things either side of `EmbeddableChat` — a `useChat` option and a component prop, neither reading the other's — which is what hid the original bug twice. + +--- + +**Fixed 2026-08-22.** `GET /chat/sessions/:id` now reports `isCommander`, derived from the session's own cwd +by the same `isCommandersDir` the list and `/chat/live` already used — so the answer travels with the +conversation instead of with the screen showing it. + +The composer reads ONE value, `manager.isCommander`, true when either source says so. Both are needed and +neither is sufficient: the SCREEN knows when Command HQ starts a commander, before a session or a transcript +exists for any route to report on, and the SESSION knows once it does, wherever it is opened. Screen-only +was this bug; session-only would have lost the brand-new composer. + +Verified on a real commander: `172cdd8c` resolves `cwd=…/commanders` and `isCommander=true`.