fix: harden last viewed target restore

Author: pallyoung

packages/server/src/__tests__/workspace-commands.test.ts

@@ -257,11 +257,67 @@ describe("Workspace Commands", () => {
       expect(readResult.ok).toBe(true);
       expect(readResult.data).toMatchObject({
         workspaceId,
-        sessionId: undefined,
       });
+      expect(readResult.data).not.toHaveProperty("sessionId");
       expect((readResult.data as { updatedAt: number }).updatedAt).toEqual(expect.any(Number));
     });
 
+    it("preserves a session id that belongs to the workspace", async () => {
+      const dir = join(tmpdir(), `workspace-target-session-test-${Date.now()}`);
+      await mkdir(dir);
+
+      const openResult = await dispatch(
+        {
+          kind: "command",
+          id: "open-workspace-target-session",
+          op: "workspace.open",
+          args: { path: dir },
+        },
+        ctx
+      );
+
+      expect(openResult.ok).toBe(true);
+      const workspaceId = (openResult.data as { id: string }).id;
+      ctx.sessionMgr.get = vi.fn((sessionId: string) =>
+        sessionId === "sess-123" ? { id: "sess-123", workspaceId } : undefined
+      ) as never;
+
+      const writeResult = await dispatch(
+        {
+          kind: "command",
+          id: "set-last-viewed-target-session",
+          op: "workspace.lastViewedTarget.set",
+          args: {
+            workspaceId,
+            sessionId: "sess-123",
+          },
+        },
+        ctx
+      );
+
+      expect(writeResult.ok).toBe(true);
+      expect(writeResult.data).toMatchObject({
+        workspaceId,
+        sessionId: "sess-123",
+      });
+
+      const readResult = await dispatch(
+        {
+          kind: "command",
+          id: "get-last-viewed-target-session",
+          op: "workspace.lastViewedTarget.get",
+          args: {},
+        },
+        ctx
+      );
+
+      expect(readResult.ok).toBe(true);
+      expect(readResult.data).toMatchObject({
+        workspaceId,
+        sessionId: "sess-123",
+      });
+    });
+
     it("drops an out-of-workspace session id while preserving the workspace target", async () => {
       const dir = join(tmpdir(), `workspace-target-mismatch-${Date.now()}`);
       await mkdir(dir);
@@ -315,5 +371,25 @@ describe("Workspace Commands", () => {
       expect(result.ok).toBe(false);
       expect(result.error?.code).toBe("workspace_not_found");
     });
+
+    it("returns null when the stored last-viewed target is malformed", async () => {
+      db.prepare("INSERT INTO user_settings (key, value) VALUES (?, ?)").run(
+        "workspace.lastViewedTarget",
+        "{not-json"
+      );
+
+      const result = await dispatch(
+        {
+          kind: "command",
+          id: "get-last-viewed-target-malformed",
+          op: "workspace.lastViewedTarget.get",
+          args: {},
+        },
+        ctx
+      );
+
+      expect(result.ok).toBe(true);
+      expect(result.data).toBeNull();
+    });
   });
 });

packages/server/src/commands/workspace-activity.ts

@@ -3,6 +3,21 @@ import { z } from "zod";
 import { registerCommand } from "../ws/dispatch.js";
 
 const WORKSPACE_LAST_VIEWED_TARGET_KEY = "workspace.lastViewedTarget";
+const workspaceLastViewedTargetSchema = z.object({
+  workspaceId: z.string(),
+  sessionId: z.string().optional(),
+  updatedAt: z.number(),
+});
+
+function parseWorkspaceLastViewedTarget(value: string): WorkspaceLastViewedTarget | null {
+  try {
+    const parsed = JSON.parse(value);
+    const result = workspaceLastViewedTargetSchema.safeParse(parsed);
+    return result.success ? result.data : null;
+  } catch {
+    return null;
+  }
+}
 
 registerCommand(
   "workspace.activate",
@@ -37,12 +52,7 @@ registerCommand("workspace.lastViewedTarget.get", z.object({}), async (_args, ct
     return null;
   }
 
-  const target = JSON.parse(row.value) as WorkspaceLastViewedTarget;
-  return {
-    workspaceId: target.workspaceId,
-    sessionId: target.sessionId,
-    updatedAt: target.updatedAt,
-  } satisfies WorkspaceLastViewedTarget;
+  return parseWorkspaceLastViewedTarget(row.value);
 });
 
 registerCommand(

packages/web/src/features/agent-panes/components/session-card.test.tsx

@@ -2,7 +2,7 @@ import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"
 import { createStore, Provider } from "jotai";
 import type { ReactNode } from "react";
 import { beforeEach, describe, expect, it, vi } from "vitest";
-import { pendingFocusSessionAtom } from "../../../atoms/app-ui";
+import { lastViewedTargetAtom, pendingFocusSessionAtom } from "../../../atoms/app-ui";
 import { wsClientAtom } from "../../../atoms/connection";
 import { sessionsAtom } from "../../../atoms/sessions";
 import {
@@ -137,6 +137,11 @@ describe("SessionCard", () => {
         },
       },
     });
+    store.set(lastViewedTargetAtom, {
+      workspaceId: "ws-123",
+      sessionId: "sess_123456",
+      updatedAt: 10,
+    });
 
     render(
       <Provider store={store}>
@@ -645,6 +650,48 @@ describe("SessionCard", () => {
     });
   });
 
