feat(web): add CopilotKit GenUI review agent - #8143
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| reportFailure: false, | ||
| }); | ||
| const startThreadTurn = useAtomCommand(threadEnvironment.startTurn, { reportFailure: false }); | ||
| const approvedSignatures = useRef(new Set<string>()); |
There was a problem hiding this comment.
🟡 Medium copilotkit/CopilotReviewAgent.tsx:278
An approval from one environment/thread can be consumed after navigation and cause apply_review_fixes to start those fixes in the newly active thread without approval there. approvedSignatures is retained for the long-lived ReviewToolHost, while reviewApprovalSignature includes only findings and verification; bind the approval key to environmentId and threadId, or clear the set when either prop changes.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/copilotkit/CopilotReviewAgent.tsx around line 278:
An approval from one environment/thread can be consumed after navigation and cause `apply_review_fixes` to start those fixes in the newly active thread without approval there. `approvedSignatures` is retained for the long-lived `ReviewToolHost`, while `reviewApprovalSignature` includes only findings and verification; bind the approval key to `environmentId` and `threadId`, or clear the set when either prop changes.
| const normalized = value | ||
| .trim() | ||
| .replace(/^[ab]\//, "") | ||
| .replaceAll("\\", "/"); |
There was a problem hiding this comment.
🟡 Medium copilotkit/copilotReview.logic.ts:103
safeWorkspaceRelativePath strips leading and trailing whitespace, so a finding for a tracked path such as src/name opens src/name instead and reports { opened: true } for the wrong file. Remove the trim() call so valid repository filenames are preserved.
| const normalized = value | |
| .trim() | |
| .replace(/^[ab]\//, "") | |
| .replaceAll("\\", "/"); | |
| const normalized = value | |
| .replace(/^[ab]\//, "") | |
| .replaceAll("\\", "/"); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/copilotkit/copilotReview.logic.ts around lines 103-106:
`safeWorkspaceRelativePath` strips leading and trailing whitespace, so a finding for a tracked path such as `src/name ` opens `src/name` instead and reports `{ opened: true }` for the wrong file. Remove the `trim()` call so valid repository filenames are preserved.
| environmentId={activeThread.environmentId} | ||
| interactionMode={interactionMode} | ||
| isWorking={isWorking} | ||
| modelSelection={activeThread.modelSelection} |
There was a problem hiding this comment.
🟡 Medium components/ChatView.tsx:7106
CopilotReviewAgent receives activeThread.modelSelection, so approving review fixes runs with the persisted model instead of the model currently selected in the composer. Use the effective composer selection from ChatComposer.getSendContext().selectedModelSelection, matching the normal send path.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatView.tsx around line 7106:
`CopilotReviewAgent` receives `activeThread.modelSelection`, so approving review fixes runs with the persisted model instead of the model currently selected in the composer. Use the effective composer selection from `ChatComposer.getSendContext().selectedModelSelection`, matching the normal send path.
There was a problem hiding this comment.
Reviewed the new web UI surface (apps/web/src/components/copilotkit/**) against the shared component system and Tailwind/CSS ownership rules. Three findings, all in CopilotReviewAgent.tsx: an unlayered third-party global stylesheet imported from a lazily loaded component, a Card call site that overrides the primitive's radius and clips its edge treatment, and a raw success banner that reconstructs the Alert primitive without its dark-mode surface or alert semantics. Everything else (Badge variants, Button micro/sm sizes, semantic color tokens, icon sizing) lines up with the existing primitives.
Posted via Macroscope — UI Consistency
| <div className="flex items-center gap-2 rounded-lg bg-success/8 p-3 text-sm text-success-foreground"> | ||
| <ShieldCheckIcon className="size-4" /> | ||
| No actionable issue found in the supplied diff. | ||
| </div> |
There was a problem hiding this comment.
This raw div reconstructs the Alert primitive (apps/web/src/components/ui/alert.tsx, variant="success"): icon slot plus message on a success surface. Rebuilding it locally drops role="alert", the primitive's icon slot sizing/tone, and its theme-tested surface — bg-success/8 here has no dark counterpart, whereas the shared treatments are border-success/32 bg-success/4 (Alert) or bg-success/8 dark:bg-success/16 (Badge), so this block reads noticeably flat in dark mode.
Using the primitive also needs an import of Alert/AlertTitle from ../ui/alert:
- <div className="flex items-center gap-2 rounded-lg bg-success/8 p-3 text-sm text-success-foreground">
- <ShieldCheckIcon className="size-4" />
- No actionable issue found in the supplied diff.
- </div>
+ <Alert variant="success">
+ <ShieldCheckIcon />
+ <AlertTitle>No actionable issue found in the supplied diff.</AlertTitle>
+ </Alert>Posted via Macroscope — UI Consistency
| const VerdictIcon = presentation.icon; | ||
|
|
||
| return ( | ||
| <Card className="my-3 overflow-hidden rounded-xl border-border/80 bg-card/80"> |
There was a problem hiding this comment.
These call-site classes break Card's own geometry contract. Card paints its light/dark edge treatment with before:inset-0 before:rounded-[calc(var(--radius-2xl)-1px)] before:shadow-[0_1px_…] dark:before:shadow-[0_-1px_…]; overflow-hidden clips that 1px shadow away entirely, and rounded-xl shrinks only the outer radius while the pseudo-element stays at the 2xl radius, so the corners no longer line up.
The nested rows already carry their own radius and border, so the clip isn't needed. If the smaller radius is a durable, shared treatment (line 441 uses it too), prefer a radius variant on Card that moves the ::before radius with it instead of overriding it per call site.
| <Card className="my-3 overflow-hidden rounded-xl border-border/80 bg-card/80"> | |
| <Card className="my-3 border-border/80 bg-card/80"> |
Posted via Macroscope — UI Consistency
| useFrontendTool, | ||
| useHumanInTheLoop, | ||
| } from "@copilotkit/react-core/v2"; | ||
| import "@copilotkit/react-core/v2/styles.css"; |
There was a problem hiding this comment.
This third-party stylesheet is imported from a lazily loaded component, so it is injected unlayered and only when the sidebar chunk loads. index.css puts app styles in Tailwind's layers (@import "tailwindcss", @layer base, @layer components), and unlayered author rules beat layered ones regardless of specificity — so any global element/:root/.dark rules in this file can override app base and utility styles document-wide, not just inside the CopilotKit sidebar, and the exact cascade position depends on when the chunk happens to load.
Consider importing it once from apps/web/src/index.css with an explicit layer so app styles keep ownership (e.g. @import "@copilotkit/react-core/v2/styles.css" layer(components);), and check the emitted CSS for global resets or theme variables it redefines.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Two Effect-convention findings in the new server-side CopilotKit code. Details inline.
Posted via Macroscope — Effect Service Conventions
| class CopilotRuntimeRequestError extends Data.TaggedError("CopilotRuntimeRequestError")<{ | ||
| readonly cause: unknown; | ||
| }> {} |
There was a problem hiding this comment.
CopilotRuntimeRequestError carries only an opaque cause, so neither the error nor the log line records which request failed, even though the method and pathname are available at the wrapping site. Consider capturing that stable context as attributes and deriving message from them, keeping the original failure as cause (the construction site at line 29 then passes method/pathname).
-class CopilotRuntimeRequestError extends Data.TaggedError("CopilotRuntimeRequestError")<{
- readonly cause: unknown;
-}> {}
+class CopilotRuntimeRequestError extends Data.TaggedError("CopilotRuntimeRequestError")<{
+ readonly method: string;
+ readonly path: string;
+ readonly cause: unknown;
+}> {
+ get message() {
+ return `CopilotKit runtime request failed: ${this.method} ${this.path}`;
+ }
+}Posted via Macroscope — Effect Service Conventions
| const reviewModel = globalThis.process.env.COPILOTKIT_REVIEW_MODEL?.trim() || "openai/gpt-5-mini"; | ||
|
|
||
| const runtime = new CopilotRuntime({ | ||
| agents: { | ||
| review: new BuiltInAgent({ | ||
| model: reviewModel, | ||
| maxSteps: 10, | ||
| prompt: REVIEW_AGENT_PROMPT, | ||
| }), | ||
| }, | ||
| runner: new InMemoryAgentRunner(), | ||
| }); |
There was a problem hiding this comment.
The CopilotKit runtime and its model configuration are built as module-level globals at import time, so this runtime-backed (network/LLM) dependency never appears in the Effect environment of copilotReviewRouteLayer, and COPILOTKIT_REVIEW_MODEL bypasses ServerConfig. Every other external runtime in apps/server (OpenCode SDK, Claude Agent SDK) is constructed inside an Effect factory and provided through a layer.
Consider exposing make (acquiring the model from ServerConfig via yield*) plus a layer for the handler, and having the route acquire it from context so the async dependency stays visible and injectable. Fix spans two files, so no inline diff.
Posted via Macroscope — Effect Service Conventions
The previous CopilotKit demo centered on a Discord bot, but the updated direction is to demonstrate generative UI inside T3 Code.
This adds a branch-review agent that reads the real branch and working-tree diff, renders an interactive review dashboard, opens affected files, and asks for approval before handing exact findings to the normal T3 coding agent.
Demo flow
Verification
Draft follow-up
Model: GPT-5.6
Harness: Codex
Note
Add CopilotKit GenUI review agent with human-in-the-loop approval
/api/copilotkitexposing areviewbuilt-in agent (defaults toopenai/gpt-5-mini,maxSteps10) using CopilotKit v2inspect_branch,open_file, andapply_review_fixeswith a human-in-the-loopapprove_fixesstep; fixes are only applied after a signature keyed to exact findings and verification commandsMAX_REVIEW_DIFF_CHARS= 70,000), safe workspace-relative path validation, and prompt building for approved fixes in copilotReview.logic.tsauthenticateRawRouteWithScopein http.ts so the CopilotKit handler can reuse auth/api/copilotkit/*route requiresAuthOrchestrationOperateScope; invalid URL conversion returns 400 and internal handler errors return a generic 500 with no detail leakage📊 Macroscope summarized b733a99. 9 files reviewed, 5 issues evaluated, 2 issues filtered, 3 comments posted
🗂️ Filtered Issues
apps/web/src/components/copilotkit/copilotReview.logic.ts — 1 comment posted, 3 evaluated, 2 filtered
diff --gitparser only accepts unquoteda/... b/...headers. Git quotes pathnames containing unusual characters (for example tabs, newlines, and depending oncore.quotePath, non-ASCII bytes), producing a header such asdiff --git "a/foo\tbar" "b/foo\tbar". The regex fails, setscurrenttonull, and omits that file and all of its changes from the review summary and file list. [ Out of scope (post-validation triage) ]summarizeUnifiedDifftreats every patch line beginning with+++or---as a file header. An added source line such as++counteris encoded as+++counter, and a deleted source line beginning with--is encoded similarly, so these real changes are omitted fromadditions/deletionsand the review dashboard reports incorrect totals. Header detection should distinguish actual+++ <path>/--- <path>metadata from content lines. [ Out of scope (post-validation triage) ]