fix(agent): optional require_tool_call runtime fabrication guard#1490
Open
kgarg2468 wants to merge 2 commits into
Open
fix(agent): optional require_tool_call runtime fabrication guard#1490kgarg2468 wants to merge 2 commits into
kgarg2468 wants to merge 2 commits into
Conversation
No runtime guard asserted that an agent actually invoked a tool, so a weak planning model could narrate a multi-step tool chain in prose instead of calling the tools — producing a plausible but ungrounded answer. The "must call >=1 tool" rule existed only as a prompt instruction, never as code. Enforce it once in the shared AgentBase so it covers all five agent frameworks (langchain, llamaindex, rocketride, crewai, deepagent): - AgentContext gains a run-scoped `invoked_tools` tally (a mutable field on the otherwise-frozen context; compare=False keeps the context hashable, and list.append is thread-safe under the GIL). - call_tool — the single choke point every driver routes real tool calls through — records each invocation (memory.peek and other local reads bypass it and correctly do not count). - run_agent raises ToolCallRequiredError -> a distinct RocketRide.agent.guard.v1 error answer when require_tool_call is on and no tool ran; meta.tool_calls now reports the count on success, guard, and error. - The crewai-manager and deepagent sub-contexts thread the parent's tally so tools a sub-agent invokes count toward the parent run's guard. Exposed as an off-by-default `require_tool_call` boolean on the six top-level agent service files, read with parse_bool (a stringified "false" leaves it off, rather than tripping the guard), with README notes + regenerated schema tables. Tests (packages/ai/tests/ai/common/agent/ + test_config_shapes.py): AgentBase guard unit tests, config resolution (flat/nested/default shapes), and end-to-end integration driving the real RocketRide wave loop and the real LangChain create_agent loop — fabrication trips the guard, grounding passes, memory.peek-only trips, and parallel tools in one wave are all counted. Closes #1403 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kgarg2468
requested review from
Rod-Christensen,
jmaionchi and
stepmikhaylov
as code owners
July 7, 2026 18:21
github-actions
Bot
added
docs
Contributor
📝 WalkthroughWalkthroughAdds an optional Changesrequire_tool_call fabrication guard
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ai/src/ai/common/agent/agent.py (1)
216-291: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDuplicate
metapayload construction across success/guard/error branches.The success, guard-trip, and generic-error branches each rebuild the same
metascaffold (framework, agent_id, run_id, started_at, ended_at, task_id handling) with slightly different styles (inline conditional-merge vs. post-hoc assignment). Extracting a small helper (e.g._base_meta(iInstance, run_id, started_at, ended_at, task_id)) would remove the duplication and prevent future field additions from silently missing one of the three branches.♻️ Sketch of shared helper
def _base_meta(self, iInstance, run_id, started_at, ended_at, task_id): meta = { 'framework': self.FRAMEWORK, 'agent_id': self._agent_id(iInstance), 'run_id': run_id, 'started_at': started_at, 'ended_at': ended_at, } if task_id: meta['task_id'] = task_id return metaEach branch would then do
meta = self._base_meta(...); meta['tool_calls'] = ....🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai/src/ai/common/agent/agent.py` around lines 216 - 291, The success, guard-trip, and error paths in the agent run flow all reconstruct the same meta fields, making the logic duplicated and easy to drift. Extract the shared meta assembly in the agent base _run logic into a helper such as _base_meta (or equivalent) that builds framework, agent_id, run_id, started_at, ended_at, and optional task_id, then have each branch in agent.py reuse it and set only branch-specific fields like tool_calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nodes/src/nodes/agent_crewai/README.md`:
- Around line 106-117: Update the Fabrication guard docs in README.md to match
the runtime contract used by the guard failure path: the failure error code
should be described as RocketRide.agent.guard.v1 instead of
RocketRide.agent.guard. Keep the existing explanation of require_tool_call and
tool-invocation behavior, but revise the guard failure wording so it matches
what consumers receive from the agent guard implementation.
In `@nodes/src/nodes/agent_crewai/services.manager.json`:
- Around line 58-63: Clarify the require_tool_call setting in
services.manager.json so it explicitly covers delegated subagent tool usage, not
just direct tool calls by the CrewAI Manager. Update the description for the
require_tool_call field to note that tool calls made by delegated subagents are
counted toward the manager’s total via sub-context propagation, while keeping
the existing guard behavior and referencing the require_tool_call setting for
easy location.
In `@nodes/src/nodes/agent_deepagent/README.md`:
- Around line 110-118: Update the Fabrication guard documentation in the README
section that describes require_tool_call so the guard failure name matches the
runtime contract used by the agent guard logic. Replace the old
RocketRide.agent.guard reference with RocketRide.agent.guard.v1 in this section,
keeping the surrounding explanation about real tool invocations and default-off
behavior unchanged.
In `@nodes/src/nodes/agent_langchain/README.md`:
- Around line 96-104: The Fabrication guard documentation still uses the
unversioned `RocketRide.agent.guard` identifier, which is inconsistent with the
Answer payload docs and the current guard failure contract. Update the
`Fabrication guard` section in the agent_langchain README to use
`RocketRide.agent.guard.v1` everywhere the guard error is mentioned, keeping the
wording aligned with the existing `Answer payload` documentation and
`require_tool_call` behavior.
In `@nodes/src/nodes/agent_llamaindex/README.md`:
- Around line 94-102: Update the Fabrication guard documentation in the
agent_llamaindex README so the guard failure payload matches the runtime
contract used by this PR. In the Require tool call description, replace the old
RocketRide.agent.guard identifier with RocketRide.agent.guard.v1, and keep the
wording aligned with the actual failure behavior exposed to consumers.
In `@packages/ai/src/ai/common/agent/agent.py`:
- Around line 237-264: The ToolCallRequiredError guard-trip path builds
answer_payload without the framework raw trace, unlike the normal success path.
Update the except ToolCallRequiredError block in agent.py so the existing raw
value in scope is attached to the stack alongside the current
RocketRide.agent.guard.v1 entry, using the same RocketRide.agent.raw.v1
structure as the success flow. Keep the guard response behavior unchanged, but
ensure answer_payload.stack preserves both the guard reason and raw trace for
debugging.
---
Outside diff comments:
In `@packages/ai/src/ai/common/agent/agent.py`:
- Around line 216-291: The success, guard-trip, and error paths in the agent run
flow all reconstruct the same meta fields, making the logic duplicated and easy
to drift. Extract the shared meta assembly in the agent base _run logic into a
helper such as _base_meta (or equivalent) that builds framework, agent_id,
run_id, started_at, ended_at, and optional task_id, then have each branch in
agent.py reuse it and set only branch-specific fields like tool_calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 40d99bb2-376c-44a6-93a6-5a75a4873eaa
📒 Files selected for processing (20)
nodes/src/nodes/agent_crewai/README.mdnodes/src/nodes/agent_crewai/crewai_manager/manager.pynodes/src/nodes/agent_crewai/services.agent.jsonnodes/src/nodes/agent_crewai/services.manager.jsonnodes/src/nodes/agent_deepagent/README.mdnodes/src/nodes/agent_deepagent/deepagent.pynodes/src/nodes/agent_deepagent/services.agent.jsonnodes/src/nodes/agent_langchain/README.mdnodes/src/nodes/agent_langchain/services.jsonnodes/src/nodes/agent_llamaindex/README.mdnodes/src/nodes/agent_llamaindex/services.jsonnodes/src/nodes/agent_rocketride/README.mdnodes/src/nodes/agent_rocketride/services.jsonpackages/ai/src/ai/common/agent/__init__.pypackages/ai/src/ai/common/agent/_internal/host.pypackages/ai/src/ai/common/agent/agent.pypackages/ai/tests/ai/common/agent/__init__.pypackages/ai/tests/ai/common/agent/test_agent_base.pypackages/ai/tests/ai/common/agent/test_require_tool_call_integration.pypackages/ai/tests/ai/common/test_config_shapes.py
Comment on lines
+58
to
+63
| "require_tool_call": { | ||
| "type": "boolean", | ||
| "title": "Require tool call", | ||
| "default": false, | ||
| "description": "Require the agent to invoke at least one tool before answering. When on, a run that answers without calling any tool fails with a guard error. Use for determinism-critical pipelines where an ungrounded or narrated answer must never be delivered. Off by default." | ||
| }, |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Clarify guard scope for delegated tool calls.
The description is generic ("invoke at least one tool before answering"), but a CrewAI Manager typically delegates work to subagents rather than calling tools directly. Per the stack context, delegated subagent tool calls are tallied into the manager's parent count via sub-context propagation — worth a short note here so operators don't misread the guard as requiring the manager itself to call a tool.
✏️ Suggested wording tweak
- "description": "Require the agent to invoke at least one tool before answering. When on, a run that answers without calling any tool fails with a guard error. Use for determinism-critical pipelines where an ungrounded or narrated answer must never be delivered. Off by default."
+ "description": "Require at least one tool call (including calls delegated to sub-agents) before answering. When on, a run that answers without any tool invocation fails with a guard error. Use for determinism-critical pipelines where an ungrounded or narrated answer must never be delivered. Off by default."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "require_tool_call": { | |
| "type": "boolean", | |
| "title": "Require tool call", | |
| "default": false, | |
| "description": "Require the agent to invoke at least one tool before answering. When on, a run that answers without calling any tool fails with a guard error. Use for determinism-critical pipelines where an ungrounded or narrated answer must never be delivered. Off by default." | |
| }, | |
| "require_tool_call": { | |
| "type": "boolean", | |
| "title": "Require tool call", | |
| "default": false, | |
| "description": "Require at least one tool call (including calls delegated to sub-agents) before answering. When on, a run that answers without any tool invocation fails with a guard error. Use for determinism-critical pipelines where an ungrounded or narrated answer must never be delivered. Off by default." | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nodes/src/nodes/agent_crewai/services.manager.json` around lines 58 - 63,
Clarify the require_tool_call setting in services.manager.json so it explicitly
covers delegated subagent tool usage, not just direct tool calls by the CrewAI
Manager. Update the description for the require_tool_call field to note that
tool calls made by delegated subagents are counted toward the manager’s total
via sub-context propagation, while keeping the existing guard behavior and
referencing the require_tool_call setting for easy location.
…swer Addresses PR review feedback: - The Fabrication-guard notes in all five agent READMEs referenced the unversioned `RocketRide.agent.guard`; the runtime emits `RocketRide.agent.guard.v1` (matching the existing `.raw.v1` / `.error.v1` identifiers). Use the versioned id consistently. - The guard-trip answer now also carries the framework's raw output as a `RocketRide.agent.raw.v1` stack entry (like the success path), so an operator investigating a tripped guard can inspect exactly what was narrated. `raw` is bound before the try, so this is safe on every path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Thanks @coderabbitai — addressed in Fixed
Considered, keeping as-is
|
Contributor
|
Tip For best results, initiate chat on the files or code changes.
On the CrewAI Manager description: agreed, keeping it as-is makes sense given the shared ✏️ Learnings added
|
kgarg2468
enabled auto-merge (squash)
July 9, 2026 00:27
joshuadarron
approved these changes
Jul 9, 2026
joshuadarron
left a comment
joshuadarron
left a comment
Collaborator
There was a problem hiding this comment.
Review
Verdict: Approve. Default-off guard, single enforcement point, thorough tests, CI green.
Verified against the code (not just the diff)
- Choke point holds:
AgentBase.call_toolis the only placecontext.tools.invokeis reached (agent.py:457); all five drivers route real tool calls through it (crewai_base.py:370, deepagent.py:204, langchain.py:189, llamaindex.py:123, executor.py:462). No bypass path found. - Sub-context threading is per-run: both crewai-manager (manager.py:220) and deepagent (
_collect_subagents, called from_runat deepagent.py:344) build sub-contexts inside the per-run path with the per-run parent list — no stale tally leaking across runs. - Hashability:
invoked_toolsusesfield(default_factory=list, compare=False)on the frozen dataclass, and there's a regression test for it. - Coercion:
parse_boolexists inai.common.utils.config_utilsand handles stringified "false"/"0"/"no";bool("false")footgun avoided and tested. - Error path:
context = None/raw = Nonebound before the try, so both except handlers are safe if_runraises early; guard answer retains the rejected framework output on the stack for diagnosis. - Counting before
invoke()(so a raising tool still counts as a grounding attempt) is a reasonable, documented choice.
Non-blocking nit
test_config_shapes.py— theTestRequireToolCallResolutiondocstring says AgentBase reads the flag "exactly asbool(config.get('require_tool_call', False))", but__init__usesparse_bool. Stale comment only; fine to fix in a follow-up.
CI: all builds, ruff, gitleaks, contracts green. Docs/schema/README generated tables all updated consistently across the six service files.
🤖 Generated with Claude Code
asclearuc
approved these changes
Jul 17, 2026
asclearuc
left a comment
asclearuc
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for this, @kgarg2468 — this is a well-scoped, well-tested fix. Turning the "must call ≥1 tool" rule from a prompt instruction into a real runtime check closes a genuine fabrication gap, and doing it once in AgentBase.call_tool so all five frameworks are covered is the right call. Nice touches: compare=False to keep the frozen AgentContext hashable (with a regression test), parse_bool instead of bool() for string-safe coercion, and sharing the parent's tally into sub-contexts so delegated calls count.
Approving. Two small, non-blocking notes:
- When the guard trips, only
content(the guard message) is written to theanswerslane — theRocketRide.agent.guard.v1stack entry that marks it as a guard trip lives only in the returned payload. A downstream node reading the lane can't distinguish a blocked run from a real answer. This matches the existing generic-error path, so it's consistent, not a regression — just flagging it as a conscious choice. - The "thread-safe under the GIL" comment in
AgentContext.invoked_toolsis correct for 3.10–3.12 but would not hold on a future free-threaded (nogil) build. No change needed now.
Good to merge once you're happy with those.
Contributor
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete. |
joshuadarron
approved these changes
Jul 18, 2026
joshuadarron
left a comment
joshuadarron
left a comment
Collaborator
There was a problem hiding this comment.
Review — require_tool_call fabrication guard ✅
Strong, well-reasoned change. The design (count at the one adapter every framework routes through, check at run-completion, opt-in and off by default) is the right shape, and the edge cases are handled with real care. I verified the load-bearing claims against the tree rather than taking them on faith:
Verified
- The single choke point actually covers all five frameworks. Every driver routes real tool execution through
AgentBase.call_tool: crewai (crewai_base.py:370), deepagent (deepagent.py:204), langchain (langchain.py:189), llamaindex (llamaindex.py:123), wave (executor.py:462). So counting incall_toolgenuinely tallies every framework — enabling the flag on any of the 6 service files won't false-trip because its tools bypassed the counter. AgentContextis@dataclass(frozen=True)(host.py:244), so the mutableinvoked_toolslist would make it unhashable if included in__hash__.field(default_factory=list, compare=False)is exactly the right fix, and adding it as a trailing defaulted field keeps every existing keyword construction valid. Nicely explained in the docstring and pinned bytest_context_stays_hashable_with_the_tool_tally.parse_boolexists (config_utils.py:49) and is used instead ofbool()— this matters:bool("false")isTrue, which would silently arm the guard on every run when the flag arrives stringified. Good catch, and regression-tested.
Things done right
- Count is recorded before
invoke(), so a tool that raises still counts — an attempted call isn't fabrication. Correct semantics, tested. except ToolCallRequiredErroris ordered beforeexcept Exception(it subclassesRuntimeError), returns a distinctRocketRide.agent.guard.v1answer, and retains the rejected framework output on the stack for diagnosis.meta.tool_callsis reported on success, guard, and error paths alike.- Sub-context threading (
invoked_tools=context.invoked_tools) in the crewai manager and deepagent so delegated tool calls count toward the parent's guard — withtest_independent_contexts_do_not_share_a_tallyguarding against accidental cross-run sharing. - Thread-safe append documented and exercised by the parallel-tools integration test;
memory.peeklocal reads correctly excluded (they never reachcall_tool) and that's tested end-to-end. - Backward compatible: off by default, additive config +
meta.tool_callsonly.
Non-blocking
- Integration coverage is Wave + LangChain only. crewai/llamaindex/deepagent counting is proven structurally (routing confirmed above) and by the sub-context unit test, but not driven end-to-end. Since the crewai-manager sub-context threading is the most intricate path, one integration smoke through the real manager loop would fully close the loop. Not a blocker given the routing is verified.
- Agent with zero tools +
require_tool_callon trips every run. Arguably correct (operator misconfig), but a config-time warning ("guard enabled but no tools connected") would be friendlier than a per-run guard error. - The
require_tool_callrows land inside theROCKETRIDE:GENERATED:PARAMSblocks; fine if produced bynodes:docs-generate(they matchservices.jsonverbatim, and the PR says the tables were regenerated) — just don't hand-maintain them. - The field isn't added to the crewai/deepagent
services.subagent.json. Reasonable (subagents are delegated, not top-level runs), just flagging the asymmetry in case it was meant to be uniform.
Approving — correct, thorough, well-tested, and it fails loud in exactly the case it's meant to catch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
require_tool_callguardrail: when enabled, anagent run that produces an answer without invoking at least one tool fails
with a
RocketRide.agent.guard.v1error instead of delivering the ungroundedtext. Fixes the gap where a weak planner could narrate a tool chain in prose
(the "must call ≥1 tool" rule existed only as a prompt instruction, not code).
AgentBase(tally onAgentContext, counted incall_tool, checked inrun_agent), so it covers all five agent frameworks;crewai-manager and deepagent sub-contexts thread the parent's tally so
delegated tool calls count.
meta.tool_callsis now reported on every answer.require_tool_call(boolean, off by default) on the six top-levelagent service files, with README notes + regenerated schema tables.
Type
Fix (bug) — adds an opt-in runtime guardrail + config field; no behavior change
when the flag is off.
Testing
./builder testpasses — see note belowAdded
packages/ai/tests/ai/common/agent/(firstAgentBaseunit tests) plusconfig cases in
test_config_shapes.py:call_toolcounting, guard trip/pass/off, sub-context sharing,context hashability, string-safe bool coercion, error-path count.
LangChain
create_agentloop — fabrication trips, grounding passes,memory.peek-only trips (local reads don't count), parallel tools all counted.format clean.
Checklist
meta.tool_callskey onlyLinked Issue
Fixes #1403
Out of scope (mentioned in the issue, deliberately not built): single-shot composite tools; provider-echoed tool-call ids (the
call_toolcounter supersedes that need as a detection mechanism).🤖 Generated with Claude Code
Summary by CodeRabbit
require_tool_callbehavior and defaults.