Skip to content

Commit c2ca91a

Browse files
committed
fix(agent): restore remembered Claude exec approvals
1 parent 675d27d commit c2ca91a

7 files changed

Lines changed: 196 additions & 19 deletions

File tree

packages/agent/README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,9 @@ Cloud provisioning can pass `--posthogExecPermissionRegex <regex>` to require
7878
one-time client approval for matching PostHog MCP `exec` sub-tools in every
7979
interactive Claude and Codex permission mode. Matching is case-insensitive
8080
against the delegated name in `call [--json] <sub-tool> ...`. These prompts do
81-
not offer an always-allow choice and are not remembered. Background runs keep
82-
their existing auto-approval behavior. The default is
81+
offer Claude users an always-allow choice remembered in local repository
82+
settings; Codex approvals remain one-time. Background runs keep their existing
83+
auto-approval behavior. The default is
8384
`(^|-)(partial-update|update|patch|delete|destroy)(-|$)`.
8485

8586
## ACP connection layer

packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ describe("canUseTool MCP approval enforcement", () => {
215215
"auto",
216216
"bypassPermissions",
217217
] as const)(
218-
"prompts for a configured PostHog exec match in %s mode without remembering",
218+
"prompts for a configured PostHog exec match in %s mode with a remembered choice",
219219
async (permissionMode) => {
220220
setMcpToolApprovalStates({ mcp__posthog__exec: "approved" });
221221

@@ -226,6 +226,8 @@ describe("canUseTool MCP approval enforcement", () => {
226226
posthogExecPermissionRegex,
227227
settingsManager: {
228228
getRepoRoot: vi.fn().mockReturnValue("/repo"),
229+
hasPostHogExecApproval: vi.fn().mockReturnValue(false),
230+
addPostHogExecApproval: vi.fn(),
229231
},
230232
},
231233
});
@@ -236,6 +238,7 @@ describe("canUseTool MCP approval enforcement", () => {
236238
expect.objectContaining({
237239
options: [
238240
expect.objectContaining({ kind: "allow_once" }),
241+
expect.objectContaining({ kind: "allow_always" }),
239242
expect.objectContaining({ kind: "reject_once" }),
240243
],
241244
toolCall: expect.objectContaining({
@@ -247,6 +250,54 @@ describe("canUseTool MCP approval enforcement", () => {
247250
},
248251
);
249252

253+
it("skips the prompt for a remembered PostHog exec sub-tool", async () => {
254+
setMcpToolApprovalStates({ mcp__posthog__exec: "approved" });
255+
256+
const context = createContext("mcp__posthog__exec", {
257+
toolInput: { command: "call experiment-update {}" },
258+
session: {
259+
permissionMode: "default",
260+
posthogExecPermissionRegex,
261+
settingsManager: {
262+
getRepoRoot: vi.fn().mockReturnValue("/repo"),
263+
hasPostHogExecApproval: vi.fn().mockReturnValue(true),
264+
addPostHogExecApproval: vi.fn(),
265+
},
266+
},
267+
});
268+
269+
const result = await canUseTool(context);
270+
271+
expect(result.behavior).toBe("allow");
272+
expect(context.client.requestPermission).not.toHaveBeenCalled();
273+
});
274+
275+
it("persists a PostHog exec sub-tool selected with allow always", async () => {
276+
setMcpToolApprovalStates({ mcp__posthog__exec: "approved" });
277+
const addPostHogExecApproval = vi.fn().mockResolvedValue(undefined);
278+
279+
const context = createContext("mcp__posthog__exec", {
280+
toolInput: { command: "call notebooks-destroy {}" },
281+
session: {
282+
permissionMode: "default",
283+
posthogExecPermissionRegex,
284+
settingsManager: {
285+
getRepoRoot: vi.fn().mockReturnValue("/repo"),
286+
hasPostHogExecApproval: vi.fn().mockReturnValue(false),
287+
addPostHogExecApproval,
288+
},
289+
},
290+
client: createClient({
291+
outcome: { outcome: "selected", optionId: "allow_always" },
292+
}),
293+
});
294+
295+
const result = await canUseTool(context);
296+
297+
expect(result.behavior).toBe("allow");
298+
expect(addPostHogExecApproval).toHaveBeenCalledWith("notebooks-destroy");
299+
});
300+
250301
it("does not gate a nonmatching PostHog sub-tool", async () => {
251302
setMcpToolApprovalStates({ mcp__posthog__exec: "approved" });
252303

packages/agent/src/adapters/claude/permissions/permission-handlers.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -586,11 +586,16 @@ async function handlePostHogExecApprovalFlow(
586586
context: ToolHandlerContext,
587587
subTool: string,
588588
): Promise<ToolPermissionResult> {
589-
const { toolName, toolInput, toolUseID, sessionId } = context;
589+
const { toolName, toolInput, toolUseID, sessionId, session } = context;
590590

591591
const response = await requestPermissionFromClient(context, {
592592
options: [
593593
{ kind: "allow_once", name: "Yes", optionId: "allow" },
594+
{
595+
kind: "allow_always",
596+
name: "Yes, always allow",
597+
optionId: "allow_always",
598+
},
594599
{
595600
kind: "reject_once",
596601
name: "Type here to tell the agent what to do differently",
@@ -622,8 +627,19 @@ async function handlePostHogExecApprovalFlow(
622627

623628
if (
624629
response.outcome?.outcome === "selected" &&
625-
response.outcome.optionId === "allow"
630+
(response.outcome.optionId === "allow" ||
631+
response.outcome.optionId === "allow_always")
626632
) {
633+
if (response.outcome.optionId === "allow_always") {
634+
try {
635+
await session.settingsManager.addPostHogExecApproval(subTool);
636+
} catch (error) {
637+
context.logger.warn(
638+
"[canUseTool] Failed to persist PostHog exec approval",
639+
{ error: error instanceof Error ? error.message : String(error) },
640+
);
641+
}
642+
}
627643
return {
628644
behavior: "allow",
629645
updatedInput: toolInput as Record<string, unknown>,
@@ -754,6 +770,12 @@ export async function canUseTool(
754770
session.posthogExecPermissionRegex,
755771
)
756772
) {
773+
if (session.settingsManager.hasPostHogExecApproval(subTool)) {
774+
return {
775+
behavior: "allow",
776+
updatedInput: toolInput as Record<string, unknown>,
777+
};
778+
}
757779
return handlePostHogExecApprovalFlow(context, subTool);
758780
}
759781
}

packages/agent/src/adapters/claude/session/settings.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,56 @@ describe("SettingsManager per-repo persistence", () => {
127127
expect(await fs.promises.readFile(filePath, "utf-8")).toBe(original);
128128
});
129129

130+
it("persists PostHog exec approvals and sees them across worktrees", async () => {
131+
const writer = new SettingsManager(worktree);
132+
await writer.initialize();
133+
await writer.addPostHogExecApproval("experiment-update");
134+
135+
const filePath = path.join(mainRepo, ".claude", "settings.local.json");
136+
const contents = JSON.parse(await fs.promises.readFile(filePath, "utf-8"));
137+
expect(contents.posthogApprovedExecTools).toEqual(["experiment-update"]);
138+
139+
const sibling = path.join(tmpRoot, "wt-ph");
140+
runGit(mainRepo, ["worktree", "add", "-b", "other-ph", sibling]);
141+
const reader = new SettingsManager(sibling);
142+
await reader.initialize();
143+
expect(reader.hasPostHogExecApproval("experiment-update")).toBe(true);
144+
expect(reader.hasPostHogExecApproval("experiment-delete")).toBe(false);
145+
});
146+
147+
it("dedupes repeated PostHog exec approvals", async () => {
148+
const manager = new SettingsManager(worktree);
149+
await manager.initialize();
150+
151+
await manager.addPostHogExecApproval("foo-update");
152+
await manager.addPostHogExecApproval("foo-update");
153+
await manager.addPostHogExecApproval("bar-delete");
154+
155+
const filePath = path.join(mainRepo, ".claude", "settings.local.json");
156+
const contents = JSON.parse(await fs.promises.readFile(filePath, "utf-8"));
157+
expect(contents.posthogApprovedExecTools).toEqual([
158+
"foo-update",
159+
"bar-delete",
160+
]);
161+
});
162+
163+
it("concurrent addPostHogExecApproval calls do not clobber each other", async () => {
164+
const manager = new SettingsManager(worktree);
165+
await manager.initialize();
166+
167+
await Promise.all([
168+
manager.addPostHogExecApproval("a-update"),
169+
manager.addPostHogExecApproval("b-delete"),
170+
manager.addPostHogExecApproval("c-destroy"),
171+
]);
172+
173+
const filePath = path.join(mainRepo, ".claude", "settings.local.json");
174+
const contents = JSON.parse(await fs.promises.readFile(filePath, "utf-8"));
175+
expect(contents.posthogApprovedExecTools).toEqual(
176+
expect.arrayContaining(["a-update", "b-delete", "c-destroy"]),
177+
);
178+
});
179+
130180
it("concurrent addAllowRules calls do not clobber each other", async () => {
131181
const manager = new SettingsManager(worktree);
132182
await manager.initialize();

packages/agent/src/adapters/claude/session/settings.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ export interface ClaudeCodeSettings {
197197
env?: Record<string, string>;
198198
model?: string;
199199
availableModels?: string[];
200+
posthogApprovedExecTools?: string[];
200201
}
201202

202203
type SettingsLayer = "user" | "project" | "local" | "enterprise";
@@ -317,6 +318,7 @@ export class SettingsManager {
317318
ask: [],
318319
};
319320
const merged: ClaudeCodeSettings = { permissions };
321+
const posthogApprovedExecTools = new Set<string>();
320322

321323
for (const { layer, settings } of allSettings) {
322324
if (settings.permissions) {
@@ -350,6 +352,15 @@ export class SettingsManager {
350352
settings.availableModels,
351353
layer,
352354
);
355+
if (settings.posthogApprovedExecTools) {
356+
for (const tool of settings.posthogApprovedExecTools) {
357+
posthogApprovedExecTools.add(tool);
358+
}
359+
}
360+
}
361+
362+
if (posthogApprovedExecTools.size > 0) {
363+
merged.posthogApprovedExecTools = Array.from(posthogApprovedExecTools);
353364
}
354365

355366
this.mergedSettings = merged;
@@ -432,6 +443,43 @@ export class SettingsManager {
432443
}
433444
}
434445

446+
hasPostHogExecApproval(subTool: string): boolean {
447+
return (
448+
this.mergedSettings.posthogApprovedExecTools?.includes(subTool) ?? false
449+
);
450+
}
451+
452+
/**
453+
* Persists an approved PostHog MCP `exec` sub-tool (e.g. `experiment-update`)
454+
* to the local settings file so future calls skip the prompt. Mirrors
455+
* `addAllowRules` — serialised via `writeMutex`, atomic temp-file + rename.
456+
*/
457+
async addPostHogExecApproval(subTool: string): Promise<void> {
458+
if (!subTool) return;
459+
if (!this.initialized) await this.initialize();
460+
await this.writeMutex.acquire();
461+
try {
462+
const filePath = this.getLocalSettingsPath();
463+
const existing = await readSettingsFileForUpdate(filePath);
464+
const current = new Set(existing.posthogApprovedExecTools ?? []);
465+
if (current.has(subTool)) {
466+
return;
467+
}
468+
current.add(subTool);
469+
const next: ClaudeCodeSettings = {
470+
...existing,
471+
posthogApprovedExecTools: Array.from(current),
472+
};
473+
await fs.promises.mkdir(path.dirname(filePath), { recursive: true });
474+
await writeFileAtomic(filePath, `${JSON.stringify(next, null, 2)}\n`);
475+
476+
this.localSettings = next;
477+
this.mergeAllSettings();
478+
} finally {
479+
this.writeMutex.release();
480+
}
481+
}
482+
435483
async setCwd(cwd: string): Promise<void> {
436484
if (this.cwd === cwd) return;
437485
if (this.initPromise) await this.initPromise;

packages/agent/src/server/agent-server.test.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,14 +1522,16 @@ describe("AgentServer HTTP Mode", () => {
15221522
request: claudePosthogExecPermissionRequest(
15231523
"call notebooks-destroy {}",
15241524
),
1525+
expectedKinds: ["allow_once", "allow_always", "reject_once"],
15251526
},
15261527
{
15271528
adapter: "Codex",
15281529
request: codexPosthogExecPermissionRequest("call notebooks-destroy {}"),
1530+
expectedKinds: ["allow_once", "reject_once"],
15291531
},
15301532
])(
1531-
"relays a configured PostHog exec match from $adapter without an always-allow option",
1532-
async ({ request }) => {
1533+
"relays a configured PostHog exec match from $adapter with adapter-specific choices",
1534+
async ({ request, expectedKinds }) => {
15331535
const testServer = exposeCloudClient(createServer());
15341536
testServer.session = null;
15351537
testServer.eventStreamSender = null;
@@ -1542,13 +1544,11 @@ describe("AgentServer HTTP Mode", () => {
15421544
const { requestPermission } = testServer.createCloudClient(basePayload);
15431545
const result = await requestPermission(request);
15441546

1545-
expect(relaySpy).toHaveBeenCalledWith(
1546-
expect.objectContaining({
1547-
options: [
1548-
expect.objectContaining({ kind: "allow_once" }),
1549-
expect.objectContaining({ kind: "reject_once" }),
1550-
],
1551-
}),
1547+
const relayed = relaySpy.mock.calls[0]?.[0] as {
1548+
options: Array<{ kind: string }>;
1549+
};
1550+
expect(relayed.options.map((option) => option.kind)).toEqual(
1551+
expectedKinds,
15521552
);
15531553
expect(result.outcome).toEqual({
15541554
outcome: "selected",

packages/agent/src/server/agent-server.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3735,17 +3735,22 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}
37353735
const posthogExecSubTool =
37363736
this.matchesPostHogExecPermissionRequest(params);
37373737
if (mode !== "background" && posthogExecSubTool) {
3738-
const promptOnceParams = {
3738+
const isClaudeCodeRequest = Boolean(
3739+
params.toolCall?._meta?.claudeCode,
3740+
);
3741+
const relayParams = {
37393742
...params,
3740-
options: params.options.filter(
3741-
(option) => option.kind !== "allow_always",
3742-
),
3743+
options: isClaudeCodeRequest
3744+
? params.options
3745+
: params.options.filter(
3746+
(option) => option.kind !== "allow_always",
3747+
),
37433748
};
37443749
this.logger.debug("Relaying configured PostHog exec permission", {
37453750
subTool: posthogExecSubTool,
37463751
sessionPermissionMode: this.getSessionPermissionMode(),
37473752
});
3748-
return this.relayPermissionToClient(promptOnceParams);
3753+
return this.relayPermissionToClient(relayParams);
37493754
}
37503755

37513756
// Relay permission requests to the connected client when:

0 commit comments

Comments
 (0)