diff --git a/docs/chat-panes-next-session.md b/docs/chat-panes-next-session.md deleted file mode 100644 index a1d8a4b0..00000000 --- a/docs/chat-panes-next-session.md +++ /dev/null @@ -1,74 +0,0 @@ -# Multi-pane chat: where it stands, and the one open bug - -Written 2026-08-10 at the end of a long session, so the next one starts from evidence instead of -re-deriving it. - -## The open bug - -**A pane pointed at a REMOTE server shows `Disconnected` and never gets a reply.** The pane opens, lists -and reads that server's conversations fine (HTTP with the API key works). Sending produces nothing. - -Console, repeatedly: - -``` -WebSocket connection to 'wss://officer.pastilhas.dev/api/chat/ws?token=ofk_…' failed: -WebSocket is closed before the connection is established. -``` - -The same message also appears for the LOCAL pane's `ws://localhost:9010/...`, yet the local pane works -and shows connected. So the message alone is not the bug — something closes the remote one and it never -comes back. - -## Ruled out, each by direct test — do not re-test these - -- **The server.** `wss://officer.pastilhas.dev/api/chat/ws?token=` opens on the first - try from outside the browser. Tested twice: with no `Origin` header, and with - `Origin: http://localhost:9010` (what the browser sends). Both `OPEN`. -- **The key.** Same key, works over HTTP for `/chat/sessions`, and opens the socket above. -- **Websocket auth not understanding API keys.** `upgradeWs` in `server.tsx` uses `resolveAuthToken`, the - same resolver as the HTTP doors, and comments say so explicitly. `chat` is an `execution` capability - and the owner passes `isWsProviderAllowed`. -- **Officer being stale on alpha.** Pulled and restarted; the failure persists. -- **Send being dropped.** Fixed in `243bd04` — `useChatWebSocket.send` used to `return` silently when the - socket was not `OPEN`. It now queues and flushes on open. That fix is real and worth keeping, but it - did not resolve this: a socket that never opens never flushes. - -## Where to look next - -`src/workspaces/hooks/src/useChatWebSocket.ts`, and specifically what happens with SEVERAL instances -mounted at once — one per pane. - -The handlers look individually correct: `close` returns early when `socketRef.current !== socket`, the -effect depends on `[url]` alone, and `isCleaningUpRef` is reset on mount. What has NOT been established -is how those interact across three simultaneous instances plus React's dev StrictMode double-invoke, -which creates, closes and recreates every socket on mount. - -Concrete things to try, cheapest first: - -1. **Instrument before theorising.** Log `url`, `readyState` and instance identity on every create, - open, close and retry. The console message says a close arrived during CONNECTING; it does not say - who called it. That is the whole question and it is one log line away. -2. **Check whether the effect re-runs.** `url` is a string, so it should be stable — but - `chatSocketUrl(serverId, localStorage.getItem('BEARER_TOKEN'))` is recomputed every render, and if - anything makes `serverId` flip (say, a pane re-resolving its target) the URL changes and the socket - is torn down mid-handshake, forever. -3. **Suspect the transcript resolver in `ChatPane`.** It calls `onTargetChange` on success, which - rewrites the tab state, which produces a new `target` object for every pane in that tab. If that ever - loops, every pane's socket is rebuilt on every pass and none survives the handshake. -4. **Try one pane pointed at the remote server, alone.** If it connects, the bug is about multiplicity - rather than about the remote server, which would be the single most useful fact to have. - -## What works, so it is not re-litigated - -- Adding a second server by URL + `ofk_` key, verified against `/api/auth/me` before storing. -- Per-pane server chips; the list, the directory picker and the transcript all follow the pane's server. -- Tabs, splitting to three panes, closing, renaming a tab, and the tab name winning the page title. -- Reading a remote conversation end to end. - -## Setup facts worth not rediscovering - -- Andre's Mac key lives in agent memory. Alpha needs its own; the one in the console above is alpha's. -- `bun dev` on the Mac serves a STALE bundle whenever a new file is added to a workspace package — the - symptom is `X is not a function` for an export that plainly exists. Restart it; do not debug it. -- The Mac is `https://macbook.pastilhas.dev` through NPM on alpha (`100.64.0.1` → Mac `100.64.0.9:9010`), - and the cert is valid under strict TLS. diff --git a/src/apps/officer-web/Screens/Dashboard/ChatHistory/index.tsx b/src/apps/officer-web/Screens/Dashboard/ChatHistory/index.tsx index 943d082e..6cbc06c7 100644 --- a/src/apps/officer-web/Screens/Dashboard/ChatHistory/index.tsx +++ b/src/apps/officer-web/Screens/Dashboard/ChatHistory/index.tsx @@ -1,12 +1,15 @@ import { useEffect, useRef } from 'react'; import { useParams, useNavigate } from 'react-router'; -import type { SelectedSession } from 'officerdev'; -import { ChatTabs, chatListPath, cwdFromSplat, useSelectedChatSession } from 'officerdev'; +import type { LayoutNode, SelectedSession } from 'officerdev'; +import { WorkspaceView, chatListPath, cwdFromSplat, useSelectedChatSession } from 'officerdev'; import { toast } from '@/components/ui/sonner'; +import { useIsMobile } from 'hooks/useIsMobile'; import { useClient } from 'hooks/useClient'; import { serverClient } from 'hooks/useServerClient'; import { errorText } from 'helpers/error-text'; +import { useDashboardState } from 'state/useDashboardState'; import type { ClaudeSessionDetail } from 'state/useClaudeSessions'; +import { defaultLayout, hasAppType } from './defaultLayout'; // How many messages to render on first open (anchored to the bottom); scroll-up pages older ones in. const CHAT_TAIL = 20; @@ -25,7 +28,26 @@ export const SessionListPage = ({ isNew }: SessionListPageProps) => { selectedRef.current = selected; // A layout persisted before the chat panels were renamed still names `officerdev/chat`, which no // longer resolves; `appTypes` lands anything unknown on the detail panel. + const workspace = useDashboardState('screens/chat', defaultLayout); + const isMobile = useIsMobile(); const navigate = useNavigate(); + const mobilePanelId = isMobile && (sessionId || isNew) ? 'chat-detail' : undefined; + + // Adopt a structural change to this screen's layout. + // + // `useDashboardState` seeds its default ONLY when the key is absent, so anyone who has ever opened + // /chat keeps the shape it had then — for good. `appTypes`/`normalizeLayout` does not help: it repairs + // which app a panel runs, never the tree, so adding the Live panel above the list would have been + // invisible to every existing user and visible only on a fresh account. + // + // Replacing outright is safe *here* specifically because the screen is `locked`: its structure is + // dictated by code and the only thing a user can have contributed is the column sizes, which is a + // cheap thing to lose once. Terminates because the replacement contains the panel it tests for. + useEffect(() => { + if (!workspace.isLoaded) return; + if (hasAppType(workspace.value, 'chat-live')) return; + workspace.setValue(defaultLayout); + }, [workspace.isLoaded, workspace.value, workspace.setValue]); // Retire a legacy `?cwd=`. Nothing reads it any more and nothing writes it, but a refresh re-requests // the address bar verbatim — so one left over from before the path-based groups sits there forever, @@ -98,15 +120,20 @@ export const SessionListPage = ({ isNew }: SessionListPageProps) => { // eslint-disable-next-line react-hooks/exhaustive-deps }, [sessionId, isNew, groupCwd]); - // The tabbed, multi-pane chat replaces the fixed three-panel workspace. The panels themselves are - // unchanged and still registered for the dashboard; what changes is that a PANE owns its conversation - // rather than the whole screen sharing one, which is what lets two machines be live side by side. - // - // `useDashboardState`/`WorkspaceView` are no longer used here. The layout that matters now is the tab - // blob in localStorage, because a tab spanning two servers cannot be stored per server. return (
- + { + // Back goes to the group's list, not the default one. On /chat/g/* that group is in the URL; + // on /chat/ it isn't (deliberately — see chat-routes.ts), so fall back to the open + // session's own directory, which the resolve above put on the selection. + if (!id) navigate(chatListPath(groupCwd ?? selectedRef.current?.cwd ?? null), { replace: true }); + }} + />
); }; diff --git a/src/apps/officer-web/state/usePageTitle.ts b/src/apps/officer-web/state/usePageTitle.ts index 1d9e666c..3693145e 100644 --- a/src/apps/officer-web/state/usePageTitle.ts +++ b/src/apps/officer-web/state/usePageTitle.ts @@ -1,7 +1,7 @@ import { useCallback, useEffect, useRef } from 'react'; import { useLocation } from 'react-router'; import type { PageTitleOverride } from 'officerdev'; -import { usePageTitleOverride, useChatTabName } from 'officerdev'; +import { usePageTitleOverride } from 'officerdev'; import { useSessionState, writeSessionValue } from 'hooks/useSessionState'; type TitleRule = { match: (p: string) => boolean; title: string }; @@ -160,7 +160,6 @@ claimTabIdentity(); export function usePageTitle() { const { pathname } = useLocation(); const override = usePageTitleOverride(); - const chatTabName = useChatTabName(); const [label, setLabel] = useSessionState(TAB_LABEL_KEY, null); useEffect(() => onTabLabelDropped(() => setLabel(null)), [setLabel]); @@ -186,10 +185,7 @@ export function usePageTitle() { const rename = useCallback((next: string) => setLabel(next.trim() || null), [setLabel]); - // A chat tab's own name is the most specific thing anyone has said about this page — more specific - // than the conversation inside it (there may be three) and more deliberate than a browser-tab name - // typed earlier on a different screen. So it wins outright. - return [chatTabName ?? label ?? override?.title ?? titleForPath(pathname), rename] as const; + return [label ?? override?.title ?? titleForPath(pathname), rename] as const; } /** diff --git a/src/servers/api/chat/session-manager.ts b/src/servers/api/chat/session-manager.ts index fcde0940..c697b332 100644 --- a/src/servers/api/chat/session-manager.ts +++ b/src/servers/api/chat/session-manager.ts @@ -24,7 +24,7 @@ class SessionManager { cwd, model, piProcess: null, - sockets: new Set(), + ws: null, lastActivity: Date.now(), idleTimer: null, streamBuffer: '', @@ -145,7 +145,7 @@ class SessionManager { attachWs(sessionId: string, ws: any): void { const session = this.sessions.get(sessionId); if (session) { - session.sockets.add(ws); + session.ws = ws; session.lastActivity = Date.now(); if (session.idleTimer) { @@ -155,24 +155,14 @@ class SessionManager { } } - /** - * Removes one socket. The caller must say WHICH — a bare `detachWs(sessionId)` used to null the - * session's only socket field, so a stale client's close event silenced whichever client had attached - * after it. A close is only the end of the conversation when nothing else is still watching. - */ - detachWs(sessionId: string, ws: any): void { + detachWs(sessionId: string): void { const session = this.sessions.get(sessionId); if (session) { - session.sockets.delete(ws); + session.ws = null; session.lastActivity = Date.now(); } } - /** Whether anything is still watching — the idle GC must not start while another client is attached. */ - hasSockets(sessionId: string): boolean { - return (this.sessions.get(sessionId)?.sockets.size ?? 0) > 0; - } - setIdleTimeout(sessionId: string, timeoutMs: number): void { const session = this.sessions.get(sessionId); if (!session) return; diff --git a/src/servers/api/chat/types.ts b/src/servers/api/chat/types.ts index d49ddc05..b428467d 100644 --- a/src/servers/api/chat/types.ts +++ b/src/servers/api/chat/types.ts @@ -299,13 +299,7 @@ export type UserSession = { cwd: string; model: string; piProcess: any | null; - /** - * Every socket watching this conversation, not the most recent one. Two panes in one window, or a - * laptop and an iPad on the same chat, are both ordinary now that a tab holds several panes — and a - * single `ws` field meant the newest attach silently stole the turn from everyone else, while any one - * of them closing set it to null and killed delivery for the rest. - */ - sockets: Set; + ws: any | null; lastActivity: number; idleTimer: Timer | null; streamBuffer: string; diff --git a/src/servers/api/chat/websocket.ts b/src/servers/api/chat/websocket.ts index ee36abe4..6429b283 100644 --- a/src/servers/api/chat/websocket.ts +++ b/src/servers/api/chat/websocket.ts @@ -158,10 +158,8 @@ export function close(ws: ServerWebSocket): void { const sessionId = wsToSessionMap.get(ws); if (sessionId) { - sessionManager.detachWs(sessionId, ws); - // Only once nothing is watching. Another pane or another device still attached means the - // conversation is live, and arming the idle GC here would collect it out from under them. - if (!sessionManager.hasSockets(sessionId)) sessionManager.setIdleTimeout(sessionId, IDLE_TIMEOUT_MS); + sessionManager.detachWs(sessionId); + sessionManager.setIdleTimeout(sessionId, IDLE_TIMEOUT_MS); } } @@ -246,7 +244,7 @@ function createMessageHandler(sessionId: string, model: string) { const session = sessionManager.getSession(sessionId); if (!session) return; foldIntoSession(session, msg, model); - for (const socket of session.sockets) sendToClient(socket as ServerWebSocket, msg, seq); + sendToClient(session.ws as ServerWebSocket | null, msg, seq); }; } @@ -653,7 +651,7 @@ async function handleResumeCursor( // the Claude sidecar about an OpenCode session, hear "not generating", and write "the agent went // away" into a turn that was running perfectly well. if (msg.generating && decision.kind !== 'assume') { - await endTurnIfAgentIsGone([ws], sessionId, decision.model); + await endTurnIfAgentIsGone(ws, sessionId, decision.model); } } @@ -823,7 +821,7 @@ async function handleAttach(ws: ServerWebSocket, msg: { claudeSessionId: * so its sessions are left alone rather than guessed at. */ async function endTurnIfAgentIsGone( - targets: Iterable | null>, + ws: ServerWebSocket | null, sessionId: string, model: string, ): Promise { @@ -836,11 +834,11 @@ async function endTurnIfAgentIsGone( const event: ServerMessage = { type: 'cut-off' }; try { const seq = await appendChatEvent(sessionId, event); - for (const target of targets) sendToClient(target, event, seq); + sendToClient(ws, event, seq); } catch (err) { - // Still tell every client — an un-replayable explanation beats a spinner that never stops. + // 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) }); - for (const target of targets) sendToClient(target, event); + sendToClient(ws, event); } logger.info('Ended a turn whose agent had gone', { sessionId }); } @@ -852,11 +850,7 @@ async function endTurnIfAgentIsGone( sidecar.onClaudeSidecarStarted(() => { for (const session of sessionManager.getAllSessions()) { if (!session.isGenerating) continue; - void endTurnIfAgentIsGone( - session.sockets as Set>, - session.sessionId, - session.model, - ); + void endTurnIfAgentIsGone(session.ws as ServerWebSocket | null, session.sessionId, session.model); } }); diff --git a/src/workspaces/hooks/src/useChatWebSocket.ts b/src/workspaces/hooks/src/useChatWebSocket.ts index ccbd1bc1..776da795 100644 --- a/src/workspaces/hooks/src/useChatWebSocket.ts +++ b/src/workspaces/hooks/src/useChatWebSocket.ts @@ -20,22 +20,6 @@ export const useChatWebSocket = ({ url, onMessage, onOpen }: UseChatWebSocketPar const onOpenRef = useRef(onOpen); onOpenRef.current = onOpen; - /** - * Messages typed before the socket was ready. - * - * `send` used to drop them: `readyState !== OPEN` returned, silently, with no error and no retry — so - * pressing enter did nothing and the turn never happened. That window is not rare. React's dev - * StrictMode double-invokes effects, so every socket is created, closed and recreated on mount, and a - * reconnect after a drop reopens it again; with several chat panes on screen there are several sockets - * doing this at once. One of them is always briefly not OPEN. - * - * Queued and flushed on open, in order. The mobile chat app does exactly this and for exactly this - * reason — the composer is allowed to fire before the transport is ready. - */ - const pendingRef = useRef([]); - /** A deferred teardown, cancelled when the effect re-runs — see the cleanup below. */ - const closeTimerRef = useRef(null); - const connect = () => { if (isCleaningUpRef.current) return; if (socketRef.current && socketRef.current.readyState === WebSocket.OPEN) return; @@ -43,30 +27,15 @@ export const useChatWebSocket = ({ url, onMessage, onOpen }: UseChatWebSocketPar const socket = new WebSocket(url); socketRef.current = socket; - // Temporary, and deliberately loud. A pane connected and then sat silent, and reasoning from this - // hook's source three times running did not explain it — the console says a socket closed, never who - // closed it or whether the message went out. `window.__officerWs = false` turns it off. - const host = new URL(url).host; - const log = (what: string, extra?: unknown) => - (window as any).__officerWs !== false && console.log(`[ws ${host}] ${what}`, extra ?? ''); - log('creating'); - socket.addEventListener('open', () => { if (socketRef.current !== socket) return; setIsConnected(true); retryRef.current = 0; - log('OPEN'); - // BEFORE onOpen, deliberately: onOpen sends the resume/attach handshake, and anything the user - // typed while connecting belongs after that, not in front of it. - const queued = pendingRef.current; - pendingRef.current = []; onOpenRef.current?.(); - for (const message of queued) socket.send(message); }); socket.addEventListener('message', (ev) => { try { - log('recv', String(ev.data).slice(0, 120)); const data = JSON.parse(typeof ev.data === 'string' ? ev.data : new TextDecoder().decode(ev.data)); onMessageRef.current(data); } catch { @@ -74,13 +43,7 @@ export const useChatWebSocket = ({ url, onMessage, onOpen }: UseChatWebSocketPar } }); - socket.addEventListener('close', (ev) => { - log('close', { - code: ev.code, - reason: ev.reason, - stale: socketRef.current !== socket, - tearingDown: isCleaningUpRef.current, - }); + socket.addEventListener('close', () => { if (isCleaningUpRef.current) return; if (socketRef.current !== socket) return; setIsConnected(false); @@ -96,60 +59,25 @@ export const useChatWebSocket = ({ url, onMessage, onOpen }: UseChatWebSocketPar }; useEffect(() => { - // A pending teardown from a remount that is about to be undone — see below. - if (closeTimerRef.current !== null) { - clearTimeout(closeTimerRef.current); - closeTimerRef.current = null; - } isCleaningUpRef.current = false; connect(); - return () => { - /** - * Close LATER, not now. - * - * Closing here directly is correct for a real unmount and disastrous for a remount, and this hook - * cannot tell them apart at the moment it runs. React's dev StrictMode double-invokes every effect - * (mount → unmount → mount), and a pane whose subtree is re-created — a tab re-render, a resolved - * transcript, a parent key change — does the same. Each time, the socket was closed while still - * CONNECTING, the browser logged "closed before the connection is established", and the replacement - * was closed in turn. A pane could churn forever and never hold a connection: exactly what a fresh - * remote pane did. - * - * Deferring by a tick makes the two distinguishable. A remount re-runs the effect immediately and - * cancels this timer, so the live socket is kept and the handshake completes. A real unmount has - * nobody to cancel it and the socket closes a frame later, which costs nothing. - */ isCleaningUpRef.current = true; if (retryTimeoutRef.current !== null) { clearTimeout(retryTimeoutRef.current); retryTimeoutRef.current = null; } - const socket = socketRef.current; - closeTimerRef.current = window.setTimeout(() => { - closeTimerRef.current = null; - if (!isCleaningUpRef.current) return; // remounted: the effect above already reclaimed it - if (socket) socket.close(); - if (socketRef.current === socket) socketRef.current = null; - }, 0); + if (socketRef.current) { + socketRef.current.close(); + socketRef.current = null; + } }; }, [url]); const send = (data: Record) => { const socket = socketRef.current; - const message = JSON.stringify(data); - if (socket && socket.readyState === WebSocket.OPEN) { - if ((window as any).__officerWs !== false) console.log(`[ws ${new URL(url).host}] send`, message.slice(0, 120)); - socket.send(message); - return; - } - if ((window as any).__officerWs !== false) - console.log(`[ws ${new URL(url).host}] QUEUED (socket ${socket?.readyState ?? 'none'})`, message.slice(0, 80)); - // Not open yet, or reconnecting. Hold it rather than dropping it — see `pendingRef`. Bounded so a - // socket that never comes back cannot grow this without limit; the oldest go first, because the - // newest message is the one the user is still waiting on. - pendingRef.current.push(message); - if (pendingRef.current.length > 50) pendingRef.current.shift(); + if (!socket || socket.readyState !== WebSocket.OPEN) return; + socket.send(JSON.stringify(data)); }; return { isConnected, send }; diff --git a/src/workspaces/officerdev/src/MusicPlayer/MusicPlayerHost.tsx b/src/workspaces/officerdev/src/MusicPlayer/MusicPlayerHost.tsx index 0c6a14cf..5f042c9e 100644 --- a/src/workspaces/officerdev/src/MusicPlayer/MusicPlayerHost.tsx +++ b/src/workspaces/officerdev/src/MusicPlayer/MusicPlayerHost.tsx @@ -119,16 +119,7 @@ export const MusicPlayerHost = () => { // Restore the saved "currently playing" on first load — paused, at its position — so a reload/return // lands back on the track. Skipped when a queue already exists (an in-app nav kept player state). - // - // DISABLED on the web (Andre, 2026-08-10). The music sidecar is not running on every machine that - // serves this app, so every page load fired `/music/now-playing` and logged a 503 in the console of a - // browser that was not there for music at all. Restoring a paused track is a nicety; a permanent error - // on every load of every screen is not worth it. The player itself is untouched — play something and - // it works; it simply no longer asks what WAS playing. - const RESTORE_NOW_PLAYING = false; - useEffect(() => { - if (!RESTORE_NOW_PLAYING) return; if (restoredRef.current) return; restoredRef.current = true; if (queue.length) return; diff --git a/src/workspaces/officerdev/src/apps/ChatHistory/ChatDetailPanel.tsx b/src/workspaces/officerdev/src/apps/ChatHistory/ChatDetailPanel.tsx index 20d349f0..704901bb 100644 --- a/src/workspaces/officerdev/src/apps/ChatHistory/ChatDetailPanel.tsx +++ b/src/workspaces/officerdev/src/apps/ChatHistory/ChatDetailPanel.tsx @@ -3,7 +3,7 @@ import { useLocation, useParams } from 'react-router'; import { Unplug } from 'lucide-react'; import { toast } from '@/components/ui/sonner'; import { EditableTitle } from '@/components/EditableTitle'; -import { usePaneSelection } from './PaneSelection'; +import { useSelectedChatSession } from '../../channels'; import { usePublishPageTitle } from '../../page-title'; import { useAuth } from 'hooks/useAuth'; import { errorText } from 'helpers/error-text'; @@ -226,7 +226,7 @@ function NewChat(props: NewChatProps) { } export const ChatDetailPanel = () => { - const [selected] = usePaneSelection(); + const [selected] = useSelectedChatSession(); // Name the page after the conversation, whenever the URL names a real one. Gated on the route param // rather than on `selected`, so `/chat` and `/chat/new` keep the plain "Chat" — the panel holds a diff --git a/src/workspaces/officerdev/src/apps/ChatHistory/ChatPane.tsx b/src/workspaces/officerdev/src/apps/ChatHistory/ChatPane.tsx deleted file mode 100644 index 24d3c58e..00000000 --- a/src/workspaces/officerdev/src/apps/ChatHistory/ChatPane.tsx +++ /dev/null @@ -1,123 +0,0 @@ -import { useEffect, useRef } from 'react'; -import { ArrowLeft, Loader2 } from 'lucide-react'; -import { serverClient } from 'hooks/useServerClient'; -import { connectionLabel } from 'hooks/connections'; -import { SessionList } from './SessionList'; -import { ChatDetailPanel } from './ChatDetailPanel'; -import type { SelectedSession } from './ChatDetailPanel'; -import { PaneSelectionProvider } from './PaneSelection'; - -/** - * One self-contained conversation column: its own server, its own list, its own chat. - * - * Modelled on the mobile app's pane, where "a pane is just a whole ChatScreen" — an empty one IS the - * conversation list, and filling it is tapping a row. That is what makes two panes independent without - * inventing a second concept: everything a conversation needs is already inside one. - * - * The web version differs in one way, deliberately. Mobile has room for a list and a chat side by side - * inside a pane; two or three of those in a browser column would leave nothing for the conversation. So - * a pane shows its LIST until something is open and the CHAT afterwards, with one way back. The tab bar - * above holds the panes; this holds one conversation. - */ -type ChatPaneProps = { - target: SelectedSession | null; - onTargetChange: (next: SelectedSession | null) => void; - /** Shown when more than one pane is open, so it is obvious which machine a column is on. */ - showServerBadge?: boolean; -}; - -export const ChatPane = ({ target, onTargetChange, showServerBadge }: ChatPaneProps) => { - const open = !!target; - - /** - * Load the transcript for a row this pane just opened. - * - * The screen-level resolver does this for the single-panel layout, but it writes to the shared - * channel — which a pane deliberately does not read. So a pane clicked a row, got `{id, title, cwd}` - * and nothing else, and rendered an empty conversation while the list behind it reset to the default - * directory. It has to resolve its own, from ITS server: two machines can hold the same uuid, so - * asking the wrong one is not merely empty, it is wrong. - */ - const resolvingRef = useRef(null); - const id = target?.id ?? null; - const needsTranscript = !!id && !id.startsWith('new:') && !target?.resumeSessionId; - const serverId = target?.serverId ?? null; - - useEffect(() => { - if (!needsTranscript || !id) return; - if (resolvingRef.current === id) return; // one fetch per row, not one per render - resolvingRef.current = id; - let cancelled = false; - (async () => { - try { - const detail = await serverClient(serverId).get<{ - model?: string | null; - messages: unknown[]; - total: number; - offset: number; - cwd: string; - title?: string | null; - partCount?: number; - }>(`/chat/sessions/${id}?limit=20`); - if (cancelled) return; - onTargetChange({ - id, - serverId, - model: detail.model, - resumeSessionId: id, - initialMessages: detail.messages as never, - total: detail.total, - initialOffset: detail.offset, - cwd: detail.cwd, - title: detail.title ?? undefined, - partCount: detail.partCount, - }); - } catch { - // Leave the pane on the row it has. Falling back to an empty chat would look like a conversation - // that lost its history rather than one that could not be read. - if (!cancelled) resolvingRef.current = null; - } - })(); - return () => { - cancelled = true; - }; - }, [needsTranscript, id, serverId, onTargetChange]); - - return ( - -
- {open && ( -
- - {showServerBadge && ( - // Which machine this column is talking to. Only worth the space when there is more than - // one pane — with a single column the chips in the list already say it. - - {connectionLabel(target?.serverId)} - - )} -
- )} - -
- {!open ? ( - - ) : needsTranscript ? ( -
- -
- ) : ( - - )} -
-
- - ); -}; diff --git a/src/workspaces/officerdev/src/apps/ChatHistory/ChatTabs.tsx b/src/workspaces/officerdev/src/apps/ChatHistory/ChatTabs.tsx deleted file mode 100644 index 2dbefff3..00000000 --- a/src/workspaces/officerdev/src/apps/ChatHistory/ChatTabs.tsx +++ /dev/null @@ -1,268 +0,0 @@ -import { useCallback, useEffect, useRef, useState } from 'react'; -import { Columns2, Plus, X } from 'lucide-react'; -import { connectionLabel } from 'hooks/connections'; -import { usePublishChatTabName } from '../../page-title'; -import { ChatPane } from './ChatPane'; -import type { SelectedSession } from './ChatDetailPanel'; - -/** - * Tabs of side-by-side conversations, each pane free to sit on a different Officer. - * - * This is the iPad layout brought to the browser: one window, one `/chat`, several live conversations - * on several machines at once. The mobile app proved the shape — what it adds over the old single-panel - * screen is that a pane owns its conversation (see `PaneSelection`) instead of the whole screen sharing - * one. - * - * ## What is stored where, which is the part that matters - * - * The layout — which tabs exist, which panes they hold, and what each pane has open — is kept in ONE - * unscoped `localStorage` entry, deliberately not per server. A tab holding one conversation from the - * laptop and one from alpha belongs to neither, so scoping it to either would be wrong. The mobile app - * makes the same call and says so. - * - * A pane's `target` carries its own `serverId`, so a restored tab reopens the right conversation on the - * right machine rather than looking it up on whichever server happens to be nearest. - * - * ## The URL - * - * `/chat/` still deep-links, and still opens in the FIRST pane. It cannot mean more than that: with - * three conversations on screen there is no single "the" conversation for the address bar to name, which - * is the one place this design gives something up. Everything else about the route conventions holds. - */ - -type Pane = { key: string; target: SelectedSession | null }; -type Tab = { key: string; title?: string; panes: Pane[] }; - -const STORE_KEY = 'officer.chat.tabs.v1'; -const MAX_PANES = 3; - -let seq = 0; -const nextKey = (prefix: string) => `${prefix}-${Date.now().toString(36)}-${seq++}`; - -function load(): Tab[] { - try { - const raw = localStorage.getItem(STORE_KEY); - const parsed = raw ? (JSON.parse(raw) as Tab[]) : null; - if (!Array.isArray(parsed) || !parsed.length) throw new Error('empty'); - // Keys were minted by a previous page whose counter restarted at zero. Re-mint them, or React can - // reuse the wrong subtree and a conversation appears in the wrong column — the mobile app hit - // exactly this and guards it the same way. - return parsed.map((tab) => ({ - ...tab, - key: nextKey('tab'), - panes: (tab.panes ?? []).slice(0, MAX_PANES).map((pane) => ({ ...pane, key: nextKey('pane') })), - })); - } catch { - return [{ key: nextKey('tab'), panes: [{ key: nextKey('pane'), target: null }] }]; - } -} - -export const ChatTabs = () => { - const [tabs, setTabs] = useState(load); - const [activeKey, setActiveKey] = useState(() => ''); - const [renamingKey, setRenamingKey] = useState(null); - const [renameValue, setRenameValue] = useState(''); - const restored = useRef(false); - // Read inside the stable callback above, so it never has to be a dependency. - const activeKeyRef = useRef(activeKey); - activeKeyRef.current = activeKey; - - // First render picks the first tab; afterwards the user owns it. - useEffect(() => { - if (restored.current) return; - restored.current = true; - setActiveKey(tabs[0]?.key ?? ''); - }, [tabs]); - - useEffect(() => { - try { - localStorage.setItem(STORE_KEY, JSON.stringify(tabs)); - } catch { - /* private mode or quota — the layout still works for this page's lifetime */ - } - }, [tabs]); - - const active = tabs.find((tab) => tab.key === activeKey) ?? tabs[0]; - - // Only a name YOU typed is published — a label derived from the conversation would just restate the - // title the chat already publishes, one tier lower, and would then outrank a browser-tab name for no - // reason the user could see. - usePublishChatTabName(active?.title?.trim() || null); - - const update = (tabKey: string, fn: (tab: Tab) => Tab) => - setTabs((prev) => prev.map((tab) => (tab.key === tabKey ? fn(tab) : tab))); - - const addTab = () => { - const tab: Tab = { key: nextKey('tab'), panes: [{ key: nextKey('pane'), target: null }] }; - setTabs((prev) => [...prev, tab]); - setActiveKey(tab.key); - }; - - const closeTab = (tabKey: string) => { - setTabs((prev) => { - const next = prev.filter((tab) => tab.key !== tabKey); - // Never leave nothing: an empty tab bar has no way back to a conversation. - const safe = next.length ? next : [{ key: nextKey('tab'), panes: [{ key: nextKey('pane'), target: null }] }]; - if (tabKey === activeKey) setActiveKey(safe[0]!.key); - return safe; - }); - }; - - const startRename = (tab: Tab) => { - setRenamingKey(tab.key); - // Seeded with the typed name only, not the derived label: pre-filling a name the user never chose - // makes Enter silently adopt it as if they had. - setRenameValue(tab.title ?? ''); - }; - - const commitRename = () => { - if (!renamingKey) return; - const next = renameValue.trim(); - // Empty hands the tab back to its derived name — the only way out, and no third state. - update(renamingKey, (tab) => ({ ...tab, title: next || undefined })); - setRenamingKey(null); - }; - - const splitPane = () => - active && - update(active.key, (tab) => - tab.panes.length >= MAX_PANES ? tab : { ...tab, panes: [...tab.panes, { key: nextKey('pane'), target: null }] }, - ); - - const closePane = (paneKey: string) => - active && - update(active.key, (tab) => - tab.panes.length <= 1 ? tab : { ...tab, panes: tab.panes.filter((pane) => pane.key !== paneKey) }, - ); - - // Stable across renders on purpose. It is handed to every pane as `onChange`, and a pane passes it - // into a context that other components read — an identity that changed every render would make any - // effect depending on it re-run forever, which is the render loop this file already caused once. - const setPaneTarget = useCallback( - (paneKey: string, target: SelectedSession | null) => - setTabs((prev) => - prev.map((tab) => - tab.key !== activeKeyRef.current - ? tab - : { ...tab, panes: tab.panes.map((pane) => (pane.key === paneKey ? { ...pane, target } : pane)) }, - ), - ), - [], - ); - - if (!active) return null; - - return ( -
- {/* Always visible: it is the only way to open a second tab or split a pane, so hiding it in the - single-conversation case would hide the feature from anyone who has not already used it. */} - { -
- {tabs.map((tab) => { - // A tab is named after what is in it: the first pane's conversation, else the machine. - const first = tab.panes[0]?.target; - const label = - tab.title || - first?.title || - (tab.panes.length > 1 ? `${tab.panes.length} panes` : connectionLabel(first?.serverId, 'Chat')); - if (renamingKey === tab.key) { - return ( - setRenameValue(ev.target.value)} - onKeyDown={(ev) => { - if (ev.key === 'Enter') commitRename(); - if (ev.key === 'Escape') setRenamingKey(null); - }} - onBlur={commitRename} - aria-label="Tab name" - placeholder={label} - className="w-32 shrink-0 rounded-t border-b border-primary/40 bg-muted px-2 py-1 text-xs outline-none" - /> - ); - } - - return ( - - ); - })} - - - - -
- } - -
- {active.panes.map((pane, index) => ( -
0 ? 'border-l border-border' : ''}`} - style={{ width: `${100 / active.panes.length}%` }} - > - {active.panes.length > 1 && ( - - )} - setPaneTarget(pane.key, next)} - showServerBadge={active.panes.length > 1} - /> -
- ))} -
-
- ); -}; diff --git a/src/workspaces/officerdev/src/apps/ChatHistory/DirPickerModal.tsx b/src/workspaces/officerdev/src/apps/ChatHistory/DirPickerModal.tsx index 37432966..01b92e60 100644 --- a/src/workspaces/officerdev/src/apps/ChatHistory/DirPickerModal.tsx +++ b/src/workspaces/officerdev/src/apps/ChatHistory/DirPickerModal.tsx @@ -8,14 +8,12 @@ type DirPickerModalProps = { open: boolean; onClose: () => void; onSelect: (absPath: string) => void; - /** Whose filesystem to browse. Absent = this origin. */ - serverId?: string | null; }; // A simplified file-browser modal for picking a working directory (returns an absolute path). // Navigates within the home root; dirs elsewhere are reachable via the selector's free-text field. -export const DirPickerModal = ({ open, onClose, onSelect, serverId }: DirPickerModalProps) => { - const api = useFilesAPI('home', serverId); +export const DirPickerModal = ({ open, onClose, onSelect }: DirPickerModalProps) => { + const api = useFilesAPI('home'); const [path, setPath] = useState('/'); // root-relative, always starts with '/' const [creating, setCreating] = useState(false); const [newName, setNewName] = useState(''); @@ -25,9 +23,7 @@ export const DirPickerModal = ({ open, onClose, onSelect, serverId }: DirPickerM const [showHidden, setShowHidden] = useState(false); const { data, isLoading, refetch } = useQuery({ - // Server in the key: two machines have different trees, and without it one machine's folders - // are served from cache under the other's name. - queryKey: ['dir-picker', path, serverId ?? null], + queryKey: ['dir-picker', path], queryFn: () => api.listDir(path), enabled: open, }); diff --git a/src/workspaces/officerdev/src/apps/ChatHistory/PaneSelection.tsx b/src/workspaces/officerdev/src/apps/ChatHistory/PaneSelection.tsx deleted file mode 100644 index 739aae42..00000000 --- a/src/workspaces/officerdev/src/apps/ChatHistory/PaneSelection.tsx +++ /dev/null @@ -1,66 +0,0 @@ -import { createContext, useContext, useMemo, useState } from 'react'; -import type { ReactNode } from 'react'; -import { useSelectedChatSession } from '../../channels'; -import type { SelectedSession } from './ChatDetailPanel'; - -/** - * Which conversation THIS pane has open. - * - * `chat:selected-session` is one channel for the whole screen, which was right while there was exactly - * one conversation on it. Two panes side by side make it wrong: both would read the same value and show - * the same chat, which is the opposite of the point. - * - * So a pane provides its own state here, and `usePaneSelection` prefers it. Outside a pane the context - * is absent and the channel is used exactly as before — every existing caller (the mobile layout, the - * dashboard's own chat panel) is untouched, which is what makes this safe to drop in. - * - * Deliberately a context rather than props: `SessionList` and `ChatDetailPanel` sit at different depths - * and neither should have to know whether it is inside a pane. - */ -type PaneSelectionValue = [SelectedSession | null, (next: SelectedSession | null) => void]; - -const PaneSelectionContext = createContext(null); - -/** - * Is this component inside a pane? - * - * A pane owns its conversation AND its directory, so it must not take either from the address bar: - * three panes cannot share one URL. Outside a pane the route stays the authority, exactly as before. - */ -export function useIsInPane(): boolean { - return useContext(PaneSelectionContext) !== null; -} - -export function usePaneSelection(): PaneSelectionValue { - const scoped = useContext(PaneSelectionContext); - const channel = useSelectedChatSession(); - // Hooks must run unconditionally, so the channel is always read; the scoped value simply wins. - return scoped ?? ([channel[0], channel[1]] as PaneSelectionValue); -} - -/** - * Give the subtree its own selection. - * - * `value`/`onChange` make it controllable, so the tab shell can persist a pane's open conversation - * across a reload — the mobile app keeps the target on the pane for the same reason, and it is what - * makes a restored tab still point at the right chat on the right machine. - */ -export const PaneSelectionProvider = ({ - children, - value, - onChange, -}: { - children: ReactNode; - value?: SelectedSession | null; - onChange?: (next: SelectedSession | null) => void; -}) => { - const [internal, setInternal] = useState(null); - const controlled = value !== undefined && !!onChange; - - const pair = useMemo( - () => (controlled ? [value ?? null, onChange!] : [internal, setInternal]), - [controlled, value, onChange, internal], - ); - - return {children}; -}; diff --git a/src/workspaces/officerdev/src/apps/ChatHistory/PwdSelector.tsx b/src/workspaces/officerdev/src/apps/ChatHistory/PwdSelector.tsx index 6799cf22..4748faa0 100644 --- a/src/workspaces/officerdev/src/apps/ChatHistory/PwdSelector.tsx +++ b/src/workspaces/officerdev/src/apps/ChatHistory/PwdSelector.tsx @@ -6,16 +6,14 @@ import { DirPickerModal } from './DirPickerModal'; type PwdSelectorProps = { value: string | null; // null = the default general_chat_sessions dir onChange: (cwd: string | null) => void; - /** Which Officer's directories to offer. Absent = this origin. */ - serverId?: string | null; }; // A shorter, friendlier label for a working directory. const shorten = (cwd: string) => cwd.replace(/^\/home\/[^/]+/, '~'); const basename = (cwd: string) => cwd.split('/').filter(Boolean).pop() ?? cwd; -export const PwdSelector = ({ value, onChange, serverId }: PwdSelectorProps) => { - const { pwds, defaultCwd } = useChatPwds(serverId); +export const PwdSelector = ({ value, onChange }: PwdSelectorProps) => { + const { pwds, defaultCwd } = useChatPwds(); const [open, setOpen] = useState(false); const [browse, setBrowse] = useState(false); const [custom, setCustom] = useState(''); @@ -98,7 +96,7 @@ export const PwdSelector = ({ value, onChange, serverId }: PwdSelectorProps) => )} - setBrowse(false)} onSelect={(p) => pick(p)} serverId={serverId} /> + setBrowse(false)} onSelect={(p) => pick(p)} /> ); }; diff --git a/src/workspaces/officerdev/src/apps/ChatHistory/SessionList.tsx b/src/workspaces/officerdev/src/apps/ChatHistory/SessionList.tsx index aedcf245..0c1eb3f9 100644 --- a/src/workspaces/officerdev/src/apps/ChatHistory/SessionList.tsx +++ b/src/workspaces/officerdev/src/apps/ChatHistory/SessionList.tsx @@ -3,7 +3,7 @@ import { useNavigate, useParams } from 'react-router'; import { Plus, MessageSquare, RefreshCw, Trash2, Pencil, Check, X, Layers } from 'lucide-react'; import { toast } from '@/components/ui/sonner'; import { DataList, DataRow, EmptyBlock, ErrorBlock, LoadingBlock, MetaItem, RelativeTime } from '@/components/Data'; -import { usePaneSelection, useIsInPane } from './PaneSelection'; +import { useSelectedChatSession } from '../../channels'; import { errorText } from 'helpers/error-text'; import { useClaudeSessions } from 'state/useClaudeSessions'; import { ServerChips } from './ServerChips'; @@ -19,16 +19,11 @@ export const SessionList = () => { // them there rather than from the selection channel means the highlight and the group are correct on // a deep link and on back/forward, before any panel has published. const { sessionId, '*': splat } = useParams<{ sessionId: string; '*': string }>(); - // Scoped to this pane when inside one, the shared channel otherwise — see PaneSelection. - const [selected, setSelected] = usePaneSelection(); + const [selected, setSelected] = useSelectedChatSession(); // A group path when we're on one; otherwise the open session's own directory, so /chat/ shows // that session among its neighbours instead of snapping the list back to the default group. Null = // the default general_chat_sessions dir. - // In a pane the directory is the pane's, not the route's — three panes cannot share one URL, and - // letting the address bar win is what reset this list to the default the moment a row was clicked. - const inPane = useIsInPane(); - const [paneCwd, setPaneCwd] = useState(null); - const activeCwd = inPane ? paneCwd : (cwdFromSplat(splat) ?? selected?.cwd ?? null); + const activeCwd = cwdFromSplat(splat) ?? selected?.cwd ?? null; // Which Officer this list is reading. Panel-local state, NOT a global: another panel showing another // machine is the entire point, and a shared "current server" would make that impossible to express. @@ -92,20 +87,17 @@ export const SessionList = () => { value={serverId} onChange={(next) => { setServerId(next); - setPaneCwd(null); // A path and a conversation from the machine you left name nothing on the one you arrived // at — the mobile app clears cwd for exactly this reason (`useChatScreen.chooseServer`). setSelected(null); - if (!inPane) navigate(chatListPath(null), { replace: true }); + navigate(chatListPath(null), { replace: true }); }} /> { setSelected(null); // sessions belong to a cwd — clear the open one when switching - if (inPane) setPaneCwd(cwd); - else navigate(chatListPath(cwd), { replace: true }); + navigate(chatListPath(cwd), { replace: true }); }} />
@@ -212,7 +204,7 @@ export const SessionList = () => { {/* The row is the link and the actions are its siblings — a