Skip to content

fix(cron): warn when toolsAllow includes web_search but no search provider is enabled#97664

Closed
Alix-007 wants to merge 2 commits into
openclaw:mainfrom
Alix-007:fix/cron-tools-allow-search-provider-check
Closed

fix(cron): warn when toolsAllow includes web_search but no search provider is enabled#97664
Alix-007 wants to merge 2 commits into
openclaw:mainfrom
Alix-007:fix/cron-tools-allow-search-provider-check

Conversation

@Alix-007

@Alix-007 Alix-007 commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Adds a cron preflight diagnostic when toolsAllow requests web_search but no web search provider plugin is enabled.
  • Keeps execution behavior unchanged: the job still runs and delivery/status are not blocked.
  • Makes the misconfiguration visible in diagnostics instead of leaving operators with a successful run that could not actually search.
  • Review focus: diagnostic-only behavior and the plugin/provider availability check.

Linked context

Closes #97654.

Related: cron isolated-agent diagnostics and tool availability handling.

Maintainer request: not directly requested by a maintainer.

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: cron jobs could allow web_search while all search provider plugins were disabled, producing no operator-visible warning.
  • Real environment tested: local OpenClaw cron isolated-agent test harness on Node 22 with web search providers mocked to an empty list.
  • Exact steps or command run after this patch: PNPM_CONFIG_MODULES_DIR=/media/vdc/0668001470/workSpace/claw-campaign/openclaw/node_modules node scripts/run-vitest.mjs src/cron/isolated-agent/run.tools-allow.test.ts -- --run -t "warns when web_search is allowed but no provider is enabled"
  • Evidence after fix:
Test Files  1 passed (1)
     Tests  1 passed | 3 skipped (4)
[test] passed 1 Vitest shard
  • Observed result after fix: cron diagnostics include a warn-severity message explaining that web_search was requested but no provider plugin is enabled, while the run status remains unchanged.
  • What was not tested: a live scheduled cron run through a deployed gateway.
  • Proof limitations or environment constraints: proof uses the cron isolated-agent harness with runtime provider list controlled to reproduce the misconfiguration deterministically.
  • Before evidence: the same configuration completed without a diagnostics warning.

Tests and validation

  • PNPM_CONFIG_MODULES_DIR=/media/vdc/0668001470/workSpace/claw-campaign/openclaw/node_modules node scripts/run-vitest.mjs src/cron/isolated-agent/run.tools-allow.test.ts -- --run -t "warns when web_search is allowed but no provider is enabled"
  • /media/vdc/0668001470/workSpace/claw-campaign/openclaw/node_modules/.bin/oxlint src/cron/isolated-agent/run.ts src/cron/isolated-agent/run.tools-allow.test.ts src/cron/isolated-agent/run.test-harness.ts
  • Regression coverage verifies diagnostic text, severity, and that passthrough execution behavior remains intact.

Risk checklist

Did user-visible behavior change? (Yes/No) Yes. Operators now see an additional warning diagnostic.

Did config, environment, or migration behavior change? (Yes/No) No.

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No) No execution behavior change; only preflight diagnostics change.

What is the highest-risk area? False-positive warning if provider availability is not loaded before the preflight.

How is that risk mitigated? The check runs after runtime plugins are loaded and uses the same provider listing path used by web search availability.

Current review state

Next action: ClawSweeper/maintainer re-review after the previous infrastructure timeout.

Waiting on: ClawSweeper to complete a fresh review; the earlier review failed due infrastructure timeout, not patch quality.

Bot/reviewer comments addressed: lint issue fixed with Set.has, focused regression coverage added, and PR body now follows the full template.

AI-assisted; implementation and validation reviewed before pushing.

@clawsweeper

clawsweeper Bot commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: the useful cron diagnostic is already covered by the open, clean, proof-positive canonical PR, while this branch uses a narrower provider-list check that misses effective provider selection and native hosted-search cases.

Root-cause cluster
Relationship: superseded
Canonical: #97677
Summary: This PR is one of several competing candidate fixes for the same cron toolsAllow web_search diagnostic issue; the open canonical PR is the viable provider-selection-based path.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Land one native-aware, provider-selection-based cron diagnostic fix through #97677, then close the linked issue and remaining duplicate PRs.

So I’m closing this here and keeping the remaining discussion on #97677.

Review details

Best possible solution:

Land one native-aware, provider-selection-based cron diagnostic fix through #97677, then close the linked issue and remaining duplicate PRs.

Do we have a high-confidence way to reproduce the issue?

Yes. Source-level reproduction is high confidence: current main exposes managed web_search separately from effective provider selection, and cron diagnostics are only built from tool/error payloads, so a successful model apology can leave no warning.

Is this the best way to solve the issue?

