fix(tools): skip allowlist guard when tools are explicitly disabled by SymbolStar · Pull Request #74139 · openclaw/openclaw

SymbolStar · 2026-04-29T06:21:09Z

Summary

When an embedded model run (e.g. llm-task invoked from a Lobster workflow) intentionally disables tools, the explicit tool allowlist guard from the parent session's tools.alsoAllow config should not fire.

Root Cause

buildEmptyExplicitToolAllowlistError in src/agents/tool-allowlist-guard.ts would see:

sources.length > 0 (from parent config's alsoAllow entries like lobster, llm-task)
callableToolNames.length === 0 (tools are intentionally disabled for the embedded run)

This combination triggered the error: "No callable tools remain after resolving explicit tool allowlist ... tools are disabled for this run".

Fix

Early-return null from the guard when disableTools === true, since disabling tools is an intentional choice that should not be treated as a configuration failure. Removed the now-unreachable disableTools branch from the error reason.

Fixes #74019

greptile-apps · 2026-04-29T06:22:07Z

Greptile Summary

This PR fixes a false-positive error in buildEmptyExplicitToolAllowlistError that fired when an embedded run intentionally disabled tools while a parent session's alsoAllow config was populated. The fix is a clean early-return guard (disableTools === true → null) and removes the now-dead branch from the error-reason ternary. Tests are updated to assert null is returned instead of an error.

Confidence Score: 5/5

Safe to merge — the change is minimal, well-tested, and correct.

The fix is a two-line early return that precisely targets the described failure mode. All pre-existing test cases continue to pass, and the updated test correctly asserts the new null return. The dead disableTools branch removal from the ternary is consistent with the early return. No logic regressions identified.

No files require special attention.

_{Reviews (1): Last reviewed commit: "fix(tools): skip allowlist guard when to..." | Re-trigger Greptile}

clawsweeper · 2026-04-29T06:25:13Z

Codex review: needs changes before merge.

Summary
The PR makes buildEmptyExplicitToolAllowlistError return null whenever disableTools is true and updates the guard test to expect no error for that disabled-tool case.

Reproducibility: yes. source-level. A direct guard call with inherited allowlist sources, zero callable tools, and disableTools: true reproduces the reported false positive; the same call shape with runtime toolsAllow demonstrates the PR regression against current main's test contract.

Next step before merge
The remaining repair is narrow and automatable: preserve the runtime conflict contract, add the disabled inherited-source case, and add the required changelog entry.

Security
Cleared: The diff does not touch CI, dependencies, lockfiles, secrets, install scripts, or publishing surfaces; the fail-closed policy regression is covered as a functional review finding.

Review findings

[P2] Preserve runtime allowlist conflicts — src/agents/tool-allowlist-guard.ts:23-24
[P3] Add the changelog entry — src/agents/tool-allowlist-guard.ts:23-24

Review details

Best possible solution:

Keep this PR open but repair it so disabled LLM-only runs ignore inherited allowlist sources while runtime toolsAllow plus disabled tools continues to error and stays covered by a test.

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

Yes, source-level. A direct guard call with inherited allowlist sources, zero callable tools, and disableTools: true reproduces the reported false positive; the same call shape with runtime toolsAllow demonstrates the PR regression against current main's test contract.

Is this the best way to solve the issue?

No. The fix direction is right, but returning null for every disabled-tool run is broader than the bug; the safer fix should distinguish inherited ambient allowlists from explicit runtime toolsAllow.

Full review comments:

[P2] Preserve runtime allowlist conflicts — src/agents/tool-allowlist-guard.ts:23-24
Returning null before checking the allowlist source suppresses the existing runtime toolsAllow conflict case. A caller that passes toolsAllow while also disabling tools would now silently run with no tools instead of getting the fail-closed error covered by current main's test, so narrow the bypass to inherited config/profile allowlists or otherwise keep runtime toolsAllow plus disableTools erroring.
Confidence: 0.9
[P3] Add the changelog entry — src/agents/tool-allowlist-guard.ts:23-24
This is a user-facing bug fix for Lobster/llm-task disabled-tool runs, but the PR only changes the guard and its test. Add a CHANGELOG.md fix entry under the active version before merge.
Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.9

Acceptance criteria:

pnpm test src/agents/tool-allowlist-guard.test.ts
pnpm check:changed

What I checked:

Current main preserves runtime toolsAllow fail-closed behavior: The current guard test expects runtime toolsAllow plus disableTools: true to return an error mentioning both the runtime allowlist and disabled tools. (src/agents/tool-allowlist-guard.test.ts:20, 5ecd01ff94d6)
PR diff suppresses all disabled-tool guard cases: The PR adds an unconditional early return for params.disableTools === true and changes the runtime toolsAllow conflict test to expect null. (src/agents/tool-allowlist-guard.ts:23, 748a217de365)
Embedded runner mixes inherited and runtime allowlist sources: The source collector includes global, provider, agent, group, sandbox, subagent, and runtime toolsAllow sources before calling the empty allowlist guard. (src/agents/pi-embedded-runner/run/attempt.ts:686, 5ecd01ff94d6)
llm-task intentionally disables tools: The llm-task plugin runs its embedded model call with disableTools: true, matching the reported [Bug]: Lobster workflow calling llm-task fails with “No callable tools remain” when tools.alsoAllow includes lobster/llm-task #74019 call shape. (extensions/llm-task/src/llm-task-tool.ts:219, 5ecd01ff94d6)
Docs define both no-tool llm-task runs and fail-closed allowlists: The llm-task docs say no tools are exposed for the run, while the tool policy docs say explicit allowlists resolving to no callable tools intentionally fail before the model call. Public docs: docs/tools/multi-agent-sandbox-tools.md. (docs/tools/multi-agent-sandbox-tools.md:230, 5ecd01ff94d6)
Feature history points to the guard and adjacent embedded-runner owners: GitHub commit history for the guard shows 61ee67ae introduced the empty explicit allowlist failure and b97a6f28 later touched the helper; adjacent embedded-runner history includes 02d7ad48 for plugin-only tool allowlists. (src/agents/tool-allowlist-guard.ts:17)

Likely related people:

steipete: Commit history for src/agents/tool-allowlist-guard.ts shows 61ee67ae introduced the empty explicit allowlist guard, b97a6f28 later maintained the helper, and llm-task history also shows related plugin-runtime work. (role: introduced behavior and recent maintainer; confidence: high; commits: 61ee67aecc3f, b97a6f284904, 67a447c17530; files: src/agents/tool-allowlist-guard.ts, src/agents/tool-allowlist-guard.test.ts, extensions/llm-task/src/llm-task-tool.ts)
obviyus: Recent embedded-runner history includes 02d7ad48 for plugin-only tool allowlists, adjacent to the callable-tool filtering and allowlist resolution path involved here. (role: adjacent owner; confidence: medium; commits: 02d7ad482083; files: src/agents/pi-embedded-runner/run/attempt.ts)
pashpashpash: Current-main blame and GitHub history show 8f4eaa9c recently touched the guard, guard test, embedded runner, llm-task, and heartbeat files while working near disabled-tool heartbeat behavior. (role: recent maintainer; confidence: medium; commits: 8f4eaa9c00be; files: src/agents/tool-allowlist-guard.ts, src/agents/tool-allowlist-guard.test.ts, src/agents/pi-embedded-runner/run/attempt.ts)

Remaining risk / open question:

A broad disableTools bypass would make caller-level runtime toolsAllow plus disabled-tools contradictions silently proceed as no-tool runs.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 5ecd01ff94d6.

When an embedded model run (e.g. llm-task from a Lobster workflow) intentionally disables tools, the explicit tool allowlist guard from the parent session's alsoAllow config should not fire. Previously it would error with 'No callable tools remain' because the allowlist sources existed but no tools were callable (by design). Return null early from buildEmptyExplicitToolAllowlistError when disableTools is true, since disabling tools is an intentional choice that should not be treated as a configuration failure. Fixes openclaw#74019

steipete · 2026-05-03T13:49:11Z

Closing as superseded by #76686, which landed as 3491060 and closed #74019. The landed fix keeps the narrower source distinction: inherited config allowlists are ignored for intentional no-tool runs while runtime toolsAllow still fails closed.

SymbolStar · 2026-05-03T14:47:39Z

Hi @steipete — friendly ping 🙏 CI is green now (rebased on latest main). Would appreciate a look when you get a chance!

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS labels Apr 29, 2026

This was referenced Apr 29, 2026

fix: catch croner errors in cron gateway handlers and fix false-positive allowlist error #74306

Closed

fix(agents): skip tool allowlist empty guard for LLM-only runs #74841

Closed

fix(agents): skip empty allowlist guard for modelRun #73031

Closed

rjdjohnston mentioned this pull request Apr 30, 2026

LLM-only modelRun fails when inherited tool allowlist is non-empty #74810

Closed

This was referenced Apr 30, 2026

[Bug]: Lobster workflow calling llm-task fails with “No callable tools remain” when tools.alsoAllow includes lobster/llm-task #74019

Closed

Lobster docs show openclaw.invoke --tool llm-task, but embedded runner lacks gateway URL/auth context #76101

Closed

szponeczek mentioned this pull request May 3, 2026

Heartbeat inferred-commitment delivery fails with empty allowlist guard when tools are disabled #76399

Closed

SymbolStar force-pushed the fix/74019-llm-task-allowlist-guard branch from 4034d53 to 748a217 Compare May 3, 2026 08:56

This was referenced May 3, 2026

Optional plugin tools allowed with alsoAllow are not discovered #76616

Closed

fix(tools): allow no-tool llm-task runs #76686

Merged

steipete closed this May 3, 2026

clawsweeper Bot mentioned this pull request Jun 21, 2026

fix(agents): skip empty allowlist guard for modelRun shapes #95605

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(tools): skip allowlist guard when tools are explicitly disabled#74139

fix(tools): skip allowlist guard when tools are explicitly disabled#74139
SymbolStar wants to merge 1 commit into
openclaw:mainfrom
SymbolStar:fix/74019-llm-task-allowlist-guard

SymbolStar commented Apr 29, 2026

Uh oh!

greptile-apps Bot commented Apr 29, 2026

Uh oh!

clawsweeper Bot commented Apr 29, 2026 •

edited

Loading

Uh oh!

steipete commented May 3, 2026

Uh oh!

SymbolStar commented May 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

SymbolStar commented Apr 29, 2026

Summary

Root Cause

Fix

Uh oh!

greptile-apps Bot commented Apr 29, 2026

Greptile Summary

Confidence Score: 5/5

Uh oh!

clawsweeper Bot commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented May 3, 2026

Uh oh!

SymbolStar commented May 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Apr 29, 2026 •

edited

Loading