From 013385e5c274ccedfc450c33b9ca0ec9917eee3d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 23 Mar 2026 22:22:44 -0700 Subject: [PATCH] refactor: polish trigger and manifest seams --- .../pi-embedded-runner/run/attempt.test.ts | 12 +++ src/agents/pi-embedded-runner/run/attempt.ts | 6 +- .../pi-embedded-runner/run/trigger-policy.ts | 22 +++++ src/extensions/public-artifacts.test.ts | 22 +++++ src/extensions/public-artifacts.ts | 91 ++++++++++++------- ui/src/ui/app-chat.ts | 10 +- .../chat/slash-command-executor.node.test.ts | 8 +- ui/src/ui/chat/slash-command-executor.ts | 11 ++- 8 files changed, 135 insertions(+), 47 deletions(-) create mode 100644 src/agents/pi-embedded-runner/run/trigger-policy.ts create mode 100644 src/extensions/public-artifacts.test.ts diff --git a/src/agents/pi-embedded-runner/run/attempt.test.ts b/src/agents/pi-embedded-runner/run/attempt.test.ts index e22470cdcfd..8835b2f9ac2 100644 --- a/src/agents/pi-embedded-runner/run/attempt.test.ts +++ b/src/agents/pi-embedded-runner/run/attempt.test.ts @@ -26,6 +26,7 @@ import { wrapStreamFnSanitizeMalformedToolCalls, wrapStreamFnTrimToolCallNames, } from "./attempt.js"; +import { shouldInjectHeartbeatPromptForTrigger } from "./trigger-policy.js"; type FakeWrappedStream = { result: () => Promise; @@ -320,6 +321,17 @@ describe("resolvePromptModeForSession", () => { }); describe("shouldInjectHeartbeatPrompt", () => { + it("uses trigger policy defaults for non-cron triggers", () => { + expect(shouldInjectHeartbeatPromptForTrigger("user")).toBe(true); + expect(shouldInjectHeartbeatPromptForTrigger("heartbeat")).toBe(true); + expect(shouldInjectHeartbeatPromptForTrigger("memory")).toBe(true); + expect(shouldInjectHeartbeatPromptForTrigger(undefined)).toBe(true); + }); + + it("uses trigger policy overrides for cron", () => { + expect(shouldInjectHeartbeatPromptForTrigger("cron")).toBe(false); + }); + it("injects the heartbeat prompt for default-agent non-cron runs", () => { expect(shouldInjectHeartbeatPrompt({ isDefaultAgent: true, trigger: "user" })).toBe(true); expect(shouldInjectHeartbeatPrompt({ isDefaultAgent: true, trigger: "heartbeat" })).toBe(true); diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index 2973eacf5ac..af396207dd9 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -158,7 +158,7 @@ import { } from "./compaction-timeout.js"; import { pruneProcessedHistoryImages } from "./history-image-prune.js"; import { detectAndLoadPromptImages } from "./images.js"; -import type { EmbeddedRunTrigger } from "./params.js"; +import { shouldInjectHeartbeatPromptForTrigger } from "./trigger-policy.js"; import type { EmbeddedRunAttemptParams, EmbeddedRunAttemptResult } from "./types.js"; type PromptBuildHookRunner = { @@ -1527,9 +1527,9 @@ export function resolvePromptModeForSession(sessionKey?: string): "minimal" | "f export function shouldInjectHeartbeatPrompt(params: { isDefaultAgent: boolean; - trigger?: EmbeddedRunTrigger; + trigger?: EmbeddedRunAttemptParams["trigger"]; }): boolean { - return params.isDefaultAgent && params.trigger !== "cron"; + return params.isDefaultAgent && shouldInjectHeartbeatPromptForTrigger(params.trigger); } export function resolveAttemptFsWorkspaceOnly(params: { diff --git a/src/agents/pi-embedded-runner/run/trigger-policy.ts b/src/agents/pi-embedded-runner/run/trigger-policy.ts new file mode 100644 index 00000000000..714a5dd9ced --- /dev/null +++ b/src/agents/pi-embedded-runner/run/trigger-policy.ts @@ -0,0 +1,22 @@ +import type { EmbeddedRunTrigger } from "./params.js"; + +type EmbeddedRunTriggerPolicy = { + injectHeartbeatPrompt: boolean; +}; + +const DEFAULT_EMBEDDED_RUN_TRIGGER_POLICY: EmbeddedRunTriggerPolicy = { + injectHeartbeatPrompt: true, +}; + +const EMBEDDED_RUN_TRIGGER_POLICY: Partial> = { + cron: { + injectHeartbeatPrompt: false, + }, +}; + +export function shouldInjectHeartbeatPromptForTrigger(trigger?: EmbeddedRunTrigger): boolean { + return ( + (trigger ? EMBEDDED_RUN_TRIGGER_POLICY[trigger] : undefined)?.injectHeartbeatPrompt ?? + DEFAULT_EMBEDDED_RUN_TRIGGER_POLICY.injectHeartbeatPrompt + ); +} diff --git a/src/extensions/public-artifacts.test.ts b/src/extensions/public-artifacts.test.ts new file mode 100644 index 00000000000..906bc951d23 --- /dev/null +++ b/src/extensions/public-artifacts.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from "vitest"; +import { + BUNDLED_RUNTIME_SIDECAR_BASENAMES, + BUNDLED_RUNTIME_SIDECAR_PATHS, + GUARDED_EXTENSION_PUBLIC_SURFACE_BASENAMES, + getPublicArtifactBasename, +} from "./public-artifacts.js"; + +describe("public artifact manifests", () => { + it("derives bundled sidecar basenames from the runtime sidecar paths", () => { + expect(BUNDLED_RUNTIME_SIDECAR_BASENAMES).toEqual([ + ...new Set(BUNDLED_RUNTIME_SIDECAR_PATHS.map(getPublicArtifactBasename)), + ]); + }); + + it("keeps every bundled sidecar basename in the guarded public surface list", () => { + const guardedBasenames = new Set(GUARDED_EXTENSION_PUBLIC_SURFACE_BASENAMES); + for (const basename of BUNDLED_RUNTIME_SIDECAR_BASENAMES) { + expect(guardedBasenames.has(basename), basename).toBe(true); + } + }); +}); diff --git a/src/extensions/public-artifacts.ts b/src/extensions/public-artifacts.ts index f9657f2a58c..bd631a331e2 100644 --- a/src/extensions/public-artifacts.ts +++ b/src/extensions/public-artifacts.ts @@ -1,34 +1,61 @@ -export const BUNDLED_RUNTIME_SIDECAR_PATHS = [ - "dist/extensions/whatsapp/light-runtime-api.js", - "dist/extensions/whatsapp/runtime-api.js", - "dist/extensions/matrix/helper-api.js", - "dist/extensions/matrix/runtime-api.js", - "dist/extensions/matrix/thread-bindings-runtime.js", - "dist/extensions/msteams/runtime-api.js", -] as const; +function assertUniqueValues(values: readonly T[], label: string): readonly T[] { + const seen = new Set(); + const duplicates = new Set(); + for (const value of values) { + if (seen.has(value)) { + duplicates.add(value); + continue; + } + seen.add(value); + } + if (duplicates.size > 0) { + throw new Error(`Duplicate ${label}: ${Array.from(duplicates).join(", ")}`); + } + return values; +} -const EXTRA_GUARDED_EXTENSION_PUBLIC_SURFACE_BASENAMES = [ - "action-runtime.runtime.js", - "action-runtime-api.js", - "allow-from.js", - "api.js", - "auth-presence.js", - "index.js", - "login-qr-api.js", - "onboard.js", - "openai-codex-catalog.js", - "provider-catalog.js", - "session-key-api.js", - "setup-api.js", - "setup-entry.js", - "timeouts.js", -] as const; +export function getPublicArtifactBasename(relativePath: string): string { + return relativePath.split("/").at(-1) ?? relativePath; +} -export const GUARDED_EXTENSION_PUBLIC_SURFACE_BASENAMES = [ - ...new Set([ - ...BUNDLED_RUNTIME_SIDECAR_PATHS.map( - (relativePath) => relativePath.split("/").at(-1) ?? relativePath, - ), - ...EXTRA_GUARDED_EXTENSION_PUBLIC_SURFACE_BASENAMES, - ]), -] as const; +export const BUNDLED_RUNTIME_SIDECAR_PATHS = assertUniqueValues( + [ + "dist/extensions/whatsapp/light-runtime-api.js", + "dist/extensions/whatsapp/runtime-api.js", + "dist/extensions/matrix/helper-api.js", + "dist/extensions/matrix/runtime-api.js", + "dist/extensions/matrix/thread-bindings-runtime.js", + "dist/extensions/msteams/runtime-api.js", + ] as const, + "bundled runtime sidecar path", +); + +const EXTRA_GUARDED_EXTENSION_PUBLIC_SURFACE_BASENAMES = assertUniqueValues( + [ + "action-runtime.runtime.js", + "action-runtime-api.js", + "allow-from.js", + "api.js", + "auth-presence.js", + "index.js", + "login-qr-api.js", + "onboard.js", + "openai-codex-catalog.js", + "provider-catalog.js", + "session-key-api.js", + "setup-api.js", + "setup-entry.js", + "timeouts.js", + ] as const, + "extra guarded extension public surface basename", +); + +export const BUNDLED_RUNTIME_SIDECAR_BASENAMES = assertUniqueValues( + [...new Set(BUNDLED_RUNTIME_SIDECAR_PATHS.map(getPublicArtifactBasename))], + "bundled runtime sidecar basename", +); + +export const GUARDED_EXTENSION_PUBLIC_SURFACE_BASENAMES = assertUniqueValues( + [...BUNDLED_RUNTIME_SIDECAR_BASENAMES, ...EXTRA_GUARDED_EXTENSION_PUBLIC_SURFACE_BASENAMES], + "guarded extension public surface basename", +); diff --git a/ui/src/ui/app-chat.ts b/ui/src/ui/app-chat.ts index b5d82581abf..092e356a391 100644 --- a/ui/src/ui/app-chat.ts +++ b/ui/src/ui/app-chat.ts @@ -305,13 +305,9 @@ async function dispatchSlashCommand( } const targetSessionKey = host.sessionKey; - const result = await executeSlashCommand( - host.client, - targetSessionKey, - name, - args, - host.chatModelCatalog, - ); + const result = await executeSlashCommand(host.client, targetSessionKey, name, args, { + chatModelCatalog: host.chatModelCatalog, + }); if (result.content) { injectCommandResult(host, result.content); diff --git a/ui/src/ui/chat/slash-command-executor.node.test.ts b/ui/src/ui/chat/slash-command-executor.node.test.ts index 06b8d227c8b..082c59d7b9f 100644 --- a/ui/src/ui/chat/slash-command-executor.node.test.ts +++ b/ui/src/ui/chat/slash-command-executor.node.test.ts @@ -285,7 +285,9 @@ describe("executeSlashCommand directives", () => { "main", "model", "gpt-5-mini", - [{ id: "gpt-5-mini", name: "gpt-5-mini", provider: "openai" }], + { + chatModelCatalog: [{ id: "gpt-5-mini", name: "gpt-5-mini", provider: "openai" }], + }, ); expect(request).toHaveBeenCalledWith("sessions.patch", { @@ -317,7 +319,9 @@ describe("executeSlashCommand directives", () => { "main", "model", "gpt-5-mini", - [{ id: "gpt-5-mini", name: "GPT-5 Mini", provider: "openai" }], + { + chatModelCatalog: [{ id: "gpt-5-mini", name: "GPT-5 Mini", provider: "openai" }], + }, ); expect(result.sessionPatch?.modelOverride).toEqual({ diff --git a/ui/src/ui/chat/slash-command-executor.ts b/ui/src/ui/chat/slash-command-executor.ts index 77b9ae7ad2a..35cef60098a 100644 --- a/ui/src/ui/chat/slash-command-executor.ts +++ b/ui/src/ui/chat/slash-command-executor.ts @@ -50,12 +50,16 @@ export type SlashCommandResult = { }; }; +export type SlashCommandContext = { + chatModelCatalog?: ModelCatalogEntry[]; +}; + export async function executeSlashCommand( client: GatewayBrowserClient, sessionKey: string, commandName: string, args: string, - chatModelCatalog: ModelCatalogEntry[] = [], + context: SlashCommandContext = {}, ): Promise { switch (commandName) { case "help": @@ -73,7 +77,7 @@ export async function executeSlashCommand( case "compact": return await executeCompact(client, sessionKey); case "model": - return await executeModel(client, sessionKey, args, chatModelCatalog); + return await executeModel(client, sessionKey, args, context); case "think": return await executeThink(client, sessionKey, args); case "fast": @@ -130,7 +134,7 @@ async function executeModel( client: GatewayBrowserClient, sessionKey: string, args: string, - chatModelCatalog: ModelCatalogEntry[], + context: SlashCommandContext, ): Promise { if (!args) { try { @@ -163,6 +167,7 @@ async function executeModel( }); const patchedModel = patched.resolved?.model ?? args.trim(); const rawOverride = createChatModelOverride(patchedModel.trim()); + const chatModelCatalog = context.chatModelCatalog ?? []; const resolvedValue = rawOverride ? normalizeChatModelOverrideValue(rawOverride, chatModelCatalog) || resolveServerChatModelValue(patchedModel, patched.resolved?.modelProvider)