Skip to content

fix: clean up pending permissions when session is deleted - #7154

Closed
hendem wants to merge 7 commits into
anomalyco:devfrom
hendem:fix/permission-approved-map-leak
Closed

fix: clean up pending permissions when session is deleted#7154
hendem wants to merge 7 commits into
anomalyco:devfrom
hendem:fix/permission-approved-map-leak

Conversation

@hendem

@hendemhendem commented Jan 7, 2026•
edited
Loading

Copy link
Copy Markdown

Fixes#3013

Summary

  • Add init(), dispose(), and clearSession() to PermissionNext namespace
  • Subscribe to session.deleted bus event to clean up orphaned pending permissions
  • Reject all pending permission requests when their session is deleted

Problem

When a session is deleted, any pending permission requests for that session remain in the pending map indefinitely. This causes:

  • Memory accumulation over time as pending entries are never removed
  • Orphaned promises that never resolve or reject

Solution

Subscribe to the session.deleted event and clean up all pending permissions for deleted sessions by:

  1. Rejecting pending promises with RejectedError
  2. Removing entries from the pending map

Testing

Added comprehensive tests in test/permission/cleanup.test.ts.

CopilotAI review requested due to automatic review settings January 7, 2026 02:09
@github-actions

github-actionsBot commented Jan 7, 2026

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

No duplicate PRs found

CopilotAI reviewed Jan 7, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

FileDescription
packages/opencode/src/permission/next.tsImplements init(), dispose(), and clearSession() functions with session.deleted event subscription to clean up orphaned pending permissions
packages/opencode/test/permission/cleanup.test.tsAdds 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.

Comment threadpackages/opencode/test/permission/cleanup.test.ts Outdated
Comment threadpackages/opencode/test/permission/cleanup.test.ts Outdated
Comment threadpackages/opencode/src/permission/next.ts Outdated
Comment threadpackages/opencode/src/permission/next.ts
Comment threadpackages/opencode/test/permission/cleanup.test.ts Outdated
@hendem
hendem requested a review from CopilotJanuary 7, 2026 03:54
CopilotAI reviewed Jan 7, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment threadpackages/opencode/test/permission/cleanup.test.ts
Comment threadpackages/opencode/src/permission/next.ts
Comment threadpackages/opencode/test/permission/cleanup.test.ts Outdated
Comment threadpackages/opencode/test/permission/cleanup.test.ts Outdated
Comment threadpackages/opencode/test/permission/cleanup.test.ts Outdated
@hendem
hendem requested a review from CopilotJanuary 7, 2026 04:41
CopilotAI reviewed Jan 7, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@hendem
hendem requested a review from CopilotJanuary 7, 2026 04:44
CopilotAI reviewed Jan 7, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment threadpackages/opencode/src/permission/next.ts Outdated
@hendem
hendem requested a review from CopilotJanuary 7, 2026 04:51
CopilotAI reviewed Jan 7, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment threadpackages/opencode/src/permission/next.ts Outdated
@hendem
hendem requested a review from CopilotJanuary 7, 2026 05:00
CopilotAI reviewed Jan 7, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions

github-actionsBot commented Jan 9, 2026

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@hendemhendem mentioned this pull request Jan 9, 2026
Closed
Mark Henderson added 3 commits January 10, 2026 14:09
Mark Henderson@fwang
fix: clean up pending permissions when session is deleted
50c3de7
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.
Mark Henderson@fwang
address PR review comments
e8ae922
- 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)
Mark Henderson@fwang
fix: use Session.Event.Deleted instead of manually constructed event
700188f
Address Copilot review comments - import Session and use the proper
Session.Event.Deleted event definition instead of bypassing type
checking with manually constructed objects.
Mark Hendersonand others added 3 commits January 10, 2026 14:09
Mark Henderson@fwang
fix: reorder operations in clearSession and reply to prevent race con…
1ca0826
…dition
@fwang
Revert "wip: zen"
0cc9f14
This reverts commit 7b9bc5370b08ec55c543de381b88eaf9c050a139.
wip: zen
Reapply "wip: zen"
This reverts commit 09516ac8c3bb7c432b28cf63faa65c0e3b9f9684.
Mark Henderson@fwang
fix: consistent ordering in reply function for once/always cases
a9f2024
@fwang
fwangforce-pushed the fix/permission-approved-map-leak branch from 9f56615 to a9f2024CompareJanuary 10, 2026 19:11
@github-actionsgithub-actionsBot mentioned this pull request Jan 13, 2026
Merged
@github-actionsgithub-actionsBot mentioned this pull request Jan 24, 2026
Closed
@thdxr
thdxrforce-pushed the dev branch 3 times, most recently from f1ae801 to 08fa7f7CompareJanuary 30, 2026 14:37
@github-actionsgithub-actionsBot mentioned this pull request Feb 28, 2026
Closed
6 tasks
@github-actions

github-actionsBot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

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.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@adamdotdevinadamdotdevinAwaiting requested review from adamdotdevin

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uses a huge amount of memory

3 participants

@hendem@fwang