fix(web): scope pending-permission scan to current turn (#899)

apps/web/e2e/tests/chat/permission-approval.spec.ts

@@ -4,18 +4,27 @@ import type { SeedData } from "../../fixtures/test-base";
 import type { ApiClient } from "../../helpers/api-client";
 import { SessionPage } from "../../pages/session-page";
 
+// Matches the number of permission prompts emitted by the mock-agent
+// `/e2e:multi-permission` scenario (apps/backend/cmd/mock-agent/scenarios.go).
+// Both approval loops below depend on this — bump together if the scenario
+// changes.
+const MULTI_PERMISSION_COUNT = 3;
+
 /**
  * Seed a task that runs the multi-permission scenario, then navigate to it.
  * The mock agent will request three permissions in sequence and block on each.
+ * The sidebar test passes a custom `title` so it can query the sidebar row by
+ * title text.
  */
 async function seedMultiPermissionTask(
   testPage: Page,
   apiClient: ApiClient,
   seedData: SeedData,
+  title = "Multi-permission approval",
 ): Promise<SessionPage> {
   const task = await apiClient.createTaskWithAgent(
     seedData.workspaceId,
-    "Multi-permission approval",
+    title,
     seedData.agentProfileId,
     {
       description: "/e2e:multi-permission",
@@ -56,11 +65,11 @@ test.describe("Permission approval persistence", () => {
   }) => {
     const session = await seedMultiPermissionTask(testPage, apiClient, seedData);
 
-    // Approve all three permission prompts as they appear. Each click unblocks
+    // Approve all permission prompts as they appear. Each click unblocks
     // the agent which then emits the next prompt; the previous one's button
     // detaches before the next one mounts, so wait for the count to be back
     // at 1 between clicks.
-    for (let i = 0; i < 3; i++) {
+    for (let i = 0; i < MULTI_PERMISSION_COUNT; i++) {
       await expect(session.permissionApproveButtons()).toHaveCount(1, { timeout: 30_000 });
       await session.permissionApproveButtons().first().click();
     }
@@ -78,4 +87,38 @@ test.describe("Permission approval persistence", () => {
     await expect(session.idleInput()).toBeVisible({ timeout: 30_000 });
     await expect(session.permissionActionRows()).toHaveCount(0);
   });
+
+  // While an agent is blocked on a permission_request, the sidebar entry for
+  // that task swaps the running spinner for the amber pending-permission icon
+  // (introduced in #882). The icon goes away once the prompt is resolved.
+  test("sidebar shows pending-permission icon while a permission prompt is open", async ({
+    testPage,
+    apiClient,
+    seedData,
+  }) => {
+    const taskTitle = "Sidebar pending permission";
+    const session = await seedMultiPermissionTask(testPage, apiClient, seedData, taskTitle);
+
+    // First permission prompt blocks the agent.
+    await expect(session.permissionApproveButtons()).toHaveCount(1, { timeout: 30_000 });
+
+    // Sidebar swaps the running spinner for the amber pending-permission icon.
+    // No `.first()` — `e2eReset` runs before every test (and every retry), so
+    // the worker workspace has exactly one task with this title; if a duplicate
+    // ever appears, strict locator resolution should fail the test rather than
+    // silently picking one row.
+    const sidebarItem = session.sidebarTaskItem(taskTitle);
+    await expect(sidebarItem.getByTestId("task-state-pending-permission")).toBeVisible({
+      timeout: 10_000,
+    });
+    await expect(sidebarItem.getByTestId("task-state-running")).toHaveCount(0);
+
+    // Approve all prompts; once the agent's turn ends, the icon is gone.
+    for (let i = 0; i < MULTI_PERMISSION_COUNT; i++) {
+      await expect(session.permissionApproveButtons()).toHaveCount(1, { timeout: 30_000 });
+      await session.permissionApproveButtons().first().click();
+    }
+    await expect(session.idleInput()).toBeVisible({ timeout: 30_000 });
+    await expect(sidebarItem.getByTestId("task-state-pending-permission")).toHaveCount(0);
+  });
 });

apps/web/lib/utils/pending-clarification.test.ts

@@ -183,4 +183,105 @@ describe("hasPendingPermissionRequest", () => {
     expect(hasPendingPermissionRequest(null)).toBe(false);
     expect(hasPendingPermissionRequest(undefined)).toBe(false);
   });
+
+  // Mixed-state: only the latest permission_request drives the UI. A stale
+  // pending row left behind by an earlier crash followed by a newer approved
+  // one must not light the amber icon — the agent is no longer blocked on
+  // the old row.
+  it("returns false when an older permission is still pending but a newer one is approved", () => {
+    expect(
+      hasPendingPermissionRequest([
+        message({ id: "old", type: "permission_request", metadata: { status: "pending" } }),
+        message({ id: "new", type: "permission_request", metadata: { status: "approved" } }),
+      ]),
+    ).toBe(false);
+  });
+
+  it("returns true when the latest permission is pending and earlier ones are resolved", () => {
+    expect(
+      hasPendingPermissionRequest([
+        message({ id: "a", type: "permission_request", metadata: { status: "approved" } }),
+        message({ id: "b", type: "permission_request", metadata: { status: "rejected" } }),
+        message({ id: "c", type: "permission_request", metadata: { status: "pending" } }),
+      ]),
+    ).toBe(true);
+  });
+
+  it("returns false when all permission requests are resolved regardless of order", () => {
+    expect(
+      hasPendingPermissionRequest([
+        message({ id: "a", type: "permission_request", metadata: { status: "approved" } }),
+        message({ id: "b", type: "permission_request", metadata: { status: "rejected" } }),
+        message({ id: "c", type: "permission_request", metadata: { status: "expired" } }),
+      ]),
+    ).toBe(false);
+  });
+});
+
+// Turn-scoped boundary: walking back across a different turn_id ends the
+// scan, so a pending row from a previous (crashed) turn must not leak into
+// the current turn's indicator.
+describe("hasPendingPermissionRequest — turn scoping", () => {
+  it("ignores a pending permission from a previous turn", () => {
+    expect(
+      hasPendingPermissionRequest([
+        message({
+          id: "old",
+          turn_id: "t1",
+          type: "permission_request",
+          metadata: { status: "pending" },
+        }),
+        message({ id: "current", turn_id: "t2", type: "message", content: "hi" }),
+      ]),
+    ).toBe(false);
+  });
+
+  it("detects a pending permission in the current turn", () => {
+    expect(
+      hasPendingPermissionRequest([
+        message({
+          id: "stale",
+          turn_id: "t1",
+          type: "permission_request",
+          metadata: { status: "pending" },
+        }),
+        message({
+          id: "active",
+          turn_id: "t2",
+          type: "permission_request",
+          metadata: { status: "pending" },
+        }),
+      ]),
+    ).toBe(true);
+  });
+
+  it("stops at the turn boundary even when an older turn has a pending permission", () => {
+    expect(
+      hasPendingPermissionRequest([
+        message({
+          id: "old-pending",
+          turn_id: "t1",
+          type: "permission_request",
+          metadata: { status: "pending" },
+        }),
+        message({
+          id: "new-approved",
+          turn_id: "t2",
+          type: "permission_request",
+          metadata: { status: "approved" },
+        }),
+      ]),
+    ).toBe(false);
+  });
+
+  // A legacy permission_request with no turn_id, sitting in a session whose
+  // latest message *does* have a turn_id, must not bypass the boundary.
+  it("ignores a legacy null-turn_id pending permission when a turn is active", () => {
+    expect(
+      hasPendingPermissionRequest([
+        message({ id: "legacy", type: "permission_request", metadata: { status: "pending" } }),
+        message({ id: "current", turn_id: "t2", type: "message", content: "hi" }),
+      ]),
+    ).toBe(false);
+  });
 });

apps/web/lib/utils/pending-clarification.ts

@@ -63,10 +63,28 @@ export function isPendingPermissionMessage(message: Message): boolean {
   return !metadata?.status || metadata.status === "pending";
 }
 
+// hasPendingPermissionRequest reports whether the *current turn* is blocked
+// on a permission_request. Scope:
+//   - Scans backwards from the end and stops at the first permission_request
+//     it sees — only the latest one drives the UI. A stale pending row left
+//     behind by an earlier crash followed by a newer approved one must not
+//     light up the amber icon (the agent is no longer blocked on the old row).
+//   - Honours turn boundaries: when the latest message has a turn_id, walking
+//     back to any row that doesn't share it ends the scan (including legacy
+//     rows with null/undefined turn_id). Old turns' permissions can never
+//     leak into the indicator and the scan stays bounded by the current turn
+//     size (typically 5–50 messages) regardless of total session length.
+//   - When the latest message itself has no turn_id (entirely pre-turn-scope
+//     data), boundary enforcement is disabled and we fall back to plain
+//     latest-only semantics across the whole array.
 export function hasPendingPermissionRequest(messages?: readonly Message[] | null): boolean {
-  if (!messages) return false;
+  if (!messages?.length) return false;
+  const latestTurnId = messages[messages.length - 1].turn_id;
   for (let i = messages.length - 1; i >= 0; i--) {
-    if (isPendingPermissionMessage(messages[i])) return true;
+    const m = messages[i];
+    if (latestTurnId && m.turn_id !== latestTurnId) break;
+    if (m.type !== "permission_request") continue;
+    return isPendingPermissionMessage(m);
   }
   return false;
 }