Skip to content

Fail closed on invalid realtime approval settings - #5029

Open
DeepanshuPal wants to merge 1 commit into
openai:mainfrom
DeepanshuPal:fix/realtime-invalid-needs-approval
Open

DeepanshuPal wants to merge 1 commit into
openai:mainfrom
DeepanshuPal:fix/realtime-invalid-needs-approval

Conversation

@DeepanshuPal

Copy link
Copy Markdown

Summary

The realtime session passed strict=False to the shared needs_approval evaluator, so an invalid setting such as "always" became False and the tool ran without approval. The core runner already raises UserError for the same input.

Use the shared strict behavior in realtime too. Invalid approval settings now raise before the tool is invoked, matching the core runner and avoiding a fail-open path.

Test plan

  • Added a realtime regression test for an invalid needs_approval value.
  • uv run pytest tests/realtime/test_session_approvals.py -q (31 passed)
  • uv run pytest tests/realtime -q (520 passed)
  • uv run ruff check src/agents/realtime/session.py tests/realtime/test_session_approvals.py
  • uv run pyright src/agents/realtime/session.py tests/realtime/test_session_approvals.py

Issue number

Closes #5024

Checks

  • I've added new tests, if relevant
  • I've confirmed all verification steps pass
  • If using Codex, I've run /review before submitting this PR

@joinwell52-AI

Copy link
Copy Markdown

Thanks for this change. It prompted a small approval-boundary experiment for our agent runtime work. Using the real RealtimeSession tool-call entry point at b6c2cef, a recording model, and a local list-writing tool, we compared ten inputs with the predecessor approval method substituted into the same candidate source. Invalid top-level settings ("always", 1, None, {}) went from one invocation each to UserError with zero invocations. The candidate's original approval test file also passed all 31 tests; the method ablation failed the invalid-setting test.

One boundary the experiment clarified for us: a callback that returns None still leads to execution in both modes. That is deliberately outside the Boolean return contract, so we are not calling it a supported-input bug. Is coercion of callback results intentional here, or would validating that result at runtime be useful too? It seems worth distinguishing valid configuration type from valid computed approval result.

Probe, pinned versions, and saved observations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

needs_approval misconfiguration raises in the core runner and silently skips approval in realtime sessions

2 participants