diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index e5658acac..da3a30ff4 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -4039,10 +4039,6 @@ export class AgentSession { return this.#tools.applyActiveToolsByName(toolNames); } - #takePendingXdevMountNotice(): CustomMessage | undefined { - return this.#tools.takePendingXdevMountNotice(); - } - /** Rediscovers reloadable skills and refreshes prompt metadata. */ refreshSkills(): Promise { return this.#tools.refreshSkills(); @@ -5018,11 +5014,10 @@ export class AgentSession { } // A pending xd:// delta accompanies the next user-authored prompt, - // never an agent-initiated continuation. - const xdevMountNotice = isUserQueuedMessage(message) ? this.#takePendingXdevMountNotice() : undefined; - if (xdevMountNotice) { - messages.push(xdevMountNotice); - } + // never an agent-initiated continuation. Reserve its pre-user position, + // but consume it only after before_agent_start determines whether the + // final provider prompt still carries the base xd:// catalog. + const xdevMountNoticeIndex = messages.length; messages.push(message); // Inject any pending "nextTurn" messages as context alongside the user message for (const msg of this.#pendingNextTurnMessages) { @@ -5053,6 +5048,7 @@ export class AgentSession { if ((this.#isDisposed && !disposingBeforeTransition) || this.#promptGeneration !== generation) return; const beforeAgentStartSystemPrompt = await this.#buildSystemPromptForAgentStart(expandedText); + let baseXdevCatalogDelivered = true; // Emit before_agent_start extension event if (this.#extensionRunner) { const result = await this.#extensionRunner.emitBeforeAgentStart( @@ -5087,6 +5083,7 @@ export class AgentSession { } if (result?.systemPrompt !== undefined) { + baseXdevCatalogDelivered = false; this.agent.setSystemPrompt(result.systemPrompt); } else { this.agent.setSystemPrompt(beforeAgentStartSystemPrompt); @@ -5110,6 +5107,12 @@ export class AgentSession { return; } } + const xdevMountNotice = isUserQueuedMessage(message) + ? this.#tools.takePendingXdevMountNotice(baseXdevCatalogDelivered) + : undefined; + if (xdevMountNotice) { + messages.splice(xdevMountNoticeIndex, 0, xdevMountNotice); + } await this.#maintenance.runPrePromptCompactionIfNeeded(messages); if (this.#promptGeneration !== generation) { diff --git a/packages/coding-agent/src/session/session-tools.ts b/packages/coding-agent/src/session/session-tools.ts index cf7b1642f..56ea27b92 100644 --- a/packages/coding-agent/src/session/session-tools.ts +++ b/packages/coding-agent/src/session/session-tools.ts @@ -779,20 +779,23 @@ export class SessionTools { } /** Consumes the hidden notice for unannounced `xd://` mount changes. */ - takePendingXdevMountNotice(): CustomMessage | undefined { + takePendingXdevMountNotice(baseCatalogDelivered: boolean): CustomMessage | undefined { const pending = this.#pendingXdevMountDelta; if (!pending) return undefined; this.#pendingXdevMountDelta = undefined; this.#ensureAnnouncedMountsSeeded(); // A pending add for a device the outgoing base prompt already lists in its - // catalog needs no notice line — but the delivery makes the model aware of - // it, so record it announced. Doing this here (at delivery) rather than when - // the prompt was rebuilt keeps add/remove coalescing intact: a device mounted - // then unmounted before any prompt is sent cancels out in - // {@link #notifyXdevMountDelta} and never emits a spurious "No longer mounted" - // notice for a device the model never saw (issue #7139 review). - for (const name of pending.added) { - if (this.#basePromptXdevNames.has(name)) this.#announcedMounts.add(name); + // catalog needs no notice line — but only when the final provider prompt + // still carries that base catalog. A `before_agent_start` replacement drops + // it, so its additions must remain in the notice. Record prompt-carried + // devices announced here, after the final prompt is known and immediately + // before delivery. The pending delta remains untouched by rebuilds, letting + // {@link #notifyXdevMountDelta} cancel a mount followed by an unmount before + // any request is sent (issue #7139 reviews). + if (baseCatalogDelivered) { + for (const name of pending.added) { + if (this.#basePromptXdevNames.has(name)) this.#announcedMounts.add(name); + } } // Only announce a net change relative to what the model already knows (from // this session and persisted history): a re-mount of an already-announced diff --git a/packages/coding-agent/test/agent-session-tool-rebuild-skip.test.ts b/packages/coding-agent/test/agent-session-tool-rebuild-skip.test.ts index dc3471b6b..0cebefd9c 100644 --- a/packages/coding-agent/test/agent-session-tool-rebuild-skip.test.ts +++ b/packages/coding-agent/test/agent-session-tool-rebuild-skip.test.ts @@ -5,6 +5,7 @@ import { createMockModel, type MockResponseSource } from "@oh-my-pi/pi-ai/provid import { buildModel } from "@oh-my-pi/pi-catalog/build"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import type { CustomTool } from "@oh-my-pi/pi-coding-agent/extensibility/custom-tools/types"; +import type { ExtensionRunner } from "@oh-my-pi/pi-coding-agent/extensibility/extensions"; import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; import { type CustomMessage, convertToLlm } from "@oh-my-pi/pi-coding-agent/session/messages"; import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; @@ -110,6 +111,8 @@ describe("AgentSession refreshMCPTools rebuild skipping", () => { * path. Existing tests leave this off, so their rebuild carries no catalog. */ exposeXdevCatalog?: boolean; + /** Optional per-turn system prompt replacement returned by before_agent_start. */ + beforeAgentStartSystemPrompt?: string[]; } function newSession( @@ -119,6 +122,8 @@ describe("AgentSession refreshMCPTools rebuild skipping", () => { session: AgentSession; /** Provider-call message snapshots (LLM-converted), one per model request. */ contexts: Message[][]; + /** Provider-call system prompt snapshots, one per model request. */ + systemPrompts: string[][]; /** Mutable registry shared with the session, for lifecycle-only mount fixtures. */ toolRegistry: Map; } { @@ -131,6 +136,7 @@ describe("AgentSession refreshMCPTools rebuild skipping", () => { if (options.xdev && !options.lazyWrite) toolRegistry.set(writeTool.name, writeTool); const mock = options.responses ? createMockModel({ responses: options.responses }) : undefined; const contexts: Message[][] = []; + const systemPrompts: string[][] = []; const agent = new Agent({ getApiKey: () => "test-key", initialState: { @@ -147,6 +153,7 @@ describe("AgentSession refreshMCPTools rebuild skipping", () => { streamFn: mock ? (model, context, streamOptions) => { contexts.push([...context.messages]); + systemPrompts.push([...(context.systemPrompt ?? [])]); return mock.stream(model, context, streamOptions); } : undefined, @@ -163,6 +170,12 @@ describe("AgentSession refreshMCPTools rebuild skipping", () => { if (!toolRegistry.has("write")) toolRegistry.set("write", writeTool); return true; }, + extensionRunner: options.beforeAgentStartSystemPrompt + ? ({ + emitBeforeAgentStart: async () => ({ systemPrompt: options.beforeAgentStartSystemPrompt }), + emit: async () => undefined, + } as unknown as ExtensionRunner) + : undefined, rebuildSystemPrompt: async (toolNames, _tools) => { const base = await rebuildSystemPrompt(toolNames); if (!options.exposeXdevCatalog) return { systemPrompt: [base] }; @@ -174,7 +187,7 @@ describe("AgentSession refreshMCPTools rebuild skipping", () => { xdev: options.xdev, }); sessions.push(session); - return { session, contexts, toolRegistry }; + return { session, contexts, systemPrompts, toolRegistry }; } it("skips rebuild when an MCP refresh produces an identical tool set", async () => { @@ -1035,6 +1048,28 @@ These tools became available: ).toHaveLength(0); }); + it("keeps the mount notice when before_agent_start replaces the catalog prompt (#7139)", async () => { + const replacementPrompt = ["extension replacement"]; + const { session, contexts, systemPrompts } = newSession(async toolNames => `tools:${toolNames.join(",")}`, { + xdev: createTestXdevState(), + responses: [{ content: ["ok"] }], + exposeXdevCatalog: true, + beforeAgentStartSystemPrompt: replacementPrompt, + }); + const search = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search nucleus"); + + // The base prompt rebuild exposes the device, but the per-turn extension + // replaces that prompt before the provider call. The mount notice is now + // the only channel making the newly mounted device visible on this turn. + await session.refreshMCPTools([search]); + await session.prompt("hi"); + + expect(systemPrompts[0]).toEqual(replacementPrompt); + const notices = mountNoticesIn(contexts[0]); + expect(notices).toHaveLength(1); + expect(notices[0]).toContain("xd://mcp__nucleus_search"); + }); + it("does not emit an unmount notice for a catalog device unmounted before delivery (#7139)", async () => { const { session } = newSession(async toolNames => `tools:${toolNames.join(",")}`, { xdev: createTestXdevState(),