session: own manager stop tasks - #923
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4912db56e2
ℹ️ 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".
Problem
SessionManagerkept cancellation-surviving stop tasks in a keyed dictionary, but created them with rawasyncio.create_task(). That made the bookkeeping intent implicit and left the task outside the runtime ownership model being adopted across the bug-resistant refactor.Changes
RuntimeTaskScopeWhy / impact
Cancelled API waiters no longer separate Session teardown from the repository's explicit task-ownership conventions. The existing force retry still targets the exact keyed task, while the scope now provides consistent ownership and terminal exception handling.
Root cause
The manager's keyed dictionary supplied strong references but predated
RuntimeTaskScope, so the same lifecycle obligation was represented as an unclassified raw spawn.Checks
session_manager.py— passedjust check— 8,580 passed / 309 skipped