fix: prevent web_search tool injection for non-Anthropic providers serving Claude models (#444) (#446)
GitHub Copilot users with Claude models got 400 errors because the native Anthropic web_search_20250305 tool was injected into requests to Copilot's API proxy, which doesn't support it. The root cause was that model_select never fires before the first API request on new sessions, so the fallback heuristic (model name starts with "claude-") couldn't distinguish direct Anthropic from proxied providers. Fix: pass the resolved Model object through to the before_provider_request event so extensions can check model.provider directly instead of relying on model name heuristics.
This commit is contained in:
parent
861a06cf34
commit
16364c7dba
5 changed files with 72 additions and 13 deletions
|
|
@ -712,7 +712,7 @@ export class ExtensionRunner {
|
|||
return currentMessages;
|
||||
}
|
||||
|
||||
async emitBeforeProviderRequest(payload: unknown): Promise<unknown> {
|
||||
async emitBeforeProviderRequest(payload: unknown, model?: { provider: string; id: string }): Promise<unknown> {
|
||||
const ctx = this.createContext();
|
||||
let currentPayload = payload;
|
||||
|
||||
|
|
@ -725,6 +725,7 @@ export class ExtensionRunner {
|
|||
const event: BeforeProviderRequestEvent = {
|
||||
type: "before_provider_request",
|
||||
payload: currentPayload,
|
||||
model,
|
||||
};
|
||||
const handlerResult = await handler(event, ctx);
|
||||
if (handlerResult !== undefined) {
|
||||
|
|
|
|||
|
|
@ -506,6 +506,8 @@ export interface ContextEvent {
|
|||
export interface BeforeProviderRequestEvent {
|
||||
type: "before_provider_request";
|
||||
payload: unknown;
|
||||
/** The resolved model for this request (provider, id, etc.) */
|
||||
model?: { provider: string; id: string };
|
||||
}
|
||||
|
||||
/** Fired after user submits prompt but before agent loop. */
|
||||
|
|
|
|||
|
|
@ -292,12 +292,12 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
|
|||
tools: [],
|
||||
},
|
||||
convertToLlm: convertToLlmWithBlockImages,
|
||||
onPayload: async (payload, _model) => {
|
||||
onPayload: async (payload, currentModel) => {
|
||||
const runner = extensionRunnerRef.current;
|
||||
if (!runner?.hasHandlers("before_provider_request")) {
|
||||
return payload;
|
||||
}
|
||||
return runner.emitBeforeProviderRequest(payload);
|
||||
return runner.emitBeforeProviderRequest(payload, currentModel);
|
||||
},
|
||||
sessionId: sessionManager.getSessionId(),
|
||||
transformContext: async (messages) => {
|
||||
|
|
|
|||
|
|
@ -105,16 +105,21 @@ export function registerNativeSearchHooks(pi: NativeSearchPI): { getIsAnthropic:
|
|||
const payload = event.payload as Record<string, unknown>;
|
||||
if (!payload) return;
|
||||
|
||||
// Detect Anthropic provider. Prefer the model_select flag when available,
|
||||
// but fall back to checking the model name in the payload. model_select
|
||||
// may not fire when the session restores with the same model already set
|
||||
// (modelsAreEqual guard in the SDK suppresses the event). When model_select
|
||||
// HAS fired and said "not Anthropic" (e.g. Copilot serving claude-*),
|
||||
// respect that — don't override with model name heuristic.
|
||||
const modelName = typeof payload.model === "string" ? payload.model : "";
|
||||
const isAnthropic = modelSelectFired
|
||||
? isAnthropicProvider
|
||||
: modelName.startsWith("claude-");
|
||||
// Detect Anthropic provider. Use the model object from the event (most
|
||||
// reliable — comes directly from the resolved Model), then fall back to
|
||||
// the model_select flag, then to the model name heuristic (last resort).
|
||||
// The model name heuristic is needed for session restores where
|
||||
// modelsAreEqual suppresses model_select AND the SDK doesn't pass model.
|
||||
const eventModel = event.model as { provider: string } | undefined;
|
||||
let isAnthropic: boolean;
|
||||
if (eventModel?.provider) {
|
||||
isAnthropic = eventModel.provider === "anthropic";
|
||||
} else if (modelSelectFired) {
|
||||
isAnthropic = isAnthropicProvider;
|
||||
} else {
|
||||
const modelName = typeof payload.model === "string" ? payload.model : "";
|
||||
isAnthropic = modelName.startsWith("claude-");
|
||||
}
|
||||
if (!isAnthropic) return;
|
||||
|
||||
// Strip thinking blocks from history to avoid signature validation errors
|
||||
|
|
|
|||
|
|
@ -177,6 +177,57 @@ test("before_provider_request does NOT inject for claude model on non-Anthropic
|
|||
);
|
||||
});
|
||||
|
||||
// ─── Issue #444 regression: Copilot claude-* model without model_select ──────
|
||||
|
||||
test("before_provider_request does NOT inject when event.model indicates non-Anthropic provider (no model_select)", async () => {
|
||||
const pi = createMockPI();
|
||||
registerNativeSearchHooks(pi);
|
||||
|
||||
// NO model_select fired — simulates a new session where model was set before
|
||||
// extensions were bound. The event.model field from the SDK reveals the true provider.
|
||||
const payload: Record<string, unknown> = {
|
||||
model: "claude-sonnet-4-6-20250514",
|
||||
tools: [{ name: "bash", type: "custom" }],
|
||||
};
|
||||
|
||||
const result = await pi.fire("before_provider_request", {
|
||||
type: "before_provider_request",
|
||||
payload,
|
||||
model: { provider: "github-copilot", id: "claude-sonnet-4-6" },
|
||||
});
|
||||
|
||||
assert.equal(result, undefined, "Should not modify payload when event.model says non-Anthropic");
|
||||
const tools = payload.tools as any[];
|
||||
assert.equal(tools.length, 1, "Should not inject web_search for Copilot provider");
|
||||
assert.ok(
|
||||
!tools.some((t: any) => t.type === "web_search_20250305"),
|
||||
"web_search_20250305 must NOT be present for Copilot"
|
||||
);
|
||||
});
|
||||
|
||||
test("before_provider_request DOES inject when event.model indicates Anthropic provider (no model_select)", async () => {
|
||||
const pi = createMockPI();
|
||||
registerNativeSearchHooks(pi);
|
||||
|
||||
// NO model_select fired, but event.model confirms Anthropic provider
|
||||
const payload: Record<string, unknown> = {
|
||||
model: "claude-sonnet-4-6-20250514",
|
||||
tools: [{ name: "bash", type: "custom" }],
|
||||
};
|
||||
|
||||
const result = await pi.fire("before_provider_request", {
|
||||
type: "before_provider_request",
|
||||
payload,
|
||||
model: { provider: "anthropic", id: "claude-sonnet-4-6" },
|
||||
});
|
||||
|
||||
const tools = ((result as any)?.tools ?? payload.tools) as any[];
|
||||
assert.ok(
|
||||
tools.some((t: any) => t.type === "web_search_20250305"),
|
||||
"Should inject web_search when event.model confirms Anthropic"
|
||||
);
|
||||
});
|
||||
|
||||
test("before_provider_request does not double-inject", async () => {
|
||||
const pi = createMockPI();
|
||||
registerNativeSearchHooks(pi);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue