From 7dd68126c4b7aff9d00c2938b1edc5b168b5a1f8 Mon Sep 17 00:00:00 2001 From: Georgiy Tarasov Date: Mon, 27 Apr 2026 16:26:07 +0200 Subject: [PATCH 1/4] permission requests --- .../permissions/permission-handlers.test.ts | 152 ++++++++++++++++++ .../claude/permissions/permission-handlers.ts | 99 ++++++++++++ .../permissions/posthog-exec-gate.test.ts | 84 ++++++++++ .../claude/permissions/posthog-exec-gate.ts | 30 ++++ .../adapters/claude/session/settings.test.ts | 50 ++++++ .../src/adapters/claude/session/settings.ts | 48 ++++++ 6 files changed, 463 insertions(+) create mode 100644 packages/agent/src/adapters/claude/permissions/posthog-exec-gate.test.ts create mode 100644 packages/agent/src/adapters/claude/permissions/posthog-exec-gate.ts diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts index 6b7139367..5d138c2f1 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts @@ -143,6 +143,158 @@ describe("canUseTool MCP approval enforcement", () => { expect(result.behavior).toBe("allow"); }); + it("bypasses the PostHog exec gate in auto mode", async () => { + setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); + const hasApproval = vi.fn().mockReturnValue(false); + const addApproval = vi.fn().mockResolvedValue(undefined); + + const context = createContext("mcp__posthog__exec", { + toolInput: { command: "call experiment-update {}" }, + session: { + permissionMode: "auto", + settingsManager: { + getRepoRoot: vi.fn().mockReturnValue("/repo"), + hasPostHogExecApproval: hasApproval, + addPostHogExecApproval: addApproval, + }, + }, + }); + const result = await canUseTool(context); + + expect(result.behavior).toBe("allow"); + expect(context.client.requestPermission).not.toHaveBeenCalled(); + expect(hasApproval).not.toHaveBeenCalled(); + expect(addApproval).not.toHaveBeenCalled(); + }); + + it("bypasses the PostHog exec gate in bypassPermissions mode", async () => { + setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); + + const context = createContext("mcp__posthog__exec", { + toolInput: { command: "call feature-flag-delete {}" }, + session: { + permissionMode: "bypassPermissions", + settingsManager: { + getRepoRoot: vi.fn().mockReturnValue("/repo"), + hasPostHogExecApproval: vi.fn().mockReturnValue(false), + addPostHogExecApproval: vi.fn(), + }, + }, + }); + const result = await canUseTool(context); + + expect(result.behavior).toBe("allow"); + expect(context.client.requestPermission).not.toHaveBeenCalled(); + }); + + it("short-circuits when a PostHog exec sub-tool was previously approved", async () => { + setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); + + const context = createContext("mcp__posthog__exec", { + toolInput: { command: "call experiment-update {}" }, + session: { + permissionMode: "default", + settingsManager: { + getRepoRoot: vi.fn().mockReturnValue("/repo"), + hasPostHogExecApproval: vi + .fn() + .mockImplementation((s: string) => s === "experiment-update"), + addPostHogExecApproval: vi.fn(), + }, + }, + }); + const result = await canUseTool(context); + + expect(result.behavior).toBe("allow"); + expect(context.client.requestPermission).not.toHaveBeenCalled(); + }); + + it("prompts for an unapproved destructive PostHog sub-tool and persists on allow_always", async () => { + setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); + const addApproval = vi.fn().mockResolvedValue(undefined); + + const context = createContext("mcp__posthog__exec", { + toolInput: { command: "call notebooks-destroy {}" }, + session: { + permissionMode: "default", + settingsManager: { + getRepoRoot: vi.fn().mockReturnValue("/repo"), + hasPostHogExecApproval: vi.fn().mockReturnValue(false), + addPostHogExecApproval: addApproval, + }, + }, + client: { + sessionUpdate: vi.fn().mockResolvedValue(undefined), + requestPermission: vi.fn().mockResolvedValue({ + outcome: { outcome: "selected", optionId: "allow_always" }, + }), + }, + }); + const result = await canUseTool(context); + + expect(result.behavior).toBe("allow"); + expect(context.client.requestPermission).toHaveBeenCalledWith( + expect.objectContaining({ + toolCall: expect.objectContaining({ + title: "The agent wants to run `notebooks-destroy` on PostHog", + }), + }), + ); + expect(addApproval).toHaveBeenCalledWith("notebooks-destroy"); + }); + + it("prompts but does not persist on allow_once", async () => { + setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); + const addApproval = vi.fn(); + + const context = createContext("mcp__posthog__exec", { + toolInput: { command: "call experiment-delete {}" }, + session: { + permissionMode: "default", + settingsManager: { + getRepoRoot: vi.fn().mockReturnValue("/repo"), + hasPostHogExecApproval: vi.fn().mockReturnValue(false), + addPostHogExecApproval: addApproval, + }, + }, + client: { + sessionUpdate: vi.fn().mockResolvedValue(undefined), + requestPermission: vi.fn().mockResolvedValue({ + outcome: { outcome: "selected", optionId: "allow" }, + }), + }, + }); + const result = await canUseTool(context); + + expect(result.behavior).toBe("allow"); + expect(addApproval).not.toHaveBeenCalled(); + }); + + it("does not gate non-destructive PostHog sub-tools", async () => { + setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); + const addApproval = vi.fn(); + + const context = createContext("mcp__posthog__exec", { + toolInput: { command: "call experiment-get-all {}" }, + session: { + permissionMode: "default", + settingsManager: { + getRepoRoot: vi.fn().mockReturnValue("/repo"), + hasPostHogExecApproval: vi.fn().mockReturnValue(false), + addPostHogExecApproval: addApproval, + }, + }, + }); + const result = await canUseTool(context); + + // Non-destructive sub-tool falls through the gate. With approved MCP state + // and non-read-only tool metadata, it hits the default permission flow, + // which auto-allows via our mocked requestPermission. The gate must not + // have prompted with a PostHog-specific title, and must not have persisted. + expect(result.behavior).toBe("allow"); + expect(addApproval).not.toHaveBeenCalled(); + }); + it("emits tool denial notification for do_not_use", async () => { setMcpToolApprovalStates({ mcp__server__denied_tool: "do_not_use", diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts index f9e6293c7..a51f529c1 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts @@ -31,6 +31,11 @@ import { buildExitPlanModePermissionOptions, buildPermissionOptions, } from "./permission-options"; +import { + extractPostHogSubTool, + isPostHogDestructiveSubTool, + isPostHogExecTool, +} from "./posthog-exec-gate"; export type ToolPermissionResult = | { @@ -487,6 +492,78 @@ async function handleMcpApprovalFlow( return { behavior: "deny", message, interrupt: !feedback }; } +async function handlePostHogExecApprovalFlow( + context: ToolHandlerContext, + subTool: string, +): Promise { + const { toolName, toolInput, toolUseID, client, sessionId, session } = + context; + + const response = await client.requestPermission({ + options: [ + { kind: "allow_once", name: "Yes", optionId: "allow" }, + { + kind: "allow_always", + name: "Yes, always allow", + optionId: "allow_always", + }, + { + kind: "reject_once", + name: "Type here to tell the agent what to do differently", + optionId: "reject", + _meta: { customInput: true }, + }, + ], + sessionId, + toolCall: { + toolCallId: toolUseID, + title: `The agent wants to run \`${subTool}\` on PostHog`, + kind: "other", + content: [ + { + type: "content" as const, + content: text( + "This will modify live PostHog data. Approve to run this sub-tool.", + ), + }, + ], + rawInput: { ...(toolInput as Record), toolName }, + }, + }); + + if (context.signal?.aborted || response.outcome?.outcome === "cancelled") { + throw new Error("Tool use aborted"); + } + + if ( + response.outcome?.outcome === "selected" && + (response.outcome.optionId === "allow" || + response.outcome.optionId === "allow_always") + ) { + if (response.outcome.optionId === "allow_always") { + try { + await session.settingsManager.addPostHogExecApproval(subTool); + } catch (error) { + context.logger.warn( + "[canUseTool] Failed to persist PostHog exec approval", + { error: error instanceof Error ? error.message : String(error) }, + ); + } + } + return { + behavior: "allow", + updatedInput: toolInput as Record, + }; + } + + const feedback = (response._meta?.customInput as string | undefined)?.trim(); + const message = feedback + ? `User refused permission to run tool with feedback: ${feedback}` + : "User refused permission to run tool"; + await emitToolDenial(context, message); + return { behavior: "deny", message, interrupt: !feedback }; +} + function handlePlanFileException( context: ToolHandlerContext, ): ToolPermissionResult | null { @@ -602,6 +679,28 @@ export async function canUseTool( if (approvalState === "needs_approval") { return handleMcpApprovalFlow(context); } + + if (isPostHogExecTool(toolName)) { + const subTool = extractPostHogSubTool(toolInput); + if (subTool && isPostHogDestructiveSubTool(subTool)) { + if ( + session.permissionMode === "auto" || + session.permissionMode === "bypassPermissions" + ) { + return { + behavior: "allow", + updatedInput: toolInput as Record, + }; + } + if (session.settingsManager.hasPostHogExecApproval(subTool)) { + return { + behavior: "allow", + updatedInput: toolInput as Record, + }; + } + return handlePostHogExecApprovalFlow(context, subTool); + } + } } if (isToolAllowedForMode(toolName, session.permissionMode)) { diff --git a/packages/agent/src/adapters/claude/permissions/posthog-exec-gate.test.ts b/packages/agent/src/adapters/claude/permissions/posthog-exec-gate.test.ts new file mode 100644 index 000000000..6c2e82a93 --- /dev/null +++ b/packages/agent/src/adapters/claude/permissions/posthog-exec-gate.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it } from "vitest"; +import { + extractPostHogSubTool, + isPostHogDestructiveSubTool, + isPostHogExecTool, +} from "./posthog-exec-gate"; + +describe("isPostHogExecTool", () => { + it("matches the bare posthog exec tool", () => { + expect(isPostHogExecTool("mcp__posthog__exec")).toBe(true); + }); + + it("matches plugin-prefixed variants", () => { + expect(isPostHogExecTool("mcp__posthog_posthog__exec")).toBe(true); + expect(isPostHogExecTool("mcp__posthog_cloud__exec")).toBe(true); + }); + + it("rejects other MCP tools", () => { + expect(isPostHogExecTool("mcp__posthog__list")).toBe(false); + expect(isPostHogExecTool("mcp__other__exec")).toBe(false); + expect(isPostHogExecTool("mcp__acp__Bash")).toBe(false); + expect(isPostHogExecTool("Bash")).toBe(false); + }); +}); + +describe("extractPostHogSubTool", () => { + it("parses a bare `call ` command", () => { + expect(extractPostHogSubTool({ command: "call experiment-update" })).toBe( + "experiment-update", + ); + }); + + it("parses `call --json `", () => { + expect( + extractPostHogSubTool({ + command: 'call --json experiment-update {"id":1}', + }), + ).toBe("experiment-update"); + }); + + it("tolerates leading whitespace", () => { + expect(extractPostHogSubTool({ command: " call foo-delete" })).toBe( + "foo-delete", + ); + }); + + it("returns null for non-`call` verbs", () => { + expect(extractPostHogSubTool({ command: "tools" })).toBeNull(); + expect(extractPostHogSubTool({ command: "search experiments" })).toBeNull(); + expect(extractPostHogSubTool({ command: "info flag-get" })).toBeNull(); + }); + + it("returns null for missing or malformed input", () => { + expect(extractPostHogSubTool(undefined)).toBeNull(); + expect(extractPostHogSubTool(null)).toBeNull(); + expect(extractPostHogSubTool({})).toBeNull(); + expect(extractPostHogSubTool({ command: 42 })).toBeNull(); + expect(extractPostHogSubTool({ command: "" })).toBeNull(); + }); +}); + +describe("isPostHogDestructiveSubTool", () => { + it("matches update/delete/destroy/partial-update as whole segments", () => { + expect(isPostHogDestructiveSubTool("experiment-update")).toBe(true); + expect(isPostHogDestructiveSubTool("feature-flag-delete")).toBe(true); + expect(isPostHogDestructiveSubTool("notebooks-destroy")).toBe(true); + expect(isPostHogDestructiveSubTool("experiment-partial-update")).toBe(true); + expect(isPostHogDestructiveSubTool("update-something")).toBe(true); + expect(isPostHogDestructiveSubTool("delete")).toBe(true); + }); + + it("does not match read verbs or unrelated tokens", () => { + expect(isPostHogDestructiveSubTool("experiment-get")).toBe(false); + expect(isPostHogDestructiveSubTool("feature-flag-list")).toBe(false); + expect(isPostHogDestructiveSubTool("experiment-create")).toBe(false); + expect(isPostHogDestructiveSubTool("insights-pause")).toBe(false); + }); + + it("does not match substrings inside other words", () => { + // "updated" should not count — must be a whole segment + expect(isPostHogDestructiveSubTool("get-updated-events")).toBe(false); + expect(isPostHogDestructiveSubTool("deleter-test")).toBe(false); + }); +}); diff --git a/packages/agent/src/adapters/claude/permissions/posthog-exec-gate.ts b/packages/agent/src/adapters/claude/permissions/posthog-exec-gate.ts new file mode 100644 index 000000000..2ecdb8c6e --- /dev/null +++ b/packages/agent/src/adapters/claude/permissions/posthog-exec-gate.ts @@ -0,0 +1,30 @@ +/** + * The PostHog MCP exposes a single `exec` dispatcher tool that runs + * subcommands like `call [--json] [json]`. Once the user approves + * `mcp__posthog__exec` once, every subsequent call goes through silently — + * including destructive ones. These helpers let `canUseTool` re-gate the + * destructive subset (update/delete family) at sub-tool granularity. + */ + +const POSTHOG_EXEC_TOOL_RE = /^mcp__posthog(?:_[^_]+)*__exec$/; + +const POSTHOG_CALL_COMMAND_RE = /^\s*call\s+(?:--json\s+)?([a-zA-Z0-9_-]+)/; + +const POSTHOG_DESTRUCTIVE_SUBTOOL_RE = + /(^|-)(partial-update|update|delete|destroy)(-|$)/i; + +export function isPostHogExecTool(toolName: string): boolean { + return POSTHOG_EXEC_TOOL_RE.test(toolName); +} + +export function extractPostHogSubTool(toolInput: unknown): string | null { + if (!toolInput || typeof toolInput !== "object") return null; + const command = (toolInput as { command?: unknown }).command; + if (typeof command !== "string") return null; + const match = command.match(POSTHOG_CALL_COMMAND_RE); + return match ? (match[1] ?? null) : null; +} + +export function isPostHogDestructiveSubTool(subTool: string): boolean { + return POSTHOG_DESTRUCTIVE_SUBTOOL_RE.test(subTool); +} diff --git a/packages/agent/src/adapters/claude/session/settings.test.ts b/packages/agent/src/adapters/claude/session/settings.test.ts index de31eb3d3..960c5d4f6 100644 --- a/packages/agent/src/adapters/claude/session/settings.test.ts +++ b/packages/agent/src/adapters/claude/session/settings.test.ts @@ -127,6 +127,56 @@ describe("SettingsManager per-repo persistence", () => { expect(await fs.promises.readFile(filePath, "utf-8")).toBe(original); }); + it("persists PostHog exec approvals and sees them across worktrees", async () => { + const writer = new SettingsManager(worktree); + await writer.initialize(); + await writer.addPostHogExecApproval("experiment-update"); + + const filePath = path.join(mainRepo, ".claude", "settings.local.json"); + const contents = JSON.parse(await fs.promises.readFile(filePath, "utf-8")); + expect(contents.posthogApprovedExecTools).toEqual(["experiment-update"]); + + const sibling = path.join(tmpRoot, "wt-ph"); + runGit(mainRepo, ["worktree", "add", "-b", "other-ph", sibling]); + const reader = new SettingsManager(sibling); + await reader.initialize(); + expect(reader.hasPostHogExecApproval("experiment-update")).toBe(true); + expect(reader.hasPostHogExecApproval("experiment-delete")).toBe(false); + }); + + it("dedupes repeated PostHog exec approvals", async () => { + const manager = new SettingsManager(worktree); + await manager.initialize(); + + await manager.addPostHogExecApproval("foo-update"); + await manager.addPostHogExecApproval("foo-update"); + await manager.addPostHogExecApproval("bar-delete"); + + const filePath = path.join(mainRepo, ".claude", "settings.local.json"); + const contents = JSON.parse(await fs.promises.readFile(filePath, "utf-8")); + expect(contents.posthogApprovedExecTools).toEqual([ + "foo-update", + "bar-delete", + ]); + }); + + it("concurrent addPostHogExecApproval calls do not clobber each other", async () => { + const manager = new SettingsManager(worktree); + await manager.initialize(); + + await Promise.all([ + manager.addPostHogExecApproval("a-update"), + manager.addPostHogExecApproval("b-delete"), + manager.addPostHogExecApproval("c-destroy"), + ]); + + const filePath = path.join(mainRepo, ".claude", "settings.local.json"); + const contents = JSON.parse(await fs.promises.readFile(filePath, "utf-8")); + expect(contents.posthogApprovedExecTools).toEqual( + expect.arrayContaining(["a-update", "b-delete", "c-destroy"]), + ); + }); + it("concurrent addAllowRules calls do not clobber each other", async () => { const manager = new SettingsManager(worktree); await manager.initialize(); diff --git a/packages/agent/src/adapters/claude/session/settings.ts b/packages/agent/src/adapters/claude/session/settings.ts index 67ce16670..0a2b8e39b 100644 --- a/packages/agent/src/adapters/claude/session/settings.ts +++ b/packages/agent/src/adapters/claude/session/settings.ts @@ -196,6 +196,7 @@ export interface ClaudeCodeSettings { permissions?: PermissionSettings; env?: Record; model?: string; + posthogApprovedExecTools?: string[]; } export type PermissionDecision = "allow" | "deny" | "ask"; @@ -295,6 +296,7 @@ export class SettingsManager { ask: [], }; const merged: ClaudeCodeSettings = { permissions }; + const posthogApprovedExecTools = new Set(); for (const settings of allSettings) { if (settings.permissions) { @@ -323,6 +325,15 @@ export class SettingsManager { if (settings.model) { merged.model = settings.model; } + if (settings.posthogApprovedExecTools) { + for (const tool of settings.posthogApprovedExecTools) { + posthogApprovedExecTools.add(tool); + } + } + } + + if (posthogApprovedExecTools.size > 0) { + merged.posthogApprovedExecTools = Array.from(posthogApprovedExecTools); } this.mergedSettings = merged; @@ -405,6 +416,43 @@ export class SettingsManager { } } + hasPostHogExecApproval(subTool: string): boolean { + return ( + this.mergedSettings.posthogApprovedExecTools?.includes(subTool) ?? false + ); + } + + /** + * Persists an approved PostHog MCP `exec` sub-tool (e.g. `experiment-update`) + * to the local settings file so future calls skip the prompt. Mirrors + * `addAllowRules` — serialised via `writeMutex`, atomic temp-file + rename. + */ + async addPostHogExecApproval(subTool: string): Promise { + if (!subTool) return; + if (!this.initialized) await this.initialize(); + await this.writeMutex.acquire(); + try { + const filePath = this.getLocalSettingsPath(); + const existing = await readSettingsFileForUpdate(filePath); + const current = new Set(existing.posthogApprovedExecTools ?? []); + if (current.has(subTool)) { + return; + } + current.add(subTool); + const next: ClaudeCodeSettings = { + ...existing, + posthogApprovedExecTools: Array.from(current), + }; + await fs.promises.mkdir(path.dirname(filePath), { recursive: true }); + await writeFileAtomic(filePath, `${JSON.stringify(next, null, 2)}\n`); + + this.localSettings = next; + this.mergeAllSettings(); + } finally { + this.writeMutex.release(); + } + } + async setCwd(cwd: string): Promise { if (this.cwd === cwd) return; if (this.initPromise) await this.initPromise; From 48dc2a7d7c051db53ca8bbacf105a71fda6b8e85 Mon Sep 17 00:00:00 2001 From: Georgiy Tarasov Date: Mon, 4 May 2026 12:19:42 +0200 Subject: [PATCH 2/4] wip --- .../agent/src/adapters/claude/hooks.test.ts | 126 +++++++++++++++++- packages/agent/src/adapters/claude/hooks.ts | 38 +++++- .../claude/permissions/permission-handlers.ts | 57 +++++++- packages/agent/src/utils/common.ts | 20 +++ 4 files changed, 233 insertions(+), 8 deletions(-) diff --git a/packages/agent/src/adapters/claude/hooks.test.ts b/packages/agent/src/adapters/claude/hooks.test.ts index ec096d79d..771aa4399 100644 --- a/packages/agent/src/adapters/claude/hooks.test.ts +++ b/packages/agent/src/adapters/claude/hooks.test.ts @@ -7,7 +7,16 @@ vi.mock("../../enrichment/file-enricher", () => ({ enrichFileForAgent: enrichFileMock, })); -import { createReadEnrichmentHook, type EnrichedReadCache } from "./hooks"; +import { Logger } from "../../utils/logger"; +import { + createPreToolUseHook, + createReadEnrichmentHook, + type EnrichedReadCache, +} from "./hooks"; +import type { + PermissionCheckResult, + SettingsManager, +} from "./session/settings"; const stubDeps = {} as FileEnrichmentDeps; @@ -187,3 +196,118 @@ describe("createReadEnrichmentHook", () => { expect(content).toBe("foo"); }); }); + +function buildPreToolUseHookInput( + toolName: string, + toolInput: Record, +): HookInput { + return { + session_id: "test-session", + transcript_path: "/tmp/transcript", + cwd: "/tmp", + hook_event_name: "PreToolUse", + tool_name: toolName, + tool_use_id: "toolu_1", + tool_input: toolInput, + } as HookInput; +} + +function buildSettingsManagerStub( + result: PermissionCheckResult, +): SettingsManager { + return { + checkPermission: () => result, + } as unknown as SettingsManager; +} + +describe("createPreToolUseHook", () => { + const logger = new Logger({ debug: false }); + + test("defers destructive PostHog exec sub-tool to canUseTool via ask", async () => { + const settingsManager = buildSettingsManagerStub({ + decision: "allow", + rule: "mcp__posthog__exec", + source: "allow", + }); + const hook = createPreToolUseHook(settingsManager, logger); + const result = await hook( + buildPreToolUseHookInput("mcp__posthog__exec", { + command: 'call dashboard-update {"id": 1, "name": "x"}', + }), + undefined, + { signal: new AbortController().signal }, + ); + + expect(result).toMatchObject({ + continue: true, + hookSpecificOutput: { + hookEventName: "PreToolUse", + permissionDecision: "ask", + }, + }); + }); + + test("allows non-destructive PostHog exec sub-tool via settings rule", async () => { + const settingsManager = buildSettingsManagerStub({ + decision: "allow", + rule: "mcp__posthog__exec", + source: "allow", + }); + const hook = createPreToolUseHook(settingsManager, logger); + const result = await hook( + buildPreToolUseHookInput("mcp__posthog__exec", { + command: 'call experiment-get {"id": 1}', + }), + undefined, + { signal: new AbortController().signal }, + ); + + expect(result).toEqual({ + continue: true, + hookSpecificOutput: { + hookEventName: "PreToolUse", + permissionDecision: "allow", + permissionDecisionReason: + "Allowed by settings rule: mcp__posthog__exec", + }, + }); + }); + + test("allows non-PostHog tool via settings rule unchanged", async () => { + const settingsManager = buildSettingsManagerStub({ + decision: "allow", + rule: "Bash(ls:*)", + source: "allow", + }); + const hook = createPreToolUseHook(settingsManager, logger); + const result = await hook( + buildPreToolUseHookInput("Bash", { command: "ls -la" }), + undefined, + { signal: new AbortController().signal }, + ); + + expect(result).toMatchObject({ + hookSpecificOutput: { permissionDecision: "allow" }, + }); + }); + + test("defers when destructive rule is partial-update", async () => { + const settingsManager = buildSettingsManagerStub({ + decision: "allow", + rule: "mcp__posthog__exec", + source: "allow", + }); + const hook = createPreToolUseHook(settingsManager, logger); + const result = await hook( + buildPreToolUseHookInput("mcp__posthog__exec", { + command: 'call cohorts-partial-update {"id": 1}', + }), + undefined, + { signal: new AbortController().signal }, + ); + + expect(result).toMatchObject({ + hookSpecificOutput: { permissionDecision: "ask" }, + }); + }); +}); diff --git a/packages/agent/src/adapters/claude/hooks.ts b/packages/agent/src/adapters/claude/hooks.ts index a9a3073f1..aae9ab47d 100644 --- a/packages/agent/src/adapters/claude/hooks.ts +++ b/packages/agent/src/adapters/claude/hooks.ts @@ -3,8 +3,14 @@ import { enrichFileForAgent, type FileEnrichmentDeps, } from "../../enrichment/file-enricher"; +import { truncateForLog } from "../../utils/common"; import type { Logger } from "../../utils/logger"; import { stripCatLineNumbers } from "./conversion/sdk-to-acp"; +import { + extractPostHogSubTool, + isPostHogDestructiveSubTool, + isPostHogExecTool, +} from "./permissions/posthog-exec-gate"; import type { SettingsManager } from "./session/settings"; import type { CodeExecutionMode } from "./tools"; @@ -231,7 +237,37 @@ export const createPreToolUseHook = toolInput, ); - if (permissionCheck.decision !== "ask") { + const isPosthogExec = isPostHogExecTool(toolName); + if (isPosthogExec) { + const subTool = extractPostHogSubTool(toolInput); + const isDestructive = subTool + ? isPostHogDestructiveSubTool(subTool) + : false; + logger.info("[PreToolUseHook] PostHog exec", { + toolName, + decision: permissionCheck.decision, + rule: permissionCheck.rule, + subTool, + isDestructive, + toolInput: truncateForLog(toolInput), + }); + + // Defer destructive PostHog exec sub-tools to canUseTool so the + // sub-tool gate can re-prompt. Returning `{ continue: true }` is + // not enough — the SDK then falls back to its default permission + // flow which re-checks the same allow rule. We must force "ask" + // so the SDK invokes canUseTool. + if (permissionCheck.decision === "allow" && subTool && isDestructive) { + return { + continue: true, + hookSpecificOutput: { + hookEventName: "PreToolUse" as const, + permissionDecision: "ask" as const, + permissionDecisionReason: `Destructive PostHog sub-tool '${subTool}' requires explicit approval`, + }, + }; + } + } else if (permissionCheck.decision !== "ask") { logger.info( `[PreToolUseHook] Tool: ${toolName}, Decision: ${permissionCheck.decision}, Rule: ${permissionCheck.rule}`, ); diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts index a51f529c1..7714a807a 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts @@ -7,6 +7,7 @@ import type { PermissionUpdate, } from "@anthropic-ai/claude-agent-sdk"; import { text } from "../../../utils/acp-content"; +import { truncateForLog } from "../../../utils/common"; import type { Logger } from "../../../utils/logger"; import { toolInfoFromToolUse } from "../conversion/tool-use-to-acp"; import { @@ -70,7 +71,7 @@ async function emitToolDenial( message: string, ): Promise { context.logger.info(`[canUseTool] Tool denied: ${context.toolName}`, { - message, + message: truncateForLog(message), }); await context.client.sessionUpdate({ sessionId: context.sessionId, @@ -496,9 +497,16 @@ async function handlePostHogExecApprovalFlow( context: ToolHandlerContext, subTool: string, ): Promise { - const { toolName, toolInput, toolUseID, client, sessionId, session } = + const { toolName, toolInput, toolUseID, client, sessionId, session, logger } = context; + logger.info("[handlePostHogExecApprovalFlow] requesting permission", { + toolName, + subTool, + toolUseID, + sessionId, + }); + const response = await client.requestPermission({ options: [ { kind: "allow_once", name: "Yes", optionId: "allow" }, @@ -531,6 +539,16 @@ async function handlePostHogExecApprovalFlow( }, }); + logger.info("[handlePostHogExecApprovalFlow] permission response", { + subTool, + outcome: response.outcome?.outcome, + optionId: + response.outcome?.outcome === "selected" + ? response.outcome.optionId + : undefined, + aborted: context.signal?.aborted, + }); + if (context.signal?.aborted || response.outcome?.outcome === "cancelled") { throw new Error("Tool use aborted"); } @@ -541,11 +559,14 @@ async function handlePostHogExecApprovalFlow( response.outcome.optionId === "allow_always") ) { if (response.outcome.optionId === "allow_always") { + logger.info("[handlePostHogExecApprovalFlow] persisting approval", { + subTool, + }); try { await session.settingsManager.addPostHogExecApproval(subTool); } catch (error) { - context.logger.warn( - "[canUseTool] Failed to persist PostHog exec approval", + logger.warn( + "[handlePostHogExecApprovalFlow] Failed to persist PostHog exec approval", { error: error instanceof Error ? error.message : String(error) }, ); } @@ -560,6 +581,10 @@ async function handlePostHogExecApprovalFlow( const message = feedback ? `User refused permission to run tool with feedback: ${feedback}` : "User refused permission to run tool"; + logger.info("[handlePostHogExecApprovalFlow] denied by user", { + subTool, + hasFeedback: !!feedback, + }); await emitToolDenial(context, message); return { behavior: "deny", message, interrupt: !feedback }; } @@ -649,7 +674,7 @@ function isDomainAllowed(hostname: string, allowedDomains: string[]): boolean { export async function canUseTool( context: ToolHandlerContext, ): Promise { - const { toolName, toolInput, session, allowedDomains } = context; + const { toolName, toolInput, session, allowedDomains, logger } = context; // Enforce domain allowlist for web tools if (allowedDomains && allowedDomains.length > 0) { @@ -682,17 +707,37 @@ export async function canUseTool( if (isPostHogExecTool(toolName)) { const subTool = extractPostHogSubTool(toolInput); - if (subTool && isPostHogDestructiveSubTool(subTool)) { + const isDestructive = subTool + ? isPostHogDestructiveSubTool(subTool) + : false; + logger.info("[canUseTool] PostHog exec sub-tool", { + toolName, + subTool, + isDestructive, + permissionMode: session.permissionMode, + rawCommand: truncateForLog( + (toolInput as { command?: unknown }).command, + ), + }); + if (subTool && isDestructive) { if ( session.permissionMode === "auto" || session.permissionMode === "bypassPermissions" ) { + logger.info( + "[canUseTool] PostHog exec auto-allowed by permission mode", + { subTool, permissionMode: session.permissionMode }, + ); return { behavior: "allow", updatedInput: toolInput as Record, }; } if (session.settingsManager.hasPostHogExecApproval(subTool)) { + logger.info( + "[canUseTool] PostHog exec auto-allowed by stored approval", + { subTool }, + ); return { behavior: "allow", updatedInput: toolInput as Record, diff --git a/packages/agent/src/utils/common.ts b/packages/agent/src/utils/common.ts index 3bcf09acd..f2f2a1130 100644 --- a/packages/agent/src/utils/common.ts +++ b/packages/agent/src/utils/common.ts @@ -34,3 +34,23 @@ export function unreachable(value: never, logger: Logger): void { } logger.error(`Unexpected case: ${valueAsString}`); } + +const DEFAULT_TRUNCATE_LIMIT = 200; + +export function truncateForLog( + value: unknown, + limit: number = DEFAULT_TRUNCATE_LIMIT, +): string { + let str: string; + if (typeof value === "string") { + str = value; + } else { + try { + str = JSON.stringify(value); + } catch { + str = String(value); + } + } + if (str.length <= limit) return str; + return `${str.slice(0, limit)}…(+${str.length - limit} chars)`; +} From 8f8e72f65f526b3655452043f8e4f3dc444c7531 Mon Sep 17 00:00:00 2001 From: Georgiy Tarasov Date: Mon, 4 May 2026 12:44:19 +0200 Subject: [PATCH 3/4] fix: remove logs --- packages/agent/src/adapters/claude/hooks.ts | 38 +++++-------- .../claude/permissions/permission-handlers.ts | 57 ++----------------- packages/agent/src/utils/common.ts | 20 ------- 3 files changed, 19 insertions(+), 96 deletions(-) diff --git a/packages/agent/src/adapters/claude/hooks.ts b/packages/agent/src/adapters/claude/hooks.ts index aae9ab47d..3dc59be89 100644 --- a/packages/agent/src/adapters/claude/hooks.ts +++ b/packages/agent/src/adapters/claude/hooks.ts @@ -3,7 +3,6 @@ import { enrichFileForAgent, type FileEnrichmentDeps, } from "../../enrichment/file-enricher"; -import { truncateForLog } from "../../utils/common"; import type { Logger } from "../../utils/logger"; import { stripCatLineNumbers } from "./conversion/sdk-to-acp"; import { @@ -237,27 +236,20 @@ export const createPreToolUseHook = toolInput, ); - const isPosthogExec = isPostHogExecTool(toolName); - if (isPosthogExec) { - const subTool = extractPostHogSubTool(toolInput); - const isDestructive = subTool - ? isPostHogDestructiveSubTool(subTool) - : false; - logger.info("[PreToolUseHook] PostHog exec", { - toolName, - decision: permissionCheck.decision, - rule: permissionCheck.rule, - subTool, - isDestructive, - toolInput: truncateForLog(toolInput), - }); + if (permissionCheck.decision !== "ask") { + logger.info( + `[PreToolUseHook] Tool: ${toolName}, Decision: ${permissionCheck.decision}, Rule: ${permissionCheck.rule}`, + ); + } - // Defer destructive PostHog exec sub-tools to canUseTool so the - // sub-tool gate can re-prompt. Returning `{ continue: true }` is - // not enough — the SDK then falls back to its default permission - // flow which re-checks the same allow rule. We must force "ask" - // so the SDK invokes canUseTool. - if (permissionCheck.decision === "allow" && subTool && isDestructive) { + // Defer destructive PostHog exec sub-tools to canUseTool so the + // sub-tool gate can re-prompt. Returning `{ continue: true }` is + // not enough — the SDK then falls back to its default permission + // flow which re-checks the same allow rule. We must force "ask" + // so the SDK invokes canUseTool. + if (permissionCheck.decision === "allow" && isPostHogExecTool(toolName)) { + const subTool = extractPostHogSubTool(toolInput); + if (subTool && isPostHogDestructiveSubTool(subTool)) { return { continue: true, hookSpecificOutput: { @@ -267,10 +259,6 @@ export const createPreToolUseHook = }, }; } - } else if (permissionCheck.decision !== "ask") { - logger.info( - `[PreToolUseHook] Tool: ${toolName}, Decision: ${permissionCheck.decision}, Rule: ${permissionCheck.rule}`, - ); } switch (permissionCheck.decision) { diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts index 7714a807a..a51f529c1 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts @@ -7,7 +7,6 @@ import type { PermissionUpdate, } from "@anthropic-ai/claude-agent-sdk"; import { text } from "../../../utils/acp-content"; -import { truncateForLog } from "../../../utils/common"; import type { Logger } from "../../../utils/logger"; import { toolInfoFromToolUse } from "../conversion/tool-use-to-acp"; import { @@ -71,7 +70,7 @@ async function emitToolDenial( message: string, ): Promise { context.logger.info(`[canUseTool] Tool denied: ${context.toolName}`, { - message: truncateForLog(message), + message, }); await context.client.sessionUpdate({ sessionId: context.sessionId, @@ -497,16 +496,9 @@ async function handlePostHogExecApprovalFlow( context: ToolHandlerContext, subTool: string, ): Promise { - const { toolName, toolInput, toolUseID, client, sessionId, session, logger } = + const { toolName, toolInput, toolUseID, client, sessionId, session } = context; - logger.info("[handlePostHogExecApprovalFlow] requesting permission", { - toolName, - subTool, - toolUseID, - sessionId, - }); - const response = await client.requestPermission({ options: [ { kind: "allow_once", name: "Yes", optionId: "allow" }, @@ -539,16 +531,6 @@ async function handlePostHogExecApprovalFlow( }, }); - logger.info("[handlePostHogExecApprovalFlow] permission response", { - subTool, - outcome: response.outcome?.outcome, - optionId: - response.outcome?.outcome === "selected" - ? response.outcome.optionId - : undefined, - aborted: context.signal?.aborted, - }); - if (context.signal?.aborted || response.outcome?.outcome === "cancelled") { throw new Error("Tool use aborted"); } @@ -559,14 +541,11 @@ async function handlePostHogExecApprovalFlow( response.outcome.optionId === "allow_always") ) { if (response.outcome.optionId === "allow_always") { - logger.info("[handlePostHogExecApprovalFlow] persisting approval", { - subTool, - }); try { await session.settingsManager.addPostHogExecApproval(subTool); } catch (error) { - logger.warn( - "[handlePostHogExecApprovalFlow] Failed to persist PostHog exec approval", + context.logger.warn( + "[canUseTool] Failed to persist PostHog exec approval", { error: error instanceof Error ? error.message : String(error) }, ); } @@ -581,10 +560,6 @@ async function handlePostHogExecApprovalFlow( const message = feedback ? `User refused permission to run tool with feedback: ${feedback}` : "User refused permission to run tool"; - logger.info("[handlePostHogExecApprovalFlow] denied by user", { - subTool, - hasFeedback: !!feedback, - }); await emitToolDenial(context, message); return { behavior: "deny", message, interrupt: !feedback }; } @@ -674,7 +649,7 @@ function isDomainAllowed(hostname: string, allowedDomains: string[]): boolean { export async function canUseTool( context: ToolHandlerContext, ): Promise { - const { toolName, toolInput, session, allowedDomains, logger } = context; + const { toolName, toolInput, session, allowedDomains } = context; // Enforce domain allowlist for web tools if (allowedDomains && allowedDomains.length > 0) { @@ -707,37 +682,17 @@ export async function canUseTool( if (isPostHogExecTool(toolName)) { const subTool = extractPostHogSubTool(toolInput); - const isDestructive = subTool - ? isPostHogDestructiveSubTool(subTool) - : false; - logger.info("[canUseTool] PostHog exec sub-tool", { - toolName, - subTool, - isDestructive, - permissionMode: session.permissionMode, - rawCommand: truncateForLog( - (toolInput as { command?: unknown }).command, - ), - }); - if (subTool && isDestructive) { + if (subTool && isPostHogDestructiveSubTool(subTool)) { if ( session.permissionMode === "auto" || session.permissionMode === "bypassPermissions" ) { - logger.info( - "[canUseTool] PostHog exec auto-allowed by permission mode", - { subTool, permissionMode: session.permissionMode }, - ); return { behavior: "allow", updatedInput: toolInput as Record, }; } if (session.settingsManager.hasPostHogExecApproval(subTool)) { - logger.info( - "[canUseTool] PostHog exec auto-allowed by stored approval", - { subTool }, - ); return { behavior: "allow", updatedInput: toolInput as Record, diff --git a/packages/agent/src/utils/common.ts b/packages/agent/src/utils/common.ts index f2f2a1130..3bcf09acd 100644 --- a/packages/agent/src/utils/common.ts +++ b/packages/agent/src/utils/common.ts @@ -34,23 +34,3 @@ export function unreachable(value: never, logger: Logger): void { } logger.error(`Unexpected case: ${valueAsString}`); } - -const DEFAULT_TRUNCATE_LIMIT = 200; - -export function truncateForLog( - value: unknown, - limit: number = DEFAULT_TRUNCATE_LIMIT, -): string { - let str: string; - if (typeof value === "string") { - str = value; - } else { - try { - str = JSON.stringify(value); - } catch { - str = String(value); - } - } - if (str.length <= limit) return str; - return `${str.slice(0, limit)}…(+${str.length - limit} chars)`; -} From 72cb1e19ecebe6212facc3de26cedb6d565b5a81 Mon Sep 17 00:00:00 2001 From: Georgiy Tarasov Date: Mon, 4 May 2026 14:25:07 +0200 Subject: [PATCH 4/4] fix --- .../claude/permissions/permission-handlers.ts | 37 ++++++++----------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts index a51f529c1..f8bb8d5d5 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts @@ -83,6 +83,18 @@ async function emitToolDenial( }); } +async function buildDenialResult( + context: ToolHandlerContext, + response: RequestPermissionResponse, +): Promise { + const feedback = (response._meta?.customInput as string | undefined)?.trim(); + const message = feedback + ? `User refused permission to run tool with feedback: ${feedback}` + : "User refused permission to run tool"; + await emitToolDenial(context, message); + return { behavior: "deny", message, interrupt: !feedback }; +} + function getPlanFromFile( session: Session, fileContentCache: { [key: string]: string }, @@ -394,16 +406,9 @@ async function handleDefaultPermissionFlow( behavior: "allow", updatedInput: toolInput as Record, }; - } else { - const feedback = ( - response._meta?.customInput as string | undefined - )?.trim(); - const message = feedback - ? `User refused permission to run tool with feedback: ${feedback}` - : "User refused permission to run tool"; - await emitToolDenial(context, message); - return { behavior: "deny", message, interrupt: !feedback }; } + + return buildDenialResult(context, response); } function parseMcpToolName(toolName: string): { @@ -484,12 +489,7 @@ async function handleMcpApprovalFlow( }; } - const feedback = (response._meta?.customInput as string | undefined)?.trim(); - const message = feedback - ? `User refused permission to run tool with feedback: ${feedback}` - : "User refused permission to run tool"; - await emitToolDenial(context, message); - return { behavior: "deny", message, interrupt: !feedback }; + return buildDenialResult(context, response); } async function handlePostHogExecApprovalFlow( @@ -556,12 +556,7 @@ async function handlePostHogExecApprovalFlow( }; } - const feedback = (response._meta?.customInput as string | undefined)?.trim(); - const message = feedback - ? `User refused permission to run tool with feedback: ${feedback}` - : "User refused permission to run tool"; - await emitToolDenial(context, message); - return { behavior: "deny", message, interrupt: !feedback }; + return buildDenialResult(context, response); } function handlePlanFileException(