Skip to content

Commit 0f5b9f8

Browse files
committed
fix(ui): MetaToolApprovalDialog crash on freeform approval diff
The dialog assumed a rigid `{ before, after, added, removed }` diff, but the backend's `ApprovalPayload.diff` is freeform JSON — `mcpmux_create_feature_set` sends `{ added_tools }`, so `diff.after.length` threw "Cannot read properties of undefined (reading 'length')" the moment the dialog rendered (i.e. whenever auto-approve was off). Read the diff defensively: coerce each field via `toStringArray`, union `added` + `added_tools`, and only show before/after counts when those keys exist. Any write tool's diff shape now renders without crashing. Signed-off-by: Mohammod Al Amin Ashik <maa.ashik00@gmail.com>
1 parent 13a0fef commit 0f5b9f8

1 file changed

Lines changed: 34 additions & 25 deletions

File tree

apps/desktop/src/features/metaTools/MetaToolApprovalDialog.tsx

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,25 @@ export interface ApprovalRequest {
1414
payload: {
1515
tool_name: string;
1616
summary: string;
17-
diff: null | {
18-
before: string[];
19-
after: string[];
20-
added: string[];
21-
removed: string[];
22-
};
17+
/**
18+
* Tool-list diff the dialog renders. Freeform by design — the backend's
19+
* `ApprovalPayload.diff` is an arbitrary JSON value and each write tool
20+
* sends a different shape (`mcpmux_create_feature_set` sends
21+
* `{ added_tools }`; others may send `{ before, after, added, removed }`).
22+
* Read it defensively (see `toStringArray`); never assume a field exists.
23+
*/
24+
diff: null | Record<string, unknown>;
2325
raw_args: unknown;
2426
affects_other_clients: boolean;
2527
};
2628
expires_at_unix_secs: number;
2729
}
2830

31+
/** Coerce a freeform JSON value into a `string[]`, dropping non-strings. */
32+
function toStringArray(v: unknown): string[] {
33+
return Array.isArray(v) ? v.filter((x): x is string => typeof x === 'string') : [];
34+
}
35+
2936
type Decision = 'allow_once' | 'always_for_this_session_and_client' | 'deny';
3037

3138
/**
@@ -73,14 +80,20 @@ export function MetaToolApprovalDialog() {
7380
[current]
7481
);
7582

76-
const diff = current?.payload.diff;
77-
const toolCount = diff?.after.length ?? null;
78-
const deltaLabel = useMemo(() => {
79-
if (!diff) return null;
80-
const added = diff.added.length;
81-
const removed = diff.removed.length;
82-
return `+${added} / -${removed}`;
83-
}, [diff]);
83+
// Normalize the freeform diff defensively — a missing field must never
84+
// throw (this previously crashed on `mcpmux_create_feature_set`, whose diff
85+
// is `{ added_tools }` and has no `after`).
86+
const rawDiff = current?.payload.diff ?? null;
87+
const added = useMemo(
88+
() => [...toStringArray(rawDiff?.added), ...toStringArray(rawDiff?.added_tools)],
89+
[rawDiff]
90+
);
91+
const removed = useMemo(() => toStringArray(rawDiff?.removed), [rawDiff]);
92+
const hasBeforeAfter = rawDiff != null && ('before' in rawDiff || 'after' in rawDiff);
93+
const beforeCount = toStringArray(rawDiff?.before).length;
94+
const afterCount = hasBeforeAfter ? toStringArray(rawDiff?.after).length : added.length;
95+
const hasDiff = rawDiff != null && (added.length > 0 || removed.length > 0 || hasBeforeAfter);
96+
const deltaLabel = `+${added.length} / -${removed.length}`;
8497

8598
if (!current) return null;
8699

@@ -118,28 +131,24 @@ export function MetaToolApprovalDialog() {
118131
</div>
119132
)}
120133

121-
{diff && (
134+
{hasDiff && (
122135
<div className="border border-[rgb(var(--border-subtle))] rounded text-xs">
123136
<div className="grid grid-cols-3 divide-x divide-[rgb(var(--border-subtle))] bg-[rgb(var(--surface))]">
124-
<Stat label="Before" value={diff.before.length} />
125-
<Stat
126-
label="After"
127-
value={toolCount ?? 0}
128-
emphasis
129-
/>
130-
<Stat label="Delta" value={deltaLabel ?? '—'} />
137+
<Stat label="Before" value={hasBeforeAfter ? beforeCount : '—'} />
138+
<Stat label="After" value={afterCount} emphasis />
139+
<Stat label="Delta" value={deltaLabel} />
131140
</div>
132-
{(diff.added.length > 0 || diff.removed.length > 0) && (
141+
{(added.length > 0 || removed.length > 0) && (
133142
<div className="max-h-40 overflow-y-auto p-2 space-y-0.5 font-mono">
134-
{diff.added.map((t) => (
143+
{added.map((t) => (
135144
<div
136145
key={`+${t}`}
137146
className="text-green-600 dark:text-green-400"
138147
>
139148
+ {t}
140149
</div>
141150
))}
142-
{diff.removed.map((t) => (
151+
{removed.map((t) => (
143152
<div
144153
key={`-${t}`}
145154
className="text-red-600 dark:text-red-400"

0 commit comments

Comments
 (0)