refactor: redesign PR status with visual prominence

.claude/agent-memory/code-reviewer/MEMORY.md

@@ -16,6 +16,14 @@
 - `[animation-fill-mode:backwards]` survives twMerge when animate-[] class is replaced — benign
   but semantically imprecise on infinite animations
 
+## Dead Code Pattern (Confirmed)
+
+- When removing a prop from a call site, also check the destructuring site of the hook that
+  originally provided the handler. E.g., removing `onReviewPR={handleOpenPR}` from `<PRActions>`
+  leaves `handleOpenPR` destructured-but-unused in `MainContent.tsx` (from `useWorkspaceActions`).
+  TypeScript does not error on unused destructured variables — only ESLint's `no-unused-vars` rule
+  catches this. Always grep for all references after removing a prop.
+
 ## Common Pitfalls Seen
 
 - `useLayoutEffect` SSR warning: hook uses `typeof window` guard for SSR safety in `useState`
@@ -150,3 +158,40 @@
 - `bg-accent-green`, `bg-accent-gold`, `bg-accent-red` ARE valid Tailwind tokens — defined in
   `global.css` `@theme` block as `--color-accent-green`, `--color-accent-gold`, `--color-accent-red`.
   Not hardcoded colors; they route through CSS variables to theme values.
+- `bg-warning`, `bg-success`, `text-warning` ARE valid — `--color-warning` and `--color-success`
+  defined in `@theme` block (lines 30, 32 of global.css). Safe to use in action button variants.
+
+## PRStatus / GhCliStatus Patterns (Confirmed)
+
+- `PRStatus.pr_url` is optional (`pr_url?: string`). Falling back to `""` on undefined produces
+  `href=""` in a Tauri WebView which navigates to `tauri://localhost/` — silent app reload. Always
+  guard: use `pr_url ?? null` and skip rendering the anchor when null.
+- `PRStatus.pr_state` includes `"closed"` (abandoned PR, not merged). Failing to handle this case
+  causes the state machine to fall through to actionable states (Fix CI, Resolve Conflicts) on a
+  dead branch — prompting the agent to work on an already-closed PR.
+- `ghStatus` being `null`/`undefined` (TanStack Query loading) should be treated as "unknown, do
+  not surface action buttons" — not as "gh is available." The guard `if (ghStatus && ...)` silently
+  passes null/undefined through to PR state evaluation.
+- `derivePRActionState` in `src/features/workspace/lib/prState.ts` is the single source of truth
+  for PR state machine. Pure function, no tests exist yet — high-value test target.
+- `PRActionState` discriminated union: 11 variants (added `closed` and `error`). `match().exhaustive()`
+  used in PRActions.tsx main render. PRLink uses a non-exhaustive if-chain guard (early return for
+  `gh_unavailable`, `no_pr`, `error`) — this remains a type-safety gap when new variants are added.
+- `prUrl = prStatus.pr_url ?? ""` in `derivePRActionState` (line 75). The `""` fallback still produces
+  `href=""` → `tauri://localhost/` reload risk, BUT `PRLink` filters on `!= "gh_unavailable" | "no_pr" | "error"`
+  so the only states that render the anchor are states where `pr_url` is always set by the backend (has_pr=true path).
+  Risk is low in practice but the type system doesn't enforce it (string, not string & URL).
+- `FAILING_CONCLUSIONS` and `PENDING_STATES` sets are defined INSIDE the request handler function
+  (inside the `app.get(...)` callback), so they are re-created on every request. Move to module scope.
+- `lastError` logic: `runGh` returns only `'unknown'` for non-specific errors, so the check
+  `lastError === 'unknown' ? 'network' : null` always maps to `'network'` when set. Correct but
+  the conditional is redundant — could just be `lastError ? 'network' : null`.
+- CI `hasPending` check has a subtle issue: when `c.conclusion == null` AND `PENDING_STATES.has(c.state)`
+  are both truthy, the OR short-circuits at `c.conclusion == null`. This is correct for in-progress checks.
+  However, a check with `conclusion === null` and `state` NOT in PENDING_STATES (e.g. a weird state)
+  would still mark it pending — acceptable given the unknown = pending safety heuristic.
+- `query.state.data` in `usePRStatus` refetchInterval callback is cast to a loose object type instead
+  of `PRStatus | null`. Should use `import type { PRStatus }` and cast to `PRStatus | null | undefined`.
+- `review_required` and `approved` review statuses are not mapped to PRActionState variants. They both
+  fall through to `awaiting_review`. This is intentional (safest default) but `review_required` could
+  deserve its own state in a future iteration.

