fix(cron): warn when web_search is in toolsAllow but no search provider is enabled#97658
fix(cron): warn when web_search is in toolsAllow but no search provider is enabled#97658krissding wants to merge 1 commit into
Conversation
…er is enabled When a cron job has web_search in its toolsAllow but every search provider plugin is disabled, the model delivers an apology and the run completes with status "ok" — no warning, no diagnostic. The operator has no signal that anything is wrong. Add a preflight check in prepareCronRunContext that logs a warning when web_search is requested but listWebSearchProviders() returns empty, directing the operator to enable a search plugin. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close as superseded: this branch is a narrower warning-only fix with known gaps, while the open sibling PR now provides the same issue’s viable native-aware, persisted-diagnostics implementation with proof and tests. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Canonical path: Land the native-aware, provider-selection-based diagnostic path in #97677 and close this narrower branch as superseded. So I’m closing this here and keeping the remaining discussion on #97677. Review detailsBest possible solution: Land the native-aware, provider-selection-based diagnostic path in #97677 and close this narrower branch as superseded. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows Is this the best way to solve the issue? No. The branch is a plausible warning-only mitigation, but it does not persist the warning into cron run diagnostics and it is superseded by a native-aware sibling PR with tests and real terminal proof. Security review: Security review cleared: The diff only adds a cron runtime warning path and does not change secrets, dependencies, workflows, package metadata, permissions, or downloaded code execution. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 389c355bcf3f. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
harjothkhara
left a comment
harjothkhara
left a comment
There was a problem hiding this comment.
I think this still misses the operator-facing surface from #97654. The new preflight only calls logWarn in prepareCronRunContext, then returns the normal prepared context. For the successful “model apologized” path, finalizeCronRun still persists diagnostics from createCronRunDiagnosticsFromAgentResult(...) only, and that helper only extracts failed tool payloads, meta.error, and meta.failureSignal.
That means cron_run_logs.diagnostics_summary / job.state.lastDiagnostics can remain empty while the run still records status: ok, which is the symptom the issue is asking to make visible. Can you carry the warning into the final RunCronAgentTurnResult.diagnostics as a cron-preflight warning, and add a focused toolsAllow: ["web_search"] regression that asserts the persisted diagnostics are present?
|
ClawSweeper applied the proposed close for this PR.
|
2 participants
Closes #97654
What Problem This Solves
When a cron agent turn specifies
web_searchintoolsAllowbut no search provider plugin is enabled, the cron completes with "ok" while the model delivers an apology — because the tool has no registered handler. The cron succeeds at the infrastructure level but fails at the task level, wasting a scheduled cycle.Why This Change Was Made
A preflight check in
prepareCronRunContextdetects the mismatch early and logs a warning with actionable guidance. This avoids silent failures and helps operators fix configuration before the cron executes.The check runs after tool expansion/normalization so aliases map to the canonical
web_searchname.listWebSearchProvidersalready exists and is the authoritative API for determining whether a search provider is available.Evidence
Source-level proof
expandToolGroups+normalizeToolNameprovide canonical tool name matching (same path used by the actual tool executor)listWebSearchProviders({ config })is the authoritative API — returns empty array when no search provider plugin is enabled[cron:${job.id}]prefix for traceability and actionable remediation textExisting code paths
Not tested
Merge Risk
Low. The change:
expandToolGroups,normalizeToolName,listWebSearchProviders)logWarn— does not block execution🤖 Generated with Claude Code