diff --git a/packages/coding-agent/src/tools/index.ts b/packages/coding-agent/src/tools/index.ts index 6eff5f0c3..61c4a8b3e 100644 --- a/packages/coding-agent/src/tools/index.ts +++ b/packages/coding-agent/src/tools/index.ts @@ -514,7 +514,14 @@ export async function createTools(session: ToolSession, toolNames?: string[]): P // Injected unconditionally into every agent, regardless of requested tool list. const autoQA = isAutoQaEnabled(session.settings); if (autoQA && !tools.some(t => t.name === "report_tool_issue")) { - const qaTool = await HIDDEN_TOOLS.report_tool_issue(session); + // Build the enum from tools we just constructed via BUILTIN_TOOLS / HIDDEN_TOOLS. + // Extension overrides (e.g. a user's custom `bash`) get added later by + // other code paths, so they're absent here — exactly what we want; MCP / + // extension tools never end up in the report enum. + const activeBuiltinNames = tools + .map(t => t.name) + .filter(name => (name in BUILTIN_TOOLS || name in HIDDEN_TOOLS) && name !== "report_tool_issue"); + const qaTool = createReportToolIssueTool(session, activeBuiltinNames); if (qaTool) { tools.push(wrapToolWithMetaNotice(qaTool)); } diff --git a/packages/coding-agent/src/tools/report-tool-issue.ts b/packages/coding-agent/src/tools/report-tool-issue.ts index 4a4018c41..35a4bbf1c 100644 --- a/packages/coding-agent/src/tools/report-tool-issue.ts +++ b/packages/coding-agent/src/tools/report-tool-issue.ts @@ -27,12 +27,18 @@ import * as z from "zod/v4"; import type { Settings } from ".."; import type { ToolSession } from "./index"; -const ReportToolIssueParams = z.object({ - tool: z.string().describe("tool name"), - report: z - .string() - .describe("unexpected behavior; generic, NEVER PII (paths, file contents, identifiers, prompt text)"), -}); +function buildReportToolIssueParams(activeBuiltinNames: readonly string[]) { + // Enum gives the model a tight schema; the runtime check in `execute` is the + // source of truth (handles models that ignore the enum and the empty-list + // fallback used by call sites that don't know the active set yet). + const toolSchema = activeBuiltinNames.length > 0 ? z.enum(activeBuiltinNames as [string, ...string[]]) : z.string(); + return z.object({ + tool: toolSchema.describe("tool name"), + report: z + .string() + .describe("unexpected behavior; generic, NEVER PII (paths, file contents, identifiers, prompt text)"), + }); +} export function isAutoQaEnabled(settings?: Settings): boolean { return $flag("PI_AUTO_QA") || !!settings?.get("dev.autoqa"); @@ -449,15 +455,19 @@ export async function flushGrievances( } } -export function createReportToolIssueTool(session: ToolSession): AgentTool { +export function createReportToolIssueTool(session: ToolSession, activeBuiltinNames: readonly string[] = []): AgentTool { const getModel = () => session.getActiveModelString?.() ?? "unknown"; + // Snapshotted at construction time. The model's enum is built from the same + // snapshot; mid-session drift (extensions registering later, etc.) is caught + // by the silent-drop guard below. + const allowedToolNames = new Set(activeBuiltinNames); return { name: "report_tool_issue", label: "Report Tool Issue", strict: false, description: "Report unexpected tool behavior for automated QA tracking.", - parameters: ReportToolIssueParams, + parameters: buildReportToolIssueParams(activeBuiltinNames), intent: "omit", async execute(_toolCallId, rawParams) { // Save is unconditional: the row lives in the user's own SQLite @@ -468,12 +478,24 @@ export function createReportToolIssueTool(session: ToolSession): AgentTool { // enforced inside `flushGrievances` via `resolvePushConfig`. try { const params = rawParams as { tool: string; report: string }; + // Some models emit `proxy_` for tools routed through a + // passthrough wrapper. Strip the prefix before allowlist check so + // `proxy_read` lands as a report against `read`, not a silent drop. + const canonicalTool = params.tool.startsWith("proxy_") ? params.tool.slice("proxy_".length) : params.tool; + // Silently drop reports targeting tools that aren't shipped built-ins + // (MCP servers, extensions that overrode a built-in name, typos). + // Not the model's fault — no error, no DB row, just acknowledge. + // Empty allowlist means the factory was called without a known active + // set, so behave as before and record everything. + if (allowedToolNames.size > 0 && !allowedToolNames.has(canonicalTool)) { + return { content: [{ type: "text", text: "Noted, thanks!" }] }; + } const db = openAutoQaDb(); if (db) { db.prepare("INSERT INTO grievances (model, version, tool, report) VALUES (?, ?, ?, ?)").run( getModel(), VERSION, - params.tool, + canonicalTool, params.report, ); // Fire-and-forget background pipeline: