fix: await cancelled output guardrail tasks on tripwire#3187
fix: await cancelled output guardrail tasks on tripwire#3187Quratulain-bilal wants to merge 4 commits intoopenai:mainfrom
Conversation
When an output guardrail trips, run_output_guardrails cancels its sibling tasks but never awaits them, unlike the input-guardrail variants which gather the cancelled tasks. This can leak pending tasks and emit "Task was destroyed but it is pending!" warnings when slow guardrails are still mid-flight at tripwire time. Mirror the input-guardrail pattern by awaiting the cancelled tasks before raising.
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
seratch
left a comment
seratch
left a comment
There was a problem hiding this comment.
Please make the regression test deterministic. Right now fast_tripwire can return before slow_then_observe_cancel has reached its await asyncio.sleep(5), so the slow task may be cancelled before its CancelledError handler is installed. In that schedule, cancelled_observed stays unset even though the production fix is correct.
A better shape would be to add a slow_started = asyncio.Event(), set it at the start of slow_then_observe_cancel, and have fast_tripwire await slow_started.wait() before returning the tripwire result. That ensures the test is actually proving that an in-flight sibling guardrail is awaited after cancellation.
Use a slow_started event so fast_tripwire only returns once the slow guardrail is parked on its sleep with the CancelledError handler installed. Without this, a fast scheduling order could cancel the slow task before it reached the try block, leaving cancelled_observed unset despite the production fix being correct.
|
fixed in 9ecf9b9. Added a |
run_input_guardrails / run_output_guardrails iterate guardrail tasks via asyncio.as_completed and only cancel siblings on the tripwire branch. If a guardrail raises a non-tripwire exception, the unfinished sibling tasks are left scheduled and continue running past the function's return — wasting work, possibly producing observable side effects, and triggering "Task was destroyed but it is pending" warnings at loop shutdown. Wrap the as_completed loop in try / except BaseException, and on any exit that is not a normal return cancel still-running siblings and gather them (return_exceptions=True) before re-raising. This is distinct from PR openai#3187, which targets the tripwire path of run_output_guardrails. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 participants
When an output guardrail trips, run_output_guardrails cancels its sibling tasks but never awaits them, unlike the input-guardrail variants which gather the cancelled tasks. This can leak pending tasks and emit "Task was destroyed but it is pending!" warnings when slow guardrails are still mid-flight at tripwire time. Mirror the input-guardrail pattern by awaiting the cancelled tasks before raising.