fix: preserve Changes data after session deletion (#3162)

apps/web/e2e/tests/session/session-tab-management.spec.ts

@@ -1,5 +1,8 @@
 import { expect } from "@playwright/test";
 import type { Page } from "@playwright/test";
+import { execFileSync } from "node:child_process";
+import fs from "node:fs";
+import path from "node:path";
 import { test } from "../../fixtures/test-base";
 import type { SeedData } from "../../fixtures/test-base";
 import type { ApiClient } from "../../helpers/api-client";
@@ -241,6 +244,102 @@ test.describe("Session tab management — close behavior", () => {
     expect(sessions.map((s) => s.id)).toEqual([session2Id]);
   });
 
+  test("deleting the active shared-environment session keeps Changes data visible", async ({
+    testPage,
+    apiClient,
+    seedData,
+  }) => {
+    test.setTimeout(150_000);
+
+    const { task, session, session1Id, session2Id } = await createTaskWithTwoSessions(
+      testPage,
+      apiClient,
+      seedData,
+      "Delete Active Session Keeps Changes",
+    );
+    const worktreePath = seedData.repositoryPath;
+    const worktreeBranch = execFileSync("git", ["branch", "--show-current"], {
+      cwd: worktreePath,
+      encoding: "utf8",
+    }).trim();
+    expect(worktreeBranch).toBeTruthy();
+
+    const localChange = "session-delete-change.txt";
+    const remoteChange = "remote-session-delete-change.ts";
+    const localChangePath = path.join(worktreePath, localChange);
+    fs.writeFileSync(localChangePath, "keep this change visible\n");
+
+    try {
+      await apiClient.mockGitHubReset();
+      await apiClient.mockGitHubSetUser("test-user");
+      await apiClient.mockGitHubAddPRs([
+        {
+          number: 3157,
+          title: "Keep Changes visible after session deletion",
+          state: "open",
+          head_branch: worktreeBranch,
+          base_branch: "main",
+          author_login: "test-user",
+          repo_owner: "testorg",
+          repo_name: "testrepo",
+          additions: 1,
+          deletions: 0,
+        },
+      ]);
+      await apiClient.mockGitHubAddPRFiles("testorg", "testrepo", 3157, [
+        {
+          filename: remoteChange,
+          status: "added",
+          additions: 1,
+          deletions: 0,
+          patch: "@@ -0,0 +1 @@\n+export const kept = true;",
+        },
+      ]);
+      await apiClient.mockGitHubAssociateTaskPR({
+        task_id: task.id,
+        workspace_id: seedData.workspaceId,
+        repository_id: seedData.repositoryId,
+        owner: "testorg",
+        repo: "testrepo",
+        pr_number: 3157,
+        pr_url: "https://github.com/testorg/testrepo/pull/3157",
+        pr_title: "Keep Changes visible after session deletion",
+        head_branch: worktreeBranch,
+        base_branch: "main",
+        author_login: "test-user",
+        additions: 1,
+        deletions: 0,
+      });
+
+      await testPage.reload();
+      await session.waitForLoad();
+      await session.sessionTabBySessionId(session1Id).click();
+      await session.clickTab("Changes");
+      await expect(session.changesFileRow(localChange)).toBeVisible({ timeout: 20_000 });
+      await session.expandPRChangesSection();
+      await expect(
+        session.prFilesSection().locator(`[data-changes-file="${remoteChange}"]`),
+      ).toBeVisible({ timeout: 20_000 });
+
+      await session.sessionTabBySessionId(session1Id).click({ button: "right" });
+      await session.contextMenuItem("Delete").click();
+      const confirmation = testPage.getByTestId("session-delete-confirm-popover");
+      await expect(confirmation).toBeVisible({ timeout: 5_000 });
+      await confirmation.getByTestId("session-delete-confirm").click();
+
+      await expect(session.sessionTabBySessionId(session1Id)).not.toBeVisible({ timeout: 15_000 });
+      await expect(session.sessionTabBySessionId(session2Id)).toBeVisible({ timeout: 5_000 });
+      await session.clickTab("Changes");
+      await expect(session.changesFileRow(localChange)).toBeVisible({ timeout: 20_000 });
+      await session.expandPRChangesSection();
+      await expect(
+        session.prFilesSection().locator(`[data-changes-file="${remoteChange}"]`),
+      ).toBeVisible({ timeout: 20_000 });
+    } finally {
+      fs.rmSync(localChangePath, { force: true });
+    }
+  });
+
   test("deleting then immediately switching tasks does not resurrect the tab", async ({
     testPage,
     apiClient,

apps/web/hooks/use-environment-session-id.test.ts

@@ -0,0 +1,82 @@
+import { renderHook } from "@testing-library/react";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+type MockStoreState = {
+  tasks: { activeSessionId: string | null };
+  environmentIdBySessionId: Record<string, string>;
+};
+
+let mockStoreState: MockStoreState;
+
+vi.mock("@/components/state-provider", () => ({
+  useAppStore: (selector: (state: MockStoreState) => unknown) => selector(mockStoreState),
+}));
+
+import { useEnvironmentSessionId } from "./use-environment-session-id";
+
+const ENVIRONMENT_ID = "environment-shared";
+const FIRST_SESSION_ID = "session-first";
+const SECOND_SESSION_ID = "session-second";
+
+function setActiveSession(activeSessionId: string, sessionIds: string[]) {
+  mockStoreState = {
+    tasks: { activeSessionId },
+    environmentIdBySessionId: Object.fromEntries(
+      sessionIds.map((sessionId) => [sessionId, ENVIRONMENT_ID]),
+    ),
+  };
+}
+
+beforeEach(() => {
+  setActiveSession(FIRST_SESSION_ID, [FIRST_SESSION_ID, SECOND_SESSION_ID]);
+});
+
+describe("useEnvironmentSessionId", () => {
+  // @covers AC-TASKS-SESSION-DELETE-RESOURCE-CLEANUP-001.9
+  it("promotes the active same-environment session after the cached session is purged", () => {
+    const { result, rerender } = renderHook(() => useEnvironmentSessionId());
+
+    expect(result.current).toBe(FIRST_SESSION_ID);
+
+    setActiveSession(SECOND_SESSION_ID, [FIRST_SESSION_ID, SECOND_SESSION_ID]);
+    rerender();
+    expect(result.current).toBe(FIRST_SESSION_ID);
+
+    setActiveSession(SECOND_SESSION_ID, [SECOND_SESSION_ID]);
+    rerender();
+
+    expect(result.current).toBe(SECOND_SESSION_ID);
+  });
+
+  it("keeps a live lookup handle stable across same-environment session switches", () => {
+    const { result, rerender } = renderHook(() => useEnvironmentSessionId());
+
+    setActiveSession(SECOND_SESSION_ID, [FIRST_SESSION_ID, SECOND_SESSION_ID]);
+    rerender();
+
+    expect(result.current).toBe(FIRST_SESSION_ID);
+  });
+
+  it("uses the active session when the environment changes", () => {
+    const { result, rerender } = renderHook(() => useEnvironmentSessionId());
+
+    mockStoreState = {
+      tasks: { activeSessionId: SECOND_SESSION_ID },
+      environmentIdBySessionId: {
+        [FIRST_SESSION_ID]: ENVIRONMENT_ID,
+        [SECOND_SESSION_ID]: "environment-second",
+      },
+    };
+    rerender();
+
+    expect(result.current).toBe(SECOND_SESSION_ID);
+  });
+
+  it("uses the active session as the pre-hydration lookup handle", () => {
+    setActiveSession(FIRST_SESSION_ID, []);
+
+    const { result } = renderHook(() => useEnvironmentSessionId());
+
+    expect(result.current).toBe(FIRST_SESSION_ID);
+  });
+});

apps/web/hooks/use-environment-session-id.ts

@@ -18,7 +18,11 @@ export function useEnvironmentSessionId(): string | null {
   const selector = useCallback((state: StoreSlice) => {
     const sid = state.tasks.activeSessionId;
     const envId = sid ? (state.environmentIdBySessionId[sid] ?? sid) : null;
-    if (envId === cacheRef.current.envId) return cacheRef.current.sessionId;
+    const cachedSessionId = cacheRef.current.sessionId;
+    const cachedSessionIsUsable =
+      cachedSessionId === sid ||
+      (cachedSessionId !== null && state.environmentIdBySessionId[cachedSessionId] === envId);
+    if (envId === cacheRef.current.envId && cachedSessionIsUsable) return cachedSessionId;
     cacheRef.current = { envId, sessionId: sid };
     return sid;
   }, []);

docs/plans/session-delete-changes-continuity/plan.md

@@ -0,0 +1,129 @@
+---
+created: 2026-08-30
+status: implemented
+requirements:
+  - REQ-TASKS-SESSION-DELETE-RESOURCE-CLEANUP-001
+system_design:
+  - ../../specs/tasks/system-design/session-delete-resource-cleanup.md
+legacy_specs: []
+---
+
+# Implementation Plan: Preserve Changes Data After Session Deletion
+
+## Overview
+
+Correct the environment-stable session selector so deleting its cached session
+promotes the surviving active session as the agentctl lookup handle. Add the
+selector regression first. Then prove that Changes keeps workspace and matching
+pull-request data after same-environment active-session deletion.
+
+## Scope
+
+### In scope
+
+- Preserve the existing no-refetch behavior for ordinary switches between live
+  sessions that share a task environment.
+- Replace a cached session handle after its environment mapping is purged and a
+  surviving active session maps to the same environment.
+- Keep environment-keyed Git, cumulative-diff, and pull-request data visible in
+  the desktop Changes panel after active-session deletion.
+- Cover the shared data path used by the existing mobile Changes surface.
+
+### Out of scope
+
+- Backend session deletion, task-environment ownership, or Git/PR APIs.
+- Changes panel layout, navigation, touch behavior, copy, or empty-state design.
+- Session-scoped file-review markers and their cache ownership.
+- Changing when ordinary same-environment tab switches refetch workspace data.
+- Recovering tasks that have no surviving session lookup handle.
+
+## Confirmed root cause
+
+`useEnvironmentSessionId` caches a session ID by environment identity. During
+active-session deletion, `useSessionActions.remove` selects the surviving
+session first. Then `removeTaskSession` purges the deleted session's environment
+mapping. Both sessions have the same environment ID. Thus, the selector keeps
+the deleted session after the mapping disappears. Git and commit hooks then use
+caches keyed by the deleted session ID. Cumulative-diff requests fail with
+`task session not found`. Branch-scoped PR selection also drops the fetched PR
+files.
+
+The smallest reliable reproduction uses two sessions that share one task
+environment. Load Changes through the first session. Delete that active session
+and reopen Changes through the surviving session. The current implementation
+shows the empty state. The environment cumulative diff and PR queries still
+contain files.
+
+## Technical approach
+
+### Environment-stable lookup handle
+
+Update `apps/web/hooks/use-environment-session-id.ts`. Reuse the cached session
+ID only when it is active or its current mapping equals the active environment.
+If the cached mapping is absent, use the current active session.
+Preserve the session-ID fallback while the active session's environment mapping
+loads.
+
+Add `apps/web/hooks/use-environment-session-id.test.ts`. The regression named
+`promotes the active same-environment session after the cached session is
+purged` must fail before the correction. It must pass after the correction.
+Adjacent cases retain the cached handle during a live same-environment switch.
+They replace it during an environment switch.
+
+### Visible session-deletion regression
+
+Extend `apps/web/e2e/tests/session/session-tab-management.spec.ts` with
+`deleting the active shared-environment session keeps Changes data visible`.
+Use the existing two-session setup. Make sure that both sessions reference the
+same environment. Seed a workspace change and a branch-matching mocked pull
+request. Open Changes through the deletion target. Then delete it through the
+visible session-tab UI. Reopen Changes. Make sure that the workspace file and
+PR file remain visible through the surviving session.
+
+The desktop test exercises the shared `useChangesPanelData` path. No mobile
+Playwright test is required because the correction changes only data-handle
+normalization. `MobileChangesPanel` already consumes the same hook. This fix
+does not change mobile composition, navigation, scrolling, touch targets, or
+viewport behavior.
+
+## Tests
+
+- `AC-TASKS-SESSION-DELETE-RESOURCE-CLEANUP-001.9` maps to
+  `apps/web/hooks/use-environment-session-id.test.ts`, including the failing
+  deleted-cache-handle regression and live same-environment stability case.
+- Existing session-runtime purge tests continue to prove that only the deleted
+  session mapping is removed while the shared environment cache survives.
+
+## E2E tests
+
+- `AC-TASKS-SESSION-DELETE-RESOURCE-CLEANUP-001.9` maps to
+  `apps/web/e2e/tests/session/session-tab-management.spec.ts` in the Chromium
+  project. The scenario deletes the active shared-environment session through
+  the UI and proves workspace and PR files remain visible in Changes.
+- Mobile parity is covered at the shared data-hook boundary because this fix
+  makes no rendered or responsive change. The shipped exemplar remains
+  `apps/web/components/task/mobile/mobile-changes-panel.tsx`, which reuses
+  `useChangesPanelData`, `ChangesPanelHeader`, and `ChangesPanelBody`.
+
+## Work orders
+
+- [x] [Task 01: Preserve the live Changes lookup handle](task-01-preserve-live-changes-handle.md)
+
+## Verification results
+
+- The selector regression failed before the production correction because it
+  retained `session-first` after that session's environment mapping was purged.
+- The focused Vitest suite passes with four cases.
+- The web TypeScript check passes.
+- The focused Chromium production-build E2E case passes with the workspace and
+  PR files visible after deletion.
+
+## Risks
+
+- Cache invalidation on every active-session switch causes needless
+  subscriptions and fetches. The correction must distinguish deleted handles
+  from live same-environment handles.
+- An active session must remain usable while its environment mapping loads.
+  Treating every missing mapping as deletion causes a startup regression.
+- The E2E pull request must match the live checkout branch and repository. This
+  prevents a fixture mismatch from changing the result for the wrong reason.

docs/plans/session-delete-changes-continuity/task-01-preserve-live-changes-handle.md

@@ -0,0 +1,95 @@
+---
+id: "01-preserve-live-changes-handle"
+title: "Preserve the live Changes lookup handle"
+status: done
+wave: 1
+depends_on: []
+plan: "plan.md"
+requirements:
+  - REQ-TASKS-SESSION-DELETE-RESOURCE-CLEANUP-001
+acceptance_criteria:
+  - AC-TASKS-SESSION-DELETE-RESOURCE-CLEANUP-001.9
+system_design:
+  - ../../specs/tasks/system-design/session-delete-resource-cleanup.md
+---
+
+# Task 01: Preserve the Live Changes Lookup Handle
+
+## Summary
+
+Make the environment-stable session selector retain only a live lookup handle.
+Use unit and browser regressions to prove the correction. Deleting an active
+session must not empty workspace or pull-request data from a shared task
+environment.
+
+## In scope
+
+- Add the environment-session selector unit regression before production code.
+- Correct cached-handle validation without changing ordinary live
+  same-environment switch behavior.
+- Add a desktop session-deletion E2E regression that verifies workspace and PR
+  files remain in Changes.
+
+## Out of scope
+
+- Backend lifecycle or API changes.
+- Changes panel presentation or mobile-specific composition changes.
+- Broad refactors of environment-keyed frontend state.
+
+## Acceptance
+
+- A cached session remains stable while it is active or remains mapped to the
+  active environment.
+- Removing the cached session's mapping promotes the current active session,
+  including when that session resolves to the same environment.
+- Deleting the active session through the UI leaves workspace and matching PR
+  files visible in Changes through the surviving session.
+
+## Verification
+
+```bash
+(cd apps && pnpm --filter @kandev/web test -- hooks/use-environment-session-id.test.ts)
+(cd apps/web && pnpm run typecheck)
+(cd apps/web && pnpm e2e:run tests/session/session-tab-management.spec.ts -- --grep "deleting the active shared-environment session keeps Changes data visible")
+```
+
+## Files likely touched
+
+- `apps/web/hooks/use-environment-session-id.ts`
+- `apps/web/hooks/use-environment-session-id.test.ts`
+- `apps/web/e2e/tests/session/session-tab-management.spec.ts`
+
+## Dependencies
+
+None.
+
+## Risks
+
+- The selector must not discard a valid cached handle during a normal
+  same-environment tab switch.
+- The pre-hydration active-session fallback must remain valid.
+- The PR fixture must be branch-scoped to the environment checkout.
+
+## Parallelism
+
+`sequential`
+
+## Inputs
+
+- `AC-TASKS-SESSION-DELETE-RESOURCE-CLEANUP-001.9`.
+- The frontend environment-handle continuity section in the paired system
+  design.
+- `useSessionActions.remove` ordering and session-runtime purge behavior.
+- Existing active-session deletion and PR Changes Playwright scenarios.
+
+## Results
+
+- Added a failing selector regression for a purged same-environment cache
+  handle, then corrected the selector to promote the surviving active session.
+- Preserved live same-environment handle stability, environment-switch
+  replacement, and the pre-hydration active-session fallback in unit coverage.
+- Added a Chromium regression that deletes the active session through the UI
+  and verifies that a workspace file and branch-matching PR file remain visible
+  in Changes.
+- Verification passed: 4 focused Vitest cases, web TypeScript, and 1 focused
+  production-build Playwright case.

docs/specs/tasks/requirements/session-delete-resource-cleanup.md

@@ -2,6 +2,7 @@
 status: draft
 system: tasks
 created: 2026-08-08
+updated: 2026-08-30
 owners:
   - cfl
 ---
@@ -27,6 +28,7 @@ A session is a conversation and execution reference inside a task; it is not the
 - **AC-TASKS-SESSION-DELETE-RESOURCE-CLEANUP-001.6:** Task archive/delete, cascade archive/delete, workspace delete, quick-chat task expiry, and explicit task-environment reset remain the owners of physical cleanup.
 - **AC-TASKS-SESSION-DELETE-RESOURCE-CLEANUP-001.7:** Task lifecycle cleanup discovers task-owned resources without joining through a session row and executes asynchronously through the durable cleanup worker.
 - **AC-TASKS-SESSION-DELETE-RESOURCE-CLEANUP-001.8:** Resources borrowed by another task or referenced through a shared environment are preserved or transferred according to existing ownership rules.
+- **AC-TASKS-SESSION-DELETE-RESOURCE-CLEANUP-001.9:** When an active session is deleted, another session can continue to use the same task environment. Desktop and mobile workspace-derived views shall continue to show current data. This data includes changed files, commits, and branch-matching remote contributions. The views shall not require a reload or replacement session.
 
 ## System design
 

docs/specs/tasks/system-design/session-delete-resource-cleanup.md

@@ -4,6 +4,7 @@ system: tasks
 requirements:
   - REQ-TASKS-SESSION-DELETE-RESOURCE-CLEANUP-001
 created: 2026-08-08
+updated: 2026-08-30
 owners:
   - cfl
 ---
@@ -17,7 +18,7 @@ This design preserves the technical source detail for `REQ-TASKS-SESSION-DELETE-
 
 | Requirement | Design section |
 | --- | --- |
-| `REQ-TASKS-SESSION-DELETE-RESOURCE-CLEANUP-001` | [Migrated source detail](#migrated-source-detail) |
+| `REQ-TASKS-SESSION-DELETE-RESOURCE-CLEANUP-001` | [Migrated source detail](#migrated-source-detail), [Frontend environment-handle continuity](#frontend-environment-handle-continuity) |
 
 ## Migrated source detail
 
@@ -66,6 +67,28 @@ task lifecycle operation takes responsibility for cleanup.
   say that the task workspace and files remain. They do not warn that
   uncommitted/unpushed workspace state will be removed.
 
+## Frontend environment-handle continuity
+
+Workspace-derived frontend state is keyed by `TaskEnvironment`, while several
+agentctl requests still require a live session ID as their lookup handle.
+Across ordinary tab switches, `useEnvironmentSessionId` can retain a session ID
+when both sessions resolve to the same environment. The retained handle is
+valid only while it is active or remains mapped to that environment.
+
+Session deletion first selects a surviving session. Then it purges the deleted
+session's runtime state. When the purge removes the retained mapping,
+`useEnvironmentSessionId` promotes the current active session as the lookup
+handle. The environment identity does not change. Environment-keyed caches for
+Git status, commits, cumulative diffs, and remote contributions remain intact.
+These caches continue to serve desktop and mobile Changes surfaces. File-review
+markers remain session-scoped and are intentionally outside this continuity
+guarantee.
+
+The selector does not treat a missing mapping as proof that an inactive cached
+session is usable. Thus, downstream hooks do not use a deleted session ID as a
+synthetic environment key. They also do not send agentctl requests for a
+session that no longer exists.
+
 ## Data Model
 
 ### `task_environments`
@@ -312,6 +335,10 @@ remain the authorization boundary for physical cleanup.
   retries after failure and backend restart using its persisted snapshot.
 - If another task/session still borrows the environment, cleanup preserves or
   transfers the resource rather than deleting it.
+- If the frontend removes the active session while another session shares its
+  environment, the environment cache remains authoritative and the surviving
+  session becomes the lookup handle. The Changes surface does not fall back to
+  an empty cache keyed by the deleted session ID.
 
 ## Persistence Guarantees
 
@@ -337,6 +364,10 @@ remain the authorization boundary for physical cleanup.
   **THEN** it reuses the task workspace and observes the marker.
 - **GIVEN** two sessions sharing a task workspace, **WHEN** one is deleted,
   **THEN** the other continues using the same directory without interruption.
+- **GIVEN** two sessions share a task environment and Changes shows workspace
+  commits or branch-matching remote contribution files, **WHEN** the active
+  session is deleted and the sibling becomes active, **THEN** desktop and mobile
+  Changes continue to show that environment's data without a reload.
 - **GIVEN** the final session was deleted and the backend restarted, **WHEN** the
   task is archived, **THEN** the task becomes archived and its canonical
   worktree is scheduled for asynchronous cleanup unless shared.