feat(desktop): inline agent-comment composer on v2 DiffPane#4966
Merged
Conversation
Add a per-selection composer in the v2 DiffPane that lets reviewers drag a line range (or click pierre's gutter "+" button) and send a comment to an existing terminal agent or a new session. UI-only — submit currently clears the selection; wiring the chosen agent to terminal.writeInput is a follow-up. Implementation notes: - DiffCommentThread → DiffAnnotationMetadata discriminated union so PR threads and the live composer ride the same renderAnnotation slot. - enableLineSelection + enableGutterUtility on the CodeView; pierre owns the button + click wiring. onGutterUtilityClick must be non-null for the button's pointer flow to fire (InteractionManager returns early otherwise), so we mirror the selection there too. - Composer annotation is injected via a new extraAnnotationsByItemId prop on useDiffCodeViewItems, pinned to the selection's end line.
Factor a reusable useSendToTerminalAgent hook (workspaceTrpc.terminal.writeInput + trailing newline + error toast) plus a formatAgentPromptWithFileContext helper that produces `In <path>:L<start>-L<end>: <comment>`. Other surfaces (file viewer selections, chat actions, tasks) can reuse the same writer without duplicating payload shape or normalization. DiffPane's submit now formats with the helper and pushes to the selected existing terminal binding. The composer shows a pending state during the write, clears the selection on success, and keeps itself open on failure so the user can retry. The "new agent session" branch toasts a stub — spinning up a new session is a separate follow-up.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds end-to-end support for composing and sending file-anchored prompts to agents from diff selections: a prompt formatter and terminal-send hook, AgentCommentComposer UI with picker and placement toggle, new target-selection and composer state hooks, a discriminated annotation union, CodeView plumbing for injected composer annotations, and pane-registry session creation wiring. ChangesAgent-backed diff composer integration
Sequence Diagram(s)sequenceDiagram
participant User
participant DiffPane
participant Composer as AgentCommentComposer
participant ComposerHook as useDiffCommentComposer
participant Registry as usePaneRegistry
participant TRPC as workspaceTrpc
participant Terminal
User->>DiffPane: Select lines / click gutter "+"
DiffPane->>ComposerHook: update selection
ComposerHook->>DiffPane: provide composerAnnotationsByItemId
DiffPane->>Composer: render composer with selection
User->>Composer: choose target (existing or new)
Composer->>Composer: persist selection (localStorage)
User->>Composer: submit comment
Composer->>ComposerHook: call submit(trimmedComment, target)
alt New agent target
ComposerHook->>Registry: onCreateNewAgentSession(configId, placement, prompt)
Registry->>TRPC: agents.run(workspaceId, configId, prompt)
TRPC->>Terminal: spawn terminal and run agent
Terminal-->>TRPC: terminalId
TRPC-->>Registry: terminal session response
Registry->>Registry: register terminal pane via store
Registry-->>ComposerHook: terminalId
else Existing session target
ComposerHook->>TRPC: terminal.writeInput(workspaceId, terminalId, prompt)
TRPC->>Terminal: send prompt to terminal session
end
ComposerHook-->>Composer: submission result
Composer->>DiffPane: clear composer state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
Ready to review this PR? Stage has broken it down into 8 individual chapters for you: 
Chapters generated by Stage for commit 276e320 on May 28, 2026 6:13am UTC. |
Contributor
Greptile SummaryAdds an inline agent-comment composer to the v2 DiffPane: selecting lines (via gutter drag or the "+" button) opens a Linear-style card pinned to the selection's end line, with a textarea, agent picker, and Cancel / Comment buttons. It also factors a reusable
Confidence Score: 4/5Safe to merge — the core submission path (writeInput mutation, error toast, composer clearing) works correctly. The two UX rough edges in the composer do not corrupt data or block any workflow. The annotation merging, discriminated-union narrowing, and dual gutter-callback paths are all sound. The only live issues are in AgentCommentComposer: the initial agent selection can silently stay wrong if bindings load after mount, and Windows/Linux users see a ⌘ hint for a shortcut they trigger with Ctrl. AgentCommentComposer.tsx — stale target initialization and cross-platform keyboard hint.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsx | New inline composer component; has a stale-target initialization issue when terminal bindings load asynchronously, and a Mac-only keyboard hint shown on all platforms. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/DiffPane.tsx | Adds composer state, gutter utility callbacks, and discriminated-union annotation plumbing; logic is sound, dual-path state set via both onSelectedLinesChange and onGutterUtilityClick is harmless. |
| apps/desktop/src/renderer/hooks/host-service/useSendToTerminalAgent/useSendToTerminalAgent.ts | New reusable hook for writing to a terminal agent pty; error toast + rethrow pattern is clean. writeInput in the useCallback dep array is safe in TanStack Query v5 (stable mutation object reference). |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/hooks/useDiffAnnotations/useDiffAnnotations.ts | Introduces DiffAnnotationMetadata discriminated union and adds kind:"thread" tag to existing annotations; straightforward and correct. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/hooks/useDiffCodeViewItems/useDiffCodeViewItems.ts | Merges extraAnnotationsByItemId on top of base annotations; merge logic and version string update for composer metadata are correct. |
Sequence Diagram
sequenceDiagram
participant User
participant CodeView as Pierre CodeView
participant DiffPane
participant Composer as AgentCommentComposer
participant Hook as useSendToTerminalAgent
participant tRPC as terminal.writeInput
User->>CodeView: drag line numbers / click gutter "+"
CodeView->>DiffPane: onSelectedLinesChange(selection)
DiffPane->>DiffPane: "setComposer({ itemId, range })"
DiffPane->>DiffPane: composerAnnotationsByItemId (memo)
DiffPane->>CodeView: items with extra annotation (kind:"composer")
CodeView->>DiffPane: renderAnnotation(annotation, item)
DiffPane->>Composer: AgentCommentComposer startLine endLine
User->>Composer: types comment, picks agent, clicks Comment
Composer->>DiffPane: "onSubmit({ comment, target })"
DiffPane->>DiffPane: formatAgentPromptWithFileContext()
DiffPane->>Hook: "send({ workspaceId, terminalId, text })"
Hook->>tRPC: "writeInput.mutateAsync({ workspaceId, terminalId, data })"
alt success
tRPC-->>Hook: ok
Hook-->>DiffPane: resolved
DiffPane->>DiffPane: handleClearSelection()
DiffPane->>CodeView: clearSelectedLines()
else error
tRPC-->>Hook: throws
Hook->>User: toast.error(Couldn't send to agent)
Hook-->>DiffPane: rethrows
DiffPane->>Composer: composer stays open
end
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsx:49-51
**Stale `target` when sessions change after mount**
`useState`'s initializer runs once; if `useTerminalAgentBindings` resolves asynchronously (empty map on first render, then populated), `target` stays `"__new__"` while real sessions appear in the picker. The user would see terminal A listed but "New agent session" already selected. The reverse is also broken: if the terminal that was `sessions[0]` at mount disconnects later, `target` keeps its dead ID — the Select falls back to the "Choose agent" placeholder visually but the resolved `AgentTarget` still carries the old `terminalId`, so clicking "Comment" would invoke the failing path and rely on the error toast to surface the problem.
Consider adding a sync effect: when `target` is `NEW_SESSION_VALUE` and sessions become non-empty, update to `sessions[0].terminalId`. You may also want to reset `target` to `sessions[0]?.terminalId ?? NEW_SESSION_VALUE` when the currently-selected terminal disappears from the list.
### Issue 2 of 2
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsx:220-232
**Mac-only keyboard hint displayed on all platforms**
`onKeyDown` correctly handles both `e.metaKey` (`⌘`) and `e.ctrlKey` (`Ctrl`), so the shortcut works on Windows/Linux too. However, `KbdEnter` always renders `⌘`, which is misleading for non-Mac users who need to press `Ctrl+Enter`.
```suggestion
const isMac =
typeof navigator !== "undefined" &&
navigator.platform.toLowerCase().includes("mac");
function KbdEnter() {
return (
<span
className={cn(
"inline-flex h-4 items-center gap-0.5 rounded-[3px] border border-primary-foreground/20 bg-primary-foreground/10 px-1",
"text-[9px] font-medium leading-none text-primary-foreground/85",
)}
>
<span>{isMac ? "⌘" : "Ctrl"}</span>
<LuCornerDownLeft className="size-2.5" strokeWidth={2.5} />
</span>
);
}
```
Reviews (1): Last reviewed commit: "feat(desktop): wire DiffPane composer to..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsx:
- Around line 49-51: The initial target state in AgentCommentComposer is set
from sessions only on mount, so when sessions arrive later the composer can
remain "__new__"; add a useEffect that watches sessions and, when sessions[0]
exists and the current target is NEW_SESSION_VALUE (or falsy), call
setTarget(sessions[0].terminalId) to sync the default target to the latest
session; reference the target state, setTarget setter, sessions array, and
NEW_SESSION_VALUE constant in your change.
- Around line 70-82: handleSubmit can surface unhandled promise rejections
because it awaits onSubmit inside a try/finally but does not catch errors;
update handleSubmit (and the analogous function around lines 90-103) to wrap the
await onSubmit(...) in a try/catch/finally: call setSubmitting(true) before,
then try { await onSubmit({ comment: comment.trim(), target: resolved }); }
catch (err) { /* handle or log the error to avoid unhandled rejection */ }
finally { setSubmitting(false); } so any rejection from onSubmit is consumed
(use the function name handleSubmit, the onSubmit prop, NEW_SESSION_VALUE and
setSubmitting to locate the code to change).
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/DiffPane.tsx:
- Around line 144-150: The outbound prompt currently omits diff-side info
causing ambiguous line refs; update the call to formatAgentPromptWithFileContext
(used where text is built) to include the selection's diff side by adding
startSide/endSide (from composer.range.startSide/composer.range.endSide or
equivalent) or, alternatively, include the actual selected snippet text
(extracted from the diff for file.path and composer.range) so deleted-side
selections remain unambiguous; modify the object passed to
formatAgentPromptWithFileContext to carry either a side property
(startSide/endSide) or a snippet property so downstream logic can disambiguate
old vs new lines.
- Around line 153-160: The try/await block calls sendToTerminalAgent and then
unconditionally calls handleClearSelection, which can wipe a newer composer when
an older send finishes; to fix, capture a stable identifier of the
selection/composer (e.g., store input.target.terminalId plus the current
selection/draft id or timestamp from the component state) into a local variable
before awaiting sendToTerminalAgent, then after the await check that the current
selection/composer still matches that captured id (compare the saved
selectionId/timestamp to the live selection state) and only call
handleClearSelection when they match; update the sendToTerminalAgent try block
and any related state access (input.target.terminalId, text,
handleClearSelection) to use this guard.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8a6081d6-3398-4524-bd0c-2b81ea6ca5d0
📒 Files selected for processing (10)
apps/desktop/src/renderer/hooks/host-service/useSendToTerminalAgent/index.tsapps/desktop/src/renderer/hooks/host-service/useSendToTerminalAgent/useSendToTerminalAgent.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/DiffPane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/hooks/useDiffAnnotations/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/hooks/useDiffAnnotations/useDiffAnnotations.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/hooks/useDiffCodeViewItems/useDiffCodeViewItems.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/hooks/useDiffCodeViewScroll/useDiffCodeViewScroll.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/hooks/useDiffCodeViewTheme/useDiffCodeViewTheme.ts
DiffPane composer's agent picker is now a two-section dropdown: - "Active sessions" lists running terminal agent bindings - "Start new session" lists the user's host agent configs with icons Default priority: 1. Last picked terminal session (persisted), if still alive 2. Most recent active session by lastEventAt 3. Last picked new-agent config (persisted), if still listed 4. First config Both picks persist independently in localStorage so toggling between existing/new doesn't trample either preference. AgentTarget gained a `configId` for the new-session branch; submit handling for `kind: "new"` is still a stub (separate follow-up will wire terminal creation + agent launch + writeInput).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 10 files
Architecture diagram
sequenceDiagram
participant User as User
participant Gutter as Gutter / Line-Numbers
participant CV as CodeView Component
participant DP as DiffPane Component
participant UDCVI as useDiffCodeViewItems
participant Comp as AgentCommentComposer
participant Hook as useSendToTerminalAgent
participant WS as workspaceTrpc.terminal
Note over User,WS: Inline agent-comment composer on v2 DiffPane
User->>Gutter: Drag line-numbers or click "+" button
Gutter->>CV: enableLineSelection or onGutterUtilityClick
CV->>DP: onSelectedLinesChange({ itemId, range })
DP->>DP: setComposer(state)
DP->>UDCVI: extraAnnotationsByItemId (composer annotation)
UDCVI->>UDCVI: Merge composer annotation with thread annotations
UDCVI-->>DP: items with composer annotation
DP->>CV: renderAnnotation(composer annotation)
CV->>Comp: AgentCommentComposer rendered at endLine
Note over Comp: Card with textarea + agent picker
alt User selects existing agent
User->>Comp: Type comment, pick agent
Comp->>Comp: formatAgentPromptWithFileContext(comment, file, range)
Comp->>Hook: sendToTerminalAgent({ workspaceId, terminalId, text })
Hook->>WS: writeInput.mutateAsync(normalized command)
alt Success
WS-->>Hook: void
Hook-->>Comp: resolve
Comp->>Comp: clear comment, close composer
Comp->>DP: onCancel
else Failure
WS-->>Hook: error
Hook->>Hook: toast.error("Couldn't send to agent")
Hook-->>Comp: throw
alt User edits and retries
Comp->>Hook: sendToTerminalAgent again
else User dismisses
Comp->>DP: Esc / Cancel
end
end
else User selects "New agent session"
User->>Comp: Type comment, pick "New agent session"
Comp->>Comp: onSubmit({ target: { kind: "new" } })
DP->>DP: toast.info("not yet wired")
Comp->>Comp: stays open
end
Note over DP,WS: Existing PR review threads unchanged (renderAnnotation handles both kinds)
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsx (1)
150-158:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrevent unhandled promise rejections.
handleSubmitawaitsonSubmitin a try/finally but has no catch block, and the form handlers at lines 166-168 and 175-177 call it without consuming the promise. While the upstreamDiffPane.handleSubmitComposercatches errors, this still allows unhandled rejection warnings.🛡️ Proposed fix
const handleSubmit = async () => { if (!canSubmit || !decodedTarget) return; setSubmitting(true); try { await onSubmit({ comment: comment.trim(), target: decodedTarget }); + } catch { + // Error surfaced by caller/toast; keep form state intact. } finally { setSubmitting(false); } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsx around lines 150 - 158, handleSubmit currently awaits onSubmit inside a try/finally but lacks a catch, which lets errors become unhandled when callers (the form handlers) call handleSubmit without awaiting; wrap the await onSubmit({ comment: comment.trim(), target: decodedTarget }) in a try/catch inside handleSubmit (before the finally) and handle errors there — e.g., log the error via process/logger or call an onError callback if available (or at minimum console.error) so exceptions from onSubmit (including DiffPane.handleSubmitComposer) are consumed and do not produce unhandled promise rejections, while still ensuring setSubmitting(false) runs in the finally block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsx:
- Around line 150-158: handleSubmit currently awaits onSubmit inside a
try/finally but lacks a catch, which lets errors become unhandled when callers
(the form handlers) call handleSubmit without awaiting; wrap the await
onSubmit({ comment: comment.trim(), target: decodedTarget }) in a try/catch
inside handleSubmit (before the finally) and handle errors there — e.g., log the
error via process/logger or call an onError callback if available (or at minimum
console.error) so exceptions from onSubmit (including
DiffPane.handleSubmitComposer) are consumed and do not produce unhandled promise
rejections, while still ensuring setSubmitting(false) runs in the finally block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d23b0746-a573-46e0-8230-cd96b50c6513
📒 Files selected for processing (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/DiffPane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/DiffPane.tsx
When the composer's target is a "new session", the submit path now spins up the picked agent in its own terminal pane, waits (best-effort) for the agent to bind, and writes the formatted comment into the running agent. Placement is user-choice via a Split / New tab toggle that only appears for new-session targets; choice persists in localStorage. Factored as a reusable `createAgentSession` util under `renderer/lib/agent-launch/` (takes store + launcher + host client poll), so other diff-adjacent flows (file viewer selection, chat actions, tasks) can spawn an agent pane without re-inventing the launch + bind-wait dance. usePaneRegistry constructs the per-workspace `createNewAgentSession` callback (resolving hostUrl + worktreePath + agent configs once) and threads it down to DiffPane as a prop. AgentTarget gained `placement` on the `new` variant. Persistence keys: - lastSelectedDiffCommentTerminalId - lastSelectedDiffCommentNewAgentConfigId - lastSelectedDiffCommentPlacement
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsx (1)
400-412: 💤 Low valueKeyboard hint shows ⌘ regardless of platform.
The
KbdEntercomponent hardcodes⌘but the keyboard handler accepts bothmetaKey(Mac) andctrlKey(Windows/Linux). Windows users will see the Mac symbol.Potential platform-aware hint
function KbdEnter() { const isMac = typeof navigator !== "undefined" && /Mac/.test(navigator.platform); return ( <span className={cn(/* ... */)}> <span>{isMac ? "⌘" : "Ctrl"}</span> <LuCornerDownLeft className="size-2.5" strokeWidth={2.5} /> </span> ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsx around lines 400 - 412, The KbdEnter component currently hardcodes the Mac symbol (⌘) which mismatches the keyboard handler that accepts metaKey or ctrlKey; update the KbdEnter function to detect platform (server-safe, e.g. const isMac = typeof navigator !== "undefined" && /Mac/.test(navigator.platform) or navigator.userAgent) and render "⌘" when isMac is true and "Ctrl" otherwise, keeping the existing markup and LuCornerDownLeft usage; ensure the detection is safe for SSR by guarding navigator access.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/renderer/lib/agent-launch/createAgentSession.ts`:
- Around line 96-110: The polling loop in waitForAgentBinding lets a single
client.terminalAgents.listByWorkspace.query(...) hang past the overall
timeoutMs; wrap each query call in a per-attempt timeout/abort so no single
request can block the loop. Implement a per-attempt Promise.race (or
AbortController if the client supports it) that rejects after a short
attemptTimeout (e.g., min(pollMs, remainingTime) or a configured perAttemptMs)
and use that instead of awaiting
client.terminalAgents.listByWorkspace.query(...) directly; ensure you still
catch the per-attempt timeout/rejection and continue the loop, referencing
waitForAgentBinding, client.terminalAgents.listByWorkspace.query, timeoutMs and
pollMs.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsx:
- Around line 400-412: The KbdEnter component currently hardcodes the Mac symbol
(⌘) which mismatches the keyboard handler that accepts metaKey or ctrlKey;
update the KbdEnter function to detect platform (server-safe, e.g. const isMac =
typeof navigator !== "undefined" && /Mac/.test(navigator.platform) or
navigator.userAgent) and render "⌘" when isMac is true and "Ctrl" otherwise,
keeping the existing markup and LuCornerDownLeft usage; ensure the detection is
safe for SSR by guarding navigator access.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ecf4dfc4-2b07-44a0-80f0-88964c791f42
📒 Files selected for processing (6)
apps/desktop/src/renderer/lib/agent-launch/createAgentSession.tsapps/desktop/src/renderer/lib/agent-launch/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/DiffPane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx
Contributor
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…comments Drop the renderer-side createAgentSession util I added in the previous commit — host-service already has the canonical prompt pipeline via agents.run (resolveHostAgentConfig + buildAgentCommandString + createTerminalSessionInternal). It bakes the prompt into the initial command using the agent config's argv/stdin transport, so the new- session path no longer needs: - separate launcher.create + writeInput - bind-wait polling against terminalAgents.listByWorkspace - worktreePath/cwd plumbing in the registry - useV2AgentConfigs lookup at the registry layer usePaneRegistry's createNewAgentSession is now a thin wrapper that calls workspaceTrpc.agents.run, places the returned session in a pane (split or new tab), and toasts on failure. Composer keeps the same prop shape but now passes the formatted prompt through; DiffPane skips the follow-up writeInput on the new branch. AgentSessionPlacement type moved into the composer file since the agent-launch util is gone.
…mponents
DiffPane and AgentCommentComposer were growing past 350-400 lines each.
Pulled out three concerns into co-located units:
hooks/useDiffCommentComposer composer state, selection callbacks,
synthetic annotation, submit dispatch
components/AgentCommentComposer
hooks/useDiffCommentTarget target+placement state, persistence,
derived default
components/AgentPickerSelect grouped dropdown (active / new)
components/AgentPlacementToggle split / new tab toggle
Net: DiffPane 362→259 lines, composer 417→186 lines.
Also fixes the "always shows Choose agent" bug: the previous useEffect-
driven default could no-op when stored ids were stale or data hadn't
loaded yet, leaving target null forever. Switched to a derived default
(useMemo over sessions+configs+localStorage) plus a user-override that
is validated against current data on each render, so:
- first render after data loads already shows the right selection
- if the stored terminal/config is gone, falls back automatically
- user picks stick for the composer's lifetime regardless of refetches
Four review findings worth fixing; the rest were either obsoleted by the
agents.run refactor (createAgentSession.ts poll timeouts) or already
covered by useDiffCommentTarget's derived-default pattern (stale target
on async session load).
- Diff-side context in outbound prompts: selections on the deletions
side were silently rendered as `path:L15` even though those line
numbers refer to the pre-diff file. AgentPromptFileContext gains an
optional `side` ("additions"|"deletions"|"mixed") and the formatter
appends "(deleted lines)" / "(across deletions and additions)" so the
agent can resolve the reference correctly.
- Submit race: useDiffCommentComposer.submit captures the composer at
call time and only clears when the live state still matches what it
submitted. A slow send on composer A no longer wipes composer B that
the user opened mid-flight.
- Unhandled promise rejection: AgentCommentComposer.handleSubmit now
catches onSubmit failures (in addition to the existing finally), so a
rejection can't leak out of the form's synchronous onSubmit/onKeyDown
handlers that invoke it without awaiting.
- Platform-aware kbd hint: KbdEnter renders "Ctrl" on Windows/Linux
instead of always showing "⌘" — the existing keydown handler already
accepts both metaKey and ctrlKey.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsx (1)
89-92:⚠️ Potential issue | 🟠 Major | ⚡ Quick winConsume async submit errors in event handlers.
handleSubmit()is fired without consuming its promise. IfonSubmitrejects, this can surface as an unhandled rejection.Proposed fix
onSubmit={(e) => { e.preventDefault(); - handleSubmit(); + void handleSubmit().catch(() => { + // Error is handled upstream (toast/caller). Keep composer open. + }); }} onKeyDown={(e) => { if (e.key === "Escape") { e.stopPropagation(); onCancel(); } if ((e.metaKey || e.ctrlKey) && e.key === "Enter" && canSubmit) { e.preventDefault(); - handleSubmit(); + void handleSubmit().catch(() => { + // Error is handled upstream (toast/caller). Keep composer open. + }); } }}Also applies to: 98-101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsx around lines 89 - 92, The onSubmit handler in AgentCommentComposer calls handleSubmit() without awaiting or handling its promise, risking unhandled rejections; update the event handlers (the form onSubmit and the other handler that invokes handleSubmit around the same area) to consume the async result by either awaiting handleSubmit() inside an async event handler or explicitly attaching .catch(...) to handle errors and prevent unhandled rejections, ensuring you preserve e.preventDefault() and call handleSubmit() on the AgentCommentComposer component's submit path.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/hooks/useDiffCommentComposer/useDiffCommentComposer.ts (2)
148-154:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard against clearing a newer composer after async submit resolves.
clear()runs after awaited sends and can wipe a newer selection/draft if the user starts another composer before the first request returns.Proposed fix
+ const submitted = composer; + + const clearIfStillCurrent = () => { + setComposer((current) => { + const same = + current?.itemId === submitted.itemId && + current.range.start === submitted.range.start && + current.range.end === submitted.range.end && + current.range.side === submitted.range.side && + current.range.endSide === submitted.range.endSide; + if (same) { + codeViewRef.current?.clearSelectedLines(); + return null; + } + return current; + }); + }; + if (input.target.kind === "new") { // ... const result = await onCreateNewAgentSession({ configId: input.target.configId, placement: input.target.placement, prompt: text, }); - if (result) clear(); + if (result) clearIfStillCurrent(); return; } try { await sendToTerminalAgent({ workspaceId, terminalId: input.target.terminalId, text, }); - clear(); + clearIfStillCurrent(); } catch {Also applies to: 157-164
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/hooks/useDiffCommentComposer/useDiffCommentComposer.ts around lines 148 - 154, The async submit path calling onCreateNewAgentSession(...) currently calls clear() unconditionally after await, which can erase a newer composer/draft; capture a stable identifier (e.g. the current composerId or draftId) from the component state before calling await in useDiffCommentComposer, then after the awaited call compare that captured id to the current id and only invoke clear() if they still match; apply this same guard to the other similar block that awaits and calls clear() (the block around the second onCreateNewAgentSession call) so that only the originating composer is cleared when the async submit resolves.
132-139:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPreserve diff-side context in the formatted prompt.
Selections on deletions/old-side are ambiguous when only
path:Lx-Lyis sent. Include side metadata in the formatter input (or selected snippet text) before dispatch.Proposed direction
const text = formatAgentPromptWithFileContext({ comment: input.comment, file: { path: file.path, startLine: composer.range.start, endLine: composer.range.end, + startSide: composer.range.side ?? composer.range.endSide ?? "additions", + endSide: composer.range.endSide ?? composer.range.side ?? "additions", }, });(Requires extending
formatAgentPromptWithFileContextcontract accordingly.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/hooks/useDiffCommentComposer/useDiffCommentComposer.ts around lines 132 - 139, The current call to formatAgentPromptWithFileContext only sends file.path and start/end lines which loses diff-side info for deletions; modify the payload passed to formatAgentPromptWithFileContext (from useDiffCommentComposer) to include the diff side (e.g., file.side = composer.side or a new property like file.diffSide) or include the selected snippet text (e.g., file.selectedText) so the formatter can disambiguate old vs new; update the formatAgentPromptWithFileContext contract/signature accordingly and ensure all callers are updated to pass the new field (reference the call site that builds the file object and the formatAgentPromptWithFileContext function itself).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsx:
- Around line 89-92: The onSubmit handler in AgentCommentComposer calls
handleSubmit() without awaiting or handling its promise, risking unhandled
rejections; update the event handlers (the form onSubmit and the other handler
that invokes handleSubmit around the same area) to consume the async result by
either awaiting handleSubmit() inside an async event handler or explicitly
attaching .catch(...) to handle errors and prevent unhandled rejections,
ensuring you preserve e.preventDefault() and call handleSubmit() on the
AgentCommentComposer component's submit path.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/hooks/useDiffCommentComposer/useDiffCommentComposer.ts:
- Around line 148-154: The async submit path calling
onCreateNewAgentSession(...) currently calls clear() unconditionally after
await, which can erase a newer composer/draft; capture a stable identifier (e.g.
the current composerId or draftId) from the component state before calling await
in useDiffCommentComposer, then after the awaited call compare that captured id
to the current id and only invoke clear() if they still match; apply this same
guard to the other similar block that awaits and calls clear() (the block around
the second onCreateNewAgentSession call) so that only the originating composer
is cleared when the async submit resolves.
- Around line 132-139: The current call to formatAgentPromptWithFileContext only
sends file.path and start/end lines which loses diff-side info for deletions;
modify the payload passed to formatAgentPromptWithFileContext (from
useDiffCommentComposer) to include the diff side (e.g., file.side =
composer.side or a new property like file.diffSide) or include the selected
snippet text (e.g., file.selectedText) so the formatter can disambiguate old vs
new; update the formatAgentPromptWithFileContext contract/signature accordingly
and ensure all callers are updated to pass the new field (reference the call
site that builds the file object and the formatAgentPromptWithFileContext
function itself).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f34e86e-dbaa-4985-b5ca-f839d38b1369
📒 Files selected for processing (10)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/DiffPane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/AgentCommentComposer.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/components/AgentPickerSelect/AgentPickerSelect.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/components/AgentPickerSelect/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/components/AgentPlacementToggle/AgentPlacementToggle.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/components/AgentPlacementToggle/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/hooks/useDiffCommentTarget/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/hooks/useDiffCommentTarget/useDiffCommentTarget.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/hooks/useDiffCommentComposer/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/hooks/useDiffCommentComposer/useDiffCommentComposer.ts
✅ Files skipped from review due to trivial changes (3)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/components/AgentPlacementToggle/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/hooks/useDiffCommentComposer/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/AgentCommentComposer/components/AgentPickerSelect/index.ts
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
5 tasks
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
useSendToTerminalAgenthook +formatAgentPromptWithFileContexthelper so other surfaces (file viewer selections, chat actions, tasks) can push prompts to an existing terminal agent with the same payload + normalization + error toast.workspaceTrpc.terminal.writeInput. "New agent session" is stubbed with a toast — separate follow-up.Implementation notes
DiffCommentThreadbecomes part of a discriminated unionDiffAnnotationMetadataso PR review threads and the live composer ride the samerenderAnnotationslot. The composer is injected via a newextraAnnotationsByItemIdprop onuseDiffCodeViewItems, pinned to the selection's end line.enableGutterUtility: true+ anonGutterUtilityClickhandler is required (without it,InteractionManager.startGutterSelectionFromPointerDownreturns early and the button's pointer flow no-ops). The callback readscodeViewRef.current.getSelectedLines()to recover the{ itemId, range }sinceonGutterUtilityClickonly receives a range.enableLineSelection+onSelectedLinesChangedirectly.Test plan
Lines N–MlabelLine NIn <path>:L<n>[-L<m>]: <comment>\nto that terminal; the composer clearswriteInputto fail (e.g., bogus terminal id) → error toast appears, composer stays openSummary by cubic
Adds an inline agent-comment composer to the v2 DiffPane so you can select lines and send a line-anchored prompt to a terminal agent. Works with existing sessions or launches a new one (Split or New tab) via the host pipeline; prompts now include diff-side context for deletions/mixed selections.
New Features
useSendToTerminalAgent→workspaceTrpc.terminal.writeInput; new session viaworkspaceTrpc.agents.runwith the prompt baked in. Placement (Split/New tab) is persisted; composer clears on success and toasts on errors.Bug Fixes
Written for commit 276e320. Summary will update on new commits.
Review in cubic
Summary by CodeRabbit
New Features
Bug Fixes