From 40c89cd380e73c97ccaa29425bc852b7047f5273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sat, 9 May 2026 11:59:16 +0100 Subject: [PATCH] fix(web): preserve dockview state across task and plan-mode switches (#855) --- .agents/skills/debug-logs/SKILL.md | 76 +++-- apps/backend/cmd/mock-agent/scenarios.go | 111 +++++++ .../task/chat/use-chat-panel-state.ts | 16 +- .../components/task/dockview-session-tabs.ts | 80 ++++- .../components/task/task-session-sidebar.tsx | 1 - apps/web/e2e/tests/git/diff-update.spec.ts | 274 ++++++++++++++++++ .../layout/preview-tab-session-switch.spec.ts | 143 +++++++++ apps/web/hooks/file-editors-sync.test.ts | 273 +++++++++++++++++ apps/web/hooks/file-editors-sync.ts | 156 ++++++++++ apps/web/hooks/use-file-editors.ts | 156 ++-------- .../web/lib/state/dockview-env-switch.test.ts | 110 ++++++- apps/web/lib/state/dockview-env-switch.ts | 194 ++++++++++--- apps/web/lib/state/dockview-store.ts | 45 ++- .../lib/state/layout-manager/merger.test.ts | 87 +++++- apps/web/lib/state/layout-manager/merger.ts | 83 ++++-- 15 files changed, 1549 insertions(+), 256 deletions(-) create mode 100644 apps/web/hooks/file-editors-sync.test.ts create mode 100644 apps/web/hooks/file-editors-sync.ts diff --git a/.agents/skills/debug-logs/SKILL.md b/.agents/skills/debug-logs/SKILL.md index 1e3dcfdcfa..9a8365e6c9 100644 --- a/.agents/skills/debug-logs/SKILL.md +++ b/.agents/skills/debug-logs/SKILL.md @@ -1,40 +1,59 @@ --- name: debug-logs -description: Add temporary debug logs to investigate issues. Use when the user needs to trace runtime behavior in the frontend or backend. Debug logs must be stripped before creating a PR. +description: Add temporary debug logs (console.log / structured Warn) to investigate issues. Use whenever the user wants to add logs, log statements, console.logs, trace, instrument, or print runtime behaviour to debug a frontend or backend issue. Triggers include "add debug logs", "add some logs", "log this", "trace this", "instrument", "investigate why", "print", "console.log around". Debug logs must be stripped before creating a PR. --- # Debug Logs Add temporary debug logs to investigate runtime issues. These logs are **never merged** — they must be removed before creating a PR. +**Use this skill any time you are about to add a `console.log`, `logger.Warn("[DEBUG] ...`, or similar transient instrumentation.** Even if the user says just "add some logs", "throw a few logs in there", "trace this", or "instrument X", apply these rules. + ## Rules 1. **All debug logs are temporary.** Strip them before running `/commit` or `/pr`. -2. Use a consistent, searchable prefix so logs can be found and removed easily. -3. Log object fields **inline** — do not log raw objects (they render as `Object` in the browser console and require expanding). +2. Use a consistent, searchable prefix so logs can be found and removed easily (e.g. `[reorder-bug]`, `[WS-DEBUG]`). +3. **Always print every value inline as a string.** Browser DevTools and many terminal viewers collapse arrays/maps/nested objects (`Array(2)`, `{...}`) — the agent must serialise these into the log message itself, not pass them as additional arguments. +4. Prefer **one template literal** per `console.log` call. Use `\n` and indentation to lay out structured data so the user can read it without clicking to expand. ## Frontend (TypeScript) - **Level:** `console.log` — not `console.debug` (hidden by default) or `console.warn` (noisy). -- **Prefix:** `[WS-DEBUG]` (or another `[AREA-DEBUG]` prefix agreed with the user). -- **Format:** Log fields inline as key-value pairs so they are visible without expanding. +- **Prefix:** `[area-bug]` / `[AREA-DEBUG]` agreed with the user. +- **Format:** A single template-literal string. Inline every field. Pre-format arrays/objects with `.map(...).join(...)` or `JSON.stringify(...)` before interpolating. -### ✅ Correct +### ✅ Correct — template literal, every value inlined ```typescript -console.log("[WS-DEBUG] subscribeSession", { - sessionId, - refCount: current + 1, - sentMessage: shouldSend, -}); +console.log( + `[reorder-bug] sidebar:render sort=${sort.key}:${sort.direction} active=${activeId ?? "-"}\n` + + ` inputOrder:\n ${inputs.map((t) => `${t.id}|${t.title}|state=${t.state}`).join("\n ")}\n` + + ` outputOrder:\n ${outputs.map((t) => t.id).join(", ")}`, +); ``` -Console output: -``` -[WS-DEBUG] subscribeSession {sessionId: 'abc', refCount: 2, sentMessage: true} +Renders as readable plain text in the console — no clicking required, copy-pasteable, diff-friendly. + +### ✅ Acceptable — flat object of primitives only + +When every value is a primitive (string/number/bool/null), an object literal is fine: + +```typescript +console.log("[WS-DEBUG] subscribeSession", { sessionId, refCount: current + 1, sent: shouldSend }); ``` -### ❌ Wrong — raw object renders as `Object` +Renders as: `[WS-DEBUG] subscribeSession {sessionId: 'abc', refCount: 2, sent: true}`. + +### ❌ Wrong — array/nested object collapses + +```typescript +console.log("[reorder-bug] render", { tasks: tasks.map(toCompact), groups }); +// Output: [reorder-bug] render {tasks: Array(2), groups: {...}} ← unreadable +``` + +Fix: pre-stringify (`tasks.map(...).join("\n ")`) and embed in a template literal. + +### ❌ Wrong — raw object passed as second arg ```typescript console.log("[WS-DEBUG] subscribeSession", session); @@ -44,17 +63,18 @@ console.log("[WS-DEBUG] subscribeSession", session); ### ❌ Wrong — wrong log level ```typescript -console.warn("[WS-DEBUG] subscribeSession", { sessionId }); // ← use console.log -console.debug("[WS-DEBUG] subscribeSession", { sessionId }); // ← hidden by default +console.warn("[WS-DEBUG] ...", { sessionId }); // ← use console.log +console.debug("[WS-DEBUG] ...", { sessionId }); // ← hidden by default ``` ## Backend (Go) - **Level:** `WARN` — stands out from normal `DEBUG`/`INFO` output without being an error. - **Prefix:** `[DEBUG]` (or another `[AREA-DEBUG]` prefix agreed with the user). -- **Method:** Use the structured logger: `s.logger.Warn("[DEBUG] description", "key", value, ...)`. +- **Method:** Use the structured logger: `s.logger.Warn("[DEBUG] description", "key", value, ...)`. Slog renders each key-value pair inline, so primitives are fine as-is. +- **For slices/maps/structs**, pre-format with `fmt.Sprintf` / `strings.Join` so the value lands on the log line as readable text instead of `[]string{...}`-style verbose output. -### ✅ Correct +### ✅ Correct — primitives as structured fields ```go s.logger.Warn("[DEBUG] handleTaskMoved entering", @@ -65,6 +85,15 @@ s.logger.Warn("[DEBUG] handleTaskMoved entering", ) ``` +### ✅ Correct — pre-format collections inline + +```go +s.logger.Warn("[DEBUG] panel order", + "task_id", taskID, + "panels", strings.Join(panelIDs, ","), +) +``` + ### ❌ Wrong — wrong level ```go @@ -72,11 +101,18 @@ s.logger.Debug("[DEBUG] handleTaskMoved", "task_id", taskID) // ← lost in nois s.logger.Error("[DEBUG] handleTaskMoved", "task_id", taskID) // ← triggers alerts ``` +## Quick Checklist (apply before every debug log you add) + +- [ ] Does the prefix match what's agreed (or already in the file) so all logs are greppable? +- [ ] Is every value a primitive at log time? If not, pre-format with `.map().join()`, `JSON.stringify`, `strings.Join`, or `fmt.Sprintf`. +- [ ] Frontend: `console.log` (not `warn`/`debug`)? Backend: `Warn` (not `Debug`/`Error`)? +- [ ] Is the call site the cheapest possible — one log per event, not per render frame? + ## Workflow 1. **Add debug logs** to the relevant code paths. Do not commit them — keep them as unstaged changes. 2. **Let the user test** the app and report back with console/log output. -3. **Iterate** — add, move, or refine logs as needed based on findings. Still no commits. +3. **Iterate** — add, move, or refine logs as needed based on findings. Still no commits. **If the user reports the values are unreadable (`Array(2)`, `Object`, `{...}`), the previous log violated rule 3; rewrite as a template literal before re-running.** 4. **Fix the issue** once the root cause is identified. 5. **Strip all debug logs** before committing the fix. Only commit the actual fix. diff --git a/apps/backend/cmd/mock-agent/scenarios.go b/apps/backend/cmd/mock-agent/scenarios.go index e6dea59122..3840e24c85 100644 --- a/apps/backend/cmd/mock-agent/scenarios.go +++ b/apps/backend/cmd/mock-agent/scenarios.go @@ -25,6 +25,9 @@ var scenarioRegistry = map[string]func(e *emitter){ "diff-expansion-setup": scenarioDiffExpansionSetup, "diff-update-setup": scenarioDiffUpdateSetup, "diff-update-modify": scenarioDiffUpdateModify, + "diff-update-streaming": scenarioDiffUpdateStreaming, + "multi-file-setup": scenarioMultiFileSetup, + "multi-file-modify": scenarioMultiFileModify, "untracked-file-setup": scenarioUntrackedFileSetup, "untracked-file-modify": scenarioUntrackedFileModify, "clarification": scenarioClarification, @@ -387,6 +390,114 @@ func scenarioDiffUpdateModify(e *emitter) { e.text("diff-update-modify complete: diff_update_test.txt now has SECOND_MODIFICATION") } +// scenarioDiffUpdateStreaming modifies the file mid-turn, emitting text both +// before and after the write so the agent's turn stays active for several +// seconds. Used to assert that open file editor / diff panels auto-update +// while the agent is still streaming. +func scenarioDiffUpdateStreaming(e *emitter) { + fixedDelay(50) + e.text("diff-update-streaming: starting work") + + fixedDelay(1000) + + filePath := "diff_update_test.txt" + modifiedContent := "line 1: SECOND_MODIFICATION\nline 2: unchanged\nline 3: ALSO_CHANGED\n" + if err := os.WriteFile(filePath, []byte(modifiedContent), 0o644); err != nil { + e.text("diff-update-streaming: write failed: " + err.Error()) + return + } + + fixedDelay(500) + e.text("diff-update-streaming: file written, continuing") + + // Long tail keeps the turn active long enough for polling+sync to fire. + fixedDelay(6000) + e.text("diff-update-streaming complete") +} + +// scenarioMultiFileSetup commits three tracked files, then modifies all three +// so each shows up in git status with FIRST_MODIFICATION. Used to test the +// case where the user has multiple file editors / diff panels open at once. +func scenarioMultiFileSetup(e *emitter) { + fixedDelay(50) + + wd, err := os.Getwd() + if err != nil { + e.text("multi-file-setup: getwd failed: " + err.Error()) + return + } + + runGitCmd := makeGitRunner(wd) + files := []string{"multi_a.txt", "multi_b.txt", "multi_c.txt"} + + // Cleanup any leftovers from a prior run. + for _, f := range files { + _ = runGitCmd("rm", "--force", f) + } + _ = runGitCmd("commit", "-m", "cleanup multi-file fixtures") + + // Commit pristine versions. + for _, f := range files { + original := fmt.Sprintf("%s line 1: original\n%s line 2: unchanged\n%s line 3: original\n", f, f, f) + if err := os.WriteFile(f, []byte(original), 0o644); err != nil { + e.text("multi-file-setup: write failed: " + err.Error()) + return + } + if err := runGitCmd("add", f); err != nil { + e.text("multi-file-setup: git add failed for " + f) + return + } + } + if err := runGitCmd("commit", "-m", "add multi-file fixtures"); err != nil { + e.text("multi-file-setup: git commit failed") + return + } + + // Modify all three so each appears in git status with FIRST_MODIFICATION. + for _, f := range files { + modified := fmt.Sprintf("%s line 1: FIRST_MODIFICATION\n%s line 2: unchanged\n%s line 3: original\n", f, f, f) + if err := os.WriteFile(f, []byte(modified), 0o644); err != nil { + e.text("multi-file-setup: write modified failed: " + err.Error()) + return + } + } + + fixedDelay(100) + e.text("multi-file-setup complete: 3 files have FIRST_MODIFICATION") +} + +// scenarioMultiFileModify modifies all three multi-file fixtures within a +// single turn, with a long trailing delay so the panels must auto-update +// while the agent is still streaming. Reproduces the user's reported case +// where multiple open editor / diff panels go stale on a real edit. +func scenarioMultiFileModify(e *emitter) { + fixedDelay(50) + e.text("multi-file-modify: starting work") + + fixedDelay(800) + + files := []string{"multi_a.txt", "multi_b.txt", "multi_c.txt"} + for i, f := range files { + modified := fmt.Sprintf( + "%s line 1: SECOND_MODIFICATION\n%s line 2: unchanged\n%s line 3: ALSO_CHANGED_%d\n", + f, f, f, i, + ) + if err := os.WriteFile(f, []byte(modified), 0o644); err != nil { + e.text("multi-file-modify: write failed: " + err.Error()) + return + } + // Stagger the writes slightly to mimic a real agent doing one tool + // call per file rather than all writes in a single instant. + fixedDelay(250) + } + + e.text("multi-file-modify: writes done, continuing") + + // Long tail keeps the turn active so we can assert mid-turn updates. + fixedDelay(5000) + e.text("multi-file-modify complete") +} + // scenarioUntrackedFileSetup creates a new untracked file. func scenarioUntrackedFileSetup(e *emitter) { fixedDelay(50) diff --git a/apps/web/components/task/chat/use-chat-panel-state.ts b/apps/web/components/task/chat/use-chat-panel-state.ts index 79db21873d..b9a0e63ffc 100644 --- a/apps/web/components/task/chat/use-chat-panel-state.ts +++ b/apps/web/components/task/chat/use-chat-panel-state.ts @@ -31,6 +31,13 @@ import type { BuiltInPreset } from "@/lib/state/layout-manager/presets"; const EMPTY_CONTEXT_FILES: ContextFile[] = []; const PLAN_CONTEXT_PATH = "plan:context"; +// Tracks sessions for which the plan layout has already been auto-applied. +// Module-scoped so it survives TaskChatPanel remounts (dockview's fromJSON +// tears down and rebuilds the portal-hosted panel, which would otherwise +// reset a component-local ref and cause the plan preset to be re-applied — +// clobbering the just-restored saved layout on env switch). +const autoAppliedPlanSessions = new Set(); + export type CommentsState = { planComments: PlanComment[]; pendingCommentsByFile: Record; @@ -82,21 +89,20 @@ function useAutoApplyPlanLayout(opts: AutoApplyPlanLayoutOpts) { setPlanMode, addContextFile, } = opts; - const autoAppliedPlanSessionRef = useRef(null); useEffect(() => { if (!resolvedSessionId || !taskId) return; // Reset the guard when plan mode is disabled so future plan-mode steps // in the same session can be auto-applied (e.g. after proceeding away and back). - if (!sessionMetaPlanMode && autoAppliedPlanSessionRef.current === resolvedSessionId) { - autoAppliedPlanSessionRef.current = null; + if (!sessionMetaPlanMode && autoAppliedPlanSessions.has(resolvedSessionId)) { + autoAppliedPlanSessions.delete(resolvedSessionId); return; } - if (autoAppliedPlanSessionRef.current === resolvedSessionId) return; + if (autoAppliedPlanSessions.has(resolvedSessionId)) return; // Only auto-apply if both the session metadata AND the current step agree on plan mode. // sessionMetaPlanMode can be stale (deepMerge hydration preserves deleted keys), // so we cross-check with the step's actual configuration. if (sessionMetaPlanMode && currentStepHasPlanMode) { - autoAppliedPlanSessionRef.current = resolvedSessionId; + autoAppliedPlanSessions.add(resolvedSessionId); setActiveDocument(resolvedSessionId, { type: "plan", taskId }); applyBuiltInPreset("plan"); setPlanMode(resolvedSessionId, true); diff --git a/apps/web/components/task/dockview-session-tabs.ts b/apps/web/components/task/dockview-session-tabs.ts index 03b46162af..f294ba874d 100644 --- a/apps/web/components/task/dockview-session-tabs.ts +++ b/apps/web/components/task/dockview-session-tabs.ts @@ -242,6 +242,58 @@ function reconcileRemovedSessionPanels( const EMPTY_SESSION_IDS_KEY = ""; +/** + * Drop the generic "chat" placeholder once a real session is active. + * Skips removal in maximized state to preserve the saved maximize layout. + * Returns true when callers should continue ensuring session panels; + * false when the maximized state intentionally suppresses them. + */ +function prepareLayoutForSessionPanels(api: DockviewApi): boolean { + const preMaximizeLayout = useDockviewStore.getState().preMaximizeLayout; + const chatPanel = api.getPanel("chat"); + if (chatPanel && !preMaximizeLayout) { + api.removePanel(chatPanel); + } + // In maximized state, session panels are intentionally absent from the + // layout — they'll be restored when the user exits maximize. + return preMaximizeLayout === null; +} + +/** + * Decide whether to force-activate the session panel after it (and any + * sibling tabs) have been ensured. + * + * - It was just created by `ensureSessionPanel` (no prior dockview state + * to honor), or + * - The hook is mounting for the first time (initial page load) — preserve + * the long-standing behavior of focusing the agent tab so the chat is + * visible immediately, even when a saved layout had a different center + * tab active, or + * - The user switched sessions within the same task (intra-task switch + * where dockview hasn't re-activated the new session for us). + * + * After an env (task) switch the prev refs are populated and the task + * changed — `restoreSavedActiveViews` has already applied the saved active + * panel for the incoming task, so calling setActive here would override it + * and force the agent tab on top of whatever the user had focused. + */ +function shouldActivateSessionPanel(args: { + sessionPanelExistedBefore: boolean; + prevTaskId: string | null; + prevSessionId: string | null; + currentTaskId: string | null; + currentSessionId: string; +}): boolean { + const { sessionPanelExistedBefore, prevTaskId, prevSessionId, currentTaskId, currentSessionId } = + args; + if (!sessionPanelExistedBefore) return true; + const isFirstMount = prevTaskId === null && prevSessionId === null; + if (isFirstMount) return true; + const taskChanged = prevTaskId !== currentTaskId; + const sessionChanged = prevSessionId !== currentSessionId; + return sessionChanged && !taskChanged; +} + /** * Open a dockview tab for every session of the active task and keep them in sync * with the store. @@ -256,6 +308,8 @@ const EMPTY_SESSION_IDS_KEY = ""; */ export function useAutoSessionTab(effectiveSessionId: string | null) { const sessionTabCreatedRef = useRef>(new Set()); + const prevTaskIdRef = useRef(null); + const prevSessionIdRef = useRef(null); const appStore = useAppStoreApi(); // Key-based dependency so the effect re-runs when the task's session list @@ -298,17 +352,7 @@ export function useAutoSessionTab(effectiveSessionId: string | null) { // be re-created by ensureSessionPanel. if (!currentSessionIds.includes(effectiveSessionId)) return; - // Remove the generic "chat" placeholder as soon as a real session is - // active — per-session tabs replace it. Skip in maximized state to - // preserve the saved maximize layout. - const chatPanel = api.getPanel("chat"); - if (chatPanel && !useDockviewStore.getState().preMaximizeLayout) { - api.removePanel(chatPanel); - } - - // In maximized state, session panels are intentionally absent from the - // layout — they'll be restored when the user exits maximize. - if (useDockviewStore.getState().preMaximizeLayout !== null) { + if (!prepareLayoutForSessionPanels(api)) { sessionTabCreatedRef.current.add(effectiveSessionId); return; } @@ -316,6 +360,7 @@ export function useAutoSessionTab(effectiveSessionId: string | null) { const initialPosition = resolveInitialPosition(api); // Active panel first so its group becomes the anchor for siblings. + const sessionPanelExistedBefore = !!api.getPanel(`session:${effectiveSessionId}`); ensureSessionPanel( api, effectiveSessionId, @@ -325,7 +370,16 @@ export function useAutoSessionTab(effectiveSessionId: string | null) { ); const activePanel = api.getPanel(`session:${effectiveSessionId}`); if (activePanel) { - activePanel.api.setActive(); + const shouldActivate = shouldActivateSessionPanel({ + sessionPanelExistedBefore, + prevTaskId: prevTaskIdRef.current, + prevSessionId: prevSessionIdRef.current, + currentTaskId: tid ?? null, + currentSessionId: effectiveSessionId, + }); + if (shouldActivate) { + activePanel.api.setActive(); + } useDockviewStore.setState({ centerGroupId: activePanel.group.id }); } @@ -337,5 +391,7 @@ export function useAutoSessionTab(effectiveSessionId: string | null) { if (sid === effectiveSessionId) continue; ensureSessionPanel(api, sid, siblingAnchor, true, sessionTabCreatedRef.current); } + prevTaskIdRef.current = tid ?? null; + prevSessionIdRef.current = effectiveSessionId; }, [effectiveSessionId, sessionIdsKey, appStore]); } diff --git a/apps/web/components/task/task-session-sidebar.tsx b/apps/web/components/task/task-session-sidebar.tsx index 175260aa74..6d5864e2c1 100644 --- a/apps/web/components/task/task-session-sidebar.tsx +++ b/apps/web/components/task/task-session-sidebar.tsx @@ -567,7 +567,6 @@ export const TaskSessionSidebar = memo(function TaskSessionSidebar({ () => applyView(displayTasks, effectiveView, { pinnedTaskIds, orderedTaskIds }), [displayTasks, effectiveView, pinnedTaskIds, orderedTaskIds], ); - return ( diff --git a/apps/web/e2e/tests/git/diff-update.spec.ts b/apps/web/e2e/tests/git/diff-update.spec.ts index 2dacf33b3e..f43014747c 100644 --- a/apps/web/e2e/tests/git/diff-update.spec.ts +++ b/apps/web/e2e/tests/git/diff-update.spec.ts @@ -69,6 +69,15 @@ function seedDiffUpdateTask(testPage: Page, apiClient: ApiClient, seedData: Seed }); } +/** Seed a task for the multi-file scenario (3 files, FIRST_MODIFICATION on all). */ +function seedMultiFileTask(testPage: Page, apiClient: ApiClient, seedData: SeedData) { + return seedTaskWithScenario(testPage, apiClient, seedData, { + title: "Multi-file Diff Update E2E", + scenarioCommand: "/e2e:multi-file-setup", + completionText: "multi-file-setup complete", + }); +} + /** Click the Changes dockview tab. */ async function openChangesTab(testPage: Page) { const changesTab = testPage.locator(".dv-default-tab", { hasText: "Changes" }); @@ -239,6 +248,271 @@ test.describe("Diff update on file change", () => { }); }); +test.describe("File editor auto-update on file change", () => { + test.describe.configure({ retries: 2, timeout: 120_000 }); + + test("editor panel auto-updates without re-opening when file changes", async ({ + testPage, + apiClient, + seedData, + }) => { + // Regression: opening a file in the FileEditorPanel and then having the + // agent modify it should reactively update the editor content WITHOUT the + // user re-clicking the file. + const { session } = await seedDiffUpdateTask(testPage, apiClient, seedData); + + // Open the file in the file editor via the Files tree. + await session.clickTab("Files"); + await expect(session.files).toBeVisible({ timeout: 10_000 }); + const fileRow = session.files.getByText("diff_update_test.txt"); + await expect(fileRow).toBeVisible({ timeout: 10_000 }); + await fileRow.click(); + + // The editor tab should appear in dockview. + const editorTab = testPage.locator(".dv-default-tab", { hasText: "diff_update_test.txt" }); + await expect(editorTab).toBeVisible({ timeout: 10_000 }); + + // Verify initial content shows FIRST_MODIFICATION (default Monaco editor). + const editorContent = testPage.locator(".view-lines").first(); + await expect(editorContent).toContainText("FIRST_MODIFICATION", { timeout: 15_000 }); + + // Switch to chat and trigger the second modification. + await session.clickSessionChatTab(); + await session.sendMessage("/e2e:diff-update-modify"); + await expect( + session.chat.getByText("diff-update-modify complete", { exact: false }), + ).toBeVisible({ timeout: 45_000 }); + + // Click the editor tab back to view it (do NOT re-open from file tree). + // The panel is still mounted, just not the active tab. + await editorTab.click(); + + // The editor should auto-update with SECOND_MODIFICATION content. + // Re-query .view-lines since DOM may have re-rendered after tab switch. + const updatedEditorContent = testPage.locator(".view-lines").first(); + await expect(updatedEditorContent).toContainText("SECOND_MODIFICATION", { timeout: 30_000 }); + await expect(updatedEditorContent).toContainText("ALSO_CHANGED", { timeout: 15_000 }); + + // FIRST_MODIFICATION should be gone. + await expect(updatedEditorContent).not.toContainText("FIRST_MODIFICATION", { timeout: 5_000 }); + }); + + test("editor + diff panels auto-update while agent is streaming (mid-turn)", async ({ + testPage, + apiClient, + seedData, + }) => { + // Reproduces the user-reported bug: open both editor and diff from the + // Changes tab, then have the agent modify the file MID-TURN (still + // streaming). Both panels must auto-update within a few seconds while + // the agent's turn is still active — without re-opening the file. + const { session } = await seedDiffUpdateTask(testPage, apiClient, seedData); + + // Open the diff panel from the Changes tab. + await openChangesTab(testPage); + await openFileDiff(testPage, "diff_update_test.txt"); + const diffsContainer = getDiffsContainer(testPage); + await expect(diffsContainer).toBeVisible({ timeout: 15_000 }); + await expect(diffsContainer.getByText("FIRST_MODIFICATION", { exact: true })).toBeVisible({ + timeout: 15_000, + }); + + // Also open the file editor for the same file via the Files tree. + await session.clickTab("Files"); + await expect(session.files).toBeVisible({ timeout: 5_000 }); + const fileRow = session.files.getByText("diff_update_test.txt"); + await expect(fileRow).toBeVisible({ timeout: 10_000 }); + await fileRow.click(); + const editorTab = testPage.locator(".dv-default-tab[type='file-editor']", { + hasText: "diff_update_test.txt", + }); + await expect(editorTab).toBeVisible({ timeout: 10_000 }); + const editorContent = testPage.locator(".view-lines").first(); + await expect(editorContent).toContainText("FIRST_MODIFICATION", { timeout: 15_000 }); + + // Trigger the streaming scenario: agent will write the file mid-turn and + // keep emitting text for ~6s afterwards. Do NOT wait for completion — + // we want to assert updates while the turn is still streaming. + await session.clickSessionChatTab(); + await session.sendMessage("/e2e:diff-update-streaming"); + + // Wait for the agent to confirm it has started — turn is now live. + await expect(session.chat.getByText("starting work", { exact: false })).toBeVisible({ + timeout: 30_000, + }); + + // While the agent is still mid-turn (it has ~6s of trailing delay), both + // panels must reflect the new content. This is the user's bug scenario. + // Click back to the editor tab to make assertions. + await editorTab.click(); + const liveEditorContent = testPage.locator(".view-lines").first(); + await expect(liveEditorContent).toContainText("SECOND_MODIFICATION", { timeout: 8_000 }); + await expect(liveEditorContent).toContainText("ALSO_CHANGED", { timeout: 5_000 }); + + // Switch to the diff tab and assert it also updated. + const diffTab = testPage.locator(".dv-default-tab[type='file-diff']", { + hasText: "diff_update_test.txt", + }); + await expect(diffTab).toBeVisible({ timeout: 10_000 }); + await diffTab.click(); + const liveDiffsContainer = getDiffsContainer(testPage); + await expect(liveDiffsContainer.getByText("SECOND_MODIFICATION", { exact: true })).toBeVisible({ + timeout: 8_000, + }); + await expect(liveDiffsContainer.getByText("ALSO_CHANGED", { exact: true })).toBeVisible({ + timeout: 5_000, + }); + }); +}); + +test.describe("Multi-file editor + diff auto-update", () => { + test.describe.configure({ retries: 2, timeout: 180_000 }); + + test("diff panel auto-updates across all 3 files during a single multi-file streaming turn", async ({ + testPage, + apiClient, + seedData, + }) => { + // Reproduces the user-reported "multiple files / diffs opened" case. + // The Changes panel mounts useFileEditors. Opening a diff for file_a + // mounts another instance. A single multi-file streaming modify turn + // must propagate to gitStatus so: + // - The currently open diff (file_a) auto-updates mid-turn. + // - Switching the preview to file_b shows file_b's NEW diff immediately + // (not the pre-turn FIRST_MODIFICATION). + // - Same for file_c. + // This exercises the gitStatus-driven re-render path without the user + // re-triggering anything per-file. + const { session } = await seedMultiFileTask(testPage, apiClient, seedData); + const fileA = "multi_a.txt"; + const fileB = "multi_b.txt"; + const fileC = "multi_c.txt"; + + // Open the diff preview for file_a from the Changes panel. + await openChangesTab(testPage); + await openFileDiff(testPage, fileA); + const diffsContainer = getDiffsContainer(testPage); + await expect(diffsContainer).toBeVisible({ timeout: 15_000 }); + await expect(diffsContainer.getByText("FIRST_MODIFICATION", { exact: true })).toBeVisible({ + timeout: 15_000, + }); + + // Trigger the multi-file streaming modification. Don't wait for completion. + await session.clickSessionChatTab(); + await session.sendMessage("/e2e:multi-file-modify"); + await expect(session.chat.getByText("starting work", { exact: false })).toBeVisible({ + timeout: 30_000, + }); + + // Click back to the file_a diff tab so its panel becomes the visible one. + const diffTabA = testPage + .locator(".dv-default-tab") + .filter({ hasText: `Diff [${fileA}]` }) + .first(); + await diffTabA.click(); + + // While the agent is still streaming, the file_a diff must reflect + // SECOND_MODIFICATION + ALSO_CHANGED_0. + await expect(diffsContainer.getByText("SECOND_MODIFICATION", { exact: true })).toBeVisible({ + timeout: 15_000, + }); + await expect(diffsContainer.getByText("ALSO_CHANGED_0", { exact: true })).toBeVisible({ + timeout: 5_000, + }); + + // Swap the diff preview to file_b — the gitStatus-driven content must + // already have SECOND_MODIFICATION available without re-running the agent. + await openChangesTab(testPage); + await openFileDiff(testPage, fileB); + await expect(diffsContainer.getByText("ALSO_CHANGED_1", { exact: true })).toBeVisible({ + timeout: 15_000, + }); + + // And again for file_c. + await openChangesTab(testPage); + await openFileDiff(testPage, fileC); + await expect(diffsContainer.getByText("ALSO_CHANGED_2", { exact: true })).toBeVisible({ + timeout: 15_000, + }); + }); +}); + +test.describe("User-save then diff view (colleague repro)", () => { + // Intermittent bug: after agent modifies a file, user opens it in editor, + // edits + saves, then switches to diff view \u2014 the diff panel shows the + // pre-save content (agent's edit only, not the user's save). Workaround + // reported: leave the task and re-enter. We try to repro by driving the + // exact UI sequence and asserting the diff contains the user's marker. + test.describe.configure({ retries: 2, timeout: 120_000 }); + + test("diff shows user's edit after open-edit-save sequence", async ({ + testPage, + apiClient, + seedData, + }) => { + const USER_MARKER = "USER_EDIT_MARKER_42"; + const { session } = await seedDiffUpdateTask(testPage, apiClient, seedData); + + // Step 1: agent has already modified diff_update_test.txt with FIRST_MODIFICATION. + // Wait for the Changes panel auto-activate (fires once gitStatus 0\u2192N) to + // settle before we click Files \u2014 otherwise the auto-activate races with + // our click and Files ends up inactive. Match any "Changes (N)" count so we + // don't fail when the badge briefly reads a different value. + await expect(testPage.locator(".dv-default-tab", { hasText: /^Changes \(\d+\)/ })).toBeVisible({ + timeout: 30_000, + }); + // Click Files and verify the file row becomes visible. If a late + // auto-activate stole focus back to Changes, click Files again. + const fileRow = session.fileTreeNode("diff_update_test.txt"); + await expect + .poll( + async () => { + await session.clickTab("Files"); + return await fileRow.isVisible(); + }, + { timeout: 20_000, intervals: [500, 1000, 2000] }, + ) + .toBe(true); + await fileRow.click(); + const editorTab = testPage.locator(".dv-default-tab[type='file-editor']", { + hasText: "diff_update_test.txt", + }); + await expect(editorTab).toBeVisible({ timeout: 10_000 }); + const editorContent = testPage.locator(".view-lines").first(); + await expect(editorContent).toContainText("FIRST_MODIFICATION", { timeout: 30_000 }); + + // Step 2: type into Monaco \u2014 add a unique marker line at the end. + // Click the view-lines area to focus the editor (Monaco's hidden textarea + // captures input but isn't directly focusable across all browsers). + await editorContent.click(); + const modifier = process.platform === "darwin" ? "Meta" : "Control"; + // Move caret to end of document, then add a new line with our marker. + await testPage.keyboard.press(`${modifier}+End`); + await testPage.keyboard.press("End"); + await testPage.keyboard.type(`\n${USER_MARKER}`); + // Confirm the marker is present in the editor before saving. + await expect(editorContent).toContainText(USER_MARKER, { timeout: 5_000 }); + + // Step 3: save with Cmd/Ctrl+S. + await testPage.keyboard.press(`${modifier}+s`); + + // Step 4: open the diff for the same file. + await openChangesTab(testPage); + await openFileDiff(testPage, "diff_update_test.txt"); + + // Step 5: the diff must reflect the user's marker. + const diffsContainer = getDiffsContainer(testPage); + await expect(diffsContainer).toBeVisible({ timeout: 15_000 }); + await expect(diffsContainer.getByText(USER_MARKER, { exact: true })).toBeVisible({ + timeout: 15_000, + }); + // FIRST_MODIFICATION should still be present (agent's earlier edit). + await expect(diffsContainer.getByText("FIRST_MODIFICATION", { exact: true })).toBeVisible({ + timeout: 5_000, + }); + }); +}); + test.describe("Untracked file diff update", () => { test.describe.configure({ retries: 2, timeout: 120_000 }); diff --git a/apps/web/e2e/tests/layout/preview-tab-session-switch.spec.ts b/apps/web/e2e/tests/layout/preview-tab-session-switch.spec.ts index 7742822fe7..725c725ce4 100644 --- a/apps/web/e2e/tests/layout/preview-tab-session-switch.spec.ts +++ b/apps/web/e2e/tests/layout/preview-tab-session-switch.spec.ts @@ -1,6 +1,8 @@ import path from "node:path"; import { expect, type Page } from "@playwright/test"; import { test } from "../../fixtures/test-base"; +import type { SeedData } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; import { SessionPage } from "../../pages/session-page"; import { GitHelper, makeGitEnv } from "../../helpers/git-helper"; import { KanbanPage } from "../../pages/kanban-page"; @@ -23,6 +25,39 @@ async function openFileInPreview(page: Page, session: SessionPage, filename: str } } +async function seedFinishedTask( + apiClient: ApiClient, + seedData: SeedData, + title: string, +): Promise<{ id: string }> { + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + title, + seedData.agentProfileId, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(task.id); + return DONE_STATES.includes(sessions[0]?.state ?? ""); + }, + { timeout: 30_000, message: `Waiting for ${title} session to finish` }, + ) + .toBe(true); + return task; +} + +/** `.dv-tab` is the wrapper dockview toggles `dv-active-tab` on. */ +function tabWrapperByText(page: Page, text: string) { + return page.locator(".dv-tab", { has: page.locator(".dv-default-tab", { hasText: text }) }); +} + test.describe("Preview tab survives session switch", () => { test("preview tab persists after switching tasks and back", async ({ testPage, @@ -122,4 +157,112 @@ test.describe("Preview tab survives session switch", () => { const fileTab = testPage.locator(".dv-default-tab").filter({ hasText: FILE_A }); await expect(fileTab).toHaveCount(1, { timeout: 15_000 }); }); + + test("promoted file tab does not duplicate after task switch round-trip", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + test.setTimeout(120_000); + + // Worker-scoped backend reuses the same repo across tests in the file, so each + // test seeds its own filename to avoid "nothing to commit" collisions. + const FILE_PROMOTED = "promoted.ts"; + const git = new GitHelper( + path.join(backend.tmpDir, "repos", "e2e-repo"), + makeGitEnv(backend.tmpDir), + ); + git.createFile(FILE_PROMOTED, "// promoted content"); + git.stageAll(); + git.commit("seed promoted"); + + const taskA = await seedFinishedTask(apiClient, seedData, "Promote Round-Trip A"); + const taskB = await seedFinishedTask(apiClient, seedData, "Promote Round-Trip B"); + + const kanban = new KanbanPage(testPage); + await kanban.goto(); + await kanban.taskCardByTitle("Promote Round-Trip A").click(); + await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 }); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await expect(session.idleInput()).toBeVisible({ timeout: 30_000 }); + + // Open file in preview, then promote via double-click. The panel keeps id + // `preview:file-editor` with `params.promoted=true` until another file is + // opened, so we sync on the italic class disappearing rather than the testid. + await openFileInPreview(testPage, session, FILE_PROMOTED); + const previewTab = testPage.getByTestId("preview-tab-file-editor"); + await expect(previewTab).toBeVisible({ timeout: 15_000 }); + await previewTab.dblclick(); + await expect(previewTab).not.toHaveClass(/italic/, { timeout: 10_000 }); + + // Round-trip: switch to Task B, then back to Task A (forces fromJSON restore). + await session.clickTaskInSidebar("Promote Round-Trip B"); + await expect(testPage).toHaveURL((url) => url.pathname.includes(taskB.id), { timeout: 15_000 }); + await session.waitForLoad(); + + await session.clickTaskInSidebar("Promote Round-Trip A"); + await expect(testPage).toHaveURL((url) => url.pathname.includes(taskA.id), { timeout: 15_000 }); + await expect(testPage.locator(".dv-dockview")).toBeVisible({ timeout: 15_000 }); + + // Regression: previously `loadAndRestoreTabs` re-added `file:promoted.ts` on + // top of the snapshot's restored `preview:file-editor` (which was persisted + // as `pinned: true` because it was a promoted preview), producing two tabs + // with the same filename. Strict count of 1 catches the duplicate. + const fileTab = testPage.locator(".dv-default-tab").filter({ hasText: FILE_PROMOTED }); + await expect(fileTab).toHaveCount(1, { timeout: 15_000 }); + }); + + test("active center tab is preserved across task switch round-trip", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + test.setTimeout(120_000); + + const FILE_ACTIVE = "active-file.ts"; + const git = new GitHelper( + path.join(backend.tmpDir, "repos", "e2e-repo"), + makeGitEnv(backend.tmpDir), + ); + git.createFile(FILE_ACTIVE, "// active content"); + git.stageAll(); + git.commit("seed active"); + + const taskA = await seedFinishedTask(apiClient, seedData, "Active Tab Round-Trip A"); + const taskB = await seedFinishedTask(apiClient, seedData, "Active Tab Round-Trip B"); + + const kanban = new KanbanPage(testPage); + await kanban.goto(); + await kanban.taskCardByTitle("Active Tab Round-Trip A").click(); + await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 }); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await expect(session.idleInput()).toBeVisible({ timeout: 30_000 }); + + // Open file in preview — it auto-activates, but we click it explicitly to + // make the intent clear and to be robust against any default-active changes. + await openFileInPreview(testPage, session, FILE_ACTIVE); + const fileTabWrapper = tabWrapperByText(testPage, FILE_ACTIVE); + await fileTabWrapper.click(); + await expect(fileTabWrapper).toHaveClass(/dv-active-tab/, { timeout: 10_000 }); + + // Round-trip + await session.clickTaskInSidebar("Active Tab Round-Trip B"); + await expect(testPage).toHaveURL((url) => url.pathname.includes(taskB.id), { timeout: 15_000 }); + await session.waitForLoad(); + + await session.clickTaskInSidebar("Active Tab Round-Trip A"); + await expect(testPage).toHaveURL((url) => url.pathname.includes(taskA.id), { timeout: 15_000 }); + await expect(testPage.locator(".dv-dockview")).toBeVisible({ timeout: 15_000 }); + + // Regression: previously `useAutoSessionTab` always called `setActive()` on + // the session panel after fromJSON, overriding the restored active state. + // The file tab the user left active must remain active across the round-trip. + // (Only one tab per group can be `dv-active-tab` so this implicitly asserts + // the session panel is not active.) + await expect(fileTabWrapper).toHaveClass(/dv-active-tab/, { timeout: 15_000 }); + }); }); diff --git a/apps/web/hooks/file-editors-sync.test.ts b/apps/web/hooks/file-editors-sync.test.ts new file mode 100644 index 0000000000..7899d9bafa --- /dev/null +++ b/apps/web/hooks/file-editors-sync.test.ts @@ -0,0 +1,273 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook, waitFor, cleanup } from "@testing-library/react"; +import { useRef } from "react"; +import type { FileEditorState } from "@/lib/state/dockview-store"; +import type { FileInfo } from "@/lib/state/store"; +import type { GitStatusEntry } from "@/lib/state/slices/session-runtime/types"; + +const mockRequestFileContent = vi.fn(); +const mockGetWebSocketClient = vi.fn(); +let openFilesMap = new Map(); + +vi.mock("@/lib/ws/workspace-files", () => ({ + requestFileContent: (...args: unknown[]) => mockRequestFileContent(...args), +})); + +vi.mock("@/lib/ws/connection", () => ({ + getWebSocketClient: () => mockGetWebSocketClient(), +})); + +vi.mock("@/lib/state/dockview-store", () => ({ + useDockviewStore: { + getState: () => ({ openFiles: openFilesMap }), + }, +})); + +vi.mock("./use-file-save-delete", () => ({ + updatePanelAfterSave: vi.fn(), +})); + +vi.mock("@/lib/utils/file-diff", () => ({ + calculateHash: async (s: string) => `h:${s.length}:${s.slice(0, 8)}`, +})); + +import { + buildGitFileSignature, + syncOpenFileFromWorkspace, + useOpenFileWorkspaceSync, +} from "./file-editors-sync"; + +const FAKE_CLIENT = {} as ReturnType; +const SESSION_ID = "sess-1"; +const PATH = "src/foo.ts"; + +function seedOpenFile(state: Partial = {}) { + openFilesMap = new Map([ + [ + PATH, + { + path: PATH, + name: "foo.ts", + content: "v1", + originalContent: "v1", + originalHash: "h:2:v1", + isDirty: false, + ...state, + }, + ], + ]); +} + +describe("buildGitFileSignature", () => { + it("returns a stable __clean__ marker for files absent from git status", () => { + expect(buildGitFileSignature(undefined)).toBe("__clean__"); + }); + + it("changes when the diff content changes (agent-edit detection)", () => { + const before = buildGitFileSignature({ + path: PATH, + status: "modified", + staged: false, + additions: 1, + deletions: 0, + diff: "@@ -1 +1 @@\n-v1\n+v2", + } as FileInfo); + const after = buildGitFileSignature({ + path: PATH, + status: "modified", + staged: false, + additions: 1, + deletions: 0, + diff: "@@ -1 +1 @@\n-v1\n+v3", + } as FileInfo); + expect(before).not.toBe(after); + }); +}); + +describe("syncOpenFileFromWorkspace", () => { + let updateFileState: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + updateFileState = vi.fn(); + }); + + it("replaces editor content when the agent modifies a clean file", async () => { + // User opens foo.ts (content "v1"). Agent then edits it on disk to "v2". + // The hook detects the git status change and calls this sync function; + // we expect the editor buffer to be replaced in place. + seedOpenFile({ content: "v1", originalContent: "v1", originalHash: "h:2:v1" }); + mockRequestFileContent.mockResolvedValueOnce({ + content: "v2", + is_binary: false, + resolved_path: PATH, + }); + + await syncOpenFileFromWorkspace({ + client: FAKE_CLIENT, + sessionId: SESSION_ID, + path: PATH, + updateFileState, + }); + + expect(mockRequestFileContent).toHaveBeenCalledWith(FAKE_CLIENT, SESSION_ID, PATH); + expect(updateFileState).toHaveBeenCalledTimes(1); + expect(updateFileState).toHaveBeenCalledWith( + PATH, + expect.objectContaining({ + content: "v2", + originalContent: "v2", + isDirty: false, + hasRemoteUpdate: false, + }), + ); + }); + + it("flags hasRemoteUpdate (instead of clobbering) when the user has unsaved edits", async () => { + // User has typed local edits (isDirty=true). Agent edits on disk in + // parallel. We must NOT silently replace the user's buffer; we surface + // a Reload affordance via hasRemoteUpdate. + seedOpenFile({ + content: "user-typed", + originalContent: "v1", + originalHash: "h:2:v1", + isDirty: true, + }); + mockRequestFileContent.mockResolvedValueOnce({ + content: "v2-from-agent", + is_binary: false, + resolved_path: PATH, + }); + + await syncOpenFileFromWorkspace({ + client: FAKE_CLIENT, + sessionId: SESSION_ID, + path: PATH, + updateFileState, + }); + + expect(updateFileState).toHaveBeenCalledTimes(1); + expect(updateFileState).toHaveBeenCalledWith( + PATH, + expect.objectContaining({ + hasRemoteUpdate: true, + remoteContent: "v2-from-agent", + }), + ); + }); + + it("is a no-op when remote content matches the editor buffer", async () => { + seedOpenFile({ content: "v1", originalContent: "v1", originalHash: "h:2:v1" }); + mockRequestFileContent.mockResolvedValueOnce({ + content: "v1", + is_binary: false, + resolved_path: PATH, + }); + + await syncOpenFileFromWorkspace({ + client: FAKE_CLIENT, + sessionId: SESSION_ID, + path: PATH, + updateFileState, + }); + + expect(updateFileState).not.toHaveBeenCalled(); + }); +}); + +function makeStatus(files: Record, timestamp: string): GitStatusEntry { + return { + branch: "main", + remote_branch: null, + modified: [], + added: [], + deleted: [], + untracked: [], + renamed: [], + ahead: 0, + behind: 0, + files, + timestamp, + } as GitStatusEntry; +} + +function modifiedFile(diff: string): FileInfo { + return { + path: PATH, + status: "modified", + staged: false, + additions: 1, + deletions: 1, + diff, + }; +} + +type SyncProps = { + gitStatus: GitStatusEntry | undefined; + openFiles: Map; + updateFileState: (path: string, updates: Partial) => void; +}; + +function renderSyncHook(initial: SyncProps) { + return renderHook( + (props: SyncProps) => { + const activeSessionIdRef = useRef(SESSION_ID); + const gitFileSignaturesRef = useRef>(new Map()); + useOpenFileWorkspaceSync({ + gitStatus: props.gitStatus, + openFiles: props.openFiles, + updateFileState: props.updateFileState, + activeSessionIdRef, + gitFileSignaturesRef, + }); + }, + { initialProps: initial }, + ); +} + +describe("useOpenFileWorkspaceSync", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockGetWebSocketClient.mockReturnValue(FAKE_CLIENT); + }); + + it("refetches the open editor buffer when the agent edits its file", async () => { + // User has foo.ts open showing "v1". Initial git status has no entry for + // foo.ts (clean). Then the agent edits the file: a fresh git status arrives + // with a modified entry. We expect the hook to fetch new content and + // call updateFileState so the editor displays "v2". + seedOpenFile({ content: "v1", originalContent: "v1", originalHash: "h:2:v1" }); + const updateFileState = vi.fn(); + mockRequestFileContent.mockResolvedValue({ + content: "v2", + is_binary: false, + resolved_path: PATH, + }); + + const initialStatus = makeStatus({}, "2026-05-08T11:00:00.000Z"); + const { rerender } = renderSyncHook({ + gitStatus: initialStatus, + openFiles: openFilesMap, + updateFileState, + }); + + // First render baselines the signature; no fetch yet. + expect(mockRequestFileContent).not.toHaveBeenCalled(); + + const editedStatus = makeStatus( + { [PATH]: modifiedFile("@@ -1 +1 @@\n-v1\n+v2") }, + "2026-05-08T11:00:02.000Z", + ); + rerender({ gitStatus: editedStatus, openFiles: openFilesMap, updateFileState }); + + await waitFor(() => expect(mockRequestFileContent).toHaveBeenCalledTimes(1)); + await waitFor(() => + expect(updateFileState).toHaveBeenCalledWith( + PATH, + expect.objectContaining({ content: "v2", originalContent: "v2" }), + ), + ); + + cleanup(); + }); +}); diff --git a/apps/web/hooks/file-editors-sync.ts b/apps/web/hooks/file-editors-sync.ts new file mode 100644 index 0000000000..5ad2915fef --- /dev/null +++ b/apps/web/hooks/file-editors-sync.ts @@ -0,0 +1,156 @@ +"use client"; + +import { useEffect } from "react"; +import type { FileEditorState } from "@/lib/state/dockview-store"; +import { useDockviewStore } from "@/lib/state/dockview-store"; +import { getWebSocketClient } from "@/lib/ws/connection"; +import { requestFileContent } from "@/lib/ws/workspace-files"; +import { calculateHash } from "@/lib/utils/file-diff"; +import { updatePanelAfterSave } from "./use-file-save-delete"; +import type { FileInfo } from "@/lib/state/store"; +import type { GitStatusEntry } from "@/lib/state/slices/session-runtime/types"; + +/** + * Builds a stable signature string from a file's git status entry. The hook + * compares signatures across renders to decide whether the editor's content + * needs re-syncing from the workspace. + */ +export function buildGitFileSignature(file: FileInfo | undefined): string { + if (!file) return "__clean__"; + return [ + file.status ?? "", + file.staged ? "1" : "0", + String(file.additions ?? 0), + String(file.deletions ?? 0), + file.old_path ?? "", + file.diff ?? "", + ].join("|"); +} + +export type SyncOpenFileArgs = { + client: ReturnType; + sessionId: string; + path: string; + updateFileState: (path: string, updates: Partial) => void; +}; + +/** + * Re-fetches file content from the workspace and reconciles it with the open + * editor buffer. Behavior depends on dirty state: + * - Clean buffer: replaces `content` + `originalContent` so the editor shows + * the latest disk state without losing user focus/cursor. + * - Dirty buffer with matching remote content: clears dirty flag (the user's + * edits happen to equal what landed on disk). + * - Dirty buffer with different remote content: surfaces a "Reload" affordance + * via `hasRemoteUpdate` so the user explicitly chooses to clobber edits. + */ +export async function syncOpenFileFromWorkspace({ + client, + sessionId, + path, + updateFileState, +}: SyncOpenFileArgs): Promise { + if (!client) return; + try { + const response = await requestFileContent(client, sessionId, path); + const latest = useDockviewStore.getState().openFiles.get(path); + if (!latest) return; + const remoteHash = await calculateHash(response.content); + + if (latest.isDirty) { + if (response.content === latest.content) { + updateFileState(path, { + originalContent: response.content, + originalHash: remoteHash, + isDirty: false, + hasRemoteUpdate: false, + remoteContent: undefined, + remoteOriginalHash: undefined, + }); + updatePanelAfterSave(path, latest.name); + return; + } + if (latest.hasRemoteUpdate && latest.remoteContent === response.content) return; + updateFileState(path, { + hasRemoteUpdate: true, + remoteContent: response.content, + remoteOriginalHash: remoteHash, + }); + return; + } + + if ( + latest.content === response.content && + latest.originalHash === remoteHash && + !latest.hasRemoteUpdate + ) { + return; + } + + updateFileState(path, { + content: response.content, + originalContent: response.content, + originalHash: remoteHash, + isDirty: false, + isBinary: response.is_binary, + hasRemoteUpdate: false, + remoteContent: undefined, + remoteOriginalHash: undefined, + }); + } catch { + // Ignore sync failures; user can continue editing. + } +} + +export type OpenFileWorkspaceSyncParams = { + gitStatus: GitStatusEntry | undefined; + openFiles: Map; + updateFileState: (path: string, updates: Partial) => void; + activeSessionIdRef: React.MutableRefObject; + gitFileSignaturesRef: React.MutableRefObject>; +}; + +/** + * Watches gitStatus + openFiles and refetches open editor buffers from disk + * whenever a file's git signature changes (the agent edited it, the user ran + * git pull, etc.). Skips the very first observation per file so that opening + * a file doesn't trigger an immediate redundant refetch. + */ +export function useOpenFileWorkspaceSync({ + gitStatus, + openFiles, + updateFileState, + activeSessionIdRef, + gitFileSignaturesRef, +}: OpenFileWorkspaceSyncParams) { + useEffect(() => { + const sigMap = gitFileSignaturesRef.current; + for (const path of Array.from(sigMap.keys())) { + if (!openFiles.has(path)) sigMap.delete(path); + } + }, [openFiles, gitFileSignaturesRef]); + + useEffect(() => { + const client = getWebSocketClient(); + const sessionId = activeSessionIdRef.current; + if (!client || !sessionId) return; + + const gitFiles = gitStatus?.files ?? {}; + const sigMap = gitFileSignaturesRef.current; + for (const [path, file] of openFiles.entries()) { + // For symlinks, also check the resolved target path in git status + const gitFileInfo = (gitFiles[path] ?? + (file.resolvedPath ? gitFiles[file.resolvedPath] : undefined)) as FileInfo | undefined; + const nextSignature = buildGitFileSignature(gitFileInfo); + const prevSignature = sigMap.get(path); + if (prevSignature === undefined) { + sigMap.set(path, nextSignature); + continue; + } + if (prevSignature === nextSignature) continue; + + sigMap.set(path, nextSignature); + void syncOpenFileFromWorkspace({ client, sessionId, path, updateFileState }); + } + }, [gitStatus, openFiles, updateFileState, activeSessionIdRef, gitFileSignaturesRef]); +} diff --git a/apps/web/hooks/use-file-editors.ts b/apps/web/hooks/use-file-editors.ts index a96a03b7d2..7f05006e81 100644 --- a/apps/web/hooks/use-file-editors.ts +++ b/apps/web/hooks/use-file-editors.ts @@ -15,9 +15,9 @@ import { calculateHash } from "@/lib/utils/file-diff"; import { useToast } from "@/components/toast-provider"; import { useSessionGitStatus } from "@/hooks/domains/session/use-session-git-status"; import type { FileContentResponse } from "@/lib/types/backend"; -import type { FileInfo } from "@/lib/state/store"; -import { useSaveDeleteActions, updatePanelAfterSave } from "./use-file-save-delete"; +import { useSaveDeleteActions } from "./use-file-save-delete"; import { PREVIEW_FILE_EDITOR_ID } from "@/lib/state/dockview-panel-actions"; +import { useOpenFileWorkspaceSync } from "./file-editors-sync"; // Module-level guard: ensures restoration only runs once across all hook instances let _restoredSessionId: string | null = null; @@ -84,83 +84,6 @@ function buildPersistedTabs( }); } -function buildGitFileSignature(file: FileInfo | undefined): string { - if (!file) return "__clean__"; - return [ - file.status ?? "", - file.staged ? "1" : "0", - String(file.additions ?? 0), - String(file.deletions ?? 0), - file.old_path ?? "", - file.diff ?? "", - ].join("|"); -} - -type SyncOpenFileArgs = { - client: ReturnType; - sessionId: string; - path: string; - updateFileState: (path: string, updates: Partial) => void; -}; - -async function syncOpenFileFromWorkspace({ - client, - sessionId, - path, - updateFileState, -}: SyncOpenFileArgs): Promise { - if (!client) return; - try { - const response = await requestFileContent(client, sessionId, path); - const latest = getOpenFiles().get(path); - if (!latest) return; - const remoteHash = await calculateHash(response.content); - - if (latest.isDirty) { - if (response.content === latest.content) { - updateFileState(path, { - originalContent: response.content, - originalHash: remoteHash, - isDirty: false, - hasRemoteUpdate: false, - remoteContent: undefined, - remoteOriginalHash: undefined, - }); - updatePanelAfterSave(path, latest.name); - return; - } - if (latest.hasRemoteUpdate && latest.remoteContent === response.content) return; - updateFileState(path, { - hasRemoteUpdate: true, - remoteContent: response.content, - remoteOriginalHash: remoteHash, - }); - return; - } - - if ( - latest.content === response.content && - latest.originalHash === remoteHash && - !latest.hasRemoteUpdate - ) { - return; - } - - updateFileState(path, { - content: response.content, - originalContent: response.content, - originalHash: remoteHash, - isDirty: false, - isBinary: response.is_binary, - hasRemoteUpdate: false, - remoteContent: undefined, - remoteOriginalHash: undefined, - }); - } catch { - // Ignore sync failures; user can continue editing. - } -} - type RestoreTabsParams = { activeSessionId: string; savedTabs: Array<{ path: string; name: string; markdownPreview?: boolean; pinned?: boolean }>; @@ -173,6 +96,25 @@ type RestoreTabsParams = { ) => void; }; +/** + * Skip persisted tabs the dockview snapshot already restored (preview slot or + * existing pinned panel). Re-adding with `pin: true` here would create a + * duplicate `file:` alongside the existing `preview:file-editor` — + * see the round-trip task switch where a promoted preview was persisted as + * pinned in localStorage and then re-opened on top of the restored preview. + */ +function isAlreadyRestored( + dockApi: ReturnType["api"], + path: string, +): boolean { + if (!dockApi) return false; + if (dockApi.getPanel(`file:${path}`)) return true; + const previewParams = dockApi.getPanel(PREVIEW_FILE_EDITOR_ID)?.params as + | Record + | undefined; + return previewParams?.previewItemId === path; +} + async function loadAndRestoreTabs(params: RestoreTabsParams, retryCount = 0): Promise { const { activeSessionId, savedTabs, savedActiveTab, setFileState, addFileEditorPanel } = params; const client = getWebSocketClient(); @@ -191,7 +133,9 @@ async function loadAndRestoreTabs(params: RestoreTabsParams, retryCount = 0): Pr // Create all panels immediately so tabs are visible right away. // Content is fetched afterwards; if it fails, `useFileLoader` in // FileEditorPanel retries when the executor becomes available. + const dockApi = useDockviewStore.getState().api; for (const savedTab of savedTabs) { + if (isAlreadyRestored(dockApi, savedTab.path)) continue; addFileEditorPanel(savedTab.path, savedTab.name, { quiet: true, pin: savedTab.pinned, @@ -215,11 +159,8 @@ async function loadAndRestoreTabs(params: RestoreTabsParams, retryCount = 0): Pr /* useFileLoader will retry when executor is ready */ } } - const dockApi = useDockviewStore.getState().api; - if (dockApi) { - const targetPanel = dockApi.getPanel(savedActiveTab); - if (targetPanel) targetPanel.api.setActive(); - } + const targetPanel = dockApi?.getPanel(savedActiveTab); + if (targetPanel) targetPanel.api.setActive(); _restorationInProgress = false; } @@ -311,53 +252,6 @@ function useFileEditorEffects({ }, [api, removeFileState]); } -type OpenFileWorkspaceSyncParams = { - gitStatus: ReturnType; - openFiles: ReturnType["openFiles"]; - updateFileState: (path: string, updates: Partial) => void; - activeSessionIdRef: React.MutableRefObject; - gitFileSignaturesRef: React.MutableRefObject>; -}; - -function useOpenFileWorkspaceSync({ - gitStatus, - openFiles, - updateFileState, - activeSessionIdRef, - gitFileSignaturesRef, -}: OpenFileWorkspaceSyncParams) { - useEffect(() => { - const sigMap = gitFileSignaturesRef.current; - for (const path of Array.from(sigMap.keys())) { - if (!openFiles.has(path)) sigMap.delete(path); - } - }, [openFiles, gitFileSignaturesRef]); - - useEffect(() => { - const client = getWebSocketClient(); - const sessionId = activeSessionIdRef.current; - if (!client || !sessionId) return; - - const gitFiles = gitStatus?.files ?? {}; - const sigMap = gitFileSignaturesRef.current; - for (const [path, file] of openFiles.entries()) { - // For symlinks, also check the resolved target path in git status - const gitFileInfo = (gitFiles[path] ?? - (file.resolvedPath ? gitFiles[file.resolvedPath] : undefined)) as FileInfo | undefined; - const nextSignature = buildGitFileSignature(gitFileInfo); - const prevSignature = sigMap.get(path); - if (prevSignature === undefined) { - sigMap.set(path, nextSignature); - continue; - } - if (prevSignature === nextSignature) continue; - - sigMap.set(path, nextSignature); - void syncOpenFileFromWorkspace({ client, sessionId, path, updateFileState }); - } - }, [gitStatus, openFiles, updateFileState, activeSessionIdRef, gitFileSignaturesRef]); -} - type FileEditorActionsParams = { activeSessionIdRef: React.MutableRefObject; setFileState: (path: string, state: FileEditorState) => void; diff --git a/apps/web/lib/state/dockview-env-switch.test.ts b/apps/web/lib/state/dockview-env-switch.test.ts index 168b04bbb0..5e6b2f25f0 100644 --- a/apps/web/lib/state/dockview-env-switch.test.ts +++ b/apps/web/lib/state/dockview-env-switch.test.ts @@ -23,9 +23,12 @@ vi.mock("./layout-manager", () => ({ import { getEnvLayout } from "@/lib/local-storage"; import { layoutStructuresMatch, savedLayoutMatchesLive } from "./layout-manager"; +const NEW_SESSION_PANEL_ID = "session:new-session"; + function makeMockApi() { return { panels: [], + groups: [], layout: vi.fn(), fromJSON: vi.fn(), getPanel: vi.fn(() => null), @@ -67,6 +70,25 @@ function makeParams(overrides?: Partial): EnvSwitchParams { }; } +function makeTwoLeafSavedLayout( + leaves: Array<{ id: string; views: string[]; activeView: string }>, + activeGroup: string, +): ReturnType { + return { + grid: { + root: { + type: "branch" as const, + data: leaves.map((leaf) => ({ type: "leaf", data: leaf })), + }, + height: 600, + width: 800, + orientation: "HORIZONTAL" as const, + }, + panels: { chat: { contentComponent: "chat" } }, + activeGroup, + } as unknown as ReturnType; +} + describe("performEnvSwitch", () => { beforeEach(() => { vi.clearAllMocks(); @@ -100,7 +122,7 @@ describe("performEnvSwitch", () => { expect(params.api.addPanel).toHaveBeenCalledWith( expect.objectContaining({ - id: "session:new-session", + id: NEW_SESSION_PANEL_ID, component: "chat", params: { sessionId: "new-session" }, }), @@ -109,11 +131,11 @@ describe("performEnvSwitch", () => { it("skips addPanel on the fast path when the session panel already exists", () => { vi.mocked(layoutStructuresMatch).mockReturnValueOnce(true); - const panel = { id: "session:new-session", api: { component: "chat" }, group: { id: "g1" } }; + const panel = { id: NEW_SESSION_PANEL_ID, api: { component: "chat" }, group: { id: "g1" } }; const params = makeParams({ api: { ...makeMockApi(), - getPanel: vi.fn((id: string) => (id === "session:new-session" ? panel : null)), + getPanel: vi.fn((id: string) => (id === NEW_SESSION_PANEL_ID ? panel : null)), } as unknown as EnvSwitchParams["api"], }); @@ -146,4 +168,86 @@ describe("performEnvSwitch", () => { expect(params.api.layout).toHaveBeenCalledWith(800, 600); expect(params.buildDefault).toHaveBeenCalledWith(params.api); }); + + it("preserves the outgoing session panel's tab index when adding the new session on the fast path", () => { + // Regression: the fast-path used to call addPanel with only + // { referenceGroup }, so dockview appended the new session tab to the end + // of the group instead of restoring it to its original slot. + vi.mocked(layoutStructuresMatch).mockReturnValueOnce(true); + const groupPanels = [ + { id: "files", api: { component: "files" } }, + { id: "session:old-session", api: { component: "chat" } }, + { id: "changes", api: { component: "changes" } }, + { id: "terminal-default", api: { component: "terminal" } }, + ]; + const groupId = "center-group"; + const outgoing = { + ...groupPanels[1], + group: { id: groupId, panels: groupPanels }, + }; + const api = { + ...makeMockApi(), + panels: [outgoing], + groups: [{ id: groupId }], + getPanel: vi.fn(() => null), + } as unknown as EnvSwitchParams["api"]; + const params = makeParams({ api }); + + performEnvSwitch(params); + + expect(api.addPanel).toHaveBeenCalledWith( + expect.objectContaining({ + id: NEW_SESSION_PANEL_ID, + position: { referenceGroup: groupId, index: 1 }, + }), + ); + }); +}); + +describe("performEnvSwitch fast-path active view restoration", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("restores saved per-group active tabs on the fast path", () => { + // Regression: the fast path skips fromJSON, so per-group active tabs + // from the outgoing env would persist into the incoming env. The saved + // layout's activeView for each group must be reapplied. + const setActiveRight = vi.fn(); + const setActiveCenter = vi.fn(); + const rightGroup = { + id: "right", + panels: [ + { id: "plan", api: { setActive: setActiveRight } }, + { id: "files", api: { setActive: vi.fn() } }, + ], + }; + const centerGroup = { + id: "center", + panels: [{ id: NEW_SESSION_PANEL_ID, api: { setActive: setActiveCenter } }], + }; + const savedLayout = makeTwoLeafSavedLayout( + [ + { id: "center", views: ["chat"], activeView: "chat" }, + { id: "right", views: ["plan", "files"], activeView: "plan" }, + ], + "right", + ); + vi.mocked(getEnvLayout).mockReturnValueOnce(savedLayout).mockReturnValueOnce(savedLayout); + vi.mocked(savedLayoutMatchesLive).mockReturnValueOnce(true); + const api = { + ...makeMockApi(), + groups: [centerGroup, rightGroup], + getPanel: vi.fn((id: string) => (id === NEW_SESSION_PANEL_ID ? centerGroup.panels[0] : null)), + } as unknown as EnvSwitchParams["api"]; + + performEnvSwitch(makeParams({ api })); + + expect(setActiveRight).toHaveBeenCalled(); + // The saved activeGroup ("right") is applied last, so its setActive must + // be the most recent — otherwise center would steal global focus. + const lastRightCall = setActiveRight.mock.invocationCallOrder.at(-1) ?? 0; + const lastCenterCall = setActiveCenter.mock.invocationCallOrder.at(-1) ?? 0; + expect(lastRightCall).toBeGreaterThan(lastCenterCall); + }); }); diff --git a/apps/web/lib/state/dockview-env-switch.ts b/apps/web/lib/state/dockview-env-switch.ts index d2057357af..1863e10aaa 100644 --- a/apps/web/lib/state/dockview-env-switch.ts +++ b/apps/web/lib/state/dockview-env-switch.ts @@ -40,6 +40,60 @@ function savedLayoutHasEphemeralPanels(serialized: SerializedDockview): boolean return Object.values(panels).some((p) => EPHEMERAL_COMPONENTS.has(p.contentComponent ?? "")); } +/** Walk the serialized grid tree collecting (groupId, activeView) for each leaf. */ +function collectSavedActiveViews( + saved: SerializedDockview, +): Array<{ groupId: string; activeView: string }> { + const out: Array<{ groupId: string; activeView: string }> = []; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const walk = (node: any): void => { + if (!node) return; + if (Array.isArray(node.data)) { + for (const child of node.data) walk(child); + return; + } + const data = node.data; + if (data?.id && data.activeView) out.push({ groupId: data.id, activeView: data.activeView }); + }; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + walk((saved as any).grid?.root); + return out; +} + +/** + * Restore each group's `activeView` from the saved layout. The fast path + * doesn't call `fromJSON`, so per-group active tabs would otherwise carry + * over from the outgoing env (e.g. Task B left "changes" focused in the + * right group, and switching back to Task A would still show "changes" + * even though Task A had "plan" active when it was last saved). + * + * The saved `activeGroup` is applied last so the resulting global focus + * matches what was persisted. + */ +function restoreSavedActiveViews(api: DockviewApi, saved: SerializedDockview): void { + const entries = collectSavedActiveViews(saved); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const savedActiveGroup = (saved as any).activeGroup as string | undefined; + const ordered = savedActiveGroup + ? [ + ...entries.filter((e) => e.groupId !== savedActiveGroup), + ...entries.filter((e) => e.groupId === savedActiveGroup), + ] + : entries; + for (const { groupId, activeView } of ordered) { + const group = api.groups.find((g) => g.id === groupId); + if (!group) continue; + const panel = group.panels.find((p) => p.id === activeView); + if (panel) { + try { + panel.api.setActive(); + } catch { + /* panel may be in a transient state */ + } + } + } +} + export type EnvSwitchParams = { api: DockviewApi; oldEnvId: string | null; @@ -53,30 +107,37 @@ export type EnvSwitchParams = { }; /** - * Remove ephemeral panels (file-editors, diffs, commit-details) from the - * live layout. These are env-scoped panels that shouldn't carry over. + * Predicate matching panels that `removeEphemeralPanels` will close. * - * When `keepSessionId` is provided, session chat panels whose ID does not - * match `session:{keepSessionId}` are also removed. This handles cross-env - * (cross-task) switches where the fast path is taken: without this, session - * tabs from the old env's task remain visible alongside the new task's tab. + * Ephemeral panels (file-editors, diffs, commit-details, etc.) are env-scoped + * and never carry across switches. When `keepSessionId` is provided, chat + * panels for any other session are also removed so the old env's session tab + * doesn't bleed into the new env. Pulled out so `computeSurvivingIndex` can + * reuse the same survival rules without duplicating them. + */ +function shouldRemoveDuringSwitch( + panel: { id: string; api: { component: string } }, + keepSessionId: string | null, +): boolean { + const comp = panel.api.component; + if (EPHEMERAL_COMPONENTS.has(comp)) return true; + if ( + keepSessionId !== null && + comp === "chat" && + panel.id.startsWith("session:") && + panel.id !== `session:${keepSessionId}` + ) { + return true; + } + return false; +} + +/** + * Close every panel that matches `shouldRemoveDuringSwitch`. Used to clear + * env-scoped ephemerals before the new env's panels are restored. */ function removeEphemeralPanels(api: DockviewApi, keepSessionId: string | null): void { - const toRemove = api.panels.filter((p) => { - const comp = p.api.component; - if (EPHEMERAL_COMPONENTS.has(comp)) { - return true; - } - if ( - keepSessionId !== null && - comp === "chat" && - p.id.startsWith("session:") && - p.id !== `session:${keepSessionId}` - ) { - return true; - } - return false; - }); + const toRemove = api.panels.filter((p) => shouldRemoveDuringSwitch(p, keepSessionId)); for (const p of toRemove) { try { p.api.close(); @@ -86,6 +147,26 @@ function removeEphemeralPanels(api: DockviewApi, keepSessionId: string | null): } } +/** + * Given the panels of a group and the id of the panel being replaced, return + * the target tab index for the replacement among the siblings that will + * survive `removeEphemeralPanels`. Returns -1 if the panel isn't in the group. + */ +function computeSurvivingIndex( + groupPanels: readonly { id: string; api: { component: string } }[], + outgoingPanelId: string | undefined, + keepSessionId: string | null, +): number { + if (!outgoingPanelId) return -1; + const idx = groupPanels.findIndex((p) => p.id === outgoingPanelId); + if (idx < 0) return -1; + let count = 0; + for (let i = 0; i < idx; i++) { + if (!shouldRemoveDuringSwitch(groupPanels[i], keepSessionId)) count++; + } + return count; +} + /** * Fast path: check if we can skip `api.fromJSON()` because the layout * structure hasn't changed. Returns group IDs if the fast path was taken, @@ -106,35 +187,66 @@ function tryFastEnvSwitch(params: EnvSwitchParams): LayoutGroupIds | null { if (!structuresMatch) return null; if (saved && savedLayoutHasEphemeralPanels(saved as SerializedDockview)) return null; - const outgoingSessionPanel = api.panels.find( - (p) => p.id.startsWith("session:") || p.api.component === "chat", - ); - const outgoingGroupId = outgoingSessionPanel?.group?.id; + // Prefer the active session panel so multi-session tasks anchor the + // incoming panel to the group the user was looking at, not whichever + // session tab happens to come first in `api.panels` iteration order. + const isSessionPanel = (p: (typeof api.panels)[number]) => + p.id.startsWith("session:") || p.api.component === "chat"; + const outgoingSessionPanel = + api.panels.find((p) => isSessionPanel(p) && p.api.isActive) ?? api.panels.find(isSessionPanel); + const outgoingGroup = outgoingSessionPanel?.group; + const outgoingGroupId = outgoingGroup?.id; + // Capture the session's index among siblings that will survive + // `removeEphemeralPanels`, so the new session panel lands in the same tab + // slot. Without this, dockview appends and the agent tab drifts to the end + // of the group on every cross-task fast-path switch. + const outgoingIndex = outgoingGroup + ? computeSurvivingIndex(outgoingGroup.panels, outgoingSessionPanel?.id, activeSessionId) + : -1; removeEphemeralPanels(api, activeSessionId); - if (activeSessionId && !api.getPanel(`session:${activeSessionId}`)) { - const sidebarPanel = api.getPanel("sidebar"); - let position: import("dockview-react").AddPanelOptions["position"]; - if (outgoingGroupId && api.groups.some((g) => g.id === outgoingGroupId)) { - position = { referenceGroup: outgoingGroupId }; - } else if (sidebarPanel) { - position = { direction: "right" as const, referencePanel: "sidebar" }; - } - api.addPanel({ - id: `session:${activeSessionId}`, - component: "chat", - tabComponent: "sessionTab", - title: "Agent", - params: { sessionId: activeSessionId }, - position, - }); + addIncomingSessionPanel(api, activeSessionId, outgoingGroupId, outgoingIndex); } + // The fast path skips `fromJSON`, so per-group active tabs from the + // outgoing env would otherwise persist into the incoming env. Reapply + // them from the saved layout to match what `fromJSON` would have done. + if (saved) restoreSavedActiveViews(api, saved as SerializedDockview); + api.layout(params.safeWidth, params.safeHeight); return applyLayoutFixups(api); } +/** + * Add the incoming task's session chat panel, restoring it to the same tab + * slot the outgoing session occupied within `outgoingGroupId` when possible. + */ +function addIncomingSessionPanel( + api: DockviewApi, + sessionId: string, + outgoingGroupId: string | undefined, + outgoingIndex: number, +): void { + let position: import("dockview-react").AddPanelOptions["position"]; + if (outgoingGroupId && api.groups.some((g) => g.id === outgoingGroupId)) { + position = + outgoingIndex >= 0 + ? { referenceGroup: outgoingGroupId, index: outgoingIndex } + : { referenceGroup: outgoingGroupId }; + } else if (api.getPanel("sidebar")) { + position = { direction: "right" as const, referencePanel: "sidebar" }; + } + api.addPanel({ + id: `session:${sessionId}`, + component: "chat", + tabComponent: "sessionTab", + title: "Agent", + params: { sessionId }, + position, + }); +} + /** * Switch the dockview layout between task environments. * diff --git a/apps/web/lib/state/dockview-store.ts b/apps/web/lib/state/dockview-store.ts index 06624e437b..08a93ed776 100644 --- a/apps/web/lib/state/dockview-store.ts +++ b/apps/web/lib/state/dockview-store.ts @@ -651,17 +651,42 @@ export const useDockviewStore = create((set, get) => ({ (window as unknown as { __dockviewApi__: DockviewApi | null }).__dockviewApi__ = api; } if (api) { + const resolveFilePath = (panelId: string | undefined): string | null => { + if (!panelId) return null; + if (panelId.startsWith("file:")) return panelId.slice(5); + if (panelId === "preview:file-editor") { + const path = (api.getPanel(panelId)?.params as Record | undefined) + ?.path as string | undefined; + return path ?? null; + } + return null; + }; api.onDidActivePanelChange((event) => { - const id = event?.id; - if (id?.startsWith("file:")) { - set({ activeFilePath: id.slice(5) }); - } else if (id === "preview:file-editor") { - const path = (api.getPanel(id)?.params as Record | undefined)?.path as - | string - | undefined; - set({ activeFilePath: path ?? null }); - } else { - set({ activeFilePath: null }); + set({ activeFilePath: resolveFilePath(event?.id) }); + }); + // Track per-panel param-change subscriptions so they can be disposed when + // the panel is removed (e.g. across env switches that re-create the + // preview panel) instead of relying on dockview's internal cleanup. + const paramSubs = new Map void }>(); + api.onDidAddPanel((panel) => { + // The preview file-editor panel reuses a single dockview panel and swaps + // its `params.path` via `updateParameters` when the user previews a + // different file. Dockview does not refire `onDidActivePanelChange` for + // params-only updates on an already-active panel, so subscribe to the + // panel's own parameter-change event and refresh `activeFilePath`. + if (panel.id !== "preview:file-editor") return; + paramSubs.get(panel.id)?.dispose(); + const sub = panel.api.onDidParametersChange(() => { + if (!panel.api.isActive) return; + set({ activeFilePath: resolveFilePath(panel.id) }); + }); + paramSubs.set(panel.id, sub); + }); + api.onDidRemovePanel((panel) => { + const sub = paramSubs.get(panel.id); + if (sub) { + sub.dispose(); + paramSubs.delete(panel.id); } }); } diff --git a/apps/web/lib/state/layout-manager/merger.test.ts b/apps/web/lib/state/layout-manager/merger.test.ts index e213e0a22e..101682e77b 100644 --- a/apps/web/lib/state/layout-manager/merger.test.ts +++ b/apps/web/lib/state/layout-manager/merger.test.ts @@ -2,6 +2,9 @@ import { describe, it, expect } from "vitest"; import type { LayoutState } from "./types"; import { mergePanelsIntoPreset } from "./merger"; +const SESSION_ABC = "session:abc"; +const SESSION_DEF = "session:def"; + function makeLayout(centerPanels: Array<{ id: string; component: string }>): LayoutState { return { columns: [ @@ -21,6 +24,34 @@ function makeLayout(centerPanels: Array<{ id: string; component: string }>): Lay }; } +function makeLayoutWithSide( + centerPanels: Array<{ id: string; component: string }>, + sideColumnId: string, + sidePanels: Array<{ id: string; component: string }>, +): LayoutState { + return { + columns: [ + { + id: "sidebar", + groups: [{ panels: [{ id: "sidebar", component: "sidebar", title: "Sidebar" }] }], + }, + { + id: "center", + groups: [{ panels: centerPanels.map((p) => ({ ...p, title: p.id })) }], + }, + { + id: sideColumnId, + groups: [{ panels: sidePanels.map((p) => ({ ...p, title: p.id })) }], + }, + ], + }; +} + +function panelIdsIn(state: LayoutState, columnId: string): string[] { + const col = state.columns.find((c) => c.id === columnId); + return col?.groups.flatMap((g) => g.panels.map((p) => p.id)) ?? []; +} + describe("mergePanelsIntoPreset", () => { it("replaces chat panel with session panels when session panels exist", () => { const currentState = makeLayout([{ id: "session:abc123", component: "chat" }]); @@ -49,8 +80,8 @@ describe("mergePanelsIntoPreset", () => { it("preserves multiple session panels and drops chat", () => { const currentState = makeLayout([ - { id: "session:abc", component: "chat" }, - { id: "session:def", component: "chat" }, + { id: SESSION_ABC, component: "chat" }, + { id: SESSION_DEF, component: "chat" }, ]); const targetPreset = makeLayout([{ id: "chat", component: "chat" }]); @@ -59,8 +90,56 @@ describe("mergePanelsIntoPreset", () => { const centerPanels = result.columns.find((c) => c.id === "center")!.groups[0].panels; const panelIds = centerPanels.map((p) => p.id); - expect(panelIds).toContain("session:abc"); - expect(panelIds).toContain("session:def"); + expect(panelIds).toContain(SESSION_ABC); + expect(panelIds).toContain(SESSION_DEF); expect(panelIds).not.toContain("chat"); }); + + it("places side extras (files/changes/terminal) in the side column when one exists", () => { + // Toggling plan mode ON: default layout has files/changes/terminal in + // "right"; the plan preset has a "plan" column. They should land there, + // not pile into center alongside the chat. + const currentState = makeLayoutWithSide([{ id: SESSION_ABC, component: "chat" }], "right", [ + { id: "files", component: "files" }, + { id: "changes", component: "changes" }, + { id: "terminal-default", component: "terminal" }, + ]); + const targetPreset = makeLayoutWithSide([{ id: "chat", component: "chat" }], "plan", [ + { id: "plan", component: "plan" }, + ]); + + const result = mergePanelsIntoPreset(currentState, targetPreset); + + expect(panelIdsIn(result, "center")).toEqual([SESSION_ABC]); + expect(panelIdsIn(result, "plan")).toEqual(["plan", "files", "changes", "terminal-default"]); + }); + + it("places the surviving plan panel in the center column, not the side column", () => { + // Toggling plan mode OFF: the plan preset has plan in its own column, + // which the default preset doesn't have. Plan should follow the chat + // into center rather than being dumped into "right" alongside files. + const currentState = makeLayoutWithSide([{ id: SESSION_ABC, component: "chat" }], "plan", [ + { id: "plan", component: "plan" }, + ]); + const targetPreset = makeLayoutWithSide([{ id: "chat", component: "chat" }], "right", [ + { id: "files", component: "files" }, + { id: "changes", component: "changes" }, + ]); + + const result = mergePanelsIntoPreset(currentState, targetPreset); + + expect(panelIdsIn(result, "center")).toEqual([SESSION_ABC, "plan"]); + expect(panelIdsIn(result, "right")).toEqual(["files", "changes"]); + }); + + it("falls back to center for side extras when the target preset has no side column", () => { + const currentState = makeLayoutWithSide([{ id: SESSION_ABC, component: "chat" }], "right", [ + { id: "files", component: "files" }, + ]); + const targetPreset = makeLayout([{ id: "chat", component: "chat" }]); + + const result = mergePanelsIntoPreset(currentState, targetPreset); + + expect(panelIdsIn(result, "center")).toEqual([SESSION_ABC, "files"]); + }); }); diff --git a/apps/web/lib/state/layout-manager/merger.ts b/apps/web/lib/state/layout-manager/merger.ts index ee44fe9a01..b29efe398a 100644 --- a/apps/web/lib/state/layout-manager/merger.ts +++ b/apps/web/lib/state/layout-manager/merger.ts @@ -5,6 +5,12 @@ import { fromDockviewApi } from "./serializer"; /** Panel IDs that always come from the preset and should never be merged. */ const PRESET_ONLY_PANELS = new Set(["sidebar"]); +/** Panel IDs that, when surviving as extras, belong next to the chat in the + * center column rather than in the side column. The plan panel is paired + * with the chat conceptually, so toggling off plan mode keeps it visible + * in the center group rather than dumping it into the right column. */ +const CENTER_EXTRA_PANELS = new Set(["plan"]); + /** Collect all panels from a LayoutState, flattened. */ function collectAllPanels(state: LayoutState): LayoutPanel[] { const panels: LayoutPanel[] = []; @@ -23,10 +29,36 @@ function collectPanelIds(state: LayoutState): Set { return new Set(collectAllPanels(state).map((p) => p.id)); } +/** Identify the column that should receive non-session extras. Prefer the last + * non-sidebar, non-center column (e.g. "right", "plan", "preview", "vscode"), + * falling back to "center" when the preset has no such side column. */ +function pickExtrasColumnId(targetPreset: LayoutState): string { + const sideCols = targetPreset.columns.filter((c) => c.id !== "sidebar" && c.id !== "center"); + return sideCols[sideCols.length - 1]?.id ?? "center"; +} + +/** Append `toAdd` panels to a group's first leaf, preserving identity when no-op. */ +function appendToFirstGroup( + groups: LayoutState["columns"][number]["groups"], + toAdd: LayoutPanel[], + filterExisting?: (p: LayoutPanel) => boolean, +): LayoutState["columns"][number]["groups"] { + return groups.map((group, idx) => { + if (idx !== 0) return group; + const basePanels = filterExisting ? group.panels.filter(filterExisting) : group.panels; + const existingIds = new Set(basePanels.map((p) => p.id)); + const additions = toAdd.filter((p) => !existingIds.has(p.id)); + if (additions.length === 0 && basePanels.length === group.panels.length) return group; + return { ...group, panels: [...basePanels, ...additions] }; + }); +} + /** * Pure merge logic: merge extra panels from the current state into a target - * preset layout. Session panels (`session:*`) replace the generic `chat` - * panel rather than coexisting alongside it. + * preset layout. Session panels (`session:*`) replace the generic `chat` + * panel in the center column. Other extras (files, changes, terminal, etc.) + * are appended to the side column when one exists (e.g. "right"/"plan"/ + * "preview"/"vscode"), otherwise they fall back to the center column. */ export function mergePanelsIntoPreset( currentState: LayoutState, @@ -43,36 +75,29 @@ export function mergePanelsIntoPreset( return targetPreset; } - const hasSessionPanels = extraPanels.some((p) => p.id.startsWith("session:")); - - console.debug( - "[layout-merger] merging extra panels into preset:", - extraPanels.map((p) => p.id), + const sessionExtras = extraPanels.filter((p) => p.id.startsWith("session:")); + const centerExtras = extraPanels.filter((p) => CENTER_EXTRA_PANELS.has(p.id)); + const sideExtras = extraPanels.filter( + (p) => !p.id.startsWith("session:") && !CENTER_EXTRA_PANELS.has(p.id), ); + const hasSessionPanels = sessionExtras.length > 0; + const extrasColumnId = pickExtrasColumnId(targetPreset); const mergedColumns = targetPreset.columns.map((col) => { - if (col.id !== "center") return col; - - const groups = col.groups.map((group, idx) => { - if (idx !== 0) return group; - - // If extra panels include session tabs, drop the generic "chat" - // placeholder — session panels replace it. - const basePanels = hasSessionPanels - ? group.panels.filter((p) => p.id !== "chat") - : group.panels; - - const existingIds = new Set(basePanels.map((p) => p.id)); - const toAdd = extraPanels.filter((p) => !existingIds.has(p.id)); - if (toAdd.length === 0 && basePanels.length === group.panels.length) return group; - - return { - ...group, - panels: [...basePanels, ...toAdd], - }; - }); - - return { ...col, groups }; + if (col.id === "center") { + // Sessions always replace the generic "chat" placeholder in center. + // Center-affinity extras (e.g. "plan") sit alongside the chat. When + // the side column doesn't exist, side extras land here too. + const fallbackSide = extrasColumnId === "center" ? sideExtras : []; + const filterChat = hasSessionPanels ? (p: LayoutPanel) => p.id !== "chat" : undefined; + const additions = [...sessionExtras, ...centerExtras, ...fallbackSide]; + if (additions.length === 0 && !filterChat) return col; + return { ...col, groups: appendToFirstGroup(col.groups, additions, filterChat) }; + } + if (col.id === extrasColumnId && sideExtras.length > 0) { + return { ...col, groups: appendToFirstGroup(col.groups, sideExtras) }; + } + return col; }); return { columns: mergedColumns };