Skip to content

Commit 8ce9bc1

Browse files
CopilotjasnsyBunsDevCopilot
authored
Resolve PR 432 merge conflicts against current main (#434)
* Add GitHub Copilot provider support * Address Copilot PR review feedback * Address follow-up Copilot review feedback * Resolve rebase conflicts on main * Update apps/server/src/provider/Layers/CopilotAdapter.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update apps/server/src/provider/Layers/CopilotAdapter.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Add PR review plan and improve action rail Add PR_REVIEW_PLAN.md describing a phased plan to finish the PR review cockpit. Update PrReviewShell.tsx to enhance the action rail and expanded review panel: add formatting helpers for review decisions and conflict status, compute file stats, surface approval blockers and gating logic, show review decision with tone, file impact summary, and blocker/count UI, and restructure the expanded area into three info cards (Review decision, File impact, Approval status). Also adjust button layout and disable logic for Approve and minor spacing/styling tweaks. --------- Co-authored-by: Jason <jasnsy@gmail.com> Co-authored-by: Val Alexander <68980965+BunsDev@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Val Alexander <bunsthedev@gmail.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 4b94f89 commit 8ce9bc1

5 files changed

Lines changed: 237 additions & 57 deletions

File tree

‎PR_REVIEW_PLAN.md‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# PR Review Completion Plan
2+
3+
Goal: make `okcode` PR Review feel comprehensive, reliable, and maintainer-grade on the existing `/_chat/pr-review` surface.
4+
5+
## Phase 1 — Finish the current review cockpit
6+
7+
### 1. Action rail polish
8+
9+
- [ ] Tighten the action rail layout so decision, blockers, file impact, and recent maintainer reviews read cleanly at a glance.
10+
- [ ] Make approval blockers explicit: conflicts, failing checks, pending checks, blocked workflow steps.
11+
- [ ] Ensure copy is compact and maintainer-oriented.
12+
13+
### 2. Review state clarity
14+
15+
- [ ] Show the current PR review decision clearly and consistently.
16+
- [ ] Confirm the latest submitted review state refreshes correctly after comment / approve / request changes.
17+
- [ ] Verify draft review body reset and query invalidation behavior after submit.
18+
19+
### 3. Review history quality
20+
21+
- [ ] Improve recent maintainer reviews presentation for scanability.
22+
- [ ] Distinguish maintainer review state from plain discussion/comment noise.
23+
- [ ] Decide whether the compact summary is enough or needs a deeper expandable history view.
24+
25+
## Phase 2 — Deepen review context
26+
27+
### 4. File-level review signal
28+
29+
- [ ] Surface clearer per-file review context: commented files, unresolved-thread files, reviewed files.
30+
- [ ] Make it easier to see where maintainer attention is still needed.
31+
- [ ] Verify selected-file behavior stays stable when data refreshes.
32+
33+
### 5. Blockers and workflow visibility
34+
35+
- [ ] Make mergeability, required checks, conflicts, and workflow blockers easier to understand from the page.
36+
- [ ] Improve “why can’t I approve yet?” guidance.
37+
- [ ] Ensure blocker state is visible without needing to hunt through inspector panels.
38+
39+
### 6. Maintainer workflow integration
40+
41+
- [ ] Confirm PR Review is fully discoverable through navigation and command palette.
42+
- [ ] Identify any remaining maintainer entry points that should route into `/_chat/pr-review`.
43+
- [ ] Avoid creating duplicate surfaces or split workflows.
44+
45+
## Phase 3 — End-to-end reliability
46+
47+
### 7. Validation and cleanup
48+
49+
- [ ] Run full validation for the changed slice, including typecheck if possible.
50+
- [ ] Fix any lint/type/test issues introduced by the PR Review work.
51+
- [ ] Keep scope tight and avoid unrelated file churn.
52+
53+
### 8. Real PR walkthrough
54+
55+
- [ ] Test the full maintainer flow against a real PR.
56+
- [ ] Verify open review, inspect files, inspect threads, view recent maintainer reviews, submit review, and refresh behavior.
57+
- [ ] Capture any UX or state-sync gaps found in real usage.
58+
59+
### 9. Final completeness pass
60+
61+
- [ ] Review the page as a whole for maintainer usability.
62+
- [ ] Confirm the implementation feels like the canonical PR Review workflow, not a partial bolt-on.
63+
- [ ] Produce a final summary of what is complete vs what is intentionally deferred.
64+
65+
## Execution order
66+
67+
1. Action rail polish
68+
2. Review state clarity
69+
3. Review history quality
70+
4. File-level review signal
71+
5. Blockers and workflow visibility
72+
6. Maintainer workflow integration
73+
7. Validation and cleanup
74+
8. Real PR walkthrough
75+
9. Final completeness pass
76+
77+
## Definition of done
78+
79+
- 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.
80+
- The page is clearly the main PR review workflow in `okcode`.
81+
- Validation is clean enough that we trust the implementation, not just the visuals.

‎apps/server/src/doctor.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,9 @@ const doctorProgram = Effect.gen(function* () {
8585
console.log("");
8686
console.log(" Codex: npm install -g @openai/codex && codex login");
8787
console.log(
88-
" Claude Code: npm install -g @anthropic-ai/claude-code && set ANTHROPIC_API_KEY or ANTHROPIC_AUTH_TOKEN",
88+
" Claude Code: npm install -g @anthropic-ai/claude-code && claude auth login (or set ANTHROPIC_API_KEY / ANTHROPIC_AUTH_TOKEN)",
8989
);
90+
console.log(" Copilot: npm install -g @github/copilot && copilot login");
9091
} else if (readyCount === statuses.length) {
9192
console.log("All providers are ready.");
9293
} else {

‎apps/web/src/appSettings.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,22 @@ export function getProviderStartOptions(
416416
},
417417
}
418418
: {}),
419+
...(settings.copilotBinaryPath || settings.copilotConfigDir
420+
? {
421+
copilot: {
422+
...(settings.copilotBinaryPath ? { binaryPath: settings.copilotBinaryPath } : {}),
423+
...(settings.copilotConfigDir ? { configDir: settings.copilotConfigDir } : {}),
424+
},
425+
}
426+
: {}),
427+
...(settings.openclawGatewayUrl || settings.openclawPassword
428+
? {
429+
openclaw: {
430+
...(settings.openclawGatewayUrl ? { gatewayUrl: settings.openclawGatewayUrl } : {}),
431+
...(settings.openclawPassword ? { password: settings.openclawPassword } : {}),
432+
},
433+
}
434+
: {}),
419435
};
420436

