diff --git a/apps/desktop/src/app/chat/preview-tile.tsx b/apps/desktop/src/app/chat/preview-tile.tsx index df6356d5e9..67a25332e3 100644 --- a/apps/desktop/src/app/chat/preview-tile.tsx +++ b/apps/desktop/src/app/chat/preview-tile.tsx @@ -33,11 +33,12 @@ import { popOutBrowserTab, type PreviewTarget } from '@/store/preview' +import { explicitOpenBlocksZone, PREVIEW_TILE_PREFIX } from '@/store/preview-explicit' import { canOpenBrowserWindow } from '@/store/windows' import { paneMirror } from './pane-mirror' -import { PreviewTilePane } from './right-rail/preview' import { forgetPreviewConsole } from './right-rail/preview-console-store' +import { PreviewTilePane } from './right-rail/preview' /** The target behind a tile id, or null once its tab is gone. */ function targetFor(tabId: string): PreviewTarget | null { @@ -169,8 +170,6 @@ function PreviewTabLead({ tabId }: { tabId: string }) { return } -const PREVIEW_TILE_PREFIX = 'preview-tile' - const previewPaneId = (tabId: string) => `${PREVIEW_TILE_PREFIX}:${tabId}` /** The pane a NEW preview tile should stack into: another preview tile already @@ -226,6 +225,13 @@ export function watchPreviewTiles(): void { const follow = () => { const tree = $layoutTree.get() const groupId = $activeTreeGroup.get() + + // Do not copy this zone over an explicit open that lives in a different + // group. A focus change after that open lifts the guard. + if (explicitOpenBlocksZone(groupId, $previewTabs.get().map(tab => tab.id))) { + return + } + const active = groupId && tree ? findGroup(tree, groupId)?.active : undefined if (!active?.startsWith(`${PREVIEW_TILE_PREFIX}:`)) { diff --git a/apps/desktop/src/app/chat/right-rail/preview-reader.test.ts b/apps/desktop/src/app/chat/right-rail/preview-reader.test.ts index fce9c41014..36e0b7dd72 100644 --- a/apps/desktop/src/app/chat/right-rail/preview-reader.test.ts +++ b/apps/desktop/src/app/chat/right-rail/preview-reader.test.ts @@ -1,8 +1,15 @@ import { beforeEach, describe, expect, it } from 'vitest' +import { group, split } from '@/components/pane-shell/tree/model' +import { + $layoutTree, + noteActiveTreeGroup, + noteHoveredTreeGroup +} from '@/components/pane-shell/tree/store' import { $rightRailActiveTabId, selectRightRailTab } from '@/store/layout' -import { closeRightRail, openPreview, type PreviewTarget } from '@/store/preview' +import { $previewTabs, closeRightRail, openPreview, type PreviewTarget } from '@/store/preview' +import { watchPreviewTiles } from '../preview-tile' import { PREVIEW_READ_MAX_CHARS, readActivePreview, registerPreviewPageReader } from './preview-reader' function urlTarget(url: string): PreviewTarget { @@ -34,6 +41,9 @@ describe('readActivePreview (read_preview tool)', () => { cleanups = [] closeRightRail() window.localStorage.clear() + noteActiveTreeGroup(null) + noteHoveredTreeGroup(null) + $layoutTree.set(null) }) it('answers null when nothing is open, so the tool reports it cleanly', async () => { @@ -137,4 +147,100 @@ describe('readActivePreview (read_preview tool)', () => { expect(await readActivePreview()).toMatchObject({ text: 'second' }) }) + + it('reads the hovered preview zone instead of the global right-rail tab', async () => { + openPreview(fileTarget('/work/a.md')) + const fileId = $rightRailActiveTabId.get()! + openPreview(urlTarget('https://example.com/tickets')) + const browserId = $rightRailActiveTabId.get()! + selectRightRailTab(fileId) + mountSplit(fileId, browserId) + noteHoveredTreeGroup('grp-browser') + + expect(await readActivePreview()).toMatchObject({ kind: 'url', url: 'https://example.com/tickets' }) + }) + + it('reads the focused preview zone instead of a stale global file tab', async () => { + openPreview(fileTarget('/work/a.md')) + const fileId = $rightRailActiveTabId.get()! + openPreview(urlTarget('https://example.com/tickets')) + const browserId = $rightRailActiveTabId.get()! + selectRightRailTab(fileId) + mountSplit(fileId, browserId) + noteActiveTreeGroup('grp-browser') + + expect(await readActivePreview()).toMatchObject({ kind: 'url', url: 'https://example.com/tickets' }) + }) + + it('reads the hovered file when that zone is what the user is looking at', async () => { + openPreview(fileTarget('/work/a.md')) + const fileId = $rightRailActiveTabId.get()! + openPreview(urlTarget('https://example.com/tickets')) + const browserId = $rightRailActiveTabId.get()! + mountSplit(fileId, browserId) + noteHoveredTreeGroup('grp-file') + + expect(await readActivePreview()).toMatchObject({ kind: 'file', path: '/work/a.md' }) + }) + + it('returns active_tab_id and the open tab list when more than one preview is mounted', async () => { + openPreview(fileTarget('/work/project-network.html')) + const fileId = $rightRailActiveTabId.get()! + openPreview(urlTarget('https://example.com/tickets')) + const browserId = $rightRailActiveTabId.get()! + + expect(await readActivePreview()).toMatchObject({ + active_tab_id: browserId, + kind: 'url', + tabs: [ + { id: fileId, kind: 'file', label: '/work/project-network.html', url: 'file:///work/project-network.html' }, + { id: browserId, kind: 'url', label: 'Browser', url: 'https://example.com/tickets' } + ], + url: 'https://example.com/tickets' + }) + expect($previewTabs.get()).toHaveLength(2) + }) }) + +describe('follow() does not overwrite an explicit open in another group', () => { + beforeEach(() => { + closeRightRail() + window.localStorage.clear() + noteActiveTreeGroup(null) + noteHoveredTreeGroup(null) + $layoutTree.set(null) + }) + + it('keeps the opened URL when the other group is still the interacted zone', async () => { + watchPreviewTiles() + openPreview(fileTarget('/work/project-network.html')) + const fileId = $rightRailActiveTabId.get()! + openPreview(urlTarget('about:blank')) + const browserId = $rightRailActiveTabId.get()! + mountSplit(fileId, browserId) + noteActiveTreeGroup('grp-file') + + openPreview(urlTarget('https://example.com/tickets')) + // reveal may not commit when the pane is already fronted; the layout + // listener is what copies the interacted zone. Fire that same listener. + $layoutTree.set(mountSplit(fileId, browserId)) + + expect($rightRailActiveTabId.get()).toBe(browserId) + expect(await readActivePreview()).toMatchObject({ + active_tab_id: browserId, + kind: 'url', + url: 'https://example.com/tickets' + }) + }) +}) + +function mountSplit(fileId: string, browserId: string) { + const tree = split('row', [ + group([`preview-tile:${browserId}`], { active: `preview-tile:${browserId}`, id: 'grp-browser' }), + group([`preview-tile:${fileId}`], { active: `preview-tile:${fileId}`, id: 'grp-file' }) + ]) + + $layoutTree.set(tree) + + return tree +} diff --git a/apps/desktop/src/app/chat/right-rail/preview-reader.ts b/apps/desktop/src/app/chat/right-rail/preview-reader.ts index 97a372db18..76c380eb52 100644 --- a/apps/desktop/src/app/chat/right-rail/preview-reader.ts +++ b/apps/desktop/src/app/chat/right-rail/preview-reader.ts @@ -5,15 +5,19 @@ * * A URL/HTML preview renders in a sandboxed owned by PreviewPane; * that pane registers a PAGE READER here (url + title + rendered text), keyed - * by tab id. `readActivePreview` resolves the ACTIVE tab from the store and - * owns the windowing: a registered reader answers with the live page's text; + * by tab id. `readActivePreview` resolves the preview the user is looking at + * (hovered zone, else focused zone, else the store) and owns the windowing: + * a registered reader answers with the live page's text; * a tab with no reader (a file peek, an artifact) still answers with its * identity and a note pointing the agent at the tool that reads that content * directly (read_file / the conversation's artifact). */ +import { findGroup } from '@/components/pane-shell/tree/model' +import { $activeTreeGroup, $hoveredTreeGroup, $layoutTree } from '@/components/pane-shell/tree/store' import { $rightRailActiveTabId } from '@/store/layout' -import { $previewTabs } from '@/store/preview' +import { $previewTabs, type PreviewTab } from '@/store/preview' +import { explicitOpenBlocksZone, PREVIEW_TILE_PREFIX } from '@/store/preview-explicit' import { nudgeOverlay } from './preview-nudge' @@ -24,12 +28,23 @@ export interface PreviewReadOptions { start?: number } +export interface PreviewReadTabSummary { + id: string + kind: string + label: string + url: string +} + export interface PreviewReadResult { + /** Set when more than one preview is mounted — the tab this read used. */ + active_tab_id?: string end: number kind: string note?: string path?: string start: number + /** Open preview tabs, set when more than one is mounted. */ + tabs?: PreviewReadTabSummary[] text: string title: string total_chars: number @@ -75,10 +90,85 @@ function windowText( return { ...base, end: to, start: from, text: text.slice(from, to), total_chars: total } } -/** Read the ACTIVE preview tab. Null only when no tab is open at all. */ -export async function readActivePreview(opts: PreviewReadOptions = {}): Promise { +function tabIdFromPreviewPane(paneId: string | undefined): null | string { + if (!paneId?.startsWith(`${PREVIEW_TILE_PREFIX}:`)) { + return null + } + + return paneId.slice(PREVIEW_TILE_PREFIX.length + 1) +} + +/** Active preview tab in a layout zone, if that tab is still open. */ +function openTabInGroup(groupId: null | string, tabs: PreviewTab[]): null | PreviewTab { + const tree = $layoutTree.get() + + if (!tree || !groupId) { + return null + } + + const tabId = tabIdFromPreviewPane(findGroup(tree, groupId)?.active) + + if (!tabId) { + return null + } + + return tabs.find(tab => tab.id === tabId) ?? null +} + +/** + * The preview the user is looking at: hovered zone, else focused zone, else + * the store. A focused zone that is still the pre-open zone does not override + * an explicit open living in a different group — that is follow()'s clobber, + * not a look. + */ +export function resolveActivePreviewTab(tabs: PreviewTab[] = $previewTabs.get()): null | PreviewTab { + if (tabs.length === 0) { + return null + } + + const hovered = openTabInGroup($hoveredTreeGroup.get(), tabs) + + if (hovered) { + return hovered + } + + const focusedId = $activeTreeGroup.get() + const focused = openTabInGroup(focusedId, tabs) + const openIds = tabs.map(tab => tab.id) + + if (focused && !explicitOpenBlocksZone(focusedId, openIds)) { + return focused + } + + return tabs.find(tab => tab.id === $rightRailActiveTabId.get()) ?? tabs[0] ?? null +} + +function tabSummary(tab: PreviewTab): PreviewReadTabSummary { + return { id: tab.id, kind: tab.target.kind, label: tab.target.label, url: tab.target.url } +} + +function zoneMeta(tab: PreviewTab, tabs: PreviewTab[]): { active_tab_id?: string; tabs?: PreviewReadTabSummary[] } { + if (tabs.length < 2) { + return {} + } + + return { active_tab_id: tab.id, tabs: tabs.map(tabSummary) } +} + +function withMultiNote(note: string | undefined, multi: boolean): string | undefined { + if (!multi) { + return note + } + + const extra = 'Multiple preview tabs are open; this read used the hovered or focused preview (see tabs).' + + return note ? `${note} ${extra}` : extra +} + +/** Read the preview the user is looking at. Null only when no tab is open at all. */ +export async function readActivePreview(opts: PreviewReadOptions = {}): Promise { const tabs = $previewTabs.get() - const tab = tabs.find(t => t.id === $rightRailActiveTabId.get()) ?? tabs[0] + const tab = resolveActivePreviewTab(tabs) if (!tab) { return null @@ -86,6 +176,8 @@ export async function readActivePreview(opts: PreviewReadOptions = {}): Promise< const { target } = tab const reader = readers.get(tab.id) + const multi = tabs.length > 1 + const meta = zoneMeta(tab, tabs) if (reader) { try { @@ -99,7 +191,14 @@ export async function readActivePreview(opts: PreviewReadOptions = {}): Promise< nudgeOverlay('read') return windowText( - { kind: target.kind, path: target.path, title: page.title || target.label, url: page.url || target.url }, + { + ...meta, + kind: target.kind, + note: withMultiNote(undefined, multi), + path: target.path, + title: page.title || target.label, + url: page.url || target.url + }, page.text, opts ) @@ -112,15 +211,18 @@ export async function readActivePreview(opts: PreviewReadOptions = {}): Promise< // No live webview behind the tab (a file peek, an artifact, or a page still // booting): answer with the tab's identity so the agent knows what's on // screen and which of its own tools reads the content directly. + const identity = + target.kind === 'file' + ? 'File preview — read the file itself with read_file.' + : target.kind === 'artifact' + ? 'Generated artifact — its content is in the conversation that produced it.' + : 'The page has not finished loading — retry in a moment.' + return windowText( { + ...meta, kind: target.kind, - note: - target.kind === 'file' - ? 'File preview — read the file itself with read_file.' - : target.kind === 'artifact' - ? 'Generated artifact — its content is in the conversation that produced it.' - : 'The page has not finished loading — retry in a moment.', + note: withMultiNote(identity, multi), path: target.path, title: target.label, url: target.url diff --git a/apps/desktop/src/store/preview-explicit.ts b/apps/desktop/src/store/preview-explicit.ts new file mode 100644 index 0000000000..6aa895f15d --- /dev/null +++ b/apps/desktop/src/store/preview-explicit.ts @@ -0,0 +1,69 @@ +/** + * An explicit preview open (tool or user `openPreview`) must survive a later + * `follow()` from a zone the user was already in. `follow()` copies that + * zone's tab into `$rightRailActiveTabId`; when the opened pane lives in a + * different group, that copy is the desync `read_preview` used to report. + * + * A focus change after the open is a new look, so follow may sync then. + * Hover is not tracked here — the reader treats the pointer as a live look. + */ + +import { findGroupOfPane } from '@/components/pane-shell/tree/model' +import { $activeTreeGroup, $layoutTree } from '@/components/pane-shell/tree/store' +import type { RightRailTabId } from '@/store/layout' + +/** Pane id prefix for a preview tab. Must match `preview-tile.tsx`. */ +export const PREVIEW_TILE_PREFIX = 'preview-tile' + +type ExplicitOpen = { + tabId: RightRailTabId + /** `$activeTreeGroup` when the open happened. Unchanged means follow is + * still the pre-open zone, not a zone the user moved to afterwards. */ + activeGroupAtSelect: null | string +} + +let explicit: ExplicitOpen | null = null + +/** Stamp a tool/user open. Call before `selectRightRailTab` so a synchronous + * follow from that select already sees the guard. */ +export function noteExplicitPreviewOpen(tabId: RightRailTabId): void { + explicit = { activeGroupAtSelect: $activeTreeGroup.get(), tabId } +} + +export function clearExplicitPreviewOpen(): void { + explicit = null +} + +function paneGroupId(tabId: string): null | string { + const tree = $layoutTree.get() + + if (!tree) { + return null + } + + return findGroupOfPane(tree, `${PREVIEW_TILE_PREFIX}:${tabId}`)?.id ?? null +} + +/** + * True when copying `sourceGroupId`'s preview would overwrite an explicit + * open that lives in a different group, and the user has not focused a + * different zone since that open. + */ +export function explicitOpenBlocksZone(sourceGroupId: null | string, openTabIds: readonly string[]): boolean { + if (!explicit || !openTabIds.includes(explicit.tabId)) { + return false + } + + if (sourceGroupId !== explicit.activeGroupAtSelect) { + return false + } + + const openedIn = paneGroupId(explicit.tabId) + + // Not placed yet: the stale zone must not win the race with reveal. + if (!openedIn) { + return true + } + + return openedIn !== sourceGroupId +} diff --git a/apps/desktop/src/store/preview.ts b/apps/desktop/src/store/preview.ts index c36aee8777..810d47483a 100644 --- a/apps/desktop/src/store/preview.ts +++ b/apps/desktop/src/store/preview.ts @@ -4,6 +4,7 @@ import { readJson, readKey, writeKey } from '@/lib/storage' import { normalize } from '@/lib/text' import { $rightRailActiveTabId, type RightRailTabId, selectRightRailTab } from './layout' +import { clearExplicitPreviewOpen, noteExplicitPreviewOpen } from './preview-explicit' import { normalizeProfileKey } from './profile' import { canOpenBrowserWindow, openBrowserInNewWindow } from './windows' @@ -544,6 +545,7 @@ export function openPreview(target: PreviewTarget) { const tab: PreviewTab = { id, target: withRenderMode(target, current[index]?.target) } $previewTabs.set(index === -1 ? [...current, tab] : current.map((item, i) => (i === index ? tab : item))) + noteExplicitPreviewOpen(id) selectRightRailTab(id) } @@ -565,6 +567,7 @@ export function newBrowserTab() { const id = mintBrowserTabId() $previewTabs.set([...$previewTabs.get(), { id, target: blankPage() }]) + noteExplicitPreviewOpen(id) selectRightRailTab(id) } @@ -581,7 +584,15 @@ export function closeRightRailTab(tabId: string) { $previewTabs.set(next) if ($rightRailActiveTabId.get() === tabId) { - selectRightRailTab(next[Math.min(index, next.length - 1)]?.id ?? null) + const nextId = next[Math.min(index, next.length - 1)]?.id ?? null + + if (nextId) { + noteExplicitPreviewOpen(nextId) + } else { + clearExplicitPreviewOpen() + } + + selectRightRailTab(nextId) } if (next.length === 0) { @@ -631,6 +642,7 @@ export function closeArtifactPreviewTabs() { /** Close every tab so the rail's panes leave the tree. */ export function closeRightRail() { + clearExplicitPreviewOpen() $previewTabs.set([]) selectRightRailTab(null) } diff --git a/tools/read_preview_tool.py b/tools/read_preview_tool.py index 7e1334d8e8..7330d17494 100644 --- a/tools/read_preview_tool.py +++ b/tools/read_preview_tool.py @@ -35,8 +35,10 @@ READ_PREVIEW_SCHEMA = { "Read what's currently shown in the in-app browser / preview pane of the " "Hermes desktop GUI (the pane open_preview opens beside this chat). Call " "with no arguments for the first window of the active tab's content. " - "Returns JSON {kind, url, title, text, start, end, total_chars, note?}: " - "a URL (Browser) tab's text is the rendered page's visible text — page " + "Returns JSON {kind, url, title, text, start, end, total_chars, note?}. " + "When more than one preview is mounted, the JSON also includes active_tab_id and tabs " + "[{id, kind, label, url}] for the hovered or focused zone, not a stale " + "global tab. A URL (Browser) tab's text is the rendered page's visible text — page " "through longer pages with `start`/`count` (character offsets, capped " "per read); a file tab answers identity only (read the file with " "read_file); an artifact tab points back at the conversation. Use after "