From c01cd21b423d893f8caa106fbf24ef2454616fcd Mon Sep 17 00:00:00 2001 From: Marcel Wege Date: Tue, 25 Aug 2026 17:24:39 +0200 Subject: [PATCH 01/11] =?UTF-8?q?test:=20W0c=20schema-fit=20gate=20?= =?UTF-8?q?=E2=80=94=20MCP=20delegation=20stays=20per-server,=20no=20migra?= =?UTF-8?q?tion=20(#860)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-agent capability UIs (#861/#862) build on the schema as it stands. Issue #862's literal "delegation choice per assignment" would require per-(agent, server) delegation storage that does not exist anywhere: `delegation` is a column on `mcp_servers` only (migration 0031), and neither `agent_tool_grants` (0003, unique per (agent_id, mcp_server_id, tool_ref) via 0014) nor `plugin_mcp_grants` (0012) carries a delegation discriminator. Coordinator decision, recorded here: delegation stays PER-SERVER. The assignment UI shows the server's mode read-only, links to the per-server setting, and any change there must be labeled as applying to every agent using that server. No migration is created by this unit — deliberately. Guarded by five static migration-scan tests appended to mcpDelegationBackfillMigration.pg.test.ts (they run even without a test Postgres); the decision is documented on the McpDelegation type in agentGraphStore.ts, next to the code a widening would have to change. --- .../src/registry/agentGraphStore.ts | 14 ++- .../mcpDelegationBackfillMigration.pg.test.ts | 95 ++++++++++++++++++- 2 files changed, 107 insertions(+), 2 deletions(-) diff --git a/middleware/packages/harness-orchestrator/src/registry/agentGraphStore.ts b/middleware/packages/harness-orchestrator/src/registry/agentGraphStore.ts index 9fcca6f4..70027944 100644 --- a/middleware/packages/harness-orchestrator/src/registry/agentGraphStore.ts +++ b/middleware/packages/harness-orchestrator/src/registry/agentGraphStore.ts @@ -223,7 +223,19 @@ export interface McpServerRow { readonly delegation: McpDelegation; } -/** How an MCP server resolves the identity a call acts as (W0-1, D2). */ +/** How an MCP server resolves the identity a call acts as (W0-1, D2). + * + * Scope decision (#860 W0c, resolving #862's "delegation choice per + * assignment"): delegation is a PER-SERVER property and stays one. There is + * deliberately NO per-(agent, server) delegation storage — not on + * `agent_tool_grants`, not on `plugin_mcp_grants`, no bridge table. A + * per-agent assignment view therefore shows the server's mode READ-ONLY and + * links to the per-server setting; changing it there applies to every agent + * holding a grant on that server and must say so. Widening this to + * per-assignment would need a new column/table plus resolution semantics in + * `resolveMcpUserKey` — that is a design decision with its own migration, not + * something a UI unit may introduce as a side effect. Guarded by the W0c + * schema-fit gate in `middleware/test/mcpDelegationBackfillMigration.pg.test.ts`. */ export type McpDelegation = 'per_user' | 'service'; /** diff --git a/middleware/test/mcpDelegationBackfillMigration.pg.test.ts b/middleware/test/mcpDelegationBackfillMigration.pg.test.ts index f34026d9..40ab561d 100644 --- a/middleware/test/mcpDelegationBackfillMigration.pg.test.ts +++ b/middleware/test/mcpDelegationBackfillMigration.pg.test.ts @@ -1,5 +1,5 @@ import { strict as assert } from 'node:assert'; -import { readFile } from 'node:fs/promises'; +import { readFile, readdir } from 'node:fs/promises'; import { after, before, describe, it } from 'node:test'; import { Pool } from 'pg'; @@ -323,3 +323,96 @@ describe('migration 0031 — delegation backfill predicate (pg)', { skip: !pgAva assert.equal(await delegationOf('no-tokens'), 'per_user'); }); }); + +/** + * ─── W0c schema-fit gate (#860) — per-agent grants need NO migration ──────── + * + * Epic #860's per-agent capability UIs (#861 plugins/tool grants, #862 MCP + * assignment) build on the schema AS IT STANDS: + * + * • `agent_tool_grants` (0003) is the per-agent MCP assignment — one row per + * grant, identity `(agent_id, mcp_server_id, tool_ref)` made unique for + * top-level MCP grants by 0014. + * • `plugin_mcp_grants` (0012) is the plugin sibling (plugins have no + * agents-table row — decision recorded on #458). + * • `delegation` lives on `mcp_servers` ONLY (0031). #862's literal wording + * "delegation choice per assignment" would require per-(agent, server) + * storage that does not exist; the coordinator resolved that drift: + * delegation STAYS per-server, the assignment UI displays it read-only and + * links to the per-server setting, whose global effect must be labeled. + * + * These tests pin that decision. If one fails, someone widened delegation to + * per-assignment (or moved the grant tables) — that is a design change with + * its own migration and review, not something a UI wave may do as a side + * effect. Static file scans only, so the gate holds even where no Postgres is + * reachable and the suite above is skipped. + */ +describe('W0c schema-fit gate — delegation stays per-server (#860)', () => { + const MIGRATIONS_DIR = new URL('../migrations/', import.meta.url); + + async function executableMigrations(): Promise> { + const names = (await readdir(MIGRATIONS_DIR)).filter((n) => n.endsWith('.sql')).sort(); + const entries = await Promise.all( + names.map(async (name): Promise => { + const raw = await readFile(new URL(name, MIGRATIONS_DIR), 'utf8'); + return [name, stripWholeLineSqlComments(raw)]; + }), + ); + return new Map(entries); + } + + it('no migration but 0031 touches `delegation` — no per-assignment storage exists', async () => { + const migrations = await executableMigrations(); + const mentioning = [...migrations] + .filter(([, sql]) => /\bdelegation\b/i.test(sql)) + .map(([name]) => name); + assert.deepEqual( + mentioning, + ['0031_mcp_oauth_iss_delegation.sql'], + 'a migration beyond 0031 introduces delegation DDL — the #860 per-server decision was overturned without a design pass', + ); + }); + + it('0031 attaches `delegation` to mcp_servers and never to a grant table', async () => { + const sql = (await executableMigrations()).get('0031_mcp_oauth_iss_delegation.sql'); + assert.ok(sql, 'migration 0031 is missing'); + assert.match(sql, /ALTER TABLE mcp_servers\s+ADD COLUMN delegation TEXT NOT NULL DEFAULT 'per_user'/); + assert.doesNotMatch( + sql, + /agent_tool_grants|plugin_mcp_grants/, + '0031 must not reach into the grant tables — delegation is a server property', + ); + }); + + it('agent_tool_grants (0003) carries the per-agent MCP assignment — and no delegation column', async () => { + const sql = (await executableMigrations()).get('0003_agent_builder_graph.sql'); + assert.ok(sql, 'migration 0003 is missing'); + const createTable = /CREATE TABLE IF NOT EXISTS agent_tool_grants \(([^;]+)\);/.exec(sql)?.[1]; + assert.ok(createTable, '0003 no longer creates agent_tool_grants'); + for (const column of ['agent_id', 'subagent_id', 'tool_kind', 'tool_ref', 'mcp_server_id']) { + assert.match(createTable, new RegExp(`\\b${column}\\b`), `agent_tool_grants lost the ${column} column`); + } + assert.doesNotMatch( + createTable, + /\bdelegation\b/i, + 'agent_tool_grants gained a delegation column — per-assignment delegation needs its own design pass, not this table', + ); + }); + + it('plugin_mcp_grants (0012) is keyed (plugin_id, mcp_server_id) — and no delegation column', async () => { + const sql = (await executableMigrations()).get('0012_plugin_mcp_grants.sql'); + assert.ok(sql, 'migration 0012 is missing'); + assert.match(sql, /PRIMARY KEY \(plugin_id, mcp_server_id\)/); + assert.doesNotMatch(sql, /\bdelegation\b/i); + }); + + it('0014 pins the grant identity the per-agent UI builds on: (agent_id, mcp_server_id, tool_ref)', async () => { + const sql = (await executableMigrations()).get('0014_mcp_grant_unique.sql'); + assert.ok(sql, 'migration 0014 is missing'); + assert.match( + sql, + /ON agent_tool_grants \(agent_id, mcp_server_id, tool_ref\)\s+WHERE agent_id IS NOT NULL AND tool_kind = 'mcp'/, + 'the top-level MCP grant identity changed — #861/#862 assignment semantics must be re-reviewed', + ); + }); +}); From bcde5bf9265d2109b4d24e7fc2cf1ae83b27560e Mon Sep 17 00:00:00 2001 From: Marcel Wege Date: Tue, 25 Aug 2026 17:39:41 +0200 Subject: [PATCH 02/11] feat(orchestrator): per-agent grant read model for the agent detail UI (W0c, #861) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent-scoped SELECT-only reads on AgentGraphStore, no DDL: - listToolGrantsForAgent(agentId): the agent_tool_grants rows of ONE agent, same shape and ordering as listAllToolGrants (agent_tool_grants_agent_idx from migration 0003 covers the WHERE). - listPluginMcpGrantsForPlugins(pluginIds): full plugin_mcp_grants rows for a set of plugin ids in one round-trip; empty input short-circuits without SQL. - ToolGrantRow.grantEpoch: config.verdictEpoch (stamped by bumpMcpGrantEpoch via jsonb_set — there is no epoch column) surfaced as a typed field so the UI never digs through untyped config. Optional so existing hand-built fixtures stay valid; the row mapper always populates it. - PluginMcpGrantRow named type replaces the inline shape of listPluginMcpGrants (structurally identical) and is re-exported through the orchestrator package index for middleware/src. Verified by middleware/test/agentGrantsStore.test.ts (stubbed pool, 9 tests). --- .../harness-orchestrator/src/index.ts | 1 + .../src/registry/agentGraphStore.ts | 87 +++++++-- middleware/test/agentGrantsStore.test.ts | 171 ++++++++++++++++++ 3 files changed, 244 insertions(+), 15 deletions(-) create mode 100644 middleware/test/agentGrantsStore.test.ts diff --git a/middleware/packages/harness-orchestrator/src/index.ts b/middleware/packages/harness-orchestrator/src/index.ts index b2803c2c..b932bd2f 100644 --- a/middleware/packages/harness-orchestrator/src/index.ts +++ b/middleware/packages/harness-orchestrator/src/index.ts @@ -115,6 +115,7 @@ export type { McpToolVerdictAckRow, McpToolVerdictRow, PersonaSkillRow, + PluginMcpGrantRow, ScheduleInput, ScheduleRow, SkillInput, diff --git a/middleware/packages/harness-orchestrator/src/registry/agentGraphStore.ts b/middleware/packages/harness-orchestrator/src/registry/agentGraphStore.ts index 9fcca6f4..321a948f 100644 --- a/middleware/packages/harness-orchestrator/src/registry/agentGraphStore.ts +++ b/middleware/packages/harness-orchestrator/src/registry/agentGraphStore.ts @@ -249,6 +249,23 @@ export interface ToolGrantRow { readonly mcpServerId: string | null; readonly config: Record; readonly createdAt: Date; + /** Grant epoch (W0c, #861): `bumpMcpGrantEpoch` stamps `config.verdictEpoch` + * (a `now()::text` timestamp) into the grant's JSONB — there is no epoch + * column. Surfaced here as a typed field so readers (the agent detail UI) + * never dig through the untyped `config`. `null` until the first bump. + * Optional so hand-built fixtures predating the field stay valid; the row + * mapper always populates it. */ + readonly grantEpoch?: string | null; +} + +/** One `plugin_mcp_grants` row (epic #459 W5, issue #458): the operator's + * explicit plugin → MCP-server grant. Named so the per-agent read model + * (W0c, #861) has a typed row instead of an inline shape. */ +export interface PluginMcpGrantRow { + readonly pluginId: string; + readonly mcpServerId: string; + readonly grantedBy: string; + readonly grantedAt: Date; } export interface ScheduleRow { @@ -576,6 +593,13 @@ interface ToolGrantDbRow { created_at: Date; } +interface PluginMcpGrantDbRow { + plugin_id: string; + mcp_server_id: string; + granted_by: string; + granted_at: Date; +} + interface ScheduleDbRow { id: string; agent_id: string; @@ -800,6 +824,9 @@ function mapMcpServer(r: McpServerDbRow): McpServerRow { } function mapToolGrant(r: ToolGrantDbRow): ToolGrantRow { + // `verdictEpoch` is written by bumpMcpGrantEpoch via jsonb_set; anything + // that is not a string (absent, or a hand-edited config) reads as null. + const epoch = r.config?.['verdictEpoch']; return { id: r.id, agentId: r.agent_id, @@ -809,6 +836,16 @@ function mapToolGrant(r: ToolGrantDbRow): ToolGrantRow { mcpServerId: r.mcp_server_id, config: r.config, createdAt: r.created_at, + grantEpoch: typeof epoch === 'string' ? epoch : null, + }; +} + +function mapPluginMcpGrant(r: PluginMcpGrantDbRow): PluginMcpGrantRow { + return { + pluginId: r.plugin_id, + mcpServerId: r.mcp_server_id, + grantedBy: r.granted_by, + grantedAt: r.granted_at, }; } @@ -1651,21 +1688,28 @@ export class AgentGraphStore { // ── Plugin → MCP server grants (epic #459 W5, issue #458) ─────────────────── - async listPluginMcpGrants(): Promise< - readonly { pluginId: string; mcpServerId: string; grantedBy: string; grantedAt: Date }[] - > { - const { rows } = await this.pool.query<{ - plugin_id: string; - mcp_server_id: string; - granted_by: string; - granted_at: Date; - }>('SELECT * FROM plugin_mcp_grants'); - return rows.map((r) => ({ - pluginId: r.plugin_id, - mcpServerId: r.mcp_server_id, - grantedBy: r.granted_by, - grantedAt: r.granted_at, - })); + async listPluginMcpGrants(): Promise { + const { rows } = await this.pool.query( + 'SELECT * FROM plugin_mcp_grants', + ); + return rows.map(mapPluginMcpGrant); + } + + /** Plugin-scoped read of `plugin_mcp_grants` (W0c, #861): full grant rows + * for a set of plugin ids — one round-trip for an agent detail page that + * shows the MCP grants of every plugin enabled on that agent. SELECT-only + * by construction (no DDL, no writes). */ + async listPluginMcpGrantsForPlugins( + pluginIds: readonly string[], + ): Promise { + if (pluginIds.length === 0) return []; + const { rows } = await this.pool.query( + `SELECT * FROM plugin_mcp_grants + WHERE plugin_id = ANY($1::text[]) + ORDER BY plugin_id, granted_at`, + [[...pluginIds]], + ); + return rows.map(mapPluginMcpGrant); } async listGrantedServerIdsForPlugin(pluginId: string): Promise { @@ -2297,6 +2341,19 @@ export class AgentGraphStore { return rows.map(mapToolGrant); } + /** Agent-scoped read of `agent_tool_grants` (W0c, #861): the grants of ONE + * agent, for the agent detail page. Same row shape as `listAllToolGrants` + * (grant epoch included via `grantEpoch`); uses the + * `agent_tool_grants_agent_idx` index from migration 0003. SELECT-only by + * construction (no DDL, no writes). */ + async listToolGrantsForAgent(agentId: string): Promise { + const { rows } = await this.pool.query( + 'SELECT * FROM agent_tool_grants WHERE agent_id = $1 ORDER BY created_at', + [agentId], + ); + return rows.map(mapToolGrant); + } + async createToolGrant(input: ToolGrantInput): Promise { if (!input.agentId && !input.subAgentId) { throw new ConfigValidationError( diff --git a/middleware/test/agentGrantsStore.test.ts b/middleware/test/agentGrantsStore.test.ts new file mode 100644 index 00000000..3ec9a68a --- /dev/null +++ b/middleware/test/agentGrantsStore.test.ts @@ -0,0 +1,171 @@ +/** + * Per-agent grant read model (W0c, epic #860, issue #861). + * + * The agent detail UI needs an agent-scoped read of `agent_tool_grants` and a + * plugin-scoped read of `plugin_mcp_grants` — SELECT-only additions on + * `AgentGraphStore`, no DDL. The "grant epoch" is NOT a column: + * `bumpMcpGrantEpoch` stamps `config.verdictEpoch` (a `now()::text` + * timestamp) into the grant's JSONB, so the store must surface it as the + * typed `ToolGrantRow.grantEpoch` field instead of making the UI dig through + * untyped config. + * + * Pool is stubbed (same pattern as agentGraphStoreSubAgentModelValidation): + * the contract under test is which SQL is sent with which params, and how + * rows map back — not Postgres itself. + */ + +import assert from 'node:assert/strict'; +import { test } from 'node:test'; + +import type { Pool } from 'pg'; + +import { AgentGraphStore } from '../packages/harness-orchestrator/src/registry/agentGraphStore.js'; + +interface QueryCall { + sql: string; + params: unknown[] | undefined; +} + +/** Capture every `query()` call and replay a canned row set. */ +function fakePool(rows: unknown[] = []): { pool: Pool; calls: QueryCall[] } { + const calls: QueryCall[] = []; + const pool = { + query: async (sql: string, params?: unknown[]) => { + calls.push({ sql, params }); + return { rows }; + }, + } as unknown as Pool; + return { pool, calls }; +} + +const AGENT_ID = '00000000-0000-0000-0000-000000000001'; +const SERVER_ID = '00000000-0000-0000-0000-00000000000f'; + +function toolGrantDbRow(overrides: Record = {}): Record { + return { + id: '00000000-0000-0000-0000-0000000000aa', + agent_id: AGENT_ID, + subagent_id: null, + tool_kind: 'mcp', + tool_ref: 'search', + mcp_server_id: SERVER_ID, + config: {}, + created_at: new Date(0), + ...overrides, + }; +} + +// ── listToolGrantsForAgent ────────────────────────────────────────────────── + +test('listToolGrantsForAgent selects only the given agent, ordered by created_at', async () => { + const { pool, calls } = fakePool([toolGrantDbRow()]); + const store = new AgentGraphStore(pool); + const rows = await store.listToolGrantsForAgent(AGENT_ID); + + assert.equal(calls.length, 1); + const { sql, params } = calls[0]!; + assert.match(sql, /FROM agent_tool_grants/); + assert.match(sql, /WHERE agent_id = \$1/); + assert.match(sql, /ORDER BY created_at/); + assert.deepEqual(params, [AGENT_ID]); + + assert.equal(rows.length, 1); + assert.equal(rows[0]!.agentId, AGENT_ID); + assert.equal(rows[0]!.toolRef, 'search'); + assert.equal(rows[0]!.mcpServerId, SERVER_ID); +}); + +test('listToolGrantsForAgent is SELECT-only — never writes', async () => { + const { pool, calls } = fakePool(); + const store = new AgentGraphStore(pool); + await store.listToolGrantsForAgent(AGENT_ID); + assert.equal(calls.length, 1); + assert.match(calls[0]!.sql.trimStart(), /^SELECT/i, 'statement is a SELECT'); + assert.doesNotMatch(calls[0]!.sql, /\b(INSERT|UPDATE|DELETE FROM|CREATE TABLE|ALTER TABLE)\b/i); +}); + +// ── grant epoch surfacing ─────────────────────────────────────────────────── + +test('grantEpoch surfaces config.verdictEpoch as a typed field', async () => { + const epoch = '2026-08-25 12:00:00.000000+00'; + const { pool } = fakePool([ + toolGrantDbRow({ config: { verdictEpoch: epoch, other: 1 } }), + ]); + const store = new AgentGraphStore(pool); + const [row] = await store.listToolGrantsForAgent(AGENT_ID); + assert.equal(row!.grantEpoch, epoch); + // The raw config stays intact for callers that need the rest of it. + assert.equal(row!.config['other'], 1); +}); + +test('grantEpoch is null before the first bumpMcpGrantEpoch', async () => { + const { pool } = fakePool([toolGrantDbRow({ config: {} })]); + const store = new AgentGraphStore(pool); + const [row] = await store.listToolGrantsForAgent(AGENT_ID); + assert.equal(row!.grantEpoch, null); +}); + +test('grantEpoch rejects a non-string verdictEpoch (hand-edited config) as null', async () => { + const { pool } = fakePool([toolGrantDbRow({ config: { verdictEpoch: 42 } })]); + const store = new AgentGraphStore(pool); + const [row] = await store.listToolGrantsForAgent(AGENT_ID); + assert.equal(row!.grantEpoch, null); +}); + +test('listAllToolGrants carries grantEpoch too (same mapper)', async () => { + const epoch = '2026-08-25 12:34:56.000000+00'; + const { pool } = fakePool([toolGrantDbRow({ config: { verdictEpoch: epoch } })]); + const store = new AgentGraphStore(pool); + const [row] = await store.listAllToolGrants(); + assert.equal(row!.grantEpoch, epoch); +}); + +// ── listPluginMcpGrantsForPlugins ─────────────────────────────────────────── + +function pluginGrantDbRow(pluginId: string): Record { + return { + plugin_id: pluginId, + mcp_server_id: SERVER_ID, + granted_by: 'operator@example.com', + granted_at: new Date(0), + }; +} + +test('listPluginMcpGrantsForPlugins reads full rows for the given plugin set', async () => { + const { pool, calls } = fakePool([ + pluginGrantDbRow('odoo-hr'), + pluginGrantDbRow('teams-channel'), + ]); + const store = new AgentGraphStore(pool); + const rows = await store.listPluginMcpGrantsForPlugins(['odoo-hr', 'teams-channel']); + + assert.equal(calls.length, 1); + const { sql, params } = calls[0]!; + assert.match(sql, /FROM plugin_mcp_grants/); + assert.match(sql, /WHERE plugin_id = ANY\(\$1::text\[\]\)/); + assert.deepEqual(params, [['odoo-hr', 'teams-channel']]); + + assert.deepEqual(rows[0], { + pluginId: 'odoo-hr', + mcpServerId: SERVER_ID, + grantedBy: 'operator@example.com', + grantedAt: new Date(0), + }); +}); + +test('listPluginMcpGrantsForPlugins short-circuits an empty plugin set without SQL', async () => { + const { pool, calls } = fakePool(); + const store = new AgentGraphStore(pool); + const rows = await store.listPluginMcpGrantsForPlugins([]); + assert.deepEqual(rows, []); + assert.equal(calls.length, 0, 'no round-trip for an agent with no plugins'); +}); + +test('listPluginMcpGrants maps rows through the same named shape', async () => { + const { pool } = fakePool([pluginGrantDbRow('odoo-hr')]); + const store = new AgentGraphStore(pool); + const [row] = await store.listPluginMcpGrants(); + assert.equal(row!.pluginId, 'odoo-hr'); + assert.equal(row!.mcpServerId, SERVER_ID); + assert.equal(row!.grantedBy, 'operator@example.com'); +}); From 888d7043a1523ed4f98f6014e478c48c5261978b Mon Sep 17 00:00:00 2001 From: Marcel Wege Date: Tue, 25 Aug 2026 17:51:52 +0200 Subject: [PATCH 03/11] feat: MCP grant allowlist + per-server delegation surfacing on the grant endpoints (#862, epic #860) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend PUT /mcp-grants so the same route carries both the historical single additive grant and an allowlist replace via toolNames[] — the allowlist IS the set of agent_tool_grants rows for the (agent, server) pair, no new storage. Every tool that would be granted still passes the shared fail-closed verdict gate (assertMcpToolAllowed) before any row is written; revokes need no gate. An optional delegation field sets the server's delegation mode at assignment time via the existing per-server setMcpServerDelegation path; the response reports the real scope with delegationScope: 'server' because delegation lives on mcp_servers, not per assignment (a per-assignment mode would need a schema migration, deliberately out of scope for this unit). Grant mutations that change a server's tool surface now refresh the dispatch policy and bump the server's grant epoch before reload — the same recipe the status/ack routes use — so affected agents actually rebuild. DELETE /mcp-grants/:grantId gets the same treatment. GET /mcp-grants rows and mcpNode now surface the per-server delegation mode, and grant rows expose their last verdict-epoch bump (grantEpoch) for the UI. diffMcpToolAllowlist in mcpToolGuard is pure editor arithmetic (deduped, sorted set difference), not a second enforcement path — enforcement stays with mcpGrantPolicy's dispatch guard and hydration filter. --- middleware/src/routes/agentBuilder.ts | 134 +++++++++++++++++++++--- middleware/src/services/mcpToolGuard.ts | 35 +++++++ middleware/test/mcpToolGuard.test.ts | 42 ++++++++ 3 files changed, 194 insertions(+), 17 deletions(-) diff --git a/middleware/src/routes/agentBuilder.ts b/middleware/src/routes/agentBuilder.ts index 56ae5987..d93b48a7 100644 --- a/middleware/src/routes/agentBuilder.ts +++ b/middleware/src/routes/agentBuilder.ts @@ -62,7 +62,7 @@ import { McpRegistryError, type McpRegistryConfig, } from '../services/mcpRegistryClient.js'; -import { scanDiscoveredTools } from '../services/mcpToolGuard.js'; +import { diffMcpToolAllowlist, scanDiscoveredTools } from '../services/mcpToolGuard.js'; import { scanSkillForRisks } from '../services/skillGuard.js'; import { importSkillMarkdown } from '../services/skillImport.js'; import { serializeSkillMarkdown } from '../services/skillLoader.js'; @@ -1828,38 +1828,116 @@ export function createAgentBuilderRouter( } }); - /** Grant one server tool to an orchestrator (top-level agent). Runs the same - * fail-closed verdict gate as the canvas, then reloads so the orchestrator - * picks the tool up on its next turn. */ + /** Grant one server tool to an orchestrator (top-level agent), or — with + * `toolNames: string[]` — replace the agent's whole tool allowlist for that + * server (issue #862). The allowlist IS the set of `agent_tool_grants` rows + * for the (agent, server) pair; there is no separate allowlist storage. + * Every tool that would be granted runs the same fail-closed verdict gate + * as the canvas BEFORE any row is written; one rejection aborts the whole + * edit. Then reloads so the orchestrator picks the change up on its next + * turn. + * + * `delegation` (optional, 'service' | 'per_user') sets the SERVER's + * delegation mode at assignment time. Delegation lives per server + * (`mcp_servers.delegation`, resolved at dispatch by `resolveMcpUserKey`) — + * a true per-assignment mode would need a schema migration, so the response + * reports the actual scope via `delegationScope: 'server'`. */ router.put('/mcp-grants', async (req: Request, res: Response) => { const l = live(res); if (!l) return; try { const agentSlug = String(req.body?.agentSlug ?? '').trim(); const mcpServerId = String(req.body?.mcpServerId ?? ''); - const toolRef = String(req.body?.toolName ?? ''); - if (agentSlug === '' || !isUuid(mcpServerId) || toolRef === '') { + const singleRef = String(req.body?.toolName ?? ''); + const listRefs: unknown = req.body?.toolNames; + const hasSingle = singleRef !== ''; + const hasList = Array.isArray(listRefs); + // Exactly one mode: the historical single additive grant, or the + // allowlist replace. Neither (or both) is a malformed request. + if (agentSlug === '' || !isUuid(mcpServerId) || hasSingle === hasList) { res.status(400).json({ error: 'invalid_grant' }); return; } + if (hasList && listRefs.some((t) => typeof t !== 'string' || t === '')) { + res.status(400).json({ error: 'invalid_grant' }); + return; + } + const rawDelegation: unknown = req.body?.delegation; + const delegation = rawDelegation === undefined ? undefined : parseDelegation(rawDelegation); + if (delegation === null) { + res.status(400).json({ error: 'invalid_delegation' }); + return; + } const agent = (await l.config.listAgents()).find((a) => a.slug === agentSlug); if (!agent) { res.status(404).json({ error: 'orchestrator_not_found', agentSlug }); return; } - const toolName = await assertMcpToolAllowed(l, mcpServerId, toolRef); - // createToolGrant is idempotent for top-level MCP grants (ON CONFLICT via - // migration 0014), so a repeat is a clean no-op; reload picks up a real - // change and is itself a no-op otherwise. - await l.graph.createToolGrant({ - agentId: agent.id, - subAgentId: null, - toolKind: 'mcp', - toolRef: toolName, + const server = (await l.graph.listMcpServers()).find((s) => s.id === mcpServerId); + if (!server) { + res.status(404).json({ error: 'mcp_server_not_found' }); + return; + } + // Gate first, write after: every candidate passes `assertMcpToolAllowed` + // (the shared fail-closed gate) and comes back as its normalized name. + const desiredRefs = hasSingle ? [singleRef] : (listRefs as string[]); + const desired: string[] = []; + for (const ref of desiredRefs) { + desired.push(await assertMcpToolAllowed(l, mcpServerId, ref)); + } + const currentGrants = (await l.graph.listAllToolGrants()).filter( + (g) => + g.toolKind === 'mcp' && + g.mcpServerId === mcpServerId && + g.agentId === agent.id && + g.subAgentId === null, + ); + const grantIdByTool = new Map( + currentGrants.map((g) => [mcpToolNameFromRef(g.toolRef, server.name), g.id]), + ); + const diff = diffMcpToolAllowlist([...grantIdByTool.keys()], desired); + // Single-grant mode stays additive (historical contract); only the + // allowlist mode revokes what fell off the list. + const toRevoke = hasList ? diff.toRevoke : []; + for (const toolName of diff.toGrant) { + // createToolGrant is idempotent for top-level MCP grants (ON CONFLICT + // via migration 0014), so a repeat is a clean no-op. + await l.graph.createToolGrant({ + agentId: agent.id, + subAgentId: null, + toolKind: 'mcp', + toolRef: toolName, + mcpServerId, + }); + } + for (const toolName of toRevoke) { + const grantId = grantIdByTool.get(toolName); + if (grantId) await l.graph.deleteToolGrant(grantId); + } + if (delegation !== undefined && delegation !== server.delegation) { + await l.graph.setMcpServerDelegation(mcpServerId, delegation); + } + if (diff.toGrant.length > 0 || toRevoke.length > 0) { + // Same recipe as server status/ack changes: policy refresh + epoch + // bump so the reload below actually rebuilds the agents whose MCP + // tool surface changed. + await refreshMcpGrantPolicy(l.graph); + await l.graph.bumpMcpGrantEpoch(mcpServerId); + } + await reload(l); + res.json({ + agentSlug, mcpServerId, + ...(hasSingle + ? { toolName: desired[0], granted: true } + : { + toolNames: [...diff.unchanged, ...diff.toGrant].sort(), + granted: diff.toGrant, + revoked: toRevoke, + }), + delegation: delegation ?? server.delegation, + delegationScope: 'server', }); - await reload(l); - res.json({ agentSlug, mcpServerId, toolName, granted: true }); } catch (err) { fail(res, err); } @@ -1887,6 +1965,13 @@ export function createAgentBuilderRouter( return; } await l.graph.deleteToolGrant(grantId); + if (grant.mcpServerId) { + // A revoke changes the holder's tool surface: refresh the dispatch + // policy and bump the server's grant epoch so the reload rebuilds the + // affected agents (a bare reload can miss config-only deltas). + await refreshMcpGrantPolicy(l.graph); + await l.graph.bumpMcpGrantEpoch(grant.mcpServerId); + } await reload(l); res.status(204).end(); } catch (err) { @@ -2286,6 +2371,14 @@ export function createAgentBuilderRouter( subAgentName: sub?.name ?? null, serverId: g.mcpServerId, serverName: server?.name ?? null, + // Delegation is a per-SERVER mode (issue #862): every assignment of + // this server acts under the same identity resolution + // (`resolveMcpUserKey`), so the row surfaces the server's mode. + delegation: server?.delegation ?? null, + // Last verdict-epoch bump of this grant (set by bumpMcpGrantEpoch); + // null until the first bump touches the row. + grantEpoch: + typeof g.config['verdictEpoch'] === 'string' ? g.config['verdictEpoch'] : null, toolName, severity: v?.severity ?? null, notYetScanned: v === undefined, @@ -2311,6 +2404,8 @@ export function createAgentBuilderRouter( subAgentName: b.contract, serverId: b.mcpServerId, serverName: server?.name ?? null, + delegation: server?.delegation ?? null, + grantEpoch: null, toolName: b.toolName, severity: v?.severity ?? null, notYetScanned: v === undefined, @@ -2327,6 +2422,8 @@ export function createAgentBuilderRouter( subAgentName: null, serverId: g.mcpServerId, serverName: serverById.get(g.mcpServerId)?.name ?? null, + delegation: serverById.get(g.mcpServerId)?.delegation ?? null, + grantEpoch: null, toolName: '*', severity: null, notYetScanned: false, @@ -2752,6 +2849,9 @@ export function mcpNode(s: McpServerRow) { status: s.status, lastDiscoveredAt: s.lastDiscoveredAt ? s.lastDiscoveredAt.toISOString() : null, discoveredTools: s.discoveredTools, + /** Per-server identity delegation (W0-1, D2) — the mode every assignment + * of this server acts under, resolved at dispatch by `resolveMcpUserKey`. */ + delegation: s.delegation, source: s.source, registryId: s.registryId, license: s.license, diff --git a/middleware/src/services/mcpToolGuard.ts b/middleware/src/services/mcpToolGuard.ts index 01848b1e..82e4bea7 100644 --- a/middleware/src/services/mcpToolGuard.ts +++ b/middleware/src/services/mcpToolGuard.ts @@ -173,6 +173,41 @@ export function computeMcpToolVerdict( }; } +// ── tool allowlist arithmetic (epic #860 W1, issue #862) ───────────────────── +// An agent's tool allowlist for an MCP server IS its set of `agent_tool_grants` +// rows for that (agent, server) pair — there is deliberately no separate +// allowlist table. This helper only computes the set difference an allowlist +// edit must apply; every name in `toGrant` still has to pass the fail-closed +// verdict gate (`assertMcpToolAllowed` in the grant routes) before a row is +// written. Editor arithmetic, not a second enforcement path — enforcement +// stays with mcpGrantPolicy's dispatch guard and hydration filter. + +export interface McpToolAllowlistDiff { + /** Desired but not yet granted — each must pass the verdict gate first. */ + readonly toGrant: readonly string[]; + /** Granted but no longer desired — revoking needs no gate. */ + readonly toRevoke: readonly string[]; + /** On both sides — untouched by the edit. */ + readonly unchanged: readonly string[]; +} + +/** Deterministic set difference between the currently granted tool names and a + * desired allowlist. Both sides are deduped; each partition is sorted so + * responses and did-anything-change decisions (epoch bump, policy refresh) + * are stable regardless of input order. */ +export function diffMcpToolAllowlist( + currentToolNames: readonly string[], + desiredToolNames: readonly string[], +): McpToolAllowlistDiff { + const current = new Set(currentToolNames); + const desired = new Set(desiredToolNames); + return { + toGrant: [...desired].filter((name) => !current.has(name)).sort(), + toRevoke: [...current].filter((name) => !desired.has(name)).sort(), + unchanged: [...desired].filter((name) => current.has(name)).sort(), + }; +} + /** Scan a whole discovery batch. A scanner crash on one descriptor must not * take down discovery: that tool degrades to `scan_failed` (visible, and the * grant gate treats it as not-ackable-as-clean rather than silently passed). */ diff --git a/middleware/test/mcpToolGuard.test.ts b/middleware/test/mcpToolGuard.test.ts index ea2a6771..7b8c51b3 100644 --- a/middleware/test/mcpToolGuard.test.ts +++ b/middleware/test/mcpToolGuard.test.ts @@ -3,6 +3,7 @@ import { strict as assert } from 'node:assert'; import { computeMcpToolVerdict, + diffMcpToolAllowlist, mcpToolContentHash, scanDiscoveredTools, scanMcpToolForRisks, @@ -136,6 +137,47 @@ describe('computeMcpToolVerdict', () => { }); }); +describe('diffMcpToolAllowlist (issue #862 — allowlist IS the grant rows)', () => { + it('grants everything when nothing is granted yet', () => { + assert.deepEqual(diffMcpToolAllowlist([], ['b', 'a']), { + toGrant: ['a', 'b'], + toRevoke: [], + unchanged: [], + }); + }); + + it('revokes everything when the desired allowlist is empty', () => { + assert.deepEqual(diffMcpToolAllowlist(['a', 'b'], []), { + toGrant: [], + toRevoke: ['a', 'b'], + unchanged: [], + }); + }); + + it('partitions into grant / revoke / unchanged', () => { + assert.deepEqual(diffMcpToolAllowlist(['keep', 'drop'], ['keep', 'add']), { + toGrant: ['add'], + toRevoke: ['drop'], + unchanged: ['keep'], + }); + }); + + it('an identical set is a no-op edit (no writes, no epoch bump)', () => { + const diff = diffMcpToolAllowlist(['a', 'b'], ['b', 'a']); + assert.deepEqual(diff.toGrant, []); + assert.deepEqual(diff.toRevoke, []); + assert.deepEqual(diff.unchanged, ['a', 'b']); + }); + + it('dedupes repeated names and sorts each partition deterministically', () => { + assert.deepEqual(diffMcpToolAllowlist(['x', 'x', 'a'], ['x', 'x', 'c', 'c', 'b']), { + toGrant: ['b', 'c'], + toRevoke: ['a'], + unchanged: ['x'], + }); + }); +}); + describe('scanDiscoveredTools', () => { it('scans a batch and keeps benign and risky verdicts separate', () => { const verdicts = scanDiscoveredTools(SERVER, [ From 2a893acdf678b77f3145e249856b63508f92c75b Mon Sep 17 00:00:00 2001 From: Marcel Wege Date: Tue, 25 Aug 2026 18:08:11 +0200 Subject: [PATCH 04/11] feat: per-agent grants read + single-plugin toggle on the operator agents router (W0c, #861) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/v1/operator/agents/:slug/grants — one read-only response for the agent detail page: the agent's own agent_tool_grants rows (grant epoch surfaced per row via the typed ToolGrantRow.grantEpoch, plus a top-level grant_epoch = latest bump) and the plugin_mcp_grants of every plugin assigned to that agent. Server names joined in for display. The graph store arrives through a new optional late-bound getAgentGraphStore option (wired in index.ts by the wiring unit); the route 503s without it. Grant WRITES stay on the agent-builder router — this extends, not duplicates, the existing /mcp-grants matrix with an agent-scoped read. GET /api/v1/operator/agents/:slug/plugins — per-agent read of the plugin assignment (same row shape as GET /), so the detail page does not filter the full dashboard payload. PATCH /api/v1/operator/agents/:slug/plugins — enable/disable ONE plugin (body { id, enabled }; the id lives in the body because plugin ids contain '/'). Preserves the row's existing config on toggle (upsert would wipe it), keeps the fallback-agent invariant (fallback always runs plugins with the global store config, {}), 404s plugin_not_assigned when disabling a plugin that was never assigned, and reloads the registry on success. No schema changes; store methods already existed. Verified by middleware/test/operatorAgentsRouter.test.ts (26 tests, 9 new). --- middleware/src/routes/operatorAgents.ts | 169 +++++++++++ middleware/test/operatorAgentsRouter.test.ts | 302 ++++++++++++++++++- 2 files changed, 470 insertions(+), 1 deletion(-) diff --git a/middleware/src/routes/operatorAgents.ts b/middleware/src/routes/operatorAgents.ts index 4474db33..fca6d81d 100644 --- a/middleware/src/routes/operatorAgents.ts +++ b/middleware/src/routes/operatorAgents.ts @@ -6,6 +6,7 @@ import { attachAllPlugins, ConfigValidationError, FALLBACK_AGENT_SLUG, + type AgentGraphStore, type ChatSessionStore, type ConfigStore, type OrchestratorRegistry, @@ -56,7 +57,10 @@ interface AgentPluginCatalogEntry { * POST /api/v1/operator/agents create agent * PATCH /api/v1/operator/agents/:slug update agent (name, privacy, status) * DELETE /api/v1/operator/agents/:slug delete agent + * GET /api/v1/operator/agents/:slug/plugins read agent plugin assignment * PUT /api/v1/operator/agents/:slug/plugins replace agent plugin set + * PATCH /api/v1/operator/agents/:slug/plugins enable/disable ONE plugin (body: { id, enabled }) + * GET /api/v1/operator/agents/:slug/grants per-agent tool grants + plugin MCP grants + grant epoch * PUT /api/v1/operator/agents/:slug/bindings replace agent channel bindings * PUT /api/v1/operator/agents/fallback set platform fallback (body: { slug | null }) * POST /api/v1/operator/agents/:slug/drain drain + clear session snapshots @@ -95,6 +99,14 @@ const AgentPluginsSchema = z.object({ ), }); +/** W0c (#861) — single-plugin toggle. The plugin id lives in the BODY, not + * the path: plugin ids contain `/` (`@omadia/odoo`), which an Express path + * segment cannot carry without double-encoding. */ +const AgentPluginToggleSchema = z.object({ + id: z.string().min(1).max(200), + enabled: z.boolean(), +}); + const AgentBindingsSchema = z.object({ bindings: z.array( z.object({ @@ -128,6 +140,11 @@ export interface OperatorAgentsRouterOptions { * in tests that build it with a bare config store. */ readonly getPluginCatalog?: () => PluginCatalog | undefined; readonly getInstalledRegistry?: () => InstalledRegistry | undefined; + /** W0c (#861) — grant read model for the agent detail page. Late-bound like + * the config store; `GET /:slug/grants` 503s when absent (tests / minimal + * mounts, or no DATABASE_URL). Read-only: this router never writes through + * the graph store. */ + readonly getAgentGraphStore?: () => AgentGraphStore | undefined; } export function createOperatorAgentsRouter( @@ -355,6 +372,158 @@ export function createOperatorAgentsRouter( } }); + // ── read plugin assignment (W0c, #861) ────────────────────────────── + // Per-agent read so the agent detail page can render the assignment + // without filtering the full GET / dashboard payload. Same row shape as + // the `plugins` array on GET /. + router.get('/:slug/plugins', async (req: Request, res: Response) => { + const live = svc(); + if (!live) return unavailable(res); + try { + const slug = slugParam(req, res); + if (!slug) return; + const existing = await live.store.getAgentBySlug(slug); + if (!existing) { + res.status(404).json({ error: 'not_found' }); + return; + } + const [settings, plugins] = await Promise.all([ + live.store.getPlatformSettings(), + live.store.listAgentPlugins(existing.id), + ]); + res.json({ + slug: existing.slug, + fallback: settings.fallbackAgentId === existing.id, + plugins: plugins.map((p) => ({ + id: p.pluginId, + config: p.config, + enabled: p.enabled, + })), + }); + } catch (err) { + badRequest(res, err); + } + }); + + // ── toggle ONE plugin (W0c, #861) ─────────────────────────────────── + // Single-flag flip so the UI does not have to PUT the whole replace-set + // just to enable/disable one plugin (a stale PUT would silently drop + // assignments made in another tab). Body: { id, enabled }. + // + // row exists → upsert with the CURRENT config, new enabled flag + // missing + enabled → assign (empty config, like a fresh multi-select add) + // missing + disabled → 404 plugin_not_assigned (nothing to disable) + router.patch('/:slug/plugins', async (req: Request, res: Response) => { + const live = svc(); + if (!live) return unavailable(res); + try { + const body = AgentPluginToggleSchema.parse(req.body); + const slug = slugParam(req, res); + if (!slug) return; + const existing = await live.store.getAgentBySlug(slug); + if (!existing) { + res.status(404).json({ error: 'not_found' }); + return; + } + // Phase B contract (same invariant as PUT /:slug/plugins): the fallback + // Agent always runs plugins with the global store config — per-Agent + // config overrides are reserved for named Agents. + const settings = await live.store.getPlatformSettings(); + const isFallback = settings.fallbackAgentId === existing.id; + const current = await live.store.listAgentPlugins(existing.id); + const row = current.find((p) => p.pluginId === body.id); + if (!row && !body.enabled) { + res.status(404).json({ error: 'plugin_not_assigned' }); + return; + } + // upsertAgentPlugin overwrites config on conflict — pass the existing + // config through so a toggle never wipes a per-Agent configuration. + const config = isFallback ? {} : (row?.config ?? {}); + await live.store.upsertAgentPlugin(existing.id, { + pluginId: body.id, + config, + enabled: body.enabled, + }); + await live.registry.reload(); + res.json({ + ok: true, + fallback: isFallback, + plugin: { id: body.id, enabled: body.enabled }, + }); + } catch (err) { + badRequest(res, err); + } + }); + + // ── per-agent grant read model (W0c, #861) ────────────────────────── + // One response for the agent detail page: the agent's own + // `agent_tool_grants` rows (grant epoch included — bumpMcpGrantEpoch + // stamps config.verdictEpoch, surfaced as `grant_epoch`) plus the + // `plugin_mcp_grants` of every plugin assigned to the agent. Read-only; + // grant WRITES stay on the agent-builder router (/api/v1/operator/mcp-grants). + router.get('/:slug/grants', async (req: Request, res: Response) => { + const live = svc(); + if (!live) return unavailable(res); + const graph = options.getAgentGraphStore?.(); + if (!graph) { + res.status(503).json({ + error: 'agent_graph_store_unavailable', + message: + 'agentGraphStore not wired into the operator-agents router.', + }); + return; + } + try { + const slug = slugParam(req, res); + if (!slug) return; + const existing = await live.store.getAgentBySlug(slug); + if (!existing) { + res.status(404).json({ error: 'not_found' }); + return; + } + const [toolGrants, plugins, servers] = await Promise.all([ + graph.listToolGrantsForAgent(existing.id), + live.store.listAgentPlugins(existing.id), + graph.listMcpServers(), + ]); + const pluginGrants = await graph.listPluginMcpGrantsForPlugins( + plugins.map((p) => p.pluginId), + ); + const serverById = new Map(servers.map((s) => [s.id, s])); + // Latest verdict-epoch bump across the agent's grants. Epochs are + // `now()::text` timestamps — lexicographic max IS the latest. + const epochs = toolGrants + .map((g) => g.grantEpoch) + .filter((e): e is string => typeof e === 'string'); + res.json({ + slug: existing.slug, + grant_epoch: + epochs.length > 0 ? epochs.reduce((a, b) => (a > b ? a : b)) : null, + tool_grants: toolGrants.map((g) => ({ + id: g.id, + tool_kind: g.toolKind, + tool_ref: g.toolRef, + sub_agent_id: g.subAgentId, + mcp_server_id: g.mcpServerId, + server_name: g.mcpServerId + ? (serverById.get(g.mcpServerId)?.name ?? null) + : null, + grant_epoch: g.grantEpoch ?? null, + created_at: g.createdAt, + })), + plugin_mcp_grants: pluginGrants.map((g) => ({ + plugin_id: g.pluginId, + mcp_server_id: g.mcpServerId, + server_name: serverById.get(g.mcpServerId)?.name ?? null, + granted_by: g.grantedBy, + granted_at: g.grantedAt, + })), + }); + } catch (err) { + badRequest(res, err); + } + }); + // ── replace plugins ───────────────────────────────────────────────── router.put('/:slug/plugins', async (req: Request, res: Response) => { const live = svc(); diff --git a/middleware/test/operatorAgentsRouter.test.ts b/middleware/test/operatorAgentsRouter.test.ts index f2630b6d..24dbca31 100644 --- a/middleware/test/operatorAgentsRouter.test.ts +++ b/middleware/test/operatorAgentsRouter.test.ts @@ -12,6 +12,11 @@ * 6. ConfigValidationError surfaces as HTTP 409. * 7. Zod errors surface as HTTP 400 with a structured `issues` array. * 8. 503 when no orchestratorRegistry is published. + * 9. W0c (#861): GET /:slug/plugins reads the assignment; PATCH + * /:slug/plugins flips ONE plugin (config preserved, fallback keeps the + * global config, unassigned+disable → 404); GET /:slug/grants returns + * the agent's tool grants (grant epoch included) + the plugin MCP + * grants of its assigned plugins, and 503s without a graph store. */ import { strict as assert } from 'node:assert'; @@ -23,6 +28,7 @@ import express from 'express'; import { ConfigValidationError, + type AgentGraphStore, type ChatSessionStore, type ConfigStore, type OrchestratorRegistry, @@ -54,8 +60,12 @@ interface BindingMem { createdAt: Date; } +let idCounter = 0; function newId(): string { - return `00000000-0000-0000-0000-${String(Date.now() % 1e12).padStart(12, '0')}`; + // Monotonic, not time-based: two agents created in the same millisecond + // must still get distinct ids (the W0c grants test creates two). + idCounter += 1; + return `00000000-0000-0000-0000-${String(idCounter).padStart(12, '0')}`; } /** @@ -185,6 +195,44 @@ class FakeConfigStore { } } +interface ToolGrantMem { + id: string; + agentId: string | null; + subAgentId: string | null; + toolKind: string; + toolRef: string; + mcpServerId: string | null; + config: Record; + createdAt: Date; + grantEpoch: string | null; +} +interface PluginMcpGrantMem { + pluginId: string; + mcpServerId: string; + grantedBy: string; + grantedAt: Date; +} + +/** Fake AgentGraphStore — only the three reads GET /:slug/grants uses. */ +class FakeGraphStore { + toolGrants: ToolGrantMem[] = []; + pluginGrants: PluginMcpGrantMem[] = []; + servers: Array<{ id: string; name: string }> = []; + + listToolGrantsForAgent(agentId: string): Promise { + return Promise.resolve(this.toolGrants.filter((g) => g.agentId === agentId)); + } + listPluginMcpGrantsForPlugins( + pluginIds: readonly string[], + ): Promise { + const wanted = new Set(pluginIds); + return Promise.resolve(this.pluginGrants.filter((g) => wanted.has(g.pluginId))); + } + listMcpServers(): Promise> { + return Promise.resolve(this.servers); + } +} + class FakeRegistry { reloadCalls = 0; invalidateCalls: Array<{ slug: string; mode: 'drain' | 'kill' }> = []; @@ -210,11 +258,13 @@ describe('createOperatorAgentsRouter', () => { let baseUrl: string; let store: FakeConfigStore; let registry: FakeRegistry; + let graph: FakeGraphStore; let sessionStore: { list: () => Promise }; before(async () => { store = new FakeConfigStore(); registry = new FakeRegistry(); + graph = new FakeGraphStore(); sessionStore = { list: () => Promise.resolve([]) }; const app = express(); app.use(express.json()); @@ -224,6 +274,7 @@ describe('createOperatorAgentsRouter', () => { getConfigStore: () => store as unknown as ConfigStore, getRegistry: () => registry as unknown as OrchestratorRegistry, getChatSessionStore: () => sessionStore as unknown as ChatSessionStore, + getAgentGraphStore: () => graph as unknown as AgentGraphStore, }), ); server = await listenLoopback(app); @@ -237,6 +288,7 @@ describe('createOperatorAgentsRouter', () => { afterEach(() => { store = new FakeConfigStore(); + graph = new FakeGraphStore(); registry.reloadCalls = 0; registry.invalidateCalls = []; }); @@ -369,6 +421,254 @@ describe('createOperatorAgentsRouter', () => { assert.deepEqual(ids, ['@omadia/a', '@omadia/b']); }); + it('GET /:slug/plugins reads the assignment (W0c #861)', async () => { + const agent = await store.createAgent({ slug: 'public', name: 'Public' }); + await store.upsertAgentPlugin(agent.id, { + pluginId: '@omadia/odoo', + config: { url: 'https://odoo.example' }, + enabled: true, + }); + await store.upsertAgentPlugin(agent.id, { + pluginId: '@omadia/confluence', + enabled: false, + }); + const res = await fetch(`${baseUrl}/public/plugins`); + assert.equal(res.status, 200); + const body = (await res.json()) as { + slug: string; + fallback: boolean; + plugins: Array<{ id: string; config: Record; enabled: boolean }>; + }; + assert.equal(body.slug, 'public'); + assert.equal(body.fallback, false); + const byId = new Map(body.plugins.map((p) => [p.id, p])); + assert.equal(byId.size, 2); + assert.deepEqual(byId.get('@omadia/odoo')?.config, { url: 'https://odoo.example' }); + assert.equal(byId.get('@omadia/odoo')?.enabled, true); + assert.equal(byId.get('@omadia/confluence')?.enabled, false); + }); + + it('GET /:slug/plugins 404s for an unknown agent', async () => { + const res = await fetch(`${baseUrl}/ghost/plugins`); + assert.equal(res.status, 404); + assert.equal(((await res.json()) as { error: string }).error, 'not_found'); + }); + + it('PATCH /:slug/plugins disables ONE plugin, preserves its config, reloads', async () => { + const agent = await store.createAgent({ slug: 'public', name: 'Public' }); + await store.upsertAgentPlugin(agent.id, { + pluginId: '@omadia/odoo', + config: { url: 'https://odoo.example' }, + enabled: true, + }); + const res = await fetch(`${baseUrl}/public/plugins`, { + method: 'PATCH', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ id: '@omadia/odoo', enabled: false }), + }); + assert.equal(res.status, 200); + const body = (await res.json()) as { + ok: boolean; + plugin: { id: string; enabled: boolean }; + }; + assert.equal(body.ok, true); + assert.deepEqual(body.plugin, { id: '@omadia/odoo', enabled: false }); + const rows = await store.listAgentPlugins(agent.id); + assert.equal(rows.length, 1); + assert.equal(rows[0]!.enabled, false); + assert.deepEqual( + rows[0]!.config, + { url: 'https://odoo.example' }, + 'toggle must not wipe the per-agent config', + ); + assert.equal(registry.reloadCalls, 1); + }); + + it('PATCH /:slug/plugins with enabled=true assigns a not-yet-assigned plugin', async () => { + const agent = await store.createAgent({ slug: 'public', name: 'Public' }); + const res = await fetch(`${baseUrl}/public/plugins`, { + method: 'PATCH', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ id: '@omadia/fresh', enabled: true }), + }); + assert.equal(res.status, 200); + const rows = await store.listAgentPlugins(agent.id); + assert.deepEqual( + rows.map((p) => ({ id: p.pluginId, enabled: p.enabled })), + [{ id: '@omadia/fresh', enabled: true }], + ); + }); + + it('PATCH /:slug/plugins with enabled=false on an unassigned plugin → 404 plugin_not_assigned', async () => { + await store.createAgent({ slug: 'public', name: 'Public' }); + const res = await fetch(`${baseUrl}/public/plugins`, { + method: 'PATCH', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ id: '@omadia/ghost', enabled: false }), + }); + assert.equal(res.status, 404); + assert.equal( + ((await res.json()) as { error: string }).error, + 'plugin_not_assigned', + ); + assert.equal(registry.reloadCalls, 0, 'no reload on a rejected toggle'); + }); + + it('PATCH /:slug/plugins keeps the fallback agent on the global config ({})', async () => { + const agent = await store.createAgent({ slug: 'special', name: 'Special' }); + await store.setFallbackAgentId(agent.id); + await store.upsertAgentPlugin(agent.id, { + pluginId: '@omadia/odoo', + config: { smuggled: true }, + enabled: true, + }); + const res = await fetch(`${baseUrl}/special/plugins`, { + method: 'PATCH', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ id: '@omadia/odoo', enabled: false }), + }); + assert.equal(res.status, 200); + assert.equal(((await res.json()) as { fallback: boolean }).fallback, true); + const rows = await store.listAgentPlugins(agent.id); + assert.deepEqual( + rows[0]!.config, + {}, + 'fallback Agent always runs plugins with the global store config', + ); + }); + + it('GET /:slug/grants returns tool grants + plugin MCP grants + grant epoch (W0c #861)', async () => { + const agent = await store.createAgent({ slug: 'public', name: 'Public' }); + const other = await store.createAgent({ slug: 'other', name: 'Other' }); + await store.upsertAgentPlugin(agent.id, { pluginId: '@omadia/odoo' }); + graph.servers = [{ id: 'srv-1', name: 'odoo-mcp' }]; + graph.toolGrants = [ + { + id: 'g-1', + agentId: agent.id, + subAgentId: null, + toolKind: 'mcp', + toolRef: 'mcp:odoo-mcp:search', + mcpServerId: 'srv-1', + config: {}, + createdAt: new Date('2026-08-01T00:00:00Z'), + grantEpoch: '2026-08-20 10:00:00+00', + }, + { + id: 'g-2', + agentId: agent.id, + subAgentId: null, + toolKind: 'builtin', + toolRef: 'web_search', + mcpServerId: null, + config: {}, + createdAt: new Date('2026-08-02T00:00:00Z'), + grantEpoch: '2026-08-21 09:30:00+00', + }, + { + id: 'g-3', + agentId: other.id, + subAgentId: null, + toolKind: 'mcp', + toolRef: 'mcp:odoo-mcp:write', + mcpServerId: 'srv-1', + config: {}, + createdAt: new Date('2026-08-03T00:00:00Z'), + grantEpoch: null, + }, + ]; + graph.pluginGrants = [ + { + pluginId: '@omadia/odoo', + mcpServerId: 'srv-1', + grantedBy: 'operator', + grantedAt: new Date('2026-08-10T00:00:00Z'), + }, + { + pluginId: '@omadia/unassigned', + mcpServerId: 'srv-1', + grantedBy: 'operator', + grantedAt: new Date('2026-08-11T00:00:00Z'), + }, + ]; + const res = await fetch(`${baseUrl}/public/grants`); + assert.equal(res.status, 200); + const body = (await res.json()) as { + slug: string; + grant_epoch: string | null; + tool_grants: Array<{ + id: string; + server_name: string | null; + grant_epoch: string | null; + }>; + plugin_mcp_grants: Array<{ plugin_id: string; server_name: string | null }>; + }; + assert.equal(body.slug, 'public'); + assert.equal(body.grant_epoch, '2026-08-21 09:30:00+00', 'latest bump wins'); + assert.deepEqual( + body.tool_grants.map((g) => g.id), + ['g-1', 'g-2'], + 'only the agent\'s own grants', + ); + assert.equal(body.tool_grants[0]!.server_name, 'odoo-mcp'); + assert.equal(body.tool_grants[0]!.grant_epoch, '2026-08-20 10:00:00+00'); + assert.deepEqual( + body.plugin_mcp_grants.map((g) => g.plugin_id), + ['@omadia/odoo'], + 'plugin grants scoped to the plugins assigned to THIS agent', + ); + assert.equal(body.plugin_mcp_grants[0]!.server_name, 'odoo-mcp'); + }); + + it('GET /:slug/grants → grant_epoch null when no grant was ever bumped', async () => { + const agent = await store.createAgent({ slug: 'public', name: 'Public' }); + graph.toolGrants = [ + { + id: 'g-1', + agentId: agent.id, + subAgentId: null, + toolKind: 'builtin', + toolRef: 'web_search', + mcpServerId: null, + config: {}, + createdAt: new Date(), + grantEpoch: null, + }, + ]; + const res = await fetch(`${baseUrl}/public/grants`); + assert.equal(res.status, 200); + const body = (await res.json()) as { grant_epoch: string | null }; + assert.equal(body.grant_epoch, null); + }); + + it('GET /:slug/grants 503s when no agent graph store is wired', async () => { + const app = express(); + app.use(express.json()); + app.use( + '/api/v1/operator/agents', + createOperatorAgentsRouter({ + getConfigStore: () => store as unknown as ConfigStore, + getRegistry: () => registry as unknown as OrchestratorRegistry, + getChatSessionStore: () => sessionStore as unknown as ChatSessionStore, + }), + ); + const s = await listenLoopback(app); + try { + await store.createAgent({ slug: 'public', name: 'Public' }); + const addr = s.address() as AddressInfo; + const res = await fetch( + `http://127.0.0.1:${String(addr.port)}/api/v1/operator/agents/public/grants`, + ); + assert.equal(res.status, 503); + assert.equal( + ((await res.json()) as { error: string }).error, + 'agent_graph_store_unavailable', + ); + } finally { + await new Promise((r) => s.close(() => r())); + } + }); + it('PUT /:slug/bindings replaces the channel bindings', async () => { const agent = await store.createAgent({ slug: 'public', name: 'Public' }); const res = await fetch(`${baseUrl}/public/bindings`, { From 90561a5713c4d7f92a91a21f0078c5bff35c12fc Mon Sep 17 00:00:00 2001 From: Marcel Wege Date: Tue, 25 Aug 2026 18:22:04 +0200 Subject: [PATCH 05/11] feat(web-ui): _lib grant/assignment types + typed callers for per-agent plugin, grant, delegation and allowlist endpoints (W0c, #861/#862) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the existing shared type surface in app/_lib (never re-declare it): - ToolGrantNode gains an OPTIONAL grantEpoch (issue #861 — stamped by bumpMcpGrantEpoch as config.verdictEpoch; absent on older middleware), following the privacyBypass/kgIngest optional-field convention. - McpServerNode gains an OPTIONAL delegation (issue #862 — per-SERVER identity mode surfaced by mcpNode; the assignment UI shows it read-only and changes it via setMcpServerDelegation with a clearly global effect). - McpGrantMatrixRow gains OPTIONAL delegation + grantEpoch decorations, mirroring the extended GET /mcp-grants rows. New typed callers, same callJson envelope against /v1/operator/*: - agents.ts: getAgentPlugins / toggleAgentPlugin (PATCH with the plugin id in the BODY — ids contain '/') and getAgentGrants (per-agent agent_tool_grants + plugin_mcp_grants + top-level grant_epoch), mirroring routes/operatorAgents.ts. - agentBuilder.ts: replaceMcpToolAllowlist (PUT /mcp-grants with toolNames[] + optional delegation; typed McpToolAllowlistResult reports granted/revoked and delegationScope: 'server'), mirroring the #862 allowlist extension of the agent-builder router. i18n hard rule support: these routes emit { error: '' } envelopes that ApiError.code (which parses { code }) cannot see, so parseOperatorAgentErrorCode / parseMcpGrantErrorCode extract and narrow the machine code to a typed union — page units map codes to catalogue keys and never render the raw body. Verified by app/_lib/__tests__/agentGrants.test.ts (wire shape, error-code extraction, optional-field type surface); full gate: lint + typecheck + test (843) + i18n:check all green. --- web-ui/app/_lib/__tests__/agentGrants.test.ts | 346 ++++++++++++++++++ web-ui/app/_lib/agentBuilder.ts | 114 ++++++ web-ui/app/_lib/agents.ts | 142 +++++++ 3 files changed, 602 insertions(+) create mode 100644 web-ui/app/_lib/__tests__/agentGrants.test.ts diff --git a/web-ui/app/_lib/__tests__/agentGrants.test.ts b/web-ui/app/_lib/__tests__/agentGrants.test.ts new file mode 100644 index 00000000..22c58c74 --- /dev/null +++ b/web-ui/app/_lib/__tests__/agentGrants.test.ts @@ -0,0 +1,346 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { + getAgentGrants, + getAgentPlugins, + parseOperatorAgentErrorCode, + toggleAgentPlugin, + type AgentGrantsDto, +} from '../agents'; +import { + parseMcpGrantErrorCode, + replaceMcpToolAllowlist, + type McpGrantMatrixRow, + type McpServerNode, + type McpToolAllowlistResult, + type ToolGrantNode, +} from '../agentBuilder'; +import { ApiError } from '../api'; + +/** + * W0c (#861/#862) — the _lib layer for the per-agent grant/assignment UI. + * + * These callers mirror the REST contracts of `routes/operatorAgents.ts` + * (per-agent grants read, plugin toggle) and `routes/agentBuilder.ts` + * (allowlist replace + delegation surfacing) in the middleware. The tests pin + * the URL/method/body wire shape and the error-code extraction the page units + * build their i18n mapping on — never the middleware behavior itself (that + * lives in middleware/test/). + */ + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }); +} + +function lastCall(mock: ReturnType): { + url: string; + init: RequestInit; +} { + const call = mock.mock.calls.at(-1); + if (!call) throw new Error('fetch was not called'); + return { url: String(call[0]), init: (call[1] ?? {}) as RequestInit }; +} + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('getAgentGrants (#861)', () => { + it('GETs the per-agent grants read model and surfaces the grant epoch', async () => { + const payload: AgentGrantsDto = { + slug: 'hr-agent', + grant_epoch: '2026-08-25 17:00:00.000000+00', + tool_grants: [ + { + id: 'g1', + tool_kind: 'mcp', + tool_ref: 'mcp:odoo:read_employees', + sub_agent_id: null, + mcp_server_id: 's1', + server_name: 'odoo', + grant_epoch: '2026-08-25 17:00:00.000000+00', + created_at: '2026-08-20T09:00:00.000Z', + }, + { + id: 'g2', + tool_kind: 'native', + tool_ref: 'web_search', + sub_agent_id: null, + mcp_server_id: null, + server_name: null, + grant_epoch: null, + created_at: '2026-08-20T09:00:00.000Z', + }, + ], + plugin_mcp_grants: [ + { + plugin_id: '@omadia/odoo', + mcp_server_id: 's1', + server_name: 'odoo', + granted_by: 'operator', + granted_at: '2026-08-19T08:00:00.000Z', + }, + ], + }; + const fetchMock = vi.fn().mockResolvedValue(jsonResponse(payload)); + vi.stubGlobal('fetch', fetchMock); + + const grants = await getAgentGrants('hr-agent'); + + const { url, init } = lastCall(fetchMock); + expect(url).toBe('/bot-api/v1/operator/agents/hr-agent/grants'); + expect(init.method).toBeUndefined(); + expect(grants.grant_epoch).toBe('2026-08-25 17:00:00.000000+00'); + expect(grants.tool_grants[0]?.grant_epoch).toBe( + '2026-08-25 17:00:00.000000+00', + ); + expect(grants.tool_grants[1]?.grant_epoch).toBeNull(); + expect(grants.plugin_mcp_grants[0]?.plugin_id).toBe('@omadia/odoo'); + }); + + it('URL-encodes the slug', async () => { + const fetchMock = vi.fn().mockResolvedValue( + jsonResponse({ + slug: 'a b', + grant_epoch: null, + tool_grants: [], + plugin_mcp_grants: [], + }), + ); + vi.stubGlobal('fetch', fetchMock); + + await getAgentGrants('a b'); + + expect(lastCall(fetchMock).url).toBe( + '/bot-api/v1/operator/agents/a%20b/grants', + ); + }); +}); + +describe('getAgentPlugins / toggleAgentPlugin (#861)', () => { + it('GETs the per-agent plugin assignment', async () => { + const fetchMock = vi.fn().mockResolvedValue( + jsonResponse({ + slug: 'hr-agent', + fallback: false, + plugins: [{ id: '@omadia/odoo', config: {}, enabled: true }], + }), + ); + vi.stubGlobal('fetch', fetchMock); + + const res = await getAgentPlugins('hr-agent'); + + expect(lastCall(fetchMock).url).toBe( + '/bot-api/v1/operator/agents/hr-agent/plugins', + ); + expect(res.fallback).toBe(false); + expect(res.plugins[0]?.enabled).toBe(true); + }); + + it('PATCHes a single-plugin toggle with the id in the BODY (ids contain "/")', async () => { + const fetchMock = vi.fn().mockResolvedValue( + jsonResponse({ + ok: true, + fallback: false, + plugin: { id: '@omadia/odoo', enabled: false }, + }), + ); + vi.stubGlobal('fetch', fetchMock); + + const res = await toggleAgentPlugin('hr-agent', '@omadia/odoo', false); + + const { url, init } = lastCall(fetchMock); + expect(url).toBe('/bot-api/v1/operator/agents/hr-agent/plugins'); + expect(init.method).toBe('PATCH'); + expect(JSON.parse(String(init.body))).toEqual({ + id: '@omadia/odoo', + enabled: false, + }); + expect(res.plugin.enabled).toBe(false); + }); + + it('throws an ApiError whose body still carries the machine code', async () => { + const fetchMock = vi + .fn() + .mockResolvedValue(jsonResponse({ error: 'plugin_not_assigned' }, 404)); + vi.stubGlobal('fetch', fetchMock); + + const err: unknown = await toggleAgentPlugin( + 'hr-agent', + '@omadia/none', + false, + ).catch((e: unknown) => e); + + expect(err).toBeInstanceOf(ApiError); + expect(parseOperatorAgentErrorCode(err)).toBe('plugin_not_assigned'); + }); +}); + +describe('parseOperatorAgentErrorCode (i18n hard rule support)', () => { + it('extracts a known { error } code from the ApiError body', () => { + const err = new ApiError(404, 'GET x failed: 404', '{"error":"not_found"}'); + expect(parseOperatorAgentErrorCode(err)).toBe('not_found'); + }); + + it('is null for an unknown code, a non-JSON body, and a non-ApiError', () => { + expect( + parseOperatorAgentErrorCode( + new ApiError(500, 'x', '{"error":"something_else"}'), + ), + ).toBeNull(); + expect( + parseOperatorAgentErrorCode(new ApiError(502, 'x', '502')), + ).toBeNull(); + expect(parseOperatorAgentErrorCode(new Error('plain'))).toBeNull(); + expect(parseOperatorAgentErrorCode(undefined)).toBeNull(); + }); +}); + +describe('replaceMcpToolAllowlist (#862)', () => { + const result: McpToolAllowlistResult = { + agentSlug: 'hr-agent', + mcpServerId: 's1', + toolNames: ['read_employees', 'read_leaves'], + granted: ['read_leaves'], + revoked: ['delete_employee'], + delegation: 'per_user', + delegationScope: 'server', + }; + + it('PUTs toolNames[] and omits delegation when not given', async () => { + const fetchMock = vi.fn().mockResolvedValue(jsonResponse(result)); + vi.stubGlobal('fetch', fetchMock); + + const res = await replaceMcpToolAllowlist('hr-agent', 's1', [ + 'read_employees', + 'read_leaves', + ]); + + const { url, init } = lastCall(fetchMock); + expect(url).toBe('/bot-api/v1/operator/mcp-grants'); + expect(init.method).toBe('PUT'); + expect(JSON.parse(String(init.body))).toEqual({ + agentSlug: 'hr-agent', + mcpServerId: 's1', + toolNames: ['read_employees', 'read_leaves'], + }); + expect(res.revoked).toEqual(['delete_employee']); + expect(res.delegationScope).toBe('server'); + }); + + it('sends delegation when given (per-SERVER switch, global effect)', async () => { + const fetchMock = vi.fn().mockResolvedValue(jsonResponse(result)); + vi.stubGlobal('fetch', fetchMock); + + await replaceMcpToolAllowlist('hr-agent', 's1', ['read_employees'], { + delegation: 'per_user', + }); + + expect(JSON.parse(String(lastCall(fetchMock).init.body))).toEqual({ + agentSlug: 'hr-agent', + mcpServerId: 's1', + toolNames: ['read_employees'], + delegation: 'per_user', + }); + }); + + it('an empty allowlist is a full revoke, not a malformed request', async () => { + const fetchMock = vi.fn().mockResolvedValue( + jsonResponse({ ...result, toolNames: [], granted: [], revoked: ['read_employees'] }), + ); + vi.stubGlobal('fetch', fetchMock); + + const res = await replaceMcpToolAllowlist('hr-agent', 's1', []); + + expect(JSON.parse(String(lastCall(fetchMock).init.body)).toolNames).toEqual([]); + expect(res.toolNames).toEqual([]); + }); +}); + +describe('parseMcpGrantErrorCode (i18n hard rule support)', () => { + it('extracts the verdict-gate rejection (409 config_validation)', () => { + const err = new ApiError( + 409, + 'PUT /v1/operator/mcp-grants failed: 409', + '{"error":"config_validation","message":"tool \\"x\\" is blocked"}', + ); + expect(parseMcpGrantErrorCode(err)).toBe('config_validation'); + }); + + it('covers the grant route codes and rejects unknown ones', () => { + for (const code of [ + 'invalid_grant', + 'invalid_delegation', + 'orchestrator_not_found', + 'mcp_server_not_found', + 'grant_not_found', + ]) { + expect( + parseMcpGrantErrorCode(new ApiError(400, 'x', `{"error":"${code}"}`)), + ).toBe(code); + } + expect( + parseMcpGrantErrorCode(new ApiError(500, 'x', '{"error":"internal"}')), + ).toBeNull(); + expect(parseMcpGrantErrorCode('not an error')).toBeNull(); + }); +}); + +describe('shared type surface (extended, not re-declared)', () => { + it('ToolGrantNode carries an OPTIONAL grantEpoch (absent on older middleware)', () => { + const oldMiddleware: ToolGrantNode = { + id: 'g1', + agentId: 'a1', + subAgentId: null, + toolKind: 'mcp', + toolRef: 'mcp:odoo:read_employees', + mcpServerId: 's1', + }; + const newMiddleware: ToolGrantNode = { + ...oldMiddleware, + grantEpoch: '2026-08-25 17:00:00.000000+00', + }; + expect(oldMiddleware.grantEpoch).toBeUndefined(); + expect(newMiddleware.grantEpoch).toBe('2026-08-25 17:00:00.000000+00'); + }); + + it('McpServerNode carries an OPTIONAL per-server delegation next to discoveredTools', () => { + const server: McpServerNode = { + id: 's1', + name: 'odoo', + transport: 'http', + endpoint: 'https://mcp.example', + status: 'enabled', + lastDiscoveredAt: null, + discoveredTools: [{ name: 'read_employees', description: 'list staff' }], + delegation: 'service', + }; + expect(server.discoveredTools[0]?.name).toBe('read_employees'); + expect(server.delegation).toBe('service'); + }); + + it('McpGrantMatrixRow carries OPTIONAL delegation + grantEpoch decorations', () => { + const row: McpGrantMatrixRow = { + grantId: 'g1', + holderKind: 'agent', + agentSlug: 'hr-agent', + agentName: 'HR', + subAgentId: null, + subAgentName: null, + serverId: 's1', + serverName: 'odoo', + toolName: 'read_employees', + severity: null, + notYetScanned: false, + acked: false, + blocked: false, + delegation: 'per_user', + grantEpoch: null, + }; + expect(row.delegation).toBe('per_user'); + expect(row.grantEpoch).toBeNull(); + }); +}); diff --git a/web-ui/app/_lib/agentBuilder.ts b/web-ui/app/_lib/agentBuilder.ts index 61a8a9b7..f5326cbf 100644 --- a/web-ui/app/_lib/agentBuilder.ts +++ b/web-ui/app/_lib/agentBuilder.ts @@ -185,6 +185,10 @@ export interface ToolGrantNode { toolKind: ToolKind; toolRef: string; mcpServerId: string | null; + /** Issue #861 — last verdict-epoch bump of this grant (`bumpMcpGrantEpoch` + * stamps `config.verdictEpoch`); `null` until the first bump touches the + * row; absent on older middleware. */ + grantEpoch?: string | null; } /** Scan verdict decoration on a discovered MCP tool (issue #454). Absent on @@ -241,6 +245,12 @@ export interface McpServerNode { kgIngest?: boolean; /** Epic #459 — declared config fields (from placeholders or a registry). */ configSchema?: McpConfigField[]; + /** Issue #862 — per-SERVER identity delegation mode (W0-1): every + * assignment of this server acts under the same identity resolution, so + * an assignment UI shows this value read-only and changes it via + * {@link setMcpServerDelegation} (global effect for every agent holding a + * grant on the server); absent on older middleware. */ + delegation?: McpDelegation; } /** Epic #459 — a declared MCP server config field. */ @@ -674,6 +684,13 @@ export interface McpGrantMatrixRow { notYetScanned: boolean; acked: boolean; blocked: boolean; + /** Issue #862 — the granted server's per-SERVER delegation mode (`null` + * when the row has no server); absent on older middleware. */ + delegation?: McpDelegation | null; + /** Issue #862 — last verdict-epoch bump of this grant; `null` until the + * first bump (always `null` on skill-binding / plugin rows); absent on + * older middleware. */ + grantEpoch?: string | null; } /** One MCP call audit entry (issue #462). No tool arguments by design. */ @@ -733,6 +750,103 @@ export async function grantMcpToolToOrchestrator( }); } +/** + * Result of an allowlist replace via `PUT /mcp-grants` with `toolNames[]` + * (issue #862). `delegationScope` is always `'server'`: delegation lives on + * `mcp_servers`, not per assignment, so a `delegation` sent with the edit + * changed the SERVER's mode — for every agent holding a grant on it. + */ +export interface McpToolAllowlistResult { + agentSlug: string; + mcpServerId: string; + /** The full allowlist after the edit (normalized tool names, sorted). */ + toolNames: string[]; + /** Tools this edit newly granted. */ + granted: string[]; + /** Tools this edit revoked (fell off the list). */ + revoked: string[]; + /** The server's delegation mode after the edit. */ + delegation: McpDelegation; + delegationScope: 'server'; +} + +/** + * Replace an agent's whole tool allowlist for one MCP server (issue #862). + * The allowlist IS the set of `agent_tool_grants` rows for the (agent, + * server) pair — tools missing from `toolNames` are revoked, new ones pass + * the same fail-closed verdict gate as a single grant (one rejection aborts + * the whole edit; nothing is written). + * + * `delegation` optionally sets the SERVER's delegation mode in the same + * call — a per-server switch with global effect, see + * {@link McpToolAllowlistResult.delegationScope}. + */ +export async function replaceMcpToolAllowlist( + agentSlug: string, + mcpServerId: string, + toolNames: string[], + opts?: { delegation?: McpDelegation }, +): Promise { + return callJson('/v1/operator/mcp-grants', { + method: 'PUT', + body: JSON.stringify({ + agentSlug, + mcpServerId, + toolNames, + ...(opts?.delegation !== undefined ? { delegation: opts.delegation } : {}), + }), + }); +} + +/** + * Machine codes the agent-builder grant routes emit as `{ error: '' }` + * (they predate the `{ code }` envelope `ApiError.code` parses, so the code + * must be read from the body). + * + * i18n HARD RULE: these are NOT user-facing text. Pages map each code to a + * message-catalogue key and render the localized copy; the raw body string + * must never reach the UI. + */ +export const MCP_GRANT_ERROR_CODES = [ + // PUT /mcp-grants — the verdict gate (`assertMcpToolAllowed`) rejects + // blocked / unscanned tools as a 409 `config_validation`. + 'config_validation', + 'invalid_delegation', + 'invalid_grant', + 'mcp_server_not_found', + 'multi_orchestrator_unavailable', + 'orchestrator_not_found', + // DELETE /mcp-grants/:grantId + 'grant_not_found', + 'invalid_grant_id', + 'not_an_mcp_grant', +] as const; + +export type McpGrantErrorCode = (typeof MCP_GRANT_ERROR_CODES)[number]; + +const MCP_GRANT_ERROR_CODE_SET: ReadonlySet = new Set( + MCP_GRANT_ERROR_CODES, +); + +/** + * Extract the machine code from a failed grant/allowlist call, or `null` + * when the error is not an {@link ApiError}, its body is not JSON, or the + * code is not one this client knows. Total by construction — a proxy's HTML + * 502 page yields `null`, never a throw. + */ +export function parseMcpGrantErrorCode(err: unknown): McpGrantErrorCode | null { + if (!(err instanceof ApiError)) return null; + try { + const parsed = JSON.parse(err.body) as { error?: unknown }; + return typeof parsed.error === 'string' && + MCP_GRANT_ERROR_CODE_SET.has(parsed.error) + ? (parsed.error as McpGrantErrorCode) + : null; + } catch { + return null; + } +} + export async function revokeMcpGrant(grantId: string): Promise { await callJson(`/v1/operator/mcp-grants/${encodeURIComponent(grantId)}`, { method: 'DELETE' }); } diff --git a/web-ui/app/_lib/agents.ts b/web-ui/app/_lib/agents.ts index afc4b899..aff42c40 100644 --- a/web-ui/app/_lib/agents.ts +++ b/web-ui/app/_lib/agents.ts @@ -174,6 +174,148 @@ export async function deleteOperatorAgent(slug: string): Promise { }); } +// ── W0c (#861) — per-agent plugin assignment + grant read model ───────── + +export interface AgentPluginsDto { + slug: string; + /** True when this agent is the platform fallback (its plugins always run + * with the global store config). */ + fallback: boolean; + plugins: OperatorAgentPluginDto[]; +} + +/** + * Per-agent read of the plugin assignment (issue #861) — same row shape as + * the `plugins` array on `GET /v1/operator/agents`, so the agent detail page + * does not have to filter the full dashboard payload. + */ +export async function getAgentPlugins(slug: string): Promise { + return callJson( + `/v1/operator/agents/${encodeURIComponent(slug)}/plugins`, + ); +} + +export interface ToggleAgentPluginResponse { + ok: boolean; + fallback: boolean; + plugin: { id: string; enabled: boolean }; +} + +/** + * Enable/disable ONE plugin on an agent (issue #861). The plugin id travels + * in the body, not the path: plugin ids contain `/` (`@omadia/odoo`), which + * an Express path segment cannot carry without double-encoding. Server-side + * the toggle preserves the row's existing per-agent config; disabling a + * plugin that was never assigned yields a 404 with `error: + * 'plugin_not_assigned'` (see {@link parseOperatorAgentErrorCode}). + */ +export async function toggleAgentPlugin( + slug: string, + pluginId: string, + enabled: boolean, +): Promise { + return callJson( + `/v1/operator/agents/${encodeURIComponent(slug)}/plugins`, + { method: 'PATCH', body: JSON.stringify({ id: pluginId, enabled }) }, + ); +} + +/** One `agent_tool_grants` row of the per-agent grant read model (issue + * #861). Snake_case mirrors the REST payload verbatim, like the other + * operator-agents DTOs in this file. */ +export interface AgentToolGrantRowDto { + id: string; + tool_kind: 'native' | 'mcp'; + tool_ref: string; + sub_agent_id: string | null; + mcp_server_id: string | null; + /** Joined-in display name; null for native tools or a deleted server. */ + server_name: string | null; + /** Issue #861 — last verdict-epoch bump of this grant (`bumpMcpGrantEpoch` + * stamps `config.verdictEpoch`); null until the first bump touches the + * row. */ + grant_epoch: string | null; + created_at: string; +} + +/** One `plugin_mcp_grants` row of a plugin assigned to the agent (#861). */ +export interface AgentPluginMcpGrantRowDto { + plugin_id: string; + mcp_server_id: string; + server_name: string | null; + granted_by: string; + granted_at: string; +} + +export interface AgentGrantsDto { + slug: string; + /** Latest verdict-epoch bump across the agent's tool grants; null when no + * grant has ever been bumped. Epochs are `now()::text` timestamps, so the + * lexicographic max the server computes IS the latest. */ + grant_epoch: string | null; + tool_grants: AgentToolGrantRowDto[]; + plugin_mcp_grants: AgentPluginMcpGrantRowDto[]; +} + +/** + * Per-agent grant read model (issue #861): the agent's own + * `agent_tool_grants` rows plus the `plugin_mcp_grants` of every plugin + * assigned to it, one response for the agent detail page. Read-only — grant + * WRITES stay on the agent-builder surface (`_lib/agentBuilder.ts`). + */ +export async function getAgentGrants(slug: string): Promise { + return callJson( + `/v1/operator/agents/${encodeURIComponent(slug)}/grants`, + ); +} + +/** + * Machine codes the operator-agents plugin/grant routes emit as + * `{ error: '' }` (they predate the `{ code }` envelope `ApiError.code` + * parses, so the code must be read from the body). + * + * i18n HARD RULE: these are NOT user-facing text. Pages map each code to a + * message-catalogue key and render the localized copy; the raw body string + * must never reach the UI. `parseOperatorAgentErrorCode` narrows to this + * union so a page's mapping can be exhaustive with a typed fallback. + */ +export const OPERATOR_AGENT_ERROR_CODES = [ + 'agent_graph_store_unavailable', + 'config_validation', + 'invalid_body', + 'invalid_slug', + 'multi_orchestrator_unavailable', + 'not_found', + 'plugin_not_assigned', +] as const; + +export type OperatorAgentErrorCode = (typeof OPERATOR_AGENT_ERROR_CODES)[number]; + +const OPERATOR_AGENT_ERROR_CODE_SET: ReadonlySet = new Set( + OPERATOR_AGENT_ERROR_CODES, +); + +/** + * Extract the machine code from a failed operator-agents call, or `null` + * when the error is not an {@link ApiError}, its body is not JSON, or the + * code is not one this client knows. Total by construction — a proxy's HTML + * 502 page yields `null`, never a throw. + */ +export function parseOperatorAgentErrorCode( + err: unknown, +): OperatorAgentErrorCode | null { + if (!(err instanceof ApiError)) return null; + try { + const parsed = JSON.parse(err.body) as { error?: unknown }; + return typeof parsed.error === 'string' && + OPERATOR_AGENT_ERROR_CODE_SET.has(parsed.error) + ? (parsed.error as OperatorAgentErrorCode) + : null; + } catch { + return null; + } +} + export async function replaceAgentPlugins( slug: string, plugins: Array<{ id: string; config?: Record; enabled?: boolean }>, From 4a09e1c600bf2bf261998d0b2a650df57aece83e Mon Sep 17 00:00:00 2001 From: Marcel Wege Date: Tue, 25 Aug 2026 18:38:57 +0200 Subject: [PATCH 06/11] feat(web-ui): agent detail route with per-agent plugin enable/disable (W0c, #861) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds /operator/agents/[slug] — the per-agent capability page for the multi-agent Teams identities epic (#860). The RSC page reuses the existing GET /v1/operator/agents list (served by every deployed middleware) and hands the matching agent to a client AgentDetail component: - assigned-plugins list with an instant per-plugin enable/disable switch via the single-plugin PATCH (toggleAgentPlugin), so flipping one flag no longer PUTs the whole set - full attach/detach + per-agent config editing reuses the dashboard's PluginsDnd editor unchanged (replace-set PUT), remounted through the now-exported pluginsRevisionKey after each save - operator-agents machine error codes map to localized detailErrors.* catalogue copy (en + de); raw bodies never reach the UI - FALLBACK_AGENT_SLUG moves to _lib/agents.ts so the dashboard and the detail route share one definition of the protected fallback Linking from the dashboard/nav and mounting the sibling grant/MCP components stays with the wiring unit. --- web-ui/app/_lib/agents.ts | 12 + .../[slug]/__tests__/AgentDetail.test.tsx | 222 ++++++++++++++++++ .../agents/[slug]/_components/AgentDetail.tsx | 208 ++++++++++++++++ web-ui/app/operator/agents/[slug]/page.tsx | 108 +++++++++ .../agents/_components/AgentsDashboard.tsx | 17 +- web-ui/messages/de.json | 16 ++ web-ui/messages/en.json | 16 ++ web-ui/scripts/i18n-identical-allowlist.json | 1 + 8 files changed, 588 insertions(+), 12 deletions(-) create mode 100644 web-ui/app/operator/agents/[slug]/__tests__/AgentDetail.test.tsx create mode 100644 web-ui/app/operator/agents/[slug]/_components/AgentDetail.tsx create mode 100644 web-ui/app/operator/agents/[slug]/page.tsx diff --git a/web-ui/app/_lib/agents.ts b/web-ui/app/_lib/agents.ts index aff42c40..e846a3f3 100644 --- a/web-ui/app/_lib/agents.ts +++ b/web-ui/app/_lib/agents.ts @@ -489,6 +489,18 @@ export async function rehydrateFallback(): Promise<{ }); } +/** + * Slug of the auto-seeded fallback orchestrator (kept in sync with + * `FALLBACK_AGENT_SLUG` in `@omadia/orchestrator`). The fallback orchestrator + * is the catch-all for unbound channel traffic, so its Disable/Delete actions + * are blocked in the UI (and server-side). Treat an orchestrator as the + * protected fallback when it carries this slug OR is the active platform + * fallback pointer — the platform pointer may be intentionally unset while the + * seeded `fallback` row still exists. Shared by the dashboard and the agent + * detail route (issue #861) so the two never disagree on who the fallback is. + */ +export const FALLBACK_AGENT_SLUG = 'fallback'; + /** * #679 / I5 — the description the middleware seeds into the fallback Agent on * first boot (`FALLBACK_AGENT_SEED_DESCRIPTION` in diff --git a/web-ui/app/operator/agents/[slug]/__tests__/AgentDetail.test.tsx b/web-ui/app/operator/agents/[slug]/__tests__/AgentDetail.test.tsx new file mode 100644 index 00000000..9ab7e3db --- /dev/null +++ b/web-ui/app/operator/agents/[slug]/__tests__/AgentDetail.test.tsx @@ -0,0 +1,222 @@ +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { ApiError } from '../../../../_lib/api'; +import type { + OperatorAgentDto, + PluginCatalogEntryDto, +} from '../../../../_lib/agents'; +import { renderWithIntl } from '../../../../_lib/test-utils'; +import { AgentDetail } from '../_components/AgentDetail'; + +/** + * Issue #861 — agent detail route, plugin-assignment slice. + * + * What is worth guarding here is the wiring, not the pixels: + * + * - one row per assigned plugin, checkbox mirroring `enabled`, catalog name + * joined in once the catalog resolves; + * - the instant toggle goes through `toggleAgentPlugin` (the single-plugin + * PATCH) with the INVERTED flag, and only a success refreshes the router; + * - a failed toggle renders the LOCALIZED message for the machine code — + * never the raw `{ error: ... }` body (web-ui i18n hard rule); + * - PluginsDnd saves are forwarded to `replaceAgentPlugins` for this + * agent's slug. + * + * PluginsDnd itself (dnd-kit drag) stays out of jsdom scope per the + * vitest.config note — it is stubbed and driven via its onReplace callback. + */ + +const { mockCatalog, mockToggle, mockReplace, mockRefresh } = vi.hoisted( + () => ({ + mockCatalog: vi.fn(), + mockToggle: vi.fn(), + mockReplace: vi.fn(), + mockRefresh: vi.fn(), + }), +); + +vi.mock('next/navigation', () => ({ + useRouter: () => ({ refresh: mockRefresh }), +})); + +// Spread the real module so parseOperatorAgentErrorCode (the code→copy +// narrowing under test) stays genuine; only the network calls are stubbed. +vi.mock('../../../../_lib/agents', async (importOriginal) => ({ + ...(await importOriginal()), + listAgentPluginCatalog: mockCatalog, + toggleAgentPlugin: mockToggle, + replaceAgentPlugins: mockReplace, +})); + +vi.mock('../../_components/PluginsDnd', () => ({ + PluginsDnd: (props: { + onReplace: ( + plugins: Array<{ + id: string; + enabled?: boolean; + config?: Record; + }>, + ) => void; + }) => ( + // eslint-disable-next-line no-restricted-syntax -- test stub standing in for the dnd editor, not a §4.2 CTA + + {isOpen ? ( +
+ + {server.discoveredTools.length === 0 ? ( +
{t('mcp.noTools')}
+ ) : ( + <> +
+ {t('mcp.discoveredTools', { count: server.discoveredTools.length })} +
+ {server.discoveredTools.map((tool) => { + const v = tool.verdict; + const ackNeeded = needsAck(tool); + const unscanned = !v || v.notYetScanned; + const checked = sel.has(tool.name); + // A blocked tool must stay ungrantable (fail-closed), + // but un-checking an already granted one must always work. + const disabled = !isGrantable(tool) && !checked; + const ackKey = `${server.id} ${tool.name}`; + return ( +
+
+ +
+ {v?.acked && !v.ackStale ? ( + + {t('mcp.acked')} + + ) : null} + +
+
+ {tool.description !== undefined && tool.description !== '' ? ( +
+ {tool.description} +
+ ) : null} + {v !== undefined && v.riskCodes.length > 0 ? ( +
+ {v.riskCodes.join(', ')} +
+ ) : null} + {ackNeeded ? ( +
+ + {t('mcp.needsAckHint')} + + +
+ ) : null} + {unscanned ? ( +
+ {t('mcp.unscannedHint')} +
+ ) : null} +
+ ); + })} +
+ + + {dirty ? ( + + ) : null} + {grantedSet.size > 0 ? ( + + ) : null} + {saved !== null && saved.id === server.id && !dirty ? ( + + {t('mcp.savedSummary', { + granted: saved.granted, + revoked: saved.revoked, + })} + + ) : null} +
+ + )} +
+ ) : null} + + ); + })} + {error !== null ?
{error}
: null} + setConfirmUnassign(null)} + onConfirm={() => { + const target = confirmUnassign; + setConfirmUnassign(null); + if (target !== null) void saveAllowlist(target, []); + }} + /> + + ); +} diff --git a/web-ui/messages/de.json b/web-ui/messages/de.json index ffdf4761..5c67cae1 100644 --- a/web-ui/messages/de.json +++ b/web-ui/messages/de.json @@ -1804,7 +1804,45 @@ "orphanDetachAllTooltip": "Entfernt alle veralteten Plugin-Zuordnungen (alte IDs aus früheren Deployments) auf einmal.", "orphanExplain": "Plugin-IDs aus früheren Seeds, die im aktuellen Katalog nicht mehr existieren (typisch: Umbenennung de.byte5.agent.* → @omadia/*). Werden beim nächsten „Speichern\" verworfen, außer du markierst sie als „Behalten\".", "orphanKeep": "Behalten", - "seededDescription": "Beim ersten Start automatisch angelegt. Empfängt Channel-Traffic ohne explizite Zuordnung, bis du eine einrichtest." + "seededDescription": "Beim ersten Start automatisch angelegt. Empfängt Channel-Traffic ohne explizite Zuordnung, bis du eine einrichtest.", + "mcp": { + "heading": "MCP-Server", + "hint": "Weise diesem Orchestrator MCP-Server zu und pflege die Tool-Allowlist. Ein Tool ist erst aufrufbar, wenn es hier freigegeben ist und den Scan-Gate passiert hat.", + "loading": "MCP-Server werden geladen…", + "loadError": "MCP-Server konnten nicht geladen werden: {detail}", + "empty": "Keine MCP-Server verfügbar. Registriere und discovere Server zuerst im MCP Control Center.", + "toolsGranted": "{granted} von {total} Tools freigegeben", + "notAssigned": "Nicht zugewiesen", + "discoveredTools": "Entdeckte Tools ({count})", + "noTools": "Noch keine Tools entdeckt — starte die Discovery zuerst im MCP Control Center.", + "toolAria": "{tool} freigeben", + "acked": "Bestätigt", + "needsAckHint": "Das Scan-Urteil muss bestätigt werden, bevor dieses Tool freigegeben werden kann.", + "unscannedHint": "Noch nicht gescannt — ungescannte Tools können nicht freigegeben werden.", + "ack": "Urteil bestätigen", + "ackConfirm": "Bestätigung wirklich erteilen", + "save": "Allowlist speichern", + "savedSummary": "Allowlist gespeichert — {granted} freigegeben, {revoked} entzogen.", + "selectAllGrantable": "Alle freigebbaren auswählen", + "discardDraft": "Änderungen verwerfen", + "unassign": "Server entfernen", + "unassignTitle": "MCP-Server entfernen", + "unassignBody": "Alle {count} Tool-Freigaben von \"{server}\" für diesen Orchestrator entziehen?", + "unassignConfirm": "Alle entziehen", + "cancel": "Abbrechen", + "errors": { + "config_validation": "Der Scan-Gate hat die Änderung abgelehnt — ein ausgewähltes Tool ist blockiert oder noch nicht gescannt.", + "invalid_delegation": "Der Server hat den Delegationsmodus abgelehnt.", + "invalid_grant": "Der Server hat die Freigabe-Anfrage abgelehnt.", + "mcp_server_not_found": "Dieser MCP-Server existiert nicht mehr — lade die Seite neu.", + "multi_orchestrator_unavailable": "Die Multi-Orchestrator-Runtime ist nicht verfügbar.", + "orchestrator_not_found": "Dieser Orchestrator existiert nicht mehr — lade die Seite neu.", + "grant_not_found": "Diese Freigabe existiert nicht mehr — lade die Seite neu.", + "invalid_grant_id": "Die Freigabe-Id ist ungültig.", + "not_an_mcp_grant": "Diese Freigabe ist keine MCP-Tool-Freigabe.", + "unknown": "Anfrage fehlgeschlagen: {detail}" + } + } }, "builder": { "issueReport": { diff --git a/web-ui/messages/en.json b/web-ui/messages/en.json index 4ba1c9b3..1ee771b7 100644 --- a/web-ui/messages/en.json +++ b/web-ui/messages/en.json @@ -1804,7 +1804,45 @@ "orphanDetachAllTooltip": "Detach every stale plugin row (old ids from earlier deployments) at once.", "orphanExplain": "Plugin ids from earlier seeds that no longer exist in the current catalog (typically a rename like de.byte5.agent.* → @omadia/*). Dropped on next Save unless you tick \"Keep\".", "orphanKeep": "Keep", - "seededDescription": "Auto-created on first boot. Receives channel traffic that has no explicit binding, until you configure one." + "seededDescription": "Auto-created on first boot. Receives channel traffic that has no explicit binding, until you configure one.", + "mcp": { + "heading": "MCP servers", + "hint": "Assign MCP servers to this orchestrator and edit its per-tool allowlist. A tool only becomes callable once it is granted here and has passed the scan gate.", + "loading": "Loading MCP servers…", + "loadError": "Failed to load MCP servers: {detail}", + "empty": "No MCP servers available. Register and discover servers in the MCP Control Center first.", + "toolsGranted": "{granted} of {total} tools granted", + "notAssigned": "Not assigned", + "discoveredTools": "Discovered tools ({count})", + "noTools": "No tools discovered yet — run discovery in the MCP Control Center first.", + "toolAria": "Grant {tool}", + "acked": "Acknowledged", + "needsAckHint": "The scan verdict must be acknowledged before this tool can be granted.", + "unscannedHint": "Not scanned yet — unscanned tools cannot be granted.", + "ack": "Acknowledge verdict", + "ackConfirm": "Confirm acknowledge", + "save": "Save allowlist", + "savedSummary": "Allowlist saved — {granted} granted, {revoked} revoked.", + "selectAllGrantable": "Select all grantable", + "discardDraft": "Discard changes", + "unassign": "Unassign server", + "unassignTitle": "Unassign MCP server", + "unassignBody": "Revoke all {count} tool grants of \"{server}\" for this orchestrator?", + "unassignConfirm": "Revoke all", + "cancel": "Cancel", + "errors": { + "config_validation": "The scan gate rejected the change — a selected tool is blocked or not scanned yet.", + "invalid_delegation": "The server rejected the delegation mode.", + "invalid_grant": "The server rejected the grant request.", + "mcp_server_not_found": "This MCP server does not exist anymore — reload the page.", + "multi_orchestrator_unavailable": "The multi-orchestrator runtime is not available.", + "orchestrator_not_found": "This orchestrator does not exist anymore — reload the page.", + "grant_not_found": "This grant does not exist anymore — reload the page.", + "invalid_grant_id": "The grant id is not valid.", + "not_an_mcp_grant": "This grant is not an MCP tool grant.", + "unknown": "Request failed: {detail}" + } + } }, "builder": { "issueReport": { From 777c0ba708fc40d354c5693940cc50744d1932e3 Mon Sep 17 00:00:00 2001 From: Marcel Wege Date: Tue, 25 Aug 2026 19:29:00 +0200 Subject: [PATCH 08/11] feat(web-ui): per-agent tool-grant list with grant-epoch display (W0c, #861) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AgentToolGrants renders the per-agent grant read model (GET /v1/operator/agents/:slug/grants via getAgentGrants): the agent's own agent_tool_grants rows (kind badge, tool ref, server, per-row grant epoch) plus the plugin_mcp_grants of every plugin assigned to it, with the latest grant epoch summarized in the heading. Read-only on purpose — grant writes stay on the existing surfaces (/admin/mcp grants tab and the per-agent MCP assignment editor); the spec says extend, not duplicate, so this consumes the _lib wrappers owned by the web-lib unit instead of adding a parallel client. The grant epoch is not a column: bumpMcpGrantEpoch stamps config.verdictEpoch (a now()::text timestamp) into the grant's JSONB, so null is a legitimate 'never bumped' state and renders as its own localized copy; non-null values are formatted for humans with the raw Postgres string kept as the tooltip (epoch staleness is compared lexicographically server-side, so the verbatim fallback never lies). i18n hard rule: all copy lives under operatorAgents.grants in en+de; route error codes are narrowed via parseOperatorAgentErrorCode and mapped to grants.errors.* catalogue keys — raw { error: '...' } bodies never render. Verified by app/operator/agents/[slug]/__tests__/AgentToolGrants.test.tsx (8 tests); full gate: lint + typecheck + vitest (851) + i18n:check green. Part of epic #860. --- .../[slug]/__tests__/AgentToolGrants.test.tsx | 181 +++++++++++++++ .../[slug]/_components/AgentToolGrants.tsx | 218 ++++++++++++++++++ web-ui/messages/de.json | 30 ++- web-ui/messages/en.json | 30 ++- 4 files changed, 457 insertions(+), 2 deletions(-) create mode 100644 web-ui/app/operator/agents/[slug]/__tests__/AgentToolGrants.test.tsx create mode 100644 web-ui/app/operator/agents/[slug]/_components/AgentToolGrants.tsx diff --git a/web-ui/app/operator/agents/[slug]/__tests__/AgentToolGrants.test.tsx b/web-ui/app/operator/agents/[slug]/__tests__/AgentToolGrants.test.tsx new file mode 100644 index 00000000..a107c881 --- /dev/null +++ b/web-ui/app/operator/agents/[slug]/__tests__/AgentToolGrants.test.tsx @@ -0,0 +1,181 @@ +import { screen, waitFor, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { renderWithIntl } from '../../../../_lib/test-utils'; +import { ApiError } from '../../../../_lib/api'; +import type { AgentGrantsDto } from '../../../../_lib/agents'; +import { AgentToolGrants } from '../_components/AgentToolGrants'; + +/** + * Issue #861 (epic #860) — per-agent tool-grant list with grant-epoch + * display. + * + * The component consumes the per-agent read model + * (`GET /v1/operator/agents/:slug/grants`): `agent_tool_grants` rows plus + * the `plugin_mcp_grants` of every plugin assigned to the agent. The grant + * epoch is `config.verdictEpoch` (a `now()::text` timestamp stamped by + * `bumpMcpGrantEpoch`), so `null` is a legitimate "never bumped" state and + * must render as such rather than as missing data. + */ + +const { mockGetAgentGrants } = vi.hoisted(() => ({ + mockGetAgentGrants: vi.fn(), +})); + +// Spread the real module so the error-code parser (the shared contract this +// UI maps to catalogue keys) stays genuine; only the network call is stubbed. +vi.mock('../../../../_lib/agents', async (importOriginal) => ({ + ...(await importOriginal()), + getAgentGrants: mockGetAgentGrants, +})); + +function grantsDto(overrides: Partial = {}): AgentGrantsDto { + return { + slug: 'sales-bot', + grant_epoch: '2026-08-01 10:00:00.123456+00', + tool_grants: [ + { + id: 'grant-1', + tool_kind: 'mcp', + tool_ref: 'odoo.search_partners', + sub_agent_id: null, + mcp_server_id: 'srv-odoo', + server_name: 'odoo-mcp', + grant_epoch: '2026-08-01 10:00:00.123456+00', + created_at: '2026-07-30T08:00:00.000Z', + }, + { + id: 'grant-2', + tool_kind: 'native', + tool_ref: 'memory.search', + sub_agent_id: null, + mcp_server_id: null, + server_name: null, + grant_epoch: null, + created_at: '2026-07-30T08:00:00.000Z', + }, + ], + plugin_mcp_grants: [ + { + plugin_id: '@omadia/odoo', + mcp_server_id: 'srv-odoo', + server_name: 'odoo-mcp', + granted_by: 'operator', + granted_at: '2026-07-31 09:30:00+00', + }, + ], + ...overrides, + }; +} + +beforeEach(() => { + mockGetAgentGrants.mockReset(); + mockGetAgentGrants.mockResolvedValue(grantsDto()); +}); + +async function renderGrants(): Promise { + renderWithIntl(); + await waitFor(() => expect(mockGetAgentGrants).toHaveBeenCalledWith('sales-bot')); +} + +function rowOf(text: string): HTMLElement { + const row = screen.getByText(text).closest('li'); + expect(row).toBeTruthy(); + return row as HTMLElement; +} + +describe('AgentToolGrants (#861)', () => { + it('lists the agent tool grants with kind, server, and a formatted grant epoch', async () => { + await renderGrants(); + + const mcpRow = rowOf('odoo.search_partners'); + expect(within(mcpRow).getByText('MCP')).toBeTruthy(); + expect(within(mcpRow).getByText('odoo-mcp')).toBeTruthy(); + // The Postgres `now()::text` epoch is formatted for humans (raw value + // kept as the tooltip), so the row shows a localized date, not the + // verbatim "+00" string. + expect(within(mcpRow).getByText(/epoch: .*2026/)).toBeTruthy(); + expect(within(mcpRow).queryByText(/\+00/)).toBeNull(); + expect( + within(mcpRow).getByTitle('2026-08-01 10:00:00.123456+00'), + ).toBeTruthy(); + }); + + it('renders a null grant epoch as its own "never bumped" state', async () => { + await renderGrants(); + + const nativeRow = rowOf('memory.search'); + expect(within(nativeRow).getByText(/never bumped/)).toBeTruthy(); + // Native grants have no MCP server to attribute. + expect(within(nativeRow).queryByText('odoo-mcp')).toBeNull(); + }); + + it('shows the latest grant epoch in the heading summary, and "never bumped" when no grant was ever bumped', async () => { + await renderGrants(); + expect(screen.getByText(/Grant epoch: .*2026/)).toBeTruthy(); + + mockGetAgentGrants.mockResolvedValue( + grantsDto({ grant_epoch: null, tool_grants: [], plugin_mcp_grants: [] }), + ); + renderWithIntl(); + expect(await screen.findByText('Grant epoch: never bumped')).toBeTruthy(); + }); + + it('lists plugin MCP grants with server and attribution', async () => { + await renderGrants(); + + const pluginRow = rowOf('@omadia/odoo'); + expect(within(pluginRow).getByText('odoo-mcp')).toBeTruthy(); + expect(within(pluginRow).getByText(/granted by operator/)).toBeTruthy(); + }); + + it('renders both empty states when the agent holds nothing', async () => { + mockGetAgentGrants.mockResolvedValue( + grantsDto({ grant_epoch: null, tool_grants: [], plugin_mcp_grants: [] }), + ); + renderWithIntl(); + + expect( + await screen.findByText('This orchestrator holds no tool grants yet.'), + ).toBeTruthy(); + expect( + screen.getByText("No MCP servers are granted to this orchestrator's plugins."), + ).toBeTruthy(); + }); + + it('maps machine error codes to localized copy — the raw body never renders', async () => { + mockGetAgentGrants.mockRejectedValue( + new ApiError(404, 'GET /v1/operator/agents/sales-bot/grants failed: 404', '{"error":"not_found"}'), + ); + renderWithIntl(); + + const alert = await screen.findByRole('alert'); + expect(alert.textContent).toContain( + 'This orchestrator does not exist anymore — reload the page.', + ); + expect(alert.textContent).not.toContain('not_found'); + }); + + it('falls back to the localized unknown-error sentence with the technical detail', async () => { + mockGetAgentGrants.mockRejectedValue(new Error('socket hang up')); + renderWithIntl(); + + const alert = await screen.findByRole('alert'); + expect(alert.textContent).toContain('Loading grants failed:'); + expect(alert.textContent).toContain('socket hang up'); + }); + + it('re-fetches on Refresh and recovers from a failed load', async () => { + mockGetAgentGrants.mockRejectedValueOnce(new Error('offline')); + renderWithIntl(); + await screen.findByRole('alert'); + + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Refresh' })); + + expect(await screen.findByText('odoo.search_partners')).toBeTruthy(); + expect(screen.queryByRole('alert')).toBeNull(); + expect(mockGetAgentGrants).toHaveBeenCalledTimes(2); + }); +}); diff --git a/web-ui/app/operator/agents/[slug]/_components/AgentToolGrants.tsx b/web-ui/app/operator/agents/[slug]/_components/AgentToolGrants.tsx new file mode 100644 index 00000000..dbb22e1a --- /dev/null +++ b/web-ui/app/operator/agents/[slug]/_components/AgentToolGrants.tsx @@ -0,0 +1,218 @@ +'use client'; + +import { useCallback, useEffect, useState } from 'react'; +import { useFormatter, useTranslations } from 'next-intl'; + +import { Button } from '@/app/_components/ui/Button'; +import { + getAgentGrants, + parseOperatorAgentErrorCode, + type AgentGrantsDto, + type AgentToolGrantRowDto, +} from '../../../../_lib/agents'; +import { humanizeApiError } from '../../_components/AgentsDashboard'; + +type Formatter = ReturnType; + +interface AgentToolGrantsProps { + /** Slug of the orchestrator whose grants are listed. */ + readonly slug: string; +} + +/** + * Issue #861 (epic #860) — per-agent tool-grant list with grant-epoch + * display, detail-page slice. + * + * Read-only on purpose: it renders the per-agent read model from + * `GET /v1/operator/agents/:slug/grants` (`getAgentGrants`) — the agent's + * own `agent_tool_grants` rows plus the `plugin_mcp_grants` of every plugin + * assigned to it. Grant WRITES stay on the existing surfaces (`/admin/mcp` + * grants tab and the per-agent MCP assignment editor) — spec says extend, + * not duplicate. + * + * The grant epoch is NOT a column: `bumpMcpGrantEpoch` stamps + * `config.verdictEpoch` (a `now()::text` timestamp) into the grant's JSONB + * when a server's tool surface or verdict state changes, and agents rebuild + * against it. A `null` epoch therefore means "never bumped", not "missing + * data" — it renders as its own localized state instead of a dash. + * + * Error copy: the routes emit machine codes as `{ error: '' }`. + * `parseOperatorAgentErrorCode` narrows them and each code maps to a + * `grants.errors.*` catalogue key; unknown failures render the localized + * fallback sentence with the technical detail as an ICU argument — raw + * bodies never reach the UI (web-ui i18n hard rule). + */ +export function AgentToolGrants(props: AgentToolGrantsProps): React.ReactElement { + const t = useTranslations('operatorAgents'); + const format = useFormatter(); + const [grants, setGrants] = useState(null); + const [error, setError] = useState(null); + const [busy, setBusy] = useState(false); + + const localizeError = useCallback( + (err: unknown): string => { + const code = parseOperatorAgentErrorCode(err); + return code !== null + ? t(`grants.errors.${code}`) + : t('grants.errors.unknown', { detail: humanizeApiError(err) }); + }, + [t], + ); + + const refresh = useCallback(async (): Promise => { + setBusy(true); + try { + const res = await getAgentGrants(props.slug); + setGrants(res); + setError(null); + } catch (err: unknown) { + setError(localizeError(err)); + } finally { + setBusy(false); + } + }, [props.slug, localizeError]); + + useEffect(() => { + void refresh(); + }, [refresh]); + + return ( +
+
+

{t('grants.heading')}

+ + {t('grants.epochSummary', { + epoch: + grants?.grant_epoch != null + ? formatEpoch(grants.grant_epoch, format) + : t('grants.epochNever'), + })} + +
+ +
+
+

{t('grants.hint')}

+ + {error && ( +
+ {error} +
+ )} + + {!grants && !error && ( +

{t('grants.loading')}

+ )} + + {grants && ( + <> + {grants.tool_grants.length === 0 ? ( +

{t('grants.empty')}

+ ) : ( +
    + {grants.tool_grants.map((row) => ( + + ))} +
+ )} + +

{t('grants.pluginHeading')}

+ {grants.plugin_mcp_grants.length === 0 ? ( +

{t('grants.pluginEmpty')}

+ ) : ( +
    + {grants.plugin_mcp_grants.map((row) => ( +
  • + + {row.plugin_id} + + + → + + + {row.server_name ?? t('grants.serverRemoved')} + + + {t('grants.grantedMeta', { + who: row.granted_by, + date: formatEpoch(row.granted_at, format), + })} + +
    • + ))} +
+ )} + + )} +
+ ); +} + +function ToolGrantRow({ + row, + format, +}: { + readonly row: AgentToolGrantRowDto; + readonly format: Formatter; +}): React.ReactElement { + const t = useTranslations('operatorAgents'); + return ( +
  • + + {t(`grants.kind.${row.tool_kind}`)} + + {row.tool_ref} + {row.tool_kind === 'mcp' && ( + + {row.server_name ?? t('grants.serverRemoved')} + + )} + + {t('grants.epochRow', { + epoch: + row.grant_epoch != null + ? formatEpoch(row.grant_epoch, format) + : t('grants.epochNever'), + })} + +
    • + ); +} + +/** + * Grant epochs are Postgres `now()::text` strings + * (`2026-08-25 18:22:04.123456+02`), not ISO — normalize the separator + * before parsing and fall back to the raw value when the string still does + * not parse. The raw value stays meaningful: epoch staleness is compared + * lexicographically server-side, so showing it verbatim never lies. + */ +function formatEpoch(value: string, format: Formatter): string { + const direct = new Date(value); + const parsed = Number.isNaN(direct.getTime()) + ? new Date(value.replace(' ', 'T')) + : direct; + if (Number.isNaN(parsed.getTime())) return value; + try { + return format.dateTime(parsed, { + year: 'numeric', + month: '2-digit', + day: '2-digit', + hour: '2-digit', + minute: '2-digit', + second: '2-digit', + }); + } catch { + return value; + } +} diff --git a/web-ui/messages/de.json b/web-ui/messages/de.json index ffdf4761..48c41ed3 100644 --- a/web-ui/messages/de.json +++ b/web-ui/messages/de.json @@ -1804,7 +1804,35 @@ "orphanDetachAllTooltip": "Entfernt alle veralteten Plugin-Zuordnungen (alte IDs aus früheren Deployments) auf einmal.", "orphanExplain": "Plugin-IDs aus früheren Seeds, die im aktuellen Katalog nicht mehr existieren (typisch: Umbenennung de.byte5.agent.* → @omadia/*). Werden beim nächsten „Speichern\" verworfen, außer du markierst sie als „Behalten\".", "orphanKeep": "Behalten", - "seededDescription": "Beim ersten Start automatisch angelegt. Empfängt Channel-Traffic ohne explizite Zuordnung, bis du eine einrichtest." + "seededDescription": "Beim ersten Start automatisch angelegt. Empfängt Channel-Traffic ohne explizite Zuordnung, bis du eine einrichtest.", + "grants": { + "heading": "Tool-Grants", + "hint": "Nur-Lese-Ansicht der Tools, die dieser Orchestrator aufrufen darf, und der MCP-Server, die seinen Plugins zugewiesen sind. Grants werden auf der MCP-Admin-Seite verwaltet.", + "loading": "Grants werden geladen…", + "refresh": "Aktualisieren", + "empty": "Dieser Orchestrator hält noch keine Tool-Grants.", + "epochSummary": "Grant-Epoche: {epoch}", + "epochRow": "Epoche: {epoch}", + "epochNever": "nie erhöht", + "serverRemoved": "(Server entfernt)", + "pluginHeading": "Plugin-MCP-Grants", + "pluginEmpty": "Den Plugins dieses Orchestrators sind keine MCP-Server zugewiesen.", + "grantedMeta": "vergeben von {who} · {date}", + "kind": { + "native": "nativ", + "mcp": "MCP" + }, + "errors": { + "agent_graph_store_unavailable": "Die Orchestrator-Registry ist gerade nicht erreichbar. Versuch es gleich noch einmal.", + "config_validation": "Der Server hat die Anfrage abgelehnt.", + "invalid_body": "Der Server hat den Anfrage-Payload abgelehnt.", + "invalid_slug": "Dieser Orchestrator-Slug ist ungültig.", + "multi_orchestrator_unavailable": "Die Multi-Orchestrator-Laufzeit ist nicht verfügbar.", + "not_found": "Diesen Orchestrator gibt es nicht mehr — lade die Seite neu.", + "plugin_not_assigned": "Dieses Plugin ist dem Orchestrator nicht mehr zugewiesen — lade die Seite neu.", + "unknown": "Laden der Grants fehlgeschlagen: {detail}" + } + } }, "builder": { "issueReport": { diff --git a/web-ui/messages/en.json b/web-ui/messages/en.json index 4ba1c9b3..1b366a30 100644 --- a/web-ui/messages/en.json +++ b/web-ui/messages/en.json @@ -1804,7 +1804,35 @@ "orphanDetachAllTooltip": "Detach every stale plugin row (old ids from earlier deployments) at once.", "orphanExplain": "Plugin ids from earlier seeds that no longer exist in the current catalog (typically a rename like de.byte5.agent.* → @omadia/*). Dropped on next Save unless you tick \"Keep\".", "orphanKeep": "Keep", - "seededDescription": "Auto-created on first boot. Receives channel traffic that has no explicit binding, until you configure one." + "seededDescription": "Auto-created on first boot. Receives channel traffic that has no explicit binding, until you configure one.", + "grants": { + "heading": "Tool grants", + "hint": "Read-only view of the tools this orchestrator may call and the MCP servers its plugins were granted. Manage grants on the MCP admin page.", + "loading": "Loading grants…", + "refresh": "Refresh", + "empty": "This orchestrator holds no tool grants yet.", + "epochSummary": "Grant epoch: {epoch}", + "epochRow": "epoch: {epoch}", + "epochNever": "never bumped", + "serverRemoved": "(server removed)", + "pluginHeading": "Plugin MCP grants", + "pluginEmpty": "No MCP servers are granted to this orchestrator's plugins.", + "grantedMeta": "granted by {who} · {date}", + "kind": { + "native": "native", + "mcp": "MCP" + }, + "errors": { + "agent_graph_store_unavailable": "The orchestrator registry is unavailable right now. Try again in a moment.", + "config_validation": "The server rejected the request.", + "invalid_body": "The server rejected the request payload.", + "invalid_slug": "This orchestrator slug is not valid.", + "multi_orchestrator_unavailable": "The multi-orchestrator runtime is not available.", + "not_found": "This orchestrator does not exist anymore — reload the page.", + "plugin_not_assigned": "This plugin is not assigned to the orchestrator anymore — reload the page.", + "unknown": "Loading grants failed: {detail}" + } + } }, "builder": { "issueReport": { From e8f3473316d0cdd3ca86fc9f37a15e2fc82c81cc Mon Sep 17 00:00:00 2001 From: Marcel Wege Date: Tue, 25 Aug 2026 19:41:58 +0200 Subject: [PATCH 09/11] feat(web-ui): surface per-server delegation mode on each agent-MCP assignment (#862, epic #860) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #862 asks for a delegation choice (service vs per_user) per agent-MCP assignment. The W0c schema-fit gate confirmed delegation is stored per SERVER only (mcp_servers.delegation, migration 0031) — there is no per-(agent, server) delegation storage, and creating one would need a migration the spec forbids. Coordinator decision: delegation stays per-server. The assignment row therefore shows the SERVER's delegation mode (header badge + expanded block), explains per_user's fail-closed identity behavior, and lets the operator switch the mode through the existing PUT /mcp-servers/:id/delegation endpoint — confirm-gated by a dialog that names the server-wide effect ("changes it for every agent using this server") before anything is written. Servers from older middleware without a delegation field render no delegation UI. Errors map through the existing MCP grant error-code catalogue (invalid_delegation, mcp_server_not_found); all copy is i18n'd in en+de. --- .../AgentMcpServers.delegation.test.tsx | 167 ++++++++++++++++++ .../[slug]/_components/AgentMcpServers.tsx | 100 +++++++++++ web-ui/messages/de.json | 12 ++ web-ui/messages/en.json | 12 ++ 4 files changed, 291 insertions(+) create mode 100644 web-ui/app/operator/agents/[slug]/__tests__/AgentMcpServers.delegation.test.tsx diff --git a/web-ui/app/operator/agents/[slug]/__tests__/AgentMcpServers.delegation.test.tsx b/web-ui/app/operator/agents/[slug]/__tests__/AgentMcpServers.delegation.test.tsx new file mode 100644 index 00000000..2b016eb0 --- /dev/null +++ b/web-ui/app/operator/agents/[slug]/__tests__/AgentMcpServers.delegation.test.tsx @@ -0,0 +1,167 @@ +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { renderWithIntl } from '../../../../_lib/test-utils'; +import { ApiError } from '../../../../_lib/api'; +import type { McpDelegation, McpServerNode } from '../../../../_lib/agentBuilder'; +import type { AgentGrantsDto } from '../../../../_lib/agents'; +import { AgentMcpServers } from '../_components/AgentMcpServers'; + +/** + * Issue #862 (epic #860) — delegation choice per agent-MCP assignment. + * + * The W0c schema-fit gate decided delegation stays PER SERVER + * (`mcp_servers.delegation`, migration 0031): there is no per-(agent, server) + * delegation storage, so the assignment row shows the SERVER's mode and + * switches it via the existing server-level endpoint — a change with a + * server-wide effect the UI must label before the operator confirms it. + */ + +const { + mockListMcpServers, + mockGetAgentGrants, + mockGetMcpAuthStatus, + mockSetMcpServerDelegation, +} = vi.hoisted(() => ({ + mockListMcpServers: vi.fn(), + mockGetAgentGrants: vi.fn(), + mockGetMcpAuthStatus: vi.fn(), + mockSetMcpServerDelegation: vi.fn(), +})); + +// Spread the real modules so types/parsers (parseMcpGrantErrorCode) stay +// genuine; only the network calls are stubbed. +vi.mock('../../../../_lib/agentBuilder', async (importOriginal) => ({ + ...(await importOriginal()), + listMcpServers: mockListMcpServers, + getMcpAuthStatus: mockGetMcpAuthStatus, + setMcpServerDelegation: mockSetMcpServerDelegation, +})); + +vi.mock('../../../../_lib/agents', async (importOriginal) => ({ + ...(await importOriginal()), + getAgentGrants: mockGetAgentGrants, +})); + +function server(delegation: McpDelegation | undefined): McpServerNode { + return { + id: 'srv-1', + name: 'odoo-mcp', + transport: 'http', + endpoint: 'https://mcp.example/mcp', + status: 'enabled', + lastDiscoveredAt: null, + discoveredTools: [], + ...(delegation !== undefined ? { delegation } : {}), + }; +} + +const NO_GRANTS: AgentGrantsDto = { + slug: 'odoo', + grant_epoch: null, + tool_grants: [], + plugin_mcp_grants: [], +}; + +beforeEach(() => { + vi.clearAllMocks(); + mockListMcpServers.mockResolvedValue({ servers: [server('service')] }); + mockGetAgentGrants.mockResolvedValue(NO_GRANTS); + // Unprotected server → the embedded McpAuthSection renders null. + mockGetMcpAuthStatus.mockResolvedValue({ connected: false, protected: false }); + mockSetMcpServerDelegation.mockResolvedValue({ id: 'srv-1', delegation: 'per_user' }); +}); + +async function renderExpanded(): Promise> { + const user = userEvent.setup(); + renderWithIntl(); + await user.click(await screen.findByRole('button', { name: /odoo-mcp/ })); + await screen.findByText('Identity delegation'); + return user; +} + +describe('AgentMcpServers delegation (#862, per-server by the W0c gate)', () => { + it('shows the server delegation mode with its server-wide scope labelling', async () => { + await renderExpanded(); + + // Mode in the assignment row (badge in the header + expanded block). + expect(screen.getAllByText('Service').length).toBeGreaterThanOrEqual(1); + expect( + screen.getByText( + 'Server-wide setting — it applies to every agent using this server, not just this one.', + ), + ).toBeTruthy(); + }); + + it('shows the fail-closed identity note for a per_user server', async () => { + mockListMcpServers.mockResolvedValue({ servers: [server('per_user')] }); + await renderExpanded(); + + expect( + screen.getByText( + "Per-user calls act under each user's own identity and fail closed when none can be resolved.", + ), + ).toBeTruthy(); + }); + + it('switches the mode only after the server-wide-effect dialog is confirmed', async () => { + const user = await renderExpanded(); + + await user.click(screen.getByRole('button', { name: 'Switch to per user…' })); + // Nothing happens until the operator confirms the global effect. + expect(mockSetMcpServerDelegation).not.toHaveBeenCalled(); + expect( + await screen.findByText( + '"odoo-mcp" delegates identity per server, not per assignment. Switching to Per user changes it for every agent using this server. Continue?', + ), + ).toBeTruthy(); + + // The post-switch refresh serves the flipped mode. + mockListMcpServers.mockResolvedValue({ servers: [server('per_user')] }); + await user.click(screen.getByRole('button', { name: 'Switch mode' })); + + await waitFor(() => + expect(mockSetMcpServerDelegation).toHaveBeenCalledWith('srv-1', 'per_user'), + ); + await waitFor(() => + expect(screen.getByRole('button', { name: 'Switch to service…' })).toBeTruthy(), + ); + }); + + it('does not switch when the dialog is cancelled', async () => { + const user = await renderExpanded(); + + await user.click(screen.getByRole('button', { name: 'Switch to per user…' })); + await user.click(await screen.findByRole('button', { name: 'Cancel' })); + + expect(mockSetMcpServerDelegation).not.toHaveBeenCalled(); + }); + + it('maps a rejected switch to the localized error code message, never the raw body', async () => { + mockSetMcpServerDelegation.mockRejectedValue( + new ApiError(400, 'Bad Request', '{"error":"invalid_delegation"}'), + ); + const user = await renderExpanded(); + + await user.click(screen.getByRole('button', { name: 'Switch to per user…' })); + await user.click(await screen.findByRole('button', { name: 'Switch mode' })); + + expect(await screen.findByText('The server rejected the delegation mode.')).toBeTruthy(); + expect(screen.queryByText('{"error":"invalid_delegation"}')).toBeNull(); + }); + + it('renders no delegation UI when the middleware does not report a mode', async () => { + mockListMcpServers.mockResolvedValue({ servers: [server(undefined)] }); + const user = userEvent.setup(); + renderWithIntl(); + await user.click(await screen.findByRole('button', { name: /odoo-mcp/ })); + + // Older middleware without `delegation` on the server row → no block, no switch. + await screen.findByText( + 'No tools discovered yet — run discovery in the MCP Control Center first.', + ); + expect(screen.queryByText('Identity delegation')).toBeNull(); + expect(screen.queryByRole('button', { name: /Switch to/ })).toBeNull(); + }); +}); diff --git a/web-ui/app/operator/agents/[slug]/_components/AgentMcpServers.tsx b/web-ui/app/operator/agents/[slug]/_components/AgentMcpServers.tsx index 0d1ea6a2..0fd2075c 100644 --- a/web-ui/app/operator/agents/[slug]/_components/AgentMcpServers.tsx +++ b/web-ui/app/operator/agents/[slug]/_components/AgentMcpServers.tsx @@ -13,6 +13,8 @@ import { listMcpServers, parseMcpGrantErrorCode, replaceMcpToolAllowlist, + setMcpServerDelegation, + type McpDelegation, type McpDiscoveredTool, type McpServerNode, } from '../../../../_lib/agentBuilder'; @@ -88,6 +90,13 @@ export function AgentMcpServers({ slug }: { slug: string }): React.ReactElement const [ackArm, setAckArm] = useState(null); const [ackBusy, setAckBusy] = useState(null); const [confirmUnassign, setConfirmUnassign] = useState(null); + /** Pending delegation switch, held until the operator confirms the + * server-wide effect in the dialog. */ + const [confirmDelegation, setConfirmDelegation] = useState<{ + server: McpServerNode; + next: McpDelegation; + } | null>(null); + const [delegationBusy, setDelegationBusy] = useState(null); const refresh = useCallback(async () => { try { @@ -156,6 +165,33 @@ export function AgentMcpServers({ slug }: { slug: string }): React.ReactElement } } + /** Localized name of a delegation mode. */ + const modeLabel = useCallback( + (mode: McpDelegation): string => + mode === 'per_user' ? t('mcp.delegation.modePerUser') : t('mcp.delegation.modeService'), + [t], + ); + + /** + * Delegation is stored PER SERVER (`mcp_servers.delegation`, migration 0031) + * — there is no per-(agent, server) storage, by the W0c schema-fit gate's + * decision (epic #860). Switching it here therefore changes WHOSE identity + * every agent's calls to this server act under, which is why the switch is + * confirm-gated and labelled with its server-wide effect. + */ + async function switchDelegation(server: McpServerNode, next: McpDelegation): Promise { + setDelegationBusy(server.id); + setError(null); + try { + await setMcpServerDelegation(server.id, next); + await refresh(); + } catch (err) { + setError(grantErrorText(err)); + } finally { + setDelegationBusy(null); + } + } + async function ack(server: McpServerNode, toolName: string): Promise { const key = `${server.id} ${toolName}`; setAckBusy(key); @@ -224,6 +260,14 @@ export function AgentMcpServers({ slug }: { slug: string }): React.ReactElement {t('statusDisabled')} ) : null} + {server.delegation !== undefined ? ( + + {modeLabel(server.delegation)} + + ) : null} {grantedSet.size > 0 ? t('mcp.toolsGranted', { @@ -236,6 +280,41 @@ export function AgentMcpServers({ slug }: { slug: string }): React.ReactElement {isOpen ? (
    + {server.delegation !== undefined ? ( +
    +
    + + {t('mcp.delegation.label')} + + + {modeLabel(server.delegation)} + + +
    +
    + {t('mcp.delegation.scopeHint')} +
    + {server.delegation === 'per_user' ? ( +
    + {t('mcp.delegation.perUserFailClosed')} +
    + ) : null} +
    + ) : null} {server.discoveredTools.length === 0 ? (
    {t('mcp.noTools')}
    ) : ( @@ -391,6 +470,27 @@ export function AgentMcpServers({ slug }: { slug: string }): React.ReactElement if (target !== null) void saveAllowlist(target, []); }} /> + setConfirmDelegation(null)} + onConfirm={() => { + const target = confirmDelegation; + setConfirmDelegation(null); + if (target !== null) void switchDelegation(target.server, target.next); + }} + /> ); } diff --git a/web-ui/messages/de.json b/web-ui/messages/de.json index 5c67cae1..d991140d 100644 --- a/web-ui/messages/de.json +++ b/web-ui/messages/de.json @@ -1830,6 +1830,18 @@ "unassignBody": "Alle {count} Tool-Freigaben von \"{server}\" für diesen Orchestrator entziehen?", "unassignConfirm": "Alle entziehen", "cancel": "Abbrechen", + "delegation": { + "label": "Identitäts-Delegation", + "modeService": "Service", + "modePerUser": "Pro Nutzer", + "scopeHint": "Server-weite Einstellung — sie gilt für jeden Agenten, der diesen Server nutzt, nicht nur für diesen.", + "perUserFailClosed": "Pro-Nutzer-Aufrufe laufen unter der Identität des jeweiligen Nutzers und schlagen fail-closed fehl, wenn keine auflösbar ist.", + "switchToService": "Auf Service umstellen…", + "switchToPerUser": "Auf pro Nutzer umstellen…", + "switchTitle": "Delegation für alle Agenten ändern", + "switchBody": "\"{server}\" delegiert Identität pro Server, nicht pro Zuweisung. Die Umstellung auf {mode} gilt für jeden Agenten, der diesen Server nutzt. Fortfahren?", + "switchConfirm": "Modus umstellen" + }, "errors": { "config_validation": "Der Scan-Gate hat die Änderung abgelehnt — ein ausgewähltes Tool ist blockiert oder noch nicht gescannt.", "invalid_delegation": "Der Server hat den Delegationsmodus abgelehnt.", diff --git a/web-ui/messages/en.json b/web-ui/messages/en.json index 1ee771b7..9c472304 100644 --- a/web-ui/messages/en.json +++ b/web-ui/messages/en.json @@ -1830,6 +1830,18 @@ "unassignBody": "Revoke all {count} tool grants of \"{server}\" for this orchestrator?", "unassignConfirm": "Revoke all", "cancel": "Cancel", + "delegation": { + "label": "Identity delegation", + "modeService": "Service", + "modePerUser": "Per user", + "scopeHint": "Server-wide setting — it applies to every agent using this server, not just this one.", + "perUserFailClosed": "Per-user calls act under each user's own identity and fail closed when none can be resolved.", + "switchToService": "Switch to service…", + "switchToPerUser": "Switch to per user…", + "switchTitle": "Change delegation for every agent", + "switchBody": "\"{server}\" delegates identity per server, not per assignment. Switching to {mode} changes it for every agent using this server. Continue?", + "switchConfirm": "Switch mode" + }, "errors": { "config_validation": "The scan gate rejected the change — a selected tool is blocked or not scanned yet.", "invalid_delegation": "The server rejected the delegation mode.", From bb016aa2855ff8e084d3f282ba64a2a67e77a315 Mon Sep 17 00:00:00 2001 From: Marcel Wege Date: Tue, 25 Aug 2026 19:56:52 +0200 Subject: [PATCH 10/11] feat(web-ui): wire per-agent grant and MCP surfaces into the agent detail page (W0c wiring, #860) Integration unit for wave W0c-agent-grants-ui: - merge all nine sibling unit branches (store reads, operator/agent-builder endpoints, _lib types+callers, detail route, tool-grant list, MCP assignment + delegation surfacing, schema-fit gate) onto one branch; resolve the messages/en.json + de.json namespace collision by keeping both new operatorAgents.grants and operatorAgents.mcp sub-namespaces - mount AgentToolGrants (#861) and AgentMcpServers (#862) below the plugin editor in /operator/agents/[slug]/page.tsx - link every dashboard agent card to its detail route (new operatorAgents.detailOpenLink key in en+de; 'Details' is a German loanword, allowlisted as such for the identical-value gate) - middleware/src/index.ts needs no diff: the new routes hang off createOperatorAgentsRouter/createAgentBuilderRouter, both already mounted behind requireAuth (index.ts:3158/:3240) Gates: web-ui lint+typecheck+vitest (870 passed) + i18n:check (3871 keys) + i18n-parity; middleware operatorAgentsRouter/agentGrantsStore/ mcpToolGuard/mcpGrantPolicy tests pass isolated. --- web-ui/app/operator/agents/[slug]/page.tsx | 10 ++++++++++ .../operator/agents/_components/AgentsDashboard.tsx | 12 +++++++++++- web-ui/messages/de.json | 1 + web-ui/messages/en.json | 1 + web-ui/scripts/i18n-identical-allowlist.json | 1 + 5 files changed, 24 insertions(+), 1 deletion(-) diff --git a/web-ui/app/operator/agents/[slug]/page.tsx b/web-ui/app/operator/agents/[slug]/page.tsx index ba35705d..3bf73df7 100644 --- a/web-ui/app/operator/agents/[slug]/page.tsx +++ b/web-ui/app/operator/agents/[slug]/page.tsx @@ -10,6 +10,8 @@ import { type OperatorAgentsListDto, } from '../../../_lib/agents'; import { AgentDetail } from './_components/AgentDetail'; +import { AgentMcpServers } from './_components/AgentMcpServers'; +import { AgentToolGrants } from './_components/AgentToolGrants'; /** * Issue #861 — per-agent capability page (epic #860). @@ -101,6 +103,14 @@ export default async function OperatorAgentDetailPage({ agent.id === list.fallback_agent_id } /> + {/* Wiring (#860): the sibling per-agent surfaces — read-only + tool-grant list (#861) and the MCP assignment / allowlist + editor (#862) — mount below the plugin editor. Both are + client components that fetch their own data by slug. */} +
    + + +
    )} diff --git a/web-ui/app/operator/agents/_components/AgentsDashboard.tsx b/web-ui/app/operator/agents/_components/AgentsDashboard.tsx index 717e0853..ac425ea5 100644 --- a/web-ui/app/operator/agents/_components/AgentsDashboard.tsx +++ b/web-ui/app/operator/agents/_components/AgentsDashboard.tsx @@ -1,6 +1,7 @@ 'use client'; import { useEffect, useMemo, useState, useTransition } from 'react'; +import Link from 'next/link'; import { useRouter } from 'next/navigation'; import { useTranslations } from 'next-intl'; @@ -542,7 +543,16 @@ function AgentCard(props: {
    -
    +
    + {/* Wiring (#860): the per-agent capability page (plugins, tool + grants, MCP assignments) lives on its own route — the card + links there instead of growing more inline editors. */} + + {t('detailOpenLink')} + {/* The standard orchestrator is the catch-all for unbound traffic; disabling or deleting it would strand that traffic, so those actions are not offered for it at all. */} diff --git a/web-ui/messages/de.json b/web-ui/messages/de.json index 7dc9daed..5c0eed9b 100644 --- a/web-ui/messages/de.json +++ b/web-ui/messages/de.json @@ -1739,6 +1739,7 @@ "agentsEmpty": "Noch keine Orchestratoren. Leg oben einen an.", "agentMeta": "id: {id} · privacy: {privacy} · status: {status} · runtime: {runtime}", "detailMetaTitle": "Orchestrator: {slug} · omadia", + "detailOpenLink": "Details", "detailBackToList": "Alle Orchestratoren", "detailAssignedHeading": "Zugewiesene Plugins", "detailAssignedHint": "Plugin nur für diesen Orchestrator aktivieren oder deaktivieren — der Schalter wird sofort gespeichert. Plugins anhängen oder lösen geht im Editor darunter.", diff --git a/web-ui/messages/en.json b/web-ui/messages/en.json index 3eb69d3b..9a9f1ebb 100644 --- a/web-ui/messages/en.json +++ b/web-ui/messages/en.json @@ -1739,6 +1739,7 @@ "agentsEmpty": "No orchestrators yet. Create one above.", "agentMeta": "id: {id} · privacy: {privacy} · status: {status} · runtime: {runtime}", "detailMetaTitle": "Orchestrator: {slug} · omadia", + "detailOpenLink": "Details", "detailBackToList": "All orchestrators", "detailAssignedHeading": "Assigned plugins", "detailAssignedHint": "Enable or disable a plugin for this orchestrator only — the switch is saved immediately. Attach or detach plugins in the editor below.", diff --git a/web-ui/scripts/i18n-identical-allowlist.json b/web-ui/scripts/i18n-identical-allowlist.json index b6187ed2..4071f623 100644 --- a/web-ui/scripts/i18n-identical-allowlist.json +++ b/web-ui/scripts/i18n-identical-allowlist.json @@ -49,6 +49,7 @@ "agentPicker.label": "glossary", "operatorAgents.agentMeta": "diagnostic", "operatorAgents.detailMetaTitle": "brand", + "operatorAgents.detailOpenLink": "loanword", "builder.simple.persona.optional": "loanword", "builder.workspace.buildBusLive": "loanword", "builder.spec.setupFields.required.off": "loanword", From c16da1c7f12722674f71130a54adaa2eaaecdc04 Mon Sep 17 00:00:00 2001 From: Marcel Wege Date: Tue, 25 Aug 2026 20:31:01 +0200 Subject: [PATCH 11/11] =?UTF-8?q?fix:=20W0c=20coordinator=20fixes=20?= =?UTF-8?q?=E2=80=94=20wire=20grants=20store,=20harden=20gate,=20single=20?= =?UTF-8?q?delegation=20surface,=20atomic=20allowlist?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integration fixes for the W0c wave (#860, #861, #862) addressing every major/blocker review finding: - index.ts: pass getAgentGraphStore to createOperatorAgentsRouter (every GET /:slug/grants 503ed without it) + static wiring pin in the router test - schema-fit gate: substring /delegation/i matcher (word-boundary regex was blind to delegation_mode — synthetic-0049 evasion now pinned by a self-test), sibling migration dirs scanned, and a source scan that forbids per-assignment delegation through agent_tool_grants.config JSONB (the no-DDL side door), recorded on the McpDelegation doc comment - listToolGrantsForAgent: include sub-agent-held grants (XOR table), matching the graph readers' attribution rule; sub_agent_id stays meaningful - PUT /mcp-grants: revoke deletes EVERY row behind a normalized tool name (raw + serverName-prefixed refs), gate runs only over diff.toGrant, and all writes go through one transactional applyMcpToolAllowlist; createEdge now persists the normalized ref; new route tests (11) cover all of it - GET /:slug/grants: tool_ref normalized via mcpToolNameFromRef so the UI can compare against discovered tool names verbatim - AgentMcpServers: delegation is READ-ONLY with server-wide-effect label + link to /admin/mcp (coordinator ruling for #862); McpAuthSection's own toggle suppressed on this page (showDelegation prop); sub-agent grants excluded from the editable allowlist - AgentToolGrants: epoch heading distinguishes unknown (loading/failed) from "never bumped" - AgentDetail: plugin toggle no longer remounts PluginsDnd (editorRevision counter) — unsaved editor drafts survive an instant toggle - web _lib: ToolGrantNode drops the phantom grantEpoch field; DTO docs pin the normalization + sub-agent contracts --- .../src/registry/agentGraphStore.ts | 79 +++- middleware/src/index.ts | 5 + middleware/src/routes/agentBuilder.ts | 63 +-- middleware/src/routes/operatorAgents.ts | 24 +- middleware/test/agentGrantsStore.test.ts | 88 ++++- .../mcpDelegationBackfillMigration.pg.test.ts | 114 +++++- middleware/test/mcpGrantsRoute.test.ts | 369 ++++++++++++++++++ middleware/test/operatorAgentsRouter.test.ts | 118 +++++- web-ui/app/_components/mcp/McpAuthSection.tsx | 9 +- web-ui/app/_lib/__tests__/agentGrants.test.ts | 18 +- web-ui/app/_lib/agentBuilder.ts | 9 +- web-ui/app/_lib/agents.ts | 17 +- .../[slug]/__tests__/AgentDetail.test.tsx | 93 ++++- .../AgentMcpServers.delegation.test.tsx | 78 ++-- .../[slug]/__tests__/AgentMcpServers.test.tsx | 29 ++ .../[slug]/__tests__/AgentToolGrants.test.tsx | 29 ++ .../agents/[slug]/_components/AgentDetail.tsx | 39 +- .../[slug]/_components/AgentMcpServers.tsx | 89 ++--- .../[slug]/_components/AgentToolGrants.tsx | 19 +- web-ui/messages/de.json | 7 +- web-ui/messages/en.json | 7 +- 21 files changed, 1087 insertions(+), 216 deletions(-) create mode 100644 middleware/test/mcpGrantsRoute.test.ts diff --git a/middleware/packages/harness-orchestrator/src/registry/agentGraphStore.ts b/middleware/packages/harness-orchestrator/src/registry/agentGraphStore.ts index a0db1894..76db67ff 100644 --- a/middleware/packages/harness-orchestrator/src/registry/agentGraphStore.ts +++ b/middleware/packages/harness-orchestrator/src/registry/agentGraphStore.ts @@ -231,11 +231,18 @@ export interface McpServerRow { * `agent_tool_grants`, not on `plugin_mcp_grants`, no bridge table. A * per-agent assignment view therefore shows the server's mode READ-ONLY and * links to the per-server setting; changing it there applies to every agent - * holding a grant on that server and must say so. Widening this to - * per-assignment would need a new column/table plus resolution semantics in - * `resolveMcpUserKey` — that is a design decision with its own migration, not - * something a UI unit may introduce as a side effect. Guarded by the W0c - * schema-fit gate in `middleware/test/mcpDelegationBackfillMigration.pg.test.ts`. */ + * holding a grant on that server and must say so. + * + * The real escape hatch is NOT a new column: `agent_tool_grants.config` + * (JSONB, migration 0003) could hold a `delegation` key today with zero DDL, + * which is why an SQL-only migration scan cannot see a widening. Storing any + * `config.delegation*` key on a grant row is therefore FORBIDDEN by + * convention — per-assignment delegation needs resolution semantics in + * `resolveMcpUserKey` and its own design pass, not a JSONB side door. Guarded + * twice by the W0c schema-fit gate in + * `middleware/test/mcpDelegationBackfillMigration.pg.test.ts`: a migration + * scan over the grant tables AND a source scan that fails on any code + * reading or writing a delegation key out of grant config. */ export type McpDelegation = 'per_user' | 'service'; /** @@ -2355,17 +2362,71 @@ export class AgentGraphStore { /** Agent-scoped read of `agent_tool_grants` (W0c, #861): the grants of ONE * agent, for the agent detail page. Same row shape as `listAllToolGrants` - * (grant epoch included via `grantEpoch`); uses the - * `agent_tool_grants_agent_idx` index from migration 0003. SELECT-only by - * construction (no DDL, no writes). */ + * (grant epoch included via `grantEpoch`). `agent_tool_grants` is a XOR + * table (0003: `agent_id` OR `subagent_id`), and the codebase attributes a + * sub-agent-held grant to its parent agent everywhere the graph is read + * (`assembleGraph`, `indexGraph.grantsByAgent`, the graph-signature + * filter) — this read matches that rule: rows held directly by the agent + * PLUS rows held by its sub-agents, distinguishable via `subAgentId`. + * SELECT-only by construction (no DDL, no writes). */ async listToolGrantsForAgent(agentId: string): Promise { const { rows } = await this.pool.query( - 'SELECT * FROM agent_tool_grants WHERE agent_id = $1 ORDER BY created_at', + `SELECT * FROM agent_tool_grants + WHERE agent_id = $1 + OR subagent_id IN (SELECT id FROM agent_subagents WHERE parent_agent_id = $1) + ORDER BY created_at`, [agentId], ); return rows.map(mapToolGrant); } + /** + * Transactional bulk edit of an agent's MCP tool allowlist for one server + * (W0c, #862). `PUT /mcp-grants` in allowlist mode is N creates + M deletes; + * done through the single-row methods a mid-edit failure would leave the + * persisted allowlist neither old nor new. Here every write shares one + * transaction: a partial failure rolls back and the route's granted/revoked + * response always describes what actually persisted. The INSERT keeps + * `createToolGrant`'s ON CONFLICT no-op contract (unique index 0014). + */ + async applyMcpToolAllowlist(input: { + readonly agentId: string; + readonly mcpServerId: string; + /** Normalized tool names to grant (rows to INSERT). */ + readonly grantRefs: readonly string[]; + /** Grant row ids to revoke (rows to DELETE). */ + readonly revokeIds: readonly string[]; + }): Promise { + const client = await this.pool.connect(); + try { + await client.query('BEGIN'); + for (const toolRef of input.grantRefs) { + await client.query( + `INSERT INTO agent_tool_grants + (agent_id, subagent_id, tool_kind, tool_ref, mcp_server_id, config) + VALUES ($1,NULL,'mcp',$2,$3,'{}'::jsonb) + ON CONFLICT (agent_id, mcp_server_id, tool_ref) + WHERE agent_id IS NOT NULL AND tool_kind = 'mcp' + DO NOTHING`, + [input.agentId, toolRef, input.mcpServerId], + ); + } + for (const id of input.revokeIds) { + await client.query('DELETE FROM agent_tool_grants WHERE id = $1', [id]); + } + await client.query('COMMIT'); + } catch (err) { + try { + await client.query('ROLLBACK'); + } catch { + // connection-level failure — the pool discards the client below. + } + throw err; + } finally { + client.release(); + } + } + async createToolGrant(input: ToolGrantInput): Promise { if (!input.agentId && !input.subAgentId) { throw new ConfigValidationError( diff --git a/middleware/src/index.ts b/middleware/src/index.ts index e79eae36..a9621158 100644 --- a/middleware/src/index.ts +++ b/middleware/src/index.ts @@ -3166,6 +3166,11 @@ async function main(): Promise { getChatSessionStore, getPluginCatalog: () => pluginCatalog, getInstalledRegistry: () => installedRegistry, + // W0c (#861) — the per-agent grant read model needs the graph store. + // Same graphPool-guarded shape as the other AgentGraphStore sites; when + // no DATABASE_URL is set the route degrades to its own 503. + getAgentGraphStore: () => + graphPool ? new AgentGraphStore(graphPool) : undefined, }), ); console.log( diff --git a/middleware/src/routes/agentBuilder.ts b/middleware/src/routes/agentBuilder.ts index d93b48a7..a7953d3d 100644 --- a/middleware/src/routes/agentBuilder.ts +++ b/middleware/src/routes/agentBuilder.ts @@ -1878,13 +1878,11 @@ export function createAgentBuilderRouter( res.status(404).json({ error: 'mcp_server_not_found' }); return; } - // Gate first, write after: every candidate passes `assertMcpToolAllowed` - // (the shared fail-closed gate) and comes back as its normalized name. + // Normalize first, gate later. `mcpToolNameFromRef` strips an optional + // ':' prefix — the same normalization `assertMcpToolAllowed` + // applies — so the diff below compares names, never raw refs. const desiredRefs = hasSingle ? [singleRef] : (listRefs as string[]); - const desired: string[] = []; - for (const ref of desiredRefs) { - desired.push(await assertMcpToolAllowed(l, mcpServerId, ref)); - } + const desired = desiredRefs.map((ref) => mcpToolNameFromRef(ref, server.name)); const currentGrants = (await l.graph.listAllToolGrants()).filter( (g) => g.toolKind === 'mcp' && @@ -1892,28 +1890,45 @@ export function createAgentBuilderRouter( g.agentId === agent.id && g.subAgentId === null, ); - const grantIdByTool = new Map( - currentGrants.map((g) => [mcpToolNameFromRef(g.toolRef, server.name), g.id]), - ); - const diff = diffMcpToolAllowlist([...grantIdByTool.keys()], desired); + // The unique index (0014) keys on the RAW tool_ref, so two persisted + // rows (e.g. 'search' and 'odoo-mcp:search') can normalize to the SAME + // tool name. A revoke must delete EVERY row behind the name — the map + // therefore holds id LISTS, not single ids. + const grantIdsByTool = new Map(); + for (const g of currentGrants) { + const name = mcpToolNameFromRef(g.toolRef, server.name); + const ids = grantIdsByTool.get(name); + if (ids) ids.push(g.id); + else grantIdsByTool.set(name, [g.id]); + } + const diff = diffMcpToolAllowlist([...grantIdsByTool.keys()], desired); // Single-grant mode stays additive (historical contract); only the // allowlist mode revokes what fell off the list. const toRevoke = hasList ? diff.toRevoke : []; + // Gate only what would be WRITTEN (`diff.toGrant`). Already-granted + // rows passed the gate at creation, and a re-discover can leave one of + // them "granted but not currently callable" (stale ack) — the dispatch + // guard in mcpGrantPolicy blocks such a tool at call time regardless, + // and refusing an unrelated clean edit because of it would jam the + // editor exactly when the operator needs it. One gate rejection still + // aborts the whole edit BEFORE any write. for (const toolName of diff.toGrant) { - // createToolGrant is idempotent for top-level MCP grants (ON CONFLICT - // via migration 0014), so a repeat is a clean no-op. - await l.graph.createToolGrant({ + await assertMcpToolAllowed(l, mcpServerId, toolName); + } + const revokeIds = toRevoke.flatMap( + (toolName) => grantIdsByTool.get(toolName) ?? [], + ); + if (diff.toGrant.length > 0 || revokeIds.length > 0) { + // One store-level transaction for the whole edit: a partial failure + // rolls back, so `granted`/`revoked` in the response always describe + // what actually persisted. + await l.graph.applyMcpToolAllowlist({ agentId: agent.id, - subAgentId: null, - toolKind: 'mcp', - toolRef: toolName, mcpServerId, + grantRefs: diff.toGrant, + revokeIds, }); } - for (const toolName of toRevoke) { - const grantId = grantIdByTool.get(toolName); - if (grantId) await l.graph.deleteToolGrant(grantId); - } if (delegation !== undefined && delegation !== server.delegation) { await l.graph.setMcpServerDelegation(mcpServerId, delegation); } @@ -2560,7 +2575,7 @@ async function createEdge( const onAgent = source.startsWith('agent:'); const subAgentId = onAgent ? null : idAfter(source, 'subagent'); const toolKind = (config['toolKind'] as 'native' | 'mcp') ?? 'native'; - const toolRef = String(config['toolRef'] ?? idAfter(target, 'tool')); + let toolRef = String(config['toolRef'] ?? idAfter(target, 'tool')); const mcpServerId = (config['mcpServerId'] as string | null) ?? null; if (!toolRef) { throw new ConfigValidationError('tool_grant requires a toolRef'); @@ -2571,7 +2586,11 @@ async function createEdge( if (!mcpServerId) { throw new ConfigValidationError('mcp tool_grant requires an mcpServerId'); } - await assertMcpToolAllowed(l, mcpServerId, toolRef); + // Persist the NORMALIZED name the gate returns, not the caller's raw + // ref: a ':'-prefixed ref would land as its own row beside + // the bare-name grant (the 0014 unique index keys on the raw ref), + // splitting one tool across two rows (W0c review). + toolRef = await assertMcpToolAllowed(l, mcpServerId, toolRef); } const grant = await l.graph.createToolGrant({ agentId: onAgent ? agent.id : null, diff --git a/middleware/src/routes/operatorAgents.ts b/middleware/src/routes/operatorAgents.ts index fca6d81d..c27a1fcb 100644 --- a/middleware/src/routes/operatorAgents.ts +++ b/middleware/src/routes/operatorAgents.ts @@ -6,6 +6,7 @@ import { attachAllPlugins, ConfigValidationError, FALLBACK_AGENT_SLUG, + mcpToolNameFromRef, type AgentGraphStore, type ChatSessionStore, type ConfigStore, @@ -456,11 +457,16 @@ export function createOperatorAgentsRouter( }); // ── per-agent grant read model (W0c, #861) ────────────────────────── - // One response for the agent detail page: the agent's own - // `agent_tool_grants` rows (grant epoch included — bumpMcpGrantEpoch - // stamps config.verdictEpoch, surfaced as `grant_epoch`) plus the - // `plugin_mcp_grants` of every plugin assigned to the agent. Read-only; - // grant WRITES stay on the agent-builder router (/api/v1/operator/mcp-grants). + // One response for the agent detail page: the agent's `agent_tool_grants` + // rows — held directly OR by one of its sub-agents, matching the graph + // reads' attribution rule; `sub_agent_id` tells the rows apart — (grant + // epoch included — bumpMcpGrantEpoch stamps config.verdictEpoch, surfaced + // as `grant_epoch`) plus the `plugin_mcp_grants` of every plugin assigned + // to the agent. `tool_ref` is normalized via `mcpToolNameFromRef` (the + // stored ref may carry a ':' prefix; every other reader + // normalizes too) so clients can compare it against discovered tool names + // verbatim. Read-only; grant WRITES stay on the agent-builder router + // (/api/v1/operator/mcp-grants). router.get('/:slug/grants', async (req: Request, res: Response) => { const live = svc(); if (!live) return unavailable(res); @@ -502,7 +508,13 @@ export function createOperatorAgentsRouter( tool_grants: toolGrants.map((g) => ({ id: g.id, tool_kind: g.toolKind, - tool_ref: g.toolRef, + tool_ref: + g.toolKind === 'mcp' && g.mcpServerId + ? mcpToolNameFromRef( + g.toolRef, + serverById.get(g.mcpServerId)?.name ?? '', + ) + : g.toolRef, sub_agent_id: g.subAgentId, mcp_server_id: g.mcpServerId, server_name: g.mcpServerId diff --git a/middleware/test/agentGrantsStore.test.ts b/middleware/test/agentGrantsStore.test.ts index 3ec9a68a..c0be61a5 100644 --- a/middleware/test/agentGrantsStore.test.ts +++ b/middleware/test/agentGrantsStore.test.ts @@ -57,7 +57,7 @@ function toolGrantDbRow(overrides: Record = {}): Record { +test('listToolGrantsForAgent reads the agent AND its sub-agents, ordered by created_at', async () => { const { pool, calls } = fakePool([toolGrantDbRow()]); const store = new AgentGraphStore(pool); const rows = await store.listToolGrantsForAgent(AGENT_ID); @@ -66,6 +66,15 @@ test('listToolGrantsForAgent selects only the given agent, ordered by created_at const { sql, params } = calls[0]!; assert.match(sql, /FROM agent_tool_grants/); assert.match(sql, /WHERE agent_id = \$1/); + // agent_tool_grants is a XOR table (0003): a sub-agent-held grant has + // agent_id NULL. The read must attribute those rows to the parent agent, + // matching assembleGraph/indexGraph — an agent_id-only filter silently + // hides sub-agent grants from the detail page (W0c review). + assert.match( + sql, + /subagent_id IN \(SELECT id FROM agent_subagents WHERE parent_agent_id = \$1\)/, + 'sub-agent-held grants must be attributed to the parent agent', + ); assert.match(sql, /ORDER BY created_at/); assert.deepEqual(params, [AGENT_ID]); @@ -75,6 +84,17 @@ test('listToolGrantsForAgent selects only the given agent, ordered by created_at assert.equal(rows[0]!.mcpServerId, SERVER_ID); }); +test('listToolGrantsForAgent maps a sub-agent-held row (agent_id NULL) faithfully', async () => { + const SUB_AGENT_ID = '00000000-0000-0000-0000-0000000000cc'; + const { pool } = fakePool([ + toolGrantDbRow({ agent_id: null, subagent_id: SUB_AGENT_ID }), + ]); + const store = new AgentGraphStore(pool); + const [row] = await store.listToolGrantsForAgent(AGENT_ID); + assert.equal(row!.agentId, null); + assert.equal(row!.subAgentId, SUB_AGENT_ID, 'sub_agent attribution must survive the mapper'); +}); + test('listToolGrantsForAgent is SELECT-only — never writes', async () => { const { pool, calls } = fakePool(); const store = new AgentGraphStore(pool); @@ -169,3 +189,69 @@ test('listPluginMcpGrants maps rows through the same named shape', async () => { assert.equal(row!.mcpServerId, SERVER_ID); assert.equal(row!.grantedBy, 'operator@example.com'); }); + +// ── applyMcpToolAllowlist (transactional bulk edit, W0c #862) ─────────────── + +/** Pool whose `connect()` hands out a capturing client — `applyMcpToolAllowlist` + * must run every write on ONE client inside BEGIN/COMMIT. */ +function fakeClientPool(failOn?: RegExp): { + pool: Pool; + calls: string[]; + released: { value: boolean }; +} { + const calls: string[] = []; + const released = { value: false }; + const client = { + query: async (sql: string, _params?: unknown[]) => { + calls.push(sql); + if (failOn && failOn.test(sql)) throw new Error(`boom on ${sql.slice(0, 30)}`); + return { rows: [] }; + }, + release: () => { + released.value = true; + }, + }; + const pool = { + connect: async () => client, + query: async () => { + throw new Error('applyMcpToolAllowlist must not use pool.query — writes belong on the transaction client'); + }, + } as unknown as Pool; + return { pool, calls, released }; +} + +test('applyMcpToolAllowlist wraps every grant and revoke in one BEGIN/COMMIT', async () => { + const { pool, calls, released } = fakeClientPool(); + const store = new AgentGraphStore(pool); + await store.applyMcpToolAllowlist({ + agentId: AGENT_ID, + mcpServerId: SERVER_ID, + grantRefs: ['read_partners', 'search'], + revokeIds: ['00000000-0000-0000-0000-0000000000aa'], + }); + assert.equal(calls[0], 'BEGIN'); + assert.equal(calls.at(-1), 'COMMIT'); + const inserts = calls.filter((c) => /INSERT INTO agent_tool_grants/.test(c)); + const deletes = calls.filter((c) => /DELETE FROM agent_tool_grants/.test(c)); + assert.equal(inserts.length, 2); + assert.equal(deletes.length, 1); + assert.match(inserts[0]!, /ON CONFLICT/, 'keeps createToolGrant\'s idempotent contract (0014)'); + assert.equal(released.value, true, 'client returned to the pool'); +}); + +test('applyMcpToolAllowlist rolls back the whole edit when one write fails', async () => { + const { pool, calls, released } = fakeClientPool(/DELETE FROM/); + const store = new AgentGraphStore(pool); + await assert.rejects( + store.applyMcpToolAllowlist({ + agentId: AGENT_ID, + mcpServerId: SERVER_ID, + grantRefs: ['read_partners'], + revokeIds: ['00000000-0000-0000-0000-0000000000aa'], + }), + /boom/, + ); + assert.equal(calls.at(-1), 'ROLLBACK', 'a partial edit must not persist'); + assert.ok(!calls.includes('COMMIT')); + assert.equal(released.value, true, 'client returned to the pool even on failure'); +}); diff --git a/middleware/test/mcpDelegationBackfillMigration.pg.test.ts b/middleware/test/mcpDelegationBackfillMigration.pg.test.ts index 40ab561d..5fec4470 100644 --- a/middleware/test/mcpDelegationBackfillMigration.pg.test.ts +++ b/middleware/test/mcpDelegationBackfillMigration.pg.test.ts @@ -350,6 +350,15 @@ describe('migration 0031 — delegation backfill predicate (pg)', { skip: !pgAva describe('W0c schema-fit gate — delegation stays per-server (#860)', () => { const MIGRATIONS_DIR = new URL('../migrations/', import.meta.url); + /** SUBSTRING match on purpose, not `\bdelegation\b`: `_` is a word + * character, so a word-boundary regex never fires after `delegation` in + * `delegation_mode`, `mcp_delegation` or `delegation_override` — the most + * natural names for a per-assignment column, and exactly the evasion the + * W0c review demonstrated with a synthetic migration. `/delegation/i` + * matches only 0031 in today's tree, so the tightening costs nothing + * (`/delegat/` would over-match 0006/0015). */ + const DELEGATION_ANYWHERE = /delegation/i; + async function executableMigrations(): Promise> { const names = (await readdir(MIGRATIONS_DIR)).filter((n) => n.endsWith('.sql')).sort(); const entries = await Promise.all( @@ -364,7 +373,7 @@ describe('W0c schema-fit gate — delegation stays per-server (#860)', () => { it('no migration but 0031 touches `delegation` — no per-assignment storage exists', async () => { const migrations = await executableMigrations(); const mentioning = [...migrations] - .filter(([, sql]) => /\bdelegation\b/i.test(sql)) + .filter(([, sql]) => DELEGATION_ANYWHERE.test(sql)) .map(([name]) => name); assert.deepEqual( mentioning, @@ -394,7 +403,7 @@ describe('W0c schema-fit gate — delegation stays per-server (#860)', () => { } assert.doesNotMatch( createTable, - /\bdelegation\b/i, + DELEGATION_ANYWHERE, 'agent_tool_grants gained a delegation column — per-assignment delegation needs its own design pass, not this table', ); }); @@ -403,7 +412,7 @@ describe('W0c schema-fit gate — delegation stays per-server (#860)', () => { const sql = (await executableMigrations()).get('0012_plugin_mcp_grants.sql'); assert.ok(sql, 'migration 0012 is missing'); assert.match(sql, /PRIMARY KEY \(plugin_id, mcp_server_id\)/); - assert.doesNotMatch(sql, /\bdelegation\b/i); + assert.doesNotMatch(sql, DELEGATION_ANYWHERE); }); it('0014 pins the grant identity the per-agent UI builds on: (agent_id, mcp_server_id, tool_ref)', async () => { @@ -415,4 +424,103 @@ describe('W0c schema-fit gate — delegation stays per-server (#860)', () => { 'the top-level MCP grant identity changed — #861/#862 assignment semantics must be re-reviewed', ); }); + + it('the gate matcher catches the demonstrated `delegation_mode` evasion (synthetic 0049)', () => { + // The W0c review injected exactly this migration into a copy of the tree + // and all five gate tests PASSED, because `\bdelegation\b` never matches + // `delegation_mode` (`_` is a word character). This self-test pins the + // hardened matcher against that evasion and its neighbours — if someone + // relaxes DELEGATION_ANYWHERE back to word boundaries, this fails first. + const synthetic0049 = [ + "ALTER TABLE agent_tool_grants ADD COLUMN delegation_mode TEXT NOT NULL DEFAULT 'inherit';", + "ALTER TABLE agent_tool_grants ADD CONSTRAINT agent_tool_grants_delegation_mode_chk", + " CHECK (delegation_mode IN ('inherit', 'service', 'per_user'));", + ].join('\n'); + assert.match(synthetic0049, DELEGATION_ANYWHERE, 'the gate matcher no longer sees delegation_mode DDL'); + assert.doesNotMatch( + synthetic0049, + /\bdelegation\b/i, + 'sanity: the OLD word-boundary matcher really was blind to this DDL — if this fires, the fixture drifted', + ); + for (const evasion of ['mcp_delegation', 'agent_delegation_override', 'DELEGATION_MODE']) { + assert.match(`ADD COLUMN ${evasion} TEXT`, DELEGATION_ANYWHERE, `matcher misses "${evasion}"`); + } + }); + + it('the sibling migration dirs cannot smuggle grant-table or delegation DDL past the scan', async () => { + // `executableMigrations()` reads only `middleware/migrations/`. Two more + // live migration series exist (`src/services/graph/migrations/`, + // `packages/harness-knowledge-graph-neon/src/migrations/`) that the scan + // above never sees — DDL added there would be invisible to the gate. Pin + // that they stay out of the delegation/grant-table business entirely, so + // `middleware/migrations/` remains the single place such DDL can appear + // (where the tests above catch it). + const SIBLING_DIRS = [ + new URL('../src/services/graph/migrations/', import.meta.url), + new URL('../packages/harness-knowledge-graph-neon/src/migrations/', import.meta.url), + ]; + for (const dir of SIBLING_DIRS) { + const names = (await readdir(dir)).filter((n) => n.endsWith('.sql')); + assert.ok(names.length > 0, `no .sql files under ${dir.pathname} — dir moved? update the gate`); + for (const name of names) { + const sql = stripWholeLineSqlComments(await readFile(new URL(name, dir), 'utf8')); + assert.doesNotMatch( + sql, + DELEGATION_ANYWHERE, + `${dir.pathname}${name} mentions delegation — that DDL belongs in middleware/migrations/ where the gate scans it`, + ); + assert.doesNotMatch( + sql, + /agent_tool_grants|plugin_mcp_grants/i, + `${dir.pathname}${name} touches a grant table — that DDL belongs in middleware/migrations/ where the gate scans it`, + ); + } + } + }); + + it('no source code reads or writes a delegation key out of grant config JSONB (the no-DDL side door)', async () => { + // `agent_tool_grants.config` (JSONB, 0003) could hold per-assignment + // delegation with ZERO migrations — the one widening no SQL scan can see. + // The convention (recorded on `McpDelegation` in agentGraphStore.ts) is + // that storing any `config.delegation*` key on a grant row is forbidden; + // this scan enforces it across the middleware and the orchestrator + // package. Comment lines are stripped so prose may explain the rule. + const SRC_ROOTS = [ + new URL('../src/', import.meta.url), + new URL('../packages/harness-orchestrator/src/', import.meta.url), + ]; + const FORBIDDEN = [ + // config.delegation / config?.delegation / config['delegation'] / config["delegation"] + /config\??\.(delegation)|config\?*\[[`'"]delegation/i, + // SQL JSONB access: config->'delegation', config->>'delegation…' + /config\s*->>?\s*'delegation/i, + // SQL JSONB write: jsonb_set(config, '{delegation…}') + /jsonb_set\([^)]*config[^)]*delegation/i, + // object literal delegation key flowing into a grant config param + /delegation[a-zA-Z]*\s*:[^,\n]*\}\s*as\s*ToolGrantInput/i, + ]; + for (const root of SRC_ROOTS) { + const files = (await readdir(root, { recursive: true })).filter( + (n) => typeof n === 'string' && n.endsWith('.ts'), + ); + assert.ok(files.length > 0, `no .ts files under ${root.pathname}`); + for (const rel of files) { + const raw = await readFile(new URL(rel, root), 'utf8'); + const code = raw + .split('\n') + .filter((line) => { + const t = line.trimStart(); + return !t.startsWith('//') && !t.startsWith('*') && !t.startsWith('/*'); + }) + .join('\n'); + for (const pattern of FORBIDDEN) { + assert.doesNotMatch( + code, + pattern, + `${root.pathname}${rel} touches a delegation key on config JSONB (${pattern}) — per-assignment delegation via grant config is forbidden by the #860 W0c decision`, + ); + } + } + } + }); }); diff --git a/middleware/test/mcpGrantsRoute.test.ts b/middleware/test/mcpGrantsRoute.test.ts new file mode 100644 index 00000000..0a8082ef --- /dev/null +++ b/middleware/test/mcpGrantsRoute.test.ts @@ -0,0 +1,369 @@ +import { describe, it, afterEach } from 'node:test'; +import { strict as assert } from 'node:assert'; +import express from 'express'; +import type { Express } from 'express'; +import type { Server } from 'node:http'; +import type { AddressInfo } from 'node:net'; + +import { createAgentBuilderRouter } from '../src/routes/agentBuilder.js'; +import { CURRENT_VERIFIER_VERSION } from '../src/services/skillVerdict.js'; + +/** + * Route tests for the MCP grant surface (W0c, #862): + * `PUT /mcp-grants` (single additive grant + allowlist replace + delegation + * write) and `DELETE /mcp-grants/:grantId`. + * + * The W0c review found the changed routes shipping with zero route tests and + * two behavioural defects this file pins: + * + * - RAW vs NORMALIZED refs: `agent_tool_grants` is unique on the RAW + * tool_ref, so 'send_email' and 'odoo-mcp:send_email' are two rows that + * normalize to ONE tool name — a revoke must delete BOTH, or the agent + * silently keeps a tool the operator just revoked. + * - Gate scope: only tools that would be WRITTEN pass the verdict gate. An + * already-granted tool whose ack went stale ("granted but not callable") + * must not veto an unrelated clean edit — the dispatch guard blocks it at + * call time regardless. + */ + +const AGENT_ID = '11111111-1111-4111-8111-111111111111'; +const SERVER_ID = '22222222-2222-4222-8222-222222222222'; +const GRANT_READ = '33333333-3333-4333-8333-333333333331'; +const GRANT_MAIL_RAW = '33333333-3333-4333-8333-333333333332'; +const GRANT_MAIL_PREFIXED = '33333333-3333-4333-8333-333333333333'; + +interface StubOptions { + /** Current agent_tool_grants rows served by listAllToolGrants. */ + grants?: ReadonlyArray>; + /** toolName → verdict severity; tools absent here have NO verdict. */ + verdicts?: Record; + /** toolNames whose needing-ack verdict has a matching ack. */ + acked?: readonly string[]; +} + +interface Harness { + baseUrl: string; + calls: { + applyAllowlist: Array>; + setDelegation: Array<[string, string]>; + bumpEpoch: string[]; + deleteGrant: string[]; + }; + close(): Promise; +} + +function grantRow(id: string, toolRef: string): Record { + return { + id, + agentId: AGENT_ID, + subAgentId: null, + toolKind: 'mcp', + toolRef, + mcpServerId: SERVER_ID, + config: {}, + createdAt: new Date(0), + grantEpoch: null, + }; +} + +async function makeHarness(opts: StubOptions = {}): Promise { + const calls: Harness['calls'] = { + applyAllowlist: [], + setDelegation: [], + bumpEpoch: [], + deleteGrant: [], + }; + const verdicts = opts.verdicts ?? {}; + const acked = new Set(opts.acked ?? []); + const server = { + id: SERVER_ID, + name: 'odoo-mcp', + transport: 'http', + endpoint: 'https://odoo.example/mcp', + status: 'enabled', + lastDiscoveredAt: null, + discoveredTools: [], + delegation: 'service', + source: 'manual', + registryId: null, + license: null, + author: null, + sourceUrl: null, + privacyBypass: false, + kgIngest: false, + configSchema: [], + config: {}, + headers: {}, + }; + const graph = { + listMcpServers: () => Promise.resolve([server]), + listAllToolGrants: () => Promise.resolve(opts.grants ?? []), + getMcpToolVerdict: (_sid: string, toolName: string, version: number) => { + assert.equal(version, CURRENT_VERIFIER_VERSION); + const severity = verdicts[toolName]; + return Promise.resolve( + severity === undefined + ? undefined + : { serverId: SERVER_ID, toolName, severity, contentHash: `hash:${toolName}` }, + ); + }, + getMcpToolVerdictAck: (_sid: string, toolName: string) => + Promise.resolve( + acked.has(toolName) + ? { serverId: SERVER_ID, toolName, contentHash: `hash:${toolName}` } + : undefined, + ), + // refreshMcpGrantPolicy reads these in bulk. + listMcpToolVerdicts: () => Promise.resolve([]), + listMcpToolVerdictAcks: () => Promise.resolve([]), + applyMcpToolAllowlist: (input: Record) => { + calls.applyAllowlist.push(input); + return Promise.resolve(); + }, + setMcpServerDelegation: (serverId: string, delegation: string) => { + calls.setDelegation.push([serverId, delegation]); + return Promise.resolve({ ...server, delegation }); + }, + bumpMcpGrantEpoch: (serverId: string) => { + calls.bumpEpoch.push(serverId); + return Promise.resolve(); + }, + deleteToolGrant: (id: string) => { + calls.deleteGrant.push(id); + return Promise.resolve(); + }, + }; + const config = { + listAgents: () => Promise.resolve([{ id: AGENT_ID, slug: 'sales' }]), + getAgentBySlug: () => Promise.resolve(undefined), + }; + + const app: Express = express(); + app.use(express.json()); + app.use( + '/api/v1/operator', + createAgentBuilderRouter({ + getConfigStore: () => config as never, + getGraphStore: () => graph as never, + getRegistry: () => undefined, + }), + ); + const httpServer: Server = await new Promise((resolve) => { + const s = app.listen(0, '127.0.0.1', () => resolve(s)); + }); + const port = (httpServer.address() as AddressInfo).port; + return { + baseUrl: `http://127.0.0.1:${String(port)}`, + calls, + async close() { + await new Promise((resolve) => httpServer.close(() => resolve())); + }, + }; +} + +async function putGrants(h: Harness, body: Record): Promise { + return fetch(`${h.baseUrl}/api/v1/operator/mcp-grants`, { + method: 'PUT', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify(body), + }); +} + +describe('PUT /mcp-grants (allowlist replace, W0c #862)', () => { + let h: Harness | undefined; + afterEach(async () => { + if (h) await h.close(); + h = undefined; + }); + + it('replaces the allowlist in one transactional store call: grants the new, revokes the dropped', async () => { + h = await makeHarness({ + grants: [grantRow(GRANT_READ, 'read_ticket'), grantRow(GRANT_MAIL_RAW, 'send_email')], + verdicts: { read_ticket: 'no_signals', list_tickets: 'no_signals', send_email: 'no_signals' }, + }); + const res = await putGrants(h, { + agentSlug: 'sales', + mcpServerId: SERVER_ID, + toolNames: ['read_ticket', 'list_tickets'], + }); + assert.equal(res.status, 200); + const body = (await res.json()) as Record; + assert.deepEqual(body['granted'], ['list_tickets']); + assert.deepEqual(body['revoked'], ['send_email']); + assert.deepEqual(body['toolNames'], ['list_tickets', 'read_ticket']); + assert.equal(h.calls.applyAllowlist.length, 1); + assert.deepEqual(h.calls.applyAllowlist[0], { + agentId: AGENT_ID, + mcpServerId: SERVER_ID, + grantRefs: ['list_tickets'], + revokeIds: [GRANT_MAIL_RAW], + }); + assert.deepEqual(h.calls.bumpEpoch, [SERVER_ID]); + }); + + it('revokes EVERY row behind a normalized name — raw and serverName-prefixed refs alike', async () => { + // agent_tool_grants is unique on the RAW tool_ref (0014): 'send_email' + // and 'odoo-mcp:send_email' are two rows for ONE tool. Revoking the tool + // must delete both, or the agent keeps a tool the response reports as + // revoked (the W0c blocker). + h = await makeHarness({ + grants: [ + grantRow(GRANT_READ, 'read_ticket'), + grantRow(GRANT_MAIL_RAW, 'send_email'), + grantRow(GRANT_MAIL_PREFIXED, 'odoo-mcp:send_email'), + ], + verdicts: { read_ticket: 'no_signals', send_email: 'no_signals' }, + }); + const res = await putGrants(h, { + agentSlug: 'sales', + mcpServerId: SERVER_ID, + toolNames: ['read_ticket'], + }); + assert.equal(res.status, 200); + const body = (await res.json()) as Record; + assert.deepEqual(body['revoked'], ['send_email']); + assert.equal(h.calls.applyAllowlist.length, 1); + assert.deepEqual( + [...(h.calls.applyAllowlist[0]!['revokeIds'] as string[])].sort(), + [GRANT_MAIL_RAW, GRANT_MAIL_PREFIXED].sort(), + 'both rows normalizing to send_email must be deleted', + ); + }); + + it('gates only tools that would be written — a stale-acked EXISTING grant cannot veto a clean edit', async () => { + // send_email is granted and its high_risk verdict lost its ack (the + // "granted but not callable" state the matrix exists to display). Adding + // the clean list_tickets must succeed; the dispatch guard handles + // send_email at call time. + h = await makeHarness({ + grants: [grantRow(GRANT_READ, 'read_ticket'), grantRow(GRANT_MAIL_RAW, 'send_email')], + verdicts: { + read_ticket: 'no_signals', + list_tickets: 'no_signals', + send_email: 'high_risk', // no ack → currently blocked + }, + }); + const res = await putGrants(h, { + agentSlug: 'sales', + mcpServerId: SERVER_ID, + toolNames: ['read_ticket', 'send_email', 'list_tickets'], + }); + assert.equal(res.status, 200); + const body = (await res.json()) as Record; + assert.deepEqual(body['granted'], ['list_tickets']); + assert.deepEqual(body['revoked'], []); + }); + + it('a gate rejection aborts the WHOLE edit before any write', async () => { + h = await makeHarness({ + grants: [grantRow(GRANT_READ, 'read_ticket')], + verdicts: { read_ticket: 'no_signals' }, // evil_tool: never scanned + }); + const res = await putGrants(h, { + agentSlug: 'sales', + mcpServerId: SERVER_ID, + toolNames: ['read_ticket', 'evil_tool'], + }); + assert.equal(res.status, 409); + const body = (await res.json()) as Record; + assert.equal(body['error'], 'config_validation'); + assert.equal(h.calls.applyAllowlist.length, 0, 'no store write after a gate rejection'); + assert.deepEqual(h.calls.bumpEpoch, []); + }); + + it('single-grant mode stays additive and backwards compatible', async () => { + h = await makeHarness({ + grants: [], + verdicts: { read_ticket: 'no_signals' }, + }); + const res = await putGrants(h, { + agentSlug: 'sales', + mcpServerId: SERVER_ID, + toolName: 'read_ticket', + }); + assert.equal(res.status, 200); + const body = (await res.json()) as Record; + assert.equal(body['toolName'], 'read_ticket'); + assert.equal(body['granted'], true); + assert.equal(h.calls.applyAllowlist.length, 1); + assert.deepEqual(h.calls.applyAllowlist[0]!['grantRefs'], ['read_ticket']); + assert.deepEqual(h.calls.applyAllowlist[0]!['revokeIds'], [], 'single mode never revokes'); + }); + + it('writes the server delegation mode when requested and reports the server scope', async () => { + h = await makeHarness({ grants: [], verdicts: { read_ticket: 'no_signals' } }); + const res = await putGrants(h, { + agentSlug: 'sales', + mcpServerId: SERVER_ID, + toolNames: ['read_ticket'], + delegation: 'per_user', + }); + assert.equal(res.status, 200); + const body = (await res.json()) as Record; + assert.equal(body['delegation'], 'per_user'); + assert.equal(body['delegationScope'], 'server'); + assert.deepEqual(h.calls.setDelegation, [[SERVER_ID, 'per_user']]); + }); + + it('rejects an invalid delegation value with 400 invalid_delegation', async () => { + h = await makeHarness(); + const res = await putGrants(h, { + agentSlug: 'sales', + mcpServerId: SERVER_ID, + toolNames: ['read_ticket'], + delegation: 'both_please', + }); + assert.equal(res.status, 400); + assert.equal(((await res.json()) as Record)['error'], 'invalid_delegation'); + }); + + it('rejects an unknown mcpServerId with 404 mcp_server_not_found', async () => { + h = await makeHarness(); + const res = await putGrants(h, { + agentSlug: 'sales', + mcpServerId: '99999999-9999-4999-8999-999999999999', + toolNames: ['read_ticket'], + }); + assert.equal(res.status, 404); + assert.equal(((await res.json()) as Record)['error'], 'mcp_server_not_found'); + }); +}); + +describe('DELETE /mcp-grants/:grantId (W0c #862)', () => { + let h: Harness | undefined; + afterEach(async () => { + if (h) await h.close(); + h = undefined; + }); + + it('deletes an MCP grant, refreshes the policy and bumps the epoch', async () => { + h = await makeHarness({ grants: [grantRow(GRANT_READ, 'read_ticket')] }); + const res = await fetch(`${h.baseUrl}/api/v1/operator/mcp-grants/${GRANT_READ}`, { + method: 'DELETE', + }); + assert.equal(res.status, 204); + assert.deepEqual(h.calls.deleteGrant, [GRANT_READ]); + assert.deepEqual(h.calls.bumpEpoch, [SERVER_ID]); + }); + + it('refuses to delete a native grant through the MCP endpoint', async () => { + h = await makeHarness({ + grants: [{ ...grantRow(GRANT_READ, 'memory.search'), toolKind: 'native', mcpServerId: null }], + }); + const res = await fetch(`${h.baseUrl}/api/v1/operator/mcp-grants/${GRANT_READ}`, { + method: 'DELETE', + }); + assert.equal(res.status, 400); + assert.equal(((await res.json()) as Record)['error'], 'not_an_mcp_grant'); + assert.deepEqual(h.calls.deleteGrant, []); + }); + + it('404s for a grant id that does not exist', async () => { + h = await makeHarness({ grants: [] }); + const res = await fetch(`${h.baseUrl}/api/v1/operator/mcp-grants/${GRANT_READ}`, { + method: 'DELETE', + }); + assert.equal(res.status, 404); + assert.equal(((await res.json()) as Record)['error'], 'grant_not_found'); + }); +}); diff --git a/middleware/test/operatorAgentsRouter.test.ts b/middleware/test/operatorAgentsRouter.test.ts index 24dbca31..53660c1b 100644 --- a/middleware/test/operatorAgentsRouter.test.ts +++ b/middleware/test/operatorAgentsRouter.test.ts @@ -20,6 +20,7 @@ */ import { strict as assert } from 'node:assert'; +import { readFile } from 'node:fs/promises'; import { after, afterEach, before, describe, it } from 'node:test'; import type { AddressInfo } from 'node:net'; import type { Server } from 'node:http'; @@ -218,9 +219,18 @@ class FakeGraphStore { toolGrants: ToolGrantMem[] = []; pluginGrants: PluginMcpGrantMem[] = []; servers: Array<{ id: string; name: string }> = []; + /** agentId → its sub-agent ids, mirroring agent_subagents. */ + subAgentsOf: Record = {}; listToolGrantsForAgent(agentId: string): Promise { - return Promise.resolve(this.toolGrants.filter((g) => g.agentId === agentId)); + // Mirrors the real store's contract (W0c): rows held directly by the + // agent PLUS rows held by one of ITS sub-agents (agent_id NULL there). + const subs = new Set(this.subAgentsOf[agentId] ?? []); + return Promise.resolve( + this.toolGrants.filter( + (g) => g.agentId === agentId || (g.subAgentId !== null && subs.has(g.subAgentId)), + ), + ); } listPluginMcpGrantsForPlugins( pluginIds: readonly string[], @@ -620,6 +630,112 @@ describe('createOperatorAgentsRouter', () => { assert.equal(body.plugin_mcp_grants[0]!.server_name, 'odoo-mcp'); }); + it('GET /:slug/grants includes grants held by the agent\'s SUB-agents, attributed via sub_agent_id (W0c)', async () => { + // agent_tool_grants is a XOR table: a sub-agent-held grant has agent_id + // NULL. Hiding those rows made the detail page claim "no grants" while + // the sub-agent could reach the server (the W0c review's failing state). + const agent = await store.createAgent({ slug: 'public', name: 'Public' }); + const other = await store.createAgent({ slug: 'other', name: 'Other' }); + graph.servers = [{ id: 'srv-1', name: 'odoo-mcp' }]; + graph.subAgentsOf = { [agent.id]: ['sa-researcher'], [other.id]: ['sa-foreign'] }; + graph.toolGrants = [ + { + id: 'g-sub', + agentId: null, + subAgentId: 'sa-researcher', + toolKind: 'mcp', + toolRef: 'odoo-mcp:search', + mcpServerId: 'srv-1', + config: {}, + createdAt: new Date('2026-08-01T00:00:00Z'), + grantEpoch: '2026-08-22 08:00:00+00', + }, + { + id: 'g-foreign', + agentId: null, + subAgentId: 'sa-foreign', + toolKind: 'mcp', + toolRef: 'odoo-mcp:write', + mcpServerId: 'srv-1', + config: {}, + createdAt: new Date('2026-08-02T00:00:00Z'), + grantEpoch: null, + }, + ]; + const res = await fetch(`${baseUrl}/public/grants`); + assert.equal(res.status, 200); + const body = (await res.json()) as { + grant_epoch: string | null; + tool_grants: Array<{ id: string; sub_agent_id: string | null; tool_ref: string }>; + }; + assert.deepEqual( + body.tool_grants.map((g) => g.id), + ['g-sub'], + "own sub-agents' grants in, other agents' sub-agents out", + ); + assert.equal(body.tool_grants[0]!.sub_agent_id, 'sa-researcher'); + assert.equal( + body.grant_epoch, + '2026-08-22 08:00:00+00', + 'a sub-agent grant epoch counts toward the agent-level max', + ); + }); + + it('GET /:slug/grants normalizes a serverName-prefixed mcp tool_ref to the bare tool name (W0c)', async () => { + // Stored refs may carry the ':' prefix (canvas edges persist + // the caller's raw ref). Every other reader normalizes via + // mcpToolNameFromRef; the read model must too, so the UI can compare + // tool_ref against discoveredTools[].name verbatim. + const agent = await store.createAgent({ slug: 'public', name: 'Public' }); + graph.servers = [{ id: 'srv-1', name: 'odoo-mcp' }]; + graph.toolGrants = [ + { + id: 'g-prefixed', + agentId: agent.id, + subAgentId: null, + toolKind: 'mcp', + toolRef: 'odoo-mcp:search_partners', + mcpServerId: 'srv-1', + config: {}, + createdAt: new Date('2026-08-01T00:00:00Z'), + grantEpoch: null, + }, + { + id: 'g-native', + agentId: agent.id, + subAgentId: null, + toolKind: 'native', + toolRef: 'memory:search', // native refs are NOT server-prefixed — stay verbatim + mcpServerId: null, + config: {}, + createdAt: new Date('2026-08-02T00:00:00Z'), + grantEpoch: null, + }, + ]; + const res = await fetch(`${baseUrl}/public/grants`); + assert.equal(res.status, 200); + const body = (await res.json()) as { tool_grants: Array<{ id: string; tool_ref: string }> }; + assert.equal(body.tool_grants[0]!.tool_ref, 'search_partners'); + assert.equal(body.tool_grants[1]!.tool_ref, 'memory:search'); + }); + + it('index.ts supplies getAgentGraphStore to the operator-agents mount (wiring pin, W0c)', async () => { + // The route tests above inject their own store; they cannot see a missing + // option at the REAL mount. That exact gap shipped once: index.ts called + // createOperatorAgentsRouter without getAgentGraphStore and every + // /grants request 503ed in every environment (W0c review blocker). Pin + // the wiring statically so the option cannot silently disappear again. + const indexSource = await readFile(new URL('../src/index.ts', import.meta.url), 'utf8'); + const mount = /createOperatorAgentsRouter\(\{([\s\S]*?)\}\)/.exec(indexSource)?.[1]; + assert.ok(mount, 'index.ts no longer mounts createOperatorAgentsRouter — update this pin'); + assert.match( + mount, + /getAgentGraphStore:/, + 'index.ts must pass getAgentGraphStore to createOperatorAgentsRouter — without it every GET /:slug/grants 503s', + ); + assert.match(mount, /new AgentGraphStore\(graphPool\)/, 'the option must construct the real store from graphPool'); + }); + it('GET /:slug/grants → grant_epoch null when no grant was ever bumped', async () => { const agent = await store.createAgent({ slug: 'public', name: 'Public' }); graph.toolGrants = [ diff --git a/web-ui/app/_components/mcp/McpAuthSection.tsx b/web-ui/app/_components/mcp/McpAuthSection.tsx index ff9679d8..83039524 100644 --- a/web-ui/app/_components/mcp/McpAuthSection.tsx +++ b/web-ui/app/_components/mcp/McpAuthSection.tsx @@ -29,8 +29,15 @@ function errText(err: unknown): string { */ export function McpAuthSection({ serverId, + showDelegation = true, }: { serverId: string; + /** The delegation block (mode + un-gated switch) is server-level admin UI. + * Pages that provide their own delegation surface — the per-agent + * assignment editor shows the mode READ-ONLY with a server-wide-effect + * label per the W0c #862 decision — pass `false` so one page never renders + * two contradictory delegation controls. */ + showDelegation?: boolean; }): React.ReactElement | null { const t = useTranslations('adminMcp'); const [status, setStatus] = useState(null); @@ -212,7 +219,7 @@ export function McpAuthSection({
    {t('auth.manualStillSupported')}
    ) : null} - {status.delegation ? ( + {showDelegation && status.delegation ? (
    {t('auth.delegationLabel')}: diff --git a/web-ui/app/_lib/__tests__/agentGrants.test.ts b/web-ui/app/_lib/__tests__/agentGrants.test.ts index 22c58c74..a7d16b96 100644 --- a/web-ui/app/_lib/__tests__/agentGrants.test.ts +++ b/web-ui/app/_lib/__tests__/agentGrants.test.ts @@ -290,8 +290,12 @@ describe('parseMcpGrantErrorCode (i18n hard rule support)', () => { }); describe('shared type surface (extended, not re-declared)', () => { - it('ToolGrantNode carries an OPTIONAL grantEpoch (absent on older middleware)', () => { - const oldMiddleware: ToolGrantNode = { + it('ToolGrantNode mirrors the canvas serializer exactly — NO grantEpoch field', () => { + // The middleware's toolGrantNode() never emits a grant epoch on the canvas + // graph payload; the epoch lives on AgentToolGrantRowDto.grant_epoch and + // McpGrantMatrixRow.grantEpoch only. A phantom optional here misled a page + // unit into rendering a column that would stay empty forever (W0c review). + const node: ToolGrantNode = { id: 'g1', agentId: 'a1', subAgentId: null, @@ -299,12 +303,10 @@ describe('shared type surface (extended, not re-declared)', () => { toolRef: 'mcp:odoo:read_employees', mcpServerId: 's1', }; - const newMiddleware: ToolGrantNode = { - ...oldMiddleware, - grantEpoch: '2026-08-25 17:00:00.000000+00', - }; - expect(oldMiddleware.grantEpoch).toBeUndefined(); - expect(newMiddleware.grantEpoch).toBe('2026-08-25 17:00:00.000000+00'); + expect(node).not.toHaveProperty('grantEpoch'); + // @ts-expect-error grantEpoch is not part of the canvas wire contract + const rejected: ToolGrantNode = { ...node, grantEpoch: 'x' }; + expect(rejected).toBeDefined(); }); it('McpServerNode carries an OPTIONAL per-server delegation next to discoveredTools', () => { diff --git a/web-ui/app/_lib/agentBuilder.ts b/web-ui/app/_lib/agentBuilder.ts index f5326cbf..b643cf96 100644 --- a/web-ui/app/_lib/agentBuilder.ts +++ b/web-ui/app/_lib/agentBuilder.ts @@ -185,10 +185,11 @@ export interface ToolGrantNode { toolKind: ToolKind; toolRef: string; mcpServerId: string | null; - /** Issue #861 — last verdict-epoch bump of this grant (`bumpMcpGrantEpoch` - * stamps `config.verdictEpoch`); `null` until the first bump touches the - * row; absent on older middleware. */ - grantEpoch?: string | null; + // NOTE deliberately NO `grantEpoch` here: the middleware's `toolGrantNode()` + // serializer (canvas graph payload) does not emit it. The grant epoch is + // surfaced by exactly two wire shapes — `AgentToolGrantRowDto.grant_epoch` + // (GET /v1/operator/agents/:slug/grants, agents.ts) and + // `McpGrantMatrixRow.grantEpoch` (GET /mcp-grants) — read it from those. } /** Scan verdict decoration on a discovered MCP tool (issue #454). Absent on diff --git a/web-ui/app/_lib/agents.ts b/web-ui/app/_lib/agents.ts index e846a3f3..79855848 100644 --- a/web-ui/app/_lib/agents.ts +++ b/web-ui/app/_lib/agents.ts @@ -226,7 +226,15 @@ export async function toggleAgentPlugin( export interface AgentToolGrantRowDto { id: string; tool_kind: 'native' | 'mcp'; + /** For `tool_kind === 'mcp'` this is the BARE tool name: the middleware + * normalizes stored refs through `mcpToolNameFromRef` before serializing + * (a persisted ref may carry a legacy ':' prefix). Safe to + * compare verbatim against `discoveredTools[].name`. */ tool_ref: string; + /** Set when a SUB-AGENT of this agent holds the grant (agent_tool_grants is + * a XOR table; the read model attributes sub-agent rows to the parent, + * like every graph read). The assignment editor must skip these rows — + * they are not part of the orchestrator's own top-level allowlist. */ sub_agent_id: string | null; mcp_server_id: string | null; /** Joined-in display name; null for native tools or a deleted server. */ @@ -258,10 +266,11 @@ export interface AgentGrantsDto { } /** - * Per-agent grant read model (issue #861): the agent's own - * `agent_tool_grants` rows plus the `plugin_mcp_grants` of every plugin - * assigned to it, one response for the agent detail page. Read-only — grant - * WRITES stay on the agent-builder surface (`_lib/agentBuilder.ts`). + * Per-agent grant read model (issue #861): the agent's `agent_tool_grants` + * rows — held directly or by one of its sub-agents (`sub_agent_id` tells them + * apart) — plus the `plugin_mcp_grants` of every plugin assigned to it, one + * response for the agent detail page. Read-only — grant WRITES stay on the + * agent-builder surface (`_lib/agentBuilder.ts`). */ export async function getAgentGrants(slug: string): Promise { return callJson( diff --git a/web-ui/app/operator/agents/[slug]/__tests__/AgentDetail.test.tsx b/web-ui/app/operator/agents/[slug]/__tests__/AgentDetail.test.tsx index 9ab7e3db..be96a662 100644 --- a/web-ui/app/operator/agents/[slug]/__tests__/AgentDetail.test.tsx +++ b/web-ui/app/operator/agents/[slug]/__tests__/AgentDetail.test.tsx @@ -50,26 +50,44 @@ vi.mock('../../../../_lib/agents', async (importOriginal) => ({ replaceAgentPlugins: mockReplace, })); -vi.mock('../../_components/PluginsDnd', () => ({ - PluginsDnd: (props: { - onReplace: ( - plugins: Array<{ - id: string; - enabled?: boolean; - config?: Record; - }>, - ) => void; - }) => ( - // eslint-disable-next-line no-restricted-syntax -- test stub standing in for the dnd editor, not a §4.2 CTA -
    + ); + }, + }; +}); function agent(over: Partial = {}): OperatorAgentDto { return { @@ -219,4 +237,39 @@ describe('AgentDetail plugin assignment', () => { ]); await waitFor(() => expect(mockRefresh).toHaveBeenCalled()); }); + + it('a successful instant toggle does NOT remount the editor — unsaved edits survive', async () => { + // Regression (W0c review): the toggle and the editor are two independent + // write paths. When they shared one remount key, a successful toggle + // remounted PluginsDnd and silently discarded every unsaved drag/config + // edit below it. The stub's mount-local "dirty" flag stands in for that + // unsaved state. + const user = userEvent.setup(); + renderWithIntl(); + + await user.click(await screen.findByTestId('plugins-dnd-edit')); + expect(screen.getByTestId('plugins-dnd-state').textContent).toBe('dirty'); + + await user.click( + await screen.findByRole('checkbox', { name: 'Enable or disable Odoo' }), + ); + await waitFor(() => expect(mockRefresh).toHaveBeenCalled()); + + expect(screen.getByTestId('plugins-dnd-state').textContent).toBe('dirty'); + }); + + it("the editor's own save DOES remount it so it reseeds from fresh props", async () => { + const user = userEvent.setup(); + renderWithIntl(); + + await user.click(await screen.findByTestId('plugins-dnd-edit')); + expect(screen.getByTestId('plugins-dnd-state').textContent).toBe('dirty'); + + await user.click(screen.getByTestId('plugins-dnd-save')); + await waitFor(() => expect(mockReplace).toHaveBeenCalled()); + + await waitFor(() => + expect(screen.getByTestId('plugins-dnd-state').textContent).toBe('clean'), + ); + }); }); diff --git a/web-ui/app/operator/agents/[slug]/__tests__/AgentMcpServers.delegation.test.tsx b/web-ui/app/operator/agents/[slug]/__tests__/AgentMcpServers.delegation.test.tsx index 2b016eb0..f1fdca75 100644 --- a/web-ui/app/operator/agents/[slug]/__tests__/AgentMcpServers.delegation.test.tsx +++ b/web-ui/app/operator/agents/[slug]/__tests__/AgentMcpServers.delegation.test.tsx @@ -1,21 +1,22 @@ -import { screen, waitFor } from '@testing-library/react'; +import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { renderWithIntl } from '../../../../_lib/test-utils'; -import { ApiError } from '../../../../_lib/api'; import type { McpDelegation, McpServerNode } from '../../../../_lib/agentBuilder'; import type { AgentGrantsDto } from '../../../../_lib/agents'; import { AgentMcpServers } from '../_components/AgentMcpServers'; /** - * Issue #862 (epic #860) — delegation choice per agent-MCP assignment. + * Issue #862 (epic #860) — delegation on the agent-MCP assignment row. * * The W0c schema-fit gate decided delegation stays PER SERVER * (`mcp_servers.delegation`, migration 0031): there is no per-(agent, server) - * delegation storage, so the assignment row shows the SERVER's mode and - * switches it via the existing server-level endpoint — a change with a - * server-wide effect the UI must label before the operator confirms it. + * delegation storage. The coordinator's follow-up ruling for the assignment + * view: the row shows the SERVER's mode READ-ONLY, labels its server-wide + * effect, and links to the MCP server settings — the single write surface. + * There must be NO delegation write path on the agent detail page, including + * the one McpAuthSection normally embeds (it is passed showDelegation={false}). */ const { @@ -81,7 +82,7 @@ async function renderExpanded(): Promise> { return user; } -describe('AgentMcpServers delegation (#862, per-server by the W0c gate)', () => { +describe('AgentMcpServers delegation (#862, read-only per the W0c ruling)', () => { it('shows the server delegation mode with its server-wide scope labelling', async () => { await renderExpanded(); @@ -105,50 +106,37 @@ describe('AgentMcpServers delegation (#862, per-server by the W0c gate)', () => ).toBeTruthy(); }); - it('switches the mode only after the server-wide-effect dialog is confirmed', async () => { - const user = await renderExpanded(); + it('is strictly read-only — no switch control, and the mode write is never called', async () => { + await renderExpanded(); - await user.click(screen.getByRole('button', { name: 'Switch to per user…' })); - // Nothing happens until the operator confirms the global effect. + expect(screen.queryByRole('button', { name: /Switch to/ })).toBeNull(); + expect(screen.queryByRole('button', { name: 'Switch mode' })).toBeNull(); expect(mockSetMcpServerDelegation).not.toHaveBeenCalled(); - expect( - await screen.findByText( - '"odoo-mcp" delegates identity per server, not per assignment. Switching to Per user changes it for every agent using this server. Continue?', - ), - ).toBeTruthy(); - - // The post-switch refresh serves the flipped mode. - mockListMcpServers.mockResolvedValue({ servers: [server('per_user')] }); - await user.click(screen.getByRole('button', { name: 'Switch mode' })); - - await waitFor(() => - expect(mockSetMcpServerDelegation).toHaveBeenCalledWith('srv-1', 'per_user'), - ); - await waitFor(() => - expect(screen.getByRole('button', { name: 'Switch to service…' })).toBeTruthy(), - ); }); - it('does not switch when the dialog is cancelled', async () => { - const user = await renderExpanded(); - - await user.click(screen.getByRole('button', { name: 'Switch to per user…' })); - await user.click(await screen.findByRole('button', { name: 'Cancel' })); + it('links to the MCP server settings, the single delegation write surface', async () => { + await renderExpanded(); - expect(mockSetMcpServerDelegation).not.toHaveBeenCalled(); + const link = screen.getByRole('link', { name: 'Change in MCP server settings' }); + expect(link.getAttribute('href')).toBe('/admin/mcp'); }); - it('maps a rejected switch to the localized error code message, never the raw body', async () => { - mockSetMcpServerDelegation.mockRejectedValue( - new ApiError(400, 'Bad Request', '{"error":"invalid_delegation"}'), - ); - const user = await renderExpanded(); - - await user.click(screen.getByRole('button', { name: 'Switch to per user…' })); - await user.click(await screen.findByRole('button', { name: 'Switch mode' })); + it('suppresses McpAuthSection\'s own delegation toggle on this page (one surface per context)', async () => { + // A protected server WITH auth-status delegation would normally render + // McpAuthSection's un-gated switch — the agent page passes + // showDelegation={false}, so no second (and un-labelled) delegation + // control can appear here. + mockGetMcpAuthStatus.mockResolvedValue({ + connected: true, + protected: true, + delegation: 'service', + identityResolved: true, + }); + await renderExpanded(); - expect(await screen.findByText('The server rejected the delegation mode.')).toBeTruthy(); - expect(screen.queryByText('{"error":"invalid_delegation"}')).toBeNull(); + expect(screen.queryByText('Acting identity')).toBeNull(); + expect(screen.queryByRole('button', { name: 'Require per-user identity' })).toBeNull(); + expect(screen.queryByRole('button', { name: 'Use a shared service identity' })).toBeNull(); }); it('renders no delegation UI when the middleware does not report a mode', async () => { @@ -157,11 +145,11 @@ describe('AgentMcpServers delegation (#862, per-server by the W0c gate)', () => renderWithIntl(); await user.click(await screen.findByRole('button', { name: /odoo-mcp/ })); - // Older middleware without `delegation` on the server row → no block, no switch. + // Middleware builds without `delegation` on the server row → no block. await screen.findByText( 'No tools discovered yet — run discovery in the MCP Control Center first.', ); expect(screen.queryByText('Identity delegation')).toBeNull(); - expect(screen.queryByRole('button', { name: /Switch to/ })).toBeNull(); + expect(screen.queryByRole('link', { name: 'Change in MCP server settings' })).toBeNull(); }); }); diff --git a/web-ui/app/operator/agents/[slug]/__tests__/AgentMcpServers.test.tsx b/web-ui/app/operator/agents/[slug]/__tests__/AgentMcpServers.test.tsx index 6ae4f8d8..f6d45fc4 100644 --- a/web-ui/app/operator/agents/[slug]/__tests__/AgentMcpServers.test.tsx +++ b/web-ui/app/operator/agents/[slug]/__tests__/AgentMcpServers.test.tsx @@ -238,6 +238,35 @@ describe('AgentMcpServers (#862)', () => { ); }); + it('ignores sub-agent-held grants — the editor manages only the orchestrator\'s own allowlist', async () => { + // The grants DTO includes rows held by the agent's sub-agents + // (sub_agent_id set). Pre-checking one here would make the header count + // wrong AND make Save/Unassign issue a top-level PUT derived from a set + // the orchestrator never held — "revoking" without touching the + // sub-agent row (W0c review). + const dto = grants(['search_partners']); + mockGetAgentGrants.mockResolvedValue({ + ...dto, + tool_grants: [ + ...dto.tool_grants, + { + id: 'g-sub', + tool_kind: 'mcp' as const, + tool_ref: 'create_invoice', + sub_agent_id: 'sa-1', + mcp_server_id: 'srv-1', + server_name: 'odoo-mcp', + grant_epoch: null, + created_at: '2026-08-25T00:00:00Z', + }, + ], + }); + await renderExpanded(); + + expect(screen.getByText('1 of 4 tools granted')).toBeTruthy(); + expect((screen.getByLabelText('Grant create_invoice') as HTMLInputElement).checked).toBe(false); + }); + it('shows the empty state when no MCP server exists', async () => { mockListMcpServers.mockResolvedValue({ servers: [] }); mockGetAgentGrants.mockResolvedValue(grants([])); diff --git a/web-ui/app/operator/agents/[slug]/__tests__/AgentToolGrants.test.tsx b/web-ui/app/operator/agents/[slug]/__tests__/AgentToolGrants.test.tsx index a107c881..2938cad9 100644 --- a/web-ui/app/operator/agents/[slug]/__tests__/AgentToolGrants.test.tsx +++ b/web-ui/app/operator/agents/[slug]/__tests__/AgentToolGrants.test.tsx @@ -178,4 +178,33 @@ describe('AgentToolGrants (#861)', () => { expect(screen.queryByRole('alert')).toBeNull(); expect(mockGetAgentGrants).toHaveBeenCalledTimes(2); }); + + it('reports the epoch as UNKNOWN, not "never bumped", while the load is pending', async () => { + // "Never bumped" is a factual claim about authorization state. While the + // DTO has not resolved, the truth is unknown — asserting "never bumped" + // during the pending window would be a false statement (W0c review). + let resolve!: (v: unknown) => void; + mockGetAgentGrants.mockReturnValue( + new Promise((r) => { + resolve = r; + }), + ); + renderWithIntl(); + + expect(screen.getByText('Grant epoch: unknown')).toBeTruthy(); + expect(screen.queryByText('Grant epoch: never bumped')).toBeNull(); + + resolve(grantsDto({ grant_epoch: null, tool_grants: [], plugin_mcp_grants: [] })); + expect(await screen.findByText('Grant epoch: never bumped')).toBeTruthy(); + expect(screen.queryByText('Grant epoch: unknown')).toBeNull(); + }); + + it('reports the epoch as UNKNOWN after a failed load — never a false "never bumped"', async () => { + mockGetAgentGrants.mockRejectedValue(new Error('offline')); + renderWithIntl(); + + await screen.findByRole('alert'); + expect(screen.getByText('Grant epoch: unknown')).toBeTruthy(); + expect(screen.queryByText('Grant epoch: never bumped')).toBeNull(); + }); }); diff --git a/web-ui/app/operator/agents/[slug]/_components/AgentDetail.tsx b/web-ui/app/operator/agents/[slug]/_components/AgentDetail.tsx index 7994805b..22e9a9b9 100644 --- a/web-ui/app/operator/agents/[slug]/_components/AgentDetail.tsx +++ b/web-ui/app/operator/agents/[slug]/_components/AgentDetail.tsx @@ -12,10 +12,7 @@ import { type OperatorAgentDto, type PluginCatalogEntryDto, } from '../../../../_lib/agents'; -import { - humanizeApiError, - pluginsRevisionKey, -} from '../../_components/AgentsDashboard'; +import { humanizeApiError } from '../../_components/AgentsDashboard'; import { PluginsDnd } from '../../_components/PluginsDnd'; interface AgentDetailProps { @@ -40,8 +37,9 @@ interface AgentDetailProps { * replace-set PUT) — spec says extend, not duplicate. * * After every successful write `router.refresh()` re-runs the parent RSC - * fetch; `pluginsRevisionKey` remounts PluginsDnd so its local state reseeds - * from the fresh props (same contract as the dashboard cards). + * fetch. PluginsDnd is remounted (reseeding its local state from fresh props) + * only after ITS OWN save — see `editorRevision` — so an instant toggle above + * never discards unsaved edits in the editor below. * * Error copy: the routes emit machine codes as `{ error: '' }`. * `parseOperatorAgentErrorCode` narrows them and each code maps to a @@ -57,6 +55,20 @@ export function AgentDetail(props: AgentDetailProps): React.ReactElement { const [error, setError] = useState(null); const [catalog, setCatalog] = useState(null); const [catalogError, setCatalogError] = useState(null); + /** + * Remount key for PluginsDnd, bumped ONLY by the editor's own save path. + * + * The dashboard keys PluginsDnd on `pluginsRevisionKey(agent)` (a hash over + * each plugin's id/enabled/config) because there the editor is the only + * writer. Here the assigned-list checkbox is a SECOND writer that mutates + * exactly the `enabled` bit such a hash includes — with a shared key, a + * successful toggle would remount the editor and silently discard every + * unsaved drag and config edit below it (W0c review). A counter the toggle + * cannot touch keeps the two write paths decoupled: toggles leave the + * editor's local state alone; the editor's own save still remounts it so it + * reseeds from the fresh props (same contract as the dashboard). + */ + const [editorRevision, setEditorRevision] = useState(0); useEffect(() => { let cancelled = false; @@ -95,11 +107,16 @@ export function AgentDetail(props: AgentDetailProps): React.ReactElement { : t('detailErrors.unknown', { detail: humanizeApiError(err) }); } - function run(label: string, op: () => Promise): void { + function run( + label: string, + op: () => Promise, + opts?: { readonly remountEditor?: boolean }, + ): void { setError(null); setBusy(label); op() .then(() => { + if (opts?.remountEditor) setEditorRevision((r) => r + 1); startTransition(() => router.refresh()); }) .catch((err: unknown) => { @@ -184,14 +201,16 @@ export function AgentDetail(props: AgentDetailProps): React.ReactElement { )} {catalog ? ( - run('plugins', () => - replaceAgentPlugins(props.agent.slug, plugins), + run( + 'plugins', + () => replaceAgentPlugins(props.agent.slug, plugins), + { remountEditor: true }, ) } /> diff --git a/web-ui/app/operator/agents/[slug]/_components/AgentMcpServers.tsx b/web-ui/app/operator/agents/[slug]/_components/AgentMcpServers.tsx index 0fd2075c..95e2963e 100644 --- a/web-ui/app/operator/agents/[slug]/_components/AgentMcpServers.tsx +++ b/web-ui/app/operator/agents/[slug]/_components/AgentMcpServers.tsx @@ -2,6 +2,7 @@ import { useCallback, useEffect, useState } from 'react'; +import Link from 'next/link'; import { useTranslations } from 'next-intl'; import { Button } from '@/app/_components/ui/Button'; @@ -13,7 +14,6 @@ import { listMcpServers, parseMcpGrantErrorCode, replaceMcpToolAllowlist, - setMcpServerDelegation, type McpDelegation, type McpDiscoveredTool, type McpServerNode, @@ -50,11 +50,17 @@ function isGrantable(tool: McpDiscoveredTool): boolean { return !needsAck(tool); } -/** The agent's granted MCP tool names, keyed by server id. */ +/** The agent's OWN granted MCP tool names, keyed by server id. Rows held by a + * sub-agent (`sub_agent_id` set) are excluded on purpose: this editor manages + * only the orchestrator's top-level allowlist, and `PUT /mcp-grants` bulk + * replace operates on exactly that set — pre-checking a sub-agent's grant + * would make Save/Unassign claim a revocation that never touches the + * sub-agent row (W0c review). Sub-agent grants stay visible read-only in the + * tool-grant list above. */ function grantedByServer(grants: AgentGrantsDto | null): Map> { const map = new Map>(); for (const g of grants?.tool_grants ?? []) { - if (g.tool_kind !== 'mcp' || g.mcp_server_id === null) continue; + if (g.tool_kind !== 'mcp' || g.mcp_server_id === null || g.sub_agent_id !== null) continue; const set = map.get(g.mcp_server_id) ?? new Set(); set.add(g.tool_ref); map.set(g.mcp_server_id, set); @@ -90,13 +96,6 @@ export function AgentMcpServers({ slug }: { slug: string }): React.ReactElement const [ackArm, setAckArm] = useState(null); const [ackBusy, setAckBusy] = useState(null); const [confirmUnassign, setConfirmUnassign] = useState(null); - /** Pending delegation switch, held until the operator confirms the - * server-wide effect in the dialog. */ - const [confirmDelegation, setConfirmDelegation] = useState<{ - server: McpServerNode; - next: McpDelegation; - } | null>(null); - const [delegationBusy, setDelegationBusy] = useState(null); const refresh = useCallback(async () => { try { @@ -172,26 +171,6 @@ export function AgentMcpServers({ slug }: { slug: string }): React.ReactElement [t], ); - /** - * Delegation is stored PER SERVER (`mcp_servers.delegation`, migration 0031) - * — there is no per-(agent, server) storage, by the W0c schema-fit gate's - * decision (epic #860). Switching it here therefore changes WHOSE identity - * every agent's calls to this server act under, which is why the switch is - * confirm-gated and labelled with its server-wide effect. - */ - async function switchDelegation(server: McpServerNode, next: McpDelegation): Promise { - setDelegationBusy(server.id); - setError(null); - try { - await setMcpServerDelegation(server.id, next); - await refresh(); - } catch (err) { - setError(grantErrorText(err)); - } finally { - setDelegationBusy(null); - } - } - async function ack(server: McpServerNode, toolName: string): Promise { const key = `${server.id} ${toolName}`; setAckBusy(key); @@ -279,7 +258,15 @@ export function AgentMcpServers({ slug }: { slug: string }): React.ReactElement {isOpen ? (
    - + {/* Delegation is a PER-SERVER setting (`mcp_servers.delegation`, + 0031) — the W0c coordinator decision for #862 is that the + assignment view shows it READ-ONLY with its server-wide + effect labelled and links to the server settings, where the + single write surface lives. `showDelegation={false}` keeps + McpAuthSection's own (un-gated) delegation toggle off this + page so the agent context has exactly one delegation + surface. */} + {server.delegation !== undefined ? (
    @@ -289,21 +276,12 @@ export function AgentMcpServers({ slug }: { slug: string }): React.ReactElement {modeLabel(server.delegation)} - + {t('mcp.delegation.serverSettingsLink')} +
    {t('mcp.delegation.scopeHint')} @@ -470,27 +448,6 @@ export function AgentMcpServers({ slug }: { slug: string }): React.ReactElement if (target !== null) void saveAllowlist(target, []); }} /> - setConfirmDelegation(null)} - onConfirm={() => { - const target = confirmDelegation; - setConfirmDelegation(null); - if (target !== null) void switchDelegation(target.server, target.next); - }} - /> ); } diff --git a/web-ui/app/operator/agents/[slug]/_components/AgentToolGrants.tsx b/web-ui/app/operator/agents/[slug]/_components/AgentToolGrants.tsx index dbb22e1a..0930a703 100644 --- a/web-ui/app/operator/agents/[slug]/_components/AgentToolGrants.tsx +++ b/web-ui/app/operator/agents/[slug]/_components/AgentToolGrants.tsx @@ -80,13 +80,20 @@ export function AgentToolGrants(props: AgentToolGrantsProps): React.ReactElement

    {t('grants.heading')}

    + {/* Three DISTINCT epoch states: unknown (still loading, or the load + failed — the true epoch cannot be asserted), never bumped (a + RESOLVED DTO whose grant_epoch is null), and a real timestamp. + Collapsing unknown into "never bumped" would make the panel state + a false fact about authorization state (W0c review). */} - {t('grants.epochSummary', { - epoch: - grants?.grant_epoch != null - ? formatEpoch(grants.grant_epoch, format) - : t('grants.epochNever'), - })} + {grants == null + ? t('grants.epochUnknown') + : t('grants.epochSummary', { + epoch: + grants.grant_epoch != null + ? formatEpoch(grants.grant_epoch, format) + : t('grants.epochNever'), + })}