feat(studio): approve MCP tools whose definition changed

A server whose tool definitions differ from what the user approved has those
tools withheld from the model, and the kernel already had an approval route
for them, but the settings row never said so and offered no way to approve.
The row now carries the withheld tools with how each differs, and a button
that sends back the digest of exactly that set, then reconnects.
This commit is contained in:
yhh committed 2026-10-12 07:42:27 -07:00
1 parent 47056a93ee
commit 470fcfea8b
13 files changed
+246 -2

No files matched your search

+1
View File
@@ -166,6 +166,7 @@ export const ACTIONS: UIAction[] = [
{ id: "skill.enabled", kind: "kernel-mutation", target: "entity", proof: "authority-effect" },
{ id: "mcp.enabled", kind: "kernel-mutation", target: "entity", proof: "interaction" },
{ id: "mcp.retry", kind: "repeatable", target: "entity", proof: "interaction" },
{ id: "mcp.trust", kind: "kernel-mutation", target: "entity", proof: "interaction" },
{ id: "mcp.remove", kind: "destructive", target: "entity", proof: "interaction" },
{ id: "mcp.load", kind: "kernel-mutation", target: "entity", proof: "interaction" },
+7
View File
@@ -549,6 +549,13 @@ export const EN: Record<string, string> = {
"忘记": "Forget",
"重连": "Reconnect",
"立即连接": "Connect now",
"定义有改动": "Definition changed",
"无法核对": "Cannot be checked",
"等待你批准的工具": "Tools waiting for your approval",
"这个服务有 {n} 个工具的定义与你批准过的不一致,已暂时不交给模型:": "{n} of this service's tools differ from what you approved and are withheld from the model for now:",
"还有 {n} 个没有列出,批准时会一并生效。": "{n} more are not listed; approving covers them too.",
"批准并重新连接": "Approve and reconnect",
"批准中…": "Approving…",
"记的是 {had},但它答的是 {got}。": "Recorded as {had}, but it answers as {got}.",
// 收工回执
"没有未经验证的部分": "nothing left unverified",
+14
View File
@@ -28,6 +28,20 @@ export interface McpEntry {
launch?: string;
// Why a pending server waits for the user, as the kernel's code.
pendingReason?: string;
// Tools this connection withheld from the model because their definition is
// not the one the user approved. heldDigest names exactly that set, and
// approving it hands the digest back so what is approved is what was shown.
// heldMore counts withheld tools past the list's cap.
held?: McpHeld[];
heldDigest?: string;
heldMore?: number;
}
// One withheld tool and how it differs from what was approved: added, changed,
// or unverifiable when the approval record could not be read.
export interface McpHeld {
name: string;
class: string;
}
// always puts a server's tools in every request; deferred reaches them through
@@ -107,6 +107,16 @@ export class MockMcp extends MockProvider {
];
}
async trustMcp(name: string, digest: string) {
const s = this.servers.find((x) => x.name === name);
if (!s) return { state: "failed", error: "no configured MCP server named " + name };
if (!s.held?.length || s.heldDigest !== digest) return { state: "failed", error: "the held MCP tool definitions changed since they were shown" };
s.held = undefined;
s.heldDigest = undefined;
s.tools = s.tools + 1;
return { state: "ready", tools: s.tools };
}
async reconnectMcp(name: string) {
const s = this.servers.find((x) => x.name === name);
if (!s) return { state: "failed", error: "no configured MCP server named " + name };
+3
View File
@@ -300,6 +300,9 @@ export interface AgentPort {
// Retries a failed or disconnected server and answers with its new state, so
// the caller never has to race a follow-up GET against the connect.
reconnectMcp(name: string): Promise<{ state: string; tools?: number; error?: string }>;
// Approves the withheld definitions the digest names, then reconnects. The
// kernel refuses a digest that no longer matches what it withholds.
trustMcp(name: string, digest: string): Promise<{ state: string; tools?: number; error?: string }>;
setMcpEnabled(name: string, enabled: boolean, scope?: ScopeLayer, root?: string): Promise<void>;
clearMcpOverride(name: string, root?: string): Promise<void>;
setMcpLoad(name: string, load: McpLoad): Promise<void>;
+11
View File
@@ -138,6 +138,17 @@ export class SsePort extends SseWorkspace implements AgentPort {
return this.post("/memory/restore", { name, revision });
}
async trustMcp(name: string, digest: string) {
const res = await fetch(this.base + "/mcp/trust", {
method: "POST",
headers: { "content-type": "application/json" },
credentials: "same-origin",
body: JSON.stringify({ name, digest }),
});
const body = (await res.json().catch(() => ({}))) as { state?: string; tools?: number | unknown[]; error?: string };
if (!res.ok && !body.error) throw new Error(`/mcp/trust: ${res.status}`);
return { state: body.state ?? (res.ok ? "ready" : "failed"), tools: typeof body.tools === "number" ? body.tools : body.tools?.length, error: body.error };
}
async reconnectMcp(name: string) {
const res = await fetch(this.base + "/mcp/reconnect", {
method: "POST",
+11
View File
@@ -21,6 +21,17 @@ export class SseMcp extends SseProvider {
// whole point of the button, so it is read rather than thrown away.
// 502 carries the diagnosis in the body — the reason the retry failed is the
// whole point of the button, so it is read rather than thrown away.
async trustMcp(name: string, digest: string) {
const res = await fetch(this.base + "/mcp/trust", {
method: "POST",
headers: { "content-type": "application/json" },
credentials: "same-origin",
body: JSON.stringify({ name, digest }),
});
const body = (await res.json().catch(() => ({}))) as { state?: string; tools?: number | unknown[]; error?: string };
if (!res.ok && !body.error) throw new Error(`/mcp/trust: ${res.status}`);
return { state: body.state ?? (res.ok ? "ready" : "failed"), tools: typeof body.tools === "number" ? body.tools : body.tools?.length, error: body.error };
}
async reconnectMcp(name: string) {
const res = await fetch(this.base + "/mcp/reconnect", {
method: "POST",
@@ -0,0 +1,6 @@
.srv-held { margin: 6px 0 0 15px; padding: 8px 10px; border: 1px solid var(--border); border-radius: var(--r-md); background: var(--overlay); font: var(--w-reg) var(--fs-ui)/1.55 var(--ui); color: var(--text); display: grid; gap: 6px; justify-items: start }
.srv-held ul { margin: 0; padding: 0; list-style: none; display: grid; gap: 2px; max-width: 100% }
.srv-held li { display: flex; gap: 8px; align-items: baseline; min-width: 0 }
.srv-held li b { font-weight: var(--w-med); overflow-wrap: anywhere }
.srv-held li i { font-style: normal; color: var(--muted); white-space: nowrap }
.srv-held .hn { color: var(--muted) }
+37 -2
View File
@@ -5,6 +5,7 @@ import { Exception } from "./CapabilityScope";
import { Clip } from "./Clip";
import { Switch } from "./Switch";
import { reason } from "../i18n/kernel";
import "../styles/mcpheld.css";
// One external service: whether it is answering, what it brought, and the two
// acts that change that — switching it off and letting it try again.
@@ -28,6 +29,14 @@ const PENDING_WHY: Record<string, string> = {
changed_since_enabled: "启用之后它要启动的内容变了,或是由旧版本启用的。确认下面的命令后重新启用。",
};
// How a withheld tool differs from what the user approved, keyed by the kernel's
// class. A class with no entry is shown as the kernel wrote it.
const HELD_CLASS: Record<string, string> = {
added: "新增",
changed: "定义有改动",
unverifiable: "无法核对",
};
// A tag only when the schema carries the server's tools or config asks it to:
// deferred is the default, and a badge on every row is a badge on none.
function loadTag(m: McpEntry): { text: string; pending: boolean } | null {
@@ -131,6 +140,30 @@ export function ServerRow({
</div>
);
const heldTools = m.held ?? [];
// The approval is for the exact set the kernel withheld; the digest it sent is
// handed back, and a set that changed meanwhile is refused rather than waved through.
const held = heldTools.length > 0 && !!m.heldDigest && (
<div className="srv-held" role="group" aria-label={t("等待你批准的工具")}>
<span className="ds">
{t("这个服务有 {n} 个工具的定义与你批准过的不一致,已暂时不交给模型:", { n: heldTools.length + (m.heldMore ?? 0) })}
</span>
<ul>
{heldTools.map((h) => (
<li key={h.name}>
<b>{h.name}</b>
<i>{t(HELD_CLASS[h.class] ?? h.class)}</i>
</li>
))}
</ul>
{!!m.heldMore && <span className="hn">{t("还有 {n} 个没有列出,批准时会一并生效。", { n: m.heldMore })}</span>}
{live && (
<button className="act" data-action="mcp.trust" data-target={m.name} disabled={!!busy} onClick={() => void run("trust", () => port.trustMcp(m.name, m.heldDigest!))}>
{t(busy === "trust" ? "批准中…" : "批准并重新连接")}
</button>
)}
</div>
);
const tools = m.toolList ?? [];
const tag = loadTag(m);
// The tool list is what gets loaded, so the choice sits above it; a server
@@ -194,6 +227,7 @@ export function ServerRow({
<div className="srv-hd">{head}</div>
{about}
{launch}
{held}
{why && <div className="why">{why}</div>}
{confirm}
</div>
@@ -201,11 +235,12 @@ export function ServerRow({
}
return (
// Asking to remove has to open the row: the confirmation lives inside the
// fold, and what the server contributes is worth seeing before dropping it.
<details className="srv mcp-server" data-st={m.state} data-local={m.localOverride ? "" : undefined} aria-busy={!!busy} open={confirming || undefined}>
// fold. A row with withheld tools opens too, so the approval is not behind a click.
<details className="srv mcp-server" data-st={m.state} data-local={m.localOverride ? "" : undefined} aria-busy={!!busy} open={confirming || heldTools.length > 0 || undefined}>
<summary>{head}</summary>
{about}
{launch}
{held}
{why && <div className="why">{why}</div>}
{confirm}
{load}
@@ -0,0 +1,67 @@
// @vitest-environment jsdom
import { afterEach, describe, expect, it, vi } from "vitest";
import { cleanup, render, screen, waitFor } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import "./testkit";
import { ServerRow } from "./ServerRow";
import type { AgentPort, McpEntry } from "../port/port";
afterEach(cleanup);
const withheld = (over: Partial<McpEntry> = {}): McpEntry => ({
name: "notes", state: "ready", enabled: true, transport: "stdio", tools: 1,
held: [{ name: "mcp__notes__search", class: "changed" }, { name: "mcp__notes__add", class: "added" }],
heldDigest: "digest-1", ...over,
});
function draw(m: McpEntry, live = true) {
const trustMcp = vi.fn(async () => ({ state: "ready", tools: 2 }));
const onDone = vi.fn();
render(<ServerRow m={m} port={{ trustMcp } as unknown as AgentPort} onDone={onDone} root="/w" live={live} />);
return { trustMcp, onDone };
}
describe("tools a server changed since the user approved them", () => {
it("lists each withheld tool with how it differs", () => {
draw(withheld());
const group = screen.getByRole("group", { name: "等待你批准的工具" });
expect(group.textContent).toContain("mcp__notes__search");
expect(group.textContent).toContain("定义有改动");
expect(group.textContent).toContain("mcp__notes__add");
expect(group.textContent).toContain("2 个工具");
});
it("opens a row that has tools, so the approval is not hidden behind a fold", () => {
draw(withheld({ toolList: [{ name: "add", description: "Add a note" }] }));
expect(document.querySelector("details.mcp-server")?.hasAttribute("open")).toBe(true);
});
it("approves exactly the digest it was shown, then refreshes the list", async () => {
const { trustMcp, onDone } = draw(withheld());
await userEvent.click(screen.getByRole("button", { name: "批准并重新连接" }));
await waitFor(() => expect(onDone).toHaveBeenCalled());
expect(trustMcp).toHaveBeenCalledTimes(1);
expect(trustMcp).toHaveBeenCalledWith("notes", "digest-1");
});
it("shows the kernel's refusal instead of claiming success when the set changed meanwhile", async () => {
const trustMcp = vi.fn(async () => ({ state: "failed", error: "the held MCP tool definitions changed since they were shown" }));
render(<ServerRow m={withheld()} port={{ trustMcp } as unknown as AgentPort} onDone={() => {}} root="/w" live />);
await userEvent.click(screen.getByRole("button", { name: "批准并重新连接" }));
expect(await screen.findByText(/changed since they were shown/)).toBeTruthy();
});
it("says how many more are withheld than listed", () => {
draw(withheld({ heldMore: 7 }));
expect(screen.getByRole("group", { name: "等待你批准的工具" }).textContent).toContain("9 个工具");
expect(screen.getByText(/还有 7 个没有列出/)).toBeTruthy();
});
it("offers no approval from a row the person cannot edit, and nothing when none is withheld", () => {
draw(withheld(), false);
expect(screen.queryByRole("button", { name: "批准并重新连接" })).toBeNull();
cleanup();
draw(withheld({ held: undefined, heldDigest: undefined }));
expect(screen.queryByRole("group", { name: "等待你批准的工具" })).toBeNull();
});
});
+30
View File
@@ -262,8 +262,25 @@ type mcpEntry struct {
Launch string `json:"launch,omitempty"`
// PendingReason says why a pending server waits for the user, as a code.
PendingReason string `json:"pendingReason,omitempty"`
// Held lists tools withheld from the model for a definition the user has not
// approved. HeldDigest names that whole set and comes back with the approval.
// HeldMore counts the withheld tools past the list's cap.
Held []mcpHeld `json:"held,omitempty"`
HeldDigest string `json:"heldDigest,omitempty"`
HeldMore int `json:"heldMore,omitempty"`
}
// mcpHeld is one withheld tool: its visible name and how it differs from what
// was approved (added, changed, or unverifiable when the record cannot be read).
type mcpHeld struct {
Name string `json:"name"`
Class string `json:"class"`
}
// mcpHeldLimit bounds how many withheld tools one row lists; the rest are only
// counted, so a server that withholds thousands cannot make the page heavy.
const mcpHeldLimit = 50
// mcpTool is one tool as the server describes it, plus the two hints that
// decide how a call is classified. Error carries a schema the host rejected:
// the tool is listed but not callable, which is a different thing from absent.
@@ -356,12 +373,25 @@ func (s *Server) mcp(w http.ResponseWriter, r *http.Request) {
}
if configured {
row.Launch = launchText(st.Entry)
row.Held, row.HeldDigest, row.HeldMore = mcpHeldViews(st.Held)
}
out = append(out, row)
}
writeJSON(w, map[string]any{"servers": out, "scope": scopeView(ctl)})
}
func mcpHeldViews(held []plugin.HeldTool) ([]mcpHeld, string, int) {
if len(held) == 0 {
return nil, "", 0
}
shown := held[:min(len(held), mcpHeldLimit)]
out := make([]mcpHeld, len(shown))
for i, h := range shown {
out[i] = mcpHeld{Name: displayText(h.VisibleName, mcpServerTextLimit), Class: string(h.Class)}
}
return out, plugin.HeldDigest(held), len(held) - len(shown)
}
// remembered fills a row that has no live connection to ask with what the last
// successful handshake recorded. It is marked as remembered rather than merged
// silently: a row that cannot tell the two apart claims a server that is off or
+48
View File
@@ -0,0 +1,48 @@
package serve
import (
"fmt"
"strings"
"testing"
"reasonix/internal/contract/tool"
"reasonix/internal/ext/plugin"
)
func TestMCPHeldViewsNameTheSetTheDigestCovers(t *testing.T) {
held := []plugin.HeldTool{
{RawName: "a", VisibleName: "mcp__s__a", Class: tool.HeldMCPAdded, Digest: "d1"},
{RawName: "b", VisibleName: "mcp__s__b", Class: tool.HeldMCPChanged, Digest: "d2", Approved: "d0"},
}
views, digest, more := mcpHeldViews(held)
if len(views) != 2 || more != 0 || views[0].Class != "added" || views[1].Class != "changed" {
t.Fatalf("views = %+v more = %d", views, more)
}
if digest != plugin.HeldDigest(held) {
t.Fatalf("digest %q is not the digest of the held set", digest)
}
}
func TestMCPHeldViewsBoundTheRowButNotTheDigest(t *testing.T) {
var held []plugin.HeldTool
for i := range mcpHeldLimit + 7 {
held = append(held, plugin.HeldTool{RawName: fmt.Sprint("t", i), VisibleName: fmt.Sprint("t", i), Class: tool.HeldMCPAdded, Digest: "d"})
}
views, digest, more := mcpHeldViews(held)
if len(views) != mcpHeldLimit || more != 7 {
t.Fatalf("listed %d, more %d", len(views), more)
}
if digest != plugin.HeldDigest(held) {
t.Fatal("the digest must cover every withheld tool, including the ones past the cap")
}
}
func TestMCPHeldViewsSanitiseServerWrittenNames(t *testing.T) {
views, _, _ := mcpHeldViews([]plugin.HeldTool{{RawName: "x", VisibleName: "evil\x1b[31m" + strings.Repeat("n", mcpServerTextLimit*2), Class: tool.HeldMCPChanged, Digest: "d"}})
if strings.ContainsRune(views[0].Name, 0x1b) || len([]rune(views[0].Name)) > mcpServerTextLimit+1 {
t.Fatalf("name was not sanitised and bounded: %q", views[0].Name)
}
if v, d, m := mcpHeldViews(nil); v != nil || d != "" || m != 0 {
t.Fatal("no held tools must produce no row fields")
}
}
+1
View File
@@ -89,6 +89,7 @@ var mirroredWireTypes = []wireMirror{
// The MCP row: a status the host answered with and the page cannot read is a
// row that goes back to reading the server's own prose for it.
{"internal/frontend/serve/catalog.go", "mcpEntry", tsMcpFile, "McpEntry"},
{"internal/frontend/serve/catalog.go", "mcpHeld", tsMcpFile, "McpHeld"},
// The fold bounds and the one in force: a field the panel cannot read is a
// threshold a user sets and never sees applied.
{"internal/session/control/compaction_settings.go", "CompactionSettings", tsModelFile, "CompactionSettings"},