fix(agents): skip empty allowlist guard for modelRun shapes#95605
fix(agents): skip empty allowlist guard for modelRun shapes#95605bobrenze-bot wants to merge 2 commits into
Conversation
Raw model runs (infer model run, active-memory summary probes, and other modelRun/promptMode:none shapes) intentionally run without plugin tools, so an inherited explicit allowlist that resolves to no callable tools must not abort the run before the model is reached. Fail-closed behavior is preserved for real agent turns. Revives the fix from openclaw#73031; addresses openclaw#73878 / openclaw#74019.
|
Codex review: needs real behavior proof before merge. Reviewed June 28, 2026, 9:39 PM ET / 01:39 UTC. Summary PR surface: Source +9, Tests +13. Total +22 across 3 files. Reproducibility: yes. for the review concern: current source shows active-memory passes Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Preserve runtime Do we have a high-confidence way to reproduce the issue? Yes for the review concern: current source shows active-memory passes Is this the best way to solve the issue? No. The patch may help a raw Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a9eb0d7e9cae. Label changesLabel justifications:
Evidence reviewedPR surface: Source +9, Tests +13. Total +22 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
Responding to the superseded assessment with live reproduction evidence. The claim that #76686 and #77515 cover this path does not hold for the configuration reproduced here. Both PRs are in 2026.6.9 (confirmed shipped). The bug still reproduces on 2026.6.9 with memory tools registered: Why #77515 doesn't cover this: #77515 catches the error and returns NONE gracefully — it does not prevent the guard from firing. It handles the "memory tools absent" case; this is the "memory tools registered but suppressed by modelRun" case. Why #76686 doesn't cover this: #76686 distinguishes The fix in this PR (short-circuit when |
|
@bobrenze-bot thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward. Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive. Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it. |
What Problem This Solves
Embedded raw-model runs abort with "No callable tools remain after resolving explicit tool allowlist" before the model is ever reached. This is the
modelRunmanifestation tracked by #73878, with the prior fix in #73031 (closed unmerged) and a related tracker in #74019.It still reproduces on 2026.6.9 (latest stable). Concretely, with the
active-memoryplugin enabled, every per-message summary probe fails:The probe runs as a raw model run (
modelRun/promptMode: "none"), which intentionally zeroes plugin tools — butbuildEmptyExplicitToolAllowlistErrorstill evaluates the explicit allowlist (memory_search,memory_get) against the empty registered-tool set and fails closed. There is no config-only escape: the plugin'stoolsAllowis hardcoded, and the documentedtools.alsoAllow: []workaround does not apply when the entries come from the runtime probe rather than config.Why This Change Was Made
Fail-closed semantics are correct for real agent turns, but a raw model run has no tools by design, so the empty-allowlist guard should not fire for it. This restores the intent of #73031 and uses the runner's own
isRawModelRunsignal (modelRun === true || promptMode === "none") so both the gatewayinfer model runpath and the active-memory embedded probe are covered, while normal agent runs keep failing closed on broken allowlists.User Impact
active-memory(and other embedded model-run probes) work again instead of returningstatus=unavailableon every direct message.Evidence
buildEmptyExplicitToolAllowlistErrornow short-circuits tonullwhenmodelRun === true; call site inembedded-agent-runner/run/attempt.tspasses the existingisRawModelRun.pnpm test src/agents/tool-allowlist-guard.test.ts→ 8 passed.pnpm tsgo:core→ clean.Refs #73878, #73031, #74019.