diff --git a/PR_REVIEW_PLAN.md b/PR_REVIEW_PLAN.md new file mode 100644 index 000000000..bb1121a42 --- /dev/null +++ b/PR_REVIEW_PLAN.md @@ -0,0 +1,81 @@ +# PR Review Completion Plan + +Goal: make `okcode` PR Review feel comprehensive, reliable, and maintainer-grade on the existing `/_chat/pr-review` surface. + +## Phase 1 — Finish the current review cockpit + +### 1. Action rail polish + +- [ ] Tighten the action rail layout so decision, blockers, file impact, and recent maintainer reviews read cleanly at a glance. +- [ ] Make approval blockers explicit: conflicts, failing checks, pending checks, blocked workflow steps. +- [ ] Ensure copy is compact and maintainer-oriented. + +### 2. Review state clarity + +- [ ] Show the current PR review decision clearly and consistently. +- [ ] Confirm the latest submitted review state refreshes correctly after comment / approve / request changes. +- [ ] Verify draft review body reset and query invalidation behavior after submit. + +### 3. Review history quality + +- [ ] Improve recent maintainer reviews presentation for scanability. +- [ ] Distinguish maintainer review state from plain discussion/comment noise. +- [ ] Decide whether the compact summary is enough or needs a deeper expandable history view. + +## Phase 2 — Deepen review context + +### 4. File-level review signal + +- [ ] Surface clearer per-file review context: commented files, unresolved-thread files, reviewed files. +- [ ] Make it easier to see where maintainer attention is still needed. +- [ ] Verify selected-file behavior stays stable when data refreshes. + +### 5. Blockers and workflow visibility + +- [ ] Make mergeability, required checks, conflicts, and workflow blockers easier to understand from the page. +- [ ] Improve “why can’t I approve yet?” guidance. +- [ ] Ensure blocker state is visible without needing to hunt through inspector panels. + +### 6. Maintainer workflow integration + +- [ ] Confirm PR Review is fully discoverable through navigation and command palette. +- [ ] Identify any remaining maintainer entry points that should route into `/_chat/pr-review`. +- [ ] Avoid creating duplicate surfaces or split workflows. + +## Phase 3 — End-to-end reliability + +### 7. Validation and cleanup + +- [ ] Run full validation for the changed slice, including typecheck if possible. +- [ ] Fix any lint/type/test issues introduced by the PR Review work. +- [ ] Keep scope tight and avoid unrelated file churn. + +### 8. Real PR walkthrough + +- [ ] Test the full maintainer flow against a real PR. +- [ ] Verify open review, inspect files, inspect threads, view recent maintainer reviews, submit review, and refresh behavior. +- [ ] Capture any UX or state-sync gaps found in real usage. + +### 9. Final completeness pass + +- [ ] Review the page as a whole for maintainer usability. +- [ ] Confirm the implementation feels like the canonical PR Review workflow, not a partial bolt-on. +- [ ] Produce a final summary of what is complete vs what is intentionally deferred. + +## Execution order + +1. Action rail polish +2. Review state clarity +3. Review history quality +4. File-level review signal +5. Blockers and workflow visibility +6. Maintainer workflow integration +7. Validation and cleanup +8. Real PR walkthrough +9. Final completeness pass + +## Definition of done + +- A maintainer can open `/_chat/pr-review`, understand PR state quickly, inspect review context, see recent maintainer activity, submit a review confidently, and trust the page to stay in sync afterward. +- The page is clearly the main PR review workflow in `okcode`. +- Validation is clean enough that we trust the implementation, not just the visuals. diff --git a/apps/server/src/doctor.ts b/apps/server/src/doctor.ts index b10c6af6f..3326b506f 100644 --- a/apps/server/src/doctor.ts +++ b/apps/server/src/doctor.ts @@ -85,8 +85,9 @@ const doctorProgram = Effect.gen(function* () { console.log(""); console.log(" Codex: npm install -g @openai/codex && codex login"); console.log( - " Claude Code: npm install -g @anthropic-ai/claude-code && set ANTHROPIC_API_KEY or ANTHROPIC_AUTH_TOKEN", + " Claude Code: npm install -g @anthropic-ai/claude-code && claude auth login (or set ANTHROPIC_API_KEY / ANTHROPIC_AUTH_TOKEN)", ); + console.log(" Copilot: npm install -g @github/copilot && copilot login"); } else if (readyCount === statuses.length) { console.log("All providers are ready."); } else { diff --git a/apps/web/src/appSettings.ts b/apps/web/src/appSettings.ts index 6e7d53eb9..e52cf25cc 100644 --- a/apps/web/src/appSettings.ts +++ b/apps/web/src/appSettings.ts @@ -416,6 +416,22 @@ export function getProviderStartOptions( }, } : {}), + ...(settings.copilotBinaryPath || settings.copilotConfigDir + ? { + copilot: { + ...(settings.copilotBinaryPath ? { binaryPath: settings.copilotBinaryPath } : {}), + ...(settings.copilotConfigDir ? { configDir: settings.copilotConfigDir } : {}), + }, + } + : {}), + ...(settings.openclawGatewayUrl || settings.openclawPassword + ? { + openclaw: { + ...(settings.openclawGatewayUrl ? { gatewayUrl: settings.openclawGatewayUrl } : {}), + ...(settings.openclawPassword ? { password: settings.openclawPassword } : {}), + }, + } + : {}), }; return Object.keys(providerOptions).length > 0 ? providerOptions : undefined; diff --git a/apps/web/src/components/chat/ProviderSetupCard.tsx b/apps/web/src/components/chat/ProviderSetupCard.tsx index 811a0fa1e..62b4b19aa 100644 --- a/apps/web/src/components/chat/ProviderSetupCard.tsx +++ b/apps/web/src/components/chat/ProviderSetupCard.tsx @@ -37,6 +37,11 @@ const PROVIDER_CONFIG = { verifyCmd: "gh auth status", note: undefined, }, + copilot: { + installCmd: "npm install -g @github/copilot", + authCmd: "copilot login", + verifyCmd: "gh auth status", + }, } as const; function StatusIcon({ status }: { status: ServerProviderStatus["status"] }) { diff --git a/apps/web/src/components/pr-review/PrReviewShell.tsx b/apps/web/src/components/pr-review/PrReviewShell.tsx index 4ade2b2c2..3c67d2969 100644 --- a/apps/web/src/components/pr-review/PrReviewShell.tsx +++ b/apps/web/src/components/pr-review/PrReviewShell.tsx @@ -54,15 +54,8 @@ import { const BOOL_SCHEMA = Schema.Boolean; -function resolvePrReviewConfigPath(projectCwd: string, configPath: string): string { - if (/^(?:[A-Za-z]:[\\/]|\/)/.test(configPath)) { - return configPath; - } - return joinPath(projectCwd, configPath); -} - function formatReviewDecision(decision: string | null | undefined): string { - if (!decision) return "No decision"; + if (!decision) return "No decision yet"; return decision.toLowerCase().replaceAll("_", " "); } @@ -78,6 +71,20 @@ function reviewDecisionTone(decision: string | null | undefined): string { } } +function formatConflictStatus(status: string | null | undefined): string { + if (!status) return "Conflict status unknown"; + if (status === "clean") return "No merge conflicts"; + if (status === "conflicted") return "Merge conflicts"; + return status.replaceAll("_", " "); +} + +function resolvePrReviewConfigPath(projectCwd: string, configPath: string): string { + if (/^(?:[A-Za-z]:[\\/]|\/)/.test(configPath)) { + return configPath; + } + return joinPath(projectCwd, configPath); +} + function formatReviewTimestamp(value: string): string { const date = new Date(value); if (Number.isNaN(date.getTime())) return value; @@ -388,6 +395,28 @@ export function PrReviewShell({ const blockingWorkflowStepsComputed = (dashboardQuery.data?.workflowSteps ?? []).filter( (step) => step.status === "blocked" || step.status === "failed", ); + const fileStats = useMemo(() => { + const files = dashboardQuery.data?.files ?? []; + return files.reduce( + (totals, file) => ({ + changedFileCount: totals.changedFileCount + 1, + additions: totals.additions + file.additions, + deletions: totals.deletions + file.deletions, + }), + { changedFileCount: 0, additions: 0, deletions: 0 }, + ); + }, [dashboardQuery.data?.files]); + const approvalBlockers = [ + ...(conflictQuery.data?.status === "conflicted" ? ["Merge conflicts must be resolved"] : []), + ...checksSummary.failing.map((name) => `Failing check: ${name}`), + ...checksSummary.pending.map((name) => `Pending check: ${name}`), + ...blockingWorkflowStepsComputed.map((step) => `Workflow blocked: ${step.title}`), + ]; + const approveDisabled = + submitReviewMutation.isPending || + conflictQuery.data?.status === "conflicted" || + checksSummary.failing.length > 0 || + checksSummary.pending.length > 0; const recentReviews = dashboardQuery.data?.pullRequest.recentReviews ?? []; const displayedRecentReviews = recentReviews.slice(0, 3); @@ -521,26 +550,40 @@ export function PrReviewShell({ {/* Collapsed bar */}
-
+
Submit review + + {formatReviewDecision(dashboardQuery.data?.pullRequest.reviewDecision)} + {dashboardQuery.data?.pullRequest.unresolvedThreadCount ?? 0} open + {fileStats.changedFileCount} files - {conflictQuery.data?.status ?? "unknown"} + {formatConflictStatus(conflictQuery.data?.status)} - {blockingWorkflowStepsComputed.length > 0 ? ( + {approvalBlockers.length > 0 ? ( - blocked + {approvalBlockers.length} blocker{approvalBlockers.length === 1 ? "" : "s"} + + ) : ( + + + ready to approve - ) : null} + )}
- - +
+
+ {approveDisabled + ? "Approval is gated until blockers are cleared." + : "Approval is available once your summary is ready."} +
+
+ + + +