Repository navigation
feat(clients): MCP app approvals say what the tool does instead of printing raw JSON #18105
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.
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import { describe, expect, it } from "vite-plus/test"; | ||
|
|
||
| import { | ||
| countRequestItemTypes, | ||
| formatRequestSize, | ||
| parseEmbeddedJson, | ||
| previewRequestScalar, | ||
| summarizeRequestValue, | ||
| } from "./ToolCallRequest.logic"; | ||
|
|
||
| describe("parseEmbeddedJson", () => { | ||
| it("unpacks a string that holds a JSON object or list", () => { | ||
| expect(parseEmbeddedJson('{\n "type": "excalidraw"\n}')).toEqual({ type: "excalidraw" }); | ||
| expect(parseEmbeddedJson(" [1, 2]")).toEqual([1, 2]); | ||
| }); | ||
|
|
||
| it("leaves other values alone", () => { | ||
| expect(parseEmbeddedJson("hello")).toBe("hello"); | ||
| expect(parseEmbeddedJson("{not json")).toBe("{not json"); | ||
| expect(parseEmbeddedJson("42")).toBe("42"); | ||
| expect(parseEmbeddedJson(42)).toBe(42); | ||
| }); | ||
| }); | ||
|
|
||
| describe("summarizeRequestValue", () => { | ||
| it("names an item by the fields that usually say what it is", () => { | ||
| expect(summarizeRequestValue({ id: "a1", type: "arrow", x: 160 })).toBe("arrow · a1"); | ||
| expect(summarizeRequestValue({ type: "text", text: "Browser", id: "browser-label" })).toBe( | ||
| "text · Browser", | ||
| ); | ||
| }); | ||
|
|
||
| it("counts fields or items when nothing names it", () => { | ||
| expect(summarizeRequestValue({ gridSize: 20, viewBackgroundColor: "#fff" })).toBe("2 fields"); | ||
| expect(summarizeRequestValue({})).toBe("empty"); | ||
| expect(summarizeRequestValue([1])).toBe("1 item"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("countRequestItemTypes", () => { | ||
| it("groups typed items", () => { | ||
| expect( | ||
| countRequestItemTypes([{ type: "text" }, { type: "rectangle" }, { type: "text" }, 3]), | ||
| ).toBe("text ×2, rectangle ×1"); | ||
| }); | ||
|
|
||
| it("says nothing for untyped lists", () => { | ||
| expect(countRequestItemTypes([1, "a", { name: "x" }])).toBeUndefined(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("previewRequestScalar", () => { | ||
| it("cuts long text to one line and notes its length", () => { | ||
| const preview = previewRequestScalar("x".repeat(5000)); | ||
| expect(preview.startsWith("x".repeat(120) + "…")).toBe(true); | ||
| expect(preview).toContain("5,000 characters"); | ||
| }); | ||
|
|
||
| it("shows short values as they are", () => { | ||
| expect(previewRequestScalar("http://localhost:5855")).toBe("http://localhost:5855"); | ||
| expect(previewRequestScalar(null)).toBe("null"); | ||
| expect(previewRequestScalar(false)).toBe("false"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("formatRequestSize", () => { | ||
| it("measures the serialized arguments", () => { | ||
| expect(formatRequestSize({ a: 1 })).toBe("7 B"); | ||
| expect(formatRequestSize({ json: "x".repeat(15_000) })).toBe("14.7 KB"); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| import * as Predicate from "effect/Predicate"; | ||
|
|
||
| // Fields that usually name what an item is, in the order a summary uses them. | ||
| const SUMMARY_FIELDS = ["type", "kind", "name", "title", "text", "label", "id"] as const; | ||
| const PREVIEW_LENGTH = 120; | ||
|
|
||
| /** | ||
| * A string argument that holds JSON, such as a serialized document, reads as | ||
| * the data it carries. Anything else stays as it is. | ||
| */ | ||
| export function parseEmbeddedJson(value: unknown): unknown { | ||
| if (typeof value !== "string") return value; | ||
| const text = value.trim(); | ||
| if (!text.startsWith("{") && !text.startsWith("[")) return value; | ||
| try { | ||
| return JSON.parse(text) as unknown; | ||
| } catch { | ||
| return value; | ||
| } | ||
| } | ||
|
|
||
| /** One line for an object or list: what it is, not everything in it. */ | ||
| export function summarizeRequestValue(value: unknown): string { | ||
| if (Array.isArray(value)) return value.length === 1 ? "1 item" : `${value.length} items`; | ||
| if (!Predicate.isObject(value)) return previewRequestScalar(value); | ||
| const named = SUMMARY_FIELDS.map((field) => value[field]) | ||
| .filter((field): field is string => typeof field === "string" && field.trim().length > 0) | ||
| .slice(0, 2); | ||
| if (named.length > 0) return named.join(" · "); | ||
| const size = Object.keys(value).length; | ||
| if (size === 0) return "empty"; | ||
| return size === 1 ? "1 field" : `${size} fields`; | ||
| } | ||
|
|
||
| /** For a list of typed items, how many of each: "text ×6, rectangle ×5". */ | ||
| export function countRequestItemTypes(items: ReadonlyArray<unknown>): string | undefined { | ||
| const counts = new Map<string, number>(); | ||
| for (const item of items) { | ||
| if (Predicate.isObject(item) && typeof item.type === "string") { | ||
| counts.set(item.type, (counts.get(item.type) ?? 0) + 1); | ||
| } | ||
| } | ||
| if (counts.size === 0) return undefined; | ||
| return [...counts].map(([type, count]) => `${type} ×${count}`).join(", "); | ||
| } | ||
|
|
||
| /** A scalar on one line, with long text cut short and its full length noted. */ | ||
| export function previewRequestScalar(value: unknown): string { | ||
| const text = typeof value === "string" ? value : (JSON.stringify(value) ?? String(value)); | ||
| if (text.length <= PREVIEW_LENGTH) return text; | ||
| return `${text.slice(0, PREVIEW_LENGTH)}… (${text.length.toLocaleString()} characters)`; | ||
| } | ||
|
|
||
| export function formatRequestSize(value: unknown): string { | ||
| const bytes = new TextEncoder().encode(JSON.stringify(value) ?? "").byteLength; | ||
| if (bytes < 1024) return `${bytes} B`; | ||
| return `${(bytes / 1024).toFixed(1)} KB`; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| import { ChevronRightIcon } from "lucide-react"; | ||
| import * as Predicate from "effect/Predicate"; | ||
| import { useState } from "react"; | ||
|
|
||
| import { useCopyToClipboard } from "~/hooks/useCopyToClipboard"; | ||
| import { cn } from "~/lib/utils"; | ||
| import { Button } from "./ui/button"; | ||
| import { Collapsible, CollapsiblePanel, CollapsibleTrigger } from "./ui/collapsible"; | ||
| import { | ||
| countRequestItemTypes, | ||
| formatRequestSize, | ||
| parseEmbeddedJson, | ||
| previewRequestScalar, | ||
| summarizeRequestValue, | ||
| } from "./ToolCallRequest.logic"; | ||
|
|
||
| // Past this many, a list shows the first ones and a count of the rest. | ||
| const LIST_LIMIT = 100; | ||
|
|
||
| /** | ||
| * A tool call's arguments, folded behind "Show request" and laid out as data: | ||
| * each argument labeled from the tool's schema, JSON inside strings unpacked, | ||
| * and every object or list closed until it is opened. | ||
| */ | ||
| export function ToolCallRequest(props: { | ||
| readonly arguments: Readonly<Record<string, unknown>>; | ||
| readonly argumentDescriptions: Readonly<Record<string, string>>; | ||
| }) { | ||
| const { copyToClipboard, isCopied } = useCopyToClipboard(); | ||
| const entries = Object.entries(props.arguments); | ||
| const size = formatRequestSize(props.arguments); | ||
|
|
||
| return ( | ||
| <Collapsible className="text-left"> | ||
| <CollapsibleTrigger className="group/request flex items-center gap-1 text-muted-foreground text-sm hover:text-foreground"> | ||
| <ChevronRightIcon className="size-3.5 transition-transform group-data-panel-open/request:rotate-90" /> | ||
| Show request · {size} | ||
| </CollapsibleTrigger> | ||
| <CollapsiblePanel> | ||
| <div className="mt-2 rounded-lg border bg-muted/72"> | ||
| <div className="max-h-[min(16rem,40dvh)] overflow-y-auto overscroll-contain p-3 text-xs"> | ||
| {entries.length === 0 ? ( | ||
| <p className="text-muted-foreground">No arguments.</p> | ||
| ) : ( | ||
| entries.map(([name, value]) => ( | ||
| <div key={name} className="not-first:mt-3"> | ||
| <div className="flex flex-wrap items-baseline gap-2"> | ||
| <span className="font-mono">{name}</span> | ||
| {props.argumentDescriptions[name] ? ( | ||
| <span className="text-muted-foreground"> | ||
| {props.argumentDescriptions[name]} | ||
| </span> | ||
| ) : null} | ||
| </div> | ||
| <RequestValue value={value} defaultOpen /> | ||
| </div> | ||
| )) | ||
| )} | ||
| </div> | ||
| <div className="flex items-center justify-between gap-2 border-t px-3 py-1.5 text-muted-foreground text-xs"> | ||
| <span> | ||
| {entries.length === 1 ? "1 argument" : `${entries.length} arguments`} · {size} | ||
| </span> | ||
| <Button | ||
| variant="link" | ||
| size="xs" | ||
| onClick={() => copyToClipboard(JSON.stringify(props.arguments, null, 2))} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: rg -n 'useCopyToClipboard|copyToClipboard' apps/web/src/components/ToolCallRequest.tsx apps/web/src
sed -n '1,80p' apps/web/src/components/ToolCallRequest.tsxRepository: pingdotgg/t3code Length of output: 16024 🏁 Script executed: set -o pipefail
printf '%s\n' '--- useCopyToClipboard ---'
nl -ba apps/web/src/hooks/useCopyToClipboard.ts | sed -n '1,285p'
printf '%s\n' '--- toast API and onError/onCopy patterns ---'
rg -n -F --glob 'apps/web/src/**/*.{ts,tsx}' 'useToast|toast(' apps/web/src | head -160
rg -n -F --glob 'apps/web/src/**/*.{ts,tsx}' 'onError:' apps/web/src | head -120
printf '%s\n' '--- ToolCallRequest callers ---'
rg -n -F --glob 'apps/web/src/**/*.{ts,tsx}' '<ToolCallRequest' apps/web/src || test "$?" -eq 1
printf '%s\n' '--- relevant toast implementation ---'
nl -ba apps/web/src/components/ui/toast.tsx | sed -n '1,180p'Repository: pingdotgg/t3code Length of output: 20941 🏁 Script executed: set -o pipefail
printf '%s\n' '--- copy error helpers and representative handlers ---'
rg -n -F --glob 'apps/web/src/**/*.{ts,tsx}' 'showAnchoredCopyErrorToast|failureToast|show.*Copy.*Toast|toastManager.add|useToast' apps/web/src
nl -ba apps/web/src/components/ChatMarkdown.tsx | sed -n '1415,1470p'
nl -ba apps/web/src/components/chat/MessageCopyButton.tsx | sed -n '1,70p'
nl -ba apps/web/src/components/DiffFilePathCopyButton.tsx | sed -n '1,55p'
nl -ba apps/web/src/hooks/useThreadActionMenu.ts | sed -n '90,140p'
printf '%s\n' '--- ConfirmDialogHost ---'
nl -ba apps/web/src/components/ConfirmDialogHost.tsx | sed -n '1,145p'
printf '%s\n' '--- toast exports and provider/context surface ---'
rg -n -F --glob 'apps/web/src/components/ui/toast.tsx' 'export|function|const' apps/web/src/components/ui/toast.tsx | head -100
nl -ba apps/web/src/components/ui/toast.tsx | sed -n '300,520p'Repository: pingdotgg/t3code Length of output: 21327 Show feedback when copying raw JSON fails.
Suggested fix-import { useState } from "react";
+import { useRef, useState } from "react";
import { useCopyToClipboard } from "~/hooks/useCopyToClipboard";
import { cn } from "~/lib/utils";
import { Button } from "./ui/button";
import { Collapsible, CollapsiblePanel, CollapsibleTrigger } from "./ui/collapsible";
+import { showAnchoredCopyErrorToast } from "./ui/anchoredCopyToast";
export function ToolCallRequest(props: {
readonly arguments: Readonly<Record<string, unknown>>;
readonly argumentDescriptions: Readonly<Record<string, string>>;
}) {
- const { copyToClipboard, isCopied } = useCopyToClipboard();
+ const ref = useRef<HTMLButtonElement>(null);
+ const { copyToClipboard, isCopied } = useCopyToClipboard({
+ target: "raw JSON",
+ onError: (error) => showAnchoredCopyErrorToast(ref, error),
+ });
const entries = Object.entries(props.arguments);
const size = formatRequestSize(props.arguments);
...
<Button
+ ref={ref}
variant="link"🤖 Prompt for AI Agents |
||
| > | ||
| {isCopied ? "Copied" : "Copy raw JSON"} | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| </CollapsiblePanel> | ||
| </Collapsible> | ||
| ); | ||
| } | ||
|
|
||
| function RequestValue(props: { | ||
| readonly name?: string; | ||
| readonly value: unknown; | ||
| readonly defaultOpen?: boolean; | ||
| }) { | ||
| const value = parseEmbeddedJson(props.value); | ||
| const unpacked = value !== props.value; | ||
| const [open, setOpen] = useState(props.defaultOpen === true); | ||
|
|
||
| if (!Array.isArray(value) && !Predicate.isObject(value)) { | ||
| return ( | ||
| <div className="flex min-w-0 items-baseline gap-2 py-0.5 pl-4"> | ||
| {props.name === undefined ? null : ( | ||
| <span className="shrink-0 font-mono text-muted-foreground">{props.name}</span> | ||
| )} | ||
| <span className="min-w-0 truncate">{previewRequestScalar(value)}</span> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| const items = Array.isArray(value) ? value : undefined; | ||
| const types = items === undefined ? undefined : countRequestItemTypes(items); | ||
| return ( | ||
| <div className="min-w-0"> | ||
| <button | ||
| type="button" | ||
| aria-expanded={open} | ||
| className="flex w-full min-w-0 cursor-pointer items-baseline gap-2 py-0.5 text-left" | ||
| onClick={() => setOpen((current) => !current)} | ||
| > | ||
| <ChevronRightIcon | ||
| className={cn( | ||
| "size-3 shrink-0 self-center text-muted-foreground transition-transform", | ||
| open && "rotate-90", | ||
| )} | ||
| /> | ||
| {props.name === undefined ? null : ( | ||
| <span className="shrink-0 font-mono text-muted-foreground">{props.name}</span> | ||
| )} | ||
| <span className="min-w-0 truncate text-muted-foreground"> | ||
| {summarizeRequestValue(value)} | ||
| </span> | ||
| {unpacked ? ( | ||
| <span className="shrink-0 rounded border px-1 font-mono text-3xs text-muted-foreground"> | ||
| JSON | ||
| </span> | ||
| ) : null} | ||
| </button> | ||
| {open ? ( | ||
| <div className="pl-4"> | ||
| {types === undefined ? null : <p className="py-0.5 text-muted-foreground">{types}</p>} | ||
| {items === undefined | ||
| ? Object.entries(value).map(([key, child]) => ( | ||
| <RequestValue key={key} name={key} value={child} /> | ||
| )) | ||
| : items.slice(0, LIST_LIMIT).map((child, index) => ( | ||
| // oxlint-disable-next-line react/no-array-index-key -- A request never reorders; items may be identical. | ||
| <RequestValue key={index} value={child} /> | ||
| ))} | ||
| {items !== undefined && items.length > LIST_LIMIT ? ( | ||
| <p className="py-0.5 text-muted-foreground"> | ||
| {(items.length - LIST_LIMIT).toLocaleString()} more | ||
| </p> | ||
| ) : null} | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| ); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: pingdotgg/t3code
Length of output: 11359
Guard the outer dismissal after handing off to the second alert.
When Android opens the second alert, it can dismiss the first alert. The first alert’s
onDismissthen resolves the outer promise withfalsebefore the user answers the second alert. Set a handoff flag and ignore that dismissal.Suggested fix
return new Promise<boolean>((resolve) => { + let handedOff = false; Alert.alert( question, description ?? "", [ { text: "Cancel", style: "cancel", onPress: () => resolve(false) }, { text: "Show request", - onPress: () => void confirm(question, request, "Allow").then(resolve), + onPress: () => { + handedOff = true; + void confirm(question, request, "Allow").then(resolve); + }, }, { text: "Allow", onPress: () => resolve(true) }, ], - { cancelable: true, onDismiss: () => resolve(false) }, + { cancelable: true, onDismiss: () => { if (!handedOff) resolve(false); } }, ); });📝 Committable suggestion
🤖 Prompt for AI Agents