From 510ba06c6656cf946c9e0ae6b47c2e894f365a8a Mon Sep 17 00:00:00 2001 From: Anton Frehser Date: Mon, 12 Oct 2026 17:29:38 +0200 Subject: [PATCH 1/2] Simplify workspace navigation and keep chat links on the current page --- client/features/applets/applet-runtime.ts | 13 +- client/features/chat/ChatPanel.tsx | 14 +- .../chat/messages/MarkdownContent.test.tsx | 3 +- .../chat/messages/MarkdownContent.tsx | 6 +- client/features/chat/sessions/api.ts | 54 ++- client/features/chat/sessions/chat-link.ts | 37 -- .../chat/sessions/useSelectedSession.ts | 5 +- client/features/collab/hooks.test.tsx | 2 +- client/features/collab/provider.tsx | 6 +- .../features/collab/workspace-users-menu.tsx | 2 +- client/features/home/HomePage.tsx | 4 +- client/features/workspace/WorkspaceScreen.tsx | 4 +- .../features/workspace/browser-tab-state.ts | 25 +- .../features/workspace/tab-resolution.test.ts | 22 +- client/features/workspace/tab-resolution.ts | 10 +- .../use-workspace-navigation.dom.test.tsx | 327 ++++++++++++++--- .../workspace/useWorkspaceNavigation.test.tsx | 180 +++++++--- .../workspace/useWorkspaceNavigation.ts | 329 +++++++----------- docs/navigation.md | 15 +- lib/navigation.test.ts | 148 +++----- lib/navigation.ts | 151 +++----- lib/workspace-tabs.test.ts | 4 +- lib/workspace-tabs.ts | 13 +- server/cli.ts | 6 +- server/control.ts | 14 +- server/tabs.ts | 5 +- server/test/applet-url.test.ts | 14 + server/test/navigation-cli.test.ts | 15 +- .../.claude/skills/moi-workspace/SKILL.md | 5 +- 29 files changed, 818 insertions(+), 615 deletions(-) delete mode 100644 client/features/chat/sessions/chat-link.ts diff --git a/client/features/applets/applet-runtime.ts b/client/features/applets/applet-runtime.ts index 6614c732..0041538f 100644 --- a/client/features/applets/applet-runtime.ts +++ b/client/features/applets/applet-runtime.ts @@ -37,7 +37,7 @@ import { toast } from '@/client/components/ui/toast' import { createRateLimiter, type RateLimiter } from '@/client/lib/rate-limit' import { useLatestRef } from '@/client/lib/use-latest-ref' import { isParamsRecord } from '@/lib/workspace-tabs' -import { resolveUrl, workspacePath } from '@/lib/navigation' +import { resolveUrl } from '@/lib/navigation' type HostAppletBridge = SharedAppletBridge & { readonly collab: AppletCollabApi | undefined @@ -127,10 +127,6 @@ function createRuntime(workspaceId: string) { connect(identity: AppletIdentity, base = '') { let alive = true const source = appletSource(identity) - const urlContext = { - apiBase: `/api/workspaces/${encodeURIComponent(workspaceId)}`, - workspacePath: workspacePath(workspaceId, base) - } const bridge: HostAppletBridge = { get collab() { return alive ? appletCollabApi : undefined @@ -149,17 +145,14 @@ function createRuntime(workspaceId: string) { navigate(url) { if (!alive) return try { - if (typeof url !== 'string') throw new Error('Navigation needs a URL') - resolveUrl(url, urlContext) - emitter.emit('navigate', url) + emitter.emit('navigate', resolveUrl(url)) } catch (error) { drop(identity, `navigate() was dropped: ${errorMessage(error)}`) } }, resolveUrl(url) { if (!alive) return '' - if (typeof url !== 'string') throw new Error('A URL is required') - return resolveUrl(url, urlContext) + return resolveUrl(url, { workspaceId, base }) }, sendChatMessage(input, legacyContext) { if (!alive) return diff --git a/client/features/chat/ChatPanel.tsx b/client/features/chat/ChatPanel.tsx index 19c33278..6d4e094d 100644 --- a/client/features/chat/ChatPanel.tsx +++ b/client/features/chat/ChatPanel.tsx @@ -1,9 +1,9 @@ import { useCallback, useEffect, useLayoutEffect, useMemo, useRef } from 'react' -import { Link } from 'wouter' +import { useRouter } from 'wouter' import { IconArrowBackUp, IconChevronDown, IconChevronsRight, IconX } from '@tabler/icons-react' import { draftSessionId } from '@/lib/session-drafts' -import { addressPath } from '@/lib/navigation' +import { resolveUrl } from '@/lib/navigation' import { useCurrentTabId } from './sessions/useSelectedSession' import { canSubmitComposerAction, focusComposer } from '@/client/components/shared/Composer' @@ -91,6 +91,7 @@ export function ChatPanel({ viewDraft }: ChatPanelProps) { const workspaceId = useWorkspaceId() + const { base } = useRouter() const { data: sessions } = useWorkspaceSessions(workspaceId) const sourceName = sessions?.find(session => session.sessionId === forkedFromSessionId)?.summary const tabId = useCurrentTabId() @@ -204,8 +205,11 @@ export function ChatPanel({ >
{forkedFromSessionId && ( -
@@ -222,7 +226,7 @@ export function ChatPanel({
- + )} {showEmptyState && ( - + ) expect(html).toContain('href="/prefix/workspace/abc/views/events?eventId=123"') expect(html).toContain('href="https://example.com/"') expect(html).toContain('href="/prefix/workspace/abc/chats/chat-1"') + expect(html).toContain('href="/prefix/workspace/abc/chat"') }) test('invalid moi links and executable URLs stay sanitized, including images', () => { diff --git a/client/features/chat/messages/MarkdownContent.tsx b/client/features/chat/messages/MarkdownContent.tsx index 6f907c64..91699edb 100644 --- a/client/features/chat/messages/MarkdownContent.tsx +++ b/client/features/chat/messages/MarkdownContent.tsx @@ -6,7 +6,7 @@ import rehypeHighlight from 'rehype-highlight' import remarkGfm from 'remark-gfm' import { useWorkspaceId } from '@/client/features/workspace/WorkspaceContext' -import { resolveUrl, workspacePath } from '@/lib/navigation' +import { resolveUrl } from '@/lib/navigation' import { cn } from '@/client/lib/cn' const remarkPlugins = [remarkGfm] @@ -64,8 +64,8 @@ export function MarkdownContent({ size = 'sm', content }: MarkdownContentProps) if (node.tagName === 'a' && key === 'href' && url.startsWith('moi:')) { try { return resolveUrl(url, { - apiBase: `/api/workspaces/${encodeURIComponent(workspaceId)}`, - workspacePath: workspacePath(workspaceId, base) + workspaceId, + base }) } catch { return '' diff --git a/client/features/chat/sessions/api.ts b/client/features/chat/sessions/api.ts index 7c8a0fa2..6f47cc62 100644 --- a/client/features/chat/sessions/api.ts +++ b/client/features/chat/sessions/api.ts @@ -1,17 +1,27 @@ import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' import type { QueryClient } from '@tanstack/react-query' +import { useCallback } from 'react' import { jsonRequest, requestJson, requestVoid } from '@/client/api/http' import { WORKSPACE_RESOURCE_OPTIONS } from '@/client/api/query-options' import { workspaceKeys } from '@/client/api/workspace-keys' import { sessionViewOptions } from '@/client/features/chat/sessions/session-view' import { readSelectedSession, writeSelectedSession } from './browser-tab-state' -import { selectedSessionKey } from './useSelectedSession' +import { + selectedSessionKey, + selectedSessionOptions, + usePinnedSession, + useSelectedSession +} from './useSelectedSession' +import { useCollabEnabled } from '@/client/features/collab' +import { useLatestRef } from '@/client/lib/use-latest-ref' +import { useWorkspaceId } from '@/client/features/workspace/WorkspaceContext' import type { SessionConfig, SessionInfo, SessionRecord, - WorkspaceSessionSelection + WorkspaceSessionSelection, + WorkspaceTabId } from '@/lib/types' export function workspaceSessionsOptions(workspaceId: string) { @@ -26,6 +36,46 @@ export function useWorkspaceSessions(workspaceId: string) { return useQuery(workspaceSessionsOptions(workspaceId)) } +// Select the linked chat on the destination tab, unpinning a different chat first. +export function useSelectLinkedChat() { + const workspaceId = useWorkspaceId() + const queryClient = useQueryClient() + const collabEnabled = useCollabEnabled() + const [, , selectSessionForTab] = useSelectedSession() + const { pinAsync } = usePinnedSession() + const stateRef = useLatestRef({ collabEnabled, selectSessionForTab, pinAsync }) + // Stable so selection updates do not restart a pending chat link. + return useCallback( + async (sessionId: string, tab: WorkspaceTabId, signal: AbortSignal): Promise => { + const { collabEnabled, selectSessionForTab, pinAsync } = stateRef.current + signal.throwIfAborted() + let sessions = + queryClient.getQueryData(workspaceKeys.sessions(workspaceId)) ?? [] + if (!sessions.some(session => session.sessionId === sessionId)) + sessions = await queryClient.fetchQuery(workspaceSessionsOptions(workspaceId)) + signal.throwIfAborted() + if (!sessions.some(session => session.sessionId === sessionId)) return false + await queryClient.ensureQueryData(selectedSessionOptions(workspaceId)) + if (collabEnabled) + await queryClient.ensureQueryData(selectedSessionOptions(workspaceId, 'browser-tab')) + signal.throwIfAborted() + const pinned = queryClient.getQueryData( + selectedSessionKey(workspaceId) + )?.pinned + if (pinned && pinned !== sessionId) { + await pinAsync(null) + signal.throwIfAborted() + } + if (pinned !== sessionId) { + await selectSessionForTab(sessionId, tab) + signal.throwIfAborted() + } + return true + }, + [queryClient, stateRef, workspaceId] + ) +} + export function removeArchivedSession( sessions: SessionInfo[] | undefined, sessionId: string diff --git a/client/features/chat/sessions/chat-link.ts b/client/features/chat/sessions/chat-link.ts deleted file mode 100644 index 4256c539..00000000 --- a/client/features/chat/sessions/chat-link.ts +++ /dev/null @@ -1,37 +0,0 @@ -import type { QueryClient } from '@tanstack/react-query' -import { workspaceKeys } from '@/client/api/workspace-keys' -import { workspaceSessionsOptions } from './api' -import { tabAvailable } from '@/client/features/workspace/tab-resolution' -import { isWorkspaceTabId } from '@/lib/workspace-tabs' -import type { SessionInfo, ViewInfo, WorkspaceTabId } from '@/lib/types' - -// Metadata alone does not establish existence: the sessions API combines the -// harness's live chat list with moi's attribution records. -export async function resolveChatLink( - queryClient: QueryClient, - workspaceId: string, - sessionId: string, - views: ViewInfo[], - signal: AbortSignal -): Promise { - signal.throwIfAborted() - let session = queryClient - .getQueryData(workspaceKeys.sessions(workspaceId)) - ?.find(session => session.sessionId === sessionId) - if (!session) { - const sessions = await queryClient.fetchQuery(workspaceSessionsOptions(workspaceId)) - session = sessions.find(session => session.sessionId === sessionId) - } - signal.throwIfAborted() - if (!session) return null - const tab = isWorkspaceTabId(session.tabId) ? session.tabId : 'overview' - if (!tabAvailable(tab, views)) { - await queryClient.refetchQueries( - { queryKey: workspaceKeys.views(workspaceId), exact: true }, - { throwOnError: true } - ) - signal.throwIfAborted() - views = queryClient.getQueryData(workspaceKeys.views(workspaceId)) ?? views - } - return tabAvailable(tab, views) ? tab : null -} diff --git a/client/features/chat/sessions/useSelectedSession.ts b/client/features/chat/sessions/useSelectedSession.ts index 2560cdff..8cca59a5 100644 --- a/client/features/chat/sessions/useSelectedSession.ts +++ b/client/features/chat/sessions/useSelectedSession.ts @@ -5,7 +5,7 @@ import { } from '@/client/features/chat/sessions/browser-tab-state' import { useCallback, useMemo } from 'react' import { useParams } from 'wouter' -import { tabFromPath } from '@/lib/navigation' +import { isWorkspaceTabId } from '@/lib/workspace-tabs' import type { WorkspaceSessionSelection, WorkspaceTabId } from '@/lib/types' import { composerDraftKey } from '@/lib/session-drafts' import { useUiStore } from '@/client/store/ui' @@ -166,7 +166,8 @@ export function renameSelectedSessionInCache( } export function useCurrentTabId(): WorkspaceTabId { - return tabFromPath(useParams()['*'] ?? '') ?? 'overview' + const tab = useParams()['*'] + return isWorkspaceTabId(tab) ? tab : 'overview' } export function selectedSessionForTab( diff --git a/client/features/collab/hooks.test.tsx b/client/features/collab/hooks.test.tsx index 91a39b36..ac1adf00 100644 --- a/client/features/collab/hooks.test.tsx +++ b/client/features/collab/hooks.test.tsx @@ -25,7 +25,7 @@ test('presence pages follow nested navigation paths and deployment bases, decodi 'views/board' ) expect(pageFromPath('/workspace/test/views/%2562oard', 'test')).toBe('views/%2562oard') - expect(pageFromPath('/workspace/test/view:board', 'test')).toBe('views/board') + expect(pageFromPath('/workspace/test/view:board', 'test')).toBe('view:board') expect(pageFromPath('/workspace/test', 'test')).toBe('overview') }) diff --git a/client/features/collab/provider.tsx b/client/features/collab/provider.tsx index ce4d4c63..db64f636 100644 --- a/client/features/collab/provider.tsx +++ b/client/features/collab/provider.tsx @@ -3,7 +3,7 @@ import type { ReactNode } from 'react' import { useRouter } from 'wouter' import { usePathname } from 'wouter/use-browser-location' -import { legacyTabFromPath, tabFromPath, workspacePath } from '@/lib/navigation' +import { tabFromPath } from '@/lib/workspace-tabs' import { CollabClient, NO_ENGINE } from './client' import type { CollabEngineApi } from './client' @@ -13,9 +13,9 @@ export const CollabContext = createContext(NO_ENGINE) const AppletPresenceContext = createContext(null) export function pageFromPath(path: string, workspaceId: string, base = ''): string { - const prefix = workspacePath(workspaceId, base) + '/' + const prefix = `${base.replace(/\/$/, '')}/workspace/${encodeURIComponent(workspaceId)}/` const page = path.startsWith(prefix) ? path.slice(prefix.length) : '' - return tabFromPath(page) ?? legacyTabFromPath(page) ?? (page || 'overview') + return tabFromPath(page) ?? (page || 'overview') } export type CollabProviderProps = { diff --git a/client/features/collab/workspace-users-menu.tsx b/client/features/collab/workspace-users-menu.tsx index 1d34d56b..260c4c4b 100644 --- a/client/features/collab/workspace-users-menu.tsx +++ b/client/features/collab/workspace-users-menu.tsx @@ -14,7 +14,7 @@ import { } from '@/client/components/ui/popover' import { cn } from '@/client/lib/cn' import type { WorkspaceTabId } from '@/lib/types' -import { tabFromPath } from '@/lib/navigation' +import { tabFromPath } from '@/lib/workspace-tabs' import { User } from './components/user' import { UserAvatarGroup } from './components/user-avatar-group' diff --git a/client/features/home/HomePage.tsx b/client/features/home/HomePage.tsx index c99ab07a..060cc4fc 100644 --- a/client/features/home/HomePage.tsx +++ b/client/features/home/HomePage.tsx @@ -34,7 +34,7 @@ import { } from '@/client/features/home/workspace-presentation' import { resolveWorkspaceTheme } from '@/lib/themes' import type { DiscoveredWorkspace, WorkspaceEntry } from '@/lib/types' -import { workspaceTabPath } from '@/lib/navigation' +import { resolveUrl } from '@/lib/navigation' import { WorkspacePreview } from './WorkspacePreview' @@ -198,7 +198,7 @@ function WorkspaceCard({ workspace }: WorkspaceCardProps) { { + const revealChat = useCallback(() => { setFloatingChatOpen(true) setChatFocusRequest(request => request + 1) }, []) @@ -174,7 +174,7 @@ export function WorkspaceScreen({ widgets, views: allViews }: WorkspaceScreenPro onNavigationClick } = useWorkspaceNavigation({ views: allViews, - onOpenChat: revealLinkedChat + onOpenChat: revealChat }) // Chat comes after navigation, and takes the address: every message carries diff --git a/client/features/workspace/browser-tab-state.ts b/client/features/workspace/browser-tab-state.ts index d318f64f..9f2c0d86 100644 --- a/client/features/workspace/browser-tab-state.ts +++ b/client/features/workspace/browser-tab-state.ts @@ -1,7 +1,9 @@ -import { useCallback, useState } from 'react' +import { useCallback, useMemo, useState } from 'react' import type { WorkspaceTabsState } from '@/lib/types' import { normalizeWorkspaceTabs } from '@/lib/workspace-layout' +import { useCollabEnabled } from '@/client/features/collab' +import { useWorkspaceLayoutCtx } from './WorkspaceLayoutContext' const keyFor = (workspaceId: string) => `moi:collab:${workspaceId}:tabs` @@ -25,20 +27,25 @@ export function writeWorkspaceTabs(workspaceId: string, tabs: WorkspaceTabsState } } -export function useWorkspaceTabs( - workspaceId: string, - enabled: boolean, - defaults: WorkspaceTabsState -) { +export function useWorkspaceTabs() { + const { workspaceId, layout, setLayout } = useWorkspaceLayoutCtx() + const enabled = useCollabEnabled() + const defaults = useMemo(() => normalizeWorkspaceTabs(layout.tabs), [layout.tabs]) const [local, setLocal] = useState(() => enabled ? readWorkspaceTabs(workspaceId, defaults) : defaults ) const setTabs = useCallback( (tabs: WorkspaceTabsState) => { - setLocal(tabs) - writeWorkspaceTabs(workspaceId, tabs) + const next = normalizeWorkspaceTabs(tabs) + if (enabled) { + setLocal(next) + writeWorkspaceTabs(workspaceId, next) + } else { + setLayout({ tabs: next }) + } + return next }, - [workspaceId] + [enabled, setLayout, workspaceId] ) return [enabled ? local : defaults, setTabs] as const } diff --git a/client/features/workspace/tab-resolution.test.ts b/client/features/workspace/tab-resolution.test.ts index 62ad911d..e6af4637 100644 --- a/client/features/workspace/tab-resolution.test.ts +++ b/client/features/workspace/tab-resolution.test.ts @@ -1,13 +1,9 @@ import { describe, expect, test } from 'bun:test' import type { ViewInfo, WorkspaceTabsState } from '@/lib/types' +import { normalizeWorkspaceTabs } from '@/lib/workspace-layout' -import { - effectiveOpenTabs, - normalizeTabsState, - resolveActiveTab, - tabAvailable -} from './tab-resolution' +import { effectiveOpenTabs, resolveActiveTab, tabAvailable } from './tab-resolution' const views: ViewInfo[] = [ { id: 'orders', status: 'compiled', title: 'Orders' }, @@ -23,13 +19,13 @@ const tabs = (open: WorkspaceTabsState['open'], active: WorkspaceTabsState['acti active }) -describe('normalizeTabsState', () => { +describe('normalizeWorkspaceTabs', () => { test('falls back to defaults on missing/empty state', () => { - expect(normalizeTabsState(undefined)).toEqual({ + expect(normalizeWorkspaceTabs(undefined)).toEqual({ open: ['overview', 'scratchpad'], active: 'overview' }) - expect(normalizeTabsState(tabs([], 'scratchpad'))).toEqual({ + expect(normalizeWorkspaceTabs(tabs([], 'scratchpad'))).toEqual({ open: ['overview', 'scratchpad'], active: 'overview' }) @@ -37,7 +33,7 @@ describe('normalizeTabsState', () => { test('pins Overview first, dedupes open, and preserves a valid active tab', () => { expect( - normalizeTabsState(tabs(['scratchpad', 'overview', 'scratchpad'], 'scratchpad')) + normalizeWorkspaceTabs(tabs(['scratchpad', 'overview', 'scratchpad'], 'scratchpad')) ).toEqual({ open: ['overview', 'scratchpad'], active: 'scratchpad' @@ -77,7 +73,7 @@ describe('resolveActiveTab', () => { const state = tabs(['overview', 'scratchpad', 'views/orders'], 'overview') test('a bare URL resolves to the saved default', () => { - expect(resolveActiveTab(null, state, views)).toBe('overview') + expect(resolveActiveTab(undefined, state, views)).toBe('overview') }) test('a valid URL tab wins, even when not in the open set', () => { @@ -91,11 +87,11 @@ describe('resolveActiveTab', () => { test('an unavailable saved default falls back to the first surviving tab', () => { const stale = tabs(['views/gone', 'views/orders'], 'views/gone') - expect(resolveActiveTab(null, stale, views)).toBe('views/orders') + expect(resolveActiveTab(undefined, stale, views)).toBe('views/orders') }) test('when nothing survives, the default open set answers', () => { const dead = tabs(['views/gone'], 'views/gone') - expect(resolveActiveTab(null, dead, [])).toBe('overview') + expect(resolveActiveTab(undefined, dead, [])).toBe('overview') }) }) diff --git a/client/features/workspace/tab-resolution.ts b/client/features/workspace/tab-resolution.ts index 11c73dfa..47897b1c 100644 --- a/client/features/workspace/tab-resolution.ts +++ b/client/features/workspace/tab-resolution.ts @@ -4,14 +4,10 @@ // turn (URL segment + layout + what actually exists) into the rendered state. import type { ViewInfo, WorkspaceTabId, WorkspaceTabsState } from '@/lib/types' import { viewIdFromTab } from '@/lib/workspace-tabs' -import { createDefaultWorkspaceTabs, normalizeWorkspaceTabs } from '@/lib/workspace-layout' +import { createDefaultWorkspaceTabs } from '@/lib/workspace-layout' const DEFAULT_TABS = createDefaultWorkspaceTabs() -export function normalizeTabsState(tabs: WorkspaceTabsState | undefined): WorkspaceTabsState { - return normalizeWorkspaceTabs(tabs) -} - // Whether a tab id points at something that exists: static tabs always do, // view tabs only while their pending or compiled view exists. export function tabAvailable(tab: WorkspaceTabId, views: ViewInfo[]) { @@ -31,11 +27,11 @@ export function effectiveOpenTabs(tabs: WorkspaceTabsState, views: ViewInfo[]): // can show recovery and report the correct address to the agent. A bare URL // uses the saved default. export function resolveActiveTab( - requested: WorkspaceTabId | null, + requested: WorkspaceTabId | undefined, tabs: WorkspaceTabsState, views: ViewInfo[] ): WorkspaceTabId { - if (requested !== null) { + if (requested !== undefined) { return requested } const open = effectiveOpenTabs(tabs, views) diff --git a/client/features/workspace/use-workspace-navigation.dom.test.tsx b/client/features/workspace/use-workspace-navigation.dom.test.tsx index e09d818b..11404a63 100644 --- a/client/features/workspace/use-workspace-navigation.dom.test.tsx +++ b/client/features/workspace/use-workspace-navigation.dom.test.tsx @@ -11,9 +11,14 @@ import { CollabContext } from '@/client/features/collab/provider' import { NO_ENGINE } from '@/client/features/collab/client' import { createFakeEngine } from '@/client/features/collab/testing/fake-engine' import * as workspaceEvents from '@/client/runtime/useWorkspaceEvents' -import { addressPath } from '@/lib/navigation' +import { resolveUrl } from '@/lib/navigation' import { createDefaultWorkspaceLayout } from '@/lib/workspace-layout' -import type { WorkspaceSessionSelection } from '@/lib/types' +import type { + SessionInfo, + WorkspaceSessionSelection, + WorkspaceTabId, + WorkspaceTabsState +} from '@/lib/types' import { Workspace } from './WorkspaceContext' import { WorkspaceLayoutContext } from './WorkspaceLayoutContext' import { useWorkspaceNavigation } from './useWorkspaceNavigation' @@ -21,12 +26,17 @@ import { useWorkspaceNavigation } from './useWorkspaceNavigation' let cleanup: () => Promise = async () => {} afterEach(() => cleanup()) -function mountNavigation(path: string, collab: boolean) { +function mountNavigation( + path: string, + collab: boolean, + tabs: WorkspaceTabsState = { open: ['overview', 'scratchpad'], active: 'scratchpad' } +) { const { window, document } = parseHTML('
') const location = new URL(`http://localhost/prefix/workspace/abc/${path}`) Object.defineProperty(window, 'location', { configurable: true, value: location }) const storage = new Map() const entries: { kind: 'push' | 'replace'; href: string }[] = [] + const pages = [location.href] const globals = { window, document, @@ -39,11 +49,18 @@ function mountNavigation(path: string, collab: boolean) { history: { pushState: (_state: unknown, _title: string, href: string) => { entries.push({ kind: 'push', href }) + pages.push(new URL(href, location).href) move(href) }, replaceState: (_state: unknown, _title: string, href: string) => { entries.push({ kind: 'replace', href }) + pages[pages.length - 1] = new URL(href, location).href move(href) + }, + back: () => { + if (pages.length <= 1) return + pages.pop() + move(pages.at(-1)!) } }, sessionStorage: { @@ -67,14 +84,21 @@ function mountNavigation(path: string, collab: boolean) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }) let selection: WorkspaceSessionSelection = { selected: { overview: 'previous' }, pinned: 'other' } - queryClient.setQueryData(workspaceKeys.sessions('abc'), [ - { sessionId: 'source', summary: 'Source', lastModified: 1, tabId: 'scratchpad' } - ]) + const responses: { sessions: SessionInfo[] | Promise } = { + sessions: [{ sessionId: 'source', summary: 'Source', lastModified: 1, tabId: 'scratchpad' }] + } + queryClient.setQueryData(workspaceKeys.sessions('abc'), responses.sessions) queryClient.setQueryData(selectedSessionKey('abc'), selection) + if (collab) + queryClient.setQueryData(selectedSessionKey('abc', 'browser-tab'), { + selected: {}, + pinned: null + }) const fetchSpy = spyOn(globalThis, 'fetch').mockImplementation( Object.assign( async (input: RequestInfo | URL, init?: RequestInit) => { const url = String(input) + if (url.endsWith('/sessions')) return Response.json(await responses.sessions) if (url.endsWith('/selected-session') && !init?.method) return Response.json(selection) const body = JSON.parse(String(init?.body)) as { sessionId: string; tabId: string } if (url.endsWith('/pinned-session')) { @@ -93,31 +117,71 @@ function mountNavigation(path: string, collab: boolean) { { preconnect: fetch.preconnect } ) ) - const navigationSpy = spyOn(workspaceEvents, 'useNavigationClient').mockImplementation(() => {}) + let navigate: (href: string) => void | Promise = () => { + throw new Error('Navigation controller was not registered') + } + const navigationSpy = spyOn(workspaceEvents, 'useNavigationClient').mockImplementation( + (_workspaceId, callback) => { + navigate = callback + } + ) const eventsSpy = spyOn(workspaceEvents, 'useWorkspaceEvent').mockImplementation(() => {}) function Probe() { const [revealed, setRevealed] = useState(false) - const { activeTab, onNavigationClick } = useWorkspaceNavigation({ - views: [], + const { activeTab, onNavigationClick, navigateToTab } = useWorkspaceNavigation({ + views: [{ id: 'orders', status: 'compiled', title: 'Orders' }], onOpenChat: () => setRevealed(true) }) return (
- + Started from another chat - + Chat link + + Open chat + + + Open current chat + + + {activeTab} {revealed ? 'yes' : 'no'}
) } + function Layout() { + const [layout, setLayout] = useState({ ...createDefaultWorkspaceLayout(), tabs }) + return ( + setLayout(current => ({ ...current, ...patch })), + name: null, + cwd: null, + provider: null, + workspaceId: 'abc', + isLoading: false + }} + > + + + ) + } const root = createRoot(document.getElementById('root')!) cleanup = async () => { await act(async () => { @@ -151,19 +215,7 @@ function mountNavigation(path: string, collab: boolean) { - {}, - name: null, - cwd: null, - provider: null, - workspaceId: 'abc', - isLoading: false - }} - > - - + @@ -171,13 +223,17 @@ function mountNavigation(path: string, collab: boolean) { ) }) - const waitForNavigation = async () => { + const waitForReveal = async () => { for (let i = 0; i < 50 && document.getElementById('revealed')?.textContent !== 'yes'; i++) await act(async () => { await Bun.sleep(10) }) - expect(location.pathname).toBe('/prefix/workspace/abc/scratchpad') - expect(document.getElementById('active-tab')?.textContent).toBe('scratchpad') + expect(document.getElementById('revealed')?.textContent).toBe('yes') + } + const waitForNavigation = async (tab: WorkspaceTabId = 'scratchpad') => { + await waitForReveal() + expect(location.pathname).toBe(`/prefix/workspace/abc/${tab}`) + expect(document.getElementById('active-tab')?.textContent).toBe(tab) expect(document.getElementById('revealed')?.textContent).toBe('yes') expect( queryClient.getQueryData(selectedSessionKey('abc'))?.pinned @@ -185,28 +241,142 @@ function mountNavigation(path: string, collab: boolean) { expect( queryClient.getQueryData( selectedSessionKey('abc', collab ? 'browser-tab' : 'shared') - )?.selected.scratchpad + )?.selected[tab] ).toBe('source') } - return { render, document, window, waitForNavigation, history: entries } + return { + render, + document, + window, + waitForNavigation, + waitForReveal, + queryClient, + fetchSpy, + responses, + storage, + navigate: (href: string) => Promise.resolve(navigate(href)), + history: entries + } } for (const collab of [false, true]) { describe(`mounted chat links with Collab ${collab ? 'enabled' : 'disabled'}`, () => { - test('a direct chat URL selects and reveals its chat through the route effect', async () => { - const fixture = mountNavigation('chats/source', collab) + test.each(['overview', 'scratchpad?after=1'])( + 'a route change to %s cancels a pending chat lookup', + async path => { + const fixture = mountNavigation('scratchpad?before=1', collab) + await fixture.render() + const lookup = Promise.withResolvers() + fixture.responses.sessions = lookup.promise + const opening = fixture.navigate('moi:/chats/new') + await act(async () => { + fixture.window.history.pushState(null, '', `/prefix/workspace/abc/${path}`) + }) + lookup.resolve([{ sessionId: 'new', summary: 'New', lastModified: 1 }]) + await act(async () => { + await expect(opening).rejects.toMatchObject({ name: 'AbortError' }) + }) + expect(fixture.window.location.pathname + fixture.window.location.search).toBe( + `/prefix/workspace/abc/${path}` + ) + expect(fixture.document.getElementById('revealed')?.textContent).toBe('no') + expect(fixture.fetchSpy.mock.calls.filter(([, init]) => init?.method === 'PUT')).toEqual([]) + } + ) + test.each(['chat', 'chats/source'])( + 'navigation immediately followed by %s keeps the new page and parameters', + async path => { + const fixture = mountNavigation('scratchpad', collab) + await fixture.render() + await act(async () => { + const switching = fixture.navigate('moi:/views/orders?order=o-3') + const opening = fixture.navigate(`moi:/${path}`) + await switching + await opening + }) + await fixture.waitForReveal() + expect(fixture.window.location.pathname).toBe('/prefix/workspace/abc/views/orders') + expect(new URLSearchParams(fixture.window.location.search).get('order')).toBe('o-3') + if (path.startsWith('chats/')) await fixture.waitForNavigation('views/orders') + } + ) + test.each(['chats', 'chats/%', 'chats/a/b'])( + 'invalid chat route %s restores the saved tab without changing selection or pin', + async path => { + const fixture = mountNavigation(path, collab) + if (collab) + fixture.queryClient.setQueryData(selectedSessionKey('abc', 'browser-tab'), { + selected: {}, + pinned: null + }) + await fixture.render() + expect(fixture.window.location.pathname).toBe('/prefix/workspace/abc/scratchpad') + expect(fixture.document.getElementById('revealed')?.textContent).toBe('no') + expect( + fixture.queryClient.getQueryData(selectedSessionKey('abc')) + ).toEqual({ + selected: { overview: 'previous' }, + pinned: 'other' + }) + expect(fixture.fetchSpy.mock.calls.filter(([, init]) => init?.method === 'PUT')).toEqual([]) + } + ) + test('tab navigation restores params and avoids a duplicate history entry', async () => { + const fixture = mountNavigation('views/orders?order=o-1&literal=%252F', collab) await fixture.render() - await fixture.waitForNavigation() - // A direct load has no origin page, so the entry route is replaced in place. - expect(fixture.history).toEqual([ - { kind: 'replace', href: '/prefix/workspace/abc/scratchpad' } - ]) + const clickTab = (tab: string) => + act(async () => { + const click = new fixture.window.Event('click', { bubbles: true, cancelable: true }) + Object.assign(click, { button: 0 }) + fixture.document.querySelector(`button[data-tab="${tab}"]`)!.dispatchEvent(click) + }) + await clickTab('scratchpad') + expect(fixture.window.location.pathname).toBe('/prefix/workspace/abc/scratchpad') + await clickTab('orders') + expect(fixture.window.location.pathname).toBe('/prefix/workspace/abc/views/orders') + const params = new URLSearchParams(fixture.window.location.search) + expect(params.get('order')).toBe('o-1') + expect(params.get('literal')).toBe('%2F') + await clickTab('orders') + expect(fixture.history.filter(entry => entry.kind === 'push')).toHaveLength(2) + const savedTabs = fixture.storage.get('moi:collab:abc:tabs') + if (collab) expect(JSON.parse(savedTabs!).active).toBe('views/orders') + else expect(savedTabs).toBeUndefined() }) - test.each(['source', 'native'])( - 'clicking a %s chat link selects and reveals its chat', + test.each([null, 'other'])( + 'a direct current-chat URL restores the saved tab and preserves pin %s and selection', + async pinned => { + const fixture = mountNavigation('chat', collab) + const selection = { selected: { overview: 'previous' }, pinned } + const local = { selected: { overview: 'local' }, pinned: null } + fixture.queryClient.setQueryData(selectedSessionKey('abc'), selection) + if (collab) + fixture.queryClient.setQueryData(selectedSessionKey('abc', 'browser-tab'), local) + await fixture.render() + await fixture.waitForReveal() + expect(fixture.window.location.pathname).toBe('/prefix/workspace/abc/scratchpad') + expect(fixture.document.getElementById('active-tab')?.textContent).toBe('scratchpad') + expect(fixture.history).toEqual([ + { kind: 'replace', href: '/prefix/workspace/abc/scratchpad' } + ]) + expect( + fixture.queryClient.getQueryData(selectedSessionKey('abc')) + ).toEqual(selection) + if (collab) + expect( + fixture.queryClient.getQueryData( + selectedSessionKey('abc', 'browser-tab') + ) + ).toEqual(local) + expect(fixture.fetchSpy.mock.calls.filter(([, init]) => init?.method === 'PUT')).toEqual([]) + } + ) + + test.each(['chat', 'native-chat'])( + 'clicking a %s link stays on the current tab and reveals the current chat', async kind => { - const fixture = mountNavigation('overview', collab) + const fixture = mountNavigation('scratchpad', collab) await fixture.render() const click = new fixture.window.Event('click', { bubbles: true, cancelable: true }) Object.assign(click, { button: 0 }) @@ -214,14 +384,85 @@ for (const collab of [false, true]) { fixture.document.querySelector(`a[data-link="${kind}"]`)!.dispatchEvent(click) }) expect(click.defaultPrevented).toBe(true) + await fixture.waitForReveal() + expect(fixture.window.location.pathname).toBe('/prefix/workspace/abc/scratchpad') + expect(fixture.history.filter(entry => entry.kind === 'push')).toHaveLength( + kind === 'native-chat' ? 0 : 1 + ) + expect( + fixture.queryClient.getQueryData(selectedSessionKey('abc')) + ).toEqual({ + selected: { overview: 'previous' }, + pinned: 'other' + }) + await act(async () => fixture.window.history.back()) + expect(fixture.window.location.pathname).toBe('/prefix/workspace/abc/scratchpad') + } + ) + + test.each(['chats/source', 'chats/%73ource'])( + 'a direct chat URL %s selects and reveals its chat through the route effect', + async path => { + const fixture = mountNavigation(path, collab) + await fixture.render() await fixture.waitForNavigation() - // One net history entry: the pushed chat route is replaced by its home - // tab, so Back returns to the page the link was clicked on. + // A direct load has no origin page, so the entry route is replaced in place. expect(fixture.history).toEqual([ - { kind: 'push', href: '/prefix/workspace/abc/chats/source' }, { kind: 'replace', href: '/prefix/workspace/abc/scratchpad' } ]) } ) + + test.each(['chat', 'chats/source'])( + 'a direct %s URL restores the last view regardless of the chat home tab', + async path => { + const fixture = mountNavigation(path, collab, { + open: ['overview', 'scratchpad', 'views/orders'], + active: 'views/orders' + }) + await fixture.render() + await fixture.waitForReveal() + expect(fixture.window.location.pathname).toBe('/prefix/workspace/abc/views/orders') + expect(fixture.document.getElementById('active-tab')?.textContent).toBe('views/orders') + if (path.startsWith('chats/')) await fixture.waitForNavigation('views/orders') + } + ) + + test.each(['chat', 'native-chat', 'source', 'native'])( + 'clicking a %s link preserves the current view and its parameters', + async kind => { + const fixture = mountNavigation('views/orders?order=o-2&literal=%252F', collab) + await fixture.render() + await act(async () => { + const click = new fixture.window.Event('click', { bubbles: true, cancelable: true }) + Object.assign(click, { button: 0 }) + fixture.document.querySelector(`a[data-link="${kind}"]`)!.dispatchEvent(click) + }) + await fixture.waitForReveal() + expect(fixture.window.location.pathname).toBe('/prefix/workspace/abc/views/orders') + const params = new URLSearchParams(fixture.window.location.search) + expect(params.get('order')).toBe('o-2') + expect(params.get('literal')).toBe('%2F') + if (kind === 'source' || kind === 'native') await fixture.waitForNavigation('views/orders') + } + ) + + test.each(['source', 'native'])( + 'clicking a %s chat link selects and reveals its chat', + async kind => { + const fixture = mountNavigation('overview', collab) + await fixture.render() + const click = new fixture.window.Event('click', { bubbles: true, cancelable: true }) + Object.assign(click, { button: 0 }) + await act(async () => { + fixture.document.querySelector(`a[data-link="${kind}"]`)!.dispatchEvent(click) + }) + expect(click.defaultPrevented).toBe(true) + await fixture.waitForNavigation('overview') + expect(fixture.history.filter(entry => entry.kind === 'push')).toHaveLength(0) + await act(async () => fixture.window.history.back()) + expect(fixture.window.location.pathname).toBe('/prefix/workspace/abc/overview') + } + ) }) } diff --git a/client/features/workspace/useWorkspaceNavigation.test.tsx b/client/features/workspace/useWorkspaceNavigation.test.tsx index 252c4491..e2da75c4 100644 --- a/client/features/workspace/useWorkspaceNavigation.test.tsx +++ b/client/features/workspace/useWorkspaceNavigation.test.tsx @@ -8,8 +8,17 @@ import { Workspace } from './WorkspaceContext' import { CollabContext } from '@/client/features/collab/provider' import { NO_ENGINE } from '@/client/features/collab/client' import { createFakeEngine } from '@/client/features/collab/testing/fake-engine' -import { selectedSessionKey } from '@/client/features/chat/sessions/useSelectedSession' -import type { SessionInfo, ViewInfo, WorkspaceSessionSelection, WorkspaceTabId } from '@/lib/types' +import { + selectedSessionKey, + useCurrentTabId +} from '@/client/features/chat/sessions/useSelectedSession' +import type { + SessionInfo, + ViewInfo, + WorkspaceSessionSelection, + WorkspaceTabId, + WorkspaceTabsState +} from '@/lib/types' import { createDefaultWorkspaceLayout } from '@/lib/workspace-layout' import * as workspaceEvents from '@/client/runtime/useWorkspaceEvents' import { WorkspaceLayoutContext } from './WorkspaceLayoutContext' @@ -21,6 +30,7 @@ type ReadNavigationOptions = { queryClient?: QueryClient onOpenChat?: () => void collab?: boolean + tabs?: WorkspaceTabsState } function readNavigation( @@ -32,7 +42,8 @@ function readNavigation( navigate, queryClient = new QueryClient(), onOpenChat = () => {}, - collab = false + collab = false, + tabs = createDefaultWorkspaceLayout().tabs }: ReadNavigationOptions = {} ) { const queryKey = workspaceKeys.views('abc') @@ -73,7 +84,7 @@ function readNavigation( {}, name: null, cwd: null, @@ -105,6 +116,15 @@ test('view params decode exactly once through the real router', () => { expect(result.appletParams).toEqual(params) }) +test('view params keep the first repeated value and ordinary object keys', () => { + const result = readNavigation('views/events', 'x=1&x=2&__proto__=safe', [ + { id: 'events', status: 'compiled', title: 'Events' } + ]) + expect(result.appletParams.x).toBe('1') + expect(Object.hasOwn(result.appletParams, '__proto__')).toBe(true) + expect(result.appletParams.__proto__).toBe('safe') +}) + test('encoded IDs resolve under a deployment base without decoding nested escapes', () => { const views: ViewInfo[] = [{ id: 'events', status: 'compiled', title: 'events' }] expect(readNavigation('views/%65vents', '', views, { base: '/prefix' }).activeTab).toBe( @@ -115,6 +135,23 @@ test('encoded IDs resolve under a deployment base without decoding nested escape ) }) +test.each([ + ['views/%65vents', 'views/events'], + ['views/%2565vents', 'overview'] +])('current tab decodes the route %s once', (path, expected) => { + function CurrentTab() { + return {useCurrentTabId()} + } + const html = renderToStaticMarkup( + + + + + + ) + expect(html).toBe(`${expected}`) +}) + test('missing views keep their destination without becoming the default tab', () => { const result = readNavigation('views/missing', '?eventId=123', []) expect(result.activeTab).toBe('views/missing') @@ -123,15 +160,16 @@ test('missing views keep their destination without becoming the default tab', () }) test('chat entry routes never render the unavailable page or become tabs', () => { + expect(readNavigation('chat', '', []).isUnavailable).toBe(false) + expect(readNavigation('chat', '', []).activeTab).toBe('overview') expect(readNavigation('chats/%61bc', '', []).isUnavailable).toBe(false) expect(readNavigation('chats/missing', '', []).activeTab).toBe('overview') }) -test('legacy browser paths select the same view under a deployment base', () => { +test('retired browser paths show recovery under a deployment base', () => { const views: ViewInfo[] = [{ id: 'events', status: 'compiled', title: 'events' }] const result = readNavigation('view:%65vents', 'eventId=123', views, { base: '/prefix' }) - expect(result.activeTab).toBe('views/events') - expect(result.isUnavailable).toBe(false) + expect(result.isUnavailable).toBe(true) expect(result.appletParams).toEqual({ eventId: '123' }) expect(readNavigation('view:%2565vents', '', views).isUnavailable).toBe(true) }) @@ -177,6 +215,8 @@ test.each(['', '/', '/prefix'])( }, queryClient }) + await navigate('moi:/overview') + expect(routedPaths).toEqual([]) await navigate('moi:/views/orders?order=o-1') await navigate('moi:/files/clips/a%20b.mp4?version=2#t=5') await navigate('https://example.com/') @@ -205,6 +245,15 @@ test.each(['', '/', '/prefix'])( `${prefix}/workspace/abc/views/orders?order=o-1`, `${prefix}/workspace/abc/views/new` ]) + + const laterRefresh = Promise.withResolvers() + freshViews = laterRefresh.promise + const cancelled = navigate('moi:/views/later') + await navigate('moi:/scratchpad') + laterRefresh.resolve([...views, { id: 'later', status: 'draft', requirements: '' }]) + await expect(Promise.resolve(cancelled)).rejects.toMatchObject({ name: 'AbortError' }) + expect(routedPaths.at(-1)).toBe(`${prefix}/workspace/abc/scratchpad`) + expect(routedPaths).not.toContain(`${prefix}/workspace/abc/views/later`) } finally { registration.mockRestore() if (windowDescriptor) Object.defineProperty(globalThis, 'window', windowDescriptor) @@ -250,8 +299,8 @@ function createChatNavigationFixture(collab: boolean) { configurable: true, value: { location: { - href: 'http://localhost/prefix/workspace/abc/views/source', - pathname: '/prefix/workspace/abc/views/source', + href: 'http://localhost/prefix/workspace/abc/scratchpad', + pathname: '/prefix/workspace/abc/scratchpad', search: '', hash: '' } @@ -296,12 +345,13 @@ function createChatNavigationFixture(collab: boolean) { navigate = callback } ) - readNavigation('overview', '', views, { + readNavigation('scratchpad', '', views, { base: '/prefix', navigate: (path, options) => routed.push({ path, ...options }), queryClient, onOpenChat: () => revealed.push(true), - collab + collab, + tabs: { open: ['overview', 'scratchpad'], active: 'scratchpad' } }) return { queryClient, @@ -334,15 +384,44 @@ for (const collab of [false, true]) { afterEach(() => fixture.cleanup()) test.each([ - ['Overview', 'general', 'overview', null], - ['Scratchpad', 'notes', 'scratchpad', null], - ['a closed compiled view and unpins another chat', 'orders', 'views/orders', 'other'], - ['a pending view and preserves the requested pin', 'draft', 'views/draft', 'draft'] - ] as const)('opens %s', async (_name, id, tab, pinned) => { + { selected: { overview: 'previous' }, pinned: null }, + { selected: { overview: 'previous' }, pinned: 'other' }, + { selected: {}, pinned: null } + ] satisfies WorkspaceSessionSelection[])( + 'opens the current chat without changing selection %j', + async selection => { + const { queryClient, navigate, routed, revealed, writes, storage } = fixture + queryClient.setQueryData(selectedSessionKey('abc'), selection) + const local = { selected: { overview: 'local' }, pinned: null } + if (collab) queryClient.setQueryData(selectedSessionKey('abc', 'browser-tab'), local) + await navigate('moi:/chat') + expect(routed).toEqual([]) + expect(revealed).toEqual([true]) + expect( + queryClient.getQueryData(selectedSessionKey('abc')) + ).toEqual(selection) + if (collab) + expect( + queryClient.getQueryData( + selectedSessionKey('abc', 'browser-tab') + ) + ).toEqual(local) + expect(writes).toEqual([]) + expect(storage.size).toBe(0) + } + ) + + test.each([ + ['a general chat', 'general', null], + ['a Scratchpad chat', 'notes', null], + ['a view chat and unpins another chat', 'orders', 'other'], + ['a pending view chat and preserves its pin', 'draft', 'draft'], + ['a chat whose original view was removed', 'gone', null] + ] as const)('opens %s on the current tab', async (_name, id, pinned) => { const { queryClient, selection, navigate, routed, revealed, writes, storage } = fixture queryClient.setQueryData(selectedSessionKey('abc'), { ...selection, pinned }) await navigate(`moi:/chats/${id}`) - expect(routed).toEqual([{ path: `/prefix/workspace/abc/${tab}`, replace: false }]) + expect(routed).toEqual([]) expect(revealed).toEqual([true]) expect( queryClient.getQueryData(selectedSessionKey('abc'))?.pinned @@ -354,13 +433,14 @@ for (const collab of [false, true]) { const scope = collab ? 'browser-tab' : 'shared' expect( queryClient.getQueryData(selectedSessionKey('abc', scope)) - ?.selected[tab] + ?.selected.scratchpad ).toBe(id) expect(writes.map(write => write.url)).toEqual([ ...(pinned ? ['/api/workspaces/abc/pinned-session'] : []), ...(collab ? [] : ['/api/workspaces/abc/selected-session']) ]) - if (collab) expect(JSON.parse(storage.get('moi:collab:abc:session')!).selected[tab]).toBe(id) + if (collab) + expect(JSON.parse(storage.get('moi:collab:abc:session')!).selected.scratchpad).toBe(id) }) test('waits for pin state on a direct load before selecting', async () => { @@ -380,19 +460,19 @@ for (const collab of [false, true]) { expect(revealed).toEqual([true]) }) - test('opens chats with persisted Agent tab attribution on Overview', async () => { + test('opens chats with persisted Agent tab attribution on the current tab', async () => { const { queryClient, sessions, navigate, routed, revealed } = fixture queryClient.setQueryData(workspaceKeys.sessions('abc'), [ ...sessions, { sessionId: 'legacy', summary: 'Legacy', lastModified: 1, tabId: 'agent' } ]) await navigate('moi:/chats/legacy') - expect(routed).toEqual([{ path: '/prefix/workspace/abc/overview', replace: false }]) + expect(routed).toEqual([]) expect(revealed).toEqual([true]) expect( queryClient.getQueryData( selectedSessionKey('abc', collab ? 'browser-tab' : 'shared') - )?.selected.overview + )?.selected.scratchpad ).toBe('legacy') }) @@ -422,7 +502,7 @@ for (const collab of [false, true]) { expect(revealed).toEqual([]) saving.resolve(Response.json({ sessionId: 'general' })) await opening - expect(routed).toEqual([{ path: '/prefix/workspace/abc/overview', replace: false }]) + expect(routed).toEqual([]) expect(revealed).toEqual([true]) }) @@ -435,41 +515,43 @@ for (const collab of [false, true]) { }) } - test.each(['missing', 'gone'])( - 'falls back for %s without changing selection or pin', - async id => { - const { queryClient, selection, navigate, writes, routed, revealed } = fixture - const original = { ...selection, pinned: 'pin' } - queryClient.setQueryData(selectedSessionKey('abc'), original) - await navigate(`moi:/chats/${id}`) - expect(routed).toEqual([{ path: '/prefix/workspace/abc/overview', replace: false }]) - expect( - queryClient.getQueryData(selectedSessionKey('abc')) - ).toEqual(original) - expect(writes).toEqual([]) - expect(revealed).toEqual([]) - } - ) - - test('navigation away cancels a pending lookup without changing selection or pin', async () => { - const { queryClient, responses, sessions, selection, navigate, writes, routed, revealed } = - fixture + test('a missing chat keeps the current tab, selection, and pin', async () => { + const { queryClient, selection, navigate, writes, routed, revealed } = fixture const original = { ...selection, pinned: 'pin' } queryClient.setQueryData(selectedSessionKey('abc'), original) - const refresh = Promise.withResolvers() - responses.sessions = refresh.promise - const opening = navigate('moi:/chats/new') - await navigate('moi:/scratchpad') - refresh.resolve([...sessions, { sessionId: 'new', summary: 'New', lastModified: 2 }]) - await expect(opening).rejects.toMatchObject({ name: 'AbortError' }) + await navigate('moi:/chats/missing') + expect(routed).toEqual([]) expect( queryClient.getQueryData(selectedSessionKey('abc')) ).toEqual(original) expect(writes).toEqual([]) expect(revealed).toEqual([]) - expect(routed.at(-1)?.path).toBe('/prefix/workspace/abc/scratchpad') }) + test.each(['moi:/overview', 'moi:/chat'])( + 'navigation to %s cancels a pending lookup without changing selection or pin', + async href => { + const { queryClient, responses, sessions, selection, navigate, writes, routed, revealed } = + fixture + const original = { ...selection, pinned: 'pin' } + queryClient.setQueryData(selectedSessionKey('abc'), original) + const refresh = Promise.withResolvers() + responses.sessions = refresh.promise + const opening = navigate('moi:/chats/new') + await navigate(href) + refresh.resolve([...sessions, { sessionId: 'new', summary: 'New', lastModified: 2 }]) + await expect(opening).rejects.toMatchObject({ name: 'AbortError' }) + expect( + queryClient.getQueryData(selectedSessionKey('abc')) + ).toEqual(original) + expect(writes).toEqual([]) + expect(revealed).toEqual(href === 'moi:/chat' ? [true] : []) + expect(routed).toEqual( + href === 'moi:/chat' ? [] : [{ path: '/prefix/workspace/abc/overview', replace: false }] + ) + } + ) + test('refreshes a missing cached chat before falling back', async () => { const { responses, sessions, navigate, revealed } = fixture responses.sessions = Promise.resolve([ diff --git a/client/features/workspace/useWorkspaceNavigation.ts b/client/features/workspace/useWorkspaceNavigation.ts index 992ae31a..0a3bd5aa 100644 --- a/client/features/workspace/useWorkspaceNavigation.ts +++ b/client/features/workspace/useWorkspaceNavigation.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useRef } from 'react' +import { useCallback, useEffect, useLayoutEffect, useMemo, useRef } from 'react' import type { MouseEvent } from 'react' import { useQueryClient } from '@tanstack/react-query' import { useLocation, useRouter } from 'wouter' @@ -8,39 +8,29 @@ import { toast } from '@/client/components/ui/toast' import { workspaceKeys } from '@/client/api/workspace-keys' import { reportAppletError } from '@/client/features/applets/applet-log' import { useAppletEvent } from '@/client/features/applets/applet-runtime' -import { useCollabEnabled } from '@/client/features/collab' import { useWorkspaceTabs } from '@/client/features/workspace/browser-tab-state' -import { normalizeTabsState, resolveActiveTab, tabAvailable } from './tab-resolution' +import { resolveActiveTab, tabAvailable } from './tab-resolution' import { useWorkspaceLayoutCtx } from './WorkspaceLayoutContext' -import { useLatestRef } from '@/client/lib/use-latest-ref' import { useNavigationClient } from '@/client/runtime/useWorkspaceEvents' -import { - usePinnedSession, - useSelectedSession, - selectedSessionKey, - selectedSessionOptions -} from '@/client/features/chat/sessions/useSelectedSession' -import { resolveChatLink } from '@/client/features/chat/sessions/chat-link' -import type { WorkspaceSessionSelection } from '@/lib/types' +import { useSelectLinkedChat } from '@/client/features/chat/sessions/api' import type { ViewInfo, WorkspaceTabId, WorkspaceTabsState } from '@/lib/types' -import { - addressPath, - canonicalSearch, - chatSessionIdFromPath, - legacyTabFromPath, - parseMoiHref, - readViewParams, - resolveUrl, - tabFromPath, - workspacePath -} from '@/lib/navigation' +import { resolveUrl } from '@/lib/navigation' +import { isWorkspaceTabId, tabFromPath } from '@/lib/workspace-tabs' type NavigationOptions = { replace?: boolean } -type OpenLinkedChatOptions = { - controller?: AbortController - // The chat route is an intermediate entry and gets replaced. Imperative - // requests push like view links, so Back returns to the origin page. - replace?: boolean +function canonicalSearch(search: string): string { + const params = new URLSearchParams(search) + params.sort() + const result = params.toString() + return result ? `?${result}` : '' +} + +function browserAddress(): string { + return window.location.pathname + canonicalSearch(window.location.search) +} + +function tabUrl(remembered: ReadonlyMap, tab: WorkspaceTabId): string { + return remembered.get(tab) ?? `moi:/${tab}` } // A convenience for returning to tabs, never a second source of active state. @@ -52,161 +42,120 @@ type UseWorkspaceNavigationOptions = { onOpenChat: () => void } -type UseOpenChatLinkOptions = { - workspaceId: string - views: ViewInfo[] -} - -// A chat link needs state prepared before its URL is valid: the chat becomes -// the selection on its home tab, and a different pinned chat is unpinned -// first. Resolves to that tab, or null when the chat or its tab is missing. -// Navigation stays with the caller. -export function useOpenChatLink({ workspaceId, views }: UseOpenChatLinkOptions) { - const queryClient = useQueryClient() - const collabEnabled = useCollabEnabled() - const [, , selectSessionForTab] = useSelectedSession() - const { pinAsync } = usePinnedSession() - const open = useLatestRef( - async (sessionId: string, signal: AbortSignal): Promise => { - const tab = await resolveChatLink(queryClient, workspaceId, sessionId, views, signal) - if (!tab) return null - // Direct loads can resolve the chat before selection and pin state load. - await queryClient.ensureQueryData(selectedSessionOptions(workspaceId)) - if (collabEnabled) - await queryClient.ensureQueryData(selectedSessionOptions(workspaceId, 'browser-tab')) - signal.throwIfAborted() - const pinned = queryClient.getQueryData( - selectedSessionKey(workspaceId) - )?.pinned - if (pinned && pinned !== sessionId) { - await pinAsync(null) - signal.throwIfAborted() - } - if (pinned !== sessionId) { - await selectSessionForTab(sessionId, tab) - signal.throwIfAborted() - } - return tab - } - ) - // Stable so a route effect does not restart a link when views or selection change. - return useCallback( - (sessionId: string, signal: AbortSignal) => open.current(sessionId, signal), - [open] - ) -} - export function useWorkspaceNavigation({ views, onOpenChat }: UseWorkspaceNavigationOptions) { const queryClient = useQueryClient() - const { layout, setLayout, workspaceId } = useWorkspaceLayoutCtx() - const collabEnabled = useCollabEnabled() - const openChat = useOpenChatLink({ workspaceId, views }) - const chatRequest = useRef(null) - const onOpenChatRef = useLatestRef(onOpenChat) - const [, navigate] = useLocation() + const { workspaceId } = useWorkspaceLayoutCtx() + const [tabsState, saveTabs] = useWorkspaceTabs() + const latest = useRef({ views, tabsState, onOpenChat }) + useLayoutEffect(() => { + latest.current = { views, tabsState, onOpenChat } + }, [views, tabsState, onOpenChat]) + const selectLinkedChat = useSelectLinkedChat() + const navigationRequest = useRef<{ controller: AbortController; address: string } | null>(null) + const [, setLocation] = useLocation() const router = useRouter() const { base } = router // wouter's public route/search hooks decode URI escapes already. Read raw // browser values so tabFromPath and URLSearchParams each decode only once. - const path = usePathname(router).slice(workspacePath(workspaceId, base).length + 1) + const prefix = `${base.replace(/\/$/, '')}/workspace/${encodeURIComponent(workspaceId)}/` + const path = usePathname(router).slice(prefix.length) const search = canonicalSearch(useSearch(router)) - const appletParams = useMemo(() => readViewParams(search), [search]) - const [workspaceTabs, setLocalTabs] = useWorkspaceTabs( - workspaceId, - collabEnabled, - normalizeTabsState(layout.tabs) - ) - const tabsState = normalizeTabsState(workspaceTabs) - const tabsStateRef = useLatestRef(tabsState) + const appletParams = useMemo(() => { + const params = new URLSearchParams(search) + return Object.fromEntries(Array.from(params.keys(), key => [key, params.get(key)!])) + }, [search]) const remembered = useMemo(() => { let entries = rememberedAddresses.get(workspaceId) if (!entries) rememberedAddresses.set(workspaceId, (entries = new Map())) return entries }, [workspaceId]) const requestedTab = tabFromPath(path) - const isChatLink = path === 'chats' || path.startsWith('chats/') - const linkedSessionId = chatSessionIdFromPath(path) - const legacyTab = requestedTab ? null : legacyTabFromPath(path) - const activeTab = resolveActiveTab(requestedTab ?? legacyTab, tabsState, views) + const isChatLink = path === 'chat' || path === 'chats' || path.startsWith('chats/') + const activeTab = resolveActiveTab(requestedTab, tabsState, views) const isUnavailable = - Boolean(path) && - !isChatLink && - !legacyTab && - (!requestedTab || !tabAvailable(requestedTab, views)) - const honored = requestedTab === activeTab && !isUnavailable + Boolean(path) && !isChatLink && (!requestedTab || !tabAvailable(requestedTab, views)) + const isAvailableTab = requestedTab !== undefined && !isUnavailable const setTabs = useCallback( (tabs: WorkspaceTabsState) => { - tabsStateRef.current = tabs - if (collabEnabled) setLocalTabs(tabs) - else setLayout({ tabs }) - }, - [setLayout, tabsStateRef, collabEnabled, setLocalTabs] - ) - - const go = useCallback( - (target: string, options: NavigationOptions = {}) => { - chatRequest.current?.abort() - const current = window.location.pathname + canonicalSearch(window.location.search) - const absolute = `${base}${target}` - if (current !== absolute || window.location.hash) navigate(target, options) + // Keep same-turn tab changes visible to navigation. + latest.current = { ...latest.current, tabsState: saveTabs(tabs) } }, - [base, navigate] + [latest, saveTabs] ) - const navigateToTab = useCallback( - (tab: WorkspaceTabId, options: NavigationOptions = {}) => { - go(remembered.get(tab) ?? addressPath(workspaceId, { tab, search: '' }), options) - }, - [go, remembered, workspaceId] - ) + const currentPage = useCallback(() => { + const currentTab = tabFromPath(window.location.pathname.slice(prefix.length)) + const { tabsState, views } = latest.current + const tab = currentTab ?? resolveActiveTab(undefined, tabsState, views) + const href = currentTab ? `moi:/${tab}${window.location.search}` : tabUrl(remembered, tab) + return { tab, href } + }, [latest, prefix, remembered]) - // Navigation owns the link's lifetime: one request at a time, cancelled by - // any navigation or route change. The chat hook prepares state; this turns - // its answer into the home tab's URL and reveals the chat. - const openLinkedChat = useCallback( + const navigate = useCallback( async ( - sessionId: string, - { controller = new AbortController(), replace = false }: OpenLinkedChatOptions = {} + href: string, + { replace = false }: NavigationOptions = {}, + controller = new AbortController() ) => { - chatRequest.current?.abort() - chatRequest.current = controller - try { - const tab = await openChat(sessionId, controller.signal) + const target = resolveUrl(href, { workspaceId, base }) + navigationRequest.current?.controller.abort() + if (!target.startsWith(prefix)) { + navigationRequest.current = null controller.signal.throwIfAborted() - // Finished before the redirect, which cancels whatever is still in flight. - chatRequest.current = null - go(addressPath(workspaceId, { tab: tab ?? 'overview', search: '' }), { replace }) - if (tab) onOpenChatRef.current() - } finally { - if (chatRequest.current === controller) chatRequest.current = null - } - }, - [go, onOpenChatRef, openChat, workspaceId] - ) - - useEffect(() => () => chatRequest.current?.abort(), [path, search, workspaceId]) - - const navigateHref = useCallback( - async (href: string, availableViews = views) => { - if (href.startsWith('moi:/files/') || !href.startsWith('moi:')) { - window.location.assign( - resolveUrl(href, { apiBase: `/api/workspaces/${encodeURIComponent(workspaceId)}` }) - ) + window.location.assign(target) return } - const address = parseMoiHref(href) - if ('sessionId' in address) { - await openLinkedChat(address.sessionId) - return + navigationRequest.current = { + controller, + address: browserAddress() + } + const commit = (url: string) => { + controller.signal.throwIfAborted() + navigationRequest.current = null + if (browserAddress() !== url || window.location.hash) setLocation(`~${url}`, { replace }) + } + try { + const targetPath = target.slice(prefix.length).split('?', 1)[0] + if (targetPath === 'chat' || targetPath.startsWith('chats/')) { + const { tab, href } = currentPage() + const shouldOpenChat = + targetPath === 'chat' + ? true + : await selectLinkedChat( + decodeURIComponent(targetPath.slice('chats/'.length)), + tab, + controller.signal + ) + commit(resolveUrl(href, { workspaceId, base })) + if (shouldOpenChat) latest.current.onOpenChat() + return + } + + if (!isWorkspaceTabId(targetPath)) + throw new Error('This destination is unavailable in this workspace') + const queryKey = workspaceKeys.views(workspaceId) + let availableViews = queryClient.getQueryData(queryKey) ?? latest.current.views + if (!tabAvailable(targetPath, availableViews)) { + await queryClient.refetchQueries({ queryKey, exact: true }, { throwOnError: true }) + availableViews = queryClient.getQueryData(queryKey) ?? latest.current.views + } + if (!tabAvailable(targetPath, availableViews)) + throw new Error('This destination is unavailable in this workspace') + commit(target) + } finally { + if (navigationRequest.current?.controller === controller) navigationRequest.current = null } - if (!tabAvailable(address.tab, availableViews)) - throw new Error('This destination is unavailable in this workspace') - go(addressPath(workspaceId, address)) }, - [go, openLinkedChat, views, workspaceId] + [base, currentPage, latest, prefix, queryClient, selectLinkedChat, setLocation, workspaceId] ) + useEffect(() => { + const request = navigationRequest.current + if (request && request.address !== browserAddress()) request.controller.abort() + }, [path, search, workspaceId]) + useEffect(() => () => navigationRequest.current?.controller.abort(), []) + const reportError = useCallback( (error: unknown) => { if (error instanceof Error && error.name === 'AbortError') return @@ -217,50 +166,42 @@ export function useWorkspaceNavigation({ views, onOpenChat }: UseWorkspaceNaviga [workspaceId] ) + const navigateToTab = useCallback( + (tab: WorkspaceTabId, options: NavigationOptions = {}) => { + void navigate(tabUrl(remembered, tab), options).catch(reportError) + }, + [navigate, remembered, reportError] + ) + useAppletEvent(workspaceId, 'navigate', href => { - void navigateHref(href).catch(reportError) - }) - useNavigationClient(workspaceId, async href => { - const queryKey = workspaceKeys.views(workspaceId) - if (href.startsWith('moi:/views/')) { - const address = parseMoiHref(href) - if ('tab' in address && !tabAvailable(address.tab, views)) - await queryClient.refetchQueries({ queryKey, exact: true }, { throwOnError: true }) - } - await navigateHref(href, queryClient.getQueryData(queryKey) ?? views) + void navigate(href).catch(reportError) }) + useNavigationClient(workspaceId, navigate) useEffect(() => { - if (!isChatLink) return - if (!linkedSessionId) { - go(addressPath(workspaceId, { tab: 'overview', search: '' }), { replace: true }) - return + if (path && !isChatLink) return + let href = tabUrl(remembered, activeTab) + if (isChatLink) { + try { + href = resolveUrl(`moi:/${path}`) + } catch { + // Invalid chat links restore the saved tab without selecting a chat. + } } const controller = new AbortController() - void openLinkedChat(linkedSessionId, { controller, replace: true }).catch(error => { + void navigate(href, { replace: true }, controller).catch(error => { if (!controller.signal.aborted) reportError(error) }) return () => controller.abort() - }, [go, isChatLink, linkedSessionId, openLinkedChat, path, reportError, search, workspaceId]) - - // Bare workspace URLs and old view bookmarks - // redirect separately from exact chat links. Missing views show recovery. - useEffect(() => { - if (isUnavailable) return - if (legacyTab) { - go(addressPath(workspaceId, { tab: legacyTab, search }), { replace: true }) - } else if (!path) { - navigateToTab(activeTab, { replace: true }) - } - }, [activeTab, go, legacyTab, navigateToTab, path, search, isUnavailable, workspaceId]) + }, [activeTab, isChatLink, navigate, path, remembered, reportError, search]) useEffect(() => { - if (!honored) return - remembered.set(activeTab, addressPath(workspaceId, { tab: activeTab, search })) - const current = tabsStateRef.current + if (!isAvailableTab) return + remembered.set(activeTab, `moi:/${activeTab}${search}`) + const current = latest.current.tabsState const open = current.open.includes(activeTab) ? current.open : [...current.open, activeTab] if (open !== current.open || current.active !== activeTab) setTabs({ open, active: activeTab }) - }, [activeTab, honored, remembered, search, setTabs, tabsStateRef, workspaceId]) + }, [activeTab, isAvailableTab, latest, remembered, search, setTabs]) // React bubbling includes applet/chat portals. Native modified clicks and // downloads keep a real href, so the browser can handle them normally. @@ -283,29 +224,13 @@ export function useWorkspaceNavigation({ views, onOpenChat }: UseWorkspaceNaviga ) return const url = new URL(anchor.href) - const prefix = workspacePath(workspaceId, base) + '/' if (url.origin !== window.location.origin || !url.pathname.startsWith(prefix)) return // In-page fragments continue using native browser behavior. if (url.hash) return - const tab = tabFromPath(url.pathname.slice(prefix.length)) - const sessionId = chatSessionIdFromPath(url.pathname.slice(prefix.length)) - if (!tab && !sessionId) return event.preventDefault() - if (sessionId) { - // Entering the route gives in-app links normal Back behavior; the - // resolver then replaces this intermediate address with the home tab. - go(url.pathname.slice(base.length) + url.search) - return - } - try { - if (!tabAvailable(tab!, views)) - throw new Error('This destination is unavailable in this workspace') - go(addressPath(workspaceId, { tab: tab!, search: canonicalSearch(url.search) })) - } catch (error) { - reportError(error) - } + void navigate(`moi:/${url.pathname.slice(prefix.length)}${url.search}`).catch(reportError) }, - [base, go, reportError, views, workspaceId] + [navigate, prefix, reportError] ) return { diff --git a/docs/navigation.md b/docs/navigation.md index a6f11560..3ec33ecd 100644 --- a/docs/navigation.md +++ b/docs/navigation.md @@ -6,8 +6,8 @@ moi links point to places in the current workspace: `moi:/views/events?eventId=123` becomes `/workspace//views/events?eventId=123`. moi fills in the workspace ID and server address. Use `moi:/` with one slash. -The browser URL sets the open tab and passes values to views. All ways of opening a workspace -link follow the same rules. +The browser URL sets the open tab and passes values to views. The app and `moi.resolveUrl` +use the same resolver in `lib/navigation.ts` for workspace links, files, and web addresses. Where links are used: @@ -23,16 +23,17 @@ Where links are used: Query values are not passed to widgets or built-in UI. - **Views:** `moi:/views/events?eventId=123` opens the `events` view. Query values become its component's `params` prop: `{ eventId: '123' }`. -- **Chats:** `moi:/chats/` selects an existing chat, opens its home tab, and - reveals the chat panel. The URL becomes the home tab's plain URL; query values are not - forwarded to the view. +- **Chats:** `moi:/chat` reveals or focuses the current chat, preserving selection and pin. + `moi:/chats/` selects an existing chat and reveals the chat panel. Both use + the workspace's last opened tab, like a bare workspace URL, and restore its remembered + view parameters. Query values on chat links are ignored. - **Files:** `moi:/files/clips/video.mp4` opens a file relative to the workspace root through `/api/workspaces//files/clips/video.mp4`. Query strings and fragments stay on the file URL. - **Web addresses:** `https://example.com` (or HTTP) opens an ordinary web address. Its query values go to that destination. -Chats without tab attribution use Overview. Missing chats or missing home tabs fall back to -Overview without changing selection. Opening a different chat unpins the current chat. +Missing chats restore the last opened tab without changing selection. Opening a different chat +unpins the current chat. Share the `moi:/chats/` link to identify an exact chat; the resulting address bar URL identifies only its tab. ## View parameters diff --git a/lib/navigation.test.ts b/lib/navigation.test.ts index 7483fdce..9830e223 100644 --- a/lib/navigation.test.ts +++ b/lib/navigation.test.ts @@ -1,70 +1,44 @@ import { describe, expect, test } from 'bun:test' -import { - addressPath, - canonicalSearch, - chatSessionIdFromPath, - destinationHref, - legacyTabFromPath, - parseMoiHref, - readViewParams, - resolveUrl, - tabFromPath, - workspaceTabPath -} from './navigation' +import { resolveUrl } from './navigation' -const context = { apiBase: '/api/workspaces/abc', workspacePath: '/workspace/abc' } +const context = { workspaceId: 'abc' } -describe('workspace addresses', () => { - test('tab IDs are the workspace-relative paths', () => { - expect(parseMoiHref('moi:/views/events?eventId=123')).toEqual({ - tab: 'views/events', - search: '?eventId=123' - }) - expect(workspaceTabPath('ws1', 'views/events')).toBe('/workspace/ws1/views/events') - }) - - test('old browser bookmarks resolve without accepting legacy tab IDs in new addresses', () => { - expect(legacyTabFromPath('view:orders')).toBe('views/orders') - expect(legacyTabFromPath('view-builder:abc')).toBeNull() - expect(legacyTabFromPath('view:%65vents')).toBe('views/events') - expect(legacyTabFromPath('view:%2565vents')).toBeNull() - expect(legacyTabFromPath('view:a/b')).toBeNull() - expect(legacyTabFromPath('views/orders')).toBeNull() - expect(() => parseMoiHref('moi:/view:orders')).toThrow() - }) +describe('resolveUrl', () => { + test.each(['overview', 'scratchpad', 'views/events', 'chat', 'chats/session-1'])( + 'resolves %s inside the supplied workspace', + path => { + expect(resolveUrl(`moi:/${path}`, context)).toBe(`/workspace/abc/${path}`) + expect(resolveUrl(`moi:/${path}`, { workspaceId: 'other', base: '/prefix/' })).toBe( + `/prefix/workspace/other/${path}` + ) + } + ) - test('round trips a destination independently of workspace, origin, and deployment prefix', () => { - const href = 'moi:/views/events?eventId=123' - const address = parseMoiHref(href) - expect(destinationHref(address)).toBe(href) - expect(addressPath('abc', address)).toBe('/workspace/abc/views/events?eventId=123') - expect(addressPath('other', address, '/prefix/')).toBe( - '/prefix/workspace/other/views/events?eventId=123' + test('normalizes portable addresses for CLI validation without a workspace', () => { + expect(resolveUrl('moi:/views/%65vents?b=2&a=1')).toBe('moi:/views/events?a=1&b=2') + expect(resolveUrl('moi:/chats/provider%3Aid', context)).toBe( + '/workspace/abc/chats/provider%3Aid' ) - expect(resolveUrl(href, context)).toBe('/workspace/abc/views/events?eventId=123') - expect(resolveUrl(href, { ...context, workspacePath: '/prefix/workspace/abc' })).toBe( - '/prefix/workspace/abc/views/events?eventId=123' + expect(resolveUrl('moi:/chats/%252f', context)).toBe('/workspace/abc/chats/%252f') + expect(resolveUrl('moi:/views/events', { workspaceId: 'a/b' })).toBe( + '/workspace/a%2Fb/views/events' ) }) - test('accepts encoded IDs supported by the applet server', () => { - expect(parseMoiHref('moi:/views/%65vents_2026-09')).toEqual({ - tab: 'views/events_2026-09', - search: '' - }) - }) - test('query values remain strings and follow URLSearchParams.get semantics', () => { + test('encodes query values once and preserves repeated values', () => { const values = { title: 'Grüße & a/b?c=#100% +', enabled: 'false', page: '02', empty: '' } - const href = `moi:/views/events?${new URLSearchParams(values)}` - expect(readViewParams(parseMoiHref(href).search)).toEqual(values) - expect(readViewParams('?x=1&x=2')).toEqual({ x: '1' }) - expect(readViewParams('?x=&x=2')).toEqual({ x: '' }) - expect(canonicalSearch('?b=2&a=1&b=3')).toBe('?a=1&b=2&b=3') - expect(readViewParams(parseMoiHref('moi:/views/events?x=1&x=2').search)).toEqual({ x: '1' }) - expect(readViewParams('')).toEqual({}) - expect(Object.hasOwn(readViewParams('__proto__=safe'), '__proto__')).toBe(true) + const resolved = new URL( + resolveUrl(`moi:/views/events?${new URLSearchParams(values)}`, context), + 'https://moi.test' + ) + for (const [key, value] of Object.entries(values)) + expect(resolved.searchParams.get(key)).toBe(value) + expect(resolveUrl('moi:/views/events?b=2&x=1&x=2&a=1', context)).toBe( + '/workspace/abc/views/events?a=1&b=2&x=1&x=2' + ) }) - test('rejects ambiguous, malformed, private, and unsupported destinations', () => { + + test('rejects malformed or unsupported addresses, including retired bookmarks', () => { for (const href of [ null, 2, @@ -72,6 +46,8 @@ describe('workspace addresses', () => { '/views/events', 'view:events', 'moi://views/events', + 'moi:/view:events', + 'moi:/agent', 'moi:/views/', 'moi:/views/a/b', 'moi:/views/%2f', @@ -82,69 +58,52 @@ describe('workspace addresses', () => { 'moi:/views/../overview', 'moi:/views/a#part', 'moi:/view-builders/a', - 'moi:/agent', + 'moi:/chat/', + 'moi:/chat#part', 'moi:/chats/', 'moi:/chats/a/b', 'moi:/chats/%2f', 'moi:/chats/%', - 'moi:/files/a.md' - ]) { - expect(() => parseMoiHref(href)).toThrow() - } - }) - test('chat entry routes round trip without becoming workspace tabs', () => { - const address = parseMoiHref('moi:/chats/%61bc') - expect(address).toEqual({ sessionId: 'abc', search: '' }) - expect(destinationHref(address)).toBe('moi:/chats/abc') - expect(addressPath('ws1', address, '/prefix')).toBe('/prefix/workspace/ws1/chats/abc') - expect(resolveUrl('moi:/chats/abc', context)).toBe('/workspace/abc/chats/abc') - expect(tabFromPath('chats/abc')).toBeNull() - expect(chatSessionIdFromPath('chats/provider%3Aid')).toBe('provider:id') - expect(chatSessionIdFromPath('chats/%252f')).toBe('%2f') - expect(chatSessionIdFromPath('chats/..')).toBeNull() + 'moi:/chats/%00', + 'moi:/chats/..' + ]) + expect(() => resolveUrl(href, context)).toThrow() }) - test('current tabs round trip through browser paths', () => { - for (const tab of ['overview', 'scratchpad', 'views/events'] as const) { - expect(tabFromPath(tab)).toBe(tab) - } - }) - test('web hrefs stay web hrefs; executable protocols cannot use the API', () => { + + test('keeps ordinary web addresses and rejects executable protocols', () => { expect(resolveUrl('https://example.com/a?q=b', context)).toBe('https://example.com/a?q=b') expect(() => resolveUrl('javascript:alert(1)', context)).toThrow() expect(() => resolveUrl('data:text/html,hello', context)).toThrow() }) -}) -describe('workspace file addresses', () => { - test('resolves files using the existing API route, including before a bridge is attached', () => { + test('resolves files through their API endpoint, including before a bridge attaches', () => { expect(resolveUrl('moi:/files/clips/video.mp4', context)).toBe( '/api/workspaces/abc/files/clips/video.mp4' ) - expect(resolveUrl('moi:/files/photo.png', { apiBase: '/api/workspaces/other' })).toBe( - '/api/workspaces/other/files/photo.png' + expect(resolveUrl('moi:/files/photo.png', { workspaceId: 'abc', base: '/prefix' })).toBe( + '/api/workspaces/abc/files/photo.png' + ) + expect(resolveUrl('moi:/files/photo.png', { apiBase: '__APPLET_API_BASE__' })).toBe( + '__APPLET_API_BASE__/files/photo.png' ) }) - test('encodes spaces and Unicode and preserves escaped filename characters exactly once', () => { + test('encodes file names once and preserves file queries and media fragments', () => { expect(resolveUrl('moi:/files/clips/Grüße intro.mp4', context)).toBe( '/api/workspaces/abc/files/clips/Gr%C3%BC%C3%9Fe%20intro.mp4' ) - const path = 'moi:/files/clips/100%25%3F%23%20intro.mp4' - expect(resolveUrl(path, context)).toBe( + expect(resolveUrl('moi:/files/clips/100%25%3F%23%20intro.mp4', context)).toBe( '/api/workspaces/abc/files/clips/100%25%3F%23%20intro.mp4' ) expect(resolveUrl('moi:/files/literal%252F.png', context)).toBe( '/api/workspaces/abc/files/literal%252F.png' ) - }) - - test('preserves file queries and media fragments', () => { - expect(resolveUrl('moi:/files/clips/video.mp4?version=2#t=10,20', context)).toBe( - '/api/workspaces/abc/files/clips/video.mp4?version=2#t=10,20' + expect(resolveUrl('moi:/files/video.mp4?version=2#t=10,20', context)).toBe( + '/api/workspaces/abc/files/video.mp4?version=2#t=10,20' ) }) - test('rejects malformed escapes, path separators, empty segments, and traversal', () => { + test('rejects unsafe file paths before URL normalization can hide traversal', () => { for (const path of [ 'moi://files/photo.png', 'moi:/files/', @@ -158,8 +117,7 @@ describe('workspace file addresses', () => { 'moi:/files/clips/%00.mp4', 'moi:/files/clips/100%.mp4', 'moi:/files/clips/%C3.mp4' - ]) { + ]) expect(() => resolveUrl(path, context)).toThrow() - } }) }) diff --git a/lib/navigation.ts b/lib/navigation.ts index 5099e4b5..da155b4c 100644 --- a/lib/navigation.ts +++ b/lib/navigation.ts @@ -1,117 +1,60 @@ -import type { WorkspaceTabId } from './types' import { isWorkspaceTabId } from './workspace-tabs' -export type ViewParams = Record -export type WorkspaceAddress = { tab: WorkspaceTabId; search: string } -export type WorkspaceDestination = WorkspaceAddress | { sessionId: string; search: string } - -export function parseMoiHref(href: unknown): WorkspaceDestination { - if (typeof href !== 'string' || !href.startsWith('moi:/') || href.startsWith('moi://')) { - throw new Error('Expected a workspace address such as moi:/views/events?eventId=123') - } - const path = href.slice(5).split(/[?#]/, 1)[0] - if (href.includes('#') || /[\s\\]/.test(path)) throw new Error('Invalid workspace address') - const tab = tabFromPath(path) - const sessionId = chatSessionIdFromPath(path) - if (!tab && !sessionId) { - throw new Error(`Unsupported workspace destination: ${path}`) - } - const query = href.indexOf('?') - const search = query < 0 ? '' : canonicalSearch(href.slice(query + 1)) - return sessionId ? { sessionId, search } : { tab: tab!, search } +type ResolveUrlContext = { + workspaceId?: string + base?: string + // Applet bundles know their file endpoint before the host attaches a bridge. + apiBase?: string } -export function chatSessionIdFromPath(path: string): string | null { - if (!path.startsWith('chats/')) return null - try { - const id = decodeURIComponent(path.slice('chats/'.length)) - return id && id !== '.' && id !== '..' && !/[/\\\s\0]/.test(id) ? id : null - } catch { - return null +// Without a workspace, validates and normalizes a portable address (used by the CLI). +export function resolveUrl( + url: unknown, + { workspaceId, base = '', apiBase }: ResolveUrlContext = {} +): string { + if (typeof url !== 'string') throw new Error('A URL is required') + if (!url.startsWith('moi:')) { + const resolved = new URL(url) + if (resolved.protocol !== 'https:' && resolved.protocol !== 'http:') + throw new Error('Unsupported URL') + return resolved.href } -} - -export function destinationHref(address: WorkspaceDestination): string { - return 'sessionId' in address - ? `moi:/chats/${encodeURIComponent(address.sessionId)}${address.search}` - : moiHref(address.tab, address.search) -} - -export function canonicalSearch(search: string): string { - // Sorting makes equivalent addresses a no-op instead of adding history entries. - const params = new URLSearchParams(search) - params.sort() - const result = params.toString() - return result ? `?${result}` : '' -} + if (!url.startsWith('moi:/') || url.startsWith('moi://')) + throw new Error('Expected a workspace address such as moi:/views/events?eventId=123') -export function readViewParams(search: string): ViewParams { - const params = new URLSearchParams(search) - return Object.fromEntries(Array.from(params.keys(), key => [key, params.get(key)!])) -} + const tail = url.slice(5) + const suffixIndex = tail.search(/[?#]/) + const pathname = suffixIndex < 0 ? tail : tail.slice(0, suffixIndex) + const suffix = suffixIndex < 0 ? '' : tail.slice(suffixIndex) + const segments = pathname.split('/').map(decodeURIComponent) + if ( + segments.some( + segment => !segment || segment === '.' || segment === '..' || /[/\\\0]/.test(segment) + ) + ) + throw new Error('Invalid workspace address') -export function tabFromPath(path: string): WorkspaceTabId | null { - try { - const tab = decodeURIComponent(path) - return isWorkspaceTabId(tab) ? tab : null - } catch { - return null + const id = workspaceId === undefined ? undefined : encodeURIComponent(workspaceId) + if (segments[0] === 'files') { + if (segments.length < 2) throw new Error('Invalid workspace file path') + const prefix = apiBase ?? (id === undefined ? 'moi:' : `/api/workspaces/${id}`) + return `${prefix}/files/${segments.slice(1).map(encodeURIComponent).join('/')}${suffix}` } -} -// Compatibility for old browser bookmarks only; saved tab state uses paths. -export function legacyTabFromPath(path: string): WorkspaceTabId | null { - const match = /^view:(.+)$/.exec(path) - return match ? tabFromPath(`views/${match[1]}`) : null -} - -export function moiHref(tab: WorkspaceTabId, search = ''): string { - return `moi:/${tab}${canonicalSearch(search)}` -} - -// Deployment-specific addressing is confined to this host adapter. Callers -// may supply the router base; portable links never contain it. -export function workspacePath(workspaceId: string, base = ''): string { - return `${base.replace(/\/$/, '')}/workspace/${encodeURIComponent(workspaceId)}` -} - -export function addressPath(workspaceId: string, address: WorkspaceDestination, base = ''): string { - return `${workspacePath(workspaceId, base)}/${destinationHref(address).slice(5)}` -} - -export function workspaceTabPath(workspaceId: string, tab: WorkspaceTabId): string { - return addressPath(workspaceId, { tab, search: '' }) -} - -type ResolveUrlContext = { apiBase: string; workspacePath?: string } + const path = segments.join('/') + if (url.includes('#') || /\s/.test(path)) throw new Error('Invalid workspace address') + if ( + path !== 'chat' && + !isWorkspaceTabId(path) && + !(segments[0] === 'chats' && segments.length === 2) + ) + throw new Error(`Unsupported workspace destination: ${path}`) -export function resolveUrl(url: string, context: ResolveUrlContext): string { - if (url.startsWith('moi:/files/')) { - const tail = url.slice('moi:/files/'.length) - const suffixIndex = tail.search(/[?#]/) - const pathname = suffixIndex < 0 ? tail : tail.slice(0, suffixIndex) - const suffix = suffixIndex < 0 ? '' : tail.slice(suffixIndex) - const segments = pathname.split('/').map(segment => { - const decoded = decodeURIComponent(segment) - if (!decoded || decoded === '.' || decoded === '..' || /[/\\\0]/.test(decoded)) { - throw new Error('Invalid workspace file path') - } - return encodeURIComponent(decoded) - }) - return `${context.apiBase}/files/${segments.join('/')}${suffix}` - } - if (url.startsWith('moi:')) { - const address = parseMoiHref(url) - return context.workspacePath - ? `${context.workspacePath}/${destinationHref(address).slice(5)}` - : '' - } - // Only explicit ordinary web addresses may leave the workspace through the - // imperative API. Native anchors keep their existing protocol policy. - const resolved = new URL(url) - if (resolved.protocol !== 'https:' && resolved.protocol !== 'http:') - throw new Error('Unsupported URL') - return resolved.href + const params = new URLSearchParams(suffix) + params.sort() + const search = params.toString() + const prefix = id === undefined ? 'moi:' : `${base.replace(/\/$/, '')}/workspace/${id}` + return `${prefix}/${segments.map(encodeURIComponent).join('/')}${search ? `?${search}` : ''}` } export type NavigationRequest = { diff --git a/lib/workspace-tabs.test.ts b/lib/workspace-tabs.test.ts index 64fc081a..e9eaf54b 100644 --- a/lib/workspace-tabs.test.ts +++ b/lib/workspace-tabs.test.ts @@ -38,8 +38,8 @@ describe('tab id round-trips', () => { test('view tabs', () => { expect(viewTabId('orders')).toBe('views/orders') expect(viewIdFromTab('views/orders')).toBe('orders') - expect(viewIdFromTab('overview')).toBeNull() - expect(viewIdFromTab('scratchpad')).toBeNull() + expect(viewIdFromTab('overview')).toBeUndefined() + expect(viewIdFromTab('scratchpad')).toBeUndefined() }) }) diff --git a/lib/workspace-tabs.ts b/lib/workspace-tabs.ts index d43b36bf..283b2903 100644 --- a/lib/workspace-tabs.ts +++ b/lib/workspace-tabs.ts @@ -10,9 +10,18 @@ export function isWorkspaceTabId(value: unknown): value is WorkspaceTabId { ) } +export function tabFromPath(path: string): WorkspaceTabId | undefined { + try { + const tab = decodeURIComponent(path) + return isWorkspaceTabId(tab) ? tab : undefined + } catch { + return undefined + } +} + export const viewTabId = (viewId: string): WorkspaceTabId => `views/${viewId}` -export const viewIdFromTab = (tab: WorkspaceTabId): string | null => - tab.startsWith('views/') ? tab.slice('views/'.length) : null +export const viewIdFromTab = (tab: WorkspaceTabId): string | undefined => + tab.startsWith('views/') ? tab.slice('views/'.length) : undefined // A record check shared by JSON boundary validators. export function isParamsRecord(value: unknown): value is Record { diff --git a/server/cli.ts b/server/cli.ts index bcf649f2..375ca9a5 100755 --- a/server/cli.ts +++ b/server/cli.ts @@ -22,7 +22,7 @@ import { deriveThemeColors } from '@/lib/themes' import type { AgentTheme, ColorTheme, FontTheme, RadiusTheme } from '@/lib/themes' -import { parseMoiHref } from '@/lib/navigation' +import { resolveUrl } from '@/lib/navigation' import { EXPERIMENTAL_FEATURES, experimentalFlagName } from '@/lib/experimental' import type { ExperimentalFeatures } from '@/lib/experimental' import type { @@ -2485,7 +2485,9 @@ const navigate = defineCommand({ }, run({ args }) { try { - parseMoiHref(args.href) + const href = resolveUrl(args.href) + if (!href.startsWith('moi:/') || href.startsWith('moi:/files/')) + throw new Error('Navigation needs a workspace page address') } catch (error) { console.error(error instanceof Error ? error.message : 'Invalid workspace address') process.exit(1) diff --git a/server/control.ts b/server/control.ts index c4829ae9..284f4846 100644 --- a/server/control.ts +++ b/server/control.ts @@ -4,7 +4,8 @@ import { resolve } from 'path' import { parseAppletSelector } from '@/lib/applet-selector' import { resolveWorkspaceTheme } from '@/lib/themes' import type { WorkspaceEntry } from '@/lib/types' -import { destinationHref, moiHref, parseMoiHref } from '@/lib/navigation' +import { resolveUrl } from '@/lib/navigation' +import { isWorkspaceTabId } from '@/lib/workspace-tabs' import { navigationRelay } from './navigation-relay' import { clearAppletLog, getAppletLog, getAppletLogCount } from './applet-log' @@ -243,7 +244,7 @@ export const control = Bun.serve({ const result = await createViewFromSession(match, data.fromSessionId, data.requirements) let warning: string | undefined try { - await navigationRelay.navigate(match.id, moiHref(`views/${result.viewId}`)) + await navigationRelay.navigate(match.id, `moi:/views/${result.viewId}`) } catch (error) { const message = error instanceof Error ? error.message : 'Navigation failed' warning = `View was created, but could not open it: ${message}. Open it with moi navigate 'moi:/views/${result.viewId}'.` @@ -321,10 +322,11 @@ export const control = Bun.serve({ const match = await resolveWorkspace(ws, data.path) if (!match) return try { - const address = parseMoiHref(data.href) - const href = destinationHref(address) - if ('tab' in address) - assertNavigableTab(address.tab, await getWorkspaceViews(match.path)) + const href = resolveUrl(data.href) + if (!href.startsWith('moi:/') || href.startsWith('moi:/files/')) + throw new Error('Navigation needs a workspace page address') + const tab = href.slice(5).split('?', 1)[0] + if (isWorkspaceTabId(tab)) assertNavigableTab(tab, await getWorkspaceViews(match.path)) await navigationRelay.navigate(match.id, href) ws.send(JSON.stringify({ ok: true, href })) } catch (error) { diff --git a/server/tabs.ts b/server/tabs.ts index fc7539ca..71ec40f1 100644 --- a/server/tabs.ts +++ b/server/tabs.ts @@ -1,6 +1,5 @@ // Tab discovery with portable navigation addresses. import type { ViewInfo, WorkspaceTabId } from '@/lib/types' -import { moiHref } from '@/lib/navigation' import { viewTabId } from '@/lib/workspace-tabs' // One row of `moi tabs`. `isDefault` marks the workspace's saved default tab @@ -27,7 +26,7 @@ export function assembleTabRows(views: ViewInfo[], defaultTab: WorkspaceTabId): ].map(row => ({ ...row, isDefault: row.id === defaultTab, - href: moiHref(row.id) + href: `moi:/${row.id}` })) } @@ -38,6 +37,6 @@ export function assertNavigableTab(tab: WorkspaceTabId, views: ViewInfo[]): void const rows = assembleTabRows(views, 'overview') if (rows.some(row => row.id === tab)) return throw new Error( - `Unknown destination "${moiHref(tab)}". Valid addresses: ${rows.map(row => row.href).join(', ')}` + `Unknown destination "moi:/${tab}". Valid addresses: ${rows.map(row => row.href).join(', ')}` ) } diff --git a/server/test/applet-url.test.ts b/server/test/applet-url.test.ts index 1c4e118b..84da568b 100644 --- a/server/test/applet-url.test.ts +++ b/server/test/applet-url.test.ts @@ -4,6 +4,7 @@ import type * as MoiRuntime from '../applets/runtime/moi' import { APPLET_API_BASE_SENTINEL } from '../applets/api-base' import { __attachBridge, resolveUrl } from '../applets/runtime/moi' +import { resolveUrl as resolveWorkspaceUrl } from '@/lib/navigation' // Evaluated as the module loads, before the host has attached any bridge. const videoUrl = resolveUrl('moi:/files/clips/a%20b.mp4#t=5') @@ -23,3 +24,16 @@ test('file URLs resolve at module load and retain their source workspace after b expect(resolveUrl('moi:/files/photo.png')).toBe(`${APPLET_API_BASE_SENTINEL}/files/photo.png`) expect(resolveUrl('moi:/views/orders')).toBe('/workspace/other/overview') }) + +test('moi.resolveUrl uses the shared resolver for workspace links through its host bridge', () => { + __attachBridge({ + resolveUrl: url => resolveWorkspaceUrl(url, { workspaceId: 'abc', base: '/prefix' }) + }) + expect(resolveUrl('moi:/chat')).toBe('/prefix/workspace/abc/chat') + expect(resolveUrl('moi:/chats/provider%3Aid')).toBe('/prefix/workspace/abc/chats/provider%3Aid') + expect(resolveUrl('moi:/views/%65vents?b=2&a=1')).toBe( + '/prefix/workspace/abc/views/events?a=1&b=2' + ) + expect(() => resolveUrl('moi:/view:events')).toThrow() + expect(() => resolveUrl('moi:/files/../private.txt')).toThrow() +}) diff --git a/server/test/navigation-cli.test.ts b/server/test/navigation-cli.test.ts index 338ad0b7..26db095a 100644 --- a/server/test/navigation-cli.test.ts +++ b/server/test/navigation-cli.test.ts @@ -3,7 +3,20 @@ import { join } from 'node:path' const CLI = join(import.meta.dir, '..', 'cli.ts') -test.each(['moi:/overview', 'moi:/chats/chat-1'])( +test.each([ + 'moi:/files/photo.png', + 'https://example.com', + 'moi:/view:orders', + 'moi:/views/../overview' +])('navigation CLI rejects %s before contacting the server', async href => { + const proc = Bun.spawn(['bun', CLI, 'navigate', href], { stdout: 'pipe', stderr: 'pipe' }) + const [error, code] = await Promise.all([new Response(proc.stderr).text(), proc.exited]) + expect(code).toBe(1) + expect(error).not.toContain('connect') + expect(error.length).toBeGreaterThan(0) +}) + +test.each(['moi:/overview', 'moi:/chat', 'moi:/chats/chat-1'])( 'navigation CLI sends %s and waits for acknowledgement', async href => { let request: unknown diff --git a/workspace/.claude/skills/moi-workspace/SKILL.md b/workspace/.claude/skills/moi-workspace/SKILL.md index 1d2177c0..ef3a83fd 100644 --- a/workspace/.claude/skills/moi-workspace/SKILL.md +++ b/workspace/.claude/skills/moi-workspace/SKILL.md @@ -120,7 +120,10 @@ Use the task-specific sections below for workflow guidance. The CLI will grow ov Use `moi:/` addresses in chat, CLI commands, and applets. They refer to the current workspace, independently of its domain and deployment path. -Supported destinations: `moi:/overview`, `moi:/scratchpad`, and `moi:/views/`. +Supported destinations: `moi:/overview`, `moi:/scratchpad`, `moi:/views/`, +`moi:/chat`, and `moi:/chats/`. Both chat links open the workspace's last opened tab +and reveal the chat. `moi:/chat` keeps the current selection and pin; an exact chat link selects +that chat. Run `moi tabs` to discover views and their addresses. Widgets are not navigation destinations. Put view params in the query string. Values are strings; read the target view's `Params` type From 659f6a26ac5cd79bca73eeb990916b69a5c5683e Mon Sep 17 00:00:00 2001 From: Anton Frehser Date: Mon, 12 Oct 2026 21:24:23 +0200 Subject: [PATCH 2/2] docs: clarify workspace navigation links --- workspace/.claude/skills/moi-workspace/SKILL.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/workspace/.claude/skills/moi-workspace/SKILL.md b/workspace/.claude/skills/moi-workspace/SKILL.md index ef3a83fd..82d8a2c8 100644 --- a/workspace/.claude/skills/moi-workspace/SKILL.md +++ b/workspace/.claude/skills/moi-workspace/SKILL.md @@ -120,10 +120,14 @@ Use the task-specific sections below for workflow guidance. The CLI will grow ov Use `moi:/` addresses in chat, CLI commands, and applets. They refer to the current workspace, independently of its domain and deployment path. -Supported destinations: `moi:/overview`, `moi:/scratchpad`, `moi:/views/`, -`moi:/chat`, and `moi:/chats/`. Both chat links open the workspace's last opened tab -and reveal the chat. `moi:/chat` keeps the current selection and pin; an exact chat link selects -that chat. +Supported destinations: + +- `moi:/overview`: open the workspace overview. +- `moi:/scratchpad`: open the shared canvas. +- `moi:/views/`: open a specific view. +- `moi:/chat`: show the current chat. +- `moi:/chats/`: open a specific chat. + Run `moi tabs` to discover views and their addresses. Widgets are not navigation destinations. Put view params in the query string. Values are strings; read the target view's `Params` type