Skip to content

0019: frontend: Clean up desktop IPC listeners - #7277

Open
illume wants to merge 5 commits into
kubernetes-sigs:mainfrom
illume:upstream-listener-cleanup-0019
Open

0019: frontend: Clean up desktop IPC listeners#7277
illume wants to merge 5 commits into
kubernetes-sigs:mainfrom
illume:upstream-listener-cleanup-0019

Conversation

@illume

@illume illume commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • release runCommand IPC subscriptions when the matching command exits
  • use the exact preload unsubscribe callback for all PluginManager completion paths
  • add focused unit and Electron end-to-end coverage for listener cleanup

Source

This upstreams patch 0019 from Azure/aks-desktop#823:
Azure/aks-desktop#823

The original downstream change was Azure/aks-desktop#394:
Azure/aks-desktop#394

Original combined commit:
e4798768d9995204e2ad32da138b11f094640828

PR 823 split that change into these reviewable patch commits:

  • 48e4e03806152cf2aa527147e9a4105150b6b082 for runCommand
  • eeba14bd5891ad62b791d001883ce66810d527d3 for PluginManager

The functional commits retain Tejhan Diallo as author and preserve the original
March 12, 2026 author and commit date. René Dudfield is included as co-author.

Improvements over the existing change

  • adds baseline tests before the production commits, keeping the stack bisectable
  • expands unit coverage to every branch in both changed files
  • verifies command event forwarding and unrelated command filtering
  • covers all plugin response outcomes and public request methods
  • adds an Electron e2e test across the real preload IPC boundary
  • splits the original combined change into focused runCommand and
    PluginManager commits

Testing

  • npx vitest run src/components/App/runCommand.test.ts src/components/App/pluginManager.test.ts --coverage.enabled --coverage.provider=istanbul --coverage.reporter=text --coverage.include=src/components/App/runCommand.ts --coverage.include=src/components/App/pluginManager.ts
    • 15 tests passed
    • 100% branch coverage for runCommand.ts
    • 100% branch coverage for pluginManager.ts
  • npm run tsc
  • focused ESLint and Prettier checks for all changed files
  • PLAYWRIGHT_TEST_MODE=app playwright test tests/listenerCleanup.spec.ts
    • 1 Electron e2e test passed

Screenshots

Not applicable. This change only affects IPC listener lifecycle and has no visual
change.

Assisted by copilot.

tejhan and others added 5 commits March 12, 2026 06:09
Use the exact cleanup callback returned by the preload bridge so wrapped
plugin response listeners are always released.

Co-authored-by: René Dudfield <renedudfield@microsoft.com>
Cover command validation, IPC payloads, and routing before listener changes.
Cover listener outcomes and public requests before changing cleanup.
Exercise the real preload boundary so desktop cleanup stays covered.
Release renderer IPC subscriptions when their command exits so completed
commands do not accumulate listeners.

Co-authored-by: René Dudfield <renedudfield@microsoft.com>
@kubernetes-prow kubernetes-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 16, 2026
@kubernetes-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: illume

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubernetes-prow kubernetes-prow Bot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 16, 2026
@kubernetes-prow
kubernetes-prow Bot requested review from vyncent-t and yolossn August 16, 2026 06:34
@illume
illume requested a balanced review from Copilot August 16, 2026 06:35
@illume
illume marked this pull request as draft August 16, 2026 06:35
@kubernetes-prow kubernetes-prow Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 16, 2026

Copilot AI 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

Prevents desktop IPC listener leaks after commands and plugin operations complete.

Changes:

  • Unsubscribes runCommand listeners on matching command exit.
  • Uses preload-provided cleanup callbacks in PluginManager.
  • Adds unit and Electron E2E coverage.

CI build, lint, docs, Storybook, and pluginctl checks passed; remaining checks are still running and the PR is draft.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
frontend/src/components/App/runCommand.ts Cleans up command listeners.
frontend/src/components/App/runCommand.test.ts Tests event routing and cleanup.
frontend/src/components/App/pluginManager.ts Uses exact unsubscribe callbacks.
frontend/src/components/App/pluginManager.test.ts Tests response and cleanup paths.
app/e2e-tests/tests/listenerCleanup.spec.ts Verifies preload unsubscribe behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@illume
illume marked this pull request as ready for review August 16, 2026 07:10
@kubernetes-prow kubernetes-prow Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 16, 2026
@kubernetes-prow
kubernetes-prow Bot requested a review from joaquimrocha August 16, 2026 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants