Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions docs/learnings/2026-04-27-fleet-bridge-handler-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Fleet bridge handler return type and lint

## Symptom

While adding the Pi plan-open bridge command, changing `FleetBridgeServer`'s `RequestHandler` from `Promise<unknown>` to `unknown | Promise<unknown>` made lint report:

- `@typescript-eslint/no-redundant-type-constituents` because `unknown` absorbs the union
- `@typescript-eslint/await-thenable` at the call site that awaited the handler result

## Fix

Keep `RequestHandler` as `Promise<unknown>` and keep bridge request handlers `async`. If a handler is currently synchronous, use a real async boundary rather than changing the framework type.

## Lesson

Do not use `unknown | Promise<unknown>` for async callback return types in this codebase. It looks flexible, but lint treats `unknown` as overriding the promise branch and then flags awaited values as non-thenable.
21 changes: 21 additions & 0 deletions docs/learnings/2026-04-28-pi-plan-modal-disconnect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Pi plan modal bridge disconnect handling

## Symptom

The Pi plan modal response flow initially assumed that once `pi.plan_open` returned, the Fleet bridge would stay connected until the user clicked Approve/Reject/Continue. If the WebSocket disconnected after opening the modal but before the response, `exit_plan_mode` could wait forever. After adding delivery failure checks, the modal could also become effectively unclosable because every dismissal path tried to send `continue` through the disconnected bridge.

## Fix

- Add a disconnect callback to the Fleet Pi bridge extension client.
- Have pending `exit_plan_mode` response waiters resolve as `continue` on bridge disconnect, matching abort behavior.
- Make renderer-side response delivery failures visible but non-blocking by showing an error with a direct Dismiss escape hatch.
- When invoking disconnect handlers, iterate over a copy so handlers can unsubscribe themselves without skipping later handlers.

## Lesson

For two-way UI flows over an agent bridge, handle all lifecycle edges explicitly:

1. Request opened but response never arrives.
2. Response send fails after user action.
3. User needs a local dismiss path even if the agent can no longer be notified.
4. Event handler arrays must tolerate handlers unregistering during dispatch.
12 changes: 12 additions & 0 deletions resources/pi-extensions/fleet-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const MAX_RECONNECT_ATTEMPTS = 10;
export type BridgeClient = {
send: (type: string, payload: Record<string, unknown>) => Promise<unknown>;
onEvent: (handler: (type: string, payload: Record<string, unknown>) => void) => void;
onDisconnect: (handler: () => void) => () => void;
isConnected: () => boolean;
};

