Skip to content

feat(agent): consume MCP tool registry (Operations profile), RBAC-filtered - #893

Closed
kriszyp wants to merge 1 commit into
feat/builtin-agent-componentfrom
feat/agent-registry-tools
Closed

feat(agent): consume MCP tool registry (Operations profile), RBAC-filtered#893
kriszyp wants to merge 1 commit into
feat/builtin-agent-componentfrom
feat/agent-registry-tools

Conversation

@kriszyp

@kriszyp kriszyp commented Jun 1, 2026

Copy link
Copy Markdown
Member

Summary

Stacked on #839. Wires the now-merged unified MCP tool registry (#615/#781, Operations profile #617) into the built-in agent's toolset, fulfilling #626's tool-composition design: the agent consumes the same registry the MCP server exposes, RBAC-filtered for its configured user.

Base: feat/builtin-agent-component (#839) — review/merge that first; this diff is just the registry integration.

What it does

  • agent/registryTools.ts (new):
    • ensureOperationsToolsRegistered() — populates the Operations profile on the main thread. Idempotent (addTool is Map.set-backed), so the agent gets these tools whether or not the operator enabled the mcp: HTTP surface.
    • composeRegistryTools(agentUser, sessionId) — drains listTools({profile:'operations'}) and adapts each ToolDescriptorAgentTool: inputSchemaparameters, annotations.destructiveHint→the loop's approval gate, and a handler wrapping getTool(name).handler (ToolResultstructuredContent; isError → throw, so the loop records a structured failure rather than aborting).
  • agent/agent.ts — resolves the configured agent user via server.getUser(config.user) so tools are filtered for that identity (an operator who sets agent.user to a restricted role gets a narrowed surface automatically), then composes registry + operator-only tools.
  • agent/toolset.ts — merges the two sources; operator-only tools win on name collision and are never shadowed by a same-named registry tool.

Where to put attention

  • RBAC identity fallback (resolveAgentUser): if server.getUser can't resolve config.user (e.g. the hdb_agent system user isn't created yet), it falls back to a super_user identity and warns. A restricted role that fails to load would over-grant under this fallback — acceptable for the default super_user agent, but the real fix is creating/loading the system user at startup (noted, follow-up).
  • Compose-once timing: registry tools are pulled once at startup. The Operations profile is registered once on the main thread, so that's stable. If/when the registry gains dynamic tools, this may need a refresh hook.
  • Destructive gating: registry tools annotated destructiveHint flow through the existing approval gate + allowDestructive filter — no separate path.

Out of scope (follow-ups)

Verification

  • npm run build / lint:required / prettier clean
  • npx mocha "unitTests/agent/**/*.test.js" — 45 passing (6 new: adapter mapping, RBAC visibleTo filtering, destructive-flag mapping, success/isError handler paths, operator-only dedupe)

Generated by an AI agent (Claude Opus 4.8).

Wires the now-merged unified tool registry (#615/#781) Operations profile
(#617) into the built-in agent's toolset, per #626's tool-composition design.

- registryTools.ts: ensureOperationsToolsRegistered() populates the registry
  on the main thread (idempotent — independent of whether the operator enabled
  the mcp: HTTP surface); composeRegistryTools(agentUser, sessionId) drains
  listTools(operations) and adapts each ToolDescriptor to an AgentTool, mapping
  annotations.destructiveHint to the loop's approval gate and wrapping
  getTool().handler (ToolResult -> structuredContent, isError -> throw).
- agent.ts: resolves the configured agent user via server.getUser(config.user)
  (super_user fallback if unavailable) so tools are RBAC-filtered for that
  identity, then composes registry + operator-only tools.
- toolset.ts: merges the two sources; operator-only tools win on name
  collisions and are never shadowed by a same-named registry tool.

Scope: Operations profile only. The Application profile (#618, per-Resource
tools) populates from the worker-thread Resources registry and is a follow-up.
The manual loop still stands until toolMode:'auto' (#612) lands.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kriszyp
kriszyp requested a review from Ethan-Arrowood June 1, 2026 03:40

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request integrates the unified MCP tool registry (specifically the Operations profile) into the built-in agent's toolset, allowing RBAC-filtered tools to be consumed by the agent based on its configured user. It introduces a new registryTools.ts file to bridge the registry and the agent, updates the tool composer to prioritize operator-only tools on name collisions, and adds corresponding unit tests. Feedback on the changes highlights a critical security vulnerability in resolveAgentUser where failing to resolve a restricted user falls back to granting full super_user permissions; it is recommended to fail-closed for non-default users. Additionally, it is suggested to change registryTools from const to let to support dynamic configuration updates.

Comment thread agent/agent.ts
Comment on lines +188 to +203
async function resolveAgentUser(username: string): Promise<AuthedUser> {
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<AuthedUser>) | 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 } } };
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-critical critical

