From 7eec66836be106d484ee1df5303b3b92993b6fd4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= <carlosmflorencio@gmail.com>
Date: Fri, 12 Jun 2026 09:50:35 +0100
Subject: [PATCH] fix: hide sidebar footer urls on hover (#1348)

---
 .../app-sidebar/app-sidebar-footer.test.tsx   | 102 ++++++++++++++++++
 .../app-sidebar/app-sidebar-footer.tsx        |  16 +--
 .../components/task/passthrough-terminal.tsx  |   2 +-
 apps/web/e2e/helpers/api-client.ts            |  36 +++++++
 .../tests/layout/plan-panel-indicator.spec.ts |  51 +++++++--
 apps/web/e2e/tests/pr/pr-status-badge.spec.ts |  79 ++++++++++++--
 .../pr/pr-watcher-dockview-layout.spec.ts     |  76 ++++++++++---
 .../tests/terminal/mobile-terminal-helpers.ts |  11 +-
 .../terminal/mobile-terminal-keybar.spec.ts   |  28 +++--
 .../terminal/mobile-terminal-scroll.spec.ts   |  21 ++--
 10 files changed, 351 insertions(+), 71 deletions(-)
 create mode 100644 apps/web/components/app-sidebar/app-sidebar-footer.test.tsx

diff --git a/apps/web/components/app-sidebar/app-sidebar-footer.test.tsx b/apps/web/components/app-sidebar/app-sidebar-footer.test.tsx
new file mode 100644
index 0000000000..0c42338e63
--- /dev/null
+++ b/apps/web/components/app-sidebar/app-sidebar-footer.test.tsx
@@ -0,0 +1,102 @@
+import { cleanup, fireEvent, render, screen } from "@testing-library/react";
+import { TooltipProvider } from "@kandev/ui/tooltip";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+const mocks = vi.hoisted(() => ({
+  routerPush: vi.fn(),
+  toggleSettingsMode: vi.fn(),
+}));
+
+const state = {
+  workspaces: { activeId: "ws-1" as string | null },
+  appSidebar: { settingsMode: false },
+  toggleAppSidebarSettingsMode: mocks.toggleSettingsMode,
+};
+
+let officeEnabled = false;
+
+vi.mock("next/navigation", () => ({
+  useRouter: () => ({ push: mocks.routerPush }),
+}));
+
+vi.mock("@/components/state-provider", () => ({
+  useAppStore: (selector: (s: typeof state) => unknown) => selector(state),
+}));
+
+vi.mock("@/hooks/domains/features/use-feature", () => ({
+  useFeature: () => officeEnabled,
+}));
+
+vi.mock("@/hooks/use-release-notes", () => ({
+  useReleaseNotes: () => ({
+    unseenEntries: [],
+    latestVersion: "0.0.0",
+    hasUnseen: false,
+    dialogOpen: false,
+    openDialog: vi.fn(),
+    closeDialog: vi.fn(),
+    hasNotes: false,
+    showTopbarButton: false,
+  }),
+}));
+
+vi.mock("@/components/improve-kandev-dialog", () => ({
+  ImproveKandevDialog: () => null,
+}));
+
+vi.mock("@/components/release-notes/release-notes-dialog", () => ({
+  ReleaseNotesDialog: () => null,
+}));
+
+vi.mock("@/components/theme-toggle", () => ({
+  ThemeToggle: () => <button type="button">Theme</button>,
+}));
+
+import { AppSidebarFooter } from "./app-sidebar-footer";
+
+function renderFooter() {
+  return render(
+    <TooltipProvider>
+      <AppSidebarFooter collapsed={false} />
+    </TooltipProvider>,
+  );
+}
+
+describe("AppSidebarFooter", () => {
+  beforeEach(() => {
+    officeEnabled = false;
+    state.appSidebar.settingsMode = false;
+    mocks.routerPush.mockClear();
+    mocks.toggleSettingsMode.mockClear();
+  });
+
+  afterEach(() => cleanup());
+
+  it("renders navigation icons as buttons so hover does not expose link URLs", () => {
+    officeEnabled = true;
+
+    renderFooter();
+
+    const statsButton = screen.getByRole("button", { name: "Stats" });
+    const officeButton = screen.getByRole("button", { name: "Office" });
+
+    expect(statsButton).toBeTruthy();
+    expect(officeButton).toBeTruthy();
+    expect(statsButton.getAttribute("href")).toBeNull();
+    expect(officeButton.getAttribute("href")).toBeNull();
+    expect(screen.queryByRole("link", { name: "Stats" })).toBeNull();
+    expect(screen.queryByRole("link", { name: "Office" })).toBeNull();
+  });
+
+  it("navigates from the Stats and Office footer buttons", () => {
+    officeEnabled = true;
+
+    renderFooter();
+
+    fireEvent.click(screen.getByRole("button", { name: "Stats" }));
+    fireEvent.click(screen.getByRole("button", { name: "Office" }));
+
+    expect(mocks.routerPush).toHaveBeenNthCalledWith(1, "/stats");
+    expect(mocks.routerPush).toHaveBeenNthCalledWith(2, "/office");
+  });
+});
diff --git a/apps/web/components/app-sidebar/app-sidebar-footer.tsx b/apps/web/components/app-sidebar/app-sidebar-footer.tsx
index 84870e4532..873daf7678 100644
--- a/apps/web/components/app-sidebar/app-sidebar-footer.tsx
+++ b/apps/web/components/app-sidebar/app-sidebar-footer.tsx
@@ -1,7 +1,6 @@
 "use client";
 
 import { useState } from "react";
-import Link from "next/link";
 import { useRouter } from "next/navigation";
 import {
   IconBuildings,
@@ -31,7 +30,6 @@ type FooterIconButtonProps = {
   label: string;
   collapsed: boolean;
   onClick?: () => void;
-  href?: string;
   badge?: boolean;
   testId?: string;
   /** Toggle state: rotates the icon a half-turn (spins back out when cleared). */
@@ -43,7 +41,6 @@ function FooterIconButton({
   label,
   collapsed,
   onClick,
-  href,
   badge,
   testId,
   active,
@@ -68,14 +65,9 @@ function FooterIconButton({
     </>
   );
 
-  const trigger = href ? (
-    <Button asChild {...buttonProps}>
-      <Link href={href} aria-label={label} data-testid={testId}>
-        {content}
-      </Link>
-    </Button>
-  ) : (
+  const trigger = (
     <Button
+      type="button"
       onClick={onClick}
       {...buttonProps}
       aria-label={label}
@@ -122,7 +114,7 @@ export function AppSidebarFooter({ collapsed }: AppSidebarFooterProps) {
         icon={IconChartBar}
         label="Stats"
         collapsed={collapsed}
-        href="/stats"
+        onClick={() => router.push("/stats")}
         testId="sidebar-stats-button"
       />
       <FooterIconButton
@@ -147,7 +139,7 @@ export function AppSidebarFooter({ collapsed }: AppSidebarFooterProps) {
           icon={IconBuildings}
           label="Office"
           collapsed={collapsed}
-          href="/office"
+          onClick={() => router.push("/office")}
           testId="sidebar-office-button"
         />
       )}
diff --git a/apps/web/components/task/passthrough-terminal.tsx b/apps/web/components/task/passthrough-terminal.tsx
index 5b813f3275..66c98c5bba 100644
--- a/apps/web/components/task/passthrough-terminal.tsx
+++ b/apps/web/components/task/passthrough-terminal.tsx
@@ -275,7 +275,7 @@ export function PassthroughTerminal(props: PassthroughTerminalProps) {
       style={{ minWidth: MIN_WIDTH, minHeight: MIN_HEIGHT }}
     >
       <div className="h-full w-full p-2 pb-3">
-        <div ref={terminalRef} className="h-full w-full" />
+        <div ref={terminalRef} data-testid="terminal-xterm-host" className="h-full w-full" />
       </div>
       <TerminalSearchBar search={search} />
       {!isConnected && (
diff --git a/apps/web/e2e/helpers/api-client.ts b/apps/web/e2e/helpers/api-client.ts
index 776a9d8e29..f1482a7c22 100644
--- a/apps/web/e2e/helpers/api-client.ts
+++ b/apps/web/e2e/helpers/api-client.ts
@@ -995,6 +995,25 @@ export class ApiClient {
     await this.request("POST", "/api/v1/github/mock/task-prs", data);
   }
 
+  async getTaskPR(taskId: string): Promise<{
+    task_id: string;
+    pr_number: number;
+    state: string;
+    review_state: string;
+    checks_state: string;
+    mergeable_state: string;
+    review_count: number;
+    pending_review_count: number;
+    required_reviews?: number | null;
+  } | null> {
+    const res = await this.rawRequest("GET", `/api/v1/github/task-prs/${taskId}`);
+    if (res.status === 404) return null;
+    if (!res.ok) {
+      throw new Error(`getTaskPR failed (${res.status}): ${await res.text()}`);
+    }
+    return res.json();
+  }
+
   async mockGitHubSeedPRFeedback(data: {
     owner: string;
     repo: string;
@@ -1118,6 +1137,23 @@ export class ApiClient {
     return this.request("GET", `/api/v1/tasks/${taskId}`);
   }
 
+  async getTaskPlan(taskId: string): Promise<{
+    task_id: string;
+    content: string;
+    title?: string;
+    created_by: string;
+    updated_at: string;
+  } | null> {
+    try {
+      return await this.wsRequest("task.plan.get", { task_id: taskId });
+    } catch (error) {
+      const message = error instanceof Error ? error.message : String(error);
+      // Backend returns "plan not found" when no plan exists for the task.
+      if (message.includes("plan not found")) return null;
+      throw error;
+    }
+  }
+
   /**
    * Send a one-shot WebSocket request and await the matching response.
    * Used for actions exposed only over WS (e.g. session.launch). Opens a
diff --git a/apps/web/e2e/tests/layout/plan-panel-indicator.spec.ts b/apps/web/e2e/tests/layout/plan-panel-indicator.spec.ts
index bceaf15deb..0409482bd3 100644
--- a/apps/web/e2e/tests/layout/plan-panel-indicator.spec.ts
+++ b/apps/web/e2e/tests/layout/plan-panel-indicator.spec.ts
@@ -30,6 +30,36 @@ function planTabIndicator(page: Page) {
   return page.getByTestId("plan-tab-indicator");
 }
 
+async function waitForAgentPlan(apiClient: ApiClient, taskId: string, contentText: string) {
+  await expect
+    .poll(
+      async () => {
+        const plan = await apiClient.getTaskPlan(taskId);
+        return plan?.created_by === "agent" && plan.content.includes(contentText);
+      },
+      {
+        timeout: 15_000,
+        message: `Expected agent-authored plan containing "${contentText}"`,
+      },
+    )
+    .toBe(true);
+}
+
+async function expectPlanIndicatorVisible(page: Page, session: SessionPage) {
+  await planTabIndicator(page)
+    .waitFor({ state: "visible", timeout: 5_000 })
+    .catch(async () => {
+      // The plan is durable backend state, but the tab indicator is armed from
+      // the task.plan.* WS push. Under shard load that push can beat the page's
+      // subscription; a reload exercises the same self-heal users get on refresh.
+      await page.reload();
+      await session.waitForLoad();
+    });
+
+  await expect(planTabLocator(page)).toBeVisible({ timeout: 15_000 });
+  await expect(planTabIndicator(page)).toBeVisible({ timeout: 15_000 });
+}
+
 async function seedTaskAndWaitForIdle(
   testPage: Page,
   apiClient: ApiClient,
@@ -67,13 +97,14 @@ test.describe("Plan panel auto-open + indicator", () => {
   }) => {
     test.setTimeout(90_000);
 
-    const { session } = await seedTaskAndWaitForIdle(
+    const { session, taskId } = await seedTaskAndWaitForIdle(
       testPage,
       apiClient,
       seedData,
       "plan indicator create",
       CREATE_PLAN_SCRIPT,
     );
+    await waitForAgentPlan(apiClient, taskId, "Step one");
 
     // Plan tab is rendered (panel mounted as a sibling of chat in the center group)
     await expect(planTabLocator(testPage)).toBeVisible({ timeout: 15_000 });
@@ -87,7 +118,7 @@ test.describe("Plan panel auto-open + indicator", () => {
     // `task.plan.created` WS push, or the eager getTaskPlan self-heal if the
     // push was missed. Both can land after the default 5s under shard load, so
     // match the 15s tab budget.
-    await expect(planTabIndicator(testPage)).toBeVisible({ timeout: 15_000 });
+    await expectPlanIndicatorVisible(testPage, session);
   });
 
   test("clicking the Plan tab clears the indicator and reveals plan content", async ({
@@ -97,15 +128,16 @@ test.describe("Plan panel auto-open + indicator", () => {
   }) => {
     test.setTimeout(90_000);
 
-    const { session } = await seedTaskAndWaitForIdle(
+    const { session, taskId } = await seedTaskAndWaitForIdle(
       testPage,
       apiClient,
       seedData,
       "plan indicator acknowledge",
       CREATE_PLAN_SCRIPT,
     );
+    await waitForAgentPlan(apiClient, taskId, "Step one");
     await expect(planTabLocator(testPage)).toBeVisible({ timeout: 15_000 });
-    await expect(planTabIndicator(testPage)).toBeVisible({ timeout: 15_000 });
+    await expectPlanIndicatorVisible(testPage, session);
 
     await session.clickTab("Plan");
 
@@ -122,15 +154,16 @@ test.describe("Plan panel auto-open + indicator", () => {
   }) => {
     test.setTimeout(120_000);
 
-    const { session } = await seedTaskAndWaitForIdle(
+    const { session, taskId } = await seedTaskAndWaitForIdle(
       testPage,
       apiClient,
       seedData,
       "plan indicator update",
       CREATE_PLAN_SCRIPT,
     );
+    await waitForAgentPlan(apiClient, taskId, "Step one");
     await expect(planTabLocator(testPage)).toBeVisible({ timeout: 15_000 });
-    await expect(planTabIndicator(testPage)).toBeVisible({ timeout: 15_000 });
+    await expectPlanIndicatorVisible(testPage, session);
 
     // Acknowledge then leave back to chat
     await session.clickTab("Plan");
@@ -141,10 +174,11 @@ test.describe("Plan panel auto-open + indicator", () => {
     // Trigger an agent update via a follow-up message
     await session.sendMessage(UPDATE_PLAN_SCRIPT);
     await expect(session.idleInput()).toBeVisible({ timeout: 45_000 });
+    await waitForAgentPlan(apiClient, taskId, "Step two");
 
     // Chat still focused, indicator re-armed
     await expect(planTabLocator(testPage)).not.toHaveClass(/dv-active-tab/);
-    await expect(planTabIndicator(testPage)).toBeVisible({ timeout: 15_000 });
+    await expectPlanIndicatorVisible(testPage, session);
 
     // Clicking the Plan tab shows the updated content
     await session.clickTab("Plan");
@@ -165,13 +199,14 @@ test.describe("Plan panel auto-open + indicator", () => {
       "plan indicator refresh",
       CREATE_PLAN_SCRIPT,
     );
+    await waitForAgentPlan(apiClient, taskId, "Step one");
     await expect(planTabLocator(testPage)).toBeVisible({ timeout: 15_000 });
     // The plan-update WS event arrives separately from the agent's idle
     // signal — the tab mounts as soon as the panel registers, but the
     // indicator only flips on once `plan.created_by === "agent"` is in the
     // store. Match the tab's 15s budget instead of using the default 5s,
     // which raced the WS push under shard load.
-    await expect(planTabIndicator(testPage)).toBeVisible({ timeout: 15_000 });
+    await expectPlanIndicatorVisible(testPage, session);
 
     // Acknowledge
     await session.clickTab("Plan");
diff --git a/apps/web/e2e/tests/pr/pr-status-badge.spec.ts b/apps/web/e2e/tests/pr/pr-status-badge.spec.ts
index b6e1583b42..a865c508d2 100644
--- a/apps/web/e2e/tests/pr/pr-status-badge.spec.ts
+++ b/apps/web/e2e/tests/pr/pr-status-badge.spec.ts
@@ -2,6 +2,7 @@ import { test, expect } from "../../fixtures/test-base";
 import { KanbanPage } from "../../pages/kanban-page";
 import { SessionPage } from "../../pages/session-page";
 import type { ApiClient } from "../../helpers/api-client";
+import type { Page } from "@playwright/test";
 
 async function seedBadgeTest(
   apiClient: ApiClient,
@@ -42,6 +43,56 @@ async function seedBadgeTest(
   return { workflow, inboxStep, workingStep, doneStep, task };
 }
 
+type TaskPR = NonNullable<Awaited<ReturnType<ApiClient["getTaskPR"]>>>;
+
+async function waitForTaskPRFields(
+  apiClient: ApiClient,
+  taskId: string,
+  expected: Partial<Pick<TaskPR, "state" | "review_state" | "checks_state" | "mergeable_state">> & {
+    pending_review_count?: number;
+  },
+) {
+  await expect
+    .poll(
+      async () => {
+        const pr = await apiClient.getTaskPR(taskId);
+        if (!pr) return false;
+        return Object.entries(expected).every(([key, value]) => pr[key as keyof TaskPR] === value);
+      },
+      {
+        timeout: 15_000,
+        message: "Expected backend TaskPR fields to match seeded mock state",
+      },
+    )
+    .toBe(true);
+}
+
+async function expectTopbarReadyState(
+  page: Page,
+  session: SessionPage,
+  expected: "true" | "false",
+) {
+  const button = session.prTopbarButton();
+  await button.waitFor({ state: "visible", timeout: 15_000 });
+
+  await button
+    .waitFor({ state: "attached", timeout: 1_000 })
+    .then(() =>
+      expect(button).toHaveAttribute("data-pr-ready-to-merge", expected, { timeout: 5_000 }),
+    )
+    .catch(async () => {
+      // The topbar PR button hydrates from task-pr state that can arrive via a
+      // github.task_pr.updated WS event. If the event was missed during task
+      // navigation, a reload rehydrates from the backend state asserted above.
+      await page.reload();
+      await session.waitForLoad();
+    });
+
+  await expect(session.prTopbarButton()).toHaveAttribute("data-pr-ready-to-merge", expected, {
+    timeout: 15_000,
+  });
+}
+
 test.describe("PR status badge", () => {
   /**
    * Regression for the "CI pending" bug: GitHub reports all checks passed
@@ -87,6 +138,7 @@ test.describe("PR status badge", () => {
       state: "open",
       checks_state: "success",
     });
+    await waitForTaskPRFields(apiClient, task.id, { state: "open", checks_state: "success" });
 
     await expect(kanban.taskCardInColumn("CI Skipped Task", doneStep.id)).toBeVisible({
       timeout: 45_000,
@@ -106,8 +158,7 @@ test.describe("PR status badge", () => {
     await expect(testPage).toHaveURL(/\/[st]\//, { timeout: 15_000 });
     const session = new SessionPage(testPage);
     await session.waitForLoad();
-    await expect(session.prTopbarButton()).toBeVisible({ timeout: 15_000 });
-    await expect(session.prTopbarButton()).toHaveAttribute("data-pr-ready-to-merge", "false");
+    await expectTopbarReadyState(testPage, session, "false");
   });
 
   /**
@@ -149,6 +200,12 @@ test.describe("PR status badge", () => {
       checks_state: "success",
       mergeable_state: "clean",
     });
+    await waitForTaskPRFields(apiClient, task.id, {
+      state: "open",
+      review_state: "approved",
+      checks_state: "success",
+      mergeable_state: "clean",
+    });
 
     await expect(kanban.taskCardInColumn("Ready To Merge Task", doneStep.id)).toBeVisible({
       timeout: 45_000,
@@ -164,10 +221,7 @@ test.describe("PR status badge", () => {
     await expect(testPage).toHaveURL(/\/[st]\//, { timeout: 15_000 });
     const session = new SessionPage(testPage);
     await session.waitForLoad();
-    await expect(session.prTopbarButton()).toBeVisible({ timeout: 15_000 });
-    await expect(session.prTopbarButton()).toHaveAttribute("data-pr-ready-to-merge", "true", {
-      timeout: 15_000,
-    });
+    await expectTopbarReadyState(testPage, session, "true");
   });
 
   /**
@@ -209,6 +263,12 @@ test.describe("PR status badge", () => {
       checks_state: "success",
       mergeable_state: "blocked",
     });
+    await waitForTaskPRFields(apiClient, task.id, {
+      state: "open",
+      review_state: "approved",
+      checks_state: "success",
+      mergeable_state: "blocked",
+    });
 
     await expect(kanban.taskCardInColumn("Blocked Task", doneStep.id)).toBeVisible({
       timeout: 45_000,
@@ -265,6 +325,13 @@ test.describe("PR status badge", () => {
       mergeable_state: "blocked",
       pending_review_count: 1,
     });
+    await waitForTaskPRFields(apiClient, task.id, {
+      state: "open",
+      review_state: "approved",
+      checks_state: "success",
+      mergeable_state: "blocked",
+      pending_review_count: 1,
+    });
 
     await expect(kanban.taskCardInColumn("Awaiting Review Task", doneStep.id)).toBeVisible({
       timeout: 45_000,
diff --git a/apps/web/e2e/tests/pr/pr-watcher-dockview-layout.spec.ts b/apps/web/e2e/tests/pr/pr-watcher-dockview-layout.spec.ts
index 62b36f9f59..058d55c286 100644
--- a/apps/web/e2e/tests/pr/pr-watcher-dockview-layout.spec.ts
+++ b/apps/web/e2e/tests/pr/pr-watcher-dockview-layout.spec.ts
@@ -1,10 +1,69 @@
 import { execSync } from "node:child_process";
 import path from "node:path";
+import type { Page } from "@playwright/test";
 import { test, expect } from "../../fixtures/test-base";
 import { makeGitEnv } from "../../helpers/git-helper";
+import type { ApiClient } from "../../helpers/api-client";
 import { KanbanPage } from "../../pages/kanban-page";
 import { SessionPage } from "../../pages/session-page";
 
+async function waitForReviewTasksInBackend(
+  apiClient: ApiClient,
+  workspaceId: string,
+  reviewStepId: string,
+  titles: string[],
+) {
+  await expect
+    .poll(
+      async () => {
+        const { tasks } = await apiClient.listTasks(workspaceId);
+        return titles.filter((title) =>
+          tasks.some((task) => task.title === title && task.workflow_step_id === reviewStepId),
+        ).length;
+      },
+      {
+        timeout: 60_000,
+        intervals: [500, 1_000, 2_000],
+        message: `Expected ${titles.length} PR tasks to exist in the Review step`,
+      },
+    )
+    .toBe(titles.length);
+}
+
+async function waitForReviewTaskCards(
+  testPage: Page,
+  kanban: KanbanPage,
+  reviewStepId: string,
+  titles: string[],
+) {
+  async function visibleCardCount() {
+    const visible = await Promise.all(
+      titles.map((title) => kanban.taskCardInColumn(title, reviewStepId).isVisible()),
+    );
+    return visible.filter(Boolean).length;
+  }
+
+  const expected = titles.length;
+  await expect
+    .poll(visibleCardCount, { timeout: 10_000, intervals: [500, 1_000] })
+    .toBe(expected)
+    .catch(async () => {
+      // The watcher creates tasks from a background poll. If task.created events
+      // arrive before the kanban subscription is fully attached, backend state is
+      // correct but one card can be absent until SSR rehydrates the board.
+      await testPage.reload();
+      await kanban.board.waitFor({ state: "visible", timeout: 15_000 });
+    });
+
+  await expect
+    .poll(visibleCardCount, {
+      timeout: 90_000,
+      intervals: [500, 1_000, 2_000],
+      message: `Expected all ${expected} PR task cards (${titles.join(", ")}) to appear in the Review column`,
+    })
+    .toBe(expected);
+}
+
 test.describe("PR watcher dockview layout stability", () => {
   /**
    * Verifies the dockview layout stays correct when switching between
@@ -149,21 +208,8 @@ test.describe("PR watcher dockview layout stability", () => {
     // out of order under load; waiting for the whole set at once avoids
     // cumulative per-card timeout pressure and settles as soon as all 3 appear.
     const prTaskTitles = [prTask1Title, prTask2Title, prTask3Title];
-    await expect
-      .poll(
-        async () => {
-          const visible = await Promise.all(
-            prTaskTitles.map((title) => kanban.taskCardInColumn(title, reviewStep.id).isVisible()),
-          );
-          return visible.filter(Boolean).length;
-        },
-        {
-          timeout: 90_000,
-          intervals: [500, 1_000, 2_000],
-          message: `Expected all 3 PR task cards (${prTaskTitles.join(", ")}) to appear in the Review column`,
-        },
-      )
-      .toBe(3);
+    await waitForReviewTasksInBackend(apiClient, seedData.workspaceId, reviewStep.id, prTaskTitles);
+    await waitForReviewTaskCards(testPage, kanban, reviewStep.id, prTaskTitles);
 
     // --- Click PR task 1 to enter session view ---
     await kanban.taskCardInColumn(prTask1Title, reviewStep.id).click();
diff --git a/apps/web/e2e/tests/terminal/mobile-terminal-helpers.ts b/apps/web/e2e/tests/terminal/mobile-terminal-helpers.ts
index 8a9144505c..d6c627b147 100644
--- a/apps/web/e2e/tests/terminal/mobile-terminal-helpers.ts
+++ b/apps/web/e2e/tests/terminal/mobile-terminal-helpers.ts
@@ -24,14 +24,19 @@ export async function switchToTerminalPanel(testPage: Page): Promise<void> {
 
 export async function readTerminalBuffer(page: Page): Promise<string> {
   return page.evaluate(() => {
-    const panel = document.querySelector('[data-testid="terminal-panel"]');
-    const xtermEl = panel?.querySelector(".xterm");
+    const panels = Array.from(document.querySelectorAll('[data-testid="terminal-panel"]'));
+    const visiblePanels = panels.filter((panel) => panel.getClientRects().length > 0);
+    const panel = visiblePanels.at(-1) ?? panels.at(-1);
     type XC = HTMLElement & { __xtermReadBuffer?: () => string };
-    const container = xtermEl?.parentElement as XC | null | undefined;
+    const container = panel?.querySelector('[data-testid="terminal-xterm-host"]') as XC | null;
     return container?.__xtermReadBuffer?.() ?? "";
   });
 }
 
+export async function focusTerminalForTyping(testPage: Page): Promise<void> {
+  await testPage.getByTestId("terminal-panel").last().getByTestId("terminal-xterm-host").click();
+}
+
 /**
  * Wait for the mobile shell to be ready by tailing xterm's buffer until it has
  * any content (a prompt is enough). Mobile mounts the terminal lazily on tab
diff --git a/apps/web/e2e/tests/terminal/mobile-terminal-keybar.spec.ts b/apps/web/e2e/tests/terminal/mobile-terminal-keybar.spec.ts
index f2d9a5e146..4269f72f34 100644
--- a/apps/web/e2e/tests/terminal/mobile-terminal-keybar.spec.ts
+++ b/apps/web/e2e/tests/terminal/mobile-terminal-keybar.spec.ts
@@ -12,7 +12,11 @@ import type { ApiClient } from "../../helpers/api-client";
 import { SessionPage } from "../../pages/session-page";
 import { MobileTerminalKeybarPage } from "../../pages/mobile-terminal-keybar-page";
 import { attachShellInputCapture, type ShellInputFrame } from "../../helpers/ws-capture";
-import { switchToTerminalPanel, waitForShellReady } from "./mobile-terminal-helpers";
+import {
+  focusTerminalForTyping,
+  switchToTerminalPanel,
+  waitForShellReady,
+} from "./mobile-terminal-helpers";
 
 async function seedTaskWithSession(
   testPage: Page,
@@ -38,14 +42,6 @@ async function seedTaskWithSession(
   return session;
 }
 
-/**
- * Bring up the OS keyboard the way a user would: tap inside the terminal so
- * xterm focuses its hidden textarea.
- */
-async function focusTerminalForTyping(session: SessionPage): Promise<void> {
-  await session.terminal.locator(".xterm").last().click();
-}
-
 /**
  * Drives the page's `visualViewport` the way the OS would when the keyboard
  * opens or the user scrolls with it open. Pure event-firing — nothing about
@@ -109,7 +105,7 @@ test.describe("Mobile terminal key-bar — user flows", () => {
     await expect(keybar.root).toBeVisible({ timeout: 10_000 });
 
     await waitForShellReady(testPage);
-    await focusTerminalForTyping(session);
+    await focusTerminalForTyping(testPage);
 
     // User types a command on the OS keyboard, then runs it.
     await testPage.keyboard.type("sleep 30");
@@ -130,13 +126,13 @@ test.describe("Mobile terminal key-bar — user flows", () => {
   }) => {
     test.setTimeout(90_000);
     const { frames } = attachShellInputCapture(testPage);
-    const session = await seedTaskWithSession(testPage, apiClient, seedData, "Keybar Ctrl+L");
+    await seedTaskWithSession(testPage, apiClient, seedData, "Keybar Ctrl+L");
     await switchToTerminalPanel(testPage);
 
     const keybar = new MobileTerminalKeybarPage(testPage);
     await expect(keybar.root).toBeVisible({ timeout: 10_000 });
     await waitForShellReady(testPage);
-    await focusTerminalForTyping(session);
+    await focusTerminalForTyping(testPage);
 
     // User taps Ctrl on the bar — visible state change.
     await keybar.ctrl.tap();
@@ -158,13 +154,13 @@ test.describe("Mobile terminal key-bar — user flows", () => {
   }) => {
     test.setTimeout(90_000);
     const { frames } = attachShellInputCapture(testPage);
-    const session = await seedTaskWithSession(testPage, apiClient, seedData, "Keybar sticky Ctrl");
+    await seedTaskWithSession(testPage, apiClient, seedData, "Keybar sticky Ctrl");
     await switchToTerminalPanel(testPage);
 
     const keybar = new MobileTerminalKeybarPage(testPage);
     await expect(keybar.root).toBeVisible({ timeout: 10_000 });
     await waitForShellReady(testPage);
-    await focusTerminalForTyping(session);
+    await focusTerminalForTyping(testPage);
 
     // Two taps → sticky.
     await keybar.ctrl.tap();
@@ -215,13 +211,13 @@ test.describe("Mobile terminal key-bar — user flows", () => {
   }) => {
     test.setTimeout(90_000);
     const { frames } = attachShellInputCapture(testPage);
-    const session = await seedTaskWithSession(testPage, apiClient, seedData, "Keybar passthrough");
+    await seedTaskWithSession(testPage, apiClient, seedData, "Keybar passthrough");
     await switchToTerminalPanel(testPage);
 
     const keybar = new MobileTerminalKeybarPage(testPage);
     await expect(keybar.root).toBeVisible({ timeout: 10_000 });
     await waitForShellReady(testPage);
-    await focusTerminalForTyping(session);
+    await focusTerminalForTyping(testPage);
 
     await testPage.keyboard.press("c");
 
diff --git a/apps/web/e2e/tests/terminal/mobile-terminal-scroll.spec.ts b/apps/web/e2e/tests/terminal/mobile-terminal-scroll.spec.ts
index 69bd9bb754..d0d3138748 100644
--- a/apps/web/e2e/tests/terminal/mobile-terminal-scroll.spec.ts
+++ b/apps/web/e2e/tests/terminal/mobile-terminal-scroll.spec.ts
@@ -10,6 +10,7 @@ import type { SeedData } from "../../fixtures/test-base";
 import type { ApiClient } from "../../helpers/api-client";
 import { SessionPage } from "../../pages/session-page";
 import {
+  focusTerminalForTyping,
   readTerminalBuffer,
   switchToTerminalPanel,
   waitForShellReady,
@@ -41,18 +42,15 @@ async function seedTaskWithSession(
 
 async function readViewportY(page: Page): Promise<number> {
   return page.evaluate(() => {
-    const panel = document.querySelector('[data-testid="terminal-panel"]');
-    const xtermEl = panel?.querySelector(".xterm");
+    const panels = Array.from(document.querySelectorAll('[data-testid="terminal-panel"]'));
+    const visiblePanels = panels.filter((panel) => panel.getClientRects().length > 0);
+    const panel = visiblePanels.at(-1) ?? panels.at(-1);
     type XC = HTMLElement & { __xtermReadViewportY?: () => number };
-    const container = xtermEl?.parentElement as XC | null | undefined;
+    const container = panel?.querySelector('[data-testid="terminal-xterm-host"]') as XC | null;
     return container?.__xtermReadViewportY?.() ?? -1;
   });
 }
 
-async function focusTerminalForTyping(session: SessionPage): Promise<void> {
-  await session.terminal.locator(".xterm").click();
-}
-
 /**
  * Dispatch a synthetic single-finger swipe on the xterm container. We bypass
  * Playwright's touchscreen because the gesture coordinates need to be relative
@@ -68,7 +66,10 @@ async function swipe(
   await page.evaluate(
     ({ direction, steps, rowsToScroll }) => {
       const panel = document.querySelector('[data-testid="terminal-panel"]');
-      const xtermEl = panel?.querySelector(".xterm") as HTMLElement | null;
+      // xterm registers touch handling on its generated element, so this test
+      // intentionally locates that implementation node for event dispatch.
+      const xterms = Array.from(panel?.querySelectorAll(".xterm") ?? []);
+      const xtermEl = (panel?.querySelector(".xterm.focus") ?? xterms.at(-1)) as HTMLElement | null;
       if (!xtermEl) throw new Error("xterm element not found");
       const rect = xtermEl.getBoundingClientRect();
       const cx = rect.left + rect.width / 2;
@@ -111,10 +112,10 @@ test.describe("Mobile passthrough terminal — touch scroll", () => {
     seedData,
   }) => {
     test.setTimeout(120_000);
-    const session = await seedTaskWithSession(testPage, apiClient, seedData, "Touch scroll");
+    await seedTaskWithSession(testPage, apiClient, seedData, "Touch scroll");
     await switchToTerminalPanel(testPage);
     await waitForShellReady(testPage);
-    await focusTerminalForTyping(session);
+    await focusTerminalForTyping(testPage);
 
     // Produce enough output to populate the scrollback past one viewport.
     await typeAndRun(testPage, "for i in $(seq 1 200); do echo line $i; done");