backend/src/routes/workspaces.ts

@@ -209,6 +209,18 @@ app.get('/gh-status', async (c) => {
   return c.json({ isInstalled: true, isAuthenticated: authResult.success });
 });
 
+// GitHub Check Suite conclusions that indicate a non-passing terminal state.
+// Full GraphQL enum: ACTION_REQUIRED, CANCELLED, FAILURE, NEUTRAL, SKIPPED,
+// STALE, STARTUP_FAILURE, SUCCESS, TIMED_OUT.
+// NEUTRAL/SKIPPED are intentionally non-blocking (count as passing).
+// STALE means re-run is needed (count as pending below).
+const FAILING_CONCLUSIONS = new Set([
+  'FAILURE', 'ERROR', 'TIMED_OUT', 'STARTUP_FAILURE', 'ACTION_REQUIRED', 'CANCELLED',
+]);
+// CheckRun `status` values that indicate the check hasn't completed yet.
+// Note: CheckRun uses `status` field, StatusContext uses `state` field.
+const PENDING_STATUSES = new Set(['PENDING', 'QUEUED', 'IN_PROGRESS', 'WAITING', 'REQUESTED']);
+
 // PR status — async, fork-aware, explicit errors
 app.get('/workspaces/:id/pr-status', withWorkspace, async (c) => {
   const workspacePath = c.get('workspacePath');
@@ -247,40 +259,86 @@ app.get('/workspaces/:id/pr-status', withWorkspace, async (c) => {
   if (isFork) attempts.push({ repoArg: upstreamUrl, headArg: headBranch });
   attempts.push({ repoArg: originUrl, headArg: headBranch });
 
+  let lastError: string | null = null;
+  let hadSuccessfulResponse = false;
+
   for (const { repoArg, headArg } of attempts) {
     const args = ['pr', 'list', '--head', headArg, '--author', '@me', '--state', 'all',
       '--json', 'number,title,url,state,mergeable,mergeStateStatus,statusCheckRollup,reviewDecision,isDraft'];
     if (repoArg) args.push('--repo', repoArg);
 
     const result = await runGh(args, { cwd: workspacePath });
     if (!result.success) {
-      // Surface specific errors (installed/auth) to the frontend
+      // Surface specific errors (installed/auth) to the frontend immediately
       if (result.error === 'gh_not_installed' || result.error === 'gh_not_authenticated' || result.error === 'timeout') {
         return c.json({ has_pr: false, error: result.error });
       }
-      continue; // Try next attempt for unknown errors (e.g. repo not found)
+      lastError = result.error; // Track for surfacing if all attempts fail
+      continue;
     }
 
     let prs: any[];
     try { prs = JSON.parse(result.stdout || '[]'); } catch { continue; }
     if (!Array.isArray(prs)) continue;
+    hadSuccessfulResponse = true;
 
+    // Priority: OPEN > MERGED > CLOSED. Open PRs are actionable,
+    // merged PRs show archive, closed PRs show a non-actionable status.
     const openPr = prs.find((pr: any) => pr.state?.toUpperCase() === 'OPEN');
     const mergedPr = prs.find((pr: any) => pr.state?.toUpperCase() === 'MERGED');
-    const pr = openPr ?? mergedPr;
+    const closedPr = prs.find((pr: any) => pr.state?.toUpperCase() === 'CLOSED');
+    const pr = openPr ?? mergedPr ?? closedPr;
 
     if (pr) {
-      const state = pr.state?.toUpperCase() === 'MERGED' ? 'merged' : 'open';
+      const upperState = pr.state?.toUpperCase();
+      const state: 'open' | 'merged' | 'closed' =
+        upperState === 'MERGED' ? 'merged' :
+        upperState === 'CLOSED' ? 'closed' : 'open';
+
+      // Closed PRs are terminal — no CI or merge status is relevant
+      if (state === 'closed') {
+        return c.json({
+          has_pr: true,
+          pr_number: pr.number,
+          pr_title: pr.title,
+          pr_url: pr.url,
+          pr_state: 'closed',
+          merge_status: 'blocked',
+          is_draft: pr.isDraft === true,
+          has_conflicts: false,
+          ci_status: 'unknown',
+          review_status: 'none',
+          error: null,
+        });
+      }
+
       let mergeStatus: 'ready' | 'blocked' | 'merged' = 'blocked';
       if (state === 'merged') mergeStatus = 'merged';
       else if (pr.mergeable === 'MERGEABLE') mergeStatus = 'ready';
 
-      // Derive CI status from statusCheckRollup
+      // Derive CI status from statusCheckRollup.
+      // The array contains TWO different object types:
+      //   - CheckRun (__typename: "CheckRun"): uses `conclusion` (SUCCESS/FAILURE/null) + `status`
+      //   - StatusContext (__typename: "StatusContext"): uses `state` (SUCCESS/FAILURE/PENDING/ERROR)
+      // We must handle both — StatusContext has no `conclusion` field at all.
       const checks: any[] = pr.statusCheckRollup ?? [];
       let ciStatus: 'passing' | 'failing' | 'pending' | 'unknown' = 'unknown';
       if (checks.length > 0) {
-        const hasFailing = checks.some((c: any) => c.conclusion === 'FAILURE' || c.conclusion === 'ERROR' || c.conclusion === 'TIMED_OUT');
-        const hasPending = checks.some((c: any) => !c.conclusion || c.state === 'PENDING' || c.state === 'QUEUED' || c.state === 'IN_PROGRESS');
+        const hasFailing = checks.some((c: any) => {
+          if (c.__typename === 'StatusContext') {
+            return c.state === 'FAILURE' || c.state === 'ERROR';
+          }
+          return FAILING_CONCLUSIONS.has(c.conclusion);
+        });
+        const hasPending = checks.some((c: any) => {
+          if (c.__typename === 'StatusContext') {
+            return c.state === 'PENDING' || c.state === 'EXPECTED';
+          }
+          // CheckRun: null conclusion means still running, STALE means re-run needed
+          return c.conclusion === 'STALE' ||
+            c.conclusion == null ||
+            PENDING_STATUSES.has(c.status);
+        });
         if (hasFailing) ciStatus = 'failing';
         else if (hasPending) ciStatus = 'pending';
         else ciStatus = 'passing';
@@ -308,7 +366,9 @@ app.get('/workspaces/:id/pr-status', withWorkspace, async (c) => {
     }
   }
 
-  return c.json({ has_pr: false, error: null });
+  // If all attempts failed with errors, surface it instead of silently showing "no PR".
+  // lastError is only set for 'unknown' errors (timeout/auth/install return immediately).
+  return c.json({ has_pr: false, error: (!hadSuccessfulResponse && lastError) ? 'network' : null });
 });
 
 // Pen files

shared/types/github.ts

@@ -18,7 +18,7 @@ export interface PRStatus {
   has_conflicts?: boolean;
   ci_status?: "passing" | "failing" | "pending" | "unknown";
   review_status?: "approved" | "changes_requested" | "review_required" | "none";
-  error?: "gh_not_installed" | "gh_not_authenticated" | "timeout" | null;
+  error?: "gh_not_installed" | "gh_not_authenticated" | "timeout" | "network" | null;
 }
 
 /**

src/app/layouts/MainContent.tsx

@@ -34,6 +34,7 @@ import { cn } from "@/shared/lib/utils";
 import { ResizablePanelGroup, ResizablePanel, ResizableHandle } from "@/components/ui/resizable";
 import { PanelLeft } from "lucide-react";
 import type { Workspace, PRStatus, GhCliStatus } from "@/shared/types";
+import { REVIEW_CODE } from "@/features/session/lib/sessionPrompts";
 import { emit } from "@/platform/tauri";
 import { useBrowserWindowStore } from "@/features/browser/store";
 import { ChatArea } from "./ChatArea";
@@ -91,7 +92,6 @@ export function MainContent({
     setSelectedTargetBranch,
     handleCreatePR,
     handleSendAgentMessage,
-    handleOpenPR,
     handleArchive,
     handleRetrySetup,
     handleViewSetupLogs,
@@ -100,7 +100,6 @@ export function MainContent({
     handleRunTask,
   } = useWorkspaceActions({
     selectedWorkspace,
-    prStatus,
     setRightSideTab,
   });
 
@@ -201,6 +200,11 @@ export function MainContent({
     [setRightSideTab]
   );
 
+  // Insert code review prompt into chat input
+  const handleInsertReviewPrompt = useCallback(() => {
+    workspaceChatPanelRef.current?.insertText(REVIEW_CODE);
+  }, [workspaceChatPanelRef]);
+
   return (
     <SidebarInset className="min-w-0">
       <div
@@ -315,7 +319,6 @@ export function MainContent({
                         ghStatus={ghStatus}
                         onCreatePR={createPRHandler ? handleCreatePR : undefined}
                         onSendAgentMessage={sendAgentMessageHandler ? handleSendAgentMessage : undefined}
-                        onReviewPR={handleOpenPR}
                         onArchive={handleArchive}
                         targetBranch={selectedTargetBranch}
                         onTargetBranchChange={setSelectedTargetBranch}
@@ -331,6 +334,7 @@ export function MainContent({
                         onOpenDiffTab={handleOpenDiff}
                         onOpenFilePreview={handleOpenFilePreview}
                         isWatched={isWatched}
+                        onReview={handleInsertReviewPrompt}
                       />
                     </div>
                   </div>

src/app/layouts/RightSidePanel.tsx

@@ -39,6 +39,8 @@ interface RightSidePanelProps {
   onOpenFilePreview: (filePath: string) => void;
   /** Whether file watcher is active -- disables polling in useFileChanges */
   isWatched?: boolean;
+  /** Insert a code review prompt into the chat input */
+  onReview?: () => void;
 }
 
 export function RightSidePanel({
@@ -47,6 +49,7 @@ export function RightSidePanel({
   onOpenDiffTab,
   onOpenFilePreview,
   isWatched = false,
+  onReview,
 }: RightSidePanelProps) {
   const { selectedFilePath, setSelectedFilePath, rightPanelTab, setRightPanelTab } =
     useWorkspaceLayout(workspace.id);
@@ -167,6 +170,7 @@ export function RightSidePanel({
           filterMode={filterMode}
           onFilterModeChange={handleFilterModeChange}
           workspaceGitInfo={workspaceGitInfo}
+          onReview={onReview}
         />
       )}
 

src/app/layouts/hooks/useWorkspaceActions.ts

@@ -19,17 +19,15 @@ import {
 import { WorkspaceService } from "@/features/workspace/api/workspace.service";
 import { queueTerminalTask } from "@/features/terminal/store/terminalTaskStore";
 import type { RightSideTab } from "@/features/workspace/store";
-import type { Workspace, PRStatus } from "@/shared/types";
+import type { Workspace } from "@/shared/types";
 
 interface UseWorkspaceActionsOptions {
   selectedWorkspace: Workspace | null;
-  prStatus: PRStatus | null;
   setRightSideTab: (tab: RightSideTab) => void;
 }
 
 export function useWorkspaceActions({
   selectedWorkspace,
-  prStatus,
   setRightSideTab,
 }: UseWorkspaceActionsOptions) {
   // PR handler bridge: ChatArea sets it, WorkspaceHeader consumes it.
@@ -76,14 +74,6 @@ export function useWorkspaceActions({
     [sendAgentMessageHandler]
   );
 
-  const handleOpenPR = useCallback(() => {
-    if (!prStatus?.pr_url) {
-      toast.error("PR link not available.");
-      return;
-    }
-    window.open(prStatus.pr_url, "_blank", "noopener,noreferrer");
-  }, [prStatus]);
-
   // --- Archive & retry ---
 
   const { mutate: archiveWorkspace } = useArchiveWorkspace();
@@ -154,7 +144,6 @@ export function useWorkspaceActions({
     // Action handlers
     handleCreatePR,
     handleSendAgentMessage,
-    handleOpenPR,
     handleArchive,
     handleRetrySetup,
     handleViewSetupLogs,

src/features/session/lib/sessionPrompts.ts

@@ -36,6 +36,16 @@ export const ADDRESS_REVIEW = "Address the review comments on the PR";
 /** Instructs the agent to merge the PR */
 export const MERGE_PR = "Merge the PR";
 
+/** Code review prompt — inserted into chat input from the Code panel Review button */
+export const REVIEW_CODE = `Review the current code changes in this workspace. Analyze the diff for:
+- Bugs, logic errors, or edge cases
+- Performance issues or unnecessary complexity
+- Security concerns (hardcoded secrets, injection, unsafe patterns)
+- Code style and consistency with the existing codebase
+- Missing error handling or tests
+
+Provide a concise summary of findings with specific file and line references.`;
+
 // ---------------------------------------------------------------------------
 // Workspace setup
 // ---------------------------------------------------------------------------

src/features/workspace/api/workspace.queries.ts

@@ -11,6 +11,7 @@ import { RepoService } from "@/features/repository/api/repository.service";
 import { queryKeys } from "@/shared/api/queryKeys";
 import { API_CONFIG } from "@/shared/config/api.config";
 import type { Workspace, RepoGroup, DiffStats } from "../types";
+import type { PRStatus } from "@/shared/types";
 import { createOptimisticWorkspace } from "../lib/workspace.utils";
 
 /**
@@ -279,7 +280,8 @@ export function useGhStatus() {
  *
  * Gated on gh CLI being installed + authenticated (like Codex).
  * Polls every 30s while agent is working (to detect PR creation),
- * stops polling when idle. Auto-refetched on session completion
+ * 60s while CI is pending post-session (CI runs 2-15min after push),
+ * stops polling otherwise. Auto-refetched on session completion
  * via useSessionEvents invalidation.
  */
 export function usePRStatus(
@@ -292,10 +294,18 @@ export function usePRStatus(
     queryFn: () => WorkspaceService.fetchPRStatus(workspaceId!),
     enabled: !!workspaceId && ghInstalled && ghAuthenticated,
     staleTime: 10_000,
-    // Poll while agent is working so we detect PR creation without manual refresh.
-    // 30s aligns with CLAUDE.md polling budget; event-driven invalidation via
-    // useSessionEvents handles the fast path (agent just created/updated a PR).
-    refetchInterval: sessionStatus === "working" ? 30_000 : false,
+    refetchInterval: (query) => {
+      // While agent is working: poll every 30s to detect PR creation/updates.
+      if (sessionStatus === "working") return 30_000;
+
+      // After agent stops: CI checks typically run 2-15min. Poll every 60s
+      // while CI is pending so the user sees updates without manual refresh.
+      const data = query.state.data as PRStatus | null | undefined;
+      if (data?.has_pr && data?.ci_status === "pending") return 60_000;
+
+      // No PR or CI resolved: stop polling. Window focus refetch handles edge cases.
+      return false;
+    },
     refetchOnWindowFocus: true,
   });
 }