fix(pi): correct the model's tool context after a project switch
Some checks failed
CI/CD / typecheck (pull_request) Successful in 1m13s
CI/CD / lint (pull_request) Successful in 2m30s
CI/CD / test (pull_request) Successful in 1m25s
CI/CD / build (pull_request) Successful in 2m28s
CI/CD / smoke (pull_request) Failing after 2m59s
CI/CD / publish (pull_request) Has been skipped
Some checks failed
CI/CD / typecheck (pull_request) Successful in 1m13s
CI/CD / lint (pull_request) Successful in 2m30s
CI/CD / test (pull_request) Successful in 1m25s
CI/CD / build (pull_request) Successful in 2m28s
CI/CD / smoke (pull_request) Failing after 2m59s
CI/CD / publish (pull_request) Has been skipped
Switching projects twice left the agent believing it had lost MCP access entirely: it kept calling the previous project's tool names, got "Tool mc_<old>_begin_session not found", and concluded no MCP tools existed. Root cause is a pi constraint, not a bug in the switch. pi has no way to unregister a tool — `registerTool` only ever does `extension.tools.set(name)` — so the previous project's `mc_*` tools stay registered and merely go inactive. `setActiveTools` correctly drops them from the live set, but the conversation still contains the old project's tool listing, so the model keeps calling names that now answer "not found". Nothing was telling the model the tool set had changed. Now the switch injects a custom message naming the active project, stating that previously listed mcpctl tool names are dead, and listing what is actually callable. Custom messages are converted to user-role messages by `convertToLlm`, so they do reach the model (unlike `appendEntry`, which is explicitly excluded from context). `display: false` keeps it out of the transcript — the notification is what the human reads. The gate is also called out explicitly, in both the notification and the injected message. A freshly switched project gets a new mcp-session-id and is therefore gated again, so "1 tool(s) ready" is correct but reads like a failure; it now says which begin_session call unlocks the rest. `toolChangeAnnouncement` is exported and unit-tested rather than left as wording only reachable through a TUI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017BMXdb2qZbPSh8Q7XpTyjB
This commit is contained in:
File diff suppressed because one or more lines are too long
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { filterProjects } from '../../../pi-ext/mcpctl-pi.js';
|
||||
import { filterProjects, toolChangeAnnouncement } from '../../../pi-ext/mcpctl-pi.js';
|
||||
|
||||
/**
|
||||
* pi's selector is a plain arrow-key list — no search — so the project picker
|
||||
@@ -71,3 +71,33 @@ describe('filterProjects', () => {
|
||||
expect(input).toEqual(PROJECTS);
|
||||
});
|
||||
});
|
||||
|
||||
describe('toolChangeAnnouncement', () => {
|
||||
it('names the active project and the live tools', () => {
|
||||
const msg = toolChangeAnnouncement('sre', ['mc_sre_begin_session']);
|
||||
expect(msg).toContain("active project is now 'sre'");
|
||||
expect(msg).toContain('mc_sre_begin_session');
|
||||
});
|
||||
|
||||
it('tells the model to ignore the previous project\'s tool names', () => {
|
||||
// pi cannot unregister tools, so stale names stay in the conversation and
|
||||
// the model keeps calling them until told otherwise.
|
||||
expect(toolChangeAnnouncement('sre', ['mc_sre_x'])).toMatch(/no longer callable|ignore them/);
|
||||
});
|
||||
|
||||
it('calls out gating so the model knows to unlock the rest', () => {
|
||||
const msg = toolChangeAnnouncement('sre', ['mc_sre_begin_session']);
|
||||
expect(msg).toContain('gated');
|
||||
expect(msg).toMatch(/call mc_sre_begin_session first/);
|
||||
});
|
||||
|
||||
it('omits the gate hint when the project is not gated', () => {
|
||||
const msg = toolChangeAnnouncement('docmost', ['mc_docmost_search', 'mc_docmost_get_page']);
|
||||
expect(msg).not.toContain('gated');
|
||||
expect(msg).toContain('mc_docmost_search, mc_docmost_get_page');
|
||||
});
|
||||
|
||||
it('is explicit when a project exposes nothing', () => {
|
||||
expect(toolChangeAnnouncement('empty', [])).toContain('No mcpctl tools are currently available');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -201,6 +201,28 @@ export function filterProjects(projects: string[], query: string, active: string
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* The message injected into the conversation after a project switch, telling
|
||||
* the model which mcpctl tools are live now.
|
||||
*
|
||||
* Exported so its wording is unit-tested; it is the only thing standing between
|
||||
* the model and a stale tool list it will otherwise keep calling.
|
||||
*/
|
||||
export function toolChangeAnnouncement(project: string, tools: string[]): string {
|
||||
const gate = tools.find((n) => n.endsWith("_begin_session"));
|
||||
const parts = [
|
||||
`[mcpctl] The active project is now '${project}'.`,
|
||||
"mcpctl tool names listed earlier in this conversation belong to the previously active project and are no longer callable — ignore them.",
|
||||
tools.length > 0
|
||||
? `Currently available mcpctl tools: ${tools.join(", ")}.`
|
||||
: "No mcpctl tools are currently available for this project.",
|
||||
];
|
||||
if (gate !== undefined) {
|
||||
parts.push(`This project is gated: call ${gate} first and its remaining tools become available.`);
|
||||
}
|
||||
return parts.join(" ");
|
||||
}
|
||||
|
||||
/** Sanitize a name for use as a pi tool name segment ([a-z0-9_]). */
|
||||
function safeSegment(name: string): string {
|
||||
return name.toLowerCase().replace(/[^a-z0-9_]+/g, "_").replace(/^_+|_+$/g, "") || "x";
|
||||
@@ -492,13 +514,45 @@ export default function (pi: ExtensionAPI) {
|
||||
await writePiState({ project: picked });
|
||||
try {
|
||||
const r = await reconcileTools(picked);
|
||||
ctx.ui.notify(`Switched to '${picked}': ${r.tools.length} tool(s) ready`, "info");
|
||||
const gate = r.tools.find((n) => n.endsWith("_begin_session"));
|
||||
ctx.ui.notify(
|
||||
`Switched to '${picked}': ${String(r.tools.length)} tool(s) ready${gate ? ` — gated, call ${gate} to unlock the rest` : ""}`,
|
||||
"info",
|
||||
);
|
||||
setStatus();
|
||||
announceToolChange(picked, r.tools);
|
||||
} catch (e) {
|
||||
ctx.ui.notify(`Could not load tools for '${picked}': ${(e as Error).message}`, "error");
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Tell the *model* that the tool set changed.
|
||||
*
|
||||
* pi cannot unregister a tool — `registerTool` only ever adds to the
|
||||
* extension's tool map — so the previous project's `mc_*` tools stay
|
||||
* registered and merely go inactive. Meanwhile the conversation still
|
||||
* contains the old project's tool list, so the model keeps calling names that
|
||||
* now answer "Tool ... not found" and concludes it has lost MCP access
|
||||
* entirely, which is what happens in practice on the second switch.
|
||||
*
|
||||
* A custom message is converted to a user-role message and does reach the
|
||||
* LLM (unlike `appendEntry`, which is explicitly excluded from context), so
|
||||
* this corrects the stale context instead of leaving the model to guess.
|
||||
* `display: false` keeps it out of the transcript — the notify above is what
|
||||
* the human reads.
|
||||
*/
|
||||
function announceToolChange(project: string, tools: string[]): void {
|
||||
try {
|
||||
pi.sendMessage(
|
||||
{ customType: "mcpctl-project-switch", content: toolChangeAnnouncement(project, tools), display: false },
|
||||
{ triggerTurn: false, deliverAs: "nextTurn" },
|
||||
);
|
||||
} catch {
|
||||
// Older pi without sendMessage — the notify above still informs the user.
|
||||
}
|
||||
}
|
||||
|
||||
// ── session lifecycle ──
|
||||
pi.on("session_start", async (_event, ctx) => {
|
||||
activeCtx = ctx;
|
||||
|
||||
Reference in New Issue
Block a user