Merge PR #6922: fix(session): gate xd:// mount notices against announced history (@roboomp)
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -4341,6 +4341,7 @@ export class AgentSession {
|
||||
this.agent.clearDeferredToolDirectives();
|
||||
this.#toolChoiceQueue.clear();
|
||||
this.#tools.clearAcpPermissionDecisions();
|
||||
this.#tools.resetAnnouncedMounts();
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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<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,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 === "</system-notice>") break;
|
||||
if (line.startsWith("Read `xd://<tool>`")) {
|
||||
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<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,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: `<system-notice>
|
||||
The xd:// device inventory changed.
|
||||
These tools became available:
|
||||
- xd://mcp__nucleus_search — Search nucleus
|
||||
- xd://mcp__retired — Retired device
|
||||
Read \`xd://<tool>\` for docs + JSON schema before first use; write the JSON args object to \`xd://<tool>\` 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.
|
||||
</system-notice>`,
|
||||
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(),
|
||||
|
||||
Reference in New Issue
Block a user