Skip to content

Commit bda8157

Browse files
authored
feat(quarantine): render input + output schema diff in tool approval UI (#639)
The tool-quarantine approval UI flagged a tool as "changed" whenever its description, input schema, OR output schema hash differed from the approved version, but rendered only the description diff. A schema-only change was therefore an invisible phantom diff — the operator saw a "changed" badge with no visible reason. Render up to three labeled before/after sections (Description, Input Schema, Output Schema), each shown only when that field actually changed, consuming the previous_output_schema / current_output_schema fields the diff endpoint now exposes (PR #638). Schema bodies are pretty-printed before the word-diff so additive changes (e.g. a new enum value) read clearly. Backend keeps rug-pull detection strict; this change is transparency only. Diff-section selection is extracted into computeToolDiffSections (utils/toolDiff) with a vitest covering the output-schema-only case. Related MCP-2096, MCP-2085
1 parent ac954e5 commit bda8157

4 files changed

Lines changed: 223 additions & 20 deletions

File tree

frontend/src/types/api.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,11 @@ export interface ToolApproval {
302302
current_description?: string
303303
previous_schema?: string
304304
current_schema?: string
305+
// Output schema diff fields (MCP-2096 / PR #638): exposed by
306+
// GET /api/v1/servers/{id}/tools/{tool}/diff so the approval UI can render
307+
// an Output-Schema diff section, not just description/input-schema.
308+
previous_output_schema?: string
309+
current_output_schema?: string
305310
enabled?: boolean
306311
disabled?: boolean
307312
}

frontend/src/utils/toolDiff.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// MCP-2096 (MCP-2085): build the list of diff sections shown in the
2+
// tool-quarantine approval UI. A tool is flagged `changed` whenever its
3+
// description OR input schema OR output schema hash differs from the approved
4+
// version, but the UI historically rendered only the description diff — so a
5+
// schema-only change read as a phantom false positive. This helper selects the
6+
// fields that actually changed and produces readable before/after bodies (JSON
7+
// schemas are pretty-printed) so the operator can see WHAT changed before
8+
// approving.
9+
10+
import type { ToolApproval } from '@/types'
11+
12+
export interface ToolDiffSection {
13+
/** Stable key for v-for + tests. */
14+
key: 'description' | 'input_schema' | 'output_schema'
15+
/** Human label for the section header. */
16+
label: string
17+
/** Short explanation of why this section matters (e.g. additive schema). */
18+
hint?: string
19+
/** Approved (previous) body, normalized for display. */
20+
before: string
21+
/** Current body, normalized for display. */
22+
after: string
23+
}
24+
25+
// Pretty-print a JSON schema string with 2-space indent so the word-diff is
26+
// line-oriented and readable. Falls back to the raw string when the body isn't
27+
// valid JSON (defensive — the backend stores schemas as JSON strings).
28+
function formatSchema(raw: string | undefined): string {
29+
if (!raw) return ''
30+
try {
31+
return JSON.stringify(JSON.parse(raw), null, 2)
32+
} catch {
33+
return raw
34+
}
35+
}
36+
37+
export function computeToolDiffSections(tool: ToolApproval): ToolDiffSection[] {
38+
const sections: ToolDiffSection[] = []
39+
40+
// 1. Description — plain text, no formatting.
41+
const prevDesc = tool.previous_description ?? ''
42+
const curDesc = tool.current_description ?? tool.description ?? ''
43+
if (prevDesc !== curDesc) {
44+
sections.push({
45+
key: 'description',
46+
label: 'Description changed',
47+
before: prevDesc,
48+
after: curDesc,
49+
})
50+
}
51+
52+
// 2. Input schema — the tool's argument contract.
53+
const prevSchema = formatSchema(tool.previous_schema)
54+
const curSchema = formatSchema(tool.current_schema)
55+
if (prevSchema !== curSchema) {
56+
sections.push({
57+
key: 'input_schema',
58+
label: 'Input schema changed',
59+
hint: 'The arguments this tool accepts changed.',
60+
before: prevSchema,
61+
after: curSchema,
62+
})
63+
}
64+
65+
// 3. Output schema — the shape of what the tool returns.
66+
const prevOut = formatSchema(tool.previous_output_schema)
67+
const curOut = formatSchema(tool.current_output_schema)
68+
if (prevOut !== curOut) {
69+
sections.push({
70+
key: 'output_schema',
71+
label: 'Output schema changed',
72+
hint: 'The shape of this tool’s results changed (e.g. a new enum value or field).',
73+
before: prevOut,
74+
after: curOut,
75+
})
76+
}
77+
78+
return sections
79+
}

frontend/src/views/ServerDetail.vue

Lines changed: 46 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -372,27 +372,45 @@
372372
</span>
373373
</div>
374374
<p
375-
v-if="tool.status !== 'changed' || !tool.previous_description"
375+
v-if="tool.status !== 'changed' || computeToolDiffSections(tool).length === 0"
376376
class="text-sm text-base-content/70 mt-1"
377377
>{{ tool.description }}</p>
378-
<!-- Show before/after diff for changed tools -->
379-
<div v-if="tool.status === 'changed' && tool.previous_description" class="mt-2 space-y-2 text-xs">
380-
<div>
381-
<div class="text-[10px] font-semibold uppercase tracking-wide text-base-content/60 mb-1">Before (approved)</div>
382-
<div class="bg-error/5 border border-error/20 px-2 py-1.5 rounded font-mono leading-relaxed">
383-
<template v-for="(part, i) in computeWordDiff(tool.previous_description, tool.current_description || tool.description)" :key="'b'+i">
384-
<span v-if="part.type === 'removed'" class="bg-error/20 text-error font-semibold px-0.5 rounded">{{ part.text }}</span>
385-
<span v-else-if="part.type === 'same'">{{ part.text }}</span>
386-
</template>
387-
</div>
388-
</div>
389-
<div>
390-
<div class="text-[10px] font-semibold uppercase tracking-wide text-base-content/60 mb-1">After (current)</div>
391-
<div class="bg-success/5 border border-success/20 px-2 py-1.5 rounded font-mono leading-relaxed">
392-
<template v-for="(part, i) in computeWordDiff(tool.previous_description, tool.current_description || tool.description)" :key="'a'+i">
393-
<span v-if="part.type === 'added'" class="bg-success/20 text-success font-semibold px-0.5 rounded">{{ part.text }}</span>
394-
<span v-else-if="part.type === 'same'">{{ part.text }}</span>
395-
</template>
378+
<!-- Per-field before/after diff for changed tools: a tool is
379+
flagged "changed" when its description, input schema, OR
380+
output schema differs from the approved version (MCP-2096).
381+
Render one section per field that actually changed so a
382+
schema-only change isn't an invisible phantom diff. -->
383+
<div
384+
v-if="tool.status === 'changed' && computeToolDiffSections(tool).length > 0"
385+
class="mt-2 space-y-3 text-xs"
386+
data-test="tool-diff"
387+
>
388+
<div
389+
v-for="section in computeToolDiffSections(tool)"
390+
:key="section.key"
391+
:data-test="'tool-diff-' + section.key"
392+
>
393+
<div class="text-[11px] font-semibold text-base-content/80 mb-0.5">{{ section.label }}</div>
394+
<div v-if="section.hint" class="text-[10px] text-base-content/50 mb-1">{{ section.hint }}</div>
395+
<div class="space-y-2">
396+
<div>
397+
<div class="text-[10px] font-semibold uppercase tracking-wide text-base-content/60 mb-1">Before (approved)</div>
398+
<div class="bg-error/5 border border-error/20 px-2 py-1.5 rounded font-mono leading-relaxed whitespace-pre-wrap">
399+
<template v-for="(part, i) in computeWordDiff(section.before, section.after)" :key="'b'+i">
400+
<span v-if="part.type === 'removed'" class="bg-error/20 text-error font-semibold px-0.5 rounded">{{ part.text }}</span>
401+
<span v-else-if="part.type === 'same'">{{ part.text }}</span>
402+
</template>
403+
</div>
404+
</div>
405+
<div>
406+
<div class="text-[10px] font-semibold uppercase tracking-wide text-base-content/60 mb-1">After (current)</div>
407+
<div class="bg-success/5 border border-success/20 px-2 py-1.5 rounded font-mono leading-relaxed whitespace-pre-wrap">
408+
<template v-for="(part, i) in computeWordDiff(section.before, section.after)" :key="'a'+i">
409+
<span v-if="part.type === 'added'" class="bg-success/20 text-success font-semibold px-0.5 rounded">{{ part.text }}</span>
410+
<span v-else-if="part.type === 'same'">{{ part.text }}</span>
411+
</template>
412+
</div>
413+
</div>
396414
</div>
397415
</div>
398416
</div>
@@ -1207,6 +1225,7 @@ import api from '@/services/api'
12071225
import { useSecurityScannerStatus } from '@/composables/useSecurityScannerStatus'
12081226
import { serverDisplayName } from '@/utils/serverRoute'
12091227
import { oauthSignInState } from '@/utils/health'
1228+
import { computeToolDiffSections } from '@/utils/toolDiff'
12101229
12111230
interface Props {
12121231
// MCP-1112: vue-router decodes the percent-encoded ':serverName' param, so
@@ -1747,7 +1766,10 @@ async function _loadToolApprovalsWithGen(gen: number) {
17471766
}
17481767
})
17491768
1750-
// Fetch diffs for changed tools to populate previous_description
1769+
// Fetch diffs for changed tools to populate the before/after fields used
1770+
// by computeToolDiffSections: description, input schema, AND output schema
1771+
// (MCP-2096 — output schema added in PR #638). Rendering only one field
1772+
// made schema-only changes look like phantom "changed" false positives.
17511773
const changedTools = approvals.filter(t => t.status === 'changed')
17521774
if (changedTools.length > 0) {
17531775
const diffPromises = changedTools.map(async (tool) => {
@@ -1756,6 +1778,10 @@ async function _loadToolApprovalsWithGen(gen: number) {
17561778
if (diffResp.success && diffResp.data) {
17571779
tool.previous_description = diffResp.data.previous_description
17581780
tool.current_description = diffResp.data.current_description
1781+
tool.previous_schema = diffResp.data.previous_schema
1782+
tool.current_schema = diffResp.data.current_schema
1783+
tool.previous_output_schema = diffResp.data.previous_output_schema
1784+
tool.current_output_schema = diffResp.data.current_output_schema
17591785
}
17601786
} catch {
17611787
// Diff fetch failed, continue without it
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { describe, it, expect } from 'vitest'
2+
import { computeToolDiffSections } from '@/utils/toolDiff'
3+
import type { ToolApproval } from '@/types'
4+
5+
// MCP-2096 (MCP-2085): the tool-quarantine diff UI previously rendered ONLY
6+
// the description diff. The backend now exposes input + output schema fields
7+
// (previous_schema/current_schema, previous_output_schema/current_output_schema
8+
// — PR #638), so a `changed` tool whose only change is a schema must surface a
9+
// labeled diff section instead of reading as a phantom false positive.
10+
11+
function makeTool(overrides: Partial<ToolApproval>): ToolApproval {
12+
return {
13+
server_name: 'srv',
14+
tool_name: 'do_thing',
15+
status: 'changed',
16+
hash: 'h',
17+
description: '',
18+
...overrides,
19+
}
20+
}
21+
22+
describe('computeToolDiffSections (MCP-2096)', () => {
23+
it('produces a non-empty, labeled Output-Schema section when ONLY the output schema changed', () => {
24+
const tool = makeTool({
25+
description: 'List files',
26+
previous_description: 'List files',
27+
current_description: 'List files',
28+
previous_schema: '{"type":"object"}',
29+
current_schema: '{"type":"object"}',
30+
previous_output_schema: '{"type":"object","properties":{"files":{"type":"array"}}}',
31+
current_output_schema:
32+
'{"type":"object","properties":{"files":{"type":"array"},"mode":{"enum":["r","w"]}}}',
33+
})
34+
35+
const sections = computeToolDiffSections(tool)
36+
37+
// Description + input schema are identical → skipped; only output schema.
38+
expect(sections).toHaveLength(1)
39+
const out = sections[0]
40+
expect(out.key).toBe('output_schema')
41+
expect(out.label.toLowerCase()).toContain('output schema')
42+
expect(out.before).not.toBe(out.after)
43+
expect(out.before.length).toBeGreaterThan(0)
44+
expect(out.after).toContain('mode')
45+
})
46+
47+
it('renders all three sections in order when description + both schemas changed', () => {
48+
const tool = makeTool({
49+
previous_description: 'old desc',
50+
current_description: 'new desc',
51+
previous_schema: '{"a":1}',
52+
current_schema: '{"a":2}',
53+
previous_output_schema: '{"b":1}',
54+
current_output_schema: '{"b":2}',
55+
})
56+
57+
const keys = computeToolDiffSections(tool).map(s => s.key)
58+
expect(keys).toEqual(['description', 'input_schema', 'output_schema'])
59+
})
60+
61+
it('skips sections whose fields are identical (no phantom diffs)', () => {
62+
const tool = makeTool({
63+
previous_description: 'same',
64+
current_description: 'same',
65+
previous_schema: '{"x":1}',
66+
current_schema: '{"x":1}',
67+
})
68+
expect(computeToolDiffSections(tool)).toHaveLength(0)
69+
})
70+
71+
it('pretty-prints JSON schema bodies so the word-diff is readable', () => {
72+
const tool = makeTool({
73+
previous_schema: '{"type":"object","properties":{"id":{"type":"string"}}}',
74+
current_schema: '{"type":"object","properties":{"id":{"type":"number"}}}',
75+
})
76+
const sections = computeToolDiffSections(tool)
77+
expect(sections).toHaveLength(1)
78+
expect(sections[0].key).toBe('input_schema')
79+
// Pretty-printed multi-line output (indented) rather than the raw one-liner.
80+
expect(sections[0].after).toContain('\n')
81+
})
82+
83+
it('falls back to the raw string when a schema body is not valid JSON', () => {
84+
const tool = makeTool({
85+
previous_output_schema: 'not-json-old',
86+
current_output_schema: 'not-json-new',
87+
})
88+
const sections = computeToolDiffSections(tool)
89+
expect(sections).toHaveLength(1)
90+
expect(sections[0].before).toBe('not-json-old')
91+
expect(sections[0].after).toBe('not-json-new')
92+
})
93+
})

0 commit comments

Comments
 (0)