fix(agents): exclude sanitized cache-read tool name from tool cache#6485
fix(agents): exclude sanitized cache-read tool name from tool cache#6485Diwak4r wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe ChangesCache Tool Name Sanitization
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
ErenAta16
left a comment
There was a problem hiding this comment.
Confirmed the name mismatch directly: CacheTools.name is the literal class attribute "Hit Cache", but the tool actually registers itself at runtime via sanitize_tool_name(self.name) (per cache_tools.py), which lowercases and underscores it to "hit_cache". So the guard's calling.tool_name != CacheTools().name was comparing the sanitized runtime name against the raw display name — never equal, so the guard's != was always True and the cache-read tool's own output got written into the cache every time. Fix aligns both sides to the sanitized form. Test fails on main and passes on the branch per the PR's own description, which lines up with what I traced through the source.
The exclusion guard compared the raw default name "Hit Cache" against the sanitized runtime name "hit_cache", so it was always true and the cache tool's own output was written into the cache. Compare with sanitize_tool_name(CacheTools().name) instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dfce3c0 to
573fc42
Compare
ErenAta16
left a comment
There was a problem hiding this comment.
Verified this matches the actual bug: CacheTools.name is the raw class attribute "Hit Cache", while the tool registers itself at runtime via sanitize_tool_name(self.name) ("hit_cache"), so the old != comparison was always true and the cache-read tool's own output got written back into the cache on every call. The fix aligns both sides to the sanitized form, and the regression test fails on main / passes on the branch as described. LGTM.
Summary
The guard in
ToolsHandler.on_tool_usethat prevents the built-in cache-read tool ("Hit Cache") from being added to the tool cache compared the raw default name"Hit Cache"against the sanitized runtime name"hit_cache". They never match, so the guard was alwaysTrueand the cache tool's own output was written into the cache — polluting it with dead entries and risking stale values.Fix
Compare against
sanitize_tool_name(CacheTools().name)so the sanitized names align.Test
Added
test_cache_tool_result_is_not_cachedtolib/crewai/tests/tools/test_tool_usage.py. FAILS before the fix, PASSES after. Fulltests/tools/test_tool_usage.py= 24 passed + new test (5 pre-existing event-bus timing failures are unrelated to this change).🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com