fix: prevent duplicate tools on provider toggle, suppress restore notifications, fix Windows test globs
- Prevent duplicate Brave tool entries when toggling providers repeatedly by filtering already-active tools before re-adding (BUG-1) - Remove single quotes from test glob patterns in package.json so Windows shell expands them correctly (BUG-2) - Fix test mock fire() to call all handlers instead of short-circuiting on first match, matching real framework behavior (BUG-3) - Suppress "Native Anthropic web search active" notification on session restore (source: "restore") to reduce UX noise (BUG-4) - Add regression tests for all 4 bugs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
e22a2f7622
commit
a595b9e28e
3 changed files with 129 additions and 5 deletions
|
|
@ -35,7 +35,7 @@
|
|||
"scripts": {
|
||||
"build": "tsc && npm run copy-themes",
|
||||
"copy-themes": "node -e \"const{mkdirSync,cpSync}=require('fs');const{resolve}=require('path');const src=resolve(__dirname,'node_modules/@mariozechner/pi-coding-agent/dist/modes/interactive/theme');mkdirSync('pkg/dist/modes/interactive/theme',{recursive:true});cpSync(src,'pkg/dist/modes/interactive/theme',{recursive:true})\"",
|
||||
"test": "node --import ./src/resources/extensions/gsd/tests/resolve-ts.mjs --experimental-strip-types --test 'src/resources/extensions/gsd/tests/*.test.ts' 'src/resources/extensions/gsd/tests/*.test.mjs' 'src/tests/*.test.ts'",
|
||||
"test": "node --import ./src/resources/extensions/gsd/tests/resolve-ts.mjs --experimental-strip-types --test src/resources/extensions/gsd/tests/*.test.ts src/resources/extensions/gsd/tests/*.test.mjs src/tests/*.test.ts",
|
||||
"dev": "tsc --watch",
|
||||
"postinstall": "node scripts/postinstall.js",
|
||||
"pi:install-global": "node scripts/install-pi-global.js",
|
||||
|
|
|
|||
|
|
@ -74,13 +74,17 @@ export function registerNativeSearchHooks(pi: NativeSearchPI): { getIsAnthropic:
|
|||
);
|
||||
} else if (!isAnthropicProvider && wasAnthropic && !hasBrave) {
|
||||
// Switching away from Anthropic without Brave — re-enable so the user
|
||||
// sees the "missing key" error rather than tools silently vanishing
|
||||
// sees the "missing key" error rather than tools silently vanishing.
|
||||
// Only add tools not already active to avoid duplicates on repeated toggles.
|
||||
const active = pi.getActiveTools();
|
||||
pi.setActiveTools([...active, ...BRAVE_TOOL_NAMES]);
|
||||
const toAdd = BRAVE_TOOL_NAMES.filter((t) => !active.includes(t));
|
||||
if (toAdd.length > 0) {
|
||||
pi.setActiveTools([...active, ...toAdd]);
|
||||
}
|
||||
}
|
||||
|
||||
// Show provider-aware diagnostics on first selection or provider change
|
||||
if (isAnthropicProvider && !wasAnthropic) {
|
||||
if (isAnthropicProvider && !wasAnthropic && event.source !== "restore") {
|
||||
ctx.ui.notify("Native Anthropic web search active", "info");
|
||||
} else if (!isAnthropicProvider && !hasBrave) {
|
||||
ctx.ui.notify(
|
||||
|
|
|
|||
|
|
@ -52,11 +52,14 @@ function createMockPI() {
|
|||
activeTools = tools;
|
||||
},
|
||||
async fire(event: string, eventData: any, ctx?: any) {
|
||||
let lastResult: any;
|
||||
for (const h of handlers) {
|
||||
if (h.event === event) {
|
||||
return await h.handler(eventData, ctx ?? mockCtx);
|
||||
const result = await h.handler(eventData, ctx ?? mockCtx);
|
||||
if (result !== undefined) lastResult = result;
|
||||
}
|
||||
}
|
||||
return lastResult;
|
||||
},
|
||||
};
|
||||
|
||||
|
|
@ -401,6 +404,123 @@ test("before_provider_request keeps Brave tools in payload when BRAVE_API_KEY se
|
|||
}
|
||||
});
|
||||
|
||||
// ─── BUG-1 regression: duplicate Brave tools on repeated provider toggle ────
|
||||
|
||||
test("model_select re-enable does not duplicate Brave tools across toggle cycles", async () => {
|
||||
const originalKey = process.env.BRAVE_API_KEY;
|
||||
delete process.env.BRAVE_API_KEY;
|
||||
|
||||
try {
|
||||
const pi = createMockPI();
|
||||
registerNativeSearchHooks(pi);
|
||||
|
||||
// Cycle 1: Anthropic disables Brave tools
|
||||
await pi.fire("model_select", {
|
||||
type: "model_select",
|
||||
model: { provider: "anthropic", name: "claude-sonnet-4-6" },
|
||||
previousModel: undefined,
|
||||
source: "set",
|
||||
});
|
||||
assert.ok(!pi.getActiveTools().includes("search-the-web"), "Disabled after 1st Anthropic select");
|
||||
|
||||
// Cycle 1: switch away re-enables
|
||||
await pi.fire("model_select", {
|
||||
type: "model_select",
|
||||
model: { provider: "openai", name: "gpt-4o" },
|
||||
previousModel: { provider: "anthropic", name: "claude-sonnet-4-6" },
|
||||
source: "set",
|
||||
});
|
||||
let active = pi.getActiveTools();
|
||||
assert.equal(
|
||||
active.filter((t) => t === "search-the-web").length, 1,
|
||||
"search-the-web exactly once after first re-enable"
|
||||
);
|
||||
|
||||
// Cycle 2: Anthropic again
|
||||
await pi.fire("model_select", {
|
||||
type: "model_select",
|
||||
model: { provider: "anthropic", name: "claude-sonnet-4-6" },
|
||||
previousModel: { provider: "openai", name: "gpt-4o" },
|
||||
source: "set",
|
||||
});
|
||||
|
||||
// Cycle 2: switch away again — must NOT accumulate duplicates
|
||||
await pi.fire("model_select", {
|
||||
type: "model_select",
|
||||
model: { provider: "openai", name: "gpt-4o" },
|
||||
previousModel: { provider: "anthropic", name: "claude-sonnet-4-6" },
|
||||
source: "set",
|
||||
});
|
||||
active = pi.getActiveTools();
|
||||
assert.equal(
|
||||
active.filter((t) => t === "search-the-web").length, 1,
|
||||
"search-the-web exactly once after second re-enable (no duplicates)"
|
||||
);
|
||||
assert.equal(
|
||||
active.filter((t) => t === "search_and_read").length, 1,
|
||||
"search_and_read exactly once (no duplicates)"
|
||||
);
|
||||
} finally {
|
||||
if (originalKey) process.env.BRAVE_API_KEY = originalKey;
|
||||
else delete process.env.BRAVE_API_KEY;
|
||||
}
|
||||
});
|
||||
|
||||
// ─── BUG-3 regression: mock fire() must call all handlers, not just first ───
|
||||
|
||||
test("mock fire() calls all handlers for the same event", async () => {
|
||||
const pi = createMockPI();
|
||||
const callOrder: number[] = [];
|
||||
|
||||
// Register two handlers for the same event
|
||||
pi.on("test_event", async () => { callOrder.push(1); return "first"; });
|
||||
pi.on("test_event", async () => { callOrder.push(2); return "second"; });
|
||||
|
||||
const result = await pi.fire("test_event", {});
|
||||
|
||||
assert.deepEqual(callOrder, [1, 2], "Both handlers should be called");
|
||||
assert.equal(result, "second", "Should return last non-undefined result");
|
||||
});
|
||||
|
||||
// ─── BUG-4 regression: no notification noise on session restore ─────────────
|
||||
|
||||
test("model_select suppresses 'Native search active' notification on session restore", async () => {
|
||||
const pi = createMockPI();
|
||||
registerNativeSearchHooks(pi);
|
||||
|
||||
await pi.fire("model_select", {
|
||||
type: "model_select",
|
||||
model: { provider: "anthropic", name: "claude-sonnet-4-6" },
|
||||
previousModel: undefined,
|
||||
source: "restore", // session restore, not user action
|
||||
});
|
||||
|
||||
const nativeNotif = pi.notifications.find(
|
||||
(n) => n.message.includes("Native Anthropic web search active")
|
||||
);
|
||||
assert.equal(
|
||||
nativeNotif, undefined,
|
||||
"Should NOT show 'Native search active' on session restore"
|
||||
);
|
||||
});
|
||||
|
||||
test("model_select DOES show notification on explicit user set", async () => {
|
||||
const pi = createMockPI();
|
||||
registerNativeSearchHooks(pi);
|
||||
|
||||
await pi.fire("model_select", {
|
||||
type: "model_select",
|
||||
model: { provider: "anthropic", name: "claude-sonnet-4-6" },
|
||||
previousModel: undefined,
|
||||
source: "set",
|
||||
});
|
||||
|
||||
const nativeNotif = pi.notifications.find(
|
||||
(n) => n.message.includes("Native Anthropic web search active")
|
||||
);
|
||||
assert.ok(nativeNotif, "Should show notification on explicit 'set' source");
|
||||
});
|
||||
|
||||
// ─── stripThinkingFromHistory tests ─────────────────────────────────────────
|
||||
|
||||
test("stripThinkingFromHistory removes thinking from earlier assistant messages", () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue