← Back to PRs

#22516: fix: add resilient tool registration with per-tool error isolation

by white-rm open 2026-02-21 07:41 View on GitHub →
agents size: L
…d validation (#22426) ## Summary Describe the problem and fix in 2–5 bullets: - Problem: - Why it matters: - What changed: - What did NOT change (scope boundary): ## Change Type (select all) - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Docs - [ ] Security hardening - [ ] Chore/infra ## Scope (select all touched areas) - [ ] Gateway / orchestration - [ ] Skills / tool execution - [ ] Auth / tokens - [ ] Memory / storage - [ ] Integrations - [ ] API / contracts - [ ] UI / DX - [ ] CI/CD / infra ## Linked Issue/PR - Closes # - Related # ## User-visible / Behavior Changes List user-visible changes (including defaults/config). If none, write `None`. ## Security Impact (required) - New permissions/capabilities? (`Yes/No`) - Secrets/tokens handling changed? (`Yes/No`) - New/changed network calls? (`Yes/No`) - Command/tool execution surface changed? (`Yes/No`) - Data access scope changed? (`Yes/No`) - If any `Yes`, explain risk + mitigation: ## Repro + Verification ### Environment - OS: - Runtime/container: - Model/provider: - Integration/channel (if any): - Relevant config (redacted): ### Steps 1. 2. 3. ### Expected - ### Actual - ## Evidence Attach at least one: - [ ] Failing test/log before + passing after - [ ] Trace/log snippets - [ ] Screenshot/recording - [ ] Perf numbers (if relevant) ## Human Verification (required) What you personally verified (not just CI), and how: - Verified scenarios: - Edge cases checked: - What you did **not** verify: ## Compatibility / Migration - Backward compatible? (`Yes/No`) - Config/env changes? (`Yes/No`) - Migration needed? (`Yes/No`) - If yes, exact upgrade steps: ## Failure Recovery (if this breaks) - How to disable/revert this change quickly: - Files/config to restore: - Known bad symptoms reviewers should watch for: ## Risks and Mitigations List only real risks for this PR. Add/remove entries as needed. If none, write `None`. - Risk: - Mitigation: <!-- greptile_comment --> <h3>Greptile Summary</h3> Added per-tool error isolation to tool registration so individual tool factory failures don't prevent other tools from loading. Includes validation checks that warn when core tools are missing after registration, helping detect initialization failures or race conditions. - Wrapped each tool creation in `openclaw-tools.ts` with a `safeTool` helper that catches errors and returns `null` instead of throwing - Added try-catch blocks in `pi-tools.ts` flatMap to isolate errors during tool wrapping/transformation - Added validation in three locations (`openclaw-tools.ts`, `pi-tools.ts`, `attempt.ts`) to detect and warn about missing core tools - Comprehensive test coverage validates the error isolation pattern and missing tool detection logic <h3>Confidence Score: 3/5</h3> - Safe to merge with one logical issue that should be fixed - The PR correctly implements error isolation for tool registration with comprehensive tests. However, the validation logic in `attempt.ts` checks tools after they've been sanitized for Google, which could cause false positive warnings when tools are legitimately filtered out by `sanitizeToolsForGoogle`. This should be validated against `toolsRaw` instead of `tools` to avoid misleading warnings. - Check `src/agents/pi-embedded-runner/run/attempt.ts` for the validation timing issue <sub>Last reviewed commit: b0cc3c1</sub> <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs