Skip to content

fix(agent): show server name and exec context in MCP permission dialog #2339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MukundaKatta wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(agent): show server name and exec context in MCP permission dialog #2339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,35 @@ describe("canUseTool MCP approval enforcement", () => {
expect(context.client.requestPermission).toHaveBeenCalled();
});

it("tags MCP tools in the default permission flow with claudeCode.toolName so the renderer can show the server name and unwrap exec dispatch args", async () => {
setMcpToolApprovalStates({ mcp__posthog__exec: "approved" });

const context = createContext("mcp__posthog__exec", {
toolInput: { command: "call experiment-get-all {}" },
});
const result = await canUseTool(context);

expect(result.behavior).toBe("allow");
expect(context.client.requestPermission).toHaveBeenCalledWith(
expect.objectContaining({
toolCall: expect.objectContaining({
_meta: { claudeCode: { toolName: "mcp__posthog__exec" } },
}),
}),
);
});

it("does not set claudeCode._meta on non-MCP tools in the default permission flow", async () => {
const context = createContext("WebFetch", {
toolInput: { url: "https://example.com" },
});
await canUseTool(context);

const call = (context.client.requestPermission as ReturnType<typeof vi.fn>)
.mock.calls[0]?.[0];
expect(call?.toolCall?._meta).toBeUndefined();
});
Comment on lines +107 to +134
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Prefer a parameterised test here

The two new tests vary only along one axis — whether the tool name starts with mcp__ — and therefore test the same branching condition in handleDefaultPermissionFlow. The team's simplicity rules ask for it.each in exactly this pattern. This also frees up the second test from accessing .mock.calls directly while the first uses toHaveBeenCalledWith, keeping the assertion style consistent.

Context Used: Do not attempt to comment on incorrect alphabetica... (source)

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts
Line: 107-134

Comment:
**Prefer a parameterised test here**

The two new tests vary only along one axis — whether the tool name starts with `mcp__` — and therefore test the same branching condition in `handleDefaultPermissionFlow`. The team's simplicity rules ask for `it.each` in exactly this pattern. This also frees up the second test from accessing `.mock.calls` directly while the first uses `toHaveBeenCalledWith`, keeping the assertion style consistent.

**Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


it("blocks do_not_use even on read-only MCP tools", async () => {
setMcpToolApprovalStates({
mcp__server__readonly_blocked: "do_not_use",
Expand Down
7 changes: 7 additions & 0 deletions packages/agent/src/adapters/claude/permissions/permission-handlers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,12 @@ async function handleDefaultPermissionFlow(
suggestions,
);

// Tag MCP tool calls so the renderer routes them through McpPermission,
// which knows how to show `serverName - toolName (MCP)` plus the unwrapped
// PostHog exec body. Without this, the dialog falls back to DefaultPermission
// and just shows the bare tool name (e.g. "exec") with no context.
const isMcpTool = toolName.startsWith("mcp__");

const response = await client.requestPermission({
options,
sessionId,
Expand All @@ -374,6 +380,7 @@ async function handleDefaultPermissionFlow(
content: toolInfo.content,
locations: toolInfo.locations,
rawInput: { ...(toolInput as Record<string, unknown>), toolName },
...(isMcpTool ? { _meta: { claudeCode: { toolName } } } : {}),
},
});

Expand Down