From 814ca0e85e6046575eee2598c4c4362593ed5623 Mon Sep 17 00:00:00 2001 From: Apinant U-suwantim Date: Sun, 30 Aug 2026 12:28:46 +0700 Subject: [PATCH 1/3] fix(pi-extension): start a turn for annotate feedback on idle oh-my-pi oh-my-pi honors an explicit deliverAs even when idle, so { deliverAs: "followUp" } parks annotate/review feedback as a pending follow-up and never starts a turn. The browser reports "Feedback Sent" while the agent never sees it. Pi documents deliverAs as the streaming queue selector and starts a turn when idle. Both hosts expose isIdle(), so drop only deliverAs while idle and keep every other caller option. Hosts without the probe, or a probe that throws, keep the caller's options unchanged. Verified RED/GREEN by neutering the helper: 4 idle-drop cases fail, then 11/11 pass with the helper restored. --- apps/pi-extension/current-pi-session.test.ts | 115 +++++++++++++++++++ apps/pi-extension/current-pi-session.ts | 38 +++++- apps/pi-extension/index.ts | 3 +- 3 files changed, 154 insertions(+), 2 deletions(-) diff --git a/apps/pi-extension/current-pi-session.test.ts b/apps/pi-extension/current-pi-session.test.ts index afccd0a2b..379e29209 100644 --- a/apps/pi-extension/current-pi-session.test.ts +++ b/apps/pi-extension/current-pi-session.test.ts @@ -5,6 +5,7 @@ import { notifyCurrentPiSession, registerCurrentPiSession, sendUserMessageToCurrentPiSession, + resolveIdleDeliveryOptions, type CurrentPiSessionRegistration, } from "./current-pi-session.ts"; @@ -68,3 +69,117 @@ describe("current Pi session feedback routing", () => { expect(replacementNotifications).toEqual(["feedback delivered"]); }); }); + +describe("resolveIdleDeliveryOptions", () => { + test("idle host drops deliverAs so the host starts a turn", () => { + expect( + resolveIdleDeliveryOptions({ isIdle: () => true }, { deliverAs: "followUp" }), + ).toBeUndefined(); + }); + + test("streaming host keeps deliverAs", () => { + expect( + resolveIdleDeliveryOptions({ isIdle: () => false }, { deliverAs: "followUp" }), + ).toEqual({ deliverAs: "followUp" }); + }); + + test("idle host keeps sibling options and drops only deliverAs", () => { + expect( + resolveIdleDeliveryOptions( + { isIdle: () => true }, + { deliverAs: "followUp", expandPromptTemplates: true }, + ), + ).toEqual({ expandPromptTemplates: true }); + }); + + test("host without isIdle passes options through unchanged", () => { + expect(resolveIdleDeliveryOptions({}, { deliverAs: "followUp" })).toEqual({ + deliverAs: "followUp", + }); + }); + + test("probe that throws passes options through unchanged", () => { + expect( + resolveIdleDeliveryOptions( + { + isIdle: () => { + throw new Error("probe exploded"); + }, + }, + { deliverAs: "followUp" }, + ), + ).toEqual({ deliverAs: "followUp" }); + }); + + test("undefined options stay undefined", () => { + expect(resolveIdleDeliveryOptions({ isIdle: () => true })).toBeUndefined(); + }); + + test("options without deliverAs are untouched even when idle", () => { + expect( + resolveIdleDeliveryOptions({ isIdle: () => true }, { expandPromptTemplates: true }), + ).toEqual({ expandPromptTemplates: true }); + }); + + test("steer is also dropped when idle", () => { + expect( + resolveIdleDeliveryOptions({ isIdle: () => true }, { deliverAs: "steer" }), + ).toBeUndefined(); + }); +}); + +describe("idle delivery through the current-session send path", () => { + test("idle replacement runtime drops deliverAs so the host starts a turn", () => { + const oldRuntime = registerSessionRuntime("same-session", [], []); + const origin = getPiSessionIdentity(oldRuntime.ctx); + + const captured: Array<{ content: unknown; options: unknown }> = []; + const idlePi = { + isIdle: () => true, + sendUserMessage: (content: unknown, options?: unknown) => { + captured.push({ content, options }); + }, + } as unknown as ExtensionAPI; + const replacement = registerCurrentPiSession(idlePi); + registrations.push(replacement); + replacement.update(createContext("same-session", [])); + oldRuntime.registration.clear(); + + const result = sendUserMessageToCurrentPiSession( + "annotation feedback", + { deliverAs: "followUp" }, + origin, + ); + + expect(result).toEqual({ ok: true }); + expect(captured).toEqual([{ content: "annotation feedback", options: undefined }]); + }); + + test("streaming replacement runtime keeps deliverAs", () => { + const oldRuntime = registerSessionRuntime("same-session", [], []); + const origin = getPiSessionIdentity(oldRuntime.ctx); + + const captured: Array<{ content: unknown; options: unknown }> = []; + const streamingPi = { + isIdle: () => false, + sendUserMessage: (content: unknown, options?: unknown) => { + captured.push({ content, options }); + }, + } as unknown as ExtensionAPI; + const replacement = registerCurrentPiSession(streamingPi); + registrations.push(replacement); + replacement.update(createContext("same-session", [])); + oldRuntime.registration.clear(); + + const result = sendUserMessageToCurrentPiSession( + "annotation feedback", + { deliverAs: "followUp" }, + origin, + ); + + expect(result).toEqual({ ok: true }); + expect(captured).toEqual([ + { content: "annotation feedback", options: { deliverAs: "followUp" } }, + ]); + }); +}); diff --git a/apps/pi-extension/current-pi-session.ts b/apps/pi-extension/current-pi-session.ts index a548b730b..840b5334e 100644 --- a/apps/pi-extension/current-pi-session.ts +++ b/apps/pi-extension/current-pi-session.ts @@ -4,6 +4,42 @@ type SendUserMessageContent = Parameters[0]; type SendUserMessageOptions = Parameters[1]; type NotificationType = "info" | "warning" | "error"; +type IdleProbeHost = { isIdle?: () => boolean }; + +/** + * Pass `deliverAs` only while the agent is streaming. + * + * Pi documents `deliverAs` as the streaming queue selector ("When the agent is + * streaming, use deliverAs to specify how to queue the message") and starts a + * turn when the session is idle. oh-my-pi instead honors an explicit + * `deliverAs` unconditionally — its contract reads "idle starts a turn; + * streaming queues as steer unless deliverAs is set" — so + * `{ deliverAs: "followUp" }` on an IDLE oh-my-pi session parks the prompt as + * a pending follow-up and never starts a turn. Annotate/review feedback is + * then persisted and acknowledged in the browser ("Feedback Sent") while the + * agent never sees it. + * + * Both hosts expose `isIdle()` on the extension API, so drop only `deliverAs` + * while idle and keep every other caller option. Hosts predating the + * capability, or a probe that throws, keep the caller's options unchanged. + */ +export function resolveIdleDeliveryOptions( + host: IdleProbeHost, + options?: SendUserMessageOptions, +): SendUserMessageOptions | undefined { + if (!options || options.deliverAs === undefined) return options; + const probe = host.isIdle; + if (typeof probe !== "function") return options; + try { + if (!probe.call(host)) return options; + } catch { + return options; + } + const next = { ...options }; + delete next.deliverAs; + return Object.keys(next).length > 0 ? next : undefined; +} + type CurrentPiSession = { token: symbol; sendUserMessage: (content: SendUserMessageContent, options?: SendUserMessageOptions) => void; @@ -96,7 +132,7 @@ function setCurrentPiSession(token: symbol, pi: ExtensionAPI, ctx?: ExtensionCon const current: CurrentPiSession = { token, sendUserMessage: (content, options) => { - pi.sendUserMessage(content, options); + pi.sendUserMessage(content, resolveIdleDeliveryOptions(pi, options)); }, }; if (ctx) { diff --git a/apps/pi-extension/index.ts b/apps/pi-extension/index.ts index 6b36649e7..a5df7360b 100644 --- a/apps/pi-extension/index.ts +++ b/apps/pi-extension/index.ts @@ -61,6 +61,7 @@ import { notifyCurrentPiSession, type PiSessionIdentity, registerCurrentPiSession, + resolveIdleDeliveryOptions, sendUserMessageToCurrentPiSession, withCurrentPiSessionFallbackHeader, } from "./current-pi-session.ts"; @@ -272,7 +273,7 @@ function sendUserMessageWithCurrentSessionFallback( if (trySendUserMessageToDifferentCurrentSession(content, options, errorMessage, origin)) return; try { - pi.sendUserMessage(content, options); + pi.sendUserMessage(content, resolveIdleDeliveryOptions(pi, options)); return; } catch (err) { if (trySendUserMessageToDifferentCurrentSession(content, options, errorMessage, origin)) return; From 5a6be1652dc28ffe98be04fa8d4698dc4f8a6a0b Mon Sep 17 00:00:00 2001 From: Apinant U-suwantim Date: Sun, 30 Aug 2026 12:33:43 +0700 Subject: [PATCH 2/3] fix(pi-extension): probe ctx.isIdle, not the ExtensionAPI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isIdle lives on ExtensionContext (ctx.isIdle), not ExtensionAPI. The previous commit probed `pi`, so the real oh-my-pi path never saw a probe and kept deliverAs on idle sessions — the original bug. Pass the updated ctx into the stored-session send and the same-session fallback. A regression test keeps isIdle:true on the API object and isIdle:false on ctx; deliverAs must survive. --- apps/pi-extension/current-pi-session.test.ts | 45 ++++++++++++++++---- apps/pi-extension/current-pi-session.ts | 8 ++-- apps/pi-extension/index.ts | 7 ++- 3 files changed, 46 insertions(+), 14 deletions(-) diff --git a/apps/pi-extension/current-pi-session.test.ts b/apps/pi-extension/current-pi-session.test.ts index 379e29209..9ffc3430b 100644 --- a/apps/pi-extension/current-pi-session.test.ts +++ b/apps/pi-extension/current-pi-session.test.ts @@ -15,7 +15,7 @@ afterEach(() => { for (const registration of registrations.splice(0)) registration.clear(); }); -function createContext(sessionId: string, notifications: string[]): ExtensionContext { +function createContext(sessionId: string, notifications: string[], idle = true): ExtensionContext { return { cwd: "/tmp", mode: "tui", @@ -25,6 +25,7 @@ function createContext(sessionId: string, notifications: string[]): ExtensionCon getSessionName: () => sessionId, }, ui: { notify: (message: string) => notifications.push(message) }, + isIdle: () => idle, } as unknown as ExtensionContext; } @@ -134,15 +135,14 @@ describe("idle delivery through the current-session send path", () => { const origin = getPiSessionIdentity(oldRuntime.ctx); const captured: Array<{ content: unknown; options: unknown }> = []; - const idlePi = { - isIdle: () => true, + const replacementPi = { sendUserMessage: (content: unknown, options?: unknown) => { captured.push({ content, options }); }, } as unknown as ExtensionAPI; - const replacement = registerCurrentPiSession(idlePi); + const replacement = registerCurrentPiSession(replacementPi); registrations.push(replacement); - replacement.update(createContext("same-session", [])); + replacement.update(createContext("same-session", [], true)); oldRuntime.registration.clear(); const result = sendUserMessageToCurrentPiSession( @@ -160,15 +160,42 @@ describe("idle delivery through the current-session send path", () => { const origin = getPiSessionIdentity(oldRuntime.ctx); const captured: Array<{ content: unknown; options: unknown }> = []; - const streamingPi = { - isIdle: () => false, + const replacementPi = { + sendUserMessage: (content: unknown, options?: unknown) => { + captured.push({ content, options }); + }, + } as unknown as ExtensionAPI; + const replacement = registerCurrentPiSession(replacementPi); + registrations.push(replacement); + replacement.update(createContext("same-session", [], false)); + oldRuntime.registration.clear(); + + const result = sendUserMessageToCurrentPiSession( + "annotation feedback", + { deliverAs: "followUp" }, + origin, + ); + + expect(result).toEqual({ ok: true }); + expect(captured).toEqual([ + { content: "annotation feedback", options: { deliverAs: "followUp" } }, + ]); + }); + + test("isIdle on ExtensionAPI is ignored; the context is the probe", () => { + const oldRuntime = registerSessionRuntime("same-session", [], []); + const origin = getPiSessionIdentity(oldRuntime.ctx); + + const captured: Array<{ content: unknown; options: unknown }> = []; + const lyingPi = { + isIdle: () => true, sendUserMessage: (content: unknown, options?: unknown) => { captured.push({ content, options }); }, } as unknown as ExtensionAPI; - const replacement = registerCurrentPiSession(streamingPi); + const replacement = registerCurrentPiSession(lyingPi); registrations.push(replacement); - replacement.update(createContext("same-session", [])); + replacement.update(createContext("same-session", [], false)); oldRuntime.registration.clear(); const result = sendUserMessageToCurrentPiSession( diff --git a/apps/pi-extension/current-pi-session.ts b/apps/pi-extension/current-pi-session.ts index 840b5334e..6a4abef61 100644 --- a/apps/pi-extension/current-pi-session.ts +++ b/apps/pi-extension/current-pi-session.ts @@ -19,9 +19,9 @@ type IdleProbeHost = { isIdle?: () => boolean }; * then persisted and acknowledged in the browser ("Feedback Sent") while the * agent never sees it. * - * Both hosts expose `isIdle()` on the extension API, so drop only `deliverAs` - * while idle and keep every other caller option. Hosts predating the - * capability, or a probe that throws, keep the caller's options unchanged. + * Both hosts expose `isIdle()` on the extension context (`ctx.isIdle`), so + * drop only `deliverAs` while idle and keep every other caller option. Hosts + * predating the capability, or a probe that throws, keep the caller's options unchanged. */ export function resolveIdleDeliveryOptions( host: IdleProbeHost, @@ -132,7 +132,7 @@ function setCurrentPiSession(token: symbol, pi: ExtensionAPI, ctx?: ExtensionCon const current: CurrentPiSession = { token, sendUserMessage: (content, options) => { - pi.sendUserMessage(content, resolveIdleDeliveryOptions(pi, options)); + pi.sendUserMessage(content, resolveIdleDeliveryOptions(ctx ?? {}, options)); }, }; if (ctx) { diff --git a/apps/pi-extension/index.ts b/apps/pi-extension/index.ts index a5df7360b..badafe8f6 100644 --- a/apps/pi-extension/index.ts +++ b/apps/pi-extension/index.ts @@ -269,11 +269,12 @@ function sendUserMessageWithCurrentSessionFallback( options: Parameters[1], errorMessage: string, origin: PiSessionIdentity, + ctx?: ExtensionContext, ): void { if (trySendUserMessageToDifferentCurrentSession(content, options, errorMessage, origin)) return; try { - pi.sendUserMessage(content, resolveIdleDeliveryOptions(pi, options)); + pi.sendUserMessage(content, resolveIdleDeliveryOptions(ctx ?? {}, options)); return; } catch (err) { if (trySendUserMessageToDifferentCurrentSession(content, options, errorMessage, origin)) return; @@ -674,6 +675,7 @@ export default function plannotator(pi: ExtensionAPI): void { { deliverAs: "followUp" }, "Plannotator code review feedback could not be sent", origin, + ctx, ); return; } @@ -697,6 +699,7 @@ export default function plannotator(pi: ExtensionAPI): void { { deliverAs: "followUp" }, "Plannotator code review feedback could not be sent", origin, + ctx, ); } catch (err) { reportBackgroundError(ctx, "Plannotator code review feedback could not be sent", err, origin); @@ -995,6 +998,7 @@ export default function plannotator(pi: ExtensionAPI): void { { deliverAs: "followUp" }, "Plannotator annotation feedback could not be sent", origin, + ctx, ); if (outcome.notification === "approved") { safeNotify(ctx, "Annotation approved.", "info", origin); @@ -1089,6 +1093,7 @@ export default function plannotator(pi: ExtensionAPI): void { { deliverAs: "followUp" }, "Plannotator message annotation feedback could not be sent", origin, + ctx, ); if (outcome.notification === "approved") { safeNotify(ctx, "Message approved.", "info", origin); From 8222ec7577b4b201a3a8aa3fb45e003c8523d2da Mon Sep 17 00:00:00 2001 From: Apinant U-suwantim Date: Sun, 30 Aug 2026 14:43:33 +0700 Subject: [PATCH 3/3] chore(pi-extension): drop idle-delivery JSDoc Matches the surrounding types: no block comment on the helper. --- apps/pi-extension/current-pi-session.ts | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/apps/pi-extension/current-pi-session.ts b/apps/pi-extension/current-pi-session.ts index 6a4abef61..d5ec14d34 100644 --- a/apps/pi-extension/current-pi-session.ts +++ b/apps/pi-extension/current-pi-session.ts @@ -6,23 +6,6 @@ type NotificationType = "info" | "warning" | "error"; type IdleProbeHost = { isIdle?: () => boolean }; -/** - * Pass `deliverAs` only while the agent is streaming. - * - * Pi documents `deliverAs` as the streaming queue selector ("When the agent is - * streaming, use deliverAs to specify how to queue the message") and starts a - * turn when the session is idle. oh-my-pi instead honors an explicit - * `deliverAs` unconditionally — its contract reads "idle starts a turn; - * streaming queues as steer unless deliverAs is set" — so - * `{ deliverAs: "followUp" }` on an IDLE oh-my-pi session parks the prompt as - * a pending follow-up and never starts a turn. Annotate/review feedback is - * then persisted and acknowledged in the browser ("Feedback Sent") while the - * agent never sees it. - * - * Both hosts expose `isIdle()` on the extension context (`ctx.isIdle`), so - * drop only `deliverAs` while idle and keep every other caller option. Hosts - * predating the capability, or a probe that throws, keep the caller's options unchanged. - */ export function resolveIdleDeliveryOptions( host: IdleProbeHost, options?: SendUserMessageOptions,