Ade Web App Launch Via Cli#381
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@copilot review but do not make fixes |
|
Warning Review limit reached
More reviews will be available in 27 minutes and 5 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR refactors the built-in browser service for multi-window support, enhances chat with Codex transcript fetching and plan-mode transitions, consolidates terminal headers into a shared component, and updates environment handling across daemon spawning and PTY configuration. ChangesBuilt-in browser multi-window support
Chat agent Codex transcript and plan-mode updates
Terminal work-surface header consolidation
Environment variable and build hash management
Chat UI panel styling and development tooling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
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. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review
Scope: 39 file(s), +2780 / −849
Verdict: Minor issues
This PR refactors Work/CLI/chat headers, scopes the built-in browser per Electron window, adds Codex goal + subagent transcript plumbing, and syncs Claude plan-mode from SDK status messages. The multi-window browser work and IPC routing look sound; the main gaps are incomplete subagent timeline rendering and heavy Codex transcript polling.
🐛 Functionality
[High] Codex subagent drill-in drops tool and user messages
File: apps/desktop/src/renderer/components/chat/AgentChatMessageList.tsx:L3804-L3871
Issue: codexThreadItemToTranscriptMessages maps Codex mcpToolCall / dynamicToolCall and userMessage items into tool_call and user_message events, but SubagentTimelineCard has no cases for those types and returns null in the default branch. Drill-in transcripts can look empty or incomplete even when the app-server returned data.
Repro: Open a Codex subagent whose thread is mostly tool calls (typical delegation). Events land in richEntries, but nothing renders.
Fix: Add tool_call and user_message cases (reuse CommandEventCard / existing user row patterns), or route unhandled types through the legacy role+text path instead of silently skipping.
⚡ Performance
[Medium] Subagent transcript polling re-fetches full Codex thread every 1.5s
File: apps/desktop/src/main/services/chat/agentChatService.ts:L22826-L22874 and apps/desktop/src/renderer/components/chat/AgentChatPane.tsx:L2465-L2494
Issue: While a subagent is running, the pane polls getSubagentTranscript every 1.5s without a limit. For live Codex runtimes, fetchCodexSubagentTranscriptFromAppServer walks up to 10 pages × 50 turns (itemsView: "full"), converts every item, then slices—so each poll can issue many app-server round-trips and parse large payloads even when the UI only needs the tail.
Impact: Noticeable main-process + Codex load during drill-in on long subagent threads (~6–7 full paginated fetches per 10s of viewing).
Fix: Pass a small limit from the renderer (e.g. last 100 messages), stop early once enough items are collected, and/or only paginate on first open then append deltas from eventHistoryBySession on subsequent polls.
🎨 UI/UX
[Low] Browser panel no longer shows what element is selected
File: apps/desktop/src/renderer/components/chat/ChatBuiltInBrowserPanel.tsx:L1735-L1806 (removed block)
Issue: The footer that showed selected element label, selector/URL, attachment ack, Clear, and Insert draft was removed. Inspect/Attach still work, but users cannot see which element is selected or confirm attachment without inferring from toolbar state.
Fix: Restore a compact status row (or show selection summary next to the Attach control) when selectedItem is set.
[Low] Browser tab controls are below comfortable touch size
File: apps/desktop/src/renderer/components/chat/ChatBuiltInBrowserPanel.tsx:L1493-L1533
Issue: Tab rows are h-[18px] with 8–11px icons and a 16px new-tab control—well under the 40px touch target guideline used elsewhere in Work surfaces.
Fix: Increase tab hit area (padding/min-height) while keeping the visual chrome compact, or add a touch-friendly density mode.
[Low] Codex goal edits commit on blur
File: apps/desktop/src/renderer/components/chat/codex/CodexGoalCard.tsx:L149-L155
Issue: The goal textarea calls submitEdit on onBlur, so tabbing away or clicking elsewhere sends /goal set … even when the user did not intend to save.
Fix: Commit only on explicit Enter (without Shift) or a Save control; treat blur as cancel unless the draft changed and was confirmed.
Notes
- Per-window built-in browser routing (
builtInBrowserService.ts+registerIpc.tspassingBrowserWindow) and targetedbuiltInBrowserEventdelivery inmain.tsare solid improvements for multi-window ADE. - Removing the in-chat
CodexGoalBannerin favor ofChatSubagentsPanel+CodexGoalCardis coherent; Agents tab gating onselectedCodexGoalis updated accordingly. - CLI header stop control removal is documented in tests as intentional (sidebar/context menu path); not flagged as a regression.
- Could not fetch PR body via
gh(401); review is based on the provided SHAs and local diff880c47790e83a6a8cc561f0d324c29826cbcaa56...8a177ecd24ab27f2853c96b85220d60f2af0e7ae.
Sent by Cursor Automation: BUGBOT in Versic
arul28
force-pushed
the
ade/ade-web-app-launch-via-cli-e16fe5c4
branch
from
8a177ec to
182ec15
Compare
|
@copilot review but do not make fixes |
Refactor Work tab surfaces to support web app launching from CLI sessions. Consolidate chat panel components, add Codex goal card rendering, fix CLI service command resolution, and sync plan-mode transitions from SDK status messages. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
arul28
force-pushed
the
ade/ade-web-app-launch-via-cli-e16fe5c4
branch
from
182ec15 to
d7dac0b
Compare
|
@copilot review but do not make fixes |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/components/chat/AgentChatMessageList.tsx (1)
3910-3993:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep rich and legacy transcript entries in their original order.
This partition renders all rich cards first and all legacy rows second. If a transcript mixes ADE events with legacy SDK messages, the drill-in loses chronology and becomes misleading.
Suggested fix
- const richEntries: Array<{ - key: string; - event: AgentChatEvent; - }> = []; - const legacyEntries: Array<{ - key: string; - message: import("../../../shared/types").AgentChatSubagentTranscriptMessage; - }> = []; - for (let i = 0; i < messages.length; i += 1) { - const m = messages[i]; - const key = m.uuid ?? `idx-${i}`; - const event = readSubagentEvent(m); - if (event) { - richEntries.push({ key, event }); - } else { - legacyEntries.push({ key, message: m }); - } - } + const entries = messages.map((message, index) => ({ + key: message.uuid ?? `idx-${index}`, + event: readSubagentEvent(message), + message, + index, + })); - if (richEntries.length === 0 && legacyEntries.length === 0) { + if (entries.length === 0) { return ( <div className={cn("flex min-h-0 flex-1 flex-col px-8 pt-12 font-sans", className)}> <p className="text-[13.5px] leading-6 text-fg/55"> {loading ? "Listening for the first message…" : "No messages from this subagent yet."} @@ - {richEntries.length ? ( - <ol className="mx-auto w-full max-w-3xl px-6 pb-6 pt-5"> - {richEntries.map((entry) => ( - <SubagentTimelineCard key={entry.key} event={entry.event} /> - ))} - </ol> - ) : null} - {legacyEntries.length ? ( - <ol className="mx-auto w-full max-w-3xl px-6 pb-10 pt-2"> - {legacyEntries.map((entry, index) => { + <ol className="mx-auto w-full max-w-3xl px-6 pb-10 pt-5"> + {entries.map((entry) => { + if (entry.event) { + return <SubagentTimelineCard key={entry.key} event={entry.event} />; + } + const text = extractSubagentMessageText(entry.message); const role: string = entry.message.type; const isUser = role === "user"; const isSystem = role === "system"; @@ return ( <li - key={entry.key ?? `${index}-${role}`} + key={entry.key ?? `${entry.index}-${role}`} className="grid grid-cols-[88px_minmax(0,1fr)] gap-x-5 border-b border-white/[0.03] py-3 last:border-b-0" > @@ </li> ); - })} - </ol> - ) : null} + })} + </ol>🤖 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/components/chat/AgentChatMessageList.tsx` around lines 3910 - 3993, The current code splits messages into richEntries and legacyEntries which loses original chronology; instead build a single ordered entries array while iterating messages (e.g. push {key, index: i, kind: "rich", event} or {key, index: i, kind: "legacy", message}) using readSubagentEvent to classify, then render by mapping that unified entries array in order and for each entry conditionally render SubagentTimelineCard(entry.event) for kind === "rich" or the legacy row (using extractSubagentMessageText, role/type handling and the same markup) for kind === "legacy"; remove the separate richEntries/legacyEntries rendering blocks so messages keep their original ordering.
🤖 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 @.agents/skills/ade-web/SKILL.md:
- Around line 100-101: Update the guidance that currently reads “Never touch the
ADE socket” to a precise constraint: state that consumers must not start,
replace, or modify the runtime/bridge socket, but may use an existing socket for
browser actions (e.g., when running `ade actions ... --socket`); mention
`browserMock.ts` and `window.ade` as the Vite-only preview stub and clarify it
does not open a runtime connection so operators won’t block legitimate
`--socket` flows. Ensure the new wording replaces the old sentence and
explicitly communicates what changed, what is blocked (starting/replacing the
socket/bridge), and the allowed next action (use existing socket for browser
actions).
In `@apps/desktop/src/main/services/builtInBrowser/builtInBrowserService.ts`:
- Around line 119-123: serviceForWindow() currently disposes fallbackService
before creating a new per-window service which loses any tabs/state that were
created via navigate/showPanel/createTab without a sourceWindow; modify
serviceForWindow/createServiceForWindow/attachToWindow flow to detect an
existing fallbackService and migrate its state into the first created per-window
service instead of disposing it (e.g., transfer tabs, active index, history,
etc.), or alternatively make navigate/showPanel/createTab reject/mutate calls
until a window is bound; specifically update serviceForWindow() to check
fallbackService and call a new transferFallbackState(fallbackService, service)
before setting fallbackService = null, and ensure windowServices.set(win.id,
service) and closedListener logic remain intact so the migrated state persists
for that window (same change also apply to the similar block around lines
160-163).
In `@apps/desktop/src/main/services/chat/agentChatService.ts`:
- Around line 22863-22866: The loop that pages through transcript items uses a
hard MAX_PAGES = 10 and silently returns a partial transcript (variables:
cursor, pages, MAX_PAGES, collected, and the surrounding fetch loop), so change
the logic to avoid silent truncation by making the cap configurable and by
returning an explicit truncation indicator: replace the fixed MAX_PAGES constant
with a configurable limit or remove it, and if the loop must stop because pages
reached the limit set a boolean (e.g. truncated = true) and include it in the
function's return value or throw a clear error; ensure the caller no longer
infers completeness from collected.length and instead checks the new
truncated/hasMore flag (update all code paths that read collected and the
function signature/return type accordingly).
- Around line 22651-22658: The fallback item id should be deterministic instead
of randomUUID() so repeated fetches keep stable IDs; in
codexThreadItemToTranscriptMessages replace the randomUUID() fallback with a
deterministic id derived from stable inputs (for example, serialize stable
fields of item plus threadId and turnId and compute a hash like SHA-256 or a
stable base64 encoding) and use that result as itemId; apply the same
deterministic-fallback change to the other similar occurrence around the codex
handling at the second block mentioned (the lines around the other occurrence)
so both places generate identical stable ids for items missing an id.
In `@apps/desktop/src/renderer/components/chat/AgentChatMessageList.tsx`:
- Around line 3785-3806: SubagentResultRow currently ignores event.status
(subagent_result.status) and always renders the green "Final result" success
card; update SubagentResultRow to read event.status and branch rendering: for
successful status keep the existing emerald card and text, but for "failed" or
"stopped" render a neutral/error style (e.g., red/gray border/background and an
appropriate label/icon and message) and surface the same summary/description if
present; ensure you reference the same event variable (and its
summary/description fallback logic) so the content stays identical while the
visual outcome matches event.status — apply the same status-aware rendering for
the similar block around the other occurrence noted (the block at the other
SubagentResultRow usage).
In
`@apps/desktop/src/renderer/components/terminals/CliSessionWorkSurfaceHeader.tsx`:
- Around line 48-99: The CLI header removed the stop control — restore a stop
affordance in CliSurfaceTrailingActions by adding an optional onStopClick prop
(e.g., onStopClick?: (session: TerminalSessionSummary, event:
ReactMouseEvent<HTMLElement>) => void) and rendering a Stop button (inside a
SmartTooltip with label like "Stop session") when the session is running (check
a session flag such as session.laneId or session.state === 'running' /
session.isRunning). Wire the button’s onClick to call onStopClick?.(session,
event), set aria-label="Stop session", and disable the button when onStopClick
is not provided; apply the same change to the other header component instance
mentioned (lines 107-173) so running agent CLI sessions regain a header stop
path.
---
Outside diff comments:
In `@apps/desktop/src/renderer/components/chat/AgentChatMessageList.tsx`:
- Around line 3910-3993: The current code splits messages into richEntries and
legacyEntries which loses original chronology; instead build a single ordered
entries array while iterating messages (e.g. push {key, index: i, kind: "rich",
event} or {key, index: i, kind: "legacy", message}) using readSubagentEvent to
classify, then render by mapping that unified entries array in order and for
each entry conditionally render SubagentTimelineCard(entry.event) for kind ===
"rich" or the legacy row (using extractSubagentMessageText, role/type handling
and the same markup) for kind === "legacy"; remove the separate
richEntries/legacyEntries rendering blocks so messages keep their original
ordering.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 75b85fc5-45ee-4107-ad69-15f663f47c47
⛔ Files ignored due to path filters (5)
AGENTS.mdis excluded by!*.mddocs/ARCHITECTURE.mdis excluded by!docs/**docs/features/chat/README.mdis excluded by!docs/**docs/features/terminals-and-sessions/README.mdis excluded by!docs/**docs/features/terminals-and-sessions/ui-surfaces.mdis excluded by!docs/**
📒 Files selected for processing (34)
.agents/skills/ade-web/SKILL.mdapps/ade-cli/src/cli.tsapps/ade-cli/src/stdioRpcDaemon.test.tsapps/ade-cli/src/tuiClient/connection.tsapps/desktop/scripts/export-browser-mock-ade-snapshot.mjsapps/desktop/src/main/main.tsapps/desktop/src/main/services/builtInBrowser/builtInBrowserService.test.tsapps/desktop/src/main/services/builtInBrowser/builtInBrowserService.tsapps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/main/services/cli/adeCliService.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/macosVm/macosVmService.tsapps/desktop/src/main/services/pty/ptyService.tsapps/desktop/src/main/services/remoteRuntime/remoteBootstrap.test.tsapps/desktop/src/renderer/components/chat/AgentChatMessageList.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/chat/ChatAppControlPanel.tsxapps/desktop/src/renderer/components/chat/ChatBuiltInBrowserPanel.test.tsxapps/desktop/src/renderer/components/chat/ChatBuiltInBrowserPanel.tsxapps/desktop/src/renderer/components/chat/ChatIosSimulatorPanel.tsxapps/desktop/src/renderer/components/chat/ChatSubagentsPanel.tsxapps/desktop/src/renderer/components/chat/codex/CodexGoalCard.test.tsxapps/desktop/src/renderer/components/chat/codex/CodexGoalCard.tsxapps/desktop/src/renderer/components/terminals/CliSessionWorkSurfaceHeader.test.tsxapps/desktop/src/renderer/components/terminals/CliSessionWorkSurfaceHeader.tsxapps/desktop/src/renderer/components/terminals/TerminalsPage.tsxapps/desktop/src/renderer/components/terminals/WorkCliSessionHeader.test.tsxapps/desktop/src/renderer/components/terminals/WorkCliSessionHeader.tsxapps/desktop/src/renderer/components/terminals/WorkViewArea.test.tsxapps/desktop/src/renderer/components/terminals/WorkViewArea.tsxapps/desktop/src/renderer/components/work/WorkSurfaceHeader.test.tsxapps/desktop/src/renderer/components/work/WorkSurfaceHeader.tsxapps/desktop/src/renderer/index.css
💤 Files with no reviewable changes (2)
- apps/desktop/src/renderer/components/terminals/WorkCliSessionHeader.test.tsx
- apps/desktop/src/renderer/components/terminals/WorkCliSessionHeader.tsx
| fallbackService?.dispose(); | ||
| fallbackService = null; | ||
| const service = createServiceForWindow(win); | ||
| const closedListener = () => { | ||
| windowServices.delete(win.id); |
There was a problem hiding this comment.
Preserve fallback browser state when the first window attaches.
serviceForWindow() disposes fallbackService before any state is transferred. Because the public API still allows navigate/showPanel/createTab without a sourceWindow, those calls can create real tabs in the fallback instance; the first attachToWindow() then swaps in a fresh empty per-window service and drops that state. Either migrate the fallback instance into the first window entry or reject mutating calls until a window is bound.
Also applies to: 160-163
🤖 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/main/services/builtInBrowser/builtInBrowserService.ts`
around lines 119 - 123, serviceForWindow() currently disposes fallbackService
before creating a new per-window service which loses any tabs/state that were
created via navigate/showPanel/createTab without a sourceWindow; modify
serviceForWindow/createServiceForWindow/attachToWindow flow to detect an
existing fallbackService and migrate its state into the first created per-window
service instead of disposing it (e.g., transfer tabs, active index, history,
etc.), or alternatively make navigate/showPanel/createTab reject/mutate calls
until a window is bound; specifically update serviceForWindow() to check
fallbackService and call a new transferFallbackState(fallbackService, service)
before setting fallbackService = null, and ensure windowServices.set(win.id,
service) and closedListener logic remain intact so the migrated state persists
for that window (same change also apply to the similar block around lines
160-163).
| let cursor: string | null = null; | ||
| let pages = 0; | ||
| const MAX_PAGES = 10; | ||
| try { |
There was a problem hiding this comment.
Don't return a silently truncated transcript.
The hard MAX_PAGES = 10 cap means any subagent thread with more than 500 items is returned as a partial transcript, but the caller treats it as complete because collected.length > 0. That drops older turns with no fallback path.
Suggested fix
do {
const params: Record<string, unknown> = {
@@
cursor = typeof response?.nextCursor === "string" ? response.nextCursor : null;
pages += 1;
} while (cursor && pages < MAX_PAGES);
+ if (cursor) return null;
} catch {Also applies to: 22867-22899
🤖 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/main/services/chat/agentChatService.ts` around lines 22863 -
22866, The loop that pages through transcript items uses a hard MAX_PAGES = 10
and silently returns a partial transcript (variables: cursor, pages, MAX_PAGES,
collected, and the surrounding fetch loop), so change the logic to avoid silent
truncation by making the cap configurable and by returning an explicit
truncation indicator: replace the fixed MAX_PAGES constant with a configurable
limit or remove it, and if the loop must stop because pages reached the limit
set a boolean (e.g. truncated = true) and include it in the function's return
value or throw a clear error; ensure the caller no longer infers completeness
from collected.length and instead checks the new truncated/hasMore flag (update
all code paths that read collected and the function signature/return type
accordingly).
- Clarify SKILL.md socket constraint (no longer contradicts --socket usage) - Use deterministic fallback IDs for Codex thread items without IDs - Log warning when transcript fetch hits MAX_PAGES cap - Render failed/stopped subagent results with appropriate colors - Restore stop-session button in CLI work surface header Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
| }} | ||
| rows={2} | ||
| className={cn( | ||
| "mt-1.5 block w-full resize-none rounded border border-amber-400/30 bg-amber-950/30", | ||
| "px-2 py-1 font-sans text-[13px] leading-snug text-amber-50", | ||
| "outline-none focus:border-amber-300/60", | ||
| )} | ||
| aria-label="Edit goal objective" | ||
| /> | ||
| ) : ( | ||
| <button | ||
| type="button" | ||
| onClick={() => onEdit && setEditing(true)} | ||
| title={onEdit ? "Click to edit" : objective} | ||
| disabled={!onEdit} | ||
| className={cn( | ||
| "mt-1.5 block w-full text-left font-sans text-[13px] leading-snug text-amber-50", | ||
| onEdit && "cursor-text rounded hover:text-amber-100", | ||
| !onEdit && "cursor-default", | ||
| )} | ||
| > | ||
| {objective} | ||
| </button> | ||
| )} | ||
|
|
||
| <div className="mt-2 flex items-center gap-2"> | ||
| {tokenBudget ? ( | ||
| <div |
There was a problem hiding this comment.
onBlur={submitEdit} fires after Escape, ignoring the cancellation
cancelEdit queues setEditing(false) + setDraft(objective) via React's batched state updates. Those updates are not committed until after the keydown handler returns. When React commits and removes the textarea from the DOM, the browser fires blur, which calls submitEdit. At that point, the closure's draft still holds the user's modified text (the setDraft(objective) batch hasn't been applied yet), so next !== objective passes and onEdit is called — sending /goal set <modified text> even though the user pressed Escape to cancel. The same mechanism also causes a double /goal set when Enter is pressed (once from onKeyDown, once from the blur on unmount).
The simplest fix is to track intent with a useRef that is flipped synchronously before the async state update, and guard onBlur against calling submitEdit after a cancel.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/components/chat/codex/CodexGoalCard.tsx
Line: 162-190
Comment:
**`onBlur={submitEdit}` fires after Escape, ignoring the cancellation**
`cancelEdit` queues `setEditing(false)` + `setDraft(objective)` via React's batched state updates. Those updates are not committed until *after* the keydown handler returns. When React commits and removes the textarea from the DOM, the browser fires `blur`, which calls `submitEdit`. At that point, the closure's `draft` still holds the user's modified text (the `setDraft(objective)` batch hasn't been applied yet), so `next !== objective` passes and `onEdit` is called — sending `/goal set <modified text>` even though the user pressed Escape to cancel. The same mechanism also causes a double `/goal set` when Enter is pressed (once from `onKeyDown`, once from the blur on unmount).
The simplest fix is to track intent with a `useRef` that is flipped synchronously before the async state update, and guard `onBlur` against calling `submitEdit` after a cancel.
How can I resolve this? If you propose a fix, please make it concise.
1 participant
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
UI/Styling
Refactors
Greptile Summary
This PR introduces per-window isolation for the built-in browser service, a new
WorkSurfaceHeadershared component consumed by both chat and CLI session headers, and a rich typed-card timeline for Codex/Cursor subagent transcripts with a newCodexGoalCardfor tracking thread objectives.createBuiltInBrowserServicenow manages aMap<windowId, service>so each Electron window gets its own browser state; IPC handlers pass the resolvedBrowserWindowassourceWindow, and events are sent directly to the originating window's renderer instead of broadcasting.WorkSurfaceHeader: A new shared header component replaces the separate inline implementations inAgentChatPaneand the removedWorkCliSessionHeader; bothCliSessionWorkSurfaceHeaderandAgentChatPanenow compose it, with the stop-session action correctly forwarded throughCliSurfaceTrailingActions.agentChatServicemaps Codex thread items toAgentChatEvent-shaped transcript messages with deterministic UUIDs;AgentChatMessageListrenders them through a typed timeline;CodexGoalCardis surfaced inChatSubagentsPaneland replaces the in-chatCodexGoalBanner.Confidence Score: 4/5
Safe to merge after fixing the CodexGoalCard Escape/blur interaction bug.
The built-in browser refactor and WorkSurfaceHeader unification are well-structured. The one concrete defect is in CodexGoalCard: pressing Escape fires onBlur on the unmounting textarea before React commits the batched setDraft(objective) reset, so submitEdit runs with the stale modified draft and calls onEdit — the cancel does the opposite of what the user intended. The same mechanism causes a double /goal set dispatch on Enter.
apps/desktop/src/renderer/components/chat/codex/CodexGoalCard.tsx — the onBlur/onKeyDown interaction needs a guard to prevent submitEdit from firing after cancelEdit.
Important Files Changed
Sequence Diagram
sequenceDiagram participant R as Renderer (IPC) participant G as guardBuiltInBrowserIpc participant M as createBuiltInBrowserService (multi-window) participant W as WindowBrowserService (per-window) participant E as Electron BrowserWindow R->>G: IPC call (e.g. builtInBrowserNavigate) G->>G: validate sender URL + rate limit G-->>R: return BrowserWindow (win) R->>M: navigate(args, win) M->>M: serviceFor(win) → windowServices.get(win.id) alt window service exists M->>W: navigate(args) else new window M->>W: createBuiltInBrowserWindowService() M->>E: win.once("closed", closedListener) M->>W: navigate(args) end W-->>R: BuiltInBrowserStatus Note over M,E: on win "closed": delete from map, dispose serviceComments Outside Diff (3)
apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx, line 606-626 (link)SessionSurfacestill acceptsonStopRunningSessionandstoppingin its prop interface (lines 582–583), and callers at lines 1370–1371 and 1726–1727 pass them in, but neither prop is forwarded toCliSessionWorkSurfaceHeader. The oldWorkCliSessionHeaderrendered aStopCirclebutton wheneversession.status === "running" && Boolean(session.ptyId) && Boolean(onStopRunningSession)— that affordance is now gone. Users running a Claude Code, Codex CLI, or Cursor CLI session have no header-level way to terminate it; the only stop path left is the context menu (kebab), which is not as discoverable. The two props inSessionSurfaceare effectively dead code at this point.Prompt To Fix With AI
apps/desktop/src/main/services/chat/agentChatService.ts, line 213-219 (link)idWhen a Codex thread item has no
id(or an empty string),randomUUID()is called to generate a fallback. BecausegetSubagentTranscriptcan be called multiple times for the same session (e.g. on reopen or scroll-back), items that lack server-assigned IDs will receive a different UUID on every fetch. Downstream consumers key onuuidto deduplicate or reconcile transcript entries, so two fetches of the same itemless item would appear as two distinct transcript messages. Replacing the fallback with a stable hash or composite key (e.g.codex-thread-item:${threadId}:${itemIndex}) would make the fallback deterministic without requiring the server to provide an id.Prompt To Fix With AI
apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx, line 568-583 (link)onStopRunningSession/stoppingare dead props inSessionSurfaceBoth props are still declared in
SessionSurface's interface (lines 582-583), destructured in its body (lines 568-569), and passed by every caller in this file (e.g. lines 1372-1373 and 1729-1730), but the switch toCliSessionWorkSurfaceHeaderdropped the forwarding — the new header accepts neither. The oldWorkCliSessionHeaderrendered aStopCirclebutton wheneversession.status === "running"+onStopRunningSessionwas provided; that affordance is now gone. Users running a Claude Code, Codex CLI, or Cursor CLI session have no header-level way to stop it. Consider either forwarding the props toCliSessionWorkSurfaceHeader/CliSurfaceTrailingActions, or removing them fromSessionSurface's interface if the stop action is intentionally retired.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (4): Last reviewed commit: "ship: iteration 1 — address CodeRabbit r..." | Re-trigger Greptile