diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d97a1f633..3acedc147 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `xd://` mount notices re-announcing already-known devices on session resume / host reconnect: the notice was diff-gated only against the in-memory mount set, which reset each resume, so reconnecting MCP/RPC-host devices re-spliced a redundant developer message into history and busted the provider prompt-cache prefix (re-billing the whole suffix at full price on metered providers). Notices now carry a structured `{ added, removed }` payload and are gated against the devices persisted history already announced, so a resume that re-establishes the same inventory emits nothing ([#6921](https://github.com/can1357/oh-my-pi/issues/6921)). + ## [17.1.8] - 2026-07-28 ### Breaking Changes diff --git a/packages/coding-agent/src/session/session-tools.ts b/packages/coding-agent/src/session/session-tools.ts index b29c51bc0..1c4974bf8 100644 --- a/packages/coding-agent/src/session/session-tools.ts +++ b/packages/coding-agent/src/session/session-tools.ts @@ -159,6 +159,18 @@ export function projectMountedMCPXdevGuidance(routes: Iterable(); #xdev: XdevState | undefined; #pendingXdevMountDelta: { added: Set; removed: Set } | undefined; + /** + * Dynamic (`xd://`) devices the model has already been told are mounted. + * Seeded lazily from persisted history on resume (see + * {@link #ensureAnnouncedMountsSeeded}) and updated as notices are emitted, so + * a host reconnect that re-mounts the same device does not re-announce it. + */ + #announcedMounts = new Set(); + #announcedMountsSeeded = false; #presentationPinnedToolNames: ReadonlySet | undefined; #runtimeSelectedToolNames: ReadonlySet | undefined; #baseSystemPrompt: string[]; @@ -674,26 +694,56 @@ export class SessionTools { this.#host.emitNotice("info", `xd://: ${parts.join("; ")}`, "xdev"); } + /** + * Seed {@link #announcedMounts} from persisted mount notices the first time a + * notice is consumed. On resume the in-memory mount set is rebuilt from + * scratch, so without replaying history every already-announced dynamic device + * would look freshly mounted and re-announce. + */ + #ensureAnnouncedMountsSeeded(): void { + if (this.#announcedMountsSeeded) return; + this.#announcedMountsSeeded = true; + for (const message of this.#host.agent.state.messages) { + if (message.role !== "custom" || message.customType !== XDEV_MOUNT_NOTICE_MESSAGE_TYPE) continue; + const details = message.details as XdevMountNoticeDetails | undefined; + if (!details) continue; + for (const name of details.added ?? []) this.#announcedMounts.add(name); + for (const name of details.removed ?? []) this.#announcedMounts.delete(name); + } + } + /** Consumes the hidden notice for unannounced `xd://` mount changes. */ - takePendingXdevMountNotice(): CustomMessage | undefined { + takePendingXdevMountNotice(): CustomMessage | undefined { const pending = this.#pendingXdevMountDelta; if (!pending) return undefined; this.#pendingXdevMountDelta = undefined; + this.#ensureAnnouncedMountsSeeded(); + // 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 + // device — the common resume/reconnect case — and an unmount for a device + // it was never told about are both suppressed, keeping the provider prompt + // cache prefix byte-stable across resumes. + const addedNames = [...pending.added].filter(name => !this.#announcedMounts.has(name)); + const removedNames = [...pending.removed].filter(name => this.#announcedMounts.has(name)); + if (addedNames.length === 0 && removedNames.length === 0) return undefined; const summaries = new Map(this.#xdev ? xdevEntries(this.#xdev).map(entry => [entry.name, entry.summary]) : []); - const added = [...pending.added].map(name => ({ name, summary: summaries.get(name) ?? "" })); - const removed = [...pending.removed].map(name => ({ name })); + const added = addedNames.map(name => ({ name, summary: summaries.get(name) ?? "" })); + const removed = removedNames.map(name => ({ name })); const docs = this.#xdev ? xdevDocsFor( this.#xdev, - pending.added, + new Set(addedNames), this.#host.settings.get("tools.xdevDocs"), this.#host.settings.get("tools.xdevInlineDevices"), ) : ""; + for (const name of addedNames) this.#announcedMounts.add(name); + for (const name of removedNames) this.#announcedMounts.delete(name); return { role: "custom", customType: XDEV_MOUNT_NOTICE_MESSAGE_TYPE, content: prompt.render(xdevMountNoticePrompt, { added, removed, docs }), + details: { added: addedNames, removed: removedNames }, attribution: "agent", display: false, timestamp: Date.now(), 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 ff09072c0..c82931cb7 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 @@ -1,12 +1,12 @@ import { afterEach, describe, expect, it, vi } from "bun:test"; -import { Agent, type AgentTool } from "@oh-my-pi/pi-agent-core"; +import { Agent, type AgentMessage, type AgentTool } from "@oh-my-pi/pi-agent-core"; import type { Message, Model } from "@oh-my-pi/pi-ai"; import { createMockModel, type MockResponseSource } from "@oh-my-pi/pi-ai/providers/mock"; 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 { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; -import { convertToLlm } from "@oh-my-pi/pi-coding-agent/session/messages"; +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"; import { collectMountedMCPToolRoutes, @@ -102,6 +102,8 @@ describe("AgentSession refreshMCPTools rebuild skipping", () => { lazyWrite?: boolean; /** Scripted mock model responses; enables driving `session.prompt()`. */ responses?: MockResponseSource; + /** Persisted history seeded into the agent, e.g. to model a resumed session. */ + initialMessages?: AgentMessage[]; } function newSession( @@ -133,7 +135,7 @@ describe("AgentSession refreshMCPTools rebuild skipping", () => { ? [readTool, initialMcp as unknown as AgentTool] : [readTool, writeTool, initialMcp as unknown as AgentTool] : [readTool, initialMcp as unknown as AgentTool], - messages: [], + messages: options.initialMessages ?? [], }, convertToLlm, streamFn: mock @@ -926,6 +928,51 @@ describe("AgentSession refreshMCPTools rebuild skipping", () => { expect(notices[0]).not.toContain("No longer mounted"); }); + it("does not re-announce devices a resumed session already announced in history", async () => { + // Model a process resume / host reconnect: persisted history already carries + // a mount notice for mcp__nucleus_search, but the fresh in-memory mount set + // starts empty. When the device reconnects, the notice must NOT re-splice a + // redundant developer message — doing so busts the provider prompt-cache + // prefix and re-bills the whole suffix on metered providers. + const priorNotice: AgentMessage = { + role: "custom", + customType: "xdev-mount-notice", + content: "The xd:// device inventory changed.\n\nxd://mcp__nucleus_search became available.", + details: { added: ["mcp__nucleus_search"], removed: [] }, + attribution: "agent", + display: false, + timestamp: Date.now() - 1000, + }; + const { session, contexts } = newSession(async toolNames => `tools:${toolNames.join(",")}`, { + xdev: createTestXdevState(), + responses: [{ content: ["ok"] }, { content: ["ok"] }], + initialMessages: [priorNotice], + }); + const search = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search nucleus"); + const fetch = createMcpCustomTool("mcp__nucleus_fetch", "nucleus", "fetch", "Fetch nucleus"); + + // The already-announced device reconnects: no new notice is spliced in. + await session.refreshMCPTools([search]); + await session.prompt("hello"); + const afterReconnect = session.agent.state.messages.filter( + message => message.role === "custom" && message.customType === "xdev-mount-notice", + ); + expect(afterReconnect).toHaveLength(1); + expect(mountNoticesIn(contexts[0])).toHaveLength(1); // only the pre-existing history notice + + // A genuinely new device still announces, and only for itself. + await session.refreshMCPTools([search, fetch]); + await session.prompt("again"); + const afterNewDevice = session.agent.state.messages.filter( + (message): message is CustomMessage => message.role === "custom" && message.customType === "xdev-mount-notice", + ); + expect(afterNewDevice).toHaveLength(2); + const fetchNotice = afterNewDevice[1]; + const fetchText = typeof fetchNotice.content === "string" ? fetchNotice.content : ""; + expect(fetchText).toContain("xd://mcp__nucleus_fetch"); + expect(fetchText).not.toContain("xd://mcp__nucleus_search"); + }); + it("keeps xd:// mount deltas model-visible without rendering them during quiet startup", async () => { const { session, contexts } = newSession(async toolNames => `tools:${toolNames.join(",")}`, { xdev: createTestXdevState(),