diff --git a/plugins/glean/.claude-plugin/plugin.json b/plugins/glean/.claude-plugin/plugin.json index 1fee628..1ff6a83 100644 --- a/plugins/glean/.claude-plugin/plugin.json +++ b/plugins/glean/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "glean-vnext", - "version": "0.2.40", + "version": "0.2.41", "description": "Glean plugin for discovering skills and running tools.", "author": { "name": "Glean" diff --git a/plugins/glean/.codex-plugin/plugin.json b/plugins/glean/.codex-plugin/plugin.json index 16fb2f1..582cf9b 100644 --- a/plugins/glean/.codex-plugin/plugin.json +++ b/plugins/glean/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "glean-vnext", - "version": "0.2.40", + "version": "0.2.41", "description": "Glean Codex plugin for discovering skills and running tools.", "author": { "name": "Glean" diff --git a/plugins/glean/.cursor-plugin/plugin.json b/plugins/glean/.cursor-plugin/plugin.json index 87422ec..c67bd88 100644 --- a/plugins/glean/.cursor-plugin/plugin.json +++ b/plugins/glean/.cursor-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "glean-vnext", "displayName": "Glean vNext", - "version": "0.2.40", + "version": "0.2.41", "description": "Search and act across your company's apps — Jira, Slack, Salesforce, Google Workspace, and more — without leaving Cursor.", "author": { "name": "Glean" diff --git a/plugins/glean/dist/index.js b/plugins/glean/dist/index.js index c46e8fc..4e06f74 100644 --- a/plugins/glean/dist/index.js +++ b/plugins/glean/dist/index.js @@ -25671,6 +25671,7 @@ async function handleFindSkills(remoteClient, skillsBaseDir, args) { } // src/tools/run-tool.ts +import { appendFileSync } from "node:fs"; import fs4 from "node:fs/promises"; import os2 from "node:os"; import path4 from "node:path"; @@ -25893,7 +25894,7 @@ function isCursorClient(mcpServer) { } async function buildApprovalMessage(mcpServer, toolName, args) { if (isCursorClient(mcpServer)) { - return `Review the tool and arguments shown above, click on Submit to allow and Cancel to deny.`; + return `Review the tool and arguments shown above, then choose Approve to run it or Deny to block it, and submit.`; } const { lines, needsFile } = buildCompactArgs(args); const message = [ @@ -25911,6 +25912,38 @@ async function buildApprovalMessage(mcpServer, toolName, args) { } return message.join("\n"); } +function approvalRequestedSchema(isCursor) { + if (!isCursor) { + return { type: "object", properties: {} }; + } + return { + type: "object", + properties: { + decision: { + type: "string", + title: "Approve this action?", + description: "Approve to run the tool, or Deny to block it.", + enum: ["Approve", "Deny"] + } + }, + required: ["decision"] + }; +} +function isApproved(isCursor, result) { + if (result.action !== "accept") return false; + if (!isCursor) return true; + return result.content?.decision === "Approve"; +} +function hitlLog(label, detail) { + const base = process.env.PLUGIN_DATA_DIR || path4.join(os2.homedir(), ".glean"); + const line = `${(/* @__PURE__ */ new Date()).toISOString()} ${label} ${JSON.stringify({ pid: process.pid, ...detail })} +`; + try { + appendFileSync(path4.join(base, "glean-server.log"), line, { mode: 384 }); + } catch { + } + console.error(line.trimEnd()); +} var elicitationIdPrimed = /* @__PURE__ */ new WeakSet(); function primeElicitationCancellation(mcpServer) { if (elicitationIdPrimed.has(mcpServer)) return; @@ -25962,6 +25995,13 @@ async function handleRunTool(remoteClient, mcpServer, skillsBaseDir, args) { throw err; } const hitlEnabled = process.env.ENABLE_HITL === "true"; + hitlLog("hitl.gate", { + tool: toolName, + hitlEnabled, + requiresApproval: !!toolMeta?.requires_approval, + elicitationCap: mcpServer.getClientCapabilities()?.elicitation ?? null, + client: mcpServer.getClientVersion()?.name ?? null + }); if (hitlEnabled && toolMeta?.requires_approval && mcpServer.getClientCapabilities()?.elicitation) { const bypass = await currentPermissionMode() === "bypassPermissions"; if (!bypass) { @@ -25971,27 +26011,39 @@ async function handleRunTool(remoteClient, mcpServer, skillsBaseDir, args) { resolvedArgs ); const timeout = hitlTimeoutMs(); + const cursor = isCursorClient(mcpServer); primeElicitationCancellation(mcpServer); + hitlLog("hitl.elicit-sent", { + tool: toolName, + schema: cursor ? "cursor-form" : "empty", + timeout + }); try { const result = await mcpServer.elicitInput( { message, - requestedSchema: { type: "object", properties: {} } + requestedSchema: approvalRequestedSchema(cursor) }, { timeout } ); - if (result.action !== "accept") { + hitlLog("hitl.elicit-result", { + tool: toolName, + action: result.action, + decision: result.content?.decision ?? null + }); + if (!isApproved(cursor, result)) { return { content: [ { type: "text", - text: `Action ${toolName} was ${result.action === "decline" ? "declined" : "cancelled"} by the user.` + text: `Action ${toolName} was ${result.action === "cancel" ? "cancelled" : "declined"} by the user.` } ] }; } } catch (err) { const detail = err instanceof Error ? err.message : String(err); + hitlLog("hitl.elicit-error", { tool: toolName, msg: detail }); return { content: [ { diff --git a/src/tools/run-tool.ts b/src/tools/run-tool.ts index 7b62228..e0068cb 100644 --- a/src/tools/run-tool.ts +++ b/src/tools/run-tool.ts @@ -2,6 +2,7 @@ import type { Client } from "@modelcontextprotocol/sdk/client/index.js"; import type { Server } from "@modelcontextprotocol/sdk/server/index.js"; import type { CallToolResult, Tool } from "@modelcontextprotocol/sdk/types.js"; import { EmptyResultSchema } from "@modelcontextprotocol/sdk/types.js"; +import { appendFileSync } from "node:fs"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -205,7 +206,7 @@ async function buildApprovalMessage( args: unknown, ): Promise { if (isCursorClient(mcpServer)) { - return `Review the tool and arguments shown above, click on Submit to allow and Cancel to deny.`; + return `Review the tool and arguments shown above, then choose Approve to run it or Deny to block it, and submit.`; } const { lines, needsFile } = buildCompactArgs(args); @@ -231,6 +232,69 @@ async function buildApprovalMessage( return message.join("\n"); } +// The elicitation schema for the approval prompt. +// +// Claude Code advertises bare `elicitation: {}` and renders the `message` text +// with Accept/Decline buttons regardless of the schema, so an empty object is +// enough there. Cursor advertises a FORM-based variant (`elicitation: +// {form:{}}`) and builds the banner FROM the schema's `properties` — an empty +// `properties` renders no fields, so Cursor shows no visible approval banner +// and the gate is silently skipped. Cursor therefore gets one required +// Approve/Deny field: it guarantees the form renders and forces an explicit +// choice. Keep the non-Cursor path empty so Claude Code's working behavior is +// untouched. +export function approvalRequestedSchema( + isCursor: boolean, +): Record { + if (!isCursor) { + return { type: "object", properties: {} }; + } + return { + type: "object", + properties: { + decision: { + type: "string", + title: "Approve this action?", + description: "Approve to run the tool, or Deny to block it.", + enum: ["Approve", "Deny"], + }, + }, + required: ["decision"], + }; +} + +// Whether an elicitation result approves the action. Non-Cursor clients gate +// purely on the action (the Accept button). Cursor submits a form, so a +// submitted "Deny" comes back as action "accept" with decision "Deny" — that is +// a rejection, not an approval, so Cursor approval also requires decision +// "Approve". +export function isApproved( + isCursor: boolean, + result: { action: string; content?: Record }, +): boolean { + if (result.action !== "accept") return false; + if (!isCursor) return true; + return result.content?.decision === "Approve"; +} + +// Structural HITL telemetry appended to the shared server log. Mirrors +// index.ts's logLine (private to that module) and additionally stamps the +// pid: multiple plugin instances can share one log file (a marketplace +// install next to a local checkout), and the pid ties each line to a +// specific process. Logs tool names and gate decisions only — never +// argument values, which can carry PII/secrets. +function hitlLog(label: string, detail: Record): void { + const base = + process.env.PLUGIN_DATA_DIR || path.join(os.homedir(), ".glean"); + const line = `${new Date().toISOString()} ${label} ${JSON.stringify({ pid: process.pid, ...detail })}\n`; + try { + appendFileSync(path.join(base, "glean-server.log"), line, { mode: 0o600 }); + } catch { + /* best-effort */ + } + console.error(line.trimEnd()); +} + // A WeakSet so a short-lived server in tests doesn't leak, // and so the burn happens exactly once per server instance. const elicitationIdPrimed = new WeakSet(); @@ -329,6 +393,13 @@ export async function handleRunTool( } const hitlEnabled = process.env.ENABLE_HITL === "true"; + hitlLog("hitl.gate", { + tool: toolName, + hitlEnabled, + requiresApproval: !!toolMeta?.requires_approval, + elicitationCap: mcpServer.getClientCapabilities()?.elicitation ?? null, + client: mcpServer.getClientVersion()?.name ?? null, + }); if ( hitlEnabled && toolMeta?.requires_approval && @@ -349,25 +420,40 @@ export async function handleRunTool( resolvedArgs, ); const timeout = hitlTimeoutMs(); + const cursor = isCursorClient(mcpServer); // Make a dummy empty request to burn JSON-RPC request id 0 primeElicitationCancellation(mcpServer); + hitlLog("hitl.elicit-sent", { + tool: toolName, + schema: cursor ? "cursor-form" : "empty", + timeout, + }); + try { const result = await mcpServer.elicitInput( { message, - requestedSchema: { type: "object", properties: {} } as any, + requestedSchema: approvalRequestedSchema(cursor) as any, }, { timeout }, ); - if (result.action !== "accept") { + hitlLog("hitl.elicit-result", { + tool: toolName, + action: result.action, + decision: + (result.content as Record | undefined) + ?.decision ?? null, + }); + + if (!isApproved(cursor, result)) { return { content: [ { type: "text", - text: `Action ${toolName} was ${result.action === "decline" ? "declined" : "cancelled"} by the user.`, + text: `Action ${toolName} was ${result.action === "cancel" ? "cancelled" : "declined"} by the user.`, }, ], }; @@ -377,6 +463,7 @@ export async function handleRunTool( // prompt times out or errors defeats its own purpose — and the SDK // rejects elicitInput precisely on request timeout. const detail = err instanceof Error ? err.message : String(err); + hitlLog("hitl.elicit-error", { tool: toolName, msg: detail }); return { content: [ { diff --git a/tests/run-tool.test.ts b/tests/run-tool.test.ts index 91ae09e..2064a98 100644 --- a/tests/run-tool.test.ts +++ b/tests/run-tool.test.ts @@ -8,6 +8,8 @@ import { FileArgsError, handleRunTool, runToolAnnotations, + approvalRequestedSchema, + isApproved, } from "../src/tools/run-tool.js"; import { buildCompactArgs, @@ -456,7 +458,9 @@ describe("handleRunTool (HITL)", () => { it("gives Cursor a one-line prompt without an arguments block", async () => { vi.stubEnv("ENABLE_HITL", "true"); const remote = makeRemote(); - const elicit = vi.fn().mockResolvedValue({ action: "accept" }); + const elicit = vi + .fn() + .mockResolvedValue({ action: "accept", content: { decision: "Approve" } }); const server = makeServer({ elicitation: true, clientName: "cursor-vscode", @@ -467,11 +471,70 @@ describe("handleRunTool (HITL)", () => { await handleRunTool(remote, server, tmpDir, baseArgs); const message = elicit.mock.calls[0][0].message as string; - expect(message).toContain("Submit to allow and Cancel to deny"); + expect(message).toContain("Approve to run it or Deny to block it"); expect(message).not.toContain("Arguments:"); expect(remote.callTool).toHaveBeenCalledTimes(1); }); + it("sends Cursor a non-empty form schema so its banner renders", async () => { + vi.stubEnv("ENABLE_HITL", "true"); + const remote = makeRemote(); + const elicit = vi + .fn() + .mockResolvedValue({ action: "accept", content: { decision: "Approve" } }); + const server = makeServer({ + elicitation: true, + clientName: "cursor-vscode", + elicit, + }); + await writeToolJson(tmpDir, "jirasearch", { requires_approval: true }); + + await handleRunTool(remote, server, tmpDir, baseArgs); + + const schema = elicit.mock.calls[0][0].requestedSchema; + expect(schema.properties.decision).toBeDefined(); + expect(schema.properties.decision.enum).toEqual(["Approve", "Deny"]); + expect(schema.required).toContain("decision"); + }); + + it("keeps an empty form schema for non-Cursor clients", async () => { + vi.stubEnv("ENABLE_HITL", "true"); + const remote = makeRemote(); + const elicit = vi.fn().mockResolvedValue({ action: "accept" }); + const server = makeServer({ + elicitation: true, + clientName: "claude-code", + elicit, + }); + await writeToolJson(tmpDir, "jirasearch", { requires_approval: true }); + + await handleRunTool(remote, server, tmpDir, baseArgs); + + const schema = elicit.mock.calls[0][0].requestedSchema; + expect(schema.properties).toEqual({}); + expect(schema.required).toBeUndefined(); + expect(remote.callTool).toHaveBeenCalledTimes(1); + }); + + it("does not execute when Cursor submits the form with Deny", async () => { + vi.stubEnv("ENABLE_HITL", "true"); + const remote = makeRemote(); + const elicit = vi + .fn() + .mockResolvedValue({ action: "accept", content: { decision: "Deny" } }); + const server = makeServer({ + elicitation: true, + clientName: "cursor-vscode", + elicit, + }); + await writeToolJson(tmpDir, "jirasearch", { requires_approval: true }); + + const result = await handleRunTool(remote, server, tmpDir, baseArgs); + + expect(remote.callTool).not.toHaveBeenCalled(); + expect((result.content[0] as { text: string }).text).toContain("declined"); + }); + it("spills large arguments to a file and keeps the prompt short", async () => { vi.stubEnv("ENABLE_HITL", "true"); const remote = makeRemote(); @@ -722,3 +785,39 @@ describe("runToolAnnotations", () => { expect(runToolAnnotations(true, false)).toBeUndefined(); }); }); + +describe("approvalRequestedSchema", () => { + it("is empty for non-Cursor clients (message-only banner)", () => { + expect(approvalRequestedSchema(false)).toEqual({ + type: "object", + properties: {}, + }); + }); + + it("carries a required Approve/Deny field for Cursor's form renderer", () => { + const schema = approvalRequestedSchema(true) as any; + expect(schema.properties.decision.enum).toEqual(["Approve", "Deny"]); + expect(schema.required).toEqual(["decision"]); + }); +}); + +describe("isApproved", () => { + it("approves a non-Cursor accept regardless of content", () => { + expect(isApproved(false, { action: "accept" })).toBe(true); + }); + + it("rejects a non-Cursor decline or cancel", () => { + expect(isApproved(false, { action: "decline" })).toBe(false); + expect(isApproved(false, { action: "cancel" })).toBe(false); + }); + + it("approves a Cursor accept only when the form decision is Approve", () => { + expect( + isApproved(true, { action: "accept", content: { decision: "Approve" } }), + ).toBe(true); + expect( + isApproved(true, { action: "accept", content: { decision: "Deny" } }), + ).toBe(false); + expect(isApproved(true, { action: "accept" })).toBe(false); + }); +});