421437
return Object.keys(providerOptions).length > 0 ? providerOptions : undefined;

‎apps/web/src/components/chat/ProviderSetupCard.tsx‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ const PROVIDER_CONFIG = {
3737
verifyCmd: "gh auth status",
3838
note: undefined,
3939
},
40+
copilot: {
41+
installCmd: "npm install -g @github/copilot",
42+
authCmd: "copilot login",
43+
verifyCmd: "gh auth status",
44+
},
4045
} as const;
4146

4247
function StatusIcon({ status }: { status: ServerProviderStatus["status"] }) {

‎apps/web/src/components/pr-review/PrReviewShell.tsx‎

Lines changed: 133 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,8 @@ import {
5454

5555
const BOOL_SCHEMA = Schema.Boolean;
5656

57-
function resolvePrReviewConfigPath(projectCwd: string, configPath: string): string {
58-
if (/^(?:[A-Za-z]:[\\/]|\/)/.test(configPath)) {
59-
return configPath;
60-
}
61-
return joinPath(projectCwd, configPath);
62-
}
63-
6457
function formatReviewDecision(decision: string | null | undefined): string {
65-
if (!decision) return "No decision";
58+
if (!decision) return "No decision yet";
6659
return decision.toLowerCase().replaceAll("_", " ");
6760
}
6861

@@ -78,6 +71,20 @@ function reviewDecisionTone(decision: string | null | undefined): string {
7871
}
7972
}
8073

74+
function formatConflictStatus(status: string | null | undefined): string {
75+
if (!status) return "Conflict status unknown";
76+
if (status === "clean") return "No merge conflicts";
77+
if (status === "conflicted") return "Merge conflicts";
78+
return status.replaceAll("_", " ");
79+
}
80+
81+
function resolvePrReviewConfigPath(projectCwd: string, configPath: string): string {
82+
if (/^(?:[A-Za-z]:[\\/]|\/)/.test(configPath)) {
83+
return configPath;
84+
}
85+
return joinPath(projectCwd, configPath);
86+
}
87+
8188
function formatReviewTimestamp(value: string): string {
8289
const date = new Date(value);
8390
if (Number.isNaN(date.getTime())) return value;
@@ -388,6 +395,28 @@ export function PrReviewShell({
388395
const blockingWorkflowStepsComputed = (dashboardQuery.data?.workflowSteps ?? []).filter(
389396
(step) => step.status === "blocked" || step.status === "failed",
390397
);
398+
const fileStats = useMemo(() => {
399+
const files = dashboardQuery.data?.files ?? [];
400+
return files.reduce(
401+
(totals, file) => ({
402+
changedFileCount: totals.changedFileCount + 1,
403+
additions: totals.additions + file.additions,
404+
deletions: totals.deletions + file.deletions,
405+
}),
406+
{ changedFileCount: 0, additions: 0, deletions: 0 },
407+
);
408+
}, [dashboardQuery.data?.files]);
409+
const approvalBlockers = [
410+
...(conflictQuery.data?.status === "conflicted" ? ["Merge conflicts must be resolved"] : []),
411+
...checksSummary.failing.map((name) => `Failing check: ${name}`),
412+
...checksSummary.pending.map((name) => `Pending check: ${name}`),
413+
...blockingWorkflowStepsComputed.map((step) => `Workflow blocked: ${step.title}`),
414+
];
415+
const approveDisabled =
416+
submitReviewMutation.isPending ||
417+
conflictQuery.data?.status === "conflicted" ||
418+
checksSummary.failing.length > 0 ||
419+
checksSummary.pending.length > 0;
391420
const recentReviews = dashboardQuery.data?.pullRequest.recentReviews ?? [];
392421
const displayedRecentReviews = recentReviews.slice(0, 3);
393422

@@ -521,26 +550,40 @@ export function PrReviewShell({
521550
{/* Collapsed bar */}
522551
<div
523552
className={cn(
524-
"flex h-10 items-center justify-between gap-3 px-4",
553+
"flex min-h-10 items-center justify-between gap-3 px-4 py-2",
525554
actionRailExpanded && "border-b border-border/50",
526555
)}
527556
>
528-
<div className="flex items-center gap-3 text-xs text-muted-foreground">
557+
<div className="flex flex-wrap items-center gap-x-3 gap-y-1 text-xs text-muted-foreground">
529558
<span className="font-medium text-foreground">Submit review</span>
559+
<span
560+
className={cn(
561+
"capitalize font-medium",
562+
reviewDecisionTone(dashboardQuery.data?.pullRequest.reviewDecision),
563+
)}
564+
>
565+
{formatReviewDecision(dashboardQuery.data?.pullRequest.reviewDecision)}
566+
</span>
530567
<span className="flex items-center gap-1">
531568
<MessageSquareIcon className="size-3" />
532569
{dashboardQuery.data?.pullRequest.unresolvedThreadCount ?? 0} open
533570
</span>
571+
<span>{fileStats.changedFileCount} files</span>
534572
<span className="flex items-center gap-1">
535573
<ShieldCheckIcon className="size-3" />
536-
{conflictQuery.data?.status ?? "unknown"}
574+
{formatConflictStatus(conflictQuery.data?.status)}
537575
</span>
538-
{blockingWorkflowStepsComputed.length > 0 ? (
576+
{approvalBlockers.length > 0 ? (
539577
<span className="flex items-center gap-1 text-amber-600 dark:text-amber-400">
540578
<SparklesIcon className="size-3" />
541-
blocked
579+
{approvalBlockers.length} blocker{approvalBlockers.length === 1 ? "" : "s"}
580+
</span>
581+
) : (
582+
<span className="flex items-center gap-1 text-emerald-600 dark:text-emerald-400">
583+
<CheckCircle2Icon className="size-3" />
584+
ready to approve
542585
</span>
543-
) : null}
586+
)}
544587
</div>
545588
<Button
546589
onClick={() => setActionRailExpanded(!actionRailExpanded)}
@@ -562,20 +605,50 @@ export function PrReviewShell({
562605
>
563606
<div className="overflow-hidden">
564607
<div className="space-y-3 px-4 py-3">
565-
<div className="flex flex-wrap items-start gap-x-4 gap-y-2 rounded-xl border border-border/60 bg-muted/30 px-3 py-2.5 text-xs">
566-
<div className="space-y-0.5">
608+
<div className="grid gap-2 lg:grid-cols-[minmax(0,1fr)_minmax(0,1fr)_minmax(0,1.3fr)]">
609+
<div className="rounded-xl border border-border/60 bg-muted/30 px-3 py-2.5 text-xs">
567610
<div className="text-[11px] uppercase tracking-wide text-muted-foreground">
568611
Review decision
569612
</div>
570613
<div
571614
className={cn(
572-
"font-medium capitalize",
615+
"mt-1 font-medium capitalize",
573616
reviewDecisionTone(dashboardQuery.data?.pullRequest.reviewDecision),
574617
)}
575618
>
576619
{formatReviewDecision(dashboardQuery.data?.pullRequest.reviewDecision)}
577620
</div>
578621
</div>
622+
<div className="rounded-xl border border-border/60 bg-muted/30 px-3 py-2.5 text-xs">
623+
<div className="text-[11px] uppercase tracking-wide text-muted-foreground">
624+
File impact
625+
</div>
626+
<div className="mt-1 font-medium text-foreground">
627+
{fileStats.changedFileCount}{" "}
628+
{fileStats.changedFileCount === 1 ? "file" : "files"}, +{fileStats.additions} /
629+
-{fileStats.deletions}
630+
</div>
631+
</div>
632+
<div className="rounded-xl border border-border/60 bg-muted/30 px-3 py-2.5 text-xs">
633+
<div className="text-[11px] uppercase tracking-wide text-muted-foreground">
634+
Approval status
635+
</div>
636+
{approvalBlockers.length > 0 ? (
637+
<ul className="mt-1 space-y-1 text-muted-foreground">
638+
{approvalBlockers.slice(0, 4).map((blocker) => (
639+
<li className="flex items-start gap-1.5" key={blocker}>
640+
<AlertTriangleIcon className="mt-0.5 size-3 shrink-0 text-amber-500" />
641+
<span>{blocker}</span>
642+
</li>
643+
))}
644+
</ul>
645+
) : (
646+
<div className="mt-1 flex items-center gap-1.5 font-medium text-emerald-600 dark:text-emerald-400">
647+
<CheckCircle2Icon className="size-3.5" />
648+
Ready to approve
649+
</div>
650+
)}
651+
</div>
579652
</div>
580653
<div className="space-y-1.5">
581654
<div className="text-[11px] uppercase tracking-wide text-muted-foreground">
@@ -626,45 +699,49 @@ export function PrReviewShell({
626699
}
627700
}}
628701
/>
629-
<div className="flex flex-wrap items-center justify-end gap-2">
630-
<Button
631-
disabled={submitReviewMutation.isPending}
632-
onClick={() => {
633-
void submitReviewMutation.mutateAsync("COMMENT");
634-
}}
635-
size="sm"
636-
variant="outline"
637-
>
638-
<MessageSquareIcon className="size-3.5" />
639-
Comment
640-
</Button>
641-
<Button
642-
disabled={
643-
submitReviewMutation.isPending ||
644-
conflictQuery.data?.status === "conflicted" ||
645-
checksSummary.failing.length > 0 ||
646-
checksSummary.pending.length > 0
647-
}
648-
onClick={() => {
649-
void submitReviewMutation.mutateAsync("APPROVE");
650-
}}
651-
size="sm"
652-
variant="secondary"
653-
>
654-
<CheckCircle2Icon className="size-3.5" />
655-
Approve
656-
</Button>
657-
<Button
658-
disabled={submitReviewMutation.isPending}
659-
onClick={() => {
660-
void submitReviewMutation.mutateAsync("REQUEST_CHANGES");
661-
}}
662-
size="sm"
663-
variant={resolveRequestChangesButtonVariant(settings.prReviewRequestChangesTone)}
664-
>
665-
<AlertTriangleIcon className="size-3.5" />
666-
Request changes
667-
</Button>
702+
<div className="flex flex-wrap items-center justify-between gap-2">
703+
<div className="text-xs text-muted-foreground">
704+
{approveDisabled
705+
? "Approval is gated until blockers are cleared."
706+
: "Approval is available once your summary is ready."}
707+
</div>
708+
<div className="flex flex-wrap items-center justify-end gap-2">
709+
<Button
710+
disabled={submitReviewMutation.isPending}
711+
onClick={() => {
712+
void submitReviewMutation.mutateAsync("COMMENT");
713+
}}
714+
size="sm"
715+
variant="outline"
716+
>
717+
<MessageSquareIcon className="size-3.5" />
718+
Comment
719+
</Button>
720+
<Button
721+
disabled={approveDisabled}
722+
onClick={() => {
723+
void submitReviewMutation.mutateAsync("APPROVE");
724+
}}
725+
size="sm"
726+
variant="secondary"
727+
>
728+
<CheckCircle2Icon className="size-3.5" />
729+
Approve
730+
</Button>
731+
<Button
732+
disabled={submitReviewMutation.isPending}
733+
onClick={() => {
734+
void submitReviewMutation.mutateAsync("REQUEST_CHANGES");
735+
}}
736+
size="sm"
737+
variant={resolveRequestChangesButtonVariant(
738+
settings.prReviewRequestChangesTone,
739+
)}
740+
>
741+
<AlertTriangleIcon className="size-3.5" />
742+
Request changes
743+
</Button>
744+
</div>
668745
</div>
669746
</div>
670747
</div>

0 commit comments

Comments
 (0)