fix: clean up pending permissions when session is deleted - #7154
Conversation
|
The following comment was made by an LLM, it may be inaccurate: No duplicate PRs found |
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
This PR adds session cleanup functionality to the PermissionNext namespace to prevent memory leaks when sessions are deleted. It introduces init(), dispose(), and clearSession() methods that subscribe to session.deleted events and clean up orphaned pending permissions.
Key changes:
- Adds event subscription mechanism to automatically clean up pending permissions when sessions are deleted
- Introduces init() and dispose() functions to manage event subscriptions lifecycle
- Adds clearSession() to reject and remove pending permissions for a specific session
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/opencode/src/permission/next.ts | Implements init(), dispose(), and clearSession() functions with session.deleted event subscription to clean up orphaned pending permissions |
| packages/opencode/test/permission/cleanup.test.ts | Adds comprehensive test coverage for the new cleanup functionality including session clearing, event subscription, disposal, and duplicate init handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
Sorry, something went wrong.
Add init(), dispose(), and clearSession() to PermissionNext namespace to properly clean up pending permission requests when a session is deleted. This prevents memory accumulation from orphaned pending entries that would never be resolved. The fix subscribes to the session.deleted bus event and rejects all pending permissions for that session, allowing proper garbage collection.
- Move subscriptions to per-instance state instead of module-level - Add Event.Replied publishing in clearSession for consistency - Make init() and dispose() async to work with instance state - Remove unnecessary setTimeout delays in tests (Bus.publish awaits handlers)
Address Copilot review comments - import Session and use the proper Session.Event.Deleted event definition instead of bypassing type checking with manually constructed objects.
This reverts commit 7b9bc5370b08ec55c543de381b88eaf9c050a139. wip: zen Reapply "wip: zen" This reverts commit 09516ac8c3bb7c432b28cf63faa65c0e3b9f9684.
|
Closing this pull request because it has had no updates for more than 60 days. If you plan to continue working on it, feel free to reopen or open a new PR. |
Sorry, something went wrong.
Fixes #3013
Summary
Problem
When a session is deleted, any pending permission requests for that session remain in the pending map indefinitely. This causes:
Solution
Subscribe to the session.deleted event and clean up all pending permissions for deleted sessions by:
Testing
Added comprehensive tests in test/permission/cleanup.test.ts.