From b42919447836d22eafc08e19aa6eeaf9ce7dc366 Mon Sep 17 00:00:00 2001 From: Gille <4317663+helix4u@users.noreply.github.com> Date: Mon, 27 Jul 2026 16:45:03 -0600 Subject: [PATCH 1/7] refactor(desktop): simplify free-text slash mode check (#72815) --- .../app/chat/composer/hooks/use-composer-trigger.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts index 5f0be41c80c..e6ab8535124 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts @@ -139,10 +139,12 @@ export function useComposerTrigger({ // Space/Tab — neither should dead-end on a popover. const argStageEmpty = trigger?.kind === '/' && slashArgStage(trigger.query) && !triggerLoading && !triggerItems.length - const slashFreeTextArgStage = - trigger?.kind === '/' && - slashArgStage(trigger.query) && - ['mixed', 'text'].includes(desktopSlashCommandArgumentMode(slashCommandToken(trigger.query)) ?? '') + const slashArgumentMode = + trigger?.kind === '/' && slashArgStage(trigger.query) + ? desktopSlashCommandArgumentMode(slashCommandToken(trigger.query)) + : null + + const slashFreeTextArgStage = slashArgumentMode === 'mixed' || slashArgumentMode === 'text' const closeTrigger = () => { setTrigger(null) From e3acdfb21da9a06f9b326f001563cf41641787c4 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Mon, 27 Jul 2026 17:50:12 -0500 Subject: [PATCH 2/7] refactor(desktop): lift the completion-accept decision out of the keydown ladder Which keys accept the highlighted completion was an inline condition in the composer's keydown god-function, untestable without a DOM harness. Move it to a pure helper beside the other slash-query utilities. --- .../app/chat/composer/composer-utils.test.ts | 51 ++++++++++++++++++- .../src/app/chat/composer/composer-utils.ts | 44 ++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/app/chat/composer/composer-utils.test.ts b/apps/desktop/src/app/chat/composer/composer-utils.test.ts index 4df8463ba2d..062b2567738 100644 --- a/apps/desktop/src/app/chat/composer/composer-utils.test.ts +++ b/apps/desktop/src/app/chat/composer/composer-utils.test.ts @@ -2,12 +2,14 @@ import type { Unstable_TriggerItem } from '@assistant-ui/core' import { describe, expect, it } from 'vitest' import { + acceptsTriggerCompletion, isPendingDraftPersistCurrent, type PendingDraftPersist, pickPlaceholder, slashArgStage, slashChipKindForItem, - slashCommandToken + slashCommandToken, + type TriggerAcceptInput } from './composer-utils' const item = (group: string): Unstable_TriggerItem => @@ -39,6 +41,53 @@ describe('slashChipKindForItem', () => { }) }) +describe('acceptsTriggerCompletion', () => { + const press = (key: string, overrides: Partial = {}) => + acceptsTriggerCompletion({ + activeExplicit: false, + freeTextArgStage: false, + key, + kind: '/', + query: 'personality alic', + ...overrides + }) + + it('accepts on Enter / Tab / Space for a finite option list', () => { + expect(press('Enter')).toBe(true) + expect(press('Tab')).toBe(true) + expect(press(' ')).toBe(true) + }) + + it('ignores keys that are neither navigation nor acceptance', () => { + expect(press('a')).toBe(false) + expect(press('Escape')).toBe(false) + }) + + it('lets an `@` mention take a literal space', () => { + expect(press(' ', { kind: '@', query: 'src/comp' })).toBe(false) + expect(press('Enter', { kind: '@', query: 'src/comp' })).toBe(true) + }) + + it('types a space on a bare `/ ` instead of accepting', () => { + expect(press(' ', { query: '' })).toBe(false) + }) + + // The `/goal ` class: the popover may be live over free-form text, so + // the keys that mean something else in prose must keep meaning it. + it('sends the prose rather than the unchosen first row', () => { + expect(press('Enter', { freeTextArgStage: true, query: 'goal ship the redesign' })).toBe(false) + expect(press(' ', { freeTextArgStage: true, query: 'goal ship the' })).toBe(false) + }) + + it('accepts on Enter once the user has arrowed to a row deliberately', () => { + expect(press('Enter', { activeExplicit: true, freeTextArgStage: true, query: 'goal stat' })).toBe(true) + }) + + it('keeps Tab as the explicit accept even over free text', () => { + expect(press('Tab', { freeTextArgStage: true, query: 'goal stat' })).toBe(true) + }) +}) + describe('pickPlaceholder', () => { it('returns a member of the pool', () => { const pool = ['a', 'b', 'c'] as const diff --git a/apps/desktop/src/app/chat/composer/composer-utils.ts b/apps/desktop/src/app/chat/composer/composer-utils.ts index 547a210f06f..21e3c1ac4a1 100644 --- a/apps/desktop/src/app/chat/composer/composer-utils.ts +++ b/apps/desktop/src/app/chat/composer/composer-utils.ts @@ -4,6 +4,8 @@ import type { SlashChipKind } from '@/components/assistant-ui/directive-text' import type { ComposerAttachment } from '@/store/composer' import { setSessionPickerOpen } from '@/store/session' +import type { TriggerState } from './text-utils' + export const COMPOSER_STACK_BREAKPOINT_PX = 320 // Above the stack breakpoint but still cramped: the model pill sheds its label @@ -59,6 +61,48 @@ export const slashArgStage = (query: string) => query.includes(' ') /** The `/command` token of a slash query (`personality x` → `/personality`). */ export const slashCommandToken = (query: string) => `/${query.split(/\s+/, 1)[0]?.toLowerCase() ?? ''}` +export interface TriggerAcceptInput { + /** The user moved the highlight themselves (arrow keys) rather than + * inheriting the list's default first row. */ + activeExplicit: boolean + /** The trigger is a slash command whose argument is arbitrary prose. */ + freeTextArgStage: boolean + key: string + kind: TriggerState['kind'] + query: string +} + +/** + * Whether a keypress accepts the highlighted completion while the popover is + * open. Tab is always an accept — it has no other meaning in the composer. + * + * Enter and Space are conditional, because both mean something else while a + * free-text argument is being written (`/goal ship the redesign`). Space types + * a space, and Enter sends the message; letting either take the popover's + * pre-highlighted row would swap the prose the user is mid-sentence on for a + * subcommand they never chose. Enter still accepts once the user has arrowed + * to a row deliberately, so the highlight never lies about what Enter will do. + */ +export function acceptsTriggerCompletion({ + activeExplicit, + freeTextArgStage, + key, + kind, + query +}: TriggerAcceptInput): boolean { + if (key === 'Tab') { + return true + } + + if (key === 'Enter') { + return !freeTextArgStage || activeExplicit + } + + // Space is slash-only (an `@` mention takes a literal space) and gated to a + // non-empty query so a bare `/ ` still types a space. + return key === ' ' && kind === '/' && Boolean(query.trim()) && !freeTextArgStage +} + export interface QueueEditState { attachments: ComposerAttachment[] draft: string From 43571601aa71fbd30e839f567e540106a92d8a03 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Mon, 27 Jul 2026 17:50:18 -0500 Subject: [PATCH 3/7] fix(desktop): don't let Enter swap a free-text slash argument for a completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/goal` keeps its completion popover open across arbitrary prose so its subcommands stay reachable. The popover highlights its first row on open, and Enter accepted that highlight unconditionally — so pressing Enter to send `/goal ship the redesign` would replace the sentence with a row the user never chose. Space was already guarded; Enter and Tab were not. Enter now accepts only after the user has arrowed to a row deliberately, so the highlight never lies about what Enter will do. Tab stays an unconditional accept, since it means nothing else in the composer. This is latent rather than reproducible today: `/goal` is absent from `SUBCOMMANDS` (its `args_hint` pipes are spaced, so the extraction regex misses them), so the backend returns no arg completions and the branch never runs. Giving `/goal` the subcommands it already advertises would resurrect the #71963 symptom in a worse form — losing the prose instead of chipping it. --- .../hooks/use-composer-trigger.test.ts | 24 ++++++++++++++ .../composer/hooks/use-composer-trigger.ts | 26 ++++++++++++--- apps/desktop/src/app/chat/composer/index.tsx | 32 ++++++++++++------- 3 files changed, 66 insertions(+), 16 deletions(-) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.test.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.test.ts index c7a4e2f138a..a4ab38b3b3a 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.test.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.test.ts @@ -148,6 +148,30 @@ describe('useComposerTrigger — free-text slash arguments', () => { expect(editor.querySelector('[data-slash-kind]')).toBeNull() }) + it('treats the default highlight as a suggestion until the user arrows to a row', () => { + const editor = mountEditor('/goal stat') + const { hook } = mountTrigger(editor, [item('/goal status', 'Options')]) + + act(() => hook.result.current.refreshTrigger()) + expect(hook.result.current.triggerActiveExplicit).toBe(false) + + act(() => hook.result.current.moveTriggerActive(1)) + expect(hook.result.current.triggerActiveExplicit).toBe(true) + }) + + it('drops a deliberate selection once the query moves on', () => { + const editor = mountEditor('/goal stat') + const { hook } = mountTrigger(editor, [item('/goal status', 'Options')]) + + act(() => hook.result.current.refreshTrigger()) + act(() => hook.result.current.moveTriggerActive(1)) + + renderComposerContents(editor, '/goal start the migration') + act(() => hook.result.current.refreshTrigger()) + + expect(hook.result.current.triggerActiveExplicit).toBe(false) + }) + it('still commits a fully typed finite option as one directive chip', () => { const editor = mountEditor('/personality creative') const { hook } = mountTrigger(editor, []) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts index 5f0be41c80c..65f09225f65 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts @@ -53,6 +53,11 @@ export function useComposerTrigger({ }: UseComposerTriggerOptions) { const [trigger, setTrigger] = useState(null) const [triggerActive, setTriggerActive] = useState(0) + // The list highlights its first row on open, which is a suggestion rather + // than a choice. This records that the user moved the highlight themselves, + // which is what lets Enter accept a completion in a free-text argument stage + // without stealing prose from everyone who never touched the arrows. + const [triggerActiveExplicit, setTriggerActiveExplicit] = useState(false) const [triggerItems, setTriggerItems] = useState([]) // Set synchronously in keydown when the open trigger popover consumes a // navigation/control key (Arrow/Enter/Tab/Escape). The subsequent keyup must @@ -63,6 +68,11 @@ export function useComposerTrigger({ // re-rendered and the handler closure sees the post-keydown state. const triggerKeyConsumedRef = useRef(false) + const resetTriggerActive = useCallback(() => { + setTriggerActive(0) + setTriggerActiveExplicit(false) + }, []) + const refreshTrigger = useCallback(() => { const editor = editorRef.current @@ -80,7 +90,7 @@ export function useComposerTrigger({ if (!rawText.includes('@') && !rawText.includes('/')) { if (trigger) { setTrigger(null) - setTriggerActive(0) + resetTriggerActive() } return @@ -109,9 +119,9 @@ export function useComposerTrigger({ // caret move (mouseup) or a stray refresh — must preserve the user's // current selection instead of snapping back to the first item. if (detected?.kind !== trigger?.kind || detected?.query !== trigger?.query) { - setTriggerActive(0) + resetTriggerActive() } - }, [editorRef, trigger]) + }, [editorRef, resetTriggerActive, trigger]) const triggerAdapter: Unstable_TriggerAdapter | null = trigger?.kind === '@' ? at.adapter : trigger?.kind === '/' ? slash.adapter : null @@ -147,7 +157,13 @@ export function useComposerTrigger({ const closeTrigger = () => { setTrigger(null) setTriggerItems([]) - setTriggerActive(0) + resetTriggerActive() + } + + /** Step the highlight, marking it as the user's own deliberate pick. */ + const moveTriggerActive = (delta: number) => { + setTriggerActiveExplicit(true) + setTriggerActive(idx => (idx + delta + triggerItems.length) % triggerItems.length) } useEffect(() => { @@ -358,12 +374,14 @@ export function useComposerTrigger({ ascendTriggerPath, closeTrigger, commitTypedSlashDirective, + moveTriggerActive, refreshTrigger, replaceTriggerWithChip, setTriggerActive, slashFreeTextArgStage, trigger, triggerActive, + triggerActiveExplicit, triggerItems, triggerKeyConsumedRef, triggerLoading diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index 6ce8289fa0a..ac4f5dd3027 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -22,7 +22,12 @@ import { $autoSpeakReplies } from '@/store/voice-prefs' import { useTheme } from '@/themes' import { AttachmentList } from './attachments' -import { COMPOSER_FADE_BACKGROUND, type QueueEditState, slashArgStage } from './composer-utils' +import { + acceptsTriggerCompletion, + COMPOSER_FADE_BACKGROUND, + type QueueEditState, + slashArgStage +} from './composer-utils' import { ContextMenu } from './context-menu' import { COMPOSER_AREAS, runComposerMiddleware } from './contrib' import { ComposerControls } from './controls' @@ -302,12 +307,14 @@ export function ChatBar({ ascendTriggerPath, closeTrigger, commitTypedSlashDirective, + moveTriggerActive, refreshTrigger, replaceTriggerWithChip, setTriggerActive, slashFreeTextArgStage, trigger, triggerActive, + triggerActiveExplicit, triggerItems, triggerKeyConsumedRef, triggerLoading @@ -528,7 +535,7 @@ export function ChatBar({ if (event.key === 'ArrowDown') { event.preventDefault() triggerKeyConsumedRef.current = true - setTriggerActive(idx => (idx + 1) % triggerItems.length) + moveTriggerActive(1) return } @@ -536,20 +543,21 @@ export function ChatBar({ if (event.key === 'ArrowUp') { event.preventDefault() triggerKeyConsumedRef.current = true - setTriggerActive(idx => (idx - 1 + triggerItems.length) % triggerItems.length) + moveTriggerActive(-1) return } - // Enter / Tab / Space all accept the highlighted item: a no-arg command - // commits its directive chip, an arg-taking command expands to its - // options step, and an arg option commits the full `/cmd arg` chip. Space - // is slash-only (an `@` mention takes a literal space) and gated to a - // non-empty query so a bare `/ ` still types a space. - const acceptOnSpace = - event.key === ' ' && trigger.kind === '/' && Boolean(trigger.query.trim()) && !slashFreeTextArgStage - - const accept = event.key === 'Enter' || event.key === 'Tab' || acceptOnSpace + // Accepting the highlighted item: a no-arg command commits its directive + // chip, an arg-taking command expands to its options step, and an arg + // option commits the full `/cmd arg` chip. + const accept = acceptsTriggerCompletion({ + activeExplicit: triggerActiveExplicit, + freeTextArgStage: slashFreeTextArgStage, + key: event.key, + kind: trigger.kind, + query: trigger.query + }) if (accept) { event.preventDefault() From 96999b116b5efcbdfbe4e84b79ef28b4fccd045a Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Mon, 27 Jul 2026 17:55:13 -0500 Subject: [PATCH 4/7] refactor(desktop): put every preview on one rail tab list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The right rail held two things at once: a list of file tabs, and a privileged "live preview" slot with a hardcoded `preview` tab id backed by a separate session-keyed registry. The two were written under different session-id rules and reconciled against each other, so an `open_preview` from a session whose stored id hadn't landed yet was set and then immediately cleared — the pane flashed and vanished. Artifacts arrived as a third list with their own pane and renderers. Now everything the rail can show is a `PreviewTarget` in `$previewTabs`, and `openPreview` is the only way in. `$previewTarget` is a computed read of the active tab, the session registry and its reconciler are gone, and artifacts render in the real preview pane through the shared mode switcher and source view instead of a parallel one. Artifact tabs stay memory-only since the registry rebuilds from the transcript. --- apps/desktop/src/app/chat/close-tab.test.ts | 41 +- apps/desktop/src/app/chat/close-tab.ts | 13 +- .../composer/status-stack/preview-row.tsx | 10 +- .../src/app/chat/right-rail/artifact-pane.tsx | 222 ------- .../chat/right-rail/artifact-renderers.tsx | 107 --- .../chat/right-rail/preview-artifact.test.tsx | 112 ++++ .../app/chat/right-rail/preview-artifact.tsx | 263 ++++++++ .../src/app/chat/right-rail/preview-file.tsx | 28 +- .../src/app/chat/right-rail/preview-pane.tsx | 16 +- .../src/app/chat/right-rail/preview.tsx | 93 +-- apps/desktop/src/app/contrib/controller.tsx | 27 +- apps/desktop/src/app/contrib/panes.tsx | 9 +- apps/desktop/src/app/contrib/wiring.tsx | 11 +- apps/desktop/src/app/right-sidebar/index.tsx | 4 +- .../app/right-sidebar/review/file-tree.tsx | 4 +- .../terminal/use-terminal-session.ts | 4 +- .../app/session/hooks/preview-open.test.tsx | 177 +++++ .../hooks/use-preview-routing.test.tsx | 184 ------ .../app/session/hooks/use-preview-routing.ts | 75 +-- .../components/assistant-ui/artifact-card.tsx | 17 +- .../markdown-text.artifacts.test.tsx | 5 +- apps/desktop/src/i18n/ar.ts | 5 +- apps/desktop/src/i18n/en.ts | 5 +- apps/desktop/src/i18n/ja.ts | 5 +- apps/desktop/src/i18n/types.ts | 5 +- apps/desktop/src/i18n/zh-hant.ts | 5 +- apps/desktop/src/i18n/zh.ts | 5 +- apps/desktop/src/store/artifacts.test.ts | 78 ++- apps/desktop/src/store/artifacts.ts | 227 +------ apps/desktop/src/store/gateway-switch.ts | 8 +- apps/desktop/src/store/layout.ts | 20 +- apps/desktop/src/store/preview.test.ts | 162 +++-- apps/desktop/src/store/preview.ts | 622 +++++------------- 33 files changed, 1000 insertions(+), 1569 deletions(-) delete mode 100644 apps/desktop/src/app/chat/right-rail/artifact-pane.tsx delete mode 100644 apps/desktop/src/app/chat/right-rail/artifact-renderers.tsx create mode 100644 apps/desktop/src/app/chat/right-rail/preview-artifact.test.tsx create mode 100644 apps/desktop/src/app/chat/right-rail/preview-artifact.tsx create mode 100644 apps/desktop/src/app/session/hooks/preview-open.test.tsx delete mode 100644 apps/desktop/src/app/session/hooks/use-preview-routing.test.tsx diff --git a/apps/desktop/src/app/chat/close-tab.test.ts b/apps/desktop/src/app/chat/close-tab.test.ts index 95847fd925c..44368c80403 100644 --- a/apps/desktop/src/app/chat/close-tab.test.ts +++ b/apps/desktop/src/app/chat/close-tab.test.ts @@ -1,14 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { $rightRailActiveTabId, RIGHT_RAIL_PREVIEW_TAB_ID } from '@/store/layout' -import { - $filePreviewTabs, - $previewTarget, - clearSessionPreviewRegistry, - type PreviewTarget, - setCurrentSessionPreviewTarget -} from '@/store/preview' -import { $activeSessionId, $selectedStoredSessionId } from '@/store/session' +import { $rightRailActiveTabId } from '@/store/layout' +import { $previewTabs, closeRightRail, openPreview, type PreviewTarget } from '@/store/preview' import { closeActiveTab } from './close-tab' @@ -26,40 +19,34 @@ function fileTarget(path: string): PreviewTarget { describe('closeActiveTab', () => { beforeEach(() => { vi.stubGlobal('document', { activeElement: null }) - $activeSessionId.set('session-1') - $selectedStoredSessionId.set(null) + closeRightRail() window.localStorage.clear() - clearSessionPreviewRegistry() }) afterEach(() => { vi.unstubAllGlobals() - $activeSessionId.set(null) - $selectedStoredSessionId.set(null) - clearSessionPreviewRegistry() + closeRightRail() window.localStorage.clear() }) it('closes the active file preview tab (⌘W happy path)', () => { - setCurrentSessionPreviewTarget(fileTarget('/work/notes.md'), 'manual') + openPreview(fileTarget('/work/notes.md'), 'manual') - expect($filePreviewTabs.get()).toHaveLength(1) + expect($previewTabs.get()).toHaveLength(1) expect($rightRailActiveTabId.get()).toBe('file:file:///work/notes.md') expect(closeActiveTab()).toBe(true) - expect($filePreviewTabs.get()).toHaveLength(0) + expect($previewTabs.get()).toHaveLength(0) }) - it('closes the visible file tab when active selection is a ghost preview', () => { - // Active tab id stuck on live-preview after that target was cleared, while - // file tabs remain (UI falls back to tabs[0] until React syncs). ⌘W must - // close the visible file tab instead of no-op'ing via closeWorkspaceTab(). - setCurrentSessionPreviewTarget(fileTarget('/work/notes.md'), 'manual') - $previewTarget.set(null) - $rightRailActiveTabId.set(RIGHT_RAIL_PREVIEW_TAB_ID) + it('closes the visible tab when the active selection points at a tab that is gone', () => { + // The rail falls back to tabs[0] until React syncs the selection, so ⌘W has + // to act on what is actually on screen rather than no-op'ing. + openPreview(fileTarget('/work/notes.md'), 'manual') + $rightRailActiveTabId.set('file:file:///work/stale.md') - expect($filePreviewTabs.get()).toHaveLength(1) + expect($previewTabs.get()).toHaveLength(1) expect(closeActiveTab()).toBe(true) - expect($filePreviewTabs.get()).toHaveLength(0) + expect($previewTabs.get()).toHaveLength(0) }) }) diff --git a/apps/desktop/src/app/chat/close-tab.ts b/apps/desktop/src/app/chat/close-tab.ts index b5e15dd30cf..bf8b2899870 100644 --- a/apps/desktop/src/app/chat/close-tab.ts +++ b/apps/desktop/src/app/chat/close-tab.ts @@ -1,8 +1,7 @@ import { closeActiveTerminal } from '@/app/right-sidebar/terminal/terminals' import { closeWorkspaceTab } from '@/components/pane-shell/tree/store' import { isFocusWithin } from '@/lib/keybinds/combo' -import { $artifactTabs } from '@/store/artifacts' -import { $filePreviewTabs, $previewTarget, closeActiveRightRailTab } from '@/store/preview' +import { $previewTabs, closeActiveRightRailTab } from '@/store/preview' import { closeSessionTile, nextSessionTileForWorkspace } from '@/store/session-states' /** @@ -28,12 +27,10 @@ export function closeActiveTab(loadSessionIntoWorkspace?: (storedSessionId: stri return true } - // Prefer tab *presence* over the derived active file target. After the live - // preview is cleared, `$rightRailActiveTabId` can stay on `preview` while - // file tabs remain (the rail UI falls back to tabs[0]). Gating only on - // `$filePreviewTarget` made ⌘W fall through to closeWorkspaceTab() and look - // broken with a file tab still on screen. - if ($previewTarget.get() || $filePreviewTabs.get().length > 0 || $artifactTabs.get().length > 0) { + // Gate on tab *presence*, not on the selection: a stale `$rightRailActiveTabId` + // would otherwise make ⌘W fall through to closeWorkspaceTab() and look broken + // with a tab still on screen. The store resolves which tab that is. + if ($previewTabs.get().length > 0) { return closeActiveRightRailTab() } diff --git a/apps/desktop/src/app/chat/composer/status-stack/preview-row.tsx b/apps/desktop/src/app/chat/composer/status-stack/preview-row.tsx index cf721d2ae93..dc40c31de2e 100644 --- a/apps/desktop/src/app/chat/composer/status-stack/preview-row.tsx +++ b/apps/desktop/src/app/chat/composer/status-stack/preview-row.tsx @@ -11,7 +11,7 @@ import { cn } from '@/lib/utils' import { PREVIEW_PANE_ID } from '@/store/layout' import { notifyError } from '@/store/notifications' import { $paneOpen } from '@/store/panes' -import { $previewTarget, dismissPreviewTarget, setCurrentSessionPreviewTarget } from '@/store/preview' +import { $previewTabSources, closePreviewForSource, openPreview } from '@/store/preview' import { type PreviewArtifact } from '@/store/preview-status' interface PreviewStatusRowProps { @@ -22,10 +22,10 @@ interface PreviewStatusRowProps { /** One detected artifact, single line, always visible: filename + open + close. */ export const PreviewStatusRow = memo(function PreviewStatusRow({ item, onDismiss }: PreviewStatusRowProps) { const { t } = useI18n() - const activePreview = useStore($previewTarget) + const openSources = useStore($previewTabSources) const previewPaneOpen = useStore($paneOpen(PREVIEW_PANE_ID)) const [opening, setOpening] = useState(false) - const isOpen = activePreview?.source === item.target && previewPaneOpen + const isOpen = openSources.includes(item.target) && previewPaneOpen const resolveTarget = async () => { const target = await normalizeOrLocalPreviewTarget(item.target, item.cwd || undefined) @@ -43,7 +43,7 @@ export const PreviewStatusRow = memo(function PreviewStatusRow({ item, onDismiss } if (isOpen) { - dismissPreviewTarget() + closePreviewForSource(item.target) return } @@ -51,7 +51,7 @@ export const PreviewStatusRow = memo(function PreviewStatusRow({ item, onDismiss setOpening(true) try { - setCurrentSessionPreviewTarget(await resolveTarget(), 'tool-result', item.target) + openPreview(await resolveTarget(), 'tool-result') } catch (error) { notifyError(error, t.preview.unavailable) } finally { diff --git a/apps/desktop/src/app/chat/right-rail/artifact-pane.tsx b/apps/desktop/src/app/chat/right-rail/artifact-pane.tsx deleted file mode 100644 index c463ee84c41..00000000000 --- a/apps/desktop/src/app/chat/right-rail/artifact-pane.tsx +++ /dev/null @@ -1,222 +0,0 @@ -import { useStore } from '@nanostores/react' -import { useEffect, useMemo, useState } from 'react' - -import { CopyButton } from '@/components/ui/copy-button' -import { Tip } from '@/components/ui/tooltip' -import { useI18n } from '@/i18n' -import { artifactDownloadName } from '@/lib/artifact-detect' -import { downloadTextFile } from '@/lib/download-text' -import { ChevronLeft, ChevronRight, Download, ExternalLink } from '@/lib/icons' -import { cn } from '@/lib/utils' -import { - $artifactRegistry, - $artifactVersionSelection, - type ArtifactRecord, - selectArtifactVersion -} from '@/store/artifacts' -import { notifyError } from '@/store/notifications' - -import { ArtifactLivePreview, ArtifactSourceView, composeArtifactHtml } from './artifact-renderers' -import { PreviewEmptyState } from './preview-file' - -type ArtifactViewMode = 'preview' | 'source' - -const MIME_BY_KIND = { code: 'text/plain', html: 'text/html', svg: 'image/svg+xml' } as const - -const HEADER_BUTTON_CLASS = - 'flex h-5 items-center gap-1 rounded-md px-1 text-[0.625rem] font-bold text-muted-foreground transition-colors hover:bg-accent hover:text-foreground disabled:pointer-events-none disabled:opacity-40' - -/** Write the composed document to a real temp file through the existing - * buffer-save IPC, then hand it to the OS browser. A blob/data URL can't - * cross into the OS default browser, so a file on disk is the honest path. */ -async function openHtmlInBrowser(content: string): Promise { - const bridge = window.hermesDesktop - - if (!bridge?.saveImageBuffer || !bridge.openExternal) { - throw new Error('Desktop bridge unavailable') - } - - const bytes = new TextEncoder().encode(composeArtifactHtml(content)) - const path = await bridge.saveImageBuffer(bytes, '.html') - - if (!path) { - throw new Error('Could not write artifact file') - } - - const fileUrl = `file://${path.startsWith('/') ? '' : '/'}${path.replace(/\\/g, '/')}` - - if (bridge.openPreviewInBrowser) { - await bridge.openPreviewInBrowser(fileUrl) - - return - } - - await bridge.openExternal(fileUrl) -} - -function VersionStepper({ - current, - onSelect, - total -}: { - current: number - onSelect: (index: number) => void - total: number -}) { - const { t } = useI18n() - const copy = t.artifactPane - - if (total < 2) { - return null - } - - return ( -
- - - - {copy.versionOf(current + 1, total)} - - - -
- ) -} - -export function ArtifactPane({ artifactId }: { artifactId: string }) { - const { t } = useI18n() - const copy = t.artifactPane - const registry = useStore($artifactRegistry) - const versionSelection = useStore($artifactVersionSelection) - // View mode is per-pane, ephemeral: renderable artifacts open in preview. - const [userMode, setUserMode] = useState(null) - - // Reset the explicit mode when the pane is reused for another artifact. - useEffect(() => { - setUserMode(null) - }, [artifactId]) - - const record = useMemo(() => { - for (const records of Object.values(registry)) { - const found = records.find(candidate => candidate.id === artifactId) - - if (found) { - return found - } - } - - return null - }, [artifactId, registry]) - - if (!record) { - return - } - - const isRenderable = record.kind === 'html' || record.kind === 'svg' - const versionIndex = Math.min(versionSelection[artifactId] ?? record.versions.length - 1, record.versions.length - 1) - const version = record.versions[versionIndex]! - const isCurrentVersion = versionIndex >= record.versions.length - 1 - const mode: ArtifactViewMode = isRenderable ? (userMode ?? 'preview') : 'source' - const downloadName = artifactDownloadName(record.kind, record.language, record.title) - - const modeLabel: Record = { - preview: copy.modePreview, - source: copy.modeSource - } - - return ( -
-
-
- selectArtifactVersion(artifactId, index)} - total={record.versions.length} - /> - {!isCurrentVersion && ( - - )} -
- {isRenderable && - (['preview', 'source'] as const).map(candidate => ( - - ))} -
- - - - - {record.kind === 'html' && window.hermesDesktop && ( - - - - )} -
-
-
- {mode === 'preview' && isRenderable ? ( - - ) : ( - - )} -
-
- ) -} diff --git a/apps/desktop/src/app/chat/right-rail/artifact-renderers.tsx b/apps/desktop/src/app/chat/right-rail/artifact-renderers.tsx deleted file mode 100644 index 00ed9a58a58..00000000000 --- a/apps/desktop/src/app/chat/right-rail/artifact-renderers.tsx +++ /dev/null @@ -1,107 +0,0 @@ -import DOMPurify from 'dompurify' -import { useMemo } from 'react' -import ShikiHighlighter from 'react-shiki' - -import { chunkTextLines, useFixedRowWindow } from '@/components/chat/fixed-row-window' -import type { ArtifactKind } from '@/lib/artifact-detect' - -const SHIKI_THEME = { dark: 'github-dark-default', light: 'github-light-default' } as const -const SOURCE_CHUNK_LINES = 200 -const SOURCE_LINE_PX = 20 -const SOURCE_OVERSCAN_LINES = 400 - -/** Windowed, Shiki-highlighted source view for artifact content. Same fixed-row - * windowing as the file preview's SourceView so a 5k-line artifact scrolls - * smoothly, minus the gutter drag/selection machinery (artifact content has no - * on-disk path to reference lines against). */ -export function ArtifactSourceView({ language, text }: { language: string; text: string }) { - const chunks = useMemo(() => chunkTextLines(text, SOURCE_CHUNK_LINES), [text]) - const lastChunk = chunks.at(-1) - const totalLines = lastChunk ? lastChunk.start + lastChunk.lines.length : 0 - - const { afterRows, beforeRows, endChunk, onScroll, scrollerRef, startChunk } = useFixedRowWindow({ - overscanRows: SOURCE_OVERSCAN_LINES, - rowPx: SOURCE_LINE_PX, - rowsPerChunk: SOURCE_CHUNK_LINES, - totalRows: totalLines - }) - - const visibleChunks = chunks.slice(startChunk, endChunk + 1) - - return ( -
-
- {beforeRows > 0 &&
} - {visibleChunks.map(chunk => ( -
- - {chunk.text} - -
- ))} - {afterRows > 0 &&
} -
-
- ) -} - -/** Wrap an HTML fragment in a minimal document shell; full documents pass - * through untouched. Keeps generated fragments (no /) rendering - * with sane defaults instead of quirks-mode soup. */ -export function composeArtifactHtml(content: string): string { - if (/]|', - '', - '', - content, - '' - ].join('\n') -} - -/** - * Sandboxed live renderer for html/svg artifact content. - * - * HTML runs in an `