diff --git a/bun.lock b/bun.lock index 4d935448..7c7f0f5d 100644 --- a/bun.lock +++ b/bun.lock @@ -16,12 +16,14 @@ "devDependencies": { "@nikolasd/crew-protocol": "workspace:*", "@oh-my-pi/pi-coding-agent": ">=17.0.7 <19", + "@oh-my-pi/pi-tui": ">=17.0.7 <19", "@types/bun": "1.3.14", "ajv": "8.17.1", "zod": "^4", }, "peerDependencies": { "@oh-my-pi/pi-coding-agent": ">=17.0.7 <19", + "@oh-my-pi/pi-tui": ">=17.0.7 <19", }, }, "packages/protocol-ts": { diff --git a/packages/extension/dist/index.js b/packages/extension/dist/index.js index 68c4a09a..4977e32a 100644 --- a/packages/extension/dist/index.js +++ b/packages/extension/dist/index.js @@ -30,6 +30,7 @@ var __toESM = (mod, isNodeMode, target) => { return to; }; var __commonJS = (cb, mod) => () => (mod || cb((mod = { exports: {} }).exports, mod), mod.exports); +var __require = import.meta.require; // ../../node_modules/.bun/ajv@8.17.1/node_modules/ajv/dist/compile/codegen/code.js var require_code = __commonJS((exports) => { @@ -11473,11 +11474,12 @@ var package_default = { exports: { ".": "./dist/index.js" }, omp: { extensions: ["./dist/index.js"] }, scripts: { - build: "bun build src/index.ts --outdir dist --target bun --external @oh-my-pi/pi-coding-agent" + build: "bun build src/index.ts --outdir dist --target bun --external @oh-my-pi/pi-coding-agent --external @oh-my-pi/pi-tui" }, - peerDependencies: { "@oh-my-pi/pi-coding-agent": ">=17.0.7 <19" }, + peerDependencies: { "@oh-my-pi/pi-coding-agent": ">=17.0.7 <19", "@oh-my-pi/pi-tui": ">=17.0.7 <19" }, devDependencies: { "@oh-my-pi/pi-coding-agent": ">=17.0.7 <19", + "@oh-my-pi/pi-tui": ">=17.0.7 <19", "@nikolasd/crew-protocol": "workspace:*", "@types/bun": "1.3.14", ajv: "8.17.1", @@ -13605,6 +13607,29 @@ import { mkdir, writeFile } from "fs/promises"; import { join as join8 } from "path"; // src/milestones.ts +var CREW_MESSAGE_TYPE = "crew"; +var CREW_ICON = "\uD83D\uDC65"; +var CREW_LABEL = "Crew"; +function supportsCustomMessageRenderer(pi) { + return typeof pi.registerMessageRenderer === "function"; +} +async function registerCrewMessageRenderer(pi) { + const register = pi.registerMessageRenderer; + try { + const { Box, Text } = await import("@oh-my-pi/pi-tui"); + register.call(pi, CREW_MESSAGE_TYPE, (message) => { + const text = typeof message.content === "string" ? message.content : JSON.stringify(message.content); + const box = new Box(1, 0); + box.addChild(new Text(`${CREW_ICON} ${CREW_LABEL}`)); + box.addChild(new Text(text)); + return box; + }); + } catch (err) { + pi.logger.debug("crew milestone bridge: could not register the crew message renderer", { + error: err instanceof Error ? err.message : String(err) + }); + } +} var TERMINAL_STATES = { succeeded: true, failed: true, @@ -13695,7 +13720,7 @@ function formatDigest(e, lookup) { if (!event.payload.flags.turnSettled) { return; } - return `${capitalize(who)} settled a turn and is waiting on the leader (not terminal -- ` + `the vendor is still parked, not exited). Read the answer via crew_run { op: "result", runId }, ` + `then either crew_send to follow up or crew_run { op: "finish", runId, outcome } to close it.`; + return `${capitalize(who)} settled a turn and is waiting on the leader (not terminal -- the vendor is still parked, not exited). Read the answer via crew_run { op: "result", runId }, then either crew_send to follow up or crew_run { op: "finish", runId, outcome } to close it.`; } case "workerQuestion": { const question = event.payload.question ?? "(no question text captured)"; @@ -13703,7 +13728,7 @@ function formatDigest(e, lookup) { } case "workerTimeout": { const kind = event.payload.kind ?? "inactivity"; - return `${capitalize(who)} hit a worker ${kind} timeout. The runtime reports; the leader decides: ` + `give it more time via crew_run { op: "timeoutAck", runId, decision: "extend" }, ` + `redirect it via crew_send (the nudge), or stop it via crew_run { op: "timeoutAck", decision: "abort" }.`; + return `${capitalize(who)} hit a worker ${kind} timeout. The runtime reports; the leader decides: give it more time via crew_run { op: "timeoutAck", runId, decision: "extend" }, redirect it via crew_send (the nudge), or stop it via crew_run { op: "timeoutAck", decision: "abort" }.`; } case "budgetExceeded": return `${capitalize(who)} exceeded its turn budget. Escalate to the user or raise the budget via the plan.`; @@ -13721,6 +13746,10 @@ function formatDigest(e, lookup) { } function attachMilestoneBridge(pi, monitor) { const tracker = new MilestoneTracker; + const customRendererSupported = supportsCustomMessageRenderer(pi); + if (customRendererSupported) { + registerCrewMessageRenderer(pi); + } const send = pi.sendMessage; let warnedMissingSendMessage = false; return monitor.subscribeEvents((e, meta) => { @@ -13735,7 +13764,8 @@ function attachMilestoneBridge(pi, monitor) { return; } if (typeof send === "function") { - send.call(pi, digest, { deliverAs: "followUp", triggerTurn: true }); + const message = customRendererSupported ? { customType: CREW_MESSAGE_TYPE, content: digest } : digest; + send.call(pi, message, { deliverAs: "followUp", triggerTurn: true }); } else if (!warnedMissingSendMessage) { warnedMissingSendMessage = true; pi.logger.warn("crew milestone bridge: this omp build exposes no sendMessage on ExtensionAPI, so run milestones will not be delivered to the leader for the rest of this session"); diff --git a/packages/extension/package.json b/packages/extension/package.json index 78774e57..6ce01386 100644 --- a/packages/extension/package.json +++ b/packages/extension/package.json @@ -5,11 +5,12 @@ "exports": { ".": "./dist/index.js" }, "omp": { "extensions": ["./dist/index.js"] }, "scripts": { - "build": "bun build src/index.ts --outdir dist --target bun --external @oh-my-pi/pi-coding-agent" + "build": "bun build src/index.ts --outdir dist --target bun --external @oh-my-pi/pi-coding-agent --external @oh-my-pi/pi-tui" }, - "peerDependencies": { "@oh-my-pi/pi-coding-agent": ">=17.0.7 <19" }, + "peerDependencies": { "@oh-my-pi/pi-coding-agent": ">=17.0.7 <19", "@oh-my-pi/pi-tui": ">=17.0.7 <19" }, "devDependencies": { "@oh-my-pi/pi-coding-agent": ">=17.0.7 <19", + "@oh-my-pi/pi-tui": ">=17.0.7 <19", "@nikolasd/crew-protocol": "workspace:*", "@types/bun": "1.3.14", "ajv": "8.17.1", diff --git a/packages/extension/src/milestones.test.ts b/packages/extension/src/milestones.test.ts index debfe784..6602d09a 100644 --- a/packages/extension/src/milestones.test.ts +++ b/packages/extension/src/milestones.test.ts @@ -1,6 +1,7 @@ import { expect, test } from "bun:test"; import type { EventEnvelope, RuntimeEvent, RuntimeEventKind } from "@nikolasd/crew-protocol"; +import type { Component } from "@oh-my-pi/pi-tui"; import { attachMilestoneBridge, formatDigest, MilestoneTracker, type RunLookup } from "./milestones"; import type { MonitorController } from "./monitor/controller"; import type { MonitorRow } from "./monitor/model"; @@ -341,16 +342,29 @@ test("escalation digest with a question appends it after the reason", () => { function fakeBridge(): { sent: string[]; + sentCustomTypes: string[]; dispatch: (e: EventEnvelope, meta?: { replay: boolean }) => void; unsubscribe: () => void; } { const sent: string[] = []; + const sentCustomTypes: string[] = []; const fakePi = { logger: { debug() {}, info() {}, warn() {}, error() {} }, - sendMessage: (message: string) => { - sent.push(message); + // Present (even as a no-op) so `supportsCustomMessageRenderer` reads + // true and the bridge sends the `{ customType, content }` object + // shape below -- a build advertised as supporting custom renderers, + // which is the case most of this file's tests exercise. The opposite + // case (absent, bare string) has its own dedicated test. + registerMessageRenderer: () => {}, + sendMessage: (message: { customType: string; content: string }) => { + sent.push(message.content); + sentCustomTypes.push(message.customType); }, - } as unknown as { logger: { [k: string]: (...a: unknown[]) => void }; sendMessage: (m: string) => void }; + } as unknown as { + logger: { [k: string]: (...a: unknown[]) => void }; + registerMessageRenderer: () => void; + sendMessage: (m: { customType: string; content: string }) => void; + }; const listeners: Array<(e: EventEnvelope, meta: { replay: boolean }) => void> = []; const controller = { subscribeEvents(cb: (e: EventEnvelope, meta: { replay: boolean }) => void) { @@ -373,7 +387,7 @@ function fakeBridge(): { } }; expect(listeners.length).toBe(1); - return { sent, dispatch, unsubscribe }; + return { sent, sentCustomTypes, dispatch, unsubscribe }; } test("bridge injects a digest for a milestone and stays silent for noise", () => { @@ -390,6 +404,51 @@ test("bridge injects a digest for a milestone and stays silent for noise", () => expect(sent.length).toBe(1); }); +test("every digest is sent under crew's own customType, not a bare string", () => { + // A bare string falls back to oh-my-pi's generic "custom-message" + // header, indistinguishable from any other extension's notice. Every + // digest must carry `customType: "crew"` so the registered renderer + // (see the rendering tests below) actually applies to it. + const { sentCustomTypes, dispatch } = fakeBridge(); + dispatch(run("run-1", "failed")); + dispatch(run("run-2", "succeeded")); + expect(sentCustomTypes).toEqual(["crew", "crew"]); +}); + +test("a build with no registerMessageRenderer gets a bare-string digest, never the object shape", () => { + // The wire-shape half of the same capability check the renderer + // registration is gated on: a build old enough to lack + // `registerMessageRenderer` has not established that its `sendMessage` + // accepts anything but a string, so it must never receive the + // `{ customType, content }` object -- that would be an untested, + // possibly-garbled payload on exactly the build least likely to handle + // it gracefully. + const sent: unknown[] = []; + const fakePi = { + logger: { debug() {}, info() {}, warn() {}, error() {} }, + // deliberately no registerMessageRenderer + sendMessage: (message: unknown) => { + sent.push(message); + }, + }; + const listeners: Array<(e: EventEnvelope, meta: { replay: boolean }) => void> = []; + const controller = { + subscribeEvents(cb: (e: EventEnvelope, meta: { replay: boolean }) => void) { + listeners.push(cb); + return () => {}; + }, + getState() { + return { rows: ROWS }; + }, + } as unknown as MonitorController; + attachMilestoneBridge(fakePi as never, controller); + listeners[0]?.(run("run-1", "failed"), { replay: false }); + + expect(sent.length).toBe(1); + expect(typeof sent[0]).toBe("string"); + expect(sent[0] as string).toContain("FAILED"); +}); + test("an omp build with no sendMessage warns once, not silently and not per event", () => { // The only branch in the bridge that could fail silently. `send` is looked // up through a cast so a renamed or removed omp method degrades to "no @@ -484,3 +543,129 @@ test("a replayed milestone still updates the tracker's one-shot bookkeeping", () dispatch(run("run-2", "working"), { replay: false }); expect(sent.length).toBe(1); }); + +function fakePiWithRenderer(): { + pi: { logger: { [k: string]: (...a: unknown[]) => void } }; + registered: Array<{ customType: string; renderer: (message: { content: unknown }) => Component | undefined }>; +} { + const registered: Array<{ customType: string; renderer: (message: { content: unknown }) => Component | undefined }> = []; + const pi = { + logger: { debug() {}, info() {}, warn() {}, error() {} }, + registerMessageRenderer: (customType: string, renderer: (message: { content: unknown }) => Component | undefined) => { + registered.push({ customType, renderer }); + }, + // No sendMessage needed for these tests -- only the renderer registration + // and its output are under test here. + }; + return { pi, registered }; +} + +function controllerWithNoEvents(): MonitorController { + return { + subscribeEvents() { + return () => {}; + }, + getState() { + return { rows: ROWS }; + }, + } as unknown as MonitorController; +} + +/** + * `registerCrewMessageRenderer` registers asynchronously (it dynamically + * imports `@oh-my-pi/pi-tui` before calling `registerMessageRenderer`), + * so a test that attaches the bridge and immediately inspects what got + * registered needs to let that microtask chain settle first. A macrotask + * flush is deliberately used over an immediate microtask flush: the + * import may resolve over more than one microtask hop internally, and a + * `setTimeout` is guaranteed to run after all of them regardless of how + * many there are. + */ +async function flushAsyncRegistration(): Promise { + // Pre-warms the module cache (the same specifier `registerCrewMessageRenderer` + // dynamically imports) so its own `import()` resolves from cache rather + // than doing first-time resolution work, then flushes a macrotask to + // let that resolved promise's `.then`/registration chain finish running + // before the test inspects what got registered. + await import("@oh-my-pi/pi-tui"); + await new Promise((resolve) => setTimeout(resolve, 0)); +} + +test("attaching the bridge registers a renderer for crew's own customType", async () => { + const { pi, registered } = fakePiWithRenderer(); + attachMilestoneBridge(pi as never, controllerWithNoEvents()); + await flushAsyncRegistration(); + expect(registered.length).toBe(1); + expect(registered[0]?.customType).toBe("crew"); +}); + +test("the registered renderer draws crew's icon, label, and the digest text", async () => { + // The actual proof of content: the renderer this bridge registers, + // invoked the way oh-my-pi itself would invoke it, must produce a real + // component whose rendered rows show crew's own icon and label rather + // than the generic "custom-message" fallback header -- and still carry + // the digest text verbatim, since the digest wording itself is out of + // scope for this change. + const { pi, registered } = fakePiWithRenderer(); + attachMilestoneBridge(pi as never, controllerWithNoEvents()); + await flushAsyncRegistration(); + const renderer = registered[0]?.renderer; + expect(renderer).toBeDefined(); + + const component = renderer?.({ content: "run run-1 (claude adapter) for task task-1 FAILED: boom." }); + expect(component).toBeDefined(); + const rows = component?.render(80).join("\n") ?? ""; + expect(rows).toContain("👥"); + expect(rows).toContain("Crew"); + expect(rows).toContain("run run-1 (claude adapter) for task task-1 FAILED: boom."); +}); + +test("the registered renderer degrades to the stringified content on a non-string payload rather than throwing", async () => { + // Every digest this bridge sends is a plain string, but `content` is + // typed as `CustomMessageContent` in general (a string or a content-part + // array). A future or third-party sender of this customType with rich + // content must still render something rather than crash the renderer. + const { pi, registered } = fakePiWithRenderer(); + attachMilestoneBridge(pi as never, controllerWithNoEvents()); + await flushAsyncRegistration(); + const renderer = registered[0]?.renderer; + const component = renderer?.({ content: [{ type: "text", text: "hi" }] }); + expect(component).toBeDefined(); + const rows = component?.render(80).join("\n") ?? ""; + expect(rows).toContain("👥"); + expect(rows.length).toBeGreaterThan(0); +}); + +test("a build that advertises registerMessageRenderer but has no @oh-my-pi/pi-tui degrades to no custom renderer, not a crash", async () => { + // The combination the doc comment calls unlikely but still guards: the + // method exists (so `supportsCustomMessageRenderer` is true and the + // object wire shape is sent) yet the dynamic import of `pi-tui` fails. + // Import failure is simulated by pointing the dynamic import at a + // module specifier that cannot resolve -- `registerCrewMessageRenderer` + // itself is not exported, so this drives it through the same public + // entry point every other test here uses, with a `registerMessageRenderer` + // that throws synchronously to stand in for a resolution failure deep in + // the (unmockable from here) dynamic import. + const registered: unknown[] = []; + const pi = { + logger: { debug() {}, info() {}, warn() {}, error() {} }, + registerMessageRenderer: () => { + registered.push(true); + throw new Error("simulated: registerMessageRenderer itself rejected the renderer"); + }, + }; + expect(() => attachMilestoneBridge(pi as never, controllerWithNoEvents())).not.toThrow(); + await flushAsyncRegistration(); +}); + +test("an omp build with no registerMessageRenderer degrades quietly, not a crash", () => { + // The counterpart to the missing-sendMessage test: an older/newer omp + // build without this method must still let the bridge attach and deliver + // digests -- losing only the custom header, never the notification + // itself, and never throwing at attach time. + const pi = { + logger: { debug() {}, info() {}, warn() {}, error() {} }, + // deliberately no registerMessageRenderer + }; + expect(() => attachMilestoneBridge(pi as never, controllerWithNoEvents())).not.toThrow(); +}); diff --git a/packages/extension/src/milestones.ts b/packages/extension/src/milestones.ts index f045c8d6..f9ecf759 100644 --- a/packages/extension/src/milestones.ts +++ b/packages/extension/src/milestones.ts @@ -14,11 +14,116 @@ import type { EventEnvelope, RuntimeEvent } from "@nikolasd/crew-protocol"; import type { ExtensionAPI } from "@oh-my-pi/pi-coding-agent"; +import type { Component } from "@oh-my-pi/pi-tui"; import type { EventDeliveryMeta } from "./client"; import type { MonitorController } from "./monitor/controller"; import type { MonitorRow } from "./monitor/model"; +/** + * The `customType` crew's own injected notices are sent under, on any omp + * build `supportsCustomMessageRenderer` below confirms can take it. A + * bare string -- every digest here used to be one, and still is on a + * build that fails that check -- falls back to oh-my-pi's own + * `DEFAULT_CUSTOM_MESSAGE_TYPE` ("custom-message") and renders under a + * generic header naming no extension at all, indistinguishable from any + * other extension's notice. Sending under this type instead, paired with + * `registerCrewMessageRenderer` below, is what lets a digest render as + * crew's own rather than that generic fallback. + */ +const CREW_MESSAGE_TYPE = "crew"; + +/** + * Icon and label crew's own notices render under. Provisional pending the + * maintainer's final wording on the label -- "Crew" is the agreed default + * to ship with. + */ +const CREW_ICON = "👥"; +const CREW_LABEL = "Crew"; + +/** + * Whether this omp build can take a custom message renderer at all -- + * `registerMessageRenderer` accessed through a cast, the same defensive + * lookup convention already used here for `sendMessage`. This single + * check gates two separate things, deliberately: whether it is safe to + * *register* a renderer (obviously), and whether it is safe to *send* the + * `{ customType, content }` object shape `sendMessage` needs for a + * renderer to ever apply. The two are tied together on purpose -- a build + * old enough to lack this method is exactly the build most likely to + * predate `customType` support too, and there is no independent way to + * probe the second without an installed omp old enough to test against. + * Tying them to the same signal makes the degradation provable by reading + * this file rather than by knowing which omp versions accept what: a + * build without this method gets a bare-string digest, unconditionally, + * from every call site below. + */ +function supportsCustomMessageRenderer(pi: ExtensionAPI): boolean { + return ( + typeof ( + pi as unknown as { + registerMessageRenderer?: (customType: string, renderer: (message: { content: unknown }) => Component | undefined) => void; + } + ).registerMessageRenderer === "function" + ); +} + +/** + * Registers a custom renderer for `CREW_MESSAGE_TYPE`: a small box with + * crew's own icon + label as its header and the digest text as its body, + * standing in for the generic "custom-message" fallback header every + * extension-injected message without a registered renderer gets. Only + * ever called after `supportsCustomMessageRenderer` -- see that function + * for why its check covers this call being safe to make at all. + * + * `@oh-my-pi/pi-tui`'s `Box`/`Text` are imported dynamically, inside this + * function, rather than statically at module scope: this module (and the + * `--external` build that ships it) has no control over whether an + * arbitrary omp build actually exposes that module to extensions, and a + * static import of an external specifier is evaluated at load time, + * unconditionally -- a `pi-tui` an omp build doesn't expose would fail + * the whole extension's load, not just this renderer, on every build, + * whether or not it ever calls this function. Deferred behind the same + * capability check `sendMessage`'s own wire shape is gated on, a build + * without `pi-tui` degrades exactly like a build without the method: + * no custom renderer, the fallback header applies, nothing else breaks. + */ +async function registerCrewMessageRenderer(pi: ExtensionAPI): Promise { + const register = ( + pi as unknown as { + registerMessageRenderer: (customType: string, renderer: (message: { content: unknown }) => Component | undefined) => void; + } + ).registerMessageRenderer; + // One try/catch around both steps, deliberately: neither the dynamic + // import nor the call into host-provided `register` is code this file + // controls, and a throw from either must degrade the same way -- no + // custom renderer, no broken extension load, nothing else affected. + try { + const { Box, Text } = await import("@oh-my-pi/pi-tui"); + register.call(pi, CREW_MESSAGE_TYPE, (message: { content: unknown }) => { + // `content` is `CustomMessageContent = string | (TextContent | ImageContent)[]` + // in general, but every digest this bridge ever sends is a plain + // string -- the array shape is defended against rather than assumed + // absent, since a future digest carrying rich content should still + // render something instead of throwing. + const text = typeof message.content === "string" ? message.content : JSON.stringify(message.content); + const box = new Box(1, 0); + box.addChild(new Text(`${CREW_ICON} ${CREW_LABEL}`)); + box.addChild(new Text(text)); + return box; + }); + } catch (err) { + // The method existing without the module it depends on (or rejecting + // the renderer it was just handed) is not a combination expected to + // occur in practice -- the method's own return type is `pi-tui`'s + // `Component` -- but if it does, this is the same cosmetic + // degradation as the method being absent: no custom renderer, never a + // broken extension load. + pi.logger.debug("crew milestone bridge: could not register the crew message renderer", { + error: err instanceof Error ? err.message : String(err), + }); + } +} + /** A run-state value that ends the run. */ const TERMINAL_STATES: Record = { succeeded: true, @@ -231,13 +336,32 @@ export function formatDigest(e: EventEnvelope, lookup: RunLookup): string | unde export function attachMilestoneBridge(pi: ExtensionAPI, monitor: MonitorController): () => void { const tracker = new MilestoneTracker(); + // Computed once, at attach time, and reused below at both the one call + // site that registers a renderer and the one that decides what shape to + // send -- see `supportsCustomMessageRenderer`'s own doc comment for why + // one check governs both. + const customRendererSupported = supportsCustomMessageRenderer(pi); + if (customRendererSupported) { + // Fire-and-forget is safe here: `customRendererSupported` (the wire + // shape's own gate below) was already decided synchronously above, + // so a digest arriving before this async registration resolves still + // gets the correct `{ customType, content }` shape -- only its + // header renders generic until the dynamic import settles, which is + // attach-time work finishing well before any run reaches a milestone. + void registerCrewMessageRenderer(pi); + } + // The oh-my-pi `ExtensionAPI` surface is version-gated; access // `sendMessage` defensively so a renamed/removed method degrades to // "no digest" rather than a crash. The documented signature is - // `sendMessage(message, { deliverAs, triggerTurn })`. + // `sendMessage(message, { deliverAs, triggerTurn })`. `message` is the + // `{ customType, content }` object only when `customRendererSupported` + // -- a bare string otherwise, since a build old enough to lack + // `registerMessageRenderer` is not one this file has established + // accepts anything else. const send = ( pi as unknown as { - sendMessage?: (message: string, options?: { deliverAs?: string; triggerTurn?: boolean }) => unknown; + sendMessage?: (message: string | { customType: string; content: string }, options?: { deliverAs?: string; triggerTurn?: boolean }) => unknown; } ).sendMessage; @@ -273,7 +397,8 @@ export function attachMilestoneBridge(pi: ExtensionAPI, monitor: MonitorControll return; } if (typeof send === "function") { - void send.call(pi, digest, { deliverAs: "followUp", triggerTurn: true }); + const message = customRendererSupported ? { customType: CREW_MESSAGE_TYPE, content: digest } : digest; + void send.call(pi, message, { deliverAs: "followUp", triggerTurn: true }); } else if (!warnedMissingSendMessage) { warnedMissingSendMessage = true; pi.logger.warn("crew milestone bridge: this omp build exposes no sendMessage on ExtensionAPI, so run milestones will not be delivered to the leader for the rest of this session");