smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(agent): resolve xd:// notice after final prompt override

Browse Source 
A before_agent_start extension can replace the base system prompt after the
mount notice was consumed. Catalog-backed additions were then marked
announced and suppressed even though the provider request no longer contained
the catalog.

Reserve the notice's pre-user message position, wait until the extension has
selected the final prompt, and suppress catalog-backed additions only when no
per-turn replacement dropped the base catalog. Explicit replacements retain
the mount notice as the device-discovery channel.

Fixes #7139
This commit is contained in:
roboomp
2026-07-31 02:45:21 +00:00
parent 0213f1c439
commit 4119bc461e
3 changed files with 60 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/session/agent-session.ts
+12 -9
View File
@@ -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<void> {
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) {
packages/coding-agent/src/session/session-tools.ts
+12 -9
View File
@@ -779,20 +779,23 @@ export class SessionTools {
}
/** Consumes the hidden notice for unannounced `xd://` mount changes. */
takePendingXdevMountNotice(): CustomMessage<XdevMountNoticeDetails> | undefined {
takePendingXdevMountNotice(baseCatalogDelivered: boolean): CustomMessage<XdevMountNoticeDetails> | 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
packages/coding-agent/test/agent-session-tool-rebuild-skip.test.ts
+36 -1
View File
@@ -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<string, AgentTool>;
} {
@@ -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(),