Skip to content

Commit a8fa73a

Browse files
committed
fix(artifacts L5): never replace a shown MCP App consent strip (Codex r7 UKEM)
A newer consent request replaced the shown strip in place, so a view could swap the payload between the user's pointerdown and click and the same Allow/Open/Add button approved the new request. While a strip is shown, newer requests are now declined, and the buttons settle only the request that is displayed (moved down from L9; the confirm arming delay stays in L9). --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high -->
1 parent 6969e26 commit a8fa73a

2 files changed

Lines changed: 59 additions & 19 deletions

File tree

‎src/browser/features/RightSidebar/ArtifactsTab/McpAppFrame.test.tsx‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,12 @@ const VIEW: McpAppViewRef = {
2727

2828
let invocation: McpAppView["invocation"] = null;
2929
let getViewCalls = 0;
30-
let toolCalls: Array<{ serverName: string; toolName: string; consented: boolean }> = [];
30+
let toolCalls: Array<{
31+
serverName: string;
32+
toolName: string;
33+
arguments: unknown;
34+
consented: boolean;
35+
}> = [];
3136

3237
function Wrapper(props: { children: ReactNode }) {
3338
const api: TestApiOverrides<APIClient> = {
@@ -46,10 +51,16 @@ function Wrapper(props: { children: ReactNode }) {
4651
},
4752
});
4853
},
49-
callTool: (input: { serverName: string; toolName: string; consented: boolean }) => {
54+
callTool: (input: {
55+
serverName: string;
56+
toolName: string;
57+
arguments: unknown;
58+
consented: boolean;
59+
}) => {
5060
toolCalls.push({
5161
serverName: input.serverName,
5262
toolName: input.toolName,
63+
arguments: input.arguments,
5364
consented: input.consented,
5465
});
5566
return Promise.resolve({
@@ -172,6 +183,29 @@ describe("McpAppFrame", () => {
172183
expect(args).toContain('"city": "Berlin"');
173184
});
174185

186+
test("a request sent while a strip is shown cannot replace it under the user's press", async () => {
187+
const { view, frame, posted } = await renderFrame();
188+
const call = (id: number, amount: number) =>
189+
postFromView(frame, {
190+
jsonrpc: "2.0",
191+
id,
192+
method: "tools/call",
193+
params: { name: "transfer", arguments: { amount } },
194+
});
195+
call(1, 1);
196+
await view.findByRole("alert");
197+
const allow = view.getByRole("button", { name: "Allow" });
198+
fireEvent.pointerDown(allow);
199+
// The view swaps in another request between the user's pointerdown and click.
200+
call(2, 9999);
201+
await waitFor(() => expect(posted.find((m) => m.id === 2)?.error).toBeDefined());
202+
expect(view.getByTestId("mcp-app-consent-args").textContent).toContain('"amount": 1');
203+
fireEvent.click(allow);
204+
await waitFor(() => expect(posted.find((m) => m.id === 1)?.result).toBeDefined());
205+
// Only the request the user saw was dispatched with consent.
206+
expect(toolCalls.filter((c) => c.consented).map((c) => c.arguments)).toEqual([{ amount: 1 }]);
207+
});
208+
175209
test("a view message reaches the composer only after Add on the full text", async () => {
176210
const inserted: unknown[] = [];
177211
const onInsert = (event: Event) => inserted.push((event as CustomEvent).detail);

‎src/browser/features/RightSidebar/ArtifactsTab/McpAppFrame.tsx‎

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,8 @@ function DesktopMcpAppFrame(props: { workspaceId: string; view: McpAppViewRef })
135135
const theme: "light" | "dark" = isLightThemeMode(themeMode) ? "light" : "dark";
136136
const [loaded, setLoaded] = useState<Loaded | null>(null);
137137
const [consent, setConsent] = useState<PendingConsent | null>(null);
138+
// Set synchronously with `consent`, so a request arriving before the re-render sees the strip.
139+
const consentRef = useRef<PendingConsent | null>(null);
138140
const [height, setHeight] = useState<number | null>(null);
139141
const [allowMessage] = useState(() => createBridgeRateLimiter());
140142
const frameRef = useRef<HTMLIFrameElement | null>(null);
@@ -228,11 +230,15 @@ function DesktopMcpAppFrame(props: { workspaceId: string; view: McpAppViewRef })
228230
},
229231
requestConsent: (request) =>
230232
new Promise<boolean>((resolve) => {
231-
setConsent((previous) => {
232-
// One strip at a time: a newer request replaces (and denies) an older one.
233-
previous?.resolve(false);
234-
return { request, resolve };
235-
});
233+
// One strip at a time, and a shown strip is never replaced: a frame could otherwise
234+
// swap it between the user's pointerdown and click. Newer requests are declined.
235+
if (consentRef.current != null) {
236+
resolve(false);
237+
return;
238+
}
239+
const pending = { request, resolve };
240+
consentRef.current = pending;
241+
setConsent(pending);
236242
}),
237243
insertIntoComposer: (text) =>
238244
window.dispatchEvent(
@@ -266,10 +272,9 @@ function DesktopMcpAppFrame(props: { workspaceId: string; view: McpAppViewRef })
266272
// Best effort when the view is switched away; Close waits for the reply instead.
267273
void host.teardown("unmount");
268274
hostRef.current = null;
269-
setConsent((previous) => {
270-
previous?.resolve(false);
271-
return null;
272-
});
275+
consentRef.current?.resolve(false);
276+
consentRef.current = null;
277+
setConsent(null);
273278
};
274279
// The host lives as long as the document; `grant` is derived from the same inputs.
275280
// eslint-disable-next-line react-hooks/exhaustive-deps
@@ -280,6 +285,13 @@ function DesktopMcpAppFrame(props: { workspaceId: string; view: McpAppViewRef })
280285
hostRef.current?.sendHostContextChanged({ theme, styles: { variables: readStyleVariables() } });
281286
}, [theme]);
282287

288+
const settleConsent = (pending: PendingConsent, allowed: boolean) => {
289+
if (consentRef.current !== pending) return;
290+
consentRef.current = null;
291+
setConsent(null);
292+
pending.resolve(allowed);
293+
};
294+
283295
const close = async () => {
284296
await hostRef.current?.teardown("closed");
285297
closeMcpAppView(workspaceId, toolCallId);
@@ -325,20 +337,14 @@ function DesktopMcpAppFrame(props: { workspaceId: string; view: McpAppViewRef })
325337
</div>
326338
<button
327339
type="button"
328-
onClick={() => {
329-
consent.resolve(true);
330-
setConsent(null);
331-
}}
340+
onClick={() => settleConsent(consent, true)}
332341
className="bg-accent text-background rounded px-2 py-0.5"
333342
>
334343
{CONSENT_ACCEPT_LABEL[consent.request.kind]}
335344
</button>
336345
<button
337346
type="button"
338-
onClick={() => {
339-
consent.resolve(false);
340-
setConsent(null);
341-
}}
347+
onClick={() => settleConsent(consent, false)}
342348
className="border-border-light rounded border px-2 py-0.5"
343349
>
344350
{consent.request.kind === "tool" ? "Deny" : "Cancel"}

0 commit comments

Comments
 (0)