feat: auto-show PR details panel when task has associated PR (#517)

2026-04-02 00:08:35 +01:00
parent 1c6506911e
commit dbe778e4e8
7 changed files with 341 additions and 70 deletions
@@ -60,7 +60,11 @@ export function PRDetailPanelComponent({ panelId }: PRDetailPanelProps) {
    );
  }

-  return <PRDetailContent taskPR={pr} sessionId={sessionId} />;
+  return (
+    <div data-testid="pr-detail-panel" className="h-full">
+      <PRDetailContent taskPR={pr} sessionId={sessionId} />
+    </div>
+  );
 }

 // --- Add PR feedback as chat context ---
@@ -205,13 +209,15 @@ function ApproveButton({
  );
 }

-function PRDetailContent({ taskPR, sessionId }: { taskPR: TaskPR; sessionId: string }) {
+export function PRDetailContent({ taskPR, sessionId }: { taskPR: TaskPR; sessionId: string }) {
  const { feedback, loading, refresh } = usePRFeedback(taskPR.owner, taskPR.repo, taskPR.pr_number);
  const { addAsContext } = useAddPRFeedbackAsContext(sessionId, taskPR.pr_number);
  const setTaskPR = useAppStore((s) => s.setTaskPR);

  // Sync live feedback data back to the store so topbar/other consumers stay up to date.
  // Use primitive deps to avoid re-render loops from object reference changes.
+  // Guard: never regress the store to a less-terminal state (e.g. merged → open)
+  // because the feedback fetch may return stale data from before a backend poll update.
  const prState = taskPR.state;
  const prMergedAt = taskPR.merged_at ?? null;
  const prClosedAt = taskPR.closed_at ?? null;
@@ -221,20 +227,29 @@ function PRDetailContent({ taskPR, sessionId }: { taskPR: TaskPR; sessionId: str
  useEffect(() => {
    if (!feedback) return;
    const livePR = feedback.pr;
+    // State priority: merged > closed > open. Never regress to a less-terminal state.
+    const stateRank = (s: string) => {
+      if (s === "merged") return 2;
+      if (s === "closed") return 1;
+      return 0;
+    };
+    const effectiveState = stateRank(livePR.state) >= stateRank(prState) ? livePR.state : prState;
+    const effectiveMergedAt = effectiveState === prState ? prMergedAt : (livePR.merged_at ?? null);
+    const effectiveClosedAt = effectiveState === prState ? prClosedAt : (livePR.closed_at ?? null);
    if (
-      livePR.state !== prState ||
-      (livePR.merged_at ?? null) !== prMergedAt ||
-      (livePR.closed_at ?? null) !== prClosedAt ||
+      effectiveState !== prState ||
+      effectiveMergedAt !== prMergedAt ||
+      effectiveClosedAt !== prClosedAt ||
      livePR.additions !== prAdditions ||
      livePR.deletions !== prDeletions
    ) {
      setTaskPR(prTaskId, {
        ...taskPR,
-        state: livePR.state,
+        state: effectiveState as TaskPR["state"],
        additions: livePR.additions,
        deletions: livePR.deletions,
-        merged_at: livePR.merged_at,
-        closed_at: livePR.closed_at,
+        merged_at: effectiveMergedAt,
+        closed_at: effectiveClosedAt,
      });
    }
    // eslint-disable-next-line react-hooks/exhaustive-deps
@@ -0,0 +1,42 @@
+import { describe, it, expect } from "vitest";
+import { shouldAutoAddPRPanel } from "../dockview-session-tabs";
+
+describe("shouldAutoAddPRPanel", () => {
+  const base = {
+    hasPR: true,
+    panelExists: false,
+    isRestoringLayout: false,
+    isMaximized: false,
+  };
+
+  it("returns 'add' when task has PR and panel does not exist", () => {
+    expect(shouldAutoAddPRPanel(base)).toBe("add");
+  });
+
+  it("returns 'none' when task has no PR", () => {
+    expect(shouldAutoAddPRPanel({ ...base, hasPR: false })).toBe("none");
+  });
+
+  it("returns 'none' when panel already exists", () => {
+    expect(shouldAutoAddPRPanel({ ...base, panelExists: true })).toBe("none");
+  });
+
+  it("returns 'none' during layout restoration", () => {
+    expect(shouldAutoAddPRPanel({ ...base, isRestoringLayout: true })).toBe("none");
+  });
+
+  it("returns 'none' during maximize state", () => {
+    expect(shouldAutoAddPRPanel({ ...base, isMaximized: true })).toBe("none");
+  });
+
+  it("returns 'add' when all conditions are met", () => {
+    expect(
+      shouldAutoAddPRPanel({
+        hasPR: true,
+        panelExists: false,
+        isRestoringLayout: false,
+        isMaximized: false,
+      }),
+    ).toBe("add");
+  });
+});
@@ -3,6 +3,7 @@ import type { DockviewReadyEvent, AddPanelOptions } from "dockview-react";
 import type { StoreApi } from "zustand";
 import type { AppState } from "@/lib/state/store";
 import { useDockviewStore } from "@/lib/state/dockview-store";
+import { useAppStore } from "@/components/state-provider";

 /**
 * Sync `activeSessionId` in the store when the user clicks a session tab.
@@ -75,6 +76,79 @@ export function setupChatPanelSafetyNet(
  });
 }

+// ---------------------------------------------------------------------------
+// Auto-show PR detail panel
+// ---------------------------------------------------------------------------
+
+/** Pure decision function for whether the PR panel should be auto-added. */
+export function shouldAutoAddPRPanel(params: {
+  hasPR: boolean;
+  panelExists: boolean;
+  isRestoringLayout: boolean;
+  isMaximized: boolean;
+}): "add" | "none" {
+  if (!params.hasPR) return "none";
+  if (params.panelExists) return "none";
+  if (params.isRestoringLayout) return "none";
+  if (params.isMaximized) return "none";
+  return "add";
+}
+
+/**
+ * Auto-add the PR detail panel to the center group when the active task
+ * has an associated pull request. The panel is added as a background tab
+ * (the session/agent tab stays focused).
+ *
+ * Reads task ID and PR state from the app store internally.
+ *
+ * Tracks per-task dismissal: if the user manually closes the PR panel,
+ * it won't be re-added for the same task until they navigate away and back.
+ */
+export function useAutoPRPanel() {
+  const taskId = useAppStore((s) => s.tasks.activeTaskId);
+  const hasPR = useAppStore((s) => {
+    const tid = s.tasks.activeTaskId;
+    return tid ? !!s.taskPRs.byTaskId[tid] : false;
+  });
+  // Subscribe to dockview API readiness so the effect re-runs when API appears.
+  const hasApi = useDockviewStore((s) => !!s.api);
+  // Track which tasks have already been handled to avoid re-adding on every
+  // store update or task switch. Only auto-add once per task visit.
+  const handledRef = useRef<Set<string>>(new Set());
+
+  useEffect(() => {
+    if (!taskId || !hasPR || !hasApi) return;
+    if (handledRef.current.has(taskId)) return;
+
+    // Defer to avoid racing with layout restore on initial load / session switch.
+    requestAnimationFrame(() => {
+      requestAnimationFrame(() => {
+        const api = useDockviewStore.getState().api;
+        if (!api) return;
+        if (useDockviewStore.getState().isRestoringLayout) return;
+        if (useDockviewStore.getState().preMaximizeLayout !== null) return;
+        if (api.getPanel("pr-detail")) {
+          // Panel exists from a restored layout — just mark as handled.
+          handledRef.current.add(taskId);
+          return;
+        }
+
+        const { centerGroupId } = useDockviewStore.getState();
+        const centerGroupExists = centerGroupId && api.groups.some((g) => g.id === centerGroupId);
+
+        api.addPanel({
+          id: "pr-detail",
+          component: "pr-detail",
+          title: "Pull Request",
+          position: centerGroupExists ? { referenceGroup: centerGroupId } : undefined,
+          inactive: true,
+        });
+        handledRef.current.add(taskId);
+      });
+    });
+  }, [taskId, hasPR, hasApi]);
+}
+
 /**
 * Auto-create a session tab when a session becomes active.
 * Replaces the generic "chat" panel with a per-session tab on first use.
@@ -0,0 +1,63 @@
+"use client";
+
+import { TabsContent } from "@kandev/ui/tabs";
+import { FileEditorContent } from "./file-editor-content";
+import { FileImageViewer } from "./file-image-viewer";
+import { FileBinaryViewer } from "./file-binary-viewer";
+import type { OpenFileTab } from "@/lib/types/backend";
+import { getFileCategory } from "@/lib/utils/file-types";
+
+export function FileTabContent({
+  tab,
+  activeSession,
+  activeSessionId,
+  isSaving,
+  onFileChange,
+  onFileSave,
+  onFileDelete,
+}: {
+  tab: OpenFileTab;
+  activeSession: { worktree_path?: string | null } | null;
+  activeSessionId: string | null;
+  isSaving: boolean;
+  onFileChange: (path: string, content: string) => void;
+  onFileSave: (path: string) => void;
+  onFileDelete: (path: string) => void;
+}) {
+  const extCategory = getFileCategory(tab.path);
+  let category: "image" | "binary" | "text" = "text";
+  if (tab.isBinary) category = extCategory === "image" ? "image" : "binary";
+
+  return (
+    <TabsContent value={`file:${tab.path}`} className="flex-1 min-h-0">
+      {category === "image" && (
+        <FileImageViewer
+          path={tab.path}
+          content={tab.content}
+          worktreePath={activeSession?.worktree_path ?? undefined}
+        />
+      )}
+      {category === "binary" && (
+        <FileBinaryViewer
+          path={tab.path}
+          worktreePath={activeSession?.worktree_path ?? undefined}
+        />
+      )}
+      {category === "text" && (
+        <FileEditorContent
+          path={tab.path}
+          content={tab.content}
+          originalContent={tab.originalContent}
+          isDirty={tab.isDirty}
+          isSaving={isSaving}
+          sessionId={activeSessionId || undefined}
+          worktreePath={activeSession?.worktree_path ?? undefined}
+          enableComments={!!activeSessionId}
+          onChange={(newContent) => onFileChange(tab.path, newContent)}
+          onSave={() => onFileSave(tab.path)}
+          onDelete={() => onFileDelete(tab.path)}
+        />
+      )}
+    </TabsContent>
+  );
+}
@@ -14,9 +14,7 @@ import {
 import { SessionPanel } from "@kandev/ui/pannel-session";
 import { TaskChatPanel } from "./task-chat-panel";
 import { TaskChangesPanel } from "./task-changes-panel";
-import { FileEditorContent } from "./file-editor-content";
-import { FileImageViewer } from "./file-image-viewer";
-import { FileBinaryViewer } from "./file-binary-viewer";
+import { FileTabContent } from "./file-tab-content";
 import { PassthroughTerminal } from "./passthrough-terminal";
 import type { OpenFileTab, FileContentResponse } from "@/lib/types/backend";
 import { useAppStore } from "@/components/state-provider";
@@ -32,9 +30,10 @@ import {
 import { useSessionGitStatus } from "@/hooks/domains/session/use-session-git-status";
 import { useSessionCommits } from "@/hooks/domains/session/use-session-commits";
 import { calculateHash } from "@/lib/utils/file-diff";
-import { getFileCategory } from "@/lib/utils/file-types";
 import { useToast } from "@/components/toast-provider";
 import { useFileTabRestoration, useFileSaveDelete } from "./task-center-panel-restoration";
+import { useActiveTaskPR } from "@/hooks/domains/github/use-task-pr";
+import { PRDetailContent } from "@/components/github/pr-detail-panel";

 import type { SelectedDiff } from "./task-layout";

@@ -231,11 +230,13 @@ function useCenterPanelTabs(
  openFileTabs: OpenFileTab[],
  handleCloseFileTab: (path: string) => void,
  hasChanges: boolean | undefined,
+  hasPR: boolean,
 ) {
  const tabs: SessionTab[] = useMemo(() => {
    const staticTabs: SessionTab[] = [
      ...(hasChanges ? [{ id: "changes", label: "All changes" }] : []),
      { id: "chat", label: "Chat" },
+      ...(hasPR ? [{ id: "pr", label: "Pull Request" }] : []),
    ];
    const fileTabs: SessionTab[] = openFileTabs.map((tab) => ({
      id: `file:${tab.path}`,
@@ -249,11 +250,12 @@ function useCenterPanelTabs(
      className: "cursor-pointer group gap-1.5 data-[state=active]:bg-muted",
    }));
    return [...staticTabs, ...fileTabs];
-  }, [openFileTabs, handleCloseFileTab, hasChanges]);
+  }, [openFileTabs, handleCloseFileTab, hasChanges, hasPR]);
  const separatorAfterIndex = useMemo(() => {
    if (openFileTabs.length === 0) return undefined;
-    return hasChanges ? 1 : 0;
-  }, [openFileTabs.length, hasChanges]);
+    const staticCount = (hasChanges ? 1 : 0) + 1 + (hasPR ? 1 : 0); // changes + chat + pr
+    return staticCount - 1;
+  }, [openFileTabs.length, hasChanges, hasPR]);
  return { tabs, separatorAfterIndex };
 }

@@ -277,6 +279,8 @@ function useCenterPanelState(props: TaskCenterPanelProps) {
    activeSessionId,
    activeTaskId,
  );
+  const taskPR = useActiveTaskPR();
+  const hasPR = !!taskPR;
  const [openFileTabs, setOpenFileTabs] = useState<OpenFileTab[]>([]);
  const [savingFiles, setSavingFiles] = useState<Set<string>>(new Set());
  const [selectedDiff, setSelectedDiff] = useState<SelectedDiff | null>(null);
@@ -309,6 +313,7 @@ function useCenterPanelState(props: TaskCenterPanelProps) {
    openFileTabs,
    fileTabOps.handleCloseFileTab,
    hasChanges,
+    hasPR,
  );

  useEffect(() => {
@@ -342,6 +347,7 @@ function useCenterPanelState(props: TaskCenterPanelProps) {
    isPassthroughMode,
    showApproveButton,
    handleApprove,
+    taskPR,
    openFileTabs,
    savingFiles,
    selectedDiff,
@@ -367,6 +373,7 @@ export const TaskCenterPanel = memo(function TaskCenterPanel(props: TaskCenterPa
    isPassthroughMode,
    showApproveButton,
    handleApprove,
+    taskPR,
    openFileTabs,
    savingFiles,
    selectedDiff,
@@ -411,6 +418,11 @@ export const TaskCenterPanel = memo(function TaskCenterPanel(props: TaskCenterPa
          onDismissTooltip={() => setShowRequestChangesTooltip(false)}
          onOpenFile={handleOpenFileFromChat}
        />
+        {taskPR && activeSessionId && (
+          <TabsContent value="pr" className="flex-1 min-h-0" data-testid="pr-detail-panel">
+            <PRDetailContent taskPR={taskPR} sessionId={activeSessionId} />
+          </TabsContent>
+        )}
        {openFileTabs.map((tab) => (
          <FileTabContent
            key={tab.path}
@@ -534,58 +546,3 @@ function ChatTabContent({
    </TabsContent>
  );
 }
-
-function FileTabContent({
-  tab,
-  activeSession,
-  activeSessionId,
-  isSaving,
-  onFileChange,
-  onFileSave,
-  onFileDelete,
-}: {
-  tab: OpenFileTab;
-  activeSession: { worktree_path?: string | null } | null;
-  activeSessionId: string | null;
-  isSaving: boolean;
-  onFileChange: (path: string, content: string) => void;
-  onFileSave: (path: string) => void;
-  onFileDelete: (path: string) => void;
-}) {
-  const extCategory = getFileCategory(tab.path);
-  let category: "image" | "binary" | "text" = "text";
-  if (tab.isBinary) category = extCategory === "image" ? "image" : "binary";
-
-  return (
-    <TabsContent value={`file:${tab.path}`} className="flex-1 min-h-0">
-      {category === "image" && (
-        <FileImageViewer
-          path={tab.path}
-          content={tab.content}
-          worktreePath={activeSession?.worktree_path ?? undefined}
-        />
-      )}
-      {category === "binary" && (
-        <FileBinaryViewer
-          path={tab.path}
-          worktreePath={activeSession?.worktree_path ?? undefined}
-        />
-      )}
-      {category === "text" && (
-        <FileEditorContent
-          path={tab.path}
-          content={tab.content}
-          originalContent={tab.originalContent}
-          isDirty={tab.isDirty}
-          isSaving={isSaving}
-          sessionId={activeSessionId || undefined}
-          worktreePath={activeSession?.worktree_path ?? undefined}
-          enableComments={!!activeSessionId}
-          onChange={(newContent) => onFileChange(tab.path, newContent)}
-          onSave={() => onFileSave(tab.path)}
-          onDelete={() => onFileDelete(tab.path)}
-        />
-      )}
-    </TabsContent>
-  );
-}
@@ -281,6 +281,16 @@ export class SessionPage {
    return this.page.getByTestId("pr-topbar-button");
  }

+  /** PR detail panel (auto-shown when task has an associated PR). */
+  prDetailPanel(): Locator {
+    return this.page.getByTestId("pr-detail-panel");
+  }
+
+  /** Dockview tab for the PR detail panel (title starts as "Pull Request", updated to "PR #N"). */
+  prDetailTab(): Locator {
+    return this.page.locator(".dv-default-tab").filter({ hasText: /^(Pull Request|PR #\d+)$/ });
+  }
+
  /** Click a dockview tab by its visible label (e.g. "Changes", "Files", "Terminal"). */
  async clickTab(label: string): Promise<void> {
    const tab = this.page.locator(`.dv-default-tab:has-text('${label}')`);
@@ -0,0 +1,110 @@
+import { test, expect } from "../../fixtures/test-base";
+import { KanbanPage } from "../../pages/kanban-page";
+import { SessionPage } from "../../pages/session-page";
+
+test.describe("PR detail panel", () => {
+  /**
+   * Verifies that the PR detail panel can be opened via the topbar button
+   * when a task has an associated pull request, and renders PR content.
+   *
+   * Setup:
+   *   Inbox → Working (auto_start, on_turn_complete → Done) → Done
+   *   Task A (with PR #101)
+   */
+  test("opens PR detail panel via topbar button", async ({ testPage, apiClient, seedData }) => {
+    test.setTimeout(120_000);
+
+    // --- Seed workflow ---
+    const workflow = await apiClient.createWorkflow(seedData.workspaceId, "PR Panel Workflow");
+
+    const inboxStep = await apiClient.createWorkflowStep(workflow.id, "Inbox", 0);
+    const workingStep = await apiClient.createWorkflowStep(workflow.id, "Working", 1);
+    const doneStep = await apiClient.createWorkflowStep(workflow.id, "Done", 2);
+
+    await apiClient.updateWorkflowStep(workingStep.id, {
+      prompt: 'e2e:message("done")\n{{task_prompt}}',
+      events: {
+        on_enter: [{ type: "auto_start_agent" }],
+        on_turn_complete: [{ type: "move_to_step", config: { step_id: doneStep.id } }],
+      },
+    });
+
+    await apiClient.saveUserSettings({
+      workspace_id: seedData.workspaceId,
+      workflow_filter_id: workflow.id,
+      enable_preview_on_click: false,
+    });
+
+    // --- Seed mock GitHub data ---
+    await apiClient.mockGitHubReset();
+    await apiClient.mockGitHubSetUser("test-user");
+
+    await apiClient.mockGitHubAddPRs([
+      {
+        number: 101,
+        title: "Fix auth bug",
+        state: "open",
+        head_branch: "fix/auth",
+        base_branch: "main",
+        author_login: "test-user",
+        repo_owner: "testorg",
+        repo_name: "testrepo",
+        additions: 10,
+        deletions: 2,
+      },
+    ]);
+
+    // --- Create task ---
+    const taskA = await apiClient.createTask(seedData.workspaceId, "Auth Fix Task", {
+      workflow_id: workflow.id,
+      workflow_step_id: inboxStep.id,
+      agent_profile_id: seedData.agentProfileId,
+      repository_ids: [seedData.repositoryId],
+    });
+
+    // Navigate to kanban BEFORE moving task
+    const kanban = new KanbanPage(testPage);
+    await kanban.goto();
+
+    // Move task to Working → auto_start → mock agent completes → Done
+    await apiClient.moveTask(taskA.id, workflow.id, workingStep.id);
+
+    // Associate PR with Task A
+    await apiClient.mockGitHubAssociateTaskPR({
+      task_id: taskA.id,
+      owner: "testorg",
+      repo: "testrepo",
+      pr_number: 101,
+      pr_url: "https://github.com/testorg/testrepo/pull/101",
+      pr_title: "Fix auth bug",
+      head_branch: "fix/auth",
+      base_branch: "main",
+      author_login: "test-user",
+      additions: 10,
+      deletions: 2,
+    });
+
+    // Wait for task to reach Done
+    await expect(kanban.taskCardInColumn("Auth Fix Task", doneStep.id)).toBeVisible({
+      timeout: 45_000,
+    });
+
+    // --- Open Task A ---
+    await kanban.taskCardInColumn("Auth Fix Task", doneStep.id).click();
+    await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 });
+
+    const session = new SessionPage(testPage);
+    await session.waitForLoad();
+    await session.idleInput().waitFor({ state: "visible", timeout: 30_000 });
+
+    // Verify PR topbar button appears with PR number
+    await expect(session.prTopbarButton()).toBeVisible({ timeout: 15_000 });
+    await expect(session.prTopbarButton()).toContainText("#101");
+
+    // Click PR topbar button to open the PR detail panel
+    await session.prTopbarButton().click();
+
+    // Verify PR detail panel renders with content
+    await expect(session.prDetailPanel()).toBeVisible({ timeout: 10_000 });
+  });
+});