No. This branch is a plausible narrow warning, but the best fix is the existing open PR that uses the runtime provider-selection path, skips default or wildcard-derived allowlists, and suppresses native hosted-search false positives.

Security review:

Security review cleared: The diff only changes cron diagnostic TypeScript and tests; it does not change secrets, dependencies, workflows, package metadata, permissions, or downloaded code execution paths.

AGENTS.md: found and applied where relevant.

What I checked:

  • Repository policy read: Root AGENTS.md was read fully; its PR review policy requires whole-path review, provider-routing compatibility care, and best-fix comparison before verdict. (AGENTS.md:25, be94853de0d0)
  • Current main diagnostic gap: Current main prepares the cron agent payload and proceeds to delivery-context resolution without checking explicit web_search allowlists against provider availability. (src/cron/isolated-agent/run.ts:756, be94853de0d0)
  • Provider selection contract: listWebSearchProviders lists providers, but resolveWebSearchProviderId is the effective selection path and can return an empty string when no explicit or credential-backed provider is selected. (src/web-search/runtime.ts:167, be94853de0d0)
  • Execution-time no-provider behavior: runWebSearch only throws after candidate resolution returns empty, so a cron run can still complete with a model apology and no preflight diagnostic on current main. (src/web-search/runtime.ts:456, be94853de0d0)
  • Native hosted search contract: OpenClaw suppresses the managed web_search tool when Codex native search activation is native_active; a cron warning must account for that path to avoid false positives. (src/agents/codex-native-web-search-core.ts:256, be94853de0d0)
  • Codex upstream source checked: Sibling Codex source creates a hosted web_search ToolSpec for cached/indexed/live modes and returns no tool for disabled or unset modes, confirming the native hosted-search path is real. (../codex/codex-rs/core/src/tools/hosted_spec.rs:20, ccdfb4f342a2)

Likely related people:

  • vincentkoc: Recent main history on src/web-search/runtime.ts includes runtime provider refactors and provider-selection work that this diagnostic must use correctly. (role: recent web_search runtime contributor; confidence: high; commits: e122cd09ab0f, b962c53e785c, c48b36a2556e; files: src/web-search/runtime.ts)
  • fuller-stack-dev: Authored the merged Codex hosted web_search work that owns the native hosted path this diagnostic must not misreport. (role: native hosted web_search feature contributor; confidence: high; commits: 1e0062b44a51; files: src/agents/codex-native-web-search-core.ts)
  • eleqtrizit: Authored the native web_search policy enforcement work behind the managed-tool suppression path. (role: native web_search policy contributor; confidence: high; commits: f0d8048aa3b4; files: src/agents/codex-native-web-search-core.ts)
  • Jerry-Xin: Recent history on src/cron/isolated-agent/run.ts includes guards and refactors in the same prepare/finalize runner area touched by this PR. (role: recent cron isolated-run contributor; confidence: medium; commits: 30b39c6eab43; files: src/cron/isolated-agent/run.ts)

Codex review notes: model internal, reasoning high; reviewed against be94853de0d0.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jun 29, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 29, 2026
Closed
@Alix-007

Alix-007 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up fix pushed to address the lint/proof gap:

  • changed the normalized allowlist lookup to Set.has, resolving the prefer-set-has lint failure
  • added focused regression coverage for toolsAllow: ["web_search"] when no web_search provider is enabled, including diagnostics and passthrough assertions
  • isolated the web_search provider list in the cron test harness so the proof does not depend on runtime plugin state
  • verified locally: node scripts/run-vitest.mjs src/cron/isolated-agent/run.tools-allow.test.ts -- --run -t "warns when web_search is allowed but no provider is enabled"
  • verified locally: oxlint src/cron/isolated-agent/run.ts src/cron/isolated-agent/run.tools-allow.test.ts src/cron/isolated-agent/run.test-harness.ts

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 29, 2026
This was referenced Jun 29, 2026
Closed
Merged
Closed
@Alix-007

Alix-007 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

PR body refreshed to the full contributor template with real behavior proof, tests, risk, and current review state.

The previous ClawSweeper run failed due infrastructure timeout, not a patch verdict. Fresh validation is now recorded in the body:

  • node scripts/run-vitest.mjs src/cron/isolated-agent/run.tools-allow.test.ts -- --run -t "warns when web_search is allowed but no provider is enabled"
  • oxlint src/cron/isolated-agent/run.ts src/cron/isolated-agent/run.tools-allow.test.ts src/cron/isolated-agent/run.test-harness.ts

@clawsweeper re-review

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. and removed merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jun 29, 2026
@clawsweeper

clawsweeper Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jun 29, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 29, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Silent failure when web_search is in toolsAllow but no search provider plugin is enabled

1 participant

@Alix-007