fix: scope one-shot event handlers to registrations - #3741
Conversation
|
Focused regression tests and repository formatting checks pass on the current head ( |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![chatgpt-codex-connector[bot]](https://avatars.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d8fc3d0bd
ℹ️ 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".
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".
|
The P2 identity-removal feedback is addressed in the current head ( |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ 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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
1 participant
Changes being requested
EventHandlerRegistrytracks one-shot handlers by callback identity, so registering the same callback for two event names lets the first event consume the one-shot registration for the other event.Reproduction
persistent.one-shot.Expected: the one-shot registration is consumed only by
one-shot, while the persistent registration remains available forpersistent.Actual: callback identity is shared across event names, so dispatching one event can suppress the other registration.
Root cause and changes
The registry stored callbacks separately from a registry-wide set of callback IDs. That set could not represent the event name or the registration mode for duplicate registrations.
Additional context & links
Validation
uv run --no-project --with-editable . --with pytest python -m pytest -c /dev/null --rootdir=. --noconftest -o addopts='' -q tests/test_event_handler.py— 4 passed.ruff check src/openai/_event_handler.py tests/test_event_handler.py— passed.ruff format --check src/openai/_event_handler.py tests/test_event_handler.py— passed.This is an internal registry-state correction. No public API signatures or event payloads change.