From 7011bbcbc4fe0d6442008148aba850bfee5e5822 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= <carlosmflorencio@gmail.com>
Date: Wed, 13 May 2026 18:39:01 +0100
Subject: [PATCH] fix: session tab leak, PR icon crash, and PR review prompt
 scope (#897)

---
 Makefile                                      |   8 +
 apps/backend/config/workflows/pr-review.yml   |  72 +++++----
 apps/web/components/github/pr-status-chip.tsx |   7 +-
 .../github/pr-task-icon.render.test.tsx       |  98 +++++++++++
 apps/web/components/github/pr-task-icon.tsx   |   5 +-
 .../task/dockview-session-tabs.test.ts        | 152 ++++++++++++++++++
 .../components/task/dockview-session-tabs.ts  |  43 +++--
 .../session/session-tab-management.spec.ts    |  62 +++++++
 .../state/slices/github/github-slice.test.ts  |  78 ++++++++-
 .../lib/state/slices/github/github-slice.ts   |   3 +-
 10 files changed, 482 insertions(+), 46 deletions(-)
 create mode 100644 apps/web/components/github/pr-task-icon.render.test.tsx
 create mode 100644 apps/web/components/task/dockview-session-tabs.test.ts

diff --git a/Makefile b/Makefile
index 3c97d013e1..e577b8669e 100644
--- a/Makefile
+++ b/Makefile
@@ -54,6 +54,7 @@ help:
 	@echo ""
 	@echo "Development Commands:"
 	@echo "  dev              Run backend + web via CLI (auto ports)"
+	@echo "  dev-prod-db      Run dev mode against the production db at ~/.kandev"
 	@echo "  dev-backend      Run backend in development mode (port 38429)"
 	@echo "  dev-web          Run web app in development mode (port 37429)"
 	@echo "  dev              Note: Uses apps/cli launcher (auto ports)"
@@ -112,6 +113,13 @@ endif
 	@echo "Launching via CLI (auto ports)..."
 	@cd $(APPS_DIR) && $(PNPM) -C cli dev -- dev
 
+.PHONY: dev-prod-db
+dev-prod-db: export KANDEV_DATABASE_PATH := $(HOME)/.kandev/data/kandev.db
+dev-prod-db:
+	@echo "⚠  dev mode against PRODUCTION db at $(KANDEV_DATABASE_PATH)"
+	@echo "   back it up first; dev binary may run unmigrated schema changes"
+	@$(MAKE) dev
+
 .PHONY: dev-backend
 dev-backend:
 	@echo "Starting backend on http://localhost:38429"
diff --git a/apps/backend/config/workflows/pr-review.yml b/apps/backend/config/workflows/pr-review.yml
index 7f43e95a67..bb3f51c63f 100644
--- a/apps/backend/config/workflows/pr-review.yml
+++ b/apps/backend/config/workflows/pr-review.yml
@@ -26,30 +26,44 @@ steps:
 
       {{task_prompt}}
 
-      HARD RULE — READ THIS FIRST
-      You are reviewing the PR's DIFF, not the repository. Findings are only valid on lines that were ADDED or MODIFIED in this PR. Unchanged lines in changed files, and any code in files the PR did not touch, are OFF-LIMITS even if they contain real bugs. If you cannot point to the exact added/modified line in the PR diff that triggers a finding, discard the finding.
+      SCOPE OVERRIDE (read first):
+      This task overrides any general "completeness" or "find all downstream changes"
+      instruction you may have. Scope is strictly the diff. Under-reporting is
+      acceptable; reporting on lines outside the diff is a failure of the task.
 
-      STEP 0: CREATE A TASK LIST
-      Before anything else, create a task list tracking the substeps below (use your todo/task tracking tool if available). Mark each task in_progress when you begin it and completed when you finish. Do not skip ahead — each step feeds the next.
+      STEP 1: DETERMINE WHAT TO REVIEW
 
-      STEP 1: GET THE PR DIFF AND ENUMERATE CHANGED LINES
+      The PR branch is checked out and the full codebase is available. You can and
+      should read surrounding code to understand changes in context.
+      - Extract the PR number from the task description (e.g. from the GitHub URL)
+      - Run: gh pr view <number> --json headRefName,baseRefName to confirm branch and base
+      - Run: git fetch origin <base_branch> to make sure your base ref is up to date
+      - Run: git diff origin/<base_branch>...HEAD --name-only to get the list of changed files
+      - Run: git diff origin/<base_branch>...HEAD to see the full diff
+      - Read each changed file in full — understand the surrounding code, not just the diff
+      - Navigate related files (callers, interfaces, tests) to understand the change end-to-end
+      - USE the full codebase for context; only REPORT issues on lines the PR added or modified
 
-      - Extract the PR number from the task description (e.g. from the GitHub URL).
-      - Run: gh pr view <number> --json headRefName,baseRefName,title,body to confirm branch, base, and intent.
-      - Run: gh pr diff <number> to fetch the canonical PR diff (this is what GitHub shows reviewers).
-      - For each file in the diff, list the changed hunks as (file, start_line-end_line) for the NEW side (added/modified lines only — ignore removed-only lines and unchanged context lines).
-      - Write this "changed-lines map" down at the top of your review notes. Every finding you later report must cite a (file, line) pair that falls inside this map.
+      STEP 1.5: BUILD AN ALLOWED-LINES SET (HARD GATE)
 
-      STEP 2: READ FOR CONTEXT, NOT FOR FINDINGS
+      - Run: git diff origin/<base_branch>...HEAD --unified=0
+      - Extract the (file, line-range) tuples of every "+" line. This is your allow-list.
+      - Findings on any line outside the allow-list are forbidden, no exceptions.
+      - A finding is forbidden even if:
+        - the PR description mentions or acknowledges the issue
+        - the line is in a file the PR modified, but on a line it didn't touch
+        - the line is in a file imported by, or imports, a changed file
+        - removing or fixing it would improve the codebase
+        - it is a "good follow-up"
 
-      Context reading is allowed only to understand whether a changed line is correct. It is never a source of findings.
-      - Open each changed file and read the surrounding function / block so you understand what the added lines do.
-      - Navigate to callers, interfaces, or tests only if needed to judge a changed line.
-      - Do NOT scan unchanged code looking for issues. Do NOT review files outside the diff.
+      The PR description is context, not scope. Acknowledged pre-existing issues,
+      planned follow-ups, and "known limitations" called out by the author are OUT
+      of scope. Do not comment on them even if they are real. The author already
+      knows.
 
-      STEP 3: REVIEW THE CHANGED LINES
+      STEP 2: REVIEW THE CHANGES
 
-      Walk through the changed-lines map hunk by hunk. For each hunk, consider these layers (skip layers that don't apply):
+      Review across these layers (skip layers that don't apply):
 
       Security (blockers if found):
       - No secrets, tokens, or credentials in code
@@ -82,30 +96,30 @@ steps:
       - as any / as unknown casts to dodge type errors instead of fixing types
       - Redundant validation where inputs are already parsed/typed
 
-      STEP 4: SELF-CHECK EVERY FINDING BEFORE REPORTING
+      OUTPUT FORMAT:
 
-      For each candidate finding, do this check. Drop the finding if any answer is "no":
-      1. Is the cited (file, line) inside the changed-lines map from STEP 1?
-      2. Can you quote the exact added/modified line from the PR diff that triggers it? (Include that quoted line in your notes.)
-      3. Is the issue caused by this PR, not pre-existing?
-      4. Are you >=80% confident it's a real issue?
+      For each finding, provide:
+      - file:line
+      - The exact "+" line from the diff the finding refers to (one-line quote).
+        If you cannot quote a "+" line from the diff, the finding is invalid — drop it.
+      - Description, why it matters, how to fix it.
 
-      STEP 5: OUTPUT
+      Only report findings you're >=80% confident about.
 
-      For each finding: file:line - description, why it matters, how to fix it, and the quoted diff line.
       Use these sections (omit empty ones):
 
       BLOCKER (must fix before merge — security, data loss, broken logic):
-      - file:line - description
+      - file:line — "+ <quoted diff line>" — description
 
       SUGGESTION (recommended, doesn't block — performance, architecture, missing tests):
-      - file:line - description
+      - file:line — "+ <quoted diff line>" — description
 
       End with a verdict: Ready to merge / Ready with suggestions / Blocked
 
-      NOT A FINDING (never report these):
-      - Any line not in the changed-lines map — even if it's a real bug
+      NOT A FINDING (skip these):
+      - Issues on lines or files the PR didn't modify — even if they are real bugs
       - Pre-existing code patterns that this PR didn't introduce or change
+      - Issues acknowledged in the PR description as known / follow-up
       - Things linters, typecheckers, or CI catch (imports, types, formatting)
       - Intentional functionality changes directly related to the PR's purpose
       - Issues explicitly suppressed in code (lint-ignore, nolint comments)
diff --git a/apps/web/components/github/pr-status-chip.tsx b/apps/web/components/github/pr-status-chip.tsx
index 153c212c41..f45298dce1 100644
--- a/apps/web/components/github/pr-status-chip.tsx
+++ b/apps/web/components/github/pr-status-chip.tsx
@@ -109,8 +109,13 @@ function ChipStatusGlyph({ status }: { status: ChipStatus }) {
     case "failed":
       return <IconCircleXFilled className="h-3.5 w-3.5 text-red-500" aria-hidden="true" />;
     case "in_progress":
+      // CI runs take minutes, so slow the spin to ~3s/rotation — the default
+      // animate-spin (1s) feels frantic for a long-running task.
       return (
-        <IconLoader2 className="h-3.5 w-3.5 text-yellow-500 animate-spin" aria-hidden="true" />
+        <IconLoader2
+          className="h-3.5 w-3.5 text-yellow-500 animate-spin [animation-duration:3s]"
+          aria-hidden="true"
+        />
       );
     case "merged":
       return <IconPointFilled className="h-3.5 w-3.5 text-purple-500" aria-hidden="true" />;
diff --git a/apps/web/components/github/pr-task-icon.render.test.tsx b/apps/web/components/github/pr-task-icon.render.test.tsx
new file mode 100644
index 0000000000..01a976a42d
--- /dev/null
+++ b/apps/web/components/github/pr-task-icon.render.test.tsx
@@ -0,0 +1,98 @@
+import { afterEach, describe, expect, it } from "vitest";
+import type { ReactNode } from "react";
+import { cleanup, render } from "@testing-library/react";
+import { TooltipProvider } from "@kandev/ui/tooltip";
+import { StateProvider } from "@/components/state-provider";
+import { PRTaskIcon } from "./pr-task-icon";
+import type { AppState } from "@/lib/state/store";
+import type { TaskPR } from "@/lib/types/github";
+
+function renderWithStore(initialState: Partial<AppState> | undefined, ui: ReactNode) {
+  return render(
+    <StateProvider initialState={initialState}>
+      <TooltipProvider>{ui}</TooltipProvider>
+    </StateProvider>,
+  );
+}
+
+function makePR(overrides: Partial<TaskPR> = {}): TaskPR {
+  return {
+    id: "id",
+    task_id: "task-1",
+    owner: "o",
+    repo: "r",
+    pr_number: 1,
+    pr_url: "",
+    pr_title: "Test PR",
+    head_branch: "feat",
+    base_branch: "main",
+    author_login: "alice",
+    state: "open",
+    review_state: "",
+    checks_state: "",
+    mergeable_state: "",
+    review_count: 0,
+    pending_review_count: 0,
+    comment_count: 0,
+    unresolved_review_threads: 0,
+    checks_total: 0,
+    checks_passing: 0,
+    additions: 0,
+    deletions: 0,
+    created_at: "",
+    merged_at: null,
+    closed_at: null,
+    last_synced_at: null,
+    updated_at: "",
+    ...overrides,
+  };
+}
+
+afterEach(() => cleanup());
+
+describe("PRTaskIcon corrupted store entry", () => {
+  // Regression: an upstream payload (partial hydration, WS reorder, etc.) once
+  // landed in taskPRs.byTaskId["task-1"] as a non-array truthy value. The
+  // length-based guards then fell through into MultiPRIcon, where for-of
+  // threw `prs is not iterable`. PRTaskIcon must bail rather than crash.
+  it("renders nothing when byTaskId[taskId] is a non-array object", () => {
+    const { container } = renderWithStore(
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      { taskPRs: { byTaskId: { "task-1": {} as any } } } as Partial<AppState>,
+      <PRTaskIcon taskId="task-1" />,
+    );
+    expect(container.firstChild).toBeNull();
+  });
+
+  it("renders nothing when byTaskId[taskId] is undefined", () => {
+    const { container } = renderWithStore(undefined, <PRTaskIcon taskId="missing" />);
+    expect(container.firstChild).toBeNull();
+  });
+
+  it("renders an icon when byTaskId[taskId] is a valid array of one PR", () => {
+    const { container } = renderWithStore(
+      { taskPRs: { byTaskId: { "task-1": [makePR()] } } },
+      <PRTaskIcon taskId="task-1" />,
+    );
+    expect(container.querySelector('[data-testid="pr-task-icon-task-1"]')).not.toBeNull();
+  });
+
+  it("renders the multi-PR icon when byTaskId[taskId] has multiple PRs", () => {
+    const { container } = renderWithStore(
+      {
+        taskPRs: {
+          byTaskId: {
+            "task-1": [
+              makePR({ id: "a", repository_id: "repo-a", pr_number: 1 }),
+              makePR({ id: "b", repository_id: "repo-b", pr_number: 2 }),
+            ],
+          },
+        },
+      },
+      <PRTaskIcon taskId="task-1" />,
+    );
+    const icon = container.querySelector('[data-testid="pr-task-icon-task-1"]');
+    expect(icon).not.toBeNull();
+    expect(icon?.getAttribute("data-pr-count")).toBe("2");
+  });
+});
diff --git a/apps/web/components/github/pr-task-icon.tsx b/apps/web/components/github/pr-task-icon.tsx
index d6147f8a16..c2585a98f6 100644
--- a/apps/web/components/github/pr-task-icon.tsx
+++ b/apps/web/components/github/pr-task-icon.tsx
@@ -101,7 +101,10 @@ export function aggregatePRStatusColor(prs: TaskPR[]): string {
 export function PRTaskIcon({ taskId }: { taskId: string }) {
   const prs = useAppStore((state) => state.taskPRs.byTaskId[taskId] ?? null);
 
-  if (!prs || prs.length === 0) return null;
+  // Defensive: an upstream payload may briefly seed byTaskId[taskId] with a
+  // non-array value (e.g. an empty object from a partial hydration). Bail
+  // instead of falling through into MultiPRIcon, where for-of throws.
+  if (!Array.isArray(prs) || prs.length === 0) return null;
   if (prs.length === 1) return <SinglePRIcon taskId={taskId} pr={prs[0]} />;
   return <MultiPRIcon taskId={taskId} prs={prs} />;
 }
diff --git a/apps/web/components/task/dockview-session-tabs.test.ts b/apps/web/components/task/dockview-session-tabs.test.ts
new file mode 100644
index 0000000000..7c72576e5b
--- /dev/null
+++ b/apps/web/components/task/dockview-session-tabs.test.ts
@@ -0,0 +1,152 @@
+import { describe, it, expect, vi } from "vitest";
+import type { DockviewApi } from "dockview-react";
+import { reconcileRemovedSessionPanels } from "./dockview-session-tabs";
+
+type FakePanel = {
+  id: string;
+  api: { close: ReturnType<typeof vi.fn<[], void>> };
+};
+
+const KEEP = "keep";
+const KEEP_PANEL = `session:${KEEP}`;
+const LEAKED_PANEL = "session:leaked";
+
+/**
+ * Builds a fake DockviewApi where `panel.api.close()` mutates the underlying
+ * `panels` array synchronously, mirroring how dockview removes a panel from
+ * its live `panels` getter the moment it's closed. This is what made the
+ * unsnapshotted `for (const panel of api.panels)` loop skip elements.
+ */
+function makeApi(panelIds: string[]): { api: DockviewApi; panels: FakePanel[] } {
+  const panels: FakePanel[] = [];
+  for (const id of panelIds) {
+    const panel: FakePanel = {
+      id,
+      api: {
+        close: vi.fn(() => {
+          const idx = panels.indexOf(panel);
+          if (idx !== -1) panels.splice(idx, 1);
+        }),
+      },
+    };
+    panels.push(panel);
+  }
+  const api = {
+    panels,
+    getPanel: (id: string) => panels.find((p) => p.id === id) ?? null,
+  } as unknown as DockviewApi;
+  return { api, panels };
+}
+
+function panelById(panels: FakePanel[], id: string): FakePanel | undefined {
+  return panels.find((p) => p.id === id);
+}
+
+describe("reconcileRemovedSessionPanels", () => {
+  it("closes a stale tracked panel that's still live in dockview", () => {
+    // createdSet has session-A; A's panel is live; A is no longer in the task's
+    // session list, so it must be closed.
+    const { api, panels } = makeApi(["session:A", KEEP_PANEL]);
+    const aPanel = panelById(panels, "session:A");
+    const createdSet = new Set(["A", KEEP]);
+
+    reconcileRemovedSessionPanels(api, createdSet, [KEEP], KEEP);
+
+    expect(aPanel?.api.close).toHaveBeenCalledTimes(1);
+    expect(createdSet.has("A")).toBe(false);
+  });
+
+  it("closes live session panels that were never tracked in createdSet (the leak)", () => {
+    // Reproduces the user-reported leak: dockview has session panels
+    // (e.g. restored from a persisted layout) for sessions that aren't in the
+    // current task's session list. createdSet is empty (or missing them)
+    // because the panels entered via `tryRestoreLayout` / `fromJSON`, not
+    // through ensureSessionPanel.
+    const { api, panels } = makeApi([LEAKED_PANEL, KEEP_PANEL]);
+    const leakedPanel = panelById(panels, LEAKED_PANEL);
+    const keepPanel = panelById(panels, KEEP_PANEL);
+    const createdSet = new Set<string>(["stale-deleted"]); // pollution from a prior right-click delete
+
+    reconcileRemovedSessionPanels(api, createdSet, [KEEP], KEEP);
+
+    expect(
+      leakedPanel?.api.close,
+      "leaked session panel must be closed even though it was never tracked",
+    ).toHaveBeenCalledTimes(1);
+    expect(keepPanel?.api.close, "keepSessionId panel must not be closed").not.toHaveBeenCalled();
+  });
+
+  it("closes every leaked panel even when close() mutates api.panels mid-iteration", () => {
+    // Regression for iterator-invalidation: with a live `panels` getter, a
+    // synchronous splice inside `close()` shifts subsequent elements left and
+    // a `for (const p of api.panels)` loop would skip the panel that moved
+    // into the just-vacated index. The implementation snapshots before
+    // iterating, so all leaked panels must still be closed in one pass.
+    const { api, panels } = makeApi([
+      "session:leak1",
+      "session:leak2",
+      "session:leak3",
+      KEEP_PANEL,
+    ]);
+    const leak1 = panelById(panels, "session:leak1");
+    const leak2 = panelById(panels, "session:leak2");
+    const leak3 = panelById(panels, "session:leak3");
+    const keepPanel = panelById(panels, KEEP_PANEL);
+
+    reconcileRemovedSessionPanels(api, new Set<string>(), [KEEP], KEEP);
+
+    expect(leak1?.api.close).toHaveBeenCalledTimes(1);
+    expect(leak2?.api.close).toHaveBeenCalledTimes(1);
+    expect(leak3?.api.close).toHaveBeenCalledTimes(1);
+    expect(keepPanel?.api.close).not.toHaveBeenCalled();
+  });
+
+  it("does not close the keepSessionId panel even if it is missing from createdSet", () => {
+    const { api, panels } = makeApi([KEEP_PANEL]);
+    const keepPanel = panelById(panels, KEEP_PANEL);
+    const createdSet = new Set<string>(); // keep was never tracked
+
+    reconcileRemovedSessionPanels(api, createdSet, [KEEP], KEEP);
+
+    expect(keepPanel?.api.close).not.toHaveBeenCalled();
+  });
+
+  it("does not close panels for sessions still present in the task's session list", () => {
+    const { api, panels } = makeApi(["session:a", "session:b"]);
+    const a = panelById(panels, "session:a");
+    const b = panelById(panels, "session:b");
+    const createdSet = new Set<string>();
+
+    reconcileRemovedSessionPanels(api, createdSet, ["a", "b"], "a");
+
+    expect(a?.api.close).not.toHaveBeenCalled();
+    expect(b?.api.close).not.toHaveBeenCalled();
+  });
+
+  it("ignores non-session panels", () => {
+    const { api, panels } = makeApi(["sidebar", "terminal:1", LEAKED_PANEL]);
+    const sidebar = panelById(panels, "sidebar");
+    const terminal = panelById(panels, "terminal:1");
+    const leaked = panelById(panels, LEAKED_PANEL);
+    const createdSet = new Set<string>();
+
+    reconcileRemovedSessionPanels(api, createdSet, [], "");
+
+    expect(sidebar?.api.close).not.toHaveBeenCalled();
+    expect(terminal?.api.close).not.toHaveBeenCalled();
+    expect(leaked?.api.close).toHaveBeenCalledTimes(1);
+  });
+
+  it("prunes stale entries from createdSet whose panels are already gone", () => {
+    // Right-click delete path: the panel was removed via
+    // containerApi.removePanel() in onDeleted, but createdSet still holds the
+    // session ID. Reconcile should drop the stale entry.
+    const { api } = makeApi([KEEP_PANEL]);
+    const createdSet = new Set<string>(["already-removed", KEEP]);
+
+    reconcileRemovedSessionPanels(api, createdSet, [KEEP], KEEP);
+
+    expect(createdSet.has("already-removed")).toBe(false);
+    expect(createdSet.has(KEEP)).toBe(true);
+  });
+});
diff --git a/apps/web/components/task/dockview-session-tabs.ts b/apps/web/components/task/dockview-session-tabs.ts
index f294ba874d..eaa40a72bd 100644
--- a/apps/web/components/task/dockview-session-tabs.ts
+++ b/apps/web/components/task/dockview-session-tabs.ts
@@ -217,26 +217,43 @@ function ensureSessionPanel(
   createdSet.add(sessionId);
 }
 
-/** Close panels we previously created for sessions no longer in the task's list. */
-function reconcileRemovedSessionPanels(
+/**
+ * Close session panels that no longer belong to the active task.
+ *
+ * Iterates `api.panels` (not `createdSet`) as the source of truth — `createdSet`
+ * is unreliable because session panels can enter dockview via `tryRestoreLayout`
+ * /`fromJSON` (never going through `ensureSessionPanel`) and can leave via
+ * external removals like the right-click delete handler. Trusting it caused
+ * tabs from a previous task to leak into the current task's view.
+ */
+export function reconcileRemovedSessionPanels(
   api: DockviewApi,
   createdSet: Set<string>,
   currentSessionIds: string[],
   keepSessionId: string,
 ): void {
   const currentIds = new Set(currentSessionIds);
-  for (const createdId of [...createdSet]) {
-    if (createdId === keepSessionId) continue;
-    if (currentIds.has(createdId)) continue;
-    const stalePanel = api.getPanel(`session:${createdId}`);
-    if (stalePanel) {
-      try {
-        stalePanel.api.close();
-      } catch {
-        /* already gone */
-      }
+  // Snapshot before iterating: closing a panel can mutate `api.panels`
+  // synchronously, which would skip elements in a `for...of` over the live
+  // array. Matches the pattern in `removeEphemeralPanels`.
+  for (const panel of [...api.panels]) {
+    if (!panel.id.startsWith("session:")) continue;
+    const sid = panel.id.slice("session:".length);
+    if (sid === keepSessionId) continue;
+    if (currentIds.has(sid)) continue;
+    try {
+      panel.api.close();
+    } catch {
+      /* already gone */
     }
-    createdSet.delete(createdId);
+    createdSet.delete(sid);
+  }
+  // Drop any remaining stale entries (panel already removed externally, e.g.
+  // by the right-click delete handler) so the ref stays in sync with reality.
+  for (const sid of [...createdSet]) {
+    if (sid === keepSessionId) continue;
+    if (currentIds.has(sid)) continue;
+    createdSet.delete(sid);
   }
 }
 
diff --git a/apps/web/e2e/tests/session/session-tab-management.spec.ts b/apps/web/e2e/tests/session/session-tab-management.spec.ts
index 10c90b69c3..b53e5d2a87 100644
--- a/apps/web/e2e/tests/session/session-tab-management.spec.ts
+++ b/apps/web/e2e/tests/session/session-tab-management.spec.ts
@@ -224,6 +224,68 @@ test.describe("Session tab management — close behavior", () => {
     await testPage.waitForTimeout(800);
     await expect(session.sessionTabBySessionId(session1Id)).not.toBeVisible();
   });
+
+  test("session tabs from one task do not leak into another task's view", async ({
+    testPage,
+    apiClient,
+    seedData,
+  }) => {
+    test.setTimeout(150_000);
+
+    const {
+      session,
+      session1Id: sessionA1Id,
+      session2Id: sessionA2Id,
+    } = await createTaskWithTwoSessions(testPage, apiClient, seedData, "Tab Leak Source A");
+
+    // Task B has only one session — never visited from the UI yet.
+    const taskB = await apiClient.createTaskWithAgent(
+      seedData.workspaceId,
+      "Tab Leak Target B",
+      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(taskB.id);
+          return DONE_STATES.includes(sessions[0]?.state ?? "");
+        },
+        { timeout: 30_000, message: "Waiting for taskB session" },
+      )
+      .toBe(true);
+    const { sessions: bSessions } = await apiClient.listTaskSessions(taskB.id);
+    const sessionB1Id = bSessions[0].id;
+
+    // Sanity: while on task A, both A sessions are visible.
+    await expect(session.sessionTabBySessionId(sessionA1Id)).toBeVisible({ timeout: 10_000 });
+    await expect(session.sessionTabBySessionId(sessionA2Id)).toBeVisible({ timeout: 5_000 });
+
+    // Switch from A → B via the sidebar.
+    await session.clickTaskInSidebar("Tab Leak Target B");
+    await expect(testPage).toHaveURL(new RegExp(`/t/${taskB.id}`), { timeout: 15_000 });
+    await session.waitForLoad();
+
+    // Task B's only session must be visible…
+    await expect(session.sessionTabBySessionId(sessionB1Id)).toBeVisible({ timeout: 10_000 });
+
+    // …and neither of task A's session tabs should have followed us in.
+    await testPage.waitForTimeout(800);
+    await expect(session.sessionTabBySessionId(sessionA1Id)).not.toBeVisible();
+    await expect(session.sessionTabBySessionId(sessionA2Id)).not.toBeVisible();
+
+    // Reload to make sure the persisted layout for task B doesn't bring them back either.
+    await testPage.reload();
+    await session.waitForLoad();
+    await expect(session.sessionTabBySessionId(sessionB1Id)).toBeVisible({ timeout: 10_000 });
+    await expect(session.sessionTabBySessionId(sessionA1Id)).not.toBeVisible();
+    await expect(session.sessionTabBySessionId(sessionA2Id)).not.toBeVisible();
+  });
 });
 
 test.describe("Session tab management — primary session persistence", () => {
diff --git a/apps/web/lib/state/slices/github/github-slice.test.ts b/apps/web/lib/state/slices/github/github-slice.test.ts
index 9cdcf5e6fb..23441a29ed 100644
--- a/apps/web/lib/state/slices/github/github-slice.test.ts
+++ b/apps/web/lib/state/slices/github/github-slice.test.ts
@@ -3,7 +3,40 @@ import { create } from "zustand";
 import { immer } from "zustand/middleware/immer";
 import { createGitHubSlice } from "./github-slice";
 import type { GitHubSlice } from "./types";
-import type { GitHubStatus } from "@/lib/types/github";
+import type { GitHubStatus, TaskPR } from "@/lib/types/github";
+
+function makePR(overrides: Partial<TaskPR> = {}): TaskPR {
+  return {
+    id: "id",
+    task_id: "task-1",
+    owner: "o",
+    repo: "r",
+    pr_number: 1,
+    pr_url: "",
+    pr_title: "Test PR",
+    head_branch: "feat",
+    base_branch: "main",
+    author_login: "alice",
+    state: "open",
+    review_state: "",
+    checks_state: "",
+    mergeable_state: "",
+    review_count: 0,
+    pending_review_count: 0,
+    comment_count: 0,
+    unresolved_review_threads: 0,
+    checks_total: 0,
+    checks_passing: 0,
+    additions: 0,
+    deletions: 0,
+    created_at: "",
+    merged_at: null,
+    closed_at: null,
+    last_synced_at: null,
+    updated_at: "",
+    ...overrides,
+  };
+}
 
 function makeStore() {
   return create<GitHubSlice>()(immer((...a) => createGitHubSlice(...a)));
@@ -104,3 +137,46 @@ describe("applyGitHubRateLimitUpdate", () => {
     expect(store.getState().githubStatus.status).toBeNull();
   });
 });
+
+describe("setTaskPR", () => {
+  it("appends a PR when the task has no rows yet", () => {
+    const store = makeStore();
+    const pr = makePR({ repository_id: "repo-a" });
+
+    store.getState().setTaskPR("task-1", pr);
+
+    expect(store.getState().taskPRs.byTaskId["task-1"]).toEqual([pr]);
+  });
+
+  it("upserts by repository_id so multi-repo PRs coexist", () => {
+    const store = makeStore();
+    const prA = makePR({ id: "a", repository_id: "repo-a", pr_number: 1 });
+    const prB = makePR({ id: "b", repository_id: "repo-b", pr_number: 2 });
+    const prAUpdated = makePR({ id: "a", repository_id: "repo-a", pr_number: 1, additions: 10 });
+
+    store.getState().setTaskPR("task-1", prA);
+    store.getState().setTaskPR("task-1", prB);
+    store.getState().setTaskPR("task-1", prAUpdated);
+
+    const list = store.getState().taskPRs.byTaskId["task-1"];
+    expect(list).toHaveLength(2);
+    expect(list.find((p) => p.repository_id === "repo-a")?.additions).toBe(10);
+    expect(list.find((p) => p.repository_id === "repo-b")?.id).toBe("b");
+  });
+
+  it("heals a corrupted non-array entry instead of throwing", () => {
+    // Simulates a stray payload landing in byTaskId[taskId] as something other
+    // than an array (e.g. a partial hydration). The next setTaskPR call must
+    // recover rather than propagate the bad shape, otherwise downstream
+    // renderers crash with `prs is not iterable`.
+    const store = makeStore();
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    store.getState().setTaskPRs({ "task-1": {} as any });
+
+    const pr = makePR({ repository_id: "repo-a" });
+    expect(() => store.getState().setTaskPR("task-1", pr)).not.toThrow();
+
+    expect(Array.isArray(store.getState().taskPRs.byTaskId["task-1"])).toBe(true);
+    expect(store.getState().taskPRs.byTaskId["task-1"]).toEqual([pr]);
+  });
+});
diff --git a/apps/web/lib/state/slices/github/github-slice.ts b/apps/web/lib/state/slices/github/github-slice.ts
index a3de113ad6..6063b6c7d5 100644
--- a/apps/web/lib/state/slices/github/github-slice.ts
+++ b/apps/web/lib/state/slices/github/github-slice.ts
@@ -44,7 +44,8 @@ function createTaskPRActions(set: ImmerSet): Pick<GitHubSlice, "setTaskPRs" | "s
         // Upsert by repository_id so multi-repo PRs coexist for the same task.
         // For legacy rows without a repository_id, match on the empty key (one
         // such row per task max), preserving prior single-PR semantics.
-        const existing = draft.taskPRs.byTaskId[taskId] ?? [];
+        const current = draft.taskPRs.byTaskId[taskId];
+        const existing = Array.isArray(current) ? current : [];
         const repoKey = pr.repository_id ?? "";
         const idx = existing.findIndex((p) => (p.repository_id ?? "") === repoKey);
         if (idx >= 0) existing[idx] = pr;