Expand All @@ -37,6 +38,7 @@ export default function (_pi: ExtensionAPI): void {
let requestId = 0;
const pending = new Map<string, { resolve: (v: unknown) => void; reject: (e: Error) => void }>();
const eventHandlers: Array<(type: string, payload: Record<string, unknown>) => void> = [];
const disconnectHandlers: Array<() => void> = [];

function connect(): void {
try {
Expand Down Expand Up @@ -73,6 +75,9 @@ export default function (_pi: ExtensionAPI): void {

ws.onclose = () => {
ws = null;
for (const handler of [...disconnectHandlers]) {
handler();
}
if (reconnectAttempts < MAX_RECONNECT_ATTEMPTS) {
reconnectAttempts++;
setTimeout(connect, RECONNECT_DELAY_MS);
Expand Down Expand Up @@ -109,6 +114,13 @@ export default function (_pi: ExtensionAPI): void {
onEvent(handler) {
eventHandlers.push(handler);
},
onDisconnect(handler) {
disconnectHandlers.push(handler);
return () => {
const index = disconnectHandlers.indexOf(handler);
if (index >= 0) disconnectHandlers.splice(index, 1);
};
},
isConnected() {
return ws !== null && ws.readyState === WebSocket.OPEN;
}
Expand Down
13 changes: 2 additions & 11 deletions resources/pi-extensions/fleet-plan-mode-policy.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
const PLAN_PREVIEW_LINES = 60;

const BLOCKED_IN_PLAN = new Set<string>(['write', 'edit', 'bash', 'fleet_run']);
const REQUIRED_PLAN_MODE_TOOLS = ['read', 'grep', 'find', 'ls', 'fleet_open', 'exit_plan_mode'];

Expand All @@ -15,13 +13,6 @@ export function getPlanModeActiveTools(currentActiveTools: string[]): string[] {
return next;
}

function previewPlan(plan: string): string {
const lines = plan.split('\n');
if (lines.length <= PLAN_PREVIEW_LINES) return plan;
const remaining = lines.length - PLAN_PREVIEW_LINES;
return `${lines.slice(0, PLAN_PREVIEW_LINES).join('\n')}\n\n(${remaining} more lines)`;
}

export function buildPlanApprovalMessage(planPath: string, plan: string): string {
return `Path: ${planPath}\n\n---\n\n${previewPlan(plan)}`;
export function buildPlanApprovalMessage(planPath: string): string {
return `Plan written to:\n${planPath}\n\nReview it in the Fleet plan modal, then approve when ready.`;
}
130 changes: 116 additions & 14 deletions resources/pi-extensions/fleet-plan-mode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* swapped to hide write/exec tools via pi.setActiveTools, with a
* tool_call blocker as a final policy gate. The LLM
* produces a markdown plan via the exit_plan_mode tool; the plan is
* written to docs/plans/YYYY-MM-DD-<topic>.md after the user approves.
* written to docs/plans/YYYY-MM-DD-<topic>.md and opened in Fleet's plan modal.
*
* Also registers pi's built-in grep/find/ls tools, which are not in
* the default toolset.
Expand All @@ -20,6 +20,7 @@ import {
type ExtensionContext
} from '@mariozechner/pi-coding-agent';
import { Type } from '@sinclair/typebox';
import { randomUUID } from 'node:crypto';
import { existsSync, mkdirSync, writeFileSync } from 'node:fs';
import { join } from 'node:path';
import {
Expand All @@ -28,6 +29,17 @@ import {
shouldBlockPlanModeTool
} from './fleet-plan-mode-policy.ts';

type FleetBridgeClient = {
send: (type: string, payload: Record<string, unknown>) => Promise<unknown>;
onEvent: (handler: (type: string, payload: Record<string, unknown>) => void) => void;
onDisconnect: (handler: () => void) => () => void;
isConnected: () => boolean;
};

declare global {
var __fleetBridge: FleetBridgeClient | null; // eslint-disable-line no-var
}

const PLAN_MODE_STATUS_KEY = 'plan-mode';
const PLAN_MODE_STATUS_LABEL = '📋 Plan Mode';

Expand All @@ -49,13 +61,65 @@ You are in plan mode. Only read-only tools are available (write, edit, bash, fle

7. YAGNI. Plan only what's asked. No speculative features, flags, or abstractions.

When you have enough that another engineer could execute without asking questions, call exit_plan_mode.`;
When you have enough that another engineer could execute without asking questions, call exit_plan_mode. The plan will be written to a markdown file and opened in Fleet for review; do not include the full plan in a normal assistant message.`;

const PLAN_MODE_BLOCK_REASON =
'Plan mode is active — this tool is disabled. Use read-only tools to investigate, then call exit_plan_mode with your plan.';

const TOPIC_PATTERN = /^[a-z0-9][a-z0-9-]*$/;

type PlanReviewAction = 'approve' | 'reject' | 'continue';

type PlanReviewResponse = {
action: PlanReviewAction;
feedback?: string;
};

type PendingPlanResponse = {
resolve: (response: PlanReviewResponse) => void;
};

const pendingPlanResponses = new Map<string, PendingPlanResponse>();

function isPlanReviewAction(value: unknown): value is PlanReviewAction {
return value === 'approve' || value === 'reject' || value === 'continue';
}

function createPlanResponseWaiter(
requestId: string,
signal: AbortSignal | undefined,
bridge: FleetBridgeClient
) {
let abortHandler: (() => void) | undefined;
let unsubscribeDisconnect: (() => void) | undefined;
let cleanup = () => {
pendingPlanResponses.delete(requestId);
};

const promise = new Promise<PlanReviewResponse>((resolve) => {
cleanup = () => {
pendingPlanResponses.delete(requestId);
if (abortHandler) signal?.removeEventListener('abort', abortHandler);
unsubscribeDisconnect?.();
};
const finish = (response: PlanReviewResponse) => {
cleanup();
resolve(response);
};
abortHandler = () => finish({ action: 'continue' });
unsubscribeDisconnect = bridge.onDisconnect(() => finish({ action: 'continue' }));

pendingPlanResponses.set(requestId, { resolve: finish });
if (signal?.aborted || !bridge.isConnected()) {
abortHandler();
return;
}
signal?.addEventListener('abort', abortHandler, { once: true });
});

return { promise, cancel: cleanup };
}

const ExitPlanModeParams = Type.Object({
plan: Type.String({
description:
Expand Down Expand Up @@ -95,6 +159,18 @@ export default function (pi: ExtensionAPI): void {
pi.registerTool(createFindToolDefinition(cwd));
pi.registerTool(createLsToolDefinition(cwd));

globalThis.__fleetBridge?.onEvent((type, payload) => {
if (type !== 'pi.plan_response') return;
const requestId = typeof payload.requestId === 'string' ? payload.requestId : '';
const action = payload.action;
if (!requestId || !isPlanReviewAction(action)) return;

pendingPlanResponses.get(requestId)?.resolve({
action,
feedback: typeof payload.feedback === 'string' ? payload.feedback : undefined
});
});

function enterPlanMode(ctx: ExtensionContext): void {
const activeTools = pi.getActiveTools();
savedActiveTools = activeTools;
Expand Down Expand Up @@ -151,6 +227,10 @@ export default function (pi: ExtensionAPI): void {
});

pi.on('session_shutdown', async (_event, ctx) => {
for (const pending of pendingPlanResponses.values()) {
pending.resolve({ action: 'continue' });
}
pendingPlanResponses.clear();
if (planMode || savedActiveTools) leavePlanMode(ctx);
});

Expand All @@ -171,10 +251,10 @@ export default function (pi: ExtensionAPI): void {
name: 'exit_plan_mode',
label: 'Exit Plan Mode',
description:
'Call this when you have a complete plan ready for the user. Writes the plan to docs/plans/YYYY-MM-DD-<topic>.md after the user approves it, then exits plan mode so you can begin executing. Pass the plan as markdown in `plan` and a short kebab-case topic in `topic`.',
'Call this when you have a complete plan ready for the user. Writes the plan to docs/plans/YYYY-MM-DD-<topic>.md, opens it in Fleet for review, then exits plan mode after approval so you can begin executing. Pass the plan as markdown in `plan` and a short kebab-case topic in `topic`.',
parameters: ExitPlanModeParams,

async execute(_toolCallId, params, _signal, _onUpdate, ctx) {
async execute(_toolCallId, params, signal, _onUpdate, ctx) {
if (!planMode) {
return {
content: [
Expand All @@ -200,27 +280,49 @@ export default function (pi: ExtensionAPI): void {
}

const planPath = resolvePlanPath(ctx.cwd, params.topic);
const approved = await ctx.ui.confirm(
'Approve plan?',
buildPlanApprovalMessage(planPath, params.plan)
);
const dir = join(ctx.cwd, 'docs', 'plans');
if (!existsSync(dir)) mkdirSync(dir, { recursive: true });
writeFileSync(planPath, params.plan, 'utf-8');

let review: PlanReviewResponse | null = null;
const bridge = globalThis.__fleetBridge;
if (bridge?.isConnected()) {
const requestId = randomUUID();
const responseWaiter = createPlanResponseWaiter(requestId, signal, bridge);
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');
}
Comment on lines +289 to +304
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


if (!review) {
const approved = await ctx.ui.confirm('Approve plan?', buildPlanApprovalMessage(planPath));
review = { action: approved ? 'approve' : 'reject' };
}

if (!approved) {
if (review.action !== 'approve') {
const feedback = review.feedback?.trim();
const actionText =
review.action === 'reject' ? 'rejected the plan' : 'requested more planning';
return {
content: [
{
type: 'text' as const,
text: 'User rejected the plan. Revise based on their feedback and call exit_plan_mode again when ready.'
text: `User ${actionText} for ${planPath}.${feedback ? ` Feedback: ${feedback}` : ''} Revise based on their feedback and call exit_plan_mode again when ready.`
}
],
details: undefined
};
}

const dir = join(ctx.cwd, 'docs', 'plans');
if (!existsSync(dir)) mkdirSync(dir, { recursive: true });
writeFileSync(planPath, params.plan, 'utf-8');

leavePlanMode(ctx);

return {
Expand Down
17 changes: 17 additions & 0 deletions src/main/__tests__/fleet-cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,23 @@ describe('--help via runCLI', () => {
});
});

// ── fleet pi plan_open tests ─────────────────────────────────────────────────

describe('fleet pi plan_open', () => {
it('requires a path before opening the socket', async () => {
const out = await runCLI(['pi', 'plan_open'], '/tmp/no-socket.sock');
expect(out).toBe('Usage: fleet pi plan_open <path>');
});

it('validates the plan file exists before opening the socket', async () => {
const out = await runCLI(
['pi', 'plan_open', '/tmp/fleet-missing-plan.md'],
'/tmp/no-socket.sock'
);
expect(out).toContain('file not found');
});
});

// ── FleetCLI.send basic tests ─────────────────────────────────────────────────

describe('FleetCLI.send', () => {
Expand Down
13 changes: 5 additions & 8 deletions src/main/__tests__/fleet-plan-mode-extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,11 @@ describe('fleet plan mode extension helpers', () => {
expect(shouldBlockPlanModeTool('read')).toBe(false);
});

it('builds approval text with target path and a 60-line plan preview', () => {
const plan = Array.from({ length: 62 }, (_, index) => `Line ${index + 1}`).join('\n');
const message = buildPlanApprovalMessage('/repo/docs/plans/2026-04-25-demo.md', plan);
it('builds approval text with target path but no plan body', () => {
const message = buildPlanApprovalMessage('/repo/docs/plans/2026-04-25-demo.md');

expect(message).toContain('Path: /repo/docs/plans/2026-04-25-demo.md');
expect(message).toContain('Line 1');
expect(message).toContain('Line 60');
expect(message).not.toContain('Line 61');
expect(message).toContain('(2 more lines)');
expect(message).toContain('/repo/docs/plans/2026-04-25-demo.md');
expect(message).toContain('Review it in the Fleet plan modal');
expect(message).not.toContain('Line 1');
});
});
8 changes: 4 additions & 4 deletions src/main/fleet-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,11 @@ export class FleetBridgeServer {
}
}

sendEvent(paneId: string, event: BridgeEvent): void {
sendEvent(paneId: string, event: BridgeEvent): boolean {
const ws = this.connections.get(paneId);
if (ws && ws.readyState === ws.OPEN) {
ws.send(JSON.stringify(event));
}
if (!ws || ws.readyState !== ws.OPEN) return false;
ws.send(JSON.stringify(event));
return true;
}

broadcast(event: BridgeEvent): void {
Expand Down
Loading
Loading