The current fallback logic in resolveAgentUser is a critical security risk. If any user fails to resolve (or exists but has no explicit permissions defined yet), the function silently falls back to granting them full super_user permissions. This allows privilege escalation.

We should only fall back to super_user if the configured user is the default 'hdb_agent'. For any other explicitly configured user, we should fail-closed by throwing an error to prevent the agent from starting with over-granted privileges.

async function resolveAgentUser(username: string): Promise<AuthedUser> {
	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<AuthedUser>) | undefined;
		if (typeof getUser === 'function') {
			const user = await getUser(username);
			if (user) return user;
		}
	} catch (err) {
		log.warn?.(
			"Agent: could not resolve user '" + username + "' (" + ((err as Error)?.message ?? err) + ")"
		);
	}
	if (username === 'hdb_agent') {
		return { username, role: { permission: { super_user: true } } };
	}
	throw new Error("Agent: failed to resolve non-default user '" + username + "' and refused super_user fallback");
}

Comment thread agent/agent.ts
// the worker-thread Resources registry.)
ensureOperationsToolsRegistered();
const agentUser = await resolveAgentUser(liveConfig.user);
const registryTools = composeRegistryTools(agentUser, `agent:registry:${liveConfig.user}`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Change const to let so that registryTools can be reassigned when the configured user is dynamically updated via setConfig.

Suggested change
const registryTools = composeRegistryTools(agentUser, `agent:registry:${liveConfig.user}`);
let registryTools = composeRegistryTools(agentUser, "agent:registry:" + liveConfig.user);

@claude

claude Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

@kriszyp
kriszyp deleted the branch feat/builtin-agent-component June 2, 2026 17:38
@kriszyp kriszyp closed this Jun 2, 2026
kriszyp pushed a commit that referenced this pull request Jul 2, 2026
…tered (#626)

Fold the unified MCP tool registry's Operations profile (#617) into the
built-in agent's toolset, fulfilling #626's tool-composition design: the agent
consumes the same registry the MCP server exposes, RBAC-filtered for its
configured user. Re-does the intent of the abandoned #893 against current main
(that branch was ~465 commits stale and used the pre-merge registry API).

- agent/registryTools.ts (new): ensureOperationsToolsRegistered() populates the
  Operations profile on the main thread (idempotent); composeRegistryTools()
  snapshots the profile, filters by visibleTo(agentUser), and adapts each
  ToolDef -> AgentTool (inputSchema->parameters, destructiveHint->approval gate,
  ToolResult->return value with isError->throw so the loop records a recoverable
  failure rather than aborting).
- agent/agent.ts: resolve the configured agent user via server.getUser (falls
  back to a super_user identity with a warning when unresolved) and thread the
  registry tools through both composeToolset calls.
- agent/toolset.ts: merge operator-only + registry tools; operator-only tools
  win on a name collision.
- unitTests/agent/registryTools.test.js: 10 tests over RBAC filtering, shape
  adaptation, destructive gating, result unwrapping, and the collision rule.
  No server boot / no LLM credits.

Enforcement note: visibleTo controls listing only; real RBAC runs per-call in
the operation handler via hdb_user set to the agent's identity.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kriszyp added a commit that referenced this pull request Jul 4, 2026
…tered (#626) (#1545)

* feat(agent): consume MCP tool registry (Operations profile), RBAC-filtered (#626)

Fold the unified MCP tool registry's Operations profile (#617) into the
built-in agent's toolset, fulfilling #626's tool-composition design: the agent
consumes the same registry the MCP server exposes, RBAC-filtered for its
configured user. Re-does the intent of the abandoned #893 against current main
(that branch was ~465 commits stale and used the pre-merge registry API).

- agent/registryTools.ts (new): ensureOperationsToolsRegistered() populates the
  Operations profile on the main thread (idempotent); composeRegistryTools()
  snapshots the profile, filters by visibleTo(agentUser), and adapts each
  ToolDef -> AgentTool (inputSchema->parameters, destructiveHint->approval gate,
  ToolResult->return value with isError->throw so the loop records a recoverable
  failure rather than aborting).
- agent/agent.ts: resolve the configured agent user via server.getUser (falls
  back to a super_user identity with a warning when unresolved) and thread the
  registry tools through both composeToolset calls.
- agent/toolset.ts: merge operator-only + registry tools; operator-only tools
  win on a name collision.
- unitTests/agent/registryTools.test.js: 10 tests over RBAC filtering, shape
  adaptation, destructive gating, result unwrapping, and the collision rule.
  No server boot / no LLM credits.

Enforcement note: visibleTo controls listing only; real RBAC runs per-call in
the operation handler via hdb_user set to the agent's identity.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(agent): fail closed + per-call identity for registry tools (#626)

Cross-model review (Codex + Gemini) flagged two authorization defects in the
registry integration; both fixed at root:

- Fail-open super_user: resolveAgentUser previously fabricated a
  { super_user: true } identity whenever agent.user couldn't be resolved,
  silently escalating a misconfigured or transient restricted service account to
  admin. Now resolveAgentIdentity only falls back to super_user for the *default*
  hdb_agent bootstrap user (whose provisioning #626 defers); an explicitly
  configured user that won't resolve throws (fail closed), and the agent runs with
  only its operator-only tools until the operator fixes agent.user.

- Stale cached identity: the agent user was resolved once at startup and closed
  over in every tool handler, so a role revocation/change wasn't honored until
  restart. The enforcement identity is now resolved *per call* (composeRegistryTools
  takes a resolveIdentity thunk), mirroring how the MCP HTTP path re-auths per
  request. The startup snapshot is used only for visibleTo listing (not a boundary).

Adjudicated out: Gemini's "setConfig ignores agent.user changes" — set_agent_config
does not accept `user` (not in its patch keys), so it can't change at runtime.

Adds 2 unit tests: per-call re-resolution honors a live role change; a fail-closed
rejection propagates and the operation never dispatches. 51 agent unit tests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(agent): V8 inspector (CDP) tools for debugging/profiling workers (#626) (#1547)

* feat(agent): V8 inspector (CDP) tools for debugging/profiling workers (#626)

Adds the operator-only inspector tool surface from the #626 design — the last
of the operator tools not yet built. The agent (main thread) attaches over the
Chrome DevTools Protocol to *worker* thread inspector ports to evaluate, set
breakpoints/logpoints, and CPU-profile a worker without stalling the thread it
runs on.

- agent/tools/inspectorTool.ts (new): buildInspectorTools(deps) → five tools:
  inspector_attach, inspector_evaluate, inspector_set_breakpoint,
  inspector_set_logpoint, inspector_profile_cpu. A minimal CDP client over `ws`
  (id-correlated request/response + event listeners), one reused connection per
  worker debug port. Deps (debug config + live worker count) are injected so the
  tool is unit-testable without a server boot.
- Safety envelope (resolvePort): requires threads_debug + threads_debug_startingPort,
  range-checks workerIndex against the live worker pool, and REJECTS workerIndex < 0
  (the main thread — a self-attach/breakpoint would deadlock the agent).
- Worker debug port = threads_debug_startingPort + workerIndex (mirrors
  server/threads/threadServer.js). Logpoints are non-pausing (breakpoint whose
  condition console.logs and returns false). CPU profiles are summarized to the
  hottest functions by self time so the observation stays small.
- evaluate + set_breakpoint are marked destructive (arbitrary in-worker code /
  can pause a live worker) → gated by the loop's approval flow.
- agent/toolset.ts + agent/agent.ts: compose inspector tools into the operator-only
  set; deps read from env (threads_debug*) and the live `workers` pool.
- unitTests/agent/inspectorTool.test.js: safety-envelope guards, summarizeProfile,
  and a LIVE CDP round-trip (opens a real inspector on an ephemeral port; drives
  attach + evaluate + profile). 60 agent unit tests green.

Stacked on #1545 (registry tools).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(agent): harden inspector CDP client after cross-model review (#626)

Cross-model review (Gemini) flagged real lifecycle/concurrency hazards in the
CDP tooling; all addressed:

- openCdp never settled if the socket closed/aborted before 'open' → sessionFor
  hung the agent. Now a pre-open close/error/abort rejects the connect promise.
- Session-cache races: cache the connection *promise* (dedupes concurrent opens);
  evict by identity so a stale close from a replaced connection can't drop a live
  session; drop a failed connect so the next call retries.
- Breakpoints could wedge a worker (paused, no resume path). Every connection now
  registers a Debugger.paused handler that logs a stack snapshot and auto-resumes,
  so a hit is observable via the Harper log but never leaves the worker paused.
- Logpoint injection: logExpression is now JSON-encoded and eval'd (not spliced as
  raw code), so it can't break out of the wrapper to force a pause. set_logpoint
  is also marked destructive (it runs an expression in the worker on every hit),
  alongside evaluate and set_breakpoint.
- CDP calls now carry a per-call abort signal + 30s timeout, so an unresponsive
  worker can't hang the agent loop. The connect signal guards only the handshake
  (aborting one caller no longer tears down the shared connection).
- Strict workerIndex parsing: reject ""/null/false/[] instead of Number()-coercing
  them to worker 0.
- Profiler.disable on cleanup; resolvePort rejects a port past 65535.
- _closeInspectorSessions handles in-flight opens.

Also fixes a test-teardown deadlock: inspector.close() blocks until CDP clients
drop, so the live-round-trip after() hook only closes our client and lets --exit
tear down the inspector. Full agent unit suite: 61 passing, build clean.

Codex leg returned no structured findings.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(agent): tolerate an already-active inspector in CDP round-trip test (#626)

The dev-mode install CI uses for unit tests sets threads.debug: true, so
threadServer opens the V8 inspector on the main process during the suite.
The live-CDP test's before hook then called inspector.open() unconditionally
and threw ERR_INSPECTOR_ALREADY_ACTIVATED. Reuse the live inspector when one
is already open; only open our own ephemeral port otherwise.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Kris Zyp <kris@harperdb.io>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Kris Zyp <kris@harperdb.io>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
kriszyp added a commit that referenced this pull request Jul 11, 2026
…nd (#626) (#1560)

* feat(agent): consume MCP tool registry (Operations profile), RBAC-filtered (#626)

Fold the unified MCP tool registry's Operations profile (#617) into the
built-in agent's toolset, fulfilling #626's tool-composition design: the agent
consumes the same registry the MCP server exposes, RBAC-filtered for its
configured user. Re-does the intent of the abandoned #893 against current main
(that branch was ~465 commits stale and used the pre-merge registry API).

- agent/registryTools.ts (new): ensureOperationsToolsRegistered() populates the
  Operations profile on the main thread (idempotent); composeRegistryTools()
  snapshots the profile, filters by visibleTo(agentUser), and adapts each
  ToolDef -> AgentTool (inputSchema->parameters, destructiveHint->approval gate,
  ToolResult->return value with isError->throw so the loop records a recoverable
  failure rather than aborting).
- agent/agent.ts: resolve the configured agent user via server.getUser (falls
  back to a super_user identity with a warning when unresolved) and thread the
  registry tools through both composeToolset calls.
- agent/toolset.ts: merge operator-only + registry tools; operator-only tools
  win on a name collision.
- unitTests/agent/registryTools.test.js: 10 tests over RBAC filtering, shape
  adaptation, destructive gating, result unwrapping, and the collision rule.
  No server boot / no LLM credits.

Enforcement note: visibleTo controls listing only; real RBAC runs per-call in
the operation handler via hdb_user set to the agent's identity.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(agent): fail closed + per-call identity for registry tools (#626)

Cross-model review (Codex + Gemini) flagged two authorization defects in the
registry integration; both fixed at root:

- Fail-open super_user: resolveAgentUser previously fabricated a
  { super_user: true } identity whenever agent.user couldn't be resolved,
  silently escalating a misconfigured or transient restricted service account to
  admin. Now resolveAgentIdentity only falls back to super_user for the *default*
  hdb_agent bootstrap user (whose provisioning #626 defers); an explicitly
  configured user that won't resolve throws (fail closed), and the agent runs with
  only its operator-only tools until the operator fixes agent.user.

- Stale cached identity: the agent user was resolved once at startup and closed
  over in every tool handler, so a role revocation/change wasn't honored until
  restart. The enforcement identity is now resolved *per call* (composeRegistryTools
  takes a resolveIdentity thunk), mirroring how the MCP HTTP path re-auths per
  request. The startup snapshot is used only for visibleTo listing (not a boundary).

Adjudicated out: Gemini's "setConfig ignores agent.user changes" — set_agent_config
does not accept `user` (not in its patch keys), so it can't change at runtime.

Adds 2 unit tests: per-call re-resolution honors a live role change; a fail-closed
rejection propagates and the operation never dispatches. 51 agent unit tests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(agent): V8 inspector (CDP) tools for debugging/profiling workers (#626)

Adds the operator-only inspector tool surface from the #626 design — the last
of the operator tools not yet built. The agent (main thread) attaches over the
Chrome DevTools Protocol to *worker* thread inspector ports to evaluate, set
breakpoints/logpoints, and CPU-profile a worker without stalling the thread it
runs on.

- agent/tools/inspectorTool.ts (new): buildInspectorTools(deps) → five tools:
  inspector_attach, inspector_evaluate, inspector_set_breakpoint,
  inspector_set_logpoint, inspector_profile_cpu. A minimal CDP client over `ws`
  (id-correlated request/response + event listeners), one reused connection per
  worker debug port. Deps (debug config + live worker count) are injected so the
  tool is unit-testable without a server boot.
- Safety envelope (resolvePort): requires threads_debug + threads_debug_startingPort,
  range-checks workerIndex against the live worker pool, and REJECTS workerIndex < 0
  (the main thread — a self-attach/breakpoint would deadlock the agent).
- Worker debug port = threads_debug_startingPort + workerIndex (mirrors
  server/threads/threadServer.js). Logpoints are non-pausing (breakpoint whose
  condition console.logs and returns false). CPU profiles are summarized to the
  hottest functions by self time so the observation stays small.
- evaluate + set_breakpoint are marked destructive (arbitrary in-worker code /
  can pause a live worker) → gated by the loop's approval flow.
- agent/toolset.ts + agent/agent.ts: compose inspector tools into the operator-only
  set; deps read from env (threads_debug*) and the live `workers` pool.
- unitTests/agent/inspectorTool.test.js: safety-envelope guards, summarizeProfile,
  and a LIVE CDP round-trip (opens a real inspector on an ephemeral port; drives
  attach + evaluate + profile). 60 agent unit tests green.

Stacked on #1545 (registry tools).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(agent): harden inspector CDP client after cross-model review (#626)

Cross-model review (Gemini) flagged real lifecycle/concurrency hazards in the
CDP tooling; all addressed:

- openCdp never settled if the socket closed/aborted before 'open' → sessionFor
  hung the agent. Now a pre-open close/error/abort rejects the connect promise.
- Session-cache races: cache the connection *promise* (dedupes concurrent opens);
  evict by identity so a stale close from a replaced connection can't drop a live
  session; drop a failed connect so the next call retries.
- Breakpoints could wedge a worker (paused, no resume path). Every connection now
  registers a Debugger.paused handler that logs a stack snapshot and auto-resumes,
  so a hit is observable via the Harper log but never leaves the worker paused.
- Logpoint injection: logExpression is now JSON-encoded and eval'd (not spliced as
  raw code), so it can't break out of the wrapper to force a pause. set_logpoint
  is also marked destructive (it runs an expression in the worker on every hit),
  alongside evaluate and set_breakpoint.
- CDP calls now carry a per-call abort signal + 30s timeout, so an unresponsive
  worker can't hang the agent loop. The connect signal guards only the handshake
  (aborting one caller no longer tears down the shared connection).
- Strict workerIndex parsing: reject ""/null/false/[] instead of Number()-coercing
  them to worker 0.
- Profiler.disable on cleanup; resolvePort rejects a port past 65535.
- _closeInspectorSessions handles in-flight opens.

Also fixes a test-teardown deadlock: inspector.close() blocks until CDP clients
drop, so the live-round-trip after() hook only closes our client and lets --exit
tear down the inspector. Full agent unit suite: 61 passing, build clean.

Codex leg returned no structured findings.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(agent): Harper best-practices grounding + agent.systemPromptAppend (#626)

Gives the built-in agent Harper conventions so it builds idiomatic apps, and
lets operators tune its persona/policy without a rebuild.

- Depends on @harperfast/skills and sources the `harper-best-practices` skill
  from it (versioned, no drift). Progressive disclosure, mirroring the skill's
  own design:
    * agent/bestPractices.ts (new): loadBestPracticesOverview() injects the
      SKILL.md overview (rule index + when-to-use, ~1.2k tokens) into the system
      prompt; buildBestPracticeTool() exposes a `harper_best_practice` tool that
      lists rules (no arg) or returns rules/<name>.md on demand — so the agent
      only spends context on the guidance relevant to the task. Both degrade to
      nothing if the package isn't resolvable (agent still runs). Rule arg is
      guarded against path traversal.
- agent.systemPromptAppend: operator text appended after the built-in grounding
  and the best-practices overview. Read from liveConfig each run, and accepted by
  set_agent_config, so it can be tuned on a running instance. Added to the config
  schema and AgentConfig.
- System prompt is now assembled: built-in grounding → best-practices overview →
  operator append.

Stacked on the inspector PR (kris/agent-inspector). Agent unit suite green
(incl. new bestPractices tests exercising the real skill package).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(agent): tolerate a pre-existing inspector session in the live CDP test (#626)

node:inspector is a single process-wide agent. In CI, threads_debug defaults
to true (installer.ts DEV_MODE_CONFIG), and server/threads/threadServer.js
opens the main-thread inspector as a top-level side effect of merely being
imported (e.g. via DurableSubscriptionsSession.ts pulling in
whenComponentsLoaded) whenever that config is on. That leaves this process's
one inspector slot already occupied by the time the "live CDP round-trip"
suite's before() hook runs, so inspector.open() throws
ERR_INSPECTOR_ALREADY_ACTIVATED.

Check inspector.url() first and reuse whatever is already listening instead
of assuming the suite is the sole owner of the process's debug port.

Verified by reproducing the exact CI error locally (mocha --require a script
that opens the inspector before test files load), confirming the fix resolves
it, and running the suite in isolation, within unitTests/agent/**, and across
3 full unitTests/**/*test.*js runs (2985 passing, 0 failing each time).

* docs(#626): document @harperfast/skills dependency; drop node:assert/strict

* feat(agent): expose the built-in agent over MCP (curated tools) (#626)

Lets any MCP client drive the built-in agent (agent_prompt, get_agent_session,
list_agent_sessions, approve_agent_action, cancel_agent_run).

Why not just `mcp.operations.allow`: the generic MCP operations profile walks
OPERATION_FUNCTION_MAP once, BEFORE this component registers its operations, so
the agent ops are added to the map too late for the walk — allow-listing them has
no effect (confirmed live: profile builds at 14 tools, then the 6 agent ops
register; tools/list never shows them). So we register a curated agent tool set
directly into the registry AFTER the ops exist.

- agent/mcpTools.ts (new): registerAgentMcpTools(operations) adds curated tools
  (proper schemas/descriptions, destructive/read-only annotations) on the
  operations profile. visibleTo is super_user-only for listing; each handler
  dispatches to the operation's execute with the MCP caller as hdb_user, so the
  op's own super_user check + downstream RBAC still apply. set_agent_config is
  intentionally NOT exposed (operator/config action).
- agent/agent.ts: call it after registering the ops. Tools sit inertly in the
  registry unless the MCP HTTP surface is enabled.

Verified live over the MCP Streamable HTTP transport: an MCP client completed
initialize → tools/list (agent tools present) → tools/call agent_prompt →
get_agent_session, and the agent ran a tool and answered.

Stacked on the best-practices PR (kris/agent-bestpractices).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(agent): preserve non-standard error details in MCP tool error responses

Fall back to String(err) before the generic message so string/plain-object
thrown values aren't reduced to a useless generic message (#1561).

* fix(agent): read best-practices from @harperfast/skills exports, not the filesystem (#626)

Per review, @harperfast/skills exposes its skill content directly as module
exports — skillSummary (SKILL.md), ruleNames, and a rules name→markdown map —
so there's no need to resolve the package on disk and read files by hand.

- bestPractices.ts: import { ruleNames, rules, skillSummary } and serve from
  them. This removes the sync readdirSync/readFileSync in the tool handler (no
  event-loop blocking on the main thread), the require.resolve path walk (ESM
  require-undefined hazard), and the path-traversal regex — rule lookup is now a
  plain map access, so malformed/traversal names simply miss.
- agent.ts: tighten the systemPromptAppend guard to an explicit string check
  before trim().
- dependencies.md: rewrite the @harperfast/skills entry to reflect module-export
  consumption (no fs access, rule bodies resident in heap, hard runtime dep).
- test: traversal/malformed names now surface as "No such best-practice rule".

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Kris Zyp <kris@harperdb.io>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant