From 0b0857a1e7c12c6854a88fb87b5b6ea5b220b057 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 27 Sep 2026 22:55:28 -0700 Subject: [PATCH 1/2] feat(telemetry): record abandoned permission prompts and modes An abandoned prompt is not a denial: Esc previously manufactured a deny outcome, hiding abandonment from telemetry. Settling with no outcome lets the gate deny the action while reporting what actually happened. --- src/permission/gate.ts | 16 ++- src/telemetry/index.ts | 2 +- src/tui/gate-wire.ts | 22 ++-- src/tui/runner/commands.ts | 9 +- tests/unit/telemetry-product-events.test.ts | 118 ++++++++++++++++++++ 5 files changed, 148 insertions(+), 19 deletions(-) diff --git a/src/permission/gate.ts b/src/permission/gate.ts index 028598739..c1805c958 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -71,6 +71,7 @@ import { NOOP_TELEMETRY, type Telemetry } from "../telemetry/index.js"; import { NOOP_APPROVAL_LOG, type ApprovalLog, + type ApprovalMode, type ApprovalOutcomeKind, } from "./approval-log.js"; @@ -82,12 +83,15 @@ function finishApprovalWait( waitSpanId: string, tool: string, outcome: ApprovalOutcome | undefined, + permissionMode: ApprovalMode, ): void { - const decision = outcome !== undefined && outcome.allow ? "allow" : "deny"; - end(waitSpanId, outcome !== undefined ? { decision } : undefined); + const decision = + outcome === undefined ? "abandoned" : outcome.allow ? "allow" : "deny"; + end(waitSpanId, { decision }); telemetry.capture("permission_prompt", { decision, permission_kind: classifyPermissionKind(tool), + permission_mode: permissionMode, }); } @@ -1089,7 +1093,13 @@ export function createPermissionGate( try { outcome = await prompt(request); } finally { - finishApprovalWait(telemetry, waitSpanId, request.tool, outcome); + finishApprovalWait( + telemetry, + waitSpanId, + request.tool, + outcome, + "interactive", + ); ask.settle(classifyOutcome(outcome)); } if ( diff --git a/src/telemetry/index.ts b/src/telemetry/index.ts index 8e6688a1d..170a6814f 100644 --- a/src/telemetry/index.ts +++ b/src/telemetry/index.ts @@ -189,7 +189,7 @@ const EVENT_PROPERTY_ALLOWLIST: Record = { "parent_trace_id", ], - permission_prompt: ["decision", "permission_kind"], + permission_prompt: ["decision", "permission_kind", "permission_mode"], compaction: ["mode", "duration_ms", "turns_before", "turns_after"], // provider/model are the canonical runtime ids (same trust class as // $ai_provider/$ai_model); error_kind is the summarizer's first-party diff --git a/src/tui/gate-wire.ts b/src/tui/gate-wire.ts index 6ccaa93fe..f07a81779 100644 --- a/src/tui/gate-wire.ts +++ b/src/tui/gate-wire.ts @@ -378,8 +378,13 @@ export function wireGates( // closeInsetOverlay, which fires onCancel after notifying close // listeners) — settle's return value is how a call site tells that // reentrant call apart from the original one. - const settle = (outcome: ApprovalOutcome): boolean => - permissionQueue.settle(id, outcome); + const settle = (outcome: ApprovalOutcome | undefined): boolean => + // An undefined outcome is an abandoned prompt (Esc): it flows through + // the queue to the gate, which records an "abandoned" telemetry + // decision and still settles the action as denied. The cast contains + // the widening to this seam — the queue and gate-event types are + // untouched. + permissionQueue.settle(id, outcome as ApprovalOutcome); const id = permissionQueue.enqueue(ev.request, (outcome) => { clearTimers(); if (openedGeneration === undefined) { @@ -437,16 +442,11 @@ export function wireGates( }; settle(approvalOutcomeFromSelection(choices, gateSelection)); }, - // Esc must settle the awaited promise (as a deny), not abandon it — - // an unresolved gate hangs the run until the process is killed. + // Esc settles the awaited promise as abandoned, not as a deny — an + // unresolved gate hangs the run until the process is killed, so the + // gate must still settle either way. onCancel: () => { - const denyId = choices.itemIds[0]; - settle( - approvalOutcomeFromSelection(choices, { - index: 0, - ...(denyId !== undefined ? { id: denyId } : {}), - }), - ); + settle(undefined); }, isGate: true, }); diff --git a/src/tui/runner/commands.ts b/src/tui/runner/commands.ts index 47acacf2e..64305e64e 100644 --- a/src/tui/runner/commands.ts +++ b/src/tui/runner/commands.ts @@ -348,15 +348,16 @@ export function createCommandLayer( }; const dispatchCommand = (name: string, args: string): void => { + // Captured before the unknown-command early return so plugin-authored + // names are bucketed (never sent raw) — the classifier maps anything it + // doesn't recognise to custom. Shared emitter so TUI and any headless + // path report the same event; known commands still emit exactly once. + captureSlashCommand(getTelemetry(), name); const command = getCommand(name); if (command === undefined) { state.systemNotice?.(`Unknown command: ${name}`); return; } - // Plugins register into the same command registry as the built-ins, so an - // unrecognised name is plugin-authored and is bucketed rather than sent. - // Shared emitter so TUI and any headless path report the same event. - captureSlashCommand(getTelemetry(), command.name); applyCommandResult(command.handler(args, commandContext)); }; state.dispatchCommand = dispatchCommand; diff --git a/tests/unit/telemetry-product-events.test.ts b/tests/unit/telemetry-product-events.test.ts index 6bd18ed5d..aa7e5e577 100644 --- a/tests/unit/telemetry-product-events.test.ts +++ b/tests/unit/telemetry-product-events.test.ts @@ -36,6 +36,9 @@ import { NOOP_TELEMETRY, type Telemetry, } from "../../src/telemetry/index.js"; +import { setTelemetry } from "../../src/telemetry/singleton.js"; +import { registerCommand } from "../../src/tui/commands/registry.js"; +import { createCommandLayer } from "../../src/tui/runner/commands.js"; import { buildSubagentEndProperties, captureSkillUsed, @@ -755,3 +758,118 @@ test("keys outside an event's allowlist are stripped from the payload", async () expect(event?.properties.command).toBeUndefined(); expect(await wire()).not.toContain("acmecorp"); }); + +// --------------------------------------------------------------------------- +// 6. permission abandoned + permission_mode (CL-9013) +// --------------------------------------------------------------------------- + +test('permission_prompt records "abandoned" when Esc settles the prompt with no outcome', async () => { + const { telemetry, wire, events } = harness(); + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + reactorGated: false, + // gate-wire.ts onCancel settles the approval promise with undefined when + // the operator presses Esc — no synthetic deny is manufactured there. + requestApproval: async () => undefined as never, + telemetry, + }); + + const verdict = await gate.evaluate({ + id: "call-1", + name: "mcp__acme-internal__deploy", + arguments: {}, + }); + + // Liveness: the gate still settles — the action is denied, never hung. + expect(verdict.allowed).toBe(false); + const [event] = await events(); + expect(event?.event).toBe("permission_prompt"); + expect(event?.properties.decision).toBe("abandoned"); + expect(event?.properties.permission_kind).toBe("mcp"); + expect(event?.properties.permission_mode).toBe("interactive"); + expect(await wire()).not.toContain("acme-internal"); +}); + +test("permission_prompt carries permission_mode on allow and deny", async () => { + for (const allow of [true, false] as const) { + const { telemetry, wire, events } = harness(); + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + reactorGated: false, + requestApproval: async () => ({ allow }), + telemetry, + }); + + await gate.evaluate({ + id: "call-1", + name: "mcp__acme-internal__deploy", + arguments: {}, + }); + + const [event] = await events(); + expect(event?.properties.decision).toBe(allow ? "allow" : "deny"); + expect(event?.properties.permission_mode).toBe("interactive"); + expect(await wire()).not.toContain("acme-internal"); + } +}); + +// --------------------------------------------------------------------------- +// 7. unknown slash commands (CL-9013) +// --------------------------------------------------------------------------- + +test("dispatching an unknown slash command emits exactly one custom event", async () => { + const { telemetry, wire, events } = harness(); + setTelemetry(telemetry); + try { + const notices: string[] = []; + const state = { + systemNotice: (text: string) => notices.push(text), + } as never; + createCommandLayer(state, {} as never); + const dispatch = ( + state as unknown as { + dispatchCommand: (name: string, args: string) => void; + } + ).dispatchCommand; + + dispatch("acmecorp-deploy-thing", ""); + + expect(notices).toHaveLength(1); + const captured = await events(); + expect(captured.filter((e) => e.event === "slash_command")).toHaveLength(1); + expect(captured[0]?.properties.command_name).toBe("custom"); + expect(await wire()).not.toContain("acmecorp"); + } finally { + setTelemetry(NOOP_TELEMETRY); + } +}); + +test("dispatching a known slash command emits exactly once", async () => { + const { telemetry, events } = harness(); + setTelemetry(telemetry); + try { + registerCommand({ + name: "cl9013-known-probe", + description: "CL-9013 exactly-once probe", + handler: () => ({ type: "noop" }), + }); + const state = {} as never; + createCommandLayer(state, {} as never); + const dispatch = ( + state as unknown as { + dispatchCommand: (name: string, args: string) => void; + } + ).dispatchCommand; + + dispatch("cl9013-known-probe", ""); + + const captured = await events(); + expect(captured.filter((e) => e.event === "slash_command")).toHaveLength(1); + } finally { + setTelemetry(NOOP_TELEMETRY); + } +}); From e24d46f378828d73de89c82a2a790c5f776008ba Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 28 Sep 2026 07:44:16 -0700 Subject: [PATCH 2/2] test(tui): settle Esc-closed gates as abandoned, not denied --- src/tui/gate-wire.test.ts | 9 ++++----- src/tui/slash-popup-gate.test.ts | 8 ++++---- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/tui/gate-wire.test.ts b/src/tui/gate-wire.test.ts index 5558b3d97..6f61f542b 100644 --- a/src/tui/gate-wire.test.ts +++ b/src/tui/gate-wire.test.ts @@ -550,7 +550,7 @@ describe("wireGates", () => { ).toEqual(["Reject", "Accept once", "Allow git A"]); closeInsetOverlay(shell); - expect(resolvedA).toEqual({ allow: false }); + expect(resolvedA).toBeUndefined(); expect(shell.overlayList).toBeNull(); emitter.emit("permission.gate", { @@ -621,7 +621,7 @@ describe("wireGates", () => { }, }); closeInsetOverlay(shell); - expect(resolvedA).toEqual({ allow: false }); + expect(resolvedA).toBeUndefined(); expect(shell.overlayList).toBeNull(); emitter.emit("permission.gate", { @@ -1674,7 +1674,7 @@ describe("operator.gate auto-cancel", () => { }); describe("Esc on a gate overlay settles the awaited promise", () => { - test("permission.gate: Esc denies instead of abandoning the promise", async () => { + test("permission.gate: Esc abandons instead of denying", async () => { await withTestRenderer(async (h) => { const shell = createAppShell(h.renderer, { terminal: { columns: 80, rows: 24 }, @@ -1699,8 +1699,7 @@ describe("Esc on a gate overlay settles the awaited promise", () => { expect(shell.overlayList).toBeNull(); expect(resolveCount).toBe(1); - expect(resolved).toEqual({ allow: false }); - expect(resolved).not.toEqual(unavailable); + expect(resolved).toBeUndefined(); } finally { shell.dispose(); } diff --git a/src/tui/slash-popup-gate.test.ts b/src/tui/slash-popup-gate.test.ts index 01fa99e98..fd38226d2 100644 --- a/src/tui/slash-popup-gate.test.ts +++ b/src/tui/slash-popup-gate.test.ts @@ -438,7 +438,7 @@ describe("slash/palette accept holds the host until dispatch settles", () => { closeInsetOverlay(shell); await Promise.resolve(); expect(shell.overlayKind).toBe("help"); - expect(resolved).toEqual({ allow: false }); + expect(resolved).toBeUndefined(); } finally { dispose(); } @@ -657,7 +657,7 @@ describe("slash/palette accept holds the host until dispatch settles", () => { closeInsetOverlay(shell); await Promise.resolve(); expect(shell.overlayKind).toBe("settings"); - expect(resolved).toEqual({ allow: false }); + expect(resolved).toBeUndefined(); } finally { dispose(); } @@ -720,7 +720,7 @@ describe("slash/palette accept holds the host until dispatch settles", () => { closeInsetOverlay(shell); await Promise.resolve(); expect(shell.overlayKind).toBe("mcp"); - expect(resolved).toEqual({ allow: false }); + expect(resolved).toBeUndefined(); } finally { dispose(); } @@ -1162,7 +1162,7 @@ describe("overlay host occupancy and opt-in deferral", () => { closeInsetOverlay(shell); await Promise.resolve(); expect(shell.overlayKind).toBe("add_provider"); - expect(resolved).toEqual({ allow: false }); + expect(resolved).toBeUndefined(); } finally { dispose(); }