Skip to content

Route terminal output sources through active parts - #332472

Merged
Dmitriy Vasyura (dmitrivMS) merged 6 commits into
mainfrom
dev/dmitriv/chat-terminal-output-source-listener-leak
Aug 26, 2026
Merged

Route terminal output sources through active parts#332472
Dmitriy Vasyura (dmitrivMS) merged 6 commits into
mainfrom
dev/dmitriv/chat-terminal-output-source-listener-leak

Conversation

@dmitrivMS

Copy link
Copy Markdown
Collaborator

Summary

  • remove the shared onDidRegisterOutputSource event from ITerminalChatService
  • notify active terminal progress parts directly when an output source is registered
  • let each part filter by terminal tool session ID before attaching the source
  • preserve the initial lookup path when the source is registered before the row renders
  • add duplicate-row regression coverage

Root cause

Every output-only terminal row subscribed to one service-wide onDidRegisterOutputSource emitter. Tool-heavy Agent Host chats accumulated one listener per rendered row until the leak detector crossed its threshold.

Recent Insiders logs identify this path directly:

potential listener LEAK detected, popular
    at ... onDidRegisterOutputSource
    at ... createToolInvocationSubPart

The output-source event had no consumers other than terminal progress rows. TerminalChatService.registerOutputSource() now updates the source map and directly notifies its existing active-progress-part registry. The event and its emitter are removed from the service contract.

Validation

  • npm run transpile-client
  • focused duplicate-row regression test: 1 passed
  • full TerminalChatService suite: 7 passed
  • chat terminal rendering suite: 15 passed
  • 50 active parts are registered, including two rows sharing the target session ID; both matching rows are notified
  • editor diagnostics: no errors
  • git diff --check

Related issues

Related to #311225
Related to #294050

These reports contain multiple independent renderer and extension-host retention paths, so the references are intentionally non-closing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 25, 2026 02:23
@dmitrivMS Dmitriy Vasyura (dmitrivMS) added bug Issue identified by VS Code Team member as probable bug error-telemetry VS Code - Issues generated by telemetry freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues terminal General terminal issues that don't fall under another label labels Aug 25, 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

Routes terminal output-source registration through active progress parts, avoiding service-wide listener accumulation.

Changes:

  • Removes the shared output-source event.
  • Directly notifies active rows, with per-row session filtering.
  • Adds duplicate-row regression coverage.
Show a summary per file
File Description
terminalChatService.test.ts Tests notification of duplicate matching rows.
terminalChatService.ts Routes registrations through active progress parts.
terminal.ts Updates terminal chat interfaces.
chatTerminalToolProgressPart.ts Filters and attaches matching output sources.

Review details

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

  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Balanced

@saroasid-web

Copy link
Copy Markdown

الجامد بزيادة

@saroasid-web

Copy link
Copy Markdown

@dmitrivMS

Copy link
Copy Markdown
Collaborator Author

Copilot resolve the merge conflicts in this pull request

Co-authored-by: dmitrivMS <9581278+dmitrivMS@users.noreply.github.com>

Copilot AI commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Resolved the merge conflicts in 8d07e46. The focused terminal chat service browser tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dmitrivMS
Dmitriy Vasyura (dmitrivMS) merged commit 8df2bf6 into main Aug 26, 2026
37 checks passed
@dmitrivMS
Dmitriy Vasyura (dmitrivMS) deleted the dev/dmitriv/chat-terminal-output-source-listener-leak branch August 26, 2026 09:40
@vs-code-engineering vs-code-engineering Bot added this to the 1.136.0 milestone Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue identified by VS Code Team member as probable bug error-telemetry VS Code - Issues generated by telemetry freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues terminal General terminal issues that don't fall under another label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants