diff --git a/apps/web/app/components/terminal/terminal-drawer.tsx b/apps/web/app/components/terminal/terminal-drawer.tsx index b4211fac18b..08128ee17d7 100644 --- a/apps/web/app/components/terminal/terminal-drawer.tsx +++ b/apps/web/app/components/terminal/terminal-drawer.tsx @@ -468,10 +468,11 @@ export default function TerminalDrawer({ onClose, cwd }: TerminalDrawerProps) { ); const handleExited = useCallback( - (id: string) => { - closeTerminal(id); + (_id: string) => { + // Keep the drawer open so the user can see "[process exited]" and any + // error output. They can close manually with Cmd+J or the close button. }, - [closeTerminal], + [], ); const hasMultiple = terminals.length > 1; diff --git a/apps/web/app/workspace/workspace-content.tsx b/apps/web/app/workspace/workspace-content.tsx index b6cb9f7182d..31802dd95c1 100644 --- a/apps/web/app/workspace/workspace-content.tsx +++ b/apps/web/app/workspace/workspace-content.tsx @@ -29,7 +29,7 @@ import { ReportViewer } from "../components/charts/report-viewer"; import { ChatPanel, type ChatPanelHandle, type SubagentSpawnInfo } from "../components/chat-panel"; import { EntryDetailModal } from "../components/workspace/entry-detail-modal"; import { useSearchIndex } from "@/lib/search-index"; -import { parseWorkspaceLink, isWorkspaceLink, parseUrlState, buildUrl, type WorkspaceUrlState } from "@/lib/workspace-links"; +import { parseWorkspaceLink, isWorkspaceLink, parseUrlState, buildUrl, buildWorkspaceSyncParams, type WorkspaceUrlState } from "@/lib/workspace-links"; import { isCodeFile } from "@/lib/report-utils"; import { CronDashboard } from "../components/cron/cron-dashboard"; import { CronJobDetail } from "../components/cron/cron-job-detail"; @@ -1341,8 +1341,6 @@ function WorkspacePageInner() { // This effect only manages shell-level params (path, chat, browse, etc.) // and preserves object-view params (viewType, filters, search, sort, etc.) // that are managed by ObjectView's own URL sync effect. - const OBJECT_VIEW_PARAMS = ["viewType", "view", "filters", "search", "sort", "page", "pageSize", "cols"]; - useEffect(() => { if (!initialPathHandled.current) return; @@ -1354,38 +1352,21 @@ function WorkspacePageInner() { } const current = new URLSearchParams(window.location.search); - const params = new URLSearchParams(); - - if (activePath) { - params.set("path", activePath); - const entry = current.get("entry"); - if (entry) params.set("entry", entry); - if (fileChatSessionId) params.set("fileChat", fileChatSessionId); - - // Cron-specific URL params (only when on a cron path) - if (activePath === "~cron") { - if (cronView !== "overview") params.set("cronView", cronView); - if (cronView === "calendar" && cronCalMode !== "month") params.set("cronCalMode", cronCalMode); - if ((cronView === "calendar" || cronView === "timeline") && cronDate) params.set("cronDate", cronDate); - } else if (activePath.startsWith("~cron/")) { - if (cronRunFilter !== "all") params.set("cronRunFilter", cronRunFilter); - if (cronRun != null) params.set("cronRun", String(cronRun)); - } - - // Preserve object-view params managed by ObjectView's URL sync effect - for (const k of OBJECT_VIEW_PARAMS) { - const v = current.get(k); - if (v) params.set(k, v); - } - } else if (activeSessionId) { - params.set("chat", activeSessionId); - if (activeSubagentKey) params.set("subagent", activeSubagentKey); - } - - if (browseDir) params.set("browse", browseDir); - if (showHidden) params.set("hidden", "1"); - if (chatSidebarPreview) params.set("preview", chatSidebarPreview.path); - if (terminalOpen) params.set("terminal", "1"); + const params = buildWorkspaceSyncParams({ + activePath, + activeSessionId, + activeSubagentKey, + fileChatSessionId, + browseDir, + showHidden, + previewPath: chatSidebarPreview?.path ?? null, + terminalOpen, + cronView, + cronCalMode, + cronDate, + cronRunFilter, + cronRun, + }, current); const nextQs = params.toString(); const currentQs = current.toString(); diff --git a/apps/web/lib/workspace-links.ts b/apps/web/lib/workspace-links.ts index 113d4ae179d..51ee4e588a9 100644 --- a/apps/web/lib/workspace-links.ts +++ b/apps/web/lib/workspace-links.ts @@ -190,6 +190,73 @@ export function serializeUrlState(state: Partial): string { return params.toString(); } +// --------------------------------------------------------------------------- +// Workspace URL sync param builder +// --------------------------------------------------------------------------- + +const OBJECT_VIEW_PARAMS = ["viewType", "view", "filters", "search", "sort", "page", "pageSize", "cols"]; + +export interface WorkspaceSyncState { + activePath: string | null; + activeSessionId: string | null; + activeSubagentKey: string | null; + fileChatSessionId: string | null; + browseDir: string | null; + showHidden: boolean; + previewPath: string | null; + terminalOpen: boolean; + cronView: CronDashboardView; + cronCalMode: CalendarMode; + cronDate: string | null; + cronRunFilter: CronRunStatusFilter; + cronRun: number | null; +} + +/** + * Build the URL params that the workspace URL sync effect should push. + * + * Pure function: takes app state + the current URL params (for preserving + * object-view and entry params managed by other effects) and returns the + * complete URLSearchParams to write. + */ +export function buildWorkspaceSyncParams( + state: WorkspaceSyncState, + currentParams: URLSearchParams, +): URLSearchParams { + const params = new URLSearchParams(); + + if (state.activePath) { + params.set("path", state.activePath); + const entry = currentParams.get("entry"); + if (entry) params.set("entry", entry); + if (state.fileChatSessionId) params.set("fileChat", state.fileChatSessionId); + + if (state.activePath === "~cron") { + if (state.cronView !== "overview") params.set("cronView", state.cronView); + if (state.cronView === "calendar" && state.cronCalMode !== "month") params.set("cronCalMode", state.cronCalMode); + if ((state.cronView === "calendar" || state.cronView === "timeline") && state.cronDate) params.set("cronDate", state.cronDate); + } else if (state.activePath.startsWith("~cron/")) { + if (state.cronRunFilter !== "all") params.set("cronRunFilter", state.cronRunFilter); + if (state.cronRun != null) params.set("cronRun", String(state.cronRun)); + } + + for (const k of OBJECT_VIEW_PARAMS) { + const v = currentParams.get(k); + if (v) params.set(k, v); + } + } else if (state.activeSessionId) { + params.set("chat", state.activeSessionId); + if (state.activeSubagentKey) params.set("subagent", state.activeSubagentKey); + } + + if (state.browseDir) params.set("browse", state.browseDir); + if (state.showHidden) params.set("hidden", "1"); + if (state.previewPath) params.set("preview", state.previewPath); + if (state.terminalOpen) params.set("terminal", "1"); + + return params; +} + /** Build a full root-route URL string from partial state. */ export function buildUrl(state: Partial): string { const qs = serializeUrlState(state); diff --git a/apps/web/lib/workspace-sync-params.test.ts b/apps/web/lib/workspace-sync-params.test.ts new file mode 100644 index 00000000000..7c7cb4305b6 --- /dev/null +++ b/apps/web/lib/workspace-sync-params.test.ts @@ -0,0 +1,230 @@ +/** + * Tests for buildWorkspaceSyncParams — the pure function that decides + * which URL params the workspace URL sync effect should write. + * + * The primary invariant: terminal state must survive alongside other + * URL param changes. A previous bug caused the sync effect to strip + * ?terminal=1 whenever it fired for non-terminal reasons, closing the + * terminal drawer on navigation. + */ +import { describe, it, expect } from "vitest"; +import { buildWorkspaceSyncParams, type WorkspaceSyncState } from "./workspace-links"; + +function defaultState(overrides: Partial = {}): WorkspaceSyncState { + return { + activePath: null, + activeSessionId: null, + activeSubagentKey: null, + fileChatSessionId: null, + browseDir: null, + showHidden: false, + previewPath: null, + terminalOpen: false, + cronView: "overview", + cronCalMode: "month", + cronDate: null, + cronRunFilter: "all", + cronRun: null, + ...overrides, + }; +} + +// --------------------------------------------------------------------------- +// Terminal param preservation (regression: terminal closing on navigation) +// --------------------------------------------------------------------------- + +describe("terminal param in workspace URL sync", () => { + it("includes terminal=1 when terminal is open (prevents terminal param being stripped)", () => { + const params = buildWorkspaceSyncParams( + defaultState({ terminalOpen: true }), + new URLSearchParams(), + ); + expect(params.get("terminal")).toBe("1"); + }); + + it("omits terminal param when terminal is closed (prevents stale terminal=1 in URL)", () => { + const params = buildWorkspaceSyncParams( + defaultState({ terminalOpen: false }), + new URLSearchParams(), + ); + expect(params.has("terminal")).toBe(false); + }); + + it("preserves terminal=1 when navigating to a file (prevents terminal close on file open)", () => { + const params = buildWorkspaceSyncParams( + defaultState({ activePath: "knowledge/notes.md", terminalOpen: true }), + new URLSearchParams("terminal=1"), + ); + expect(params.get("terminal")).toBe("1"); + expect(params.get("path")).toBe("knowledge/notes.md"); + }); + + it("preserves terminal=1 when switching between files (prevents terminal close on navigation)", () => { + const current = new URLSearchParams("path=old-file.md&terminal=1"); + const params = buildWorkspaceSyncParams( + defaultState({ activePath: "new-file.md", terminalOpen: true }), + current, + ); + expect(params.get("terminal")).toBe("1"); + expect(params.get("path")).toBe("new-file.md"); + }); + + it("preserves terminal=1 when switching to chat (prevents terminal close on mode switch)", () => { + const params = buildWorkspaceSyncParams( + defaultState({ activeSessionId: "sess-42", terminalOpen: true }), + new URLSearchParams("terminal=1"), + ); + expect(params.get("terminal")).toBe("1"); + expect(params.get("chat")).toBe("sess-42"); + }); + + it("preserves terminal=1 alongside browse mode (prevents terminal close in browse)", () => { + const params = buildWorkspaceSyncParams( + defaultState({ browseDir: "/Users/me/projects", terminalOpen: true }), + new URLSearchParams("terminal=1"), + ); + expect(params.get("terminal")).toBe("1"); + expect(params.get("browse")).toBe("/Users/me/projects"); + }); + + it("does not produce a URL diff when only terminal is present in current URL and state matches", () => { + const current = new URLSearchParams("terminal=1"); + const params = buildWorkspaceSyncParams( + defaultState({ terminalOpen: true }), + current, + ); + expect(params.toString()).toBe(current.toString()); + }); + + it("does not produce a URL diff when file+terminal in current URL and state matches", () => { + const current = new URLSearchParams("path=doc.md&terminal=1"); + const params = buildWorkspaceSyncParams( + defaultState({ activePath: "doc.md", terminalOpen: true }), + current, + ); + expect(params.toString()).toBe(current.toString()); + }); +}); + +// --------------------------------------------------------------------------- +// Core param building (non-terminal) +// --------------------------------------------------------------------------- + +describe("buildWorkspaceSyncParams core behavior", () => { + it("sets path when activePath is present", () => { + const params = buildWorkspaceSyncParams( + defaultState({ activePath: "knowledge/readme.md" }), + new URLSearchParams(), + ); + expect(params.get("path")).toBe("knowledge/readme.md"); + }); + + it("sets chat + subagent when in chat mode", () => { + const params = buildWorkspaceSyncParams( + defaultState({ activeSessionId: "sess-1", activeSubagentKey: "child-a" }), + new URLSearchParams(), + ); + expect(params.get("chat")).toBe("sess-1"); + expect(params.get("subagent")).toBe("child-a"); + }); + + it("path takes priority over chat (only one navigation mode at a time)", () => { + const params = buildWorkspaceSyncParams( + defaultState({ activePath: "file.md", activeSessionId: "sess-1" }), + new URLSearchParams(), + ); + expect(params.get("path")).toBe("file.md"); + expect(params.has("chat")).toBe(false); + }); + + it("preserves entry param from current URL when path is set (entry modal is independent)", () => { + const params = buildWorkspaceSyncParams( + defaultState({ activePath: "leads" }), + new URLSearchParams("entry=leads:abc"), + ); + expect(params.get("entry")).toBe("leads:abc"); + }); + + it("does not carry entry param when in chat mode (entry only meaningful with path)", () => { + const params = buildWorkspaceSyncParams( + defaultState({ activeSessionId: "sess-1" }), + new URLSearchParams("entry=leads:abc"), + ); + expect(params.has("entry")).toBe(false); + }); + + it("preserves object-view params from current URL (managed by ObjectView's own effect)", () => { + const current = new URLSearchParams("path=leads&viewType=kanban&search=acme&sort=W10%3D&page=2&pageSize=25&cols=name,status&view=Active&filters=e30%3D"); + const params = buildWorkspaceSyncParams( + defaultState({ activePath: "leads" }), + current, + ); + expect(params.get("viewType")).toBe("kanban"); + expect(params.get("search")).toBe("acme"); + expect(params.get("view")).toBe("Active"); + expect(params.get("page")).toBe("2"); + expect(params.get("pageSize")).toBe("25"); + }); + + it("does not carry object-view params in chat mode", () => { + const current = new URLSearchParams("viewType=kanban&search=acme"); + const params = buildWorkspaceSyncParams( + defaultState({ activeSessionId: "sess-1" }), + current, + ); + expect(params.has("viewType")).toBe(false); + expect(params.has("search")).toBe(false); + }); + + it("includes browse dir and hidden flag", () => { + const params = buildWorkspaceSyncParams( + defaultState({ browseDir: "/tmp", showHidden: true }), + new URLSearchParams(), + ); + expect(params.get("browse")).toBe("/tmp"); + expect(params.get("hidden")).toBe("1"); + }); + + it("includes preview path", () => { + const params = buildWorkspaceSyncParams( + defaultState({ activePath: "doc.md", previewPath: "other.md" }), + new URLSearchParams(), + ); + expect(params.get("preview")).toBe("other.md"); + }); + + it("includes cron view params for ~cron path", () => { + const params = buildWorkspaceSyncParams( + defaultState({ activePath: "~cron", cronView: "calendar", cronCalMode: "week", cronDate: "2026-03-09" }), + new URLSearchParams(), + ); + expect(params.get("cronView")).toBe("calendar"); + expect(params.get("cronCalMode")).toBe("week"); + expect(params.get("cronDate")).toBe("2026-03-09"); + }); + + it("omits default cron view (overview) to keep URL clean", () => { + const params = buildWorkspaceSyncParams( + defaultState({ activePath: "~cron", cronView: "overview" }), + new URLSearchParams(), + ); + expect(params.has("cronView")).toBe(false); + }); + + it("includes cron run filter for ~cron/ job paths", () => { + const params = buildWorkspaceSyncParams( + defaultState({ activePath: "~cron/job-1", cronRunFilter: "error", cronRun: 12345 }), + new URLSearchParams(), + ); + expect(params.get("cronRunFilter")).toBe("error"); + expect(params.get("cronRun")).toBe("12345"); + }); + + it("returns empty params when no state is set (bare / route)", () => { + const params = buildWorkspaceSyncParams( + defaultState(), + new URLSearchParams(), + ); + expect(params.toString()).toBe(""); + }); +});