Skip to content

Ensure a summarizer stop request is respected after connecting#26776

Merged
kian-thompson merged 1 commit intomicrosoft:mainfrom
kian-thompson:61863-summarizer-stop-race
Mar 20, 2026
Merged

Ensure a summarizer stop request is respected after connecting#26776
kian-thompson merged 1 commit intomicrosoft:mainfrom
kian-thompson:61863-summarizer-stop-race

Conversation

@kian-thompson
Copy link
Copy Markdown
Contributor

@kian-thompson kian-thompson commented Mar 18, 2026

When taking a look at summarizer telemetry, I observed a weird race condition around connection and last summaries. If the parent disconnects and reconnects before the summarizer finishes loading then it won't stop as the parent expects.

Example flow:

  1. Parent is elected
  2. Start loading summarizer
  3. Parent disconnects
  4. Attempt to call summarizer?.stop(...) does nothing since the summarizer has not finished loading (see createSummarizerFn())
  5. Parent reconnects
  6. Summarizer finishes loading and returns instance to parent
  7. Summarizer doesn't recognize that it was supposed to stop, so it continues to live as normal
  8. Parent hits "SummarizerStopTimeout" since it expected its summarizer to stop

This PR adds a stop check right after step 6.

AB#61863

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 fixes a race where a stop request (triggered while the summarizer is still being created) could be lost, causing the summarizer to continue running and eventually hit SummarizerStopTimeout. It adds a mechanism to remember the stop reason during async summarizer creation and replay it immediately once the summarizer instance is available.

Changes:

  • Track a pending stop reason while awaiting createSummarizerFn() and replay it immediately after the summarizer is created.
  • Clear pending stop intent during cleanup/dispose to avoid cross-cycle contamination.
  • Add a targeted unit test covering “stop requested before summarizer creation completes” and validating last-summary behavior.

Reviewed changes

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

File Description
packages/runtime/container-runtime/src/summary/summaryManager.ts Adds pendingStopReason to persist and replay stop intent after async summarizer creation.
packages/runtime/container-runtime/src/test/summary/summaryManager.spec.ts Adds a regression test for the stop-before-create race and test plumbing to observe stop reason + summarize events.

Comment thread packages/runtime/container-runtime/src/summary/summaryManager.ts
Copy link
Copy Markdown

@shlevari shlevari left a comment

Choose a reason for hiding this comment

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

Approved with suggestions for added comment

Comment thread packages/runtime/container-runtime/src/summary/summaryManager.ts
@kian-thompson kian-thompson merged commit 155b1c2 into microsoft:main Mar 20, 2026
44 checks passed
@kian-thompson kian-thompson deleted the 61863-summarizer-stop-race branch March 20, 2026 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants