Conversation
- Add amber info banner in BlockerResolutionView when open blockers exist:
"Blockers pause agent execution... Answer below, then go to Tasks to restart."
- Update BlockerCard task context to link to /tasks ("Raised by Task <id>")
- Update submitted confirmation to "Answer recorded. Go to Tasks and restart
execution to resume the agent." with a 'Go to Tasks' button
WalkthroughEnhanced blocker component UI with improved messaging and navigation. Added contextual information about execution pause, updated confirmation messages after answering blockers, and introduced links to navigate to the tasks page for resuming execution. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code ReviewThis is a clean, focused UX improvement. Well-scoped, idiomatic for the Next.js/Tailwind/shadcn stack. What is Working Well
Issue: Link Text vs. Navigation DestinationIn BlockerCard.tsx the link reads "Raised by Task {blocker.task_id}" but navigates to /tasks (the full unfiltered list). The text implies you land on that specific task, but you do not. Options:
The post-answer "Go to Tasks" button is fine as-is — label matches behavior. Minor: Route String Duplication"/tasks" appears twice in BlockerCard.tsx. A shared route constant (e.g. ROUTES.tasks) would prevent silent breakage if the route ever changes. Minor: Missing data-testid on New ElementsBlockerCard already has data-testid="blocker-card". The new banner in BlockerResolutionView and the "Go to Tasks" button lack test IDs — worth adding if automated coverage of these elements is planned. Overall: ready to merge once the "Raised by Task" link destination is clarified or corrected. Everything else is polish-level. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web-ui/src/components/blockers/BlockerCard.tsx`:
- Around line 58-67: The success state in BlockerCard.tsx is being cleared by an
automatic call to onAnswered() after 1500ms, which makes the "Go to Tasks" CTA
disappear too quickly; change the behavior so the success state persists until
user action by removing or significantly extending the auto-timer and instead
invoking onAnswered() when the user clicks the "Go to Tasks" Button/Link (or
when they explicitly dismiss), ensuring the Button's click handler triggers
onAnswered() and then navigates to /tasks so the recovery CTA remains reliable.
In `@web-ui/src/components/blockers/BlockerResolutionView.tsx`:
- Around line 85-93: The info banner in BlockerResolutionView.tsx (rendered when
openBlockers.length > 0) uses amber palette Tailwind classes; update the banner
div to use the Nova/Shadcn gray-only tokens instead (replace classes like
border-amber-200 bg-amber-50 text-amber-900 text-amber-800 dark:border-amber-800
dark:bg-amber-950/30 dark:text-amber-300 with the equivalent gray variants from
the Nova preset and ensure proper dark-mode tokens), and if any icon was added
here or nearby, swap it to a Hugeicons import (per repository guideline) rather
than lucide-react; keep the existing conditional render and className structure
on the same element so behavior (openBlockers.length > 0) is unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c41bd6f6-0e99-4e66-8926-01ac6f811a00
📒 Files selected for processing (2)
web-ui/src/components/blockers/BlockerCard.tsxweb-ui/src/components/blockers/BlockerResolutionView.tsx
| <CardContent className="flex items-center justify-between gap-4 p-4"> | ||
| <div className="flex items-center gap-2"> | ||
| <CheckmarkCircle01Icon className="h-5 w-5 shrink-0 text-green-600 dark:text-green-400" /> | ||
| <p className="text-sm font-medium text-green-800 dark:text-green-300"> | ||
| Answer recorded. Go to Tasks and restart execution to resume the agent. | ||
| </p> | ||
| </div> | ||
| <Button size="sm" variant="outline" asChild> | ||
| <Link href="/tasks">Go to Tasks</Link> | ||
| </Button> |
There was a problem hiding this comment.
Success recovery CTA is too short-lived to be reliable.
Because onAnswered() is auto-called after 1500ms (Line 43), this success state (and its “Go to Tasks” button) can disappear before users act, which weakens the new recovery guidance.
Suggested change
-import { useState, useEffect, useRef } from 'react';
+import { useState } from 'react';
@@
- const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
-
- // Clean up timeout on unmount
- useEffect(() => {
- return () => {
- if (timerRef.current) clearTimeout(timerRef.current);
- };
- }, []);
@@
await blockersApi.answer(workspacePath, blocker.id, answer.trim());
setSubmitted(true);
- timerRef.current = setTimeout(() => onAnswered(), 1500);
@@
- <Button size="sm" variant="outline" asChild>
- <Link href="/tasks">Go to Tasks</Link>
+ <Button size="sm" variant="outline" asChild>
+ <Link href="/tasks" onClick={onAnswered}>Go to Tasks</Link>
</Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <CardContent className="flex items-center justify-between gap-4 p-4"> | |
| <div className="flex items-center gap-2"> | |
| <CheckmarkCircle01Icon className="h-5 w-5 shrink-0 text-green-600 dark:text-green-400" /> | |
| <p className="text-sm font-medium text-green-800 dark:text-green-300"> | |
| Answer recorded. Go to Tasks and restart execution to resume the agent. | |
| </p> | |
| </div> | |
| <Button size="sm" variant="outline" asChild> | |
| <Link href="/tasks">Go to Tasks</Link> | |
| </Button> | |
| <CardContent className="flex items-center justify-between gap-4 p-4"> | |
| <div className="flex items-center gap-2"> | |
| <CheckmarkCircle01Icon className="h-5 w-5 shrink-0 text-green-600 dark:text-green-400" /> | |
| <p className="text-sm font-medium text-green-800 dark:text-green-300"> | |
| Answer recorded. Go to Tasks and restart execution to resume the agent. | |
| </p> | |
| </div> | |
| <Button size="sm" variant="outline" asChild> | |
| <Link href="/tasks" onClick={onAnswered}>Go to Tasks</Link> | |
| </Button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web-ui/src/components/blockers/BlockerCard.tsx` around lines 58 - 67, The
success state in BlockerCard.tsx is being cleared by an automatic call to
onAnswered() after 1500ms, which makes the "Go to Tasks" CTA disappear too
quickly; change the behavior so the success state persists until user action by
removing or significantly extending the auto-timer and instead invoking
onAnswered() when the user clicks the "Go to Tasks" Button/Link (or when they
explicitly dismiss), ensuring the Button's click handler triggers onAnswered()
and then navigates to /tasks so the recovery CTA remains reliable.
| {/* Info banner — shown only when there are open blockers */} | ||
| {openBlockers.length > 0 && ( | ||
| <div className="rounded-lg border border-amber-200 bg-amber-50 px-4 py-3 text-sm dark:border-amber-800 dark:bg-amber-950/30"> | ||
| <p className="font-medium text-amber-900 dark:text-amber-200">AI execution is paused.</p> | ||
| <p className="mt-0.5 text-amber-800 dark:text-amber-300"> | ||
| Blockers pause agent execution until you provide the requested information. Answer the question below, then go to Tasks to restart execution. | ||
| </p> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Banner color tokens violate the enforced gray-only UI scheme.
This new banner uses amber palette classes, which conflicts with the repository UI guideline for web-ui/src.
Suggested change
- {openBlockers.length > 0 && (
- <div className="rounded-lg border border-amber-200 bg-amber-50 px-4 py-3 text-sm dark:border-amber-800 dark:bg-amber-950/30">
- <p className="font-medium text-amber-900 dark:text-amber-200">AI execution is paused.</p>
- <p className="mt-0.5 text-amber-800 dark:text-amber-300">
+ {openBlockers.length > 0 && (
+ <div className="rounded-lg border border-border bg-muted/50 px-4 py-3 text-sm">
+ <p className="font-medium text-foreground">AI execution is paused.</p>
+ <p className="mt-0.5 text-muted-foreground">
Blockers pause agent execution until you provide the requested information. Answer the question below, then go to Tasks to restart execution.
</p>
</div>
)}As per coding guidelines: "web-ui/src/**/*.{ts,tsx}: Web UI must use Shadcn/UI (Nova preset) with gray color scheme and Hugeicons (@hugeicons/react); never use lucide-react".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {/* Info banner — shown only when there are open blockers */} | |
| {openBlockers.length > 0 && ( | |
| <div className="rounded-lg border border-amber-200 bg-amber-50 px-4 py-3 text-sm dark:border-amber-800 dark:bg-amber-950/30"> | |
| <p className="font-medium text-amber-900 dark:text-amber-200">AI execution is paused.</p> | |
| <p className="mt-0.5 text-amber-800 dark:text-amber-300"> | |
| Blockers pause agent execution until you provide the requested information. Answer the question below, then go to Tasks to restart execution. | |
| </p> | |
| </div> | |
| )} | |
| {/* Info banner — shown only when there are open blockers */} | |
| {openBlockers.length > 0 && ( | |
| <div className="rounded-lg border border-border bg-muted/50 px-4 py-3 text-sm"> | |
| <p className="font-medium text-foreground">AI execution is paused.</p> | |
| <p className="mt-0.5 text-muted-foreground"> | |
| Blockers pause agent execution until you provide the requested information. Answer the question below, then go to Tasks to restart execution. | |
| </p> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web-ui/src/components/blockers/BlockerResolutionView.tsx` around lines 85 -
93, The info banner in BlockerResolutionView.tsx (rendered when
openBlockers.length > 0) uses amber palette Tailwind classes; update the banner
div to use the Nova/Shadcn gray-only tokens instead (replace classes like
border-amber-200 bg-amber-50 text-amber-900 text-amber-800 dark:border-amber-800
dark:bg-amber-950/30 dark:text-amber-300 with the equivalent gray variants from
the Nova preset and ensure proper dark-mode tokens), and if any icon was added
here or nearby, swap it to a Hugeicons import (per repository guideline) rather
than lucide-react; keep the existing conditional render and className structure
on the same element so behavior (openBlockers.length > 0) is unchanged.
Follow-up ReviewMy previous review flagged three things. Here is the status. Still open: Raised-by-Task link destination The link text 'Task {blocker.task_id}' still implies task-specific navigation, but href='/tasks' lands on the full unfiltered list. The mismatch has not changed. Before merging, pick one: change link text to 'Go to Tasks' (matches the button, honest about destination); or route to /tasks?highlight={blocker.task_id} if the tasks page supports that; or use a task detail route if one exists. Good: timeout/auto-redirect removed Removing timerRef and setTimeout(() => onAnswered(), 1500) in favour of an explicit user click is cleaner — no race condition on unmount, and the user controls when navigation happens. The useEffect cleanup is correctly removed as well. Minor: banner color vs. PR description mismatch The PR summary and CodeRabbit walkthrough both call this an 'amber info banner', but the actual classes are bg-muted/50 border-border (neutral). That is fine — neutral styling is acceptable here — but the description is slightly misleading. No code change needed. Still ready to merge once the Raised-by-Task link text or destination is corrected. |
1 participant
Summary
BlockerResolutionView(shown only when open blockers exist): "AI execution is paused. Blockers pause agent execution until you provide the requested information. Answer below, then go to Tasks to restart execution."BlockerCard: "Raised by Task {id}" links to/tasksso users can navigate to the originating taskCloses #473
Test plan
Summary by CodeRabbit
New Features
Improvements