From c54c4521940782a20cfb68d71b60347517cb780b Mon Sep 17 00:00:00 2001 From: Pragati Agrawal Date: Sun, 19 Jul 2026 14:52:47 +0530 Subject: [PATCH 1/2] Fix Cursor HITL approval banner not rendering (empty elicitation form) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cursor advertises a form-based elicitation capability (`elicitation: {form:{}}`) and builds the approval banner from the elicitation schema's `properties`. The HITL gate sent `requestedSchema: {type:"object", properties:{}}` to every client, so Cursor had no fields to render and showed no visible approval banner — the action ran ungated. Claude Code renders the message text with Accept/Decline regardless of schema, so it was unaffected. Branch the schema by client: Cursor gets a single required Approve/Deny field (guarantees the form renders and forces an explicit choice); the non-Cursor path keeps the empty schema so Claude Code behavior is unchanged. A submitted "Deny" (action "accept" + decision "Deny") is treated as a rejection. Co-Authored-By: Claude Opus 4.8 --- plugins/glean/.claude-plugin/plugin.json | 2 +- plugins/glean/.codex-plugin/plugin.json | 2 +- plugins/glean/.cursor-plugin/plugin.json | 2 +- plugins/glean/dist/index.js | 31 ++++++- src/tools/run-tool.ts | 54 +++++++++++- tests/run-tool.test.ts | 103 ++++++++++++++++++++++- 6 files changed, 181 insertions(+), 13 deletions(-) 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..bdbdb8a 100644 --- a/plugins/glean/dist/index.js +++ b/plugins/glean/dist/index.js @@ -25893,7 +25893,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 +25911,28 @@ 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"; +} var elicitationIdPrimed = /* @__PURE__ */ new WeakSet(); function primeElicitationCancellation(mcpServer) { if (elicitationIdPrimed.has(mcpServer)) return; @@ -25971,21 +25993,22 @@ async function handleRunTool(remoteClient, mcpServer, skillsBaseDir, args) { resolvedArgs ); const timeout = hitlTimeoutMs(); + const cursor = isCursorClient(mcpServer); primeElicitationCancellation(mcpServer); try { const result = await mcpServer.elicitInput( { message, - requestedSchema: { type: "object", properties: {} } + requestedSchema: approvalRequestedSchema(cursor) }, { timeout } ); - if (result.action !== "accept") { + 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.` } ] }; diff --git a/src/tools/run-tool.ts b/src/tools/run-tool.ts index 7b62228..3d6c05c 100644 --- a/src/tools/run-tool.ts +++ b/src/tools/run-tool.ts @@ -205,7 +205,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 +231,51 @@ 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"; +} + // 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(); @@ -349,6 +394,7 @@ 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); @@ -357,17 +403,17 @@ export async function handleRunTool( const result = await mcpServer.elicitInput( { message, - requestedSchema: { type: "object", properties: {} } as any, + requestedSchema: approvalRequestedSchema(cursor) as any, }, { timeout }, ); - if (result.action !== "accept") { + 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.`, }, ], }; 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); + }); +}); From 13aa977e2d919b6f74caf9bf0e2b461993b29f7c Mon Sep 17 00:00:00 2001 From: Pragati Agrawal Date: Mon, 20 Jul 2026 12:26:28 +0530 Subject: [PATCH 2/2] Add pid-stamped HITL gate telemetry to run_tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server log had no line between "run_tool was called" and an elicitation error, so "gate never reached" (routing, missing tool JSON, requires_approval false) was indistinguishable from "elicitation sent but the client rendered nothing" — the exact ambiguity that made the Cursor no-banner bug hard to localize. Log the gate decision, the elicitation send (with which schema variant), the result, and errors. Lines are pid-stamped because multiple plugin instances (marketplace install next to a local checkout) can share one log file. Tool names and structural facts only — never argument values. Co-Authored-By: Claude Fable 5 --- plugins/glean/dist/index.js | 29 ++++++++++++++++++++++++++ src/tools/run-tool.ts | 41 +++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/plugins/glean/dist/index.js b/plugins/glean/dist/index.js index bdbdb8a..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"; @@ -25933,6 +25934,16 @@ function isApproved(isCursor, result) { 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; @@ -25984,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) { @@ -25995,6 +26013,11 @@ async function handleRunTool(remoteClient, mcpServer, skillsBaseDir, args) { 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( { @@ -26003,6 +26026,11 @@ async function handleRunTool(remoteClient, mcpServer, skillsBaseDir, args) { }, { timeout } ); + hitlLog("hitl.elicit-result", { + tool: toolName, + action: result.action, + decision: result.content?.decision ?? null + }); if (!isApproved(cursor, result)) { return { content: [ @@ -26015,6 +26043,7 @@ async function handleRunTool(remoteClient, mcpServer, skillsBaseDir, args) { } } 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 3d6c05c..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"; @@ -276,6 +277,24 @@ export function isApproved( 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(); @@ -374,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 && @@ -399,6 +425,12 @@ export async function handleRunTool( // 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( { @@ -408,6 +440,14 @@ export async function handleRunTool( { timeout }, ); + hitlLog("hitl.elicit-result", { + tool: toolName, + action: result.action, + decision: + (result.content as Record | undefined) + ?.decision ?? null, + }); + if (!isApproved(cursor, result)) { return { content: [ @@ -423,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: [ {