Skip to content

fix(vscode-ide-companion): stop leaking gemini.diff.accept and onDidChangeWorkspaceFolders disposables - #28526

Closed
godququ5-code wants to merge 3 commits into
google-gemini:mainfrom
godququ5-code:fix/vscode-companion-disposable-leak
Closed

fix(vscode-ide-companion): stop leaking gemini.diff.accept and onDidChangeWorkspaceFolders disposables#28526
godququ5-code wants to merge 3 commits into
google-gemini:mainfrom
godququ5-code:fix/vscode-companion-disposable-leak

Conversation

@godququ5-code

Copy link
Copy Markdown

Summary

Fixes #27790.

The two context.subscriptions.push(...) calls in activate() wrapped a pair of registrations in a stray parenthesis pair, collapsing them into a comma expression whose value was only the last operand. As a result the gemini.diff.accept command Disposable and the onDidChangeWorkspaceFolders listener Disposable were created but never added to context.subscriptions, so they were never disposed on deactivation. On re-activation within the same extension host this threw command 'gemini.diff.accept' already exists, and the stale onDidChangeWorkspaceFolders listener kept firing ideServer.syncEnvVars() against a stopped IDEServer.

  • Removed the extra parentheses so each registration is its own argument to push().
  • Added a regression test in extension.test.ts that tags the created Disposables and asserts both gemini.diff.accept and onDidChangeWorkspaceFolders land in context.subscriptions.

Test plan

  • cd packages/vscode-ide-companion && npm test (vitest): new test in extension.test.ts passes; existing tests unaffected.

…hangeWorkspaceFolders disposables

The two context.subscriptions.push(...) calls wrapped a pair of
registrations in a stray parenthesis pair, collapsing them into a comma
expression whose value was only the last operand. As a result the
`gemini.diff.accept` command Disposable and the `onDidChangeWorkspaceFolders`
listener Disposable were created but never added to context.subscriptions, so
they were never disposed on deactivation (re-activation then threw
'command already exists' and a stale listener kept firing).

Remove the extra parentheses so each registration is its own argument to
push(). Add a regression test that tags the created Disposables and asserts
both land in context.subscriptions.

Fixes google-gemini#27790
@godququ5-code
godququ5-code requested a review from a team as a code owner July 24, 2026 06:38
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a resource leak issue where specific command and listener registrations were not being added to the extension's subscription list due to incorrect syntax. By removing the stray parentheses, these objects are now correctly tracked, ensuring they are properly disposed of during extension deactivation and preventing duplicate registration errors on re-activation.

Highlights

  • Fixed Disposable Registration: Removed incorrect parentheses in context.subscriptions.push() calls that were causing comma expressions, preventing some Disposables from being tracked and disposed of correctly.
  • Regression Testing: Added a new test case in extension.test.ts to verify that both the gemini.diff.accept command and the onDidChangeWorkspaceFolders listener are properly registered in context.subscriptions.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@github-actions github-actions Bot added the size/s A small PR label Jul 24, 2026
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

📊 PR Size: size/S

  • Lines changed: 41
  • Additions: +37
  • Deletions: -4
  • Files changed: 2

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request fixes a bug where certain VS Code command and listener registrations were wrapped in stray parentheses, causing them to be evaluated as comma expressions and preventing them from being tracked for disposal in context.subscriptions. A new test is added to verify that all created Disposables are correctly registered. The reviewer suggested using optional chaining when inspecting context.subscriptions in the test to avoid potential TypeError exceptions from undefined elements.

Comment thread packages/vscode-ide-companion/src/extension.test.ts Outdated
@gemini-cli gemini-cli Bot added priority/p2 Important but can be addressed in a future release. area/core Issues related to User Interface, OS Support, Core Functionality labels Jul 24, 2026
Per gemini-code-assist[bot] review on google-gemini#28526, the mocked VS Code APIs
can push `undefined` entries into `context.subscriptions` by default in
the test environment. Accessing `d.command`/`d.kind` directly therefore
throws `TypeError: Cannot read properties of undefined`.

Widen the element type to `{ command?; kind? } | undefined` and use
optional chaining (`d?.command` / `d?.kind`) so the assertion tolerates
`undefined` entries without changing what the test actually verifies.
@gemini-cli

gemini-cli Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Hi there! Thank you for your interest in contributing to Gemini CLI.

To ensure we maintain high code quality and focus on our prioritized roadmap, we only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'.

This PR will be closed in 7 days if it remains without that designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding.

@godququ5-code

Copy link
Copy Markdown
Author

Thanks for the guidance. I checked issue #27790: it currently has good first issue but not help wanted, and PR #28580 now covers the same bug. PR #28526 contains the registration fix plus a regression test, and its current checks pass. I am happy to follow the maintainers preferred PR and will not duplicate work. If #28526 should still be considered, please advise whether the issue should receive help wanted or whether I should defer to #28580 before the seven-day window.

@godququ5-code

Copy link
Copy Markdown
Author

@DavidAPierce — apologies for the direct ping, and please ignore if this isn't your area. I noticed you've been the reviewer on essentially all recent packages/vscode-ide-companion changes (#28510, #28552, #28569, #28573, #28707), so you seemed like the right person to ask.

This PR is scheduled for automatic closure tomorrow (Aug 8) under the "no help wanted label" policy — the bot notice landed on Aug 1. I followed the CONTRIBUTING.md process and nominated the underlying issue #27790 for the label on Aug 4, but it hasn't been triaged yet.

Quick summary so a decision is cheap to make:

  • Bug: in activate(), two context.subscriptions.push(...) calls use comma expressions, so gemini.diff.accept and onDidChangeWorkspaceFolders are registered but never tracked for disposal — they leak on deactivate/reload. Still reproducible on main.
  • Why existing tests don't catch it: expect(vscode.workspace.onDidGrantWorkspaceTrust).toHaveBeenCalled() passes even with the bug present, because a comma expression evaluates both operands. The new tests assert on the contents of context.subscriptions instead, and fail when the fix is reverted.
  • Diff: +37/-4, one source file plus tests. Checks are green.

Two community PRs fix this (#28526 opened Jul 24, #28580 opened Jul 29) and three contributors have now proposed patches, so there's clearly appetite to help here — the only thing gating review is the missing label.

Completely fine if the answer is "not a priority right now" — a one-line reply saying so would let everyone involved stop waiting, and I'd have no issue with this PR being closed. Thanks for your time either way.

@gemini-cli

gemini-cli Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding.

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

Labels

area/core Issues related to User Interface, OS Support, Core Functionality priority/p2 Important but can be addressed in a future release. size/s A small PR status/pr-nudge-sent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(vscode-ide-companion): comma operator in activate() leaks two Disposables (gemini.diff.accept, onDidChangeWorkspaceFolders)

1 participant