fix(tools): fail closed on non-bool approval results

[sylvesterkaczmarek]

Summary

Fixes #4845.

Callable needs_approval policies are declared to return bool, but the approval helper currently coerces any result with bool(). A predicate that accidentally returns None or another falsy non-bool value therefore becomes False and can let a guarded tool run without approval.

This change preserves explicit True and False results and treats any non-bool callable result as requiring approval. Truthy non-bool results keep their existing effective behaviour, while falsy non-bool results now fail closed.

Tests

Adds regression coverage for synchronous and asynchronous non-bool results and verifies explicit boolean results are unchanged.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-04T06:36:34.070810Z	4c94d91	PR opened
🔒 Security Review	✅ Completed 2026-09-04T06:39:19.732558Z	4c94d91	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[fscfede-beep]

There is one remaining fail-open path outside this diff. On current main, src/agents/mcp/server.py::_get_needs_approval_for_tool() wraps callable MCP approval policies separately and still ends with return bool(result). Because this PR only changes evaluate_needs_approval_setting(), an MCP callable that returns None (or another falsy non-bool) is still coerced to False there and can skip approval.

Could this wrapper be routed through evaluate_needs_approval_setting(policy, run_context, agent, tool) as well? A focused regression would be an MCP tool with sync/async approval policies returning None, asserting an approval interruption and zero MCP tool calls. That would make the fail-closed rule consistent across ordinary function tools and MCP tools.