fix(terminal): keep terminal drawer open when shell process exits
The terminal drawer was flickering closed immediately after opening because the spawned shell process exited with code 1, triggering handleExited → closeTerminal → onClose which unmounted the entire drawer within ~500ms. - Stop auto-closing the drawer on process exit so users can see error output - Extract URL param building into testable buildWorkspaceSyncParams function that correctly includes terminal state, preventing param stripping on navigation - Add 21 tests covering terminal param preservation across navigation scenarios
This commit is contained in:
kumarabhirup
parent
e7ab121879
commit
e49b74c990
@ -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;
|
||||
|
||||
@ -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();
|
||||
|
||||
@ -190,6 +190,73 @@ export function serializeUrlState(state: Partial<WorkspaceUrlState>): 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<WorkspaceUrlState>): string {
|
||||
const qs = serializeUrlState(state);
|
||||
|
||||
230
apps/web/lib/workspace-sync-params.test.ts
Normal file
230
apps/web/lib/workspace-sync-params.test.ts
Normal file
@ -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> = {}): 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("");
|
||||
});
|
||||
});
|
||||
Loading…
x
Reference in New Issue
Block a user