diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index b90018a26..62c3f96fb 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -10,6 +10,7 @@ - Fixed legacy pi extensions importing `parseArgs` or `CONFIG_DIR_NAME` from the `@earendil-works/pi-coding-agent` package root failing Bun's static export check during validation, which blocked installs such as `omp install npm:pi-cursor-sdk` ([#6907](https://github.com/can1357/oh-my-pi/pull/6907) by [@Gy-Hu](https://github.com/Gy-Hu)). - Fixed ACP clients (e.g. Zed) not learning about a model change that happens from inside the agent loop — prewalk hand-offs, retry-fallback, model cycling — so the client's model picker/status bar kept showing the session's starting model even though the active model had actually switched. `AgentSession` now emits a `model_changed` event from its single model-mutation choke point, and the ACP surface's lifetime subscription re-pushes `config_option_update` on it. The same event now also keeps the TUI status line and collab guests' footer state in sync, and a failed session switch that rolls back to the previous model emits a corrective event so subscribers don't keep advertising the never-committed target. - Fixed the assistant-facing available-resource summary omitting parameterized MCP resource templates, so a failed `mcp://` read now lists templates alongside concrete resources ([#6911](https://github.com/can1357/oh-my-pi/issues/6911)). +- 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—including legacy rendered notices from before the structured payload—so a resume that re-establishes the same inventory emits nothing. The announced baseline is reset when the transcript is replaced (`/new`, `switchSession`, `branch`), so a device reconnecting into the fresh history announces again ([#6921](https://github.com/can1357/oh-my-pi/issues/6921)). ## [17.1.8] - 2026-07-28 diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 871b49d87..903718616 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -4341,6 +4341,7 @@ export class AgentSession { this.agent.clearDeferredToolDirectives(); this.#toolChoiceQueue.clear(); this.#tools.clearAcpPermissionDecisions(); + this.#tools.resetAnnouncedMounts(); } /** diff --git a/packages/coding-agent/src/session/session-tools.ts b/packages/coding-agent/src/session/session-tools.ts index b29c51bc0..4479cffa2 100644 --- a/packages/coding-agent/src/session/session-tools.ts +++ b/packages/coding-agent/src/session/session-tools.ts @@ -1,6 +1,6 @@ import type { Agent, AgentTool } from "@oh-my-pi/pi-agent-core"; import type { Model } from "@oh-my-pi/pi-ai"; -import { logger, prompt, stringProperty } from "@oh-my-pi/pi-utils"; +import { isRecord, logger, prompt, stringProperty } from "@oh-my-pi/pi-utils"; import { reset as resetCapabilities } from "../capability"; import type { ModelRegistry } from "../config/model-registry"; import { formatModelString } from "../config/model-resolver"; @@ -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,108 @@ export class SessionTools { this.#host.emitNotice("info", `xd://: ${parts.join("; ")}`, "xdev"); } + /** + * Forget the announced-mount baseline for a replaced transcript. Called when + * session history is swapped wholesale (`/new`, `switchSession`, `branch`): the + * previous transcript's persisted notices no longer apply, so the next notice + * re-seeds from the new history and a device reconnecting into it announces + * again. + * + * The pending delta is deliberately preserved: it holds mounts that are still + * live but not yet delivered to the model, and `branch()` does not rebuild the + * base system prompt, so dropping it would leave the branched transcript + * unaware of a still-mounted device that no later refresh would re-queue. + */ + resetAnnouncedMounts(): void { + this.#announcedMounts.clear(); + this.#announcedMountsSeeded = false; + } + + /** + * 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; + if ( + isRecord(details) && + Array.isArray(details.added) && + details.added.every(name => typeof name === "string") && + Array.isArray(details.removed) && + details.removed.every(name => typeof name === "string") + ) { + for (const name of details.added) this.#announcedMounts.add(name); + for (const name of details.removed) this.#announcedMounts.delete(name); + continue; + } + + // Releases before structured notice details persisted only the rendered + // prompt. Replay its two stable inventory sections so the first resume + // after upgrading does not re-announce every dynamic device once. + if (typeof message.content !== "string") continue; + let section: "added" | "removed" | undefined; + for (const line of message.content.split("\n")) { + if (line === "These tools became available:") { + section = "added"; + continue; + } + if (line.startsWith("No longer mounted")) { + section = "removed"; + continue; + } + if (line === "Configured inline device docs:" || line === "") break; + if (line.startsWith("Read `xd://`")) { + section = undefined; + continue; + } + if (!section) continue; + const match = /^- xd:\/\/(\S+?)(?:\s+—|$)/.exec(line); + const name = match?.[1]; + if (!name) continue; + if (section === "added") this.#announcedMounts.add(name); + else 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..aa5dc2209 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,139 @@ describe("AgentSession refreshMCPTools rebuild skipping", () => { expect(notices[0]).not.toContain("No longer mounted"); }); + it.each([ + { + priorNotice: { + 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: 1, + } satisfies AgentMessage, + }, + { + priorNotice: { + role: "custom", + customType: "xdev-mount-notice", + content: ` +The xd:// device inventory changed. +These tools became available: +- xd://mcp__nucleus_search — Search nucleus +- xd://mcp__retired — Retired device +Read \`xd://\` for docs + JSON schema before first use; write the JSON args object to \`xd://\` to execute. +No longer mounted (writes to these devices will fail): +- xd://mcp__retired +Configured inline device docs: +These tools became available: +- xd://mcp__nucleus_fetch — This is inline documentation, not an inventory entry. +`, + attribution: "agent", + display: false, + timestamp: 1, + } satisfies AgentMessage, + }, + ])("does not re-announce devices a resumed session already announced in history", async ({ priorNotice }) => { + // 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 { 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("re-announces a device after the transcript is replaced by /new", async () => { + const xdev = createTestXdevState(); + const { session } = newSession(async toolNames => `tools:${toolNames.join(",")}`, { + xdev, + responses: [{ content: ["ok"] }, { content: ["ok"] }], + }); + const search = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search nucleus"); + + // Announce the device in the original transcript. + await session.refreshMCPTools([search]); + await session.prompt("hello"); + expect( + session.agent.state.messages.filter( + message => message.role === "custom" && message.customType === "xdev-mount-notice", + ), + ).toHaveLength(1); + + // /new swaps in a fresh transcript that no longer carries the notice. A + // resume/reconnect rebuilds the mount set from scratch, so model the device + // dropping out across the boundary. + await session.newSession(); + xdev.mountedNames.clear(); + + // The same device reconnects into the new transcript: because the announced + // baseline was reset with the transcript, it must announce again (otherwise + // the new conversation never learns the device is available). + await session.refreshMCPTools([search]); + await session.prompt("world"); + const newTranscriptNotices = session.agent.state.messages.filter( + (message): message is CustomMessage => message.role === "custom" && message.customType === "xdev-mount-notice", + ); + expect(newTranscriptNotices).toHaveLength(1); + const text = typeof newTranscriptNotices[0].content === "string" ? newTranscriptNotices[0].content : ""; + expect(text).toContain("xd://mcp__nucleus_search"); + }); + + it("preserves an undelivered mount notice across a branch that does not rebuild the prompt", async () => { + const { session } = newSession(async toolNames => `tools:${toolNames.join(",")}`, { + xdev: createTestXdevState(), + responses: [{ content: ["ok"] }, { content: ["ok"] }], + }); + session.subscribe(() => {}); + const search = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search nucleus"); + + // A user turn establishes a branch point. + await session.prompt("first"); + // The device mounts but the user branches before the next prompt consumes + // its queued notice. `branch()` does not rebuild the base system prompt, so + // the delta is the only channel that can tell the branched transcript the + // device exists. + await session.refreshMCPTools([search]); + const branchable = session.getUserMessagesForBranching(); + expect(branchable.length).toBeGreaterThan(0); + await session.branch(branchable[0].entryId); + + await session.prompt("second"); + const notices = session.agent.state.messages.filter( + (message): message is CustomMessage => message.role === "custom" && message.customType === "xdev-mount-notice", + ); + expect(notices).toHaveLength(1); + const text = typeof notices[0].content === "string" ? notices[0].content : ""; + expect(text).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(),