Skip to content

fix: reset Agents SDK header overrides on errors and cancellation - #136

Open
rioyu123 wants to merge 1 commit into
openai:mainfrom
rioyu123:fix/header-override-cleanup
Open

rioyu123 wants to merge 1 commit into
openai:mainfrom
rioyu123:fix/header-override-cleanup

Conversation

@rioyu123

Copy link
Copy Markdown

Summary

agents_sdk_user_agent_override() only reset the Agents SDK header overrides after a normal exit. If the body raised or was cancelled, later work in the same context kept ChatKit's User-Agent override instead of the headers the caller had set before.

Both resets now run in finally, so the previous values are restored and the original exception still propagates.

One case can't be handled from inside the helper. If a suspended stream generator is finalized in a different context, for example by the event loop after the stream is abandoned, the tokens can't be reset there. The helper tolerates that ValueError so cleanup doesn't raise a new exception. That path behaves as it did before this change: the values in the originating context are not restored.

Normal completion is unchanged, and there are no public API changes.

Test plan

  • Added 15 cases using the real ChatKit helper and Agents SDK context variables: normal and exceptional exits, empty and existing overrides, nested scopes, cancellation of an actual asyncio task, closing a suspended generator from another task, and the complete server response path when respond() fails.
  • On the base, 11 of these cases fail. The 4 that pass are the normal-exit and cross-context-close cases, which the base already handled. All 15 pass with this change. Without the ValueError guard, the 2 cross-context close cases fail.
  • make lint, make pyright, and make test pass; the full suite has 145 passing tests.
  • make format-check still reports the same four existing files as the unmodified base. The added test file passes formatting; unrelated files were left unchanged.

Validation used Python 3.14.4 and the locked dependencies, without model API requests.

This branch has not been deployed

No deployments
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.

1 participant