diff --git a/docs/extensions.md b/docs/extensions.md index 6cfdecb47225..d4ef28489f28 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -324,7 +324,10 @@ ACP clients can defer agent-initiated turns, in which case requests that would start one are retained as hidden next-turn messages. `pi.sendUserMessage(content, { deliverAs })` submits text/images without slash-command -or prompt-template expansion. Omit `deliverAs` to start a prompt when idle; +or prompt-template expansion unless you pass `expandPromptTemplates: true`. With it, +a registered `/skill:` command and prompt templates expand on every delivery +path; extension, custom, and file slash commands run only when `deliverAs` is +omitted (or is `"aside"` while idle). Omit `deliverAs` to start a prompt when idle; while streaming, omission queues a steer. Explicit `"steer"` or `"followUp"` queues the prompt even when idle instead of starting a turn directly. `"aside"` injects at the next step boundary while a run is live and starts a turn when diff --git a/docs/sdk.md b/docs/sdk.md index 69b9aebded4a..d453b054a972 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -269,7 +269,7 @@ Behavior: Related APIs: -- `sendUserMessage(content, { deliverAs?, attribution? })` +- `sendUserMessage(content, { deliverAs?, attribution?, expandPromptTemplates? })` - `steer(text, images?, { attribution? })` - `followUp(text, images?, { synthetic?, attribution? })` - `sendCustomMessage({ customType, content, ... }, { deliverAs?, triggerTurn? })` diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 5b72b7f888dd..d71d20f7285c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -525,6 +525,7 @@ ### Added +- Extensions can pass `expandPromptTemplates: true` to `pi.sendUserMessage()` so a `/skill:` command or prompt template they send expands like typed input, matching Pi; Pi extensions with skill-alias commands now load the skill instead of sending the literal command text ([#14608](https://github.com/can1357/oh-my-pi/pull/14608) by [@andrebrait](https://github.com/andrebrait)) - Added an agents HUD pill counting running subagents, opening the agent hub on click - In Tern the thinking level shows as the composer model chip's icon instead of a separate chip, still cycling on click, while `statusLine.compactThinkingLevel` (Compact Thinking Level, on by default) is on - Added `computer.zoom()` and window-local `zoom()` in JavaScript and Python Eval, with native-detail region captures that preserve full-screenshot click coordinates. diff --git a/packages/coding-agent/src/extensibility/extensions/types.ts b/packages/coding-agent/src/extensibility/extensions/types.ts index 402fc3455d55..eed382c36dca 100644 --- a/packages/coding-agent/src/extensibility/extensions/types.ts +++ b/packages/coding-agent/src/extensibility/extensions/types.ts @@ -1826,7 +1826,9 @@ export interface ExtensionAPI { /** Send a user prompt: idle starts a turn; streaming queues as steer unless deliverAs is set. * `deliverAs: "aside"` injects at the next step boundary without interrupting the in-flight tool - * batch while streaming, except that it ends a running interruptible `wait`; idle still starts a turn. */ + * batch while streaming, except that it ends a running interruptible `wait`; idle still starts a turn. + * Text is sent as written unless `expandPromptTemplates` is set, which expands `/skill:` commands + * and prompt templates. */ sendUserMessage(content: string | (TextContent | ImageContent)[], options?: SendUserMessageOptions): void; /** Append a custom entry to the session for state persistence (not sent to LLM). */ diff --git a/packages/coding-agent/src/session/agent-session-types.ts b/packages/coding-agent/src/session/agent-session-types.ts index eed06874bda3..49235641eadf 100644 --- a/packages/coding-agent/src/session/agent-session-types.ts +++ b/packages/coding-agent/src/session/agent-session-types.ts @@ -475,6 +475,8 @@ export interface SendUserMessageOptions { deliverAs?: "steer" | "followUp" | "aside"; /** Explicit billing/initiator attribution. */ attribution?: MessageAttribution; + /** Expand a registered `/skill:` command and prompt templates (default: false). */ + expandPromptTemplates?: boolean; } /** Result from a handoff operation. */ diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 50465f8b9a7d..25951645de9e 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -161,7 +161,7 @@ import { createExtensionModelQuery } from "../extensibility/extensions/model-api import type { CompactOptions, ContextUsage } from "../extensibility/extensions/types"; import type { CustomCommandContext } from "../extensibility/custom-commands/types"; import { SkillDescriptionCatalog } from "../extensibility/skill-descriptions"; -import type { Skill, SkillWarning } from "../extensibility/skills"; +import { buildSkillPromptMessage, parseSkillInvocation, type Skill, type SkillWarning } from "../extensibility/skills"; import { expandSlashCommand, type FileSlashCommand, loadSlashCommands } from "../extensibility/slash-commands"; import { normalizeToolEventInput, resolveToolEventInput } from "../extensibility/tool-event-input"; import { GoalRuntime } from "../goals/runtime"; @@ -7598,13 +7598,13 @@ export class AgentSession implements SettingsScope { throw new AgentBusyError(); } - await this.#queueCustomMessage(message, streamingBehavior, { + const queued = await this.#queueCustomMessage(message, streamingBehavior, { queueChipText: options?.queueChipText, prependMessages: keywordNotices, onPromptAdmitted: options?.onPromptAdmitted, }); - outcome.sessionClaimed = true; - return true; + outcome.sessionClaimed = queued; + return queued; } const customMessage: CustomMessage = { @@ -7643,14 +7643,14 @@ export class AgentSession implements SettingsScope { outcome.sessionClaimed = this.agent.state.isStreaming; throw new AgentBusyError(); } - await this.#queueCustomMessage(message, streamingBehavior, { + const queued = await this.#queueCustomMessage(message, streamingBehavior, { queueChipText: options?.queueChipText, preprocessed: { content: preparedMessage.content, descriptionNotice }, prependMessages: keywordNotices, onPromptAdmitted: options?.onPromptAdmitted, }); - outcome.sessionClaimed = true; - return true; + outcome.sessionClaimed = queued; + return queued; } outcome.sessionClaimed = await this.#promptWithMessage(preparedMessage, textContent, { ...options, @@ -8629,7 +8629,8 @@ export class AgentSession implements SettingsScope { } } - /** Queue a custom message without starting a turn, matching steer/follow-up/aside delivery. */ + /** Queue a custom message without starting a turn, matching steer/follow-up/aside delivery. + * Resolves false when an abort or session change during image preparation dropped it. */ async #queueCustomMessage( message: Pick, "customType" | "content" | "display" | "details" | "attribution">, deliverAs: "steer" | "followUp" | "aside", @@ -8642,9 +8643,10 @@ export class AgentSession implements SettingsScope { /** Called synchronously once the message is pushed onto its queue. See {@link PromptOptions.onPromptAdmitted}. */ onPromptAdmitted?: () => void; }, - ): Promise { + ): Promise { // Captured before the normalization await below — see #sessionGeneration's doc comment. const sessionGeneration = this.#sessionGeneration; + const promptGeneration = this.#promptGeneration; const details = options?.queueChipText !== undefined ? ({ @@ -8673,8 +8675,17 @@ export class AgentSession implements SettingsScope { const descriptionNotice = preprocessed ? preprocessed.descriptionNotice : await this.#buildSkillImageDescriptionNotice(normalizedAppMessage); + // abort() clears the steer/follow-up queues and disposal flushes every queue; a message + // still being prepared when either lands must not be queued afterwards (as in #queueUserMessage). + if ( + this.#isDisposed || + (deliverAs !== "aside" && + (this.#promptGeneration !== promptGeneration || this.#sessionGeneration !== sessionGeneration)) + ) { + return false; + } if (deliverAs === "aside") { - if (await this.#sessionGenerationChanged(sessionGeneration)) return; + if (await this.#sessionGenerationChanged(sessionGeneration)) return false; // Non-interrupting: rides the same step-boundary aside poll as // sendCustomMessage's streaming aside branch — not an agent-core queue // entry, so no drain-retry latch and no idle-queue drain scheduling. @@ -8689,7 +8700,7 @@ export class AgentSession implements SettingsScope { // left to drain it. Resuming here is a no-op while streaming and wakes/folds // correctly once idle, matching #queueUserMessage's aside branch. this.#resumeStrandedIrcAsides(); - return; + return true; } this.#allowQueuedMessageDrainRetry(); // Keyword notices and their user message must enter the queue in one synchronous phase. @@ -8704,6 +8715,7 @@ export class AgentSession implements SettingsScope { } onPromptAdmitted?.(); this.#scheduleIdleQueueDrain(); + return true; } /** @@ -8915,6 +8927,11 @@ export class AgentSession implements SettingsScope { * Omitted `deliverAs` starts a turn when idle and queues as a steer while streaming. * Explicit `deliverAs` queues without starting a turn in either state; `aside` at * an idle session instead starts a turn, since there is no live run to inject into. + * + * `expandPromptTemplates` (default false) expands a registered `/skill:` and + * prompt templates on every delivery path. Extension, custom and file slash commands + * run only through prompt() (omitted `deliverAs`, or `aside` at idle); explicit + * steer/follow-up queueing sends any other `/` text to the model as written. */ async sendUserMessage( content: string | (TextContent | ImageContent)[], @@ -8940,25 +8957,71 @@ export class AgentSession implements SettingsScope { if (images.length === 0) images = undefined; } + const expand = options?.expandPromptTemplates === true; + const invocation = expand && this.skillsSettings?.enableSkillCommands ? parseSkillInvocation(text) : undefined; + const skill = invocation && this.skills.find(candidate => candidate.name === invocation.name); + if (invocation && skill) { + // Admit before reading SKILL.md so shutdown and settle checks see this send, + // as they see an option-off send admitted synchronously by prompt(). + await this.#admitSubmission(async () => { + // abort() or a session change during the read must drop the send, like prompt()'s + // image preprocessing does, instead of starting a turn the user just stopped. + const promptGeneration = this.#promptGeneration; + const sessionGeneration = this.#sessionGeneration; + const built = await buildSkillPromptMessage(skill, invocation, "user"); + if ( + this.#promptGeneration !== promptGeneration || + this.#sessionGeneration !== sessionGeneration || + this.#isDisposed + ) { + return; + } + const skillContent = images ? [{ type: "text" as const, text: built.message }, ...images] : built.message; + if (options?.attribution === "agent") { + // The skill custom message is user-invoked by definition; an agent-attributed send + // stays a plain user-role message, like every other sendUserMessage path. + await this.sendUserMessage(skillContent, { deliverAs: options.deliverAs, attribution: "agent" }); + return; + } + await this.promptCustomMessage( + { + customType: SKILL_PROMPT_MESSAGE_TYPE, + content: skillContent, + display: true, + details: built.details, + attribution: "user", + }, + { + streamingBehavior: options?.deliverAs ?? "steer", + queueOnly: options?.deliverAs === "steer" || options?.deliverAs === "followUp", + queueChipText: text, + }, + ); + }); + return; + } + const queuedText = expand ? expandPromptTemplate(text, [...this.#promptTemplates]) : text; + const queueOptions = { attribution: options?.attribution, rawText: text }; + let deliveredAsAside = false; if (options?.deliverAs === "aside") { if (this.isStreaming) { - await this.#queueUserMessage(text, images, "aside", { attribution: options.attribution }); + await this.#queueUserMessage(queuedText, images, "aside", queueOptions); return; } // Idle: fall through to the prompt flow below (starts a turn, like an omitted // deliverAs) — there is no live run to inject an aside into. deliveredAsAside = true; } else if (options?.deliverAs === "followUp") { - await this.#queueUserMessage(text, images, "followUp", { attribution: options.attribution }); + await this.#queueUserMessage(queuedText, images, "followUp", queueOptions); return; } else if (options?.deliverAs === "steer") { - await this.#queueUserMessage(text, images, "steer", { attribution: options.attribution }); + await this.#queueUserMessage(queuedText, images, "steer", queueOptions); return; } - // Use prompt() with expandPromptTemplates: false to skip command handling and template - // expansion. prompt() awaits manual-compaction cleanup and (on the non-streaming path) + // prompt() handles commands and templates only when expandPromptTemplates is set. + // It awaits manual-compaction cleanup and (on the non-streaming path) // image normalization/vision description before dispatching, so a stream can start in // that gap; prompt() re-checks isStreaming at each await boundary and queues via // `streamingBehavior` when it does. Passing "aside" through (instead of hard-coding @@ -8966,7 +9029,7 @@ export class AgentSession implements SettingsScope { // tool-batch-aborting steer. await this.prompt(text, { attribution: options?.attribution, - expandPromptTemplates: false, + expandPromptTemplates: expand, images, streamingBehavior: deliveredAsAside ? "aside" : "steer", }); diff --git a/packages/coding-agent/test/agent-session-send-user-message-expand.test.ts b/packages/coding-agent/test/agent-session-send-user-message-expand.test.ts new file mode 100644 index 000000000000..93d13e737be4 --- /dev/null +++ b/packages/coding-agent/test/agent-session-send-user-message-expand.test.ts @@ -0,0 +1,200 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import * as path from "node:path"; +import { Agent } from "@oh-my-pi/pi-agent-core"; +import { AssistantMessageEventStream } from "@oh-my-pi/pi-ai/utils/event-stream"; +import { getBundledModel } from "@oh-my-pi/pi-catalog/models"; +import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import * as skillsModule from "@oh-my-pi/pi-coding-agent/extensibility/skills"; +import * as imageLoading from "@oh-my-pi/pi-coding-agent/utils/image-loading"; +import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; +import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; +import { convertToLlm, SKILL_PROMPT_MESSAGE_TYPE } from "@oh-my-pi/pi-coding-agent/session/messages"; +import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; +import { TempDir } from "@oh-my-pi/pi-utils"; +import { createAssistantMessage } from "./helpers/agent-session-setup"; + +const SKILL_BODY = "Demo skill body: count the widgets."; + +describe("AgentSession.sendUserMessage expandPromptTemplates", () => { + let tempDir: TempDir; + let authStorage: AuthStorage | undefined; + let session: AgentSession; + /** Set by a test to hold the next model turn open until it resolves `release`. */ + let heldTurn: { started: PromiseWithResolvers; release: PromiseWithResolvers } | undefined; + const observedTurns: string[] = []; + const observedRoles: string[] = []; + let skillsSettings: { enableSkillCommands: boolean }; + + beforeEach(async () => { + tempDir = TempDir.createSync("@pi-send-user-message-expand-"); + observedTurns.length = 0; + observedRoles.length = 0; + heldTurn = undefined; + skillsSettings = { enableSkillCommands: true }; + const skillDir = path.join(tempDir.path(), "demo"); + const skillPath = path.join(skillDir, "SKILL.md"); + await Bun.write(skillPath, `---\nname: demo\ndescription: Demo skill\n---\n\n${SKILL_BODY}\n`); + + authStorage = await AuthStorage.create(":memory:"); + authStorage.keys.setRuntime("anthropic", "test-key"); + const model = getBundledModel("anthropic", "claude-sonnet-4-5"); + if (!model) throw new Error("Expected claude-sonnet-4-5 model to exist"); + + const agent = new Agent({ + getApiKey: () => "test-key", + initialState: { model, systemPrompt: ["Test"], tools: [], messages: [] }, + convertToLlm, + streamFn: (_model, context) => { + const last = context.messages.at(-1); + const content = last?.content; + observedRoles.push(last?.role ?? ""); + observedTurns.push( + typeof content === "string" + ? content + : Array.isArray(content) + ? content.map(block => (block.type === "text" ? block.text : "")).join("\n") + : "", + ); + const stream = new AssistantMessageEventStream(); + const finish = () => { + const response = createAssistantMessage("done"); + stream.push({ type: "start", partial: response }); + stream.push({ type: "done", reason: "stop", message: response }); + }; + const hold = heldTurn; + heldTurn = undefined; + if (hold) { + hold.release.promise.then(finish); + hold.started.resolve(); + } else queueMicrotask(finish); + return stream; + }, + }); + + session = new AgentSession({ + agent, + sessionManager: SessionManager.inMemory(tempDir.path()), + settings: Settings.isolated({ "compaction.enabled": false }), + modelRegistry: new ModelRegistry(authStorage), + skills: [{ name: "demo", description: "Demo skill", filePath: skillPath, baseDir: skillDir, source: "test" }], + skillsSettings, + promptTemplates: [{ name: "tpl", description: "Demo template", content: "TPL BODY $1", source: "(test)" }], + }); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + await session?.dispose(); + authStorage?.close(); + authStorage = undefined; + tempDir.removeSync(); + }); + + it("keeps extension text literal by default and expands a /skill: command on opt-in", async () => { + await session.sendUserMessage("/skill:demo first"); + await session.waitForIdle(); + await session.sendUserMessage("/skill:demo second", { expandPromptTemplates: true }); + await session.waitForIdle(); + + expect(observedTurns[0]).toBe("/skill:demo first"); + expect(observedTurns[1]).toContain(SKILL_BODY); + expect(observedTurns[1]).toContain("second"); + }); + + it("keeps /skill: text literal when skill commands are disabled", async () => { + skillsSettings.enableSkillCommands = false; + await session.sendUserMessage("/skill:demo off", { expandPromptTemplates: true }); + await session.waitForIdle(); + + expect(observedTurns).toEqual(["/skill:demo off"]); + }); + + it("queues an idle explicit follow-up skill without starting a turn", async () => { + await session.sendUserMessage("/skill:demo later", { expandPromptTemplates: true, deliverAs: "followUp" }); + + expect(observedTurns).toHaveLength(0); + expect(session.queuedMessageCount).toBe(1); + }); + + it("queues an expanded prompt template that the submitted text can still remove", async () => { + await session.sendUserMessage("/tpl x", { expandPromptTemplates: true, deliverAs: "steer" }); + + expect(session.getQueuedMessages().steering).toEqual(["TPL BODY x"]); + expect(session.removeQueuedMessage("/tpl x", "steering")).toBe(true); + expect(session.queuedMessageCount).toBe(0); + }); + + it("drops a skill send when abort lands while SKILL.md is read", async () => { + const read = Promise.withResolvers(); + const build = skillsModule.buildSkillPromptMessage; + vi.spyOn(skillsModule, "buildSkillPromptMessage").mockImplementation(async (...args) => { + await read.promise; + return build(...args); + }); + const send = session.sendUserMessage("/skill:demo late", { expandPromptTemplates: true }); + await session.abort(); + read.resolve(); + await send; + await session.waitForIdle(); + + expect(observedTurns).toHaveLength(0); + }); + + for (const [label, interrupt] of [ + ["abort", (target: AgentSession) => target.abort()], + ["disposal", (target: AgentSession) => target.beginDispose()], + ] as const) { + it(`reports a queued custom message dropped by ${label} during image normalization`, async () => { + const normalizing = Promise.withResolvers(); + const normalized = Promise.withResolvers(); + vi.spyOn(imageLoading, "normalizeModelContextImages").mockImplementation(async images => { + normalizing.resolve(); + await normalized.promise; + return images; + }); + const send = session.promptCustomMessage( + { + customType: SKILL_PROMPT_MESSAGE_TYPE, + content: [ + { type: "text", text: "skill body" }, + { type: "image", data: "aW1n", mimeType: "image/png" }, + ], + display: true, + attribution: "user", + }, + { streamingBehavior: "followUp", queueOnly: true }, + ); + await normalizing.promise; + await interrupt(session); + normalized.resolve(); + + expect(await send).toBe(false); + expect(session.queuedMessageCount).toBe(0); + expect(observedTurns).toHaveLength(0); + }); + } + + it("delivers an agent-attributed expanded skill as a user-role message", async () => { + await session.sendUserMessage("/skill:demo relay", { expandPromptTemplates: true, attribution: "agent" }); + await session.waitForIdle(); + + expect(observedRoles).toEqual(["user"]); + expect(observedTurns[0]).toContain(SKILL_BODY); + }); + + it("expands a /skill: command queued as a follow-up while the agent is streaming", async () => { + const hold = { started: Promise.withResolvers(), release: Promise.withResolvers() }; + heldTurn = hold; + const first = session.sendUserMessage("start"); + await hold.started.promise; + await session.sendUserMessage("/skill:demo queued", { expandPromptTemplates: true, deliverAs: "followUp" }); + hold.release.resolve(); + await first; + await session.waitForIdle(); + + expect(observedTurns).toHaveLength(2); + expect(observedTurns[1]).toContain(SKILL_BODY); + expect(observedTurns[1]).toContain("queued"); + }); +});