diff --git a/docs/chat-ui-walkthrough.md b/docs/chat-ui-walkthrough.md index 54a34b2f..71369a50 100644 --- a/docs/chat-ui-walkthrough.md +++ b/docs/chat-ui-walkthrough.md @@ -212,6 +212,69 @@ be built, but it is its own piece of work. --- +## 13. The chat URL: `?cwd=` is gone, and a session decides its own directory + +**Where:** the address bar, everywhere in `/chat`. Also the **Run agent** dialog in the file browser +right-click menu. + +**What to do:** + +1. Open `/chat`, pick a project in the pwd dropdown. The URL is now `/chat/g/home/pastilhas/dockers/…` — + a readable path, not `?cwd=%2Fhome%2Fpastilhas%2F…`. +2. Click a conversation. The URL becomes just `/chat/` — **no group at all**. Copy it. +3. Paste it into a fresh tab. It should open the same conversation, with the list beside it scoped to + that conversation's project, and the pwd dropdown showing that project. +4. **The one that used to be broken:** in that fresh tab, send a message _immediately_, before anything + settles. It should run in the project directory, not in the general chat directory. +5. Right-click a folder in the file browser → **Run agent** → run one → **Open in chat**. It should land + on that folder's group list. +6. Mobile: open a conversation, hit back. You should return to the project's list, not to an empty + default one. + +**What changed, and why it is more than cosmetic.** + +The group used to be a query parameter, and the session's working directory was read _back off that +query parameter_ to decide where the agent actually executes. So the address bar was the authority on +where code runs. Two consequences, both of which you had noticed as "sometimes it doesn't load in the +right place": + +- A pasted or refreshed `/chat/` arrives with **no `?cwd=` at all**. The screen then fetched the + session and wrote the cwd into the URL — but there was a window before that landed, and a turn sent in + that window ran in the default `general_chat_sessions` directory instead of the project. +- Even after it landed, the URL was hand-editable, so a `?cwd=` naming one project and a session + belonging to another could disagree. Nothing reconciled them; the URL simply won. + +The fix is not really "path instead of query string" — that part is presentation. It is that **there is +now one source of truth for a session's directory: the session.** `loadClaudeSessionById` already scans +every project group and reads the real cwd out of the transcript, so the id alone determines it. The +directory now travels on the selection (`SelectedSession.cwd`), which is what the composer reads. The URL +no longer carries it for a session, so it cannot contradict it. + +A group, on the other hand, genuinely _is_ addressable state and belongs in the URL — so it is a path +suffix behind a `g/` discriminator: `/chat/g/home/me/project`, `/chat/new/g/home/me/project`. Spelled as +a path rather than a percent-encoded blob because Officer always sits behind a reverse proxy and `%2F` is +exactly the character proxies normalise or reject. + +The vocabulary lives in one file, `apps/ChatHistory/chat-routes.ts` — `chatListPath`, `chatNewPath`, +`chatSessionPath`, `cwdFromSplat` — so a link built in a panel and a link built in a screen cannot drift. + +**The agentic side, which you flagged.** `startAgentRun` used to return a literal +`chatUrl: '/chat?cwd=…'` — the server holding an opinion about frontend URL shape, and therefore holding +a copy that goes stale the moment that shape changes. It now returns just `cwd` (which it already did), +and `AgentRunnerModal` builds the link with `chatListPath`. One less place that knows what a chat URL +looks like. + +**Also fixed in passing:** the permalink written after a turn completes (`useChat`) used to carry the +whole query string forward, which re-attached a stale `?cwd=` to the new session's URL. It now strips +`cwd` and leaves everything else alone. + +**Not verified:** as with everything else here, none of this has been through a browser. The typecheck is +clean and the route ranking has been checked against React Router 7's scoring (a `/chat/g/*` pattern +scores 23 against `/chat/:sessionId`'s 17, so a group path can never be mistaken for a session id) — but +step 4 above is the one I most want you to actually try, because it is the bug this was for. + +--- + ## Things noticed and deliberately left alone - **`useChatWebSocket` silently ignores unparseable frames.** That one is intentional and the comment diff --git a/src/apps/officer-web/App.tsx b/src/apps/officer-web/App.tsx index 3e6d4a9c..8729a895 100644 --- a/src/apps/officer-web/App.tsx +++ b/src/apps/officer-web/App.tsx @@ -35,8 +35,14 @@ export function App() { } /> } /> } /> + {/* A project group is a working directory, so it is spelled as a path, not ?cwd= — see + apps/ChatHistory/chat-routes.ts. The splat has to be last in a pattern, which is why + "new" comes before the group rather than after it. A session carries no group: the + transcript records its own cwd and the server resolves it by id. */} } /> } /> + } /> + } /> } /> } /> } /> diff --git a/src/apps/officer-web/Screens/Dashboard/ChatHistory/index.tsx b/src/apps/officer-web/Screens/Dashboard/ChatHistory/index.tsx index 56681142..eac936fa 100644 --- a/src/apps/officer-web/Screens/Dashboard/ChatHistory/index.tsx +++ b/src/apps/officer-web/Screens/Dashboard/ChatHistory/index.tsx @@ -1,10 +1,11 @@ import { useEffect, useMemo, useRef } from 'react'; -import { useParams, useNavigate, useSearchParams } from 'react-router'; +import { useParams, useNavigate } from 'react-router'; import type { LayoutNode, SelectedSession } from 'officerdev'; -import { WorkspaceView } from 'officerdev'; +import { WorkspaceView, chatListPath, cwdFromSplat } from 'officerdev'; import { toast } from '@/components/ui/sonner'; import { useIsMobile } from 'hooks/useIsMobile'; import { useClient } from 'hooks/useClient'; +import { errorText } from 'helpers/error-text'; import { useDashboardState } from 'state/useDashboardState'; import type { ClaudeSessionDetail } from 'state/useClaudeSessions'; import { usePanelChannel } from 'hooks/usePanelChannel'; @@ -37,9 +38,10 @@ type SessionListPageProps = { }; export const SessionListPage = ({ isNew }: SessionListPageProps) => { - const { sessionId } = useParams<{ sessionId: string }>(); + // The group is a path suffix now, not ?cwd= — see officerdev/apps/ChatHistory/chat-routes.ts. + const { sessionId, '*': splat } = useParams<{ sessionId: string; '*': string }>(); + const groupCwd = cwdFromSplat(splat); const [selected, setSelected] = usePanelChannel('chat:selected-session', null); - const [, setSearchParams] = useSearchParams(); const client = useClient(); const selectedRef = useRef(selected); selectedRef.current = selected; @@ -69,7 +71,9 @@ export const SessionListPage = ({ isNew }: SessionListPageProps) => { // scrolling up lazy-loads older messages. `total`/`offset` tell the chat where the window sits. useEffect(() => { if (isNew) { - setSelected({ id: `new:${Date.now()}` }); + // A new chat has no transcript to read a cwd from, so the group in the URL is the authority — + // and it has to be on the selection, because that is what the composer runs in. + setSelected({ id: `new:${Date.now()}`, cwd: groupCwd }); return; } if (!sessionId) return; @@ -79,16 +83,9 @@ export const SessionListPage = ({ isNew }: SessionListPageProps) => { try { const detail = await client.get(`/chat/sessions/${sessionId}?limit=${CHAT_TAIL}`); if (cancelled) return; - // replace: resolving a deep link is a canonicalisation, not a navigation the back button owes you. - setSearchParams( - (prev) => { - const next = new URLSearchParams(prev); - if (detail.cwd) next.set('cwd', detail.cwd); - else next.delete('cwd'); - return next; - }, - { replace: true }, - ); + // The session's own cwd rides on the selection rather than being written back into the URL. + // It used to do both, and the URL copy was the one the composer read — so a deep link ran its + // first turn in the default dir during the window before this resolve landed. setSelected({ id: sessionId, model: detail.model, @@ -96,13 +93,14 @@ export const SessionListPage = ({ isNew }: SessionListPageProps) => { initialMessages: detail.messages as unknown as NonNullable['initialMessages'], total: detail.total, initialOffset: detail.offset, + cwd: detail.cwd, }); } catch (err) { if (cancelled) return; // Falling back to a bare id still opens a usable pane, but silently: you get an empty chat and // no hint that the transcript could not be read, which is indistinguishable from a new session. // Most often the id is stale — the transcript was deleted or pruned out from under the link. - toast.error(`Could not load this conversation: ${err instanceof Error ? err.message : 'not found'}`); + toast.error(`Could not load this conversation: ${errorText(err, 'not found')}`); setSelected({ id: sessionId }); } })(); @@ -110,7 +108,7 @@ export const SessionListPage = ({ isNew }: SessionListPageProps) => { cancelled = true; }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [sessionId, isNew]); + }, [sessionId, isNew, groupCwd]); return (
@@ -119,9 +117,10 @@ export const SessionListPage = ({ isNew }: SessionListPageProps) => { locked mobilePanelId={mobilePanelId} onMobilePanelChange={(id) => { - // Keep the query string: ?cwd= names the project group the list is showing, so dropping it on - // mobile back sends you to an empty general_chat_sessions instead of the list you came from. - if (!id) navigate({ pathname: '/chat', search: window.location.search }, { replace: true }); + // 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/servers/api/agents/agent-runner.ts b/src/servers/api/agents/agent-runner.ts index bad08f5f..24d94f0b 100644 --- a/src/servers/api/agents/agent-runner.ts +++ b/src/servers/api/agents/agent-runner.ts @@ -117,10 +117,14 @@ type StartAgentRunParams = { export type StartAgentRunResult = { sessionKey: string; dirName: string; + /** + * The directory the run executes in — which is also its project group in /chat, so the caller can + * build the link to it. This used to also return a ready-made `chatUrl`, which meant the server held + * an opinion about frontend URL shape and drifted the moment that shape changed. `cwd` is the fact; + * the URL is the frontend's business (`chatListPath`). + */ cwd: string; model: string; - /** Where to look at this run: the agent's own project group in /chat, newest session on top. */ - chatUrl: string; }; export async function startAgentRun(params: StartAgentRunParams): Promise { @@ -204,6 +208,5 @@ export async function startAgentRun(params: StartAgentRunParams): Promise 0 + /** + * The directory this session actually runs in, taken from its own transcript. Carried here rather + * than read back off the URL: the id already determines it, and when the URL was the authority a + * fresh deep-link had no cwd at all until the resolve landed — so a turn sent in that window ran in + * the default dir instead of the project. For a new chat it is the group the list is showing. + */ + cwd?: string | null; } | null; const CHANNEL = 'chat:selected-session'; @@ -71,9 +78,10 @@ type NewChatProps = { initialMessages?: ChatMessage[]; total?: number; initialOffset?: number; + sessionCwd?: string | null; }; -function NewChat({ resumeSummary, resumeSessionId, initialMessages, total, initialOffset }: NewChatProps) { +function NewChat({ resumeSummary, resumeSessionId, initialMessages, total, initialOffset, sessionCwd }: NewChatProps) { const location = useLocation(); const locationState = location.state as ChatLocationState; const { invalidate: invalidateClaudeSessions } = useClaudeSessions(); @@ -98,10 +106,12 @@ function NewChat({ resumeSummary, resumeSessionId, initialMessages, total, initi : undefined, }); - // Run the session in the pwd the URL names; absent → backend default (general_chat_sessions). - const [searchParams] = useSearchParams(); - const activeCwd = searchParams.get('cwd'); - const cwd = activeCwd ? { path: activeCwd } : locationState?.cwd; + // Run the session in ITS OWN directory. This used to read `?cwd=` straight off the URL, which made + // the address bar the authority on where an agent executes — so a pasted /chat/ ran its first + // turn in the default dir until the deep-link resolve caught up, and a hand-edited cwd could point a + // resumed session anywhere. The session's transcript is the only thing that knows, so it wins; + // absent (a genuinely new chat) → the group the list is on, else the backend default. + const cwd = sessionCwd ? { path: sessionCwd } : locationState?.cwd; const initialMessage = locationState?.initialMessage ? { @@ -152,6 +162,7 @@ export const ChatDetailPanel = () => { initialMessages={selected.initialMessages} total={selected.total} initialOffset={selected.initialOffset} + sessionCwd={selected.cwd} /> ); }; diff --git a/src/workspaces/officerdev/src/apps/ChatHistory/SessionList.tsx b/src/workspaces/officerdev/src/apps/ChatHistory/SessionList.tsx index b00f8ef8..be0164ed 100644 --- a/src/workspaces/officerdev/src/apps/ChatHistory/SessionList.tsx +++ b/src/workspaces/officerdev/src/apps/ChatHistory/SessionList.tsx @@ -1,5 +1,5 @@ import { useRef, useState, useCallback } from 'react'; -import { useNavigate, useParams, useSearchParams } from 'react-router'; +import { useNavigate, useParams } from 'react-router'; import { Plus, MessageSquare, RefreshCw, Trash2, Pencil, Check, X } from 'lucide-react'; import { toast } from '@/components/ui/sonner'; import { DataList, DataRow, EmptyBlock, ErrorBlock, LoadingBlock, RelativeTime } from '@/components/Data'; @@ -7,23 +7,23 @@ import { usePanelChannel } from 'hooks/usePanelChannel'; import { errorText } from 'helpers/error-text'; import { useClaudeSessions } from 'state/useClaudeSessions'; import type { SelectedSession } from './ChatDetailPanel'; +import { chatListPath, chatNewPath, chatSessionPath, cwdFromSplat } from './chat-routes'; import { PwdSelector } from './PwdSelector'; // Reads the /chat conversation list from Claude's own transcript store (source of truth). // Clicking a session loads its transcript and continues the real Claude session via --resume. export const SessionList = () => { const navigate = useNavigate(); - // The working directory the list operates on (absent = the default general_chat_sessions dir). - // In the URL, not a channel: which project group you're looking at is addressable state, so - // /chat?cwd= has to be a link anyone can hand out — an agent run points at its own group - // this way, and it survives a refresh. - const [searchParams, setSearchParams] = useSearchParams(); - const activeCwd = searchParams.get('cwd'); - // Which session is open is the URL too. Reading it from the route rather than the selection channel - // means the highlight is correct on a deep link and on a back/forward, before any panel has published. - const { sessionId } = useParams<{ sessionId: string }>(); - const { sessions, isLoading, error, refetch, deleteSession, renameSession } = useClaudeSessions(activeCwd); + // Which session is open, and which project group we're browsing, both come from the route. Reading + // 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 }>(); const [selected, setSelected] = usePanelChannel('chat:selected-session', null); + // 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. + const activeCwd = cwdFromSplat(splat) ?? selected?.cwd ?? null; + const { sessions, isLoading, error, refetch, deleteSession, renameSession } = useClaudeSessions(activeCwd); const [editingId, setEditingId] = useState(null); const [editValue, setEditValue] = useState(''); const [confirmingId, setConfirmingId] = useState(null); @@ -36,8 +36,9 @@ export const SessionList = () => { } }, []); - const search = searchParams.toString(); - const linkTo = (id: string) => (search ? `/chat/${id}?${search}` : `/chat/${id}`); + // No group in a session link, deliberately: the id already determines the directory. It used to + // carry `?cwd=`, which is how a link could name a group the session doesn't belong to. + const linkTo = chatSessionPath; const startRename = (id: string, current: string) => { setConfirmingId(null); @@ -65,7 +66,9 @@ export const SessionList = () => { try { await deleteSession(id); if (selected?.id === id) setSelected(null); - if (sessionId === id) navigate({ pathname: '/chat', search }, { replace: true }); + // Back to the group's list, not the default one — deleting the open session shouldn't also move + // you out of the project you were working in. + if (sessionId === id) navigate(chatListPath(activeCwd), { replace: true }); } catch (err) { toast.error(`Could not delete the session: ${errorText(err)}`); } @@ -77,16 +80,8 @@ export const SessionList = () => { { - setSearchParams( - (prev) => { - const next = new URLSearchParams(prev); - if (cwd) next.set('cwd', cwd); - else next.delete('cwd'); - return next; - }, - { replace: true }, - ); setSelected(null); // sessions belong to a cwd — clear the open one when switching + navigate(chatListPath(cwd), { replace: true }); }} />
@@ -100,9 +95,10 @@ export const SessionList = () => {