fix(session): gated xd:// mount notices against announced history
Mount-notice injection diff-gated only against the in-memory mountedNames
set, which is reseeded on every process resume / host reconnect. Dynamic
devices (MCP / RPC host) already announced in persisted history therefore
re-announced, splicing a redundant developer message that busts the
provider prompt-cache prefix and re-bills the whole suffix at full price
on metered providers.
Notices now persist a structured { added, removed } payload. On the first
consumption after resume the announced-device baseline is reconstructed
from history, and only a net change relative to what the model already
knows is announced, so a resume re-establishing the same inventory emits
nothing.
Fixes #6921
(cherry picked from commit 03c2ed5189510f41431bd164fa80187a69ed8de9)
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -159,6 +159,18 @@ export function projectMountedMCPXdevGuidance(routes: Iterable<MountedMCPToolRou
|
||||
|
||||
const XDEV_MOUNT_NOTICE_MESSAGE_TYPE = "xdev-mount-notice";
|
||||
|
||||
/**
|
||||
* Structured payload persisted on each {@link XDEV_MOUNT_NOTICE_MESSAGE_TYPE}
|
||||
* custom message. Lets a resumed session reconstruct which dynamic devices the
|
||||
* model has already been told about, so reconnecting hosts do not re-announce
|
||||
* (and re-splice a redundant developer message that busts the provider
|
||||
* prompt-cache prefix).
|
||||
*/
|
||||
interface XdevMountNoticeDetails {
|
||||
added: string[];
|
||||
removed: string[];
|
||||
}
|
||||
|
||||
/** Owns tool registration, presentation, prompt rebuilding, skills, and permissions. */
|
||||
export class SessionTools {
|
||||
readonly #host: SessionToolsHost;
|
||||
@@ -172,6 +184,14 @@ export class SessionTools {
|
||||
#rpcHostToolNames = new Set<string>();
|
||||
#xdev: XdevState | undefined;
|
||||
#pendingXdevMountDelta: { added: Set<string>; removed: Set<string> } | 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<string>();
|
||||
#announcedMountsSeeded = false;
|
||||
#presentationPinnedToolNames: ReadonlySet<string> | undefined;
|
||||
#runtimeSelectedToolNames: ReadonlySet<string> | 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<XdevMountNoticeDetails> | 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(),
|
||||
|
||||
@@ -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(),
|
||||
|
||||
Reference in New Issue
Block a user