put the previewed browser tab in the url

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-07 11:56:45 +00:00
co-authored by Claude Opus 5
parent 6c47cbeb74
commit 374140d3a6
7 changed files with 124 additions and 61 deletions
+12 -3
View File
@@ -39,7 +39,8 @@ That fix is the template for the HIGH items below. **Caveat:** the fix only did
was in this list; the Projects feature was removed end to end on 2026-07-30, so the route is gone with it. H3 was in this list; the Projects feature was removed end to end on 2026-07-30, so the route is gone with it. H3
and navigate-site 22 below still name it — they are the record of work that happened, not of code that exists.) and navigate-site 22 below still name it — they are the record of work that happened, not of code that exists.)
**Flat screens:** `/` `/files` `/music` `/tasks` `/skills` `/processes` `/activity` **Flat screens:** `/` `/files` `/music` `/tasks` `/skills` `/processes` `/activity`
`/system-monitor` `/task-logs` `/plans` `/terminal` `/desktop` `/browser` `/code-editor`. `/system-monitor` `/task-logs` `/plans` `/terminal` `/desktop` `/code-editor`. (`/browser` is a pair now too —
`/browser/:tabId`.)
The five settings pages are route **pairs** now, not flat screens — `/settings/{profile,ai,system,integrations,user-management}` The five settings pages are route **pairs** now, not flat screens — `/settings/{profile,ai,system,integrations,user-management}`
plus a `:section` each (M6). There has never been a `/settings/apps`; that entry was wrong when this list plus a `:section` each (M6). There has never been a `/settings/apps`; that entry was wrong when this list
was written. was written.
@@ -101,7 +102,15 @@ is **one design decision** that cascades across many files:
- **Dock / Header active styling** (`Dock.tsx:43,78` · `Header.tsx:53,112`) — destinations are already real - **Dock / Header active styling** (`Dock.tsx:43,78` · `Header.tsx:53,112`) — destinations are already real
`<Link>`s; only the *active* class is JS-derived from `useLocation`. Switch to react-router `<NavLink>` and drop the hand-rolled `isActive`. `<Link>`s; only the *active* class is JS-derived from `useLocation`. Switch to react-router `<NavLink>` and drop the hand-rolled `isActive`.
- **Browser tabs** (`Browser/TabList.tsx:93`) — ephemeral CDP targets (polled, come/go every few seconds); a durable `/browser/:targetId` is dubious. Probably leave as selection. - ~~**Browser tabs**~~ (`Browser/TabList.tsx:93`) — **done, against this file's own advice.** The objection
was that a CDP target id is ephemeral, so a durable `/browser/:tabId` is dubious. True of *bookmarking*,
and irrelevant to everything else the URL buys: the id was in an onClick closure, three components read a
`BROWSER_SELECTED_TAB` global, and the row could not be cmd-clicked. Staleness is handled where it
actually shows up — the preview now distinguishes "no tab open" from "that tab is no longer attached"
by checking the polled target list, which it gets from the same React Query key the list uses, so it
costs no extra request. En route: the row's Focus and Close buttons were nested *inside* the row
`<button>`, which is invalid HTML and only worked because of two `stopPropagation` calls; they are
siblings of the anchor now. And its "Set up in Integrations" was a raw `<a href>` that reloaded the SPA.
- **Jobs step/iteration** (`Jobs/JobDetail.tsx:234,255`) — intra-detail sub-selection; only if we want `/jobs/:id/:stepKey` deep links. - **Jobs step/iteration** (`Jobs/JobDetail.tsx:234,255`) — intra-detail sub-selection; only if we want `/jobs/:id/:stepKey` deep links.
- **Preview slug** (`Preview/PreviewApp.tsx:23`) — Preview usually derives its target from the surrounding dashboard/project `cwd`; the manual picker is a fallback. Probably leave URL-less. - **Preview slug** (`Preview/PreviewApp.tsx:23`) — Preview usually derives its target from the surrounding dashboard/project `cwd`; the manual picker is a fallback. Probably leave URL-less.
- **FileBrowser widget** (`FileBrowserWidget/.../BrowseTab.tsx:36`) — compact sidebar widget; keeping its browse cursor local is defensible. - **FileBrowser widget** (`FileBrowserWidget/.../BrowseTab.tsx:36`) — compact sidebar widget; keeping its browse cursor local is defensible.
@@ -209,7 +218,7 @@ publishers means changing the chat panel, which is another agent's, so it is wri
- [ ] Dock + Header + mobile sheet → react-router `<NavLink>` for active state; drop hand-rolled `isActive` (`Dock.tsx`, `Header.tsx`). - [ ] Dock + Header + mobile sheet → react-router `<NavLink>` for active state; drop hand-rolled `isActive` (`Dock.tsx`, `Header.tsx`).
- [ ] "New Chat" → `<Link to="/chat/new">` (`SessionList.tsx:68`) once H4's channel cleanup lands. - [ ] "New Chat" → `<Link to="/chat/new">` (`SessionList.tsx:68`) once H4's channel cleanup lands.
- [ ] Jobs back button + any `navigate('/jobs')` → shared `BackButton` (`JobDetail.tsx:709`). - [ ] Jobs back button + any `navigate('/jobs')` → shared `BackButton` (`JobDetail.tsx:709`).
- [ ] Decide/skip: Browser tabs, Jobs step deep-link, Preview slug, FileBrowser widget, Music-favorites/Monitor-scope view toggles. - [ ] Decide/skip: Jobs step deep-link, Preview slug, FileBrowser widget, Music-favorites view toggle. (Browser tabs: **done** — see the LOW section. Monitor scope: **done** as M10, it was not a view toggle.)
### Cross-cutting for the refactor itself ### Cross-cutting for the refactor itself
- [ ] Standardise a URL-as-source-of-truth pattern for panel selection (replace the `usePanelChannel`/`useGlobal` - [ ] Standardise a URL-as-source-of-truth pattern for panel selection (replace the `usePanelChannel`/`useGlobal`
+12 -1
View File
@@ -667,7 +667,18 @@ All the same bug: an app guessing "am I being closed?" from an unmount, or payin
walk the sweep instead of leaving the mailbox. `EMAIL_FOLDER` stays a global for now: it is read in walk the sweep instead of leaving the mailbox. `EMAIL_FOLDER` stays a global for now: it is read in
one component and is view state rather than selection. Putting it in `?folder=` is a real but one component and is view state rather than selection. Putting it in `?folder=` is a real but
separate item. separate item.
Remaining here: Music, Browser. **Chat detail (`chat:selected-session`) is chat-owned — **Browser is done** — `/browser/:tabId`, `BROWSER_SELECTED_TAB` deleted from its three readers.
The audit had this one filed under "probably leave as selection", on the grounds that a CDP target
id dies with the browser session so a durable URL is dubious. That is an argument about
*bookmarking*, and bookmarking is the least of what the URL was buying: the id lived in an onClick
closure, three components shared a global, and a tab row could not be cmd-clicked. The staleness
the audit worried about is real and is now handled where it shows — the preview tells "no tab open"
from "that tab is no longer attached" by checking the polled target list, which both panels read
from one React Query key, so it costs no extra request. Two defects fell out on the way: the row's
Focus and Close buttons were nested inside the row `<button>` (invalid HTML, held together by two
`stopPropagation` calls — they are siblings of the anchor now), and "Set up in Integrations" was a
raw `<a href>` that reloaded the whole SPA.
Remaining here: Music. **Chat detail (`chat:selected-session`) is chat-owned —
leave it; it is written up in `COMMS/`.** leave it; it is written up in `COMMS/`.**
### 5.9 The context has grown an app-config section — _(found 2026-08-07)_ ### 5.9 The context has grown an app-config section — _(found 2026-08-07)_
+1
View File
@@ -95,6 +95,7 @@ export function App() {
<Route path="/email" element={<Dashboard.EmailScreen />} /> <Route path="/email" element={<Dashboard.EmailScreen />} />
<Route path="/email/:emailId" element={<Dashboard.EmailScreen />} /> <Route path="/email/:emailId" element={<Dashboard.EmailScreen />} />
<Route path="/browser" element={<Dashboard.BrowserScreen />} /> <Route path="/browser" element={<Dashboard.BrowserScreen />} />
<Route path="/browser/:tabId" element={<Dashboard.BrowserScreen />} />
<Route path="/terminal" element={<Dashboard.TerminalScreen />} /> <Route path="/terminal" element={<Dashboard.TerminalScreen />} />
<Route path="/desktop" element={<Dashboard.DesktopScreen />} /> <Route path="/desktop" element={<Dashboard.DesktopScreen />} />
<Route path="/auth/signout" element={<Authentication.SignoutScreen />} /> <Route path="/auth/signout" element={<Authentication.SignoutScreen />} />
@@ -1,12 +1,13 @@
import { useMemo, useCallback } from 'react'; import { useMemo, useCallback } from 'react';
import { useNavigate } from 'react-router';
import type { LayoutNode, PanelComponents } from 'officerdev'; import type { LayoutNode, PanelComponents } from 'officerdev';
import { WorkspaceView, ChatPanelWrapper } from 'officerdev'; import { WorkspaceView, ChatPanelWrapper } from 'officerdev';
import { useDashboardState } from 'state/useDashboardState'; import { useDashboardState } from 'state/useDashboardState';
import { useIsMobile } from 'hooks/useIsMobile'; import { useIsMobile } from 'hooks/useIsMobile';
import { useGlobal } from 'hooks/useGlobal';
import { defaultLayout } from './defaultLayout'; import { defaultLayout } from './defaultLayout';
import { TabList } from './TabList'; import { TabList } from './TabList';
import { TabPreview } from './TabPreview'; import { TabPreview } from './TabPreview';
import { useSelectedTabId } from './shared';
const PROMPT_PREFIX = `You are a browser assistant. The user has connected Chrome tabs via the Officer Browser Relay. You have a 'browser' tool — use it to list tabs, take screenshots, navigate, evaluate JavaScript, and get page info. When asked about a page, take a screenshot first.`; const PROMPT_PREFIX = `You are a browser assistant. The user has connected Chrome tabs via the Officer Browser Relay. You have a 'browser' tool — use it to list tabs, take screenshots, navigate, evaluate JavaScript, and get page info. When asked about a page, take a screenshot first.`;
@@ -15,7 +16,10 @@ const BrowserChatPanel = () => <ChatPanelWrapper panelId="browser-chat" promptPr
export const BrowserScreen = () => { export const BrowserScreen = () => {
const isMobile = useIsMobile(); const isMobile = useIsMobile();
const [selectedId, setSelectedId] = useGlobal<string | null>('BROWSER_SELECTED_TAB', null); const navigate = useNavigate();
// No redirect guard: like /email, the bare /browser is the list with nothing open, and a tab id that
// no longer resolves gets the preview's "no longer attached" state rather than a rewritten address.
const selectedId = useSelectedTabId();
const workspace = useDashboardState<LayoutNode>('screens/browser', defaultLayout); const workspace = useDashboardState<LayoutNode>('screens/browser', defaultLayout);
const components: PanelComponents = useMemo( const components: PanelComponents = useMemo(
@@ -28,7 +32,7 @@ export const BrowserScreen = () => {
); );
const mobilePanelId = isMobile && selectedId ? 'browser-preview' : undefined; const mobilePanelId = isMobile && selectedId ? 'browser-preview' : undefined;
const onMobileBack = useCallback(() => setSelectedId(null), [setSelectedId]); const onMobileBack = useCallback(() => navigate('/browser'), [navigate]);
return ( return (
<div className="h-full w-full pt-2"> <div className="h-full w-full pt-2">
@@ -1,20 +1,13 @@
import { useQuery } from '@tanstack/react-query'; import { useQuery } from '@tanstack/react-query';
import { Link } from 'react-router';
import { Globe, Loader2, X, Eye } from 'lucide-react'; import { Globe, Loader2, X, Eye } from 'lucide-react';
import { toast } from 'sonner'; import { toast } from 'sonner';
import { useClient } from 'hooks/useClient'; import { useClient } from 'hooks/useClient';
import { useGlobal } from 'hooks/useGlobal'; import { browserTabPath, useBrowserTargets, useSelectedTabId, type BrowserTarget } from './shared';
type Target = {
id: string;
sessionId: string;
type: string;
title: string;
url: string;
};
export const TabList = () => { export const TabList = () => {
const client = useClient(); const client = useClient();
const [selectedId, setSelectedId] = useGlobal<string | null>('BROWSER_SELECTED_TAB', null); const selectedId = useSelectedTabId();
const { data: status } = useQuery({ const { data: status } = useQuery({
queryKey: ['browser-status'], queryKey: ['browser-status'],
@@ -22,14 +15,9 @@ export const TabList = () => {
refetchInterval: 5000, refetchInterval: 5000,
}); });
const { data: targets, isLoading } = useQuery({ const { data: targets, isLoading } = useBrowserTargets(!!status?.extensionConnected);
queryKey: ['browser-targets'],
queryFn: () => client.get<Target[]>('/browser/targets'),
refetchInterval: 3000,
enabled: !!status?.extensionConnected,
});
const targetList: Target[] = targets ?? []; const targetList: BrowserTarget[] = targets ?? [];
const handleClose = async (targetId: string) => { const handleClose = async (targetId: string) => {
try { try {
@@ -64,12 +52,10 @@ export const TabList = () => {
<div className="flex flex-col items-center justify-center gap-3 p-4 text-sm flex-1"> <div className="flex flex-col items-center justify-center gap-3 p-4 text-sm flex-1">
<Globe className="h-8 w-8 opacity-30" /> <Globe className="h-8 w-8 opacity-30" />
<p className="opacity-60 text-center">Extension not connected</p> <p className="opacity-60 text-center">Extension not connected</p>
<a {/* A <Link>, not an <a href>: a raw href to an in-app route reloads the whole SPA. */}
href="/settings/integrations" <Link to="/settings/integrations" className="text-xs text-duck-teal underline">
className="text-xs text-duck-teal underline"
>
Set up in Integrations Set up in Integrations
</a> </Link>
</div> </div>
)} )}
@@ -89,41 +75,40 @@ export const TabList = () => {
{isConnected && targetList.length > 0 && ( {isConnected && targetList.length > 0 && (
<div className="flex flex-1 flex-col divide-y divide-white/10"> <div className="flex flex-1 flex-col divide-y divide-white/10">
{/* Focus and Close are siblings of the anchor, not children of it: an <a> may not contain a
<button>, and the old row was a <button> containing two more — invalid either way, and it
was only the stopPropagation calls holding it together. */}
{targetList.map((target) => ( {targetList.map((target) => (
<button <div
key={target.id} key={target.id}
onClick={() => setSelectedId(target.id)} className={`group relative flex transition-colors ${
className={`group flex flex-col gap-0.5 px-3 py-2.5 text-left transition-colors cursor-pointer ${
selectedId === target.id ? 'bg-accent' : 'hover:bg-accent/50' selectedId === target.id ? 'bg-accent' : 'hover:bg-accent/50'
}`} }`}
> >
<div className="flex items-center gap-2"> <Link
<span className="truncate text-sm font-medium">{target.title || 'Untitled'}</span> to={browserTabPath(target.id)}
<div className="ml-auto flex shrink-0 items-center gap-1 opacity-0 transition-opacity group-hover:opacity-100"> className="flex min-w-0 flex-1 flex-col gap-0.5 px-3 py-2.5 text-left"
<button >
onClick={(ev) => { <span className="truncate pr-14 text-sm font-medium">{target.title || 'Untitled'}</span>
ev.stopPropagation(); <span className="truncate text-xs opacity-50">{target.url}</span>
handleActivate(target.id); </Link>
}} <div className="absolute right-3 top-2.5 flex shrink-0 items-center gap-1 opacity-0 transition-opacity group-hover:opacity-100">
className="rounded p-0.5 hover:bg-black/10" <button
title="Focus tab" onClick={() => handleActivate(target.id)}
> className="rounded p-0.5 hover:bg-black/10 cursor-pointer"
<Eye className="h-3.5 w-3.5" /> title="Focus tab"
</button> >
<button <Eye className="h-3.5 w-3.5" />
onClick={(ev) => { </button>
ev.stopPropagation(); <button
handleClose(target.id); onClick={() => handleClose(target.id)}
}} className="rounded p-0.5 hover:bg-black/10 cursor-pointer"
className="rounded p-0.5 hover:bg-black/10" title="Close tab"
title="Close tab" >
> <X className="h-3.5 w-3.5" />
<X className="h-3.5 w-3.5" /> </button>
</button>
</div>
</div> </div>
<span className="truncate text-xs opacity-50">{target.url}</span> </div>
</button>
))} ))}
</div> </div>
)} )}
@@ -1,14 +1,20 @@
import { useState, useCallback } from 'react'; import { useState, useCallback } from 'react';
import { Link } from 'react-router';
import { useQuery } from '@tanstack/react-query'; import { useQuery } from '@tanstack/react-query';
import { Globe, RefreshCw, Send, Terminal, Loader2 } from 'lucide-react'; import { Globe, RefreshCw, Send, Terminal, Loader2 } from 'lucide-react';
import { toast } from 'sonner'; import { toast } from 'sonner';
import { Button } from '@/components/ui/button'; import { Button } from '@/components/ui/button';
import { useClient } from 'hooks/useClient'; import { useClient } from 'hooks/useClient';
import { useGlobal } from 'hooks/useGlobal'; import { useBrowserTargets, useSelectedTabId } from './shared';
export const TabPreview = () => { export const TabPreview = () => {
const client = useClient(); const client = useClient();
const [selectedId] = useGlobal<string | null>('BROWSER_SELECTED_TAB', null); // Reads the URL itself rather than being told by the list.
const selectedId = useSelectedTabId();
const { data: targets } = useBrowserTargets(!!selectedId);
// A target id dies with the browser session, so a link or a reload can name a tab that is no longer
// attached. Screenshotting it would 404 every ten seconds; better to say so once.
const isKnownTab = !!selectedId && !!targets?.some((t) => t.id === selectedId);
const [navUrl, setNavUrl] = useState(''); const [navUrl, setNavUrl] = useState('');
const [evalExpr, setEvalExpr] = useState(''); const [evalExpr, setEvalExpr] = useState('');
const [evalResult, setEvalResult] = useState<string | null>(null); const [evalResult, setEvalResult] = useState<string | null>(null);
@@ -23,7 +29,7 @@ export const TabPreview = () => {
} = useQuery({ } = useQuery({
queryKey: ['browser-screenshot', selectedId], queryKey: ['browser-screenshot', selectedId],
queryFn: () => client.get<{ data: string; format: string }>(`/browser/targets/${selectedId}/screenshot`), queryFn: () => client.get<{ data: string; format: string }>(`/browser/targets/${selectedId}/screenshot`),
enabled: !!selectedId, enabled: isKnownTab,
refetchInterval: 10000, refetchInterval: 10000,
}); });
@@ -64,6 +70,19 @@ export const TabPreview = () => {
); );
} }
// Waiting on the list is not the same as knowing the tab is gone, so only say so once it has arrived.
if (targets && !isKnownTab) {
return (
<div className="flex h-full flex-col items-center justify-center gap-2 text-sm opacity-50">
<Globe className="h-8 w-8" />
<span>That tab is no longer attached</span>
<Link to="/browser" className="text-xs underline">
Back to the tab list
</Link>
</div>
);
}
return ( return (
<div className="flex h-full flex-col overflow-hidden"> <div className="flex h-full flex-col overflow-hidden">
{/* URL bar */} {/* URL bar */}
@@ -0,0 +1,34 @@
import { useQuery } from '@tanstack/react-query';
import { useParams } from 'react-router';
import { useClient } from 'hooks/useClient';
// Which attached tab is being previewed is `/browser/:tabId`. It used to be a `BROWSER_SELECTED_TAB`
// global read by three components, which meant the address bar said `/browser` no matter what you were
// looking at, and a tab row was a `<button>` with the id in its onClick closure.
export type BrowserTarget = {
id: string;
sessionId: string;
type: string;
title: string;
url: string;
};
export const browserTabPath = (id: string) => `/browser/${encodeURIComponent(id)}`;
/** The previewed tab, or null on the bare `/browser` route — the list with nothing open. */
export const useSelectedTabId = (): string | null => useParams<{ tabId?: string }>().tabId ?? null;
// Both panels want this list and React Query serves both observers from one key, so the preview being
// able to tell "no tab open" from "that tab is gone" costs no extra request. It needs to: a CDP target
// id dies with the browser session, and now that the id is in the URL a reload can carry a stale one in
// — which the old global, living in memory, could never do.
export const useBrowserTargets = (enabled: boolean) => {
const client = useClient();
return useQuery({
queryKey: ['browser-targets'],
queryFn: () => client.get<BrowserTarget[]>('/browser/targets'),
refetchInterval: 3000,
enabled,
});
};