You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A real integration surfaced a gap in our tool model.
xeloxa/temodar-agent is an AI-powered WordPress security analyzer built on @jackchen_me/open-multi-agent. It wires up most of the framework's orchestration surface (Agent, AgentPool, OpenMultiAgent, runTeam, runTasks, ToolRegistry, ToolExecutor, the built-in tools, defineTool, onProgress, onTrace, beforeRun/afterRun, outputSchema, loopDetection). On top of it, the integrator hand-rolled a per-call risk gate (classifyBashRisk / classifyToolRisk) that sits inside every tool's execute function via a wrapToolWithEvents helper.
The gate classifies each invocation as safe | review | high:
review — ambiguous invocations request human confirmation
high — destructive/sensitive patterns (rm, sudo, curl | bash) block by default
In a follow-up discussion (xeloxa/temodar-agent#9 comment) the author articulated the underlying problem in their own words:
OMA's tool system solves "which tools can this agent access?", but we also needed a separate check for "should this specific invocation be allowed right now?". That mattered especially for broad tools like bash, where ls and pwd are very different from rm, sudo, or networked commands.
And, on whether this belongs in the framework:
per-call risk/approval feels like a separate concern that could make sense as a native pattern, especially for shell and filesystem tools.
This is a concrete signal from a production user that the framework has no seam for call-level gating, and that users end up monkey-patching every tool definition to get one.
Problem
Today the framework has two layers of tool control:
Nothing at the call site — once a tool is allowed, every invocation runs. bash is a single allowed name that covers ls -la and rm -rf / equally.
The integrator's workaround is to wrap every ToolDefinition.execute at registration time and inline their own classifier + approval loop (reference). This works but is invasive: it couples risk logic to tool wiring, has to be repeated per tool, and cannot compose with onTrace or future observability cleanly.
The goal of this proposal is to add a minimal, opt-in seam one layer down from #83 so that call-level policies can be written in user code without wrapping tool definitions.
Proposal
Three pieces, all opt-in, all off by default.
1. onToolCall middleware hook
A single async callback that fires once per tool invocation, after Zod validation, before tool.execute() runs. Natural insertion point is in ToolExecutor.runTool() (src/tool/executor.ts, between lines 147 and 158).
Rough shape (follows the existing beforeRun / afterRun style of sync-or-async):
interfaceToolCallContext{readonlytoolName: stringreadonlyinput: Record<string,unknown>// post-schema-validationreadonlyagentName: stringreadonlyrunId?: stringreadonlytaskId?: string}typeToolCallDecision=|{action: 'allow'}|{action: 'deny';reason?: string}// New hook, added alongside existing ones (NOT to be confused with// the task-level OrchestratorConfig.onApproval, which fires between// orchestration rounds, not per tool call).
readonly onToolCall?: (context: ToolCallContext)=>ToolCallDecision|Promise<ToolCallDecision>
On deny the executor synthesizes a standard error ToolResult (same path as existing validation failures) carrying the reason string. No exception is thrown — the LLM sees a structured refusal and can adapt.
Human-in-the-loop is handled inside the user's callback. They await their own UI / CLI / file-based prompt, then return allow or deny. The framework does not prescribe a review state or a separate approval channel; that keeps the API surface small and lets users plug in whatever review mechanism suits their app.
2. Optional shell risk classifier
A small helper module exported separately, off by default, for users who want the xeloxa safe | review | high semantics without writing regex tables from scratch. Rough shape:
Implementation lands independently and uses pattern lists written from scratch (no code copied from temodar-agent, which is Apache-2.0). Users can extend or fully replace it; it is convenience, not policy.
3. Trace integration
Extend ToolCallTrace with optional fields so gate decisions flow through onTrace for observability:
New onToolCall hook on OrchestratorConfig (and likely AgentConfig — see open questions)
Insertion point in ToolExecutor.runTool()
Optional classifier helper for bash patterns, shipped behind a subpath export
ToolCallTrace extension for observability
Documentation showing the xeloxa-style use case end-to-end
Out:
A general policy engine (rate limits, quotas, RBAC, per-user scoping). If someone needs those they can build them inside the hook.
Changes to feat: add tool allowlist, denylist, preset list #83's allowlist/denylist resolution. That layer answers "is this tool reachable"; this proposal answers "is this call allowed". Both layers coexist unchanged.
Risk gating limited to built-ins. Because the middleware lives at ToolExecutor.runTool(), user-defined tools pass through the same hook for free. This is a nice side-effect, not the primary goal.
Non-goals
This is not a sandbox or security boundary. A middleware that returns deny still relies on cooperating code. If the threat model requires real isolation, use Docker / a VM / seccomp — the hook is a coordination layer, not containment.
Not prescriptive. The primary API is the raw hook. The shell classifier is convenience only and sits behind a secondary export.
Not a replacement for onApproval. That hook gates between orchestrator task rounds and stays as-is. The two operate at orthogonal layers.
Open questions
Scope of the hook.OrchestratorConfig, AgentConfig, or both with agent override? Both would let a team default the policy while one specialist agent relaxes it. Precedent supports both styles (onApproval is orchestrator-level, beforeRun/afterRun are agent-level).
Single hook or array of middleware. Array composes better (classify + log + audit), single is simpler to document. Suggest starting with single, add array later if the need is real.
Classifier packaging. Core (open-multi-agent/classifiers subpath export) or separate npm package? Core keeps the install surface small; separate package avoids bloating the core with pattern tables.
Abort on deny vs error result. Deny synthesizing an error ToolResult lets the LLM recover and retry, which is usually the right thing. An alternative would be to abort the whole run; probably too blunt as the default but worth noting.
Trace semantics. Should gated: true, gateAction: 'allow' emit even on pass-through (for audit logs), or only on deny (to keep trace volume low)?
Interaction with disallowedTools. A denied tool should never reach the hook (current behavior); the hook only runs for tools already resolved as reachable. Worth documenting explicitly.
Prior art in existing hooks: AgentConfig.beforeRun / afterRun (src/types.ts:241), OrchestratorConfig.onApproval (src/types.ts:428, task-level — not to be confused with this proposal)
Status
Candidate for a future v1.x minor. Happy to split implementation across:
Core hook + executor integration + trace field (small PR)
Motivation
A real integration surfaced a gap in our tool model.
xeloxa/temodar-agentis an AI-powered WordPress security analyzer built on@jackchen_me/open-multi-agent. It wires up most of the framework's orchestration surface (Agent,AgentPool,OpenMultiAgent,runTeam,runTasks,ToolRegistry,ToolExecutor, the built-in tools,defineTool,onProgress,onTrace,beforeRun/afterRun,outputSchema,loopDetection). On top of it, the integrator hand-rolled a per-call risk gate (classifyBashRisk/classifyToolRisk) that sits inside every tool'sexecutefunction via awrapToolWithEventshelper.The gate classifies each invocation as
safe | review | high:safe— read-only commands (ls,cat,grep) auto-passreview— ambiguous invocations request human confirmationhigh— destructive/sensitive patterns (rm,sudo,curl | bash) block by defaultIn a follow-up discussion (xeloxa/temodar-agent#9 comment) the author articulated the underlying problem in their own words:
And, on whether this belongs in the framework:
This is a concrete signal from a production user that the framework has no seam for call-level gating, and that users end up monkey-patching every tool definition to get one.
Problem
Today the framework has two layers of tool control:
bashis a single allowed name that coversls -laandrm -rf /equally.The integrator's workaround is to wrap every
ToolDefinition.executeat registration time and inline their own classifier + approval loop (reference). This works but is invasive: it couples risk logic to tool wiring, has to be repeated per tool, and cannot compose withonTraceor future observability cleanly.The goal of this proposal is to add a minimal, opt-in seam one layer down from #83 so that call-level policies can be written in user code without wrapping tool definitions.
Proposal
Three pieces, all opt-in, all off by default.
1.
onToolCallmiddleware hookA single async callback that fires once per tool invocation, after Zod validation, before
tool.execute()runs. Natural insertion point is inToolExecutor.runTool()(src/tool/executor.ts, between lines 147 and 158).Rough shape (follows the existing
beforeRun/afterRunstyle of sync-or-async):On
denythe executor synthesizes a standard errorToolResult(same path as existing validation failures) carrying thereasonstring. No exception is thrown — the LLM sees a structured refusal and can adapt.Human-in-the-loop is handled inside the user's callback. They
awaittheir own UI / CLI / file-based prompt, then returnallowordeny. The framework does not prescribe a review state or a separate approval channel; that keeps the API surface small and lets users plug in whatever review mechanism suits their app.2. Optional shell risk classifier
A small helper module exported separately, off by default, for users who want the xeloxa
safe | review | highsemantics without writing regex tables from scratch. Rough shape:Implementation lands independently and uses pattern lists written from scratch (no code copied from
temodar-agent, which is Apache-2.0). Users can extend or fully replace it; it is convenience, not policy.3. Trace integration
Extend
ToolCallTracewith optional fields so gate decisions flow throughonTracefor observability:Scope
In:
onToolCallhook onOrchestratorConfig(and likelyAgentConfig— see open questions)ToolExecutor.runTool()bashpatterns, shipped behind a subpath exportToolCallTraceextension for observabilityOut:
ToolExecutorso MCP tools would benefit automatically once they route through the same executor, but no MCP-specific work is in scope here.ToolExecutor.runTool(), user-defined tools pass through the same hook for free. This is a nice side-effect, not the primary goal.Non-goals
denystill relies on cooperating code. If the threat model requires real isolation, use Docker / a VM / seccomp — the hook is a coordination layer, not containment.onApproval. That hook gates between orchestrator task rounds and stays as-is. The two operate at orthogonal layers.Open questions
OrchestratorConfig,AgentConfig, or both with agent override? Both would let a team default the policy while one specialist agent relaxes it. Precedent supports both styles (onApprovalis orchestrator-level,beforeRun/afterRunare agent-level).open-multi-agent/classifierssubpath export) or separate npm package? Core keeps the install surface small; separate package avoids bloating the core with pattern tables.ToolResultlets the LLM recover and retry, which is usually the right thing. An alternative would be to abort the whole run; probably too blunt as the default but worth noting.gated: true, gateAction: 'allow'emit even on pass-through (for audit logs), or only on deny (to keep trace volume low)?disallowedTools. A denied tool should never reach the hook (current behavior); the hook only runs for tools already resolved as reachable. Worth documenting explicitly.References
ai/node_runner/src/index.tsAgentConfig.beforeRun/afterRun(src/types.ts:241),OrchestratorConfig.onApproval(src/types.ts:428, task-level — not to be confused with this proposal)Status
Candidate for a future v1.x minor. Happy to split implementation across:
No timeline commitment.