diff --git a/shared/glean/mcp/src/index.ts b/shared/glean/mcp/src/index.ts index 0b8ead2..c0acea4 100644 --- a/shared/glean/mcp/src/index.ts +++ b/shared/glean/mcp/src/index.ts @@ -20,7 +20,7 @@ import { closeCallbackServer, } from "./auth-callback-server.js"; import { handleFindSkills } from "./tools/find-skills.js"; -import { handleRunTool, runToolAnnotations } from "./tools/run-tool.js"; +import { handleRunTool, runToolAnnotations, shouldAskForToolApproval } from "./tools/run-tool.js"; import { evictStaleSkills } from "./skill-writer.js"; import { loadServerUrl, @@ -174,8 +174,10 @@ function getOAuthProvider(): GleanOAuthClientProvider { return oauthProvider; } -function getRemoteClientOpts(): RemoteClientOptions { - const supportsElicitation = !!server.getClientCapabilities()?.elicitation; +async function getRemoteClientOpts(): Promise { + const supportsElicitation = + !!server.getClientCapabilities()?.elicitation && + (await shouldAskForToolApproval()); return { authProvider: getOAuthProvider(), ...(supportsElicitation @@ -373,7 +375,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { try { remoteClient = await createRemoteClient( serverUrl, - getRemoteClientOpts(), + await getRemoteClientOpts(), `tools-list-${process.pid}`, ); } catch (err) { @@ -455,7 +457,7 @@ async function connectWithSignIn( try { const client = await createRemoteClient( serverUrl, - getRemoteClientOpts(), + await getRemoteClientOpts(), `setup-${process.pid}`, ); return { ok: true, client }; @@ -492,7 +494,7 @@ async function connectWithSignIn( try { const client = await createRemoteClient( serverUrl, - getRemoteClientOpts(), + await getRemoteClientOpts(), `setup-${process.pid}`, ); // Unexpectedly connected without needing auth — done. @@ -534,7 +536,7 @@ async function connectWithSignIn( try { const client = await createRemoteClient( serverUrl, - getRemoteClientOpts(), + await getRemoteClientOpts(), `setup-${process.pid}`, ); return { ok: true, client }; @@ -650,7 +652,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { } const dispatchCtx: DispatchContext = { serverUrl, - remoteClientOpts: getRemoteClientOpts(), + remoteClientOpts: await getRemoteClientOpts(), authRedirectText: AUTH_REDIRECT_TO_SETUP_TEXT, logLine, }; @@ -684,7 +686,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { try { remoteClient = await createRemoteClient( serverUrl, - getRemoteClientOpts(), + await getRemoteClientOpts(), sessionId, ); } catch (err) { @@ -748,7 +750,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { try { remoteClient = await createRemoteClient( serverUrl, - getRemoteClientOpts(), + await getRemoteClientOpts(), sessionId, ); } catch (err) { diff --git a/shared/glean/mcp/src/tools/run-tool.ts b/shared/glean/mcp/src/tools/run-tool.ts index f218d28..c1caeda 100644 --- a/shared/glean/mcp/src/tools/run-tool.ts +++ b/shared/glean/mcp/src/tools/run-tool.ts @@ -322,6 +322,11 @@ async function currentPermissionMode(): Promise { } } +export async function shouldAskForToolApproval(): Promise { + return process.env.ENABLE_HITL === "true" && + (await currentPermissionMode()) !== "bypassPermissions"; +} + function humanizeMs(ms: number): string { const seconds = Math.round(ms / 1000); if (seconds < 120) return `${seconds}s`; @@ -482,9 +487,11 @@ export async function handleRunTool( } const remoteArgs = buildRemoteArgs(serverId, toolName, resolvedArgs); - // Read-only tools are exempt from Glean's configurable approval requirements, - // so they do not need an approval lookup. - if (isKnownReadOnlyTool(toolMetadata, serverId, toolName)) { + // Opt-outs skip the entire approval flow, without reading or saving preferences. + if ( + !(await shouldAskForToolApproval()) || + isKnownReadOnlyTool(toolMetadata, serverId, toolName) + ) { return callRemoteTool(remoteClient, "run_tool", remoteArgs); } diff --git a/shared/glean/mcp/tests/run-tool.test.ts b/shared/glean/mcp/tests/run-tool.test.ts index 17a6339..29fa684 100644 --- a/shared/glean/mcp/tests/run-tool.test.ts +++ b/shared/glean/mcp/tests/run-tool.test.ts @@ -7,6 +7,7 @@ import { buildRemoteArgs, FileArgsError, getToolApproval, + shouldAskForToolApproval, handleRunTool, runToolAnnotations, elicitationFailureText, @@ -380,6 +381,28 @@ describe("handleRunTool (HITL)", () => { vi.unstubAllEnvs(); }); + it.each([ + { hitl: "false", mode: "default", expected: false }, + { hitl: "false", mode: "bypassPermissions", expected: false }, + { hitl: undefined, mode: "default", expected: false }, + { hitl: "true", mode: "bypassPermissions", expected: false }, + { hitl: "true", mode: "default", expected: true }, + { hitl: "true", mode: undefined, expected: true }, + ])("approval guard: HITL=$hitl, mode=$mode", async ({ hitl, mode, expected }) => { + vi.stubEnv("ENABLE_HITL", hitl); + vi.stubEnv("CLAUDE_PLUGIN_DATA", tmpDir); + vi.stubEnv("GLEAN_SESSION_ID", "guard-session"); + if (mode) await writeModeMarker(tmpDir, "guard-session", mode); + await expect(shouldAskForToolApproval()).resolves.toBe(expected); + if (!expected) { + const remote = makeRemote(); + const server = makeServer({ elicitation: true }); + await handleRunTool(remote, server, tmpDir, baseArgs, ALL_ON); + expect(server.elicitInput).not.toHaveBeenCalled(); + expect(remote.callTool.mock.calls.map(([request]) => request.name)).toEqual(["run_tool"]); + } + }); + it("does not elicit when the client lacks elicitation capability", async () => { vi.stubEnv("ENABLE_HITL", "true"); const remote = makeRemote();