mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(external-pr): surface in-flight reviews in the panel, not just completed
The PR-review queue only listed COMPLETED reviews and hid when empty, so while a review was in_progress the panel showed nothing — no sign a review was happening or where its findings go (the reviewer posts its change-request on the PR itself). Add TaskService.list_external_pr_reviews (active reviews + awaiting-decision, minus cancelled/decided/dismissed); the route uses it. The panel card now shows active reviews with a 'Reviewing' badge and a link to the PR where the change-request lands, and the Supersede/Dismiss actions only once the review completes.
This commit is contained in:
@@ -21,7 +21,7 @@ import {
|
|||||||
DialogHeader,
|
DialogHeader,
|
||||||
DialogTitle,
|
DialogTitle,
|
||||||
} from "@/components/ui/dialog";
|
} from "@/components/ui/dialog";
|
||||||
import { GitPullRequest, ExternalLink, Rocket, XCircle, FileText } from "lucide-react";
|
import { GitPullRequest, ExternalLink, Rocket, XCircle, FileText, Loader2 } from "lucide-react";
|
||||||
import Link from "next/link";
|
import Link from "next/link";
|
||||||
import { type Task } from "@/types";
|
import { type Task } from "@/types";
|
||||||
import { toast } from "sonner";
|
import { toast } from "sonner";
|
||||||
@@ -133,47 +133,66 @@ export function PrReviewQueue({ className }: PrReviewQueueProps) {
|
|||||||
</Badge>
|
</Badge>
|
||||||
</CardTitle>
|
</CardTitle>
|
||||||
<CardDescription>
|
<CardDescription>
|
||||||
External PRs the org reviewed — supersede or dismiss
|
External PRs the org is reviewing or has reviewed — the reviewer
|
||||||
|
posts its change-request on the PR; supersede or dismiss once it
|
||||||
|
lands
|
||||||
</CardDescription>
|
</CardDescription>
|
||||||
</CardHeader>
|
</CardHeader>
|
||||||
<CardContent>
|
<CardContent>
|
||||||
<div className="space-y-3">
|
<div className="space-y-3">
|
||||||
{items.map((task) => (
|
{items.map((task) => {
|
||||||
|
const awaiting = (task.status || "").toLowerCase() === "completed";
|
||||||
|
return (
|
||||||
<div
|
<div
|
||||||
key={task.id}
|
key={task.id}
|
||||||
className="flex items-start justify-between p-4 border rounded-lg hover:bg-muted/50 transition-colors"
|
className="flex items-start justify-between p-4 border rounded-lg hover:bg-muted/50 transition-colors"
|
||||||
>
|
>
|
||||||
<div className="flex-1 min-w-0">
|
<div className="flex-1 min-w-0">
|
||||||
|
<div className="flex items-center gap-2">
|
||||||
<Link
|
<Link
|
||||||
href={`/tasks/${task.id}`}
|
href={`/tasks/${task.id}`}
|
||||||
className="font-medium hover:underline line-clamp-1"
|
className="font-medium hover:underline line-clamp-1"
|
||||||
>
|
>
|
||||||
{task.title}
|
{task.title}
|
||||||
</Link>
|
</Link>
|
||||||
|
{awaiting ? (
|
||||||
|
<Badge variant="secondary" className="shrink-0">
|
||||||
|
Awaiting your call
|
||||||
|
</Badge>
|
||||||
|
) : (
|
||||||
|
<Badge variant="outline" className="shrink-0 gap-1">
|
||||||
|
<Loader2 className="h-3 w-3 animate-spin" />
|
||||||
|
Reviewing
|
||||||
|
</Badge>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
{task.description && (
|
{task.description && (
|
||||||
<p className="text-sm text-muted-foreground mt-1 line-clamp-2">
|
<p className="text-sm text-muted-foreground mt-1 line-clamp-2">
|
||||||
{task.description}
|
{task.description}
|
||||||
</p>
|
</p>
|
||||||
)}
|
)}
|
||||||
</div>
|
|
||||||
<div className="flex items-center gap-2 ml-4 flex-shrink-0">
|
|
||||||
{task.pr_url && (
|
{task.pr_url && (
|
||||||
<a
|
<a
|
||||||
href={task.pr_url}
|
href={task.pr_url}
|
||||||
target="_blank"
|
target="_blank"
|
||||||
rel="noopener noreferrer"
|
rel="noopener noreferrer"
|
||||||
title="View PR on GitHub"
|
className="mt-1 inline-flex items-center gap-1 text-xs text-primary hover:underline"
|
||||||
>
|
>
|
||||||
<Button variant="ghost" size="sm">
|
{awaiting
|
||||||
<ExternalLink className="h-4 w-4" />
|
? "Change-request posted on the PR"
|
||||||
</Button>
|
: "Findings will post as a change-request on the PR"}
|
||||||
|
<ExternalLink className="h-3 w-3" />
|
||||||
</a>
|
</a>
|
||||||
)}
|
)}
|
||||||
|
</div>
|
||||||
|
<div className="flex items-center gap-2 ml-4 flex-shrink-0">
|
||||||
<Link href={`/tasks/${task.id}`} title="Review details">
|
<Link href={`/tasks/${task.id}`} title="Review details">
|
||||||
<Button variant="ghost" size="sm">
|
<Button variant="ghost" size="sm">
|
||||||
<FileText className="h-4 w-4" />
|
<FileText className="h-4 w-4" />
|
||||||
</Button>
|
</Button>
|
||||||
</Link>
|
</Link>
|
||||||
|
{awaiting && (
|
||||||
|
<>
|
||||||
<Button
|
<Button
|
||||||
variant="outline"
|
variant="outline"
|
||||||
size="sm"
|
size="sm"
|
||||||
@@ -191,9 +210,12 @@ export function PrReviewQueue({ className }: PrReviewQueueProps) {
|
|||||||
<Rocket className="h-4 w-4 mr-1" />
|
<Rocket className="h-4 w-4 mr-1" />
|
||||||
Supersede
|
Supersede
|
||||||
</Button>
|
</Button>
|
||||||
|
</>
|
||||||
|
)}
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
))}
|
);
|
||||||
|
})}
|
||||||
</div>
|
</div>
|
||||||
</CardContent>
|
</CardContent>
|
||||||
</Card>
|
</Card>
|
||||||
|
|||||||
@@ -647,10 +647,13 @@ async def get_external_pr_reviews(
|
|||||||
agent: CurrentAgentContext,
|
agent: CurrentAgentContext,
|
||||||
permissions: PermissionServiceDep,
|
permissions: PermissionServiceDep,
|
||||||
) -> list[TaskResponse]:
|
) -> list[TaskResponse]:
|
||||||
"""Inbound external PRs that were reviewed and await the CEO's decision.
|
"""Inbound external PRs the org is reviewing or has reviewed.
|
||||||
|
|
||||||
The PR-review decision queue: completed external-PR review tasks the CEO has
|
The PR-review queue: external-PR review tasks still in flight (the reviewer
|
||||||
neither superseded nor dismissed. Org-wide; visible to PMs and above.
|
is working) OR completed and awaiting the CEO's decision (not yet superseded
|
||||||
|
or dismissed). Active reviews surface so the panel shows a review underway
|
||||||
|
and links to the PR where the change-request is posted, instead of going
|
||||||
|
dark until it finishes. Org-wide; visible to PMs and above.
|
||||||
"""
|
"""
|
||||||
can_view_all = permissions.can_perform_task_action(agent, TaskAction.VIEW_ALL)
|
can_view_all = permissions.can_perform_task_action(agent, TaskAction.VIEW_ALL)
|
||||||
is_pm = agent.role in (AgentRole.CELL_PM, AgentRole.MAIN_PM)
|
is_pm = agent.role in (AgentRole.CELL_PM, AgentRole.MAIN_PM)
|
||||||
@@ -661,7 +664,7 @@ async def get_external_pr_reviews(
|
|||||||
detail="Only PMs and management can view the PR-review queue",
|
detail="Only PMs and management can view the PR-review queue",
|
||||||
)
|
)
|
||||||
service = get_task_service(db)
|
service = get_task_service(db)
|
||||||
tasks = await service.list_external_pr_reviews_awaiting_decision()
|
tasks = await service.list_external_pr_reviews()
|
||||||
return task_list_to_response(tasks)
|
return task_list_to_response(tasks)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -741,6 +741,37 @@ class TaskService(BaseService):
|
|||||||
if "dismissed=1" not in (t.quick_context or "").split()
|
if "dismissed=1" not in (t.quick_context or "").split()
|
||||||
]
|
]
|
||||||
|
|
||||||
|
async def list_external_pr_reviews(self) -> list[TaskTable]:
|
||||||
|
"""Live external-PR reviews for the panel: in-flight PLUS awaiting-decision.
|
||||||
|
|
||||||
|
Every ``source='external_pr'`` task that is not finished-and-decided:
|
||||||
|
active reviews (the reviewer is still working — pending / claimed /
|
||||||
|
in_progress / verifying / blocked / paused) AND completed reviews the CEO
|
||||||
|
has neither superseded (``confirmed_by_human=True``) nor dismissed
|
||||||
|
(``dismissed=1`` in quick_context). Cancelled tasks are excluded.
|
||||||
|
|
||||||
|
Active reviews are surfaced on purpose: the reviewer posts its
|
||||||
|
change-request to the PR itself, so the panel must show that a review is
|
||||||
|
underway and link to that PR rather than going dark until the review
|
||||||
|
finishes. Each task carries its ``status`` so the panel can tell
|
||||||
|
"reviewing" apart from "awaiting your decision".
|
||||||
|
"""
|
||||||
|
result = await self.session.execute(
|
||||||
|
select(TaskTable).where(
|
||||||
|
TaskTable.source == "external_pr",
|
||||||
|
TaskTable.status != TaskStatus.CANCELLED,
|
||||||
|
or_(
|
||||||
|
TaskTable.status != TaskStatus.COMPLETED,
|
||||||
|
TaskTable.confirmed_by_human.is_(False),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
return [
|
||||||
|
t
|
||||||
|
for t in result.scalars().all()
|
||||||
|
if "dismissed=1" not in (t.quick_context or "").split()
|
||||||
|
]
|
||||||
|
|
||||||
async def dismiss_external_pr_review(self, task_id: UUID) -> TaskTable | None:
|
async def dismiss_external_pr_review(self, task_id: UUID) -> TaskTable | None:
|
||||||
"""CEO declines to act on a reviewed external PR — drop it from the queue.
|
"""CEO declines to act on a reviewed external PR — drop it from the queue.
|
||||||
|
|
||||||
|
|||||||
@@ -76,6 +76,17 @@ async def test_list_awaiting_decision_excludes_dismissed() -> None:
|
|||||||
assert out == [pending]
|
assert out == [pending]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_list_external_pr_reviews_excludes_dismissed() -> None:
|
||||||
|
# The panel queue surfaces in-flight reviews too (the status filter lives in
|
||||||
|
# SQL); here we pin the post-query behavior: dismissed reviews drop out.
|
||||||
|
reviewing = MagicMock(quick_context="external_pr_head=abc")
|
||||||
|
dismissed = MagicMock(quick_context="external_pr_head=def dismissed=1")
|
||||||
|
svc = _service([reviewing, dismissed])
|
||||||
|
out = await svc.list_external_pr_reviews()
|
||||||
|
assert out == [reviewing]
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_dismiss_marks_and_is_idempotent() -> None:
|
async def test_dismiss_marks_and_is_idempotent() -> None:
|
||||||
task = MagicMock(source="external_pr", quick_context="external_pr_head=abc")
|
task = MagicMock(source="external_pr", quick_context="external_pr_head=abc")
|
||||||
|
|||||||
Reference in New Issue
Block a user