feat(pi): open plan mode docs in Fleet modal#201
Conversation
khang859
left a comment
There was a problem hiding this comment.
Code review findings:
-
High — concurrent plan modals can strand an agent forever
src/renderer/src/App.tsxstores only one globalplanModaland replaces it on everypi.plan_openevent:window.fleet.pi.onPlanOpen((payload) => { setPlanModal(payload); });
Each Pi agent then waits on its own
requestIdinresources/pi-extensions/fleet-plan-mode.ts:await bridge.send('pi.plan_open', { path: planPath, requestId }); review = await responseWaiter.promise;
If two Pi panes enter plan review at the same time, the second event replaces the first modal. The first request is no longer reachable from the UI, so that agent can wait indefinitely. This is especially relevant because Fleet’s core use case is running multiple agents simultaneously.
Suggested fix: queue plan-review requests, render multiple modals, or explicitly send a
continue/cancel response to the currently displayed request before replacing it. -
Medium — approval/rejection responses can be silently dropped
src/main/ipc-handlers.tsalways resolvesPI_PLAN_RESPONDafter calling:fleetBridge.sendEvent(req.paneId, { ... });
But
FleetBridgeServer.sendEventis a no-op if the WebSocket is missing or not open:const ws = this.connections.get(paneId); if (ws && ws.readyState === ws.OPEN) { ws.send(JSON.stringify(event)); }
The renderer closes the modal in
finally, so if the bridge connection is gone or reconnecting, the user thinks they approved/rejected but Pi never receives the response and continues waiting.Suggested fix: make
sendEventreturn success/failure, throw fromPI_PLAN_RESPONDwhen delivery fails, and keep the modal open with an error/retry path. -
Low/Medium — reopening the same plan path while the modal is mounted won’t refresh content
PiPlanModalkeysMarkdownPaneonly byfilePath:<MarkdownPane key={filePath} paneId={paneIdRef.current} filePath={filePath} />
MarkdownPaneonly reloads whenfilePathchanges. If the same path is opened again while the modal remains mounted (for example after the file changed, or two events for the same path), the modal can show stale content.Suggested fix: include
requestIdor an open counter in the key/dependency, e.g. key by${filePath}:${plan.requestId ?? openSequence}.
Verification run locally:
npm run typecheck✅npm test -- --run src/main/__tests__/fleet-plan-mode-extension.test.ts src/main/__tests__/fleet-cli.test.ts✅
khang859
left a comment
There was a problem hiding this comment.
Code review
Found 2 issues related to bridge-disconnect handling around the new plan-modal flow. Checked for bugs and CLAUDE.md compliance.
| try { | ||
| await window.fleet.pi.respondToPlan({ | ||
| paneId: plan.paneId, | ||
| requestId: plan.requestId, | ||
| action, | ||
| feedback: feedback.trim() || undefined | ||
| }); | ||
| onClose(); | ||
| } catch (err) { | ||
| setSubmitError(err instanceof Error ? err.message : String(err)); | ||
| } finally { | ||
| setSubmittingAction(null); | ||
| } |
There was a problem hiding this comment.
Modal becomes unclosable when the Pi bridge disconnects mid-review.
When canRespond is true, every dismissal path — the X button (L125), ESC (L71-L79), and the three action buttons — funnels through respond(), which calls window.fleet.pi.respondToPlan(...). The IPC handler throws when fleetBridge.sendEvent returns false (see ipc-handlers.ts PI_PLAN_RESPOND and fleet-bridge.ts sendEvent returns false when the WebSocket is missing or not OPEN). The catch block here only sets submitError and never calls onClose(), so once the bridge has dropped, every retry shows the same error and the user is stuck behind a modal that covers the entire app. There is no backdrop click handler either.
Fix at a high level: surface a fallback dismiss path when delivery fails (e.g., a "Dismiss" button that calls onClose() directly when submitError is set, or auto-close on IPC failure with a toast).
Fix src/renderer/src/components/PiPlanModal.tsx:46-58: when respondToPlan throws (bridge disconnected), the modal cannot be dismissed. Add an escape hatch — e.g., render a "Dismiss" button next to the error banner that calls onClose() directly, so the user can close the modal even when the bridge is down. Verify with both bridge-connected and bridge-disconnected cases.
| if (bridge?.isConnected()) { | ||
| const requestId = randomUUID(); | ||
| const responseWaiter = createPlanResponseWaiter(requestId, signal); | ||
| try { | ||
| await bridge.send('pi.plan_open', { path: planPath, requestId }); | ||
| review = await responseWaiter.promise; | ||
| } catch (err) { | ||
| responseWaiter.cancel(); | ||
| ctx.ui.notify( | ||
| `Plan written, but Fleet could not open it: ${err instanceof Error ? err.message : String(err)}`, | ||
| 'warning' | ||
| ); | ||
| } | ||
| } else { | ||
| ctx.ui.notify('Plan written, but Fleet bridge is not connected.', 'warning'); | ||
| } |
There was a problem hiding this comment.
exit_plan_mode hangs forever if the Fleet bridge disconnects between bridge.send resolving and pi.plan_response arriving.
createPlanResponseWaiter (L87-L113) only resolves on (a) a matching pi.plan_response event, or (b) the tool-call signal aborting. The FleetBridgeClient type (L32-L36) exposes no disconnect/close event, and the try/catch wraps only bridge.send — once it returns, control passes to await responseWaiter.promise. If the WS closes after that point (Fleet quits, network drop, etc.), no exception is thrown into the catch and pendingPlanResponses keeps the entry forever. session_shutdown (L221-L227) only fires on the Pi session ending, not on bridge disconnect, so the tool call hangs indefinitely.
Fix resources/pi-extensions/fleet-plan-mode.ts:281-296: bridge mid-review disconnect leaves exit_plan_mode hanging forever. Add a disconnect path — e.g., expose an onDisconnect/onClose hook on FleetBridgeClient (and wire it from the main process), and have createPlanResponseWaiter resolve to { action: 'continue' } when disconnect fires, mirroring the existing abort-signal handler. Optionally add a long timeout as a safety net. Verify by killing the WebSocket between bridge.send and the user clicking Approve/Reject.
Summary
fleet pi plan_open <path>plus bridge/IPC plumbingVerification
npm run typechecknpm test -- --run src/main/__tests__/fleet-plan-mode-extension.test.ts src/main/__tests__/fleet-cli.test.ts