diff --git a/agent/agent.ts b/agent/agent.ts index b6c4ad826..cf6920741 100644 --- a/agent/agent.ts +++ b/agent/agent.ts @@ -9,8 +9,9 @@ * * The component intentionally avoids `handleApplication`: it has nothing * worker-thread-shaped to do. Operator-only tools (FS, schedule, fetch) are - * inline; registry-backed tools (#615/#617/#618) will fold in via toolset.ts - * once those land. + * inline; RBAC-filtered tools from the unified MCP registry (#615/#781, + * Operations profile #617) fold in via `registryTools.ts` for the agent's + * configured user. */ import { dirname, isAbsolute, resolve as resolvePath } from 'node:path'; @@ -21,6 +22,9 @@ import { composeToolset } from './toolset.ts'; import { buildOperations } from './operations.ts'; import { runAgent, _resetInFlightForTests } from './loop.ts'; import { appendMessage, getSession } from './session.ts'; +import { ensureOperationsToolsRegistered, composeRegistryTools } from './registryTools.ts'; +import { server } from '../server/Server.ts'; +import type { AuthedUser } from '../components/mcp/toolRegistry.ts'; import type { AgentConfig, AgentScopes, AgentTool } from './types.ts'; const log = harperLogger.loggerWithTag('agent'); @@ -64,10 +68,22 @@ export async function startOnMainThread(opts: StartOpts): Promise { const models = new Models(); const abortControllers = new Map(); let liveConfig: AgentConfig = config; + + // Populate the MCP registry's Operations profile (idempotent) and resolve the agent's RBAC + // identity, then pull the RBAC-filtered registry tools for that user. Computed once at startup: + // the agent acts as a single configured user, and the Operations profile is registered once on + // the main thread. (Application-profile per-Resource tools are a follow-up — they populate from + // the worker-thread Resources registry.) + ensureOperationsToolsRegistered(); + const agentUser = await resolveAgentUser(liveConfig.user); + const registryTools = composeRegistryTools(agentUser, `agent:registry:${liveConfig.user}`); + let composed = composeToolset({ allowDestructive: liveConfig.allowDestructive, onFollowup: handleFollowup, + registryTools, }); + log.info?.(`Agent: composed ${registryTools.length} RBAC-filtered registry tool(s) for user '${liveConfig.user}'`); // Only warn when the operator explicitly configured `maxCostUsd`. Logging on the default // every boot would flood the log without telling anyone anything actionable. @@ -140,6 +156,7 @@ export async function startOnMainThread(opts: StartOpts): Promise { composed = composeToolset({ allowDestructive: liveConfig.allowDestructive, onFollowup: handleFollowup, + registryTools, }); } // NOTE: an already in-flight run captured its toolset (and autoApprove) at start, so flipping @@ -161,6 +178,30 @@ export async function startOnMainThread(opts: StartOpts): Promise { log.info?.(`Agent component initialized with ${composed.tools.length} tools`); } +/** + * Resolve the agent's configured user to its full permission object so registry tools are + * RBAC-filtered correctly. Falls back to a super_user identity (the documented `hdb_agent` + * default) if the user can't be loaded yet — e.g. the system user hasn't been created, or the + * security layer isn't initialized in a given context. A restricted role that fails to load would + * over-grant under this fallback, so the failure is logged at warn. + */ +async function resolveAgentUser(username: string): Promise { + try { + // `server.getUser(username)` resolves the stored user incl. role/permissions. Typed loosely + // here because Server.ts declares extra auth params this lookup-only call doesn't need. + const getUser = (server as any).getUser as ((u: string) => Promise) | undefined; + if (typeof getUser === 'function') { + const user = await getUser(username); + if (user?.role?.permission) return user; + } + } catch (err) { + log.warn?.( + `Agent: could not resolve user '${username}' (${(err as Error)?.message ?? err}); using super_user fallback` + ); + } + return { username, role: { permission: { super_user: true } } }; +} + function resolveScopes( config: AgentConfig, getConfigPath: (param: string) => string | undefined, diff --git a/agent/registryTools.ts b/agent/registryTools.ts new file mode 100644 index 000000000..19b4b6dde --- /dev/null +++ b/agent/registryTools.ts @@ -0,0 +1,96 @@ +/** + * Bridges the unified MCP tool registry (#615/#781, Operations profile #617) + * into the built-in agent's toolset. + * + * The agent consumes the SAME registry the MCP server exposes, RBAC-filtered + * for the agent's configured user — so an operator who points `agent.user` at + * a restricted role automatically gets a narrowed tool surface, with no + * per-tool wiring here. Operator-only tools (FS, schedule, fetch) stay inline + * in `toolset.ts`; they are intentionally NOT in the registry. + * + * Scope (v1): the **Operations** profile only. Those tools are registered on + * the main thread (where the agent runs) by `registerOperationsTools()`, which + * is idempotent — so the agent populates them itself rather than depending on + * the operator having enabled the `mcp:` HTTP surface. The **Application** + * profile (#618, per-Resource tools) is populated from the worker-thread + * Resources registry and is a follow-up. + */ + +import harperLogger from '../utility/logging/harper_logger.ts'; +import { getTool, listTools, type AuthedUser, type ToolResult } from '../components/mcp/toolRegistry.ts'; +import { registerOperationsTools } from '../components/mcp/tools/operations.ts'; +import type { AgentTool, AgentToolContext } from './types.ts'; + +const log = harperLogger.loggerWithTag('agent'); +const REGISTRY_PAGE_SIZE = 200; + +/** + * Ensure the Operations-profile tools exist in the registry. Idempotent + * (`addTool` is `Map.set`-backed), so it's safe whether or not the MCP + * component already registered them. Failures are logged, not thrown — a + * registry hiccup must not stop the agent from starting with its inline tools. + */ +export function ensureOperationsToolsRegistered(): void { + try { + registerOperationsTools(); + } catch (err) { + log.warn?.(`Agent: failed to populate operations tools in MCP registry: ${(err as Error)?.message ?? err}`); + } +} + +/** + * Build {@link AgentTool}s from the registry's Operations profile, filtered to + * what `agentUser` may invoke. Pagination is drained fully — the agent wants + * every visible tool, not a page. + */ +export function composeRegistryTools(agentUser: AuthedUser, sessionId: string): AgentTool[] { + const tools: AgentTool[] = []; + let cursor: string | undefined; + do { + const page = listTools({ user: agentUser, profile: 'operations', sessionId, cursor, limit: REGISTRY_PAGE_SIZE }); + for (const descriptor of page.tools) { + tools.push( + adaptRegistryTool( + descriptor.name, + descriptor.description, + descriptor.inputSchema, + agentUser, + descriptor.annotations?.destructiveHint === true + ) + ); + } + cursor = page.nextCursor; + } while (cursor); + return tools; +} + +function adaptRegistryTool( + name: string, + description: string, + inputSchema: object, + agentUser: AuthedUser, + destructive: boolean +): AgentTool { + return { + def: { name, description, parameters: inputSchema }, + destructive, + handler: async (args: object, ctx: AgentToolContext) => { + const tool = getTool(name); + if (!tool) throw new Error(`registry tool '${name}' no longer registered`); + const result: ToolResult = await tool.handler(args ?? {}, { + user: agentUser, + profile: 'operations', + sessionId: ctx.sessionId, + }); + if (result.isError) { + // Surface the registry tool's error text to the loop, which records it as a structured + // failure observation (the model can then adjust) rather than aborting the run. + const text = result.content?.find((c) => c.type === 'text')?.text ?? `tool '${name}' failed`; + throw new Error(text); + } + // Prefer the structured payload; fall back to concatenated text content. + if (result.structuredContent !== undefined) return result.structuredContent; + return result.content?.map((c) => c.text ?? '').join('') ?? ''; + }, + }; +} diff --git a/agent/toolset.ts b/agent/toolset.ts index 48342ee27..af0e8165d 100644 --- a/agent/toolset.ts +++ b/agent/toolset.ts @@ -1,24 +1,33 @@ /** * Tool composer for the built-in agent (#626). * - * Operator-only tools (FS, schedule, fetch) are inline today. Once the - * unified MCP tool registry (#615) lands with the Operations (#617) and - * Application (#618) profiles, this is the seam where RBAC-filtered - * registry tools get folded in for the agent's configured user. The shape - * of {@link composeToolset} won't change — only the body gains a registry - * lookup. + * Two sources, composed per the design: + * 1. Operator-only tools (FS, schedule, fetch) — inline here, NOT in the MCP + * registry (their runtime assumptions only hold on the main thread). + * 2. RBAC-filtered tools from the unified MCP registry (#615/#781 + Operations + * profile #617), passed in by `agent.ts` already filtered for the agent's + * configured user. See `registryTools.ts`. + * + * Operator-only tools win on a name collision — they are the curated, + * main-thread-safe surface and must not be shadowed by a same-named registry + * tool. */ import { fsTools } from './tools/fsTools.ts'; import { httpFetchTool } from './tools/httpFetchTool.ts'; import { buildScheduleTool, type ScheduleToolDeps, type ScheduledFollowup } from './tools/scheduleTool.ts'; +import harperLogger from '../utility/logging/harper_logger.ts'; import type { AgentTool } from './types.ts'; +const log = harperLogger.loggerWithTag('agent'); + export interface ComposeToolsetOpts extends ScheduleToolDeps { /** When `false`, destructive tools are filtered out at composition time. */ allowDestructive?: boolean; /** Operator-injected extras (tests, custom plugins). */ extraTools?: AgentTool[]; + /** RBAC-filtered tools from the MCP registry (Operations profile), pre-filtered for the agent user. */ + registryTools?: AgentTool[]; } export interface ComposedToolset { @@ -28,7 +37,17 @@ export interface ComposedToolset { export function composeToolset(opts: ComposeToolsetOpts): ComposedToolset { const schedule = buildScheduleTool(opts); - const all: AgentTool[] = [...fsTools, httpFetchTool, schedule.tool, ...(opts.extraTools ?? [])]; + // Operator-only first so they take precedence on name collisions. + const operatorOnly: AgentTool[] = [...fsTools, httpFetchTool, schedule.tool, ...(opts.extraTools ?? [])]; + const operatorNames = new Set(operatorOnly.map((t) => t.def.name)); + const registry = (opts.registryTools ?? []).filter((t) => { + if (operatorNames.has(t.def.name)) { + log.trace?.(`Agent: registry tool '${t.def.name}' shadowed by operator-only tool of the same name; skipping`); + return false; + } + return true; + }); + const all: AgentTool[] = [...operatorOnly, ...registry]; const tools = opts.allowDestructive === false ? all.filter((t) => !t.destructive) : all; return { tools, scheduled: schedule.pending }; } diff --git a/unitTests/agent/registryTools.test.js b/unitTests/agent/registryTools.test.js new file mode 100644 index 000000000..d5afeeda6 --- /dev/null +++ b/unitTests/agent/registryTools.test.js @@ -0,0 +1,92 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const { addTool, _resetRegistryForTest } = require('#src/components/mcp/toolRegistry'); +const { composeRegistryTools } = require('#src/agent/registryTools'); +const { composeToolset } = require('#src/agent/toolset'); + +const superUser = { username: 'hdb_agent', role: { permission: { super_user: true } } }; +const limitedUser = { username: 'ro', role: { permission: { operations: ['describe_all'] } } }; + +function fakeOpTool(name, overrides = {}) { + return { + name, + description: `op ${name}`, + inputSchema: { type: 'object' }, + profile: 'operations', + visibleTo: () => true, + handler: async () => ({ content: [{ type: 'text', text: 'ok' }], structuredContent: { ran: name } }), + ...overrides, + }; +} + +describe('agent/registryTools', () => { + beforeEach(() => _resetRegistryForTest()); + afterEach(() => _resetRegistryForTest()); + + it('adapts visible operations-profile tools to AgentTools', async () => { + addTool(fakeOpTool('describe_all')); + addTool(fakeOpTool('search')); + const tools = composeRegistryTools(superUser, 'sess'); + const names = tools.map((t) => t.def.name).sort(); + assert.deepEqual(names, ['describe_all', 'search']); + // parameters carries the registry inputSchema + assert.deepEqual(tools[0].def.parameters, { type: 'object' }); + }); + + it('respects RBAC visibleTo — excludes tools the user cannot invoke', async () => { + addTool(fakeOpTool('describe_all', { visibleTo: () => true })); + addTool(fakeOpTool('drop_table', { visibleTo: (u) => u.role?.permission?.super_user === true })); + const visible = composeRegistryTools(limitedUser, 'sess').map((t) => t.def.name); + assert.deepEqual(visible, ['describe_all']); + const all = composeRegistryTools(superUser, 'sess') + .map((t) => t.def.name) + .sort(); + assert.deepEqual(all, ['describe_all', 'drop_table']); + }); + + it('maps destructiveHint annotation to the AgentTool destructive flag', async () => { + addTool(fakeOpTool('restart', { annotations: { destructiveHint: true } })); + addTool(fakeOpTool('describe_all', { annotations: { readOnlyHint: true } })); + const tools = composeRegistryTools(superUser, 'sess'); + const restart = tools.find((t) => t.def.name === 'restart'); + const describe = tools.find((t) => t.def.name === 'describe_all'); + assert.equal(restart.destructive, true); + assert.notEqual(describe.destructive, true); + }); + + it('adapter handler returns structuredContent on success', async () => { + addTool(fakeOpTool('search')); + const [tool] = composeRegistryTools(superUser, 'sess'); + const result = await tool.handler({ q: 1 }, { sessionId: 'sess', scopes: {} }); + assert.deepEqual(result, { ran: 'search' }); + }); + + it('adapter handler throws the error text when the registry tool returns isError', async () => { + addTool( + fakeOpTool('drop_table', { + handler: async () => ({ isError: true, content: [{ type: 'text', text: 'permission denied' }] }), + }) + ); + const [tool] = composeRegistryTools(superUser, 'sess'); + await assert.rejects(tool.handler({}, { sessionId: 'sess', scopes: {} }), /permission denied/); + }); + + it('composeToolset drops a registry tool that collides with an operator-only tool name', () => { + // `read_file` is an operator-only FS tool; a registry tool of the same name must not shadow it. + const fakeRegistryReadFile = { + def: { name: 'read_file', description: 'registry shadow', parameters: { type: 'object' } }, + handler: async () => ({ shadow: true }), + }; + const { tools } = composeToolset({ + onFollowup: () => {}, + registryTools: [fakeRegistryReadFile], + }); + const readFileTools = tools.filter((t) => t.def.name === 'read_file'); + assert.equal(readFileTools.length, 1); + assert.equal( + readFileTools[0].def.description, + 'Read a UTF-8 text file within componentsRoot, logDir, or configDir.' + ); + }); +});