+  it("does not persist the global last-viewed target again when the active session card is clicked", () => {
+    const sendCommand = vi.fn().mockResolvedValue(undefined);
+    const { store } = createSessionStore(
+      {
+        terminalId: "term-live",
+        state: "running",
+        endedAt: undefined,
+      },
+      sendCommand
+    );
+
+    store.set(workspacesAtom, {
+      "ws-123": {
+        id: "ws-123",
+        path: "/tmp/ws-123",
+        targetRuntime: "native",
+        openedAt: Date.now() - 10_000,
+        lastActiveAt: Date.now(),
+        uiState: {
+          leftPanelWidth: 320,
+          bottomPanelHeight: 240,
+          focusMode: false,
+          activeSessionId: "sess_123456",
+        },
+      },
+    });
+
+    render(
+      <Provider store={store}>
+        <SessionCard sessionId="sess_123456" />
+      </Provider>
+    );
+
+    fireEvent.click(document.querySelector('[data-session-id="sess_123456"]')!);
+
+    expect(sendCommand).not.toHaveBeenCalledWith(
+      "workspace.lastViewedTarget.set",
+      expect.anything(),
+      undefined
+    );
+  });
+
   it("does not persist activeSessionId when header action buttons are clicked", async () => {
     const sendCommand = vi.fn().mockResolvedValue({ ok: true });
     const onClose = vi.fn();

packages/web/src/features/agent-panes/views/shared/session-card.tsx

@@ -103,6 +103,10 @@ export const SessionCard: FC<SessionCardProps> = ({
       return;
     }
 
+    if (workspace?.uiState.activeSessionId === session.id) {
+      return;
+    }
+
     void persistLastViewedTarget({
       workspaceId: session.workspaceId,
       sessionId: session.id,

packages/web/src/features/topbar/components/tab.test.tsx

@@ -2,7 +2,7 @@ import type { Workspace } from "@coder-studio/core";
 import { fireEvent, render, screen, waitFor } from "@testing-library/react";
 import { createStore, Provider } from "jotai";
 import { beforeEach, describe, expect, it, vi } from "vitest";
-import { localeAtom } from "../../../atoms/app-ui";
+import { lastViewedTargetAtom, localeAtom } from "../../../atoms/app-ui";
 import { wsClientAtom } from "../../../atoms/connection";
 import {
   activeWorkspaceIdAtom,
@@ -130,6 +130,30 @@ describe("WorkspaceTab", () => {
     });
   });
 
+  it("does not persist again when the active workspace tab is clicked", () => {
+    const workspace = createWorkspace("ws-2", "/tmp/two");
+    const sendCommand = vi.fn();
+    const store = createStore();
+
+    store.set(localeAtom, "en");
+    store.set(wsClientAtom, { sendCommand } as never);
+    store.set(activeWorkspaceIdAtom, "ws-2");
+    store.set(lastViewedTargetAtom, {
+      workspaceId: "ws-2",
+      updatedAt: 10,
+    });
+
+    renderWorkspaceTab(store, workspace, { isActive: true, value: "ws-2" });
+
+    fireEvent.click(screen.getByRole("tab", { name: /two/i }));
+
+    expect(sendCommand).not.toHaveBeenCalledWith(
+      "workspace.lastViewedTarget.set",
+      expect.anything(),
+      undefined
+    );
+  });
+
   it("closes the active workspace without route navigation and falls back to the next ordered workspace", async () => {
     const firstWorkspace = createWorkspace("ws-1", "/tmp/one");
     const secondWorkspace = createWorkspace("ws-2", "/tmp/two");

packages/web/src/features/topbar/components/tab.tsx

@@ -38,6 +38,10 @@ export const WorkspaceTab: FC<WorkspaceTabProps> = ({ workspace, isActive }) =>
   const displayName = formatWorkspaceLabel(workspace) || workspace.id;
 
   const handleClick = () => {
+    if (isActive) {
+      return;
+    }
+
     setActiveWorkspace(workspace.id);
     void persistLastViewedTarget({ workspaceId: workspace.id });
   };

packages/web/src/features/workspace/actions/use-persist-workspace-last-viewed-target.ts

@@ -11,6 +11,7 @@ interface PersistWorkspaceLastViewedTargetInput {
 
 export function usePersistWorkspaceLastViewedTarget() {
   const dispatch = useAtomValue(dispatchCommandAtom);
+  const lastViewedTarget = useAtomValue(lastViewedTargetAtom);
   const setLastViewedTarget = useSetAtom(lastViewedTargetAtom);
 
   return useCallback(
@@ -19,6 +20,13 @@ export function usePersistWorkspaceLastViewedTarget() {
         return null;
       }
 
+      if (
+        lastViewedTarget?.workspaceId === workspaceId &&
+        (lastViewedTarget.sessionId ?? undefined) === sessionId
+      ) {
+        return lastViewedTarget;
+      }
+
       const optimisticTarget: WorkspaceLastViewedTarget = {
         workspaceId,
         sessionId,
@@ -38,6 +46,6 @@ export function usePersistWorkspaceLastViewedTarget() {
       setLastViewedTarget(result.data);
       return result.data;
     },
-    [dispatch, setLastViewedTarget]
+    [dispatch, lastViewedTarget, setLastViewedTarget]
   );
 }