Skip to content

fix(mcp): prune dead notification subscribers - #11032

Merged
filipkujawa merged 1 commit into
aaif-goose:mainfrom
teddytennant:fix/prune-mcp-notification-subscribers-11023
Aug 10, 2026
Merged

fix(mcp): prune dead notification subscribers#11032
filipkujawa merged 1 commit into
aaif-goose:mainfrom
teddytennant:fix/prune-mcp-notification-subscribers-11023

Conversation

@teddytennant

@teddytennant teddytennant commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

GooseClient notification fan-out never removed dead subscribers. Each tool call calls subscribe(), which pushes a Sender into a connection-scoped vec; when the tool call ends the Receiver is dropped, but the Sender stayed forever. Fan-out sites ignored try_send failures, so the handler list grew by one entry per tool call for the life of the extension connection.

This change prunes closed senders during fan-out (same pattern as summon.rs):

  • Add fan_out_notification that retains only live senders (Ok / Full keep; Closed drops).
  • Use it from on_progress and on_logging_message.
  • Build the notification once per fan-out instead of cloning params per dead handler.

Fixes #11023

Test plan

  • cargo fmt -p goose
  • cargo test -p goose fan_out_notification_prunes_closed_subscribers
  • cargo test -p goose --lib agents::mcp_client (27 passed)
  • cargo clippy -p goose --all-targets -- -D warnings

@filipkujawa filipkujawa mentioned this pull request Aug 10, 2026
Closed
@filipkujawa
filipkujawa added this pull request to the merge queue Aug 10, 2026
Merged via the queue into aaif-goose:main with commit 6cf0bf1 Aug 10, 2026
24 checks passed
michaelneale added a commit that referenced this pull request Aug 10, 2026
@michaelneale
Merge remote-tracking branch 'origin/main' into micn/roaming-agents-pr
dc973ca 
* origin/main:
  fix(mcp): prune dead notification subscribers (#11032)
  chore: remove the extension and tool count suggestion (#10869)
  feat: compaction in the GDK (#11042)
  fix(provider): retry transient errors on first stream item before ending turn (#10968)
  feat(cli): add /new to start a fresh session without restarting (#10767)
  feat(acp): title new sessions from _meta.sessionTitle (#10712)
  fix: adjust rmcp::model::Meta ref (#11107)
  Skip hook loading and lifecycle events for subagents (#10596)
  Sanitize Unicode tags in Responses output (#10745)
  fix(conversation): sanitize nested tool responses (#10609)
  fix(hints): bound recursive file expansion (#10546)
  fix(providers): drop stale signed thinking blocks after a mid-conversation model switch (#10007)
  fix(desktop): clarify compact cost display (#11093)
  Index messages by (session_id, created_timestamp, id) to stop on-disk sort storms (#10874)
  docs: add tool shim guide covering when to enable, backends, and troubleshooting (#10858)
  fix(deep-link): route extension/session deep links to regular windows not standalone app windows (#10908)
  fix(ui): raise chat input z-index so slash menu appears above loading indicator (#11015)
  fix(ui): support remote working directory for external backend (#10827)
lifeizhou-ap added a commit that referenced this pull request Aug 11, 2026
@lifeizhou-ap
Merge branch 'main' into lifei/native-steering-claude-acp
b63522e 
* main:
  fix(mcp): prune dead notification subscribers (#11032)
  chore: remove the extension and tool count suggestion (#10869)
  feat: compaction in the GDK (#11042)
  fix(provider): retry transient errors on first stream item before ending turn (#10968)
  feat(cli): add /new to start a fresh session without restarting (#10767)
  feat(acp): title new sessions from _meta.sessionTitle (#10712)
  fix: adjust rmcp::model::Meta ref (#11107)
  Skip hook loading and lifecycle events for subagents (#10596)
  Sanitize Unicode tags in Responses output (#10745)
  fix(conversation): sanitize nested tool responses (#10609)
  fix(hints): bound recursive file expansion (#10546)
  fix(providers): drop stale signed thinking blocks after a mid-conversation model switch (#10007)
  fix(desktop): clarify compact cost display (#11093)
  Index messages by (session_id, created_timestamp, id) to stop on-disk sort storms (#10874)
  docs: add tool shim guide covering when to enable, backends, and troubleshooting (#10858)
  fix(deep-link): route extension/session deep links to regular windows not standalone app windows (#10908)
  fix(ui): raise chat input z-index so slash menu appears above loading indicator (#11015)
  fix(ui): support remote working directory for external backend (#10827)
@github-actions github-actions Bot mentioned this pull request Aug 11, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DOsinga DOsinga DOsinga approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

MCP notification subscribers are never pruned: one dead Sender leaks per tool call, for the life of the connection

3 participants

@teddytennant @DOsinga @filipkujawa