Skip to content

[Server] Fix a flaky timing test and show why a concurrent session writer fails - #717

Merged
chr-hertel merged 2 commits into
modelcontextprotocol:mainfrom
chr-hertel:fix-stateless-responder-timing
Nov 9, 2026
Merged

chr-hertel merged 2 commits into
modelcontextprotocol:mainfrom
chr-hertel:fix-stateless-responder-timing

Conversation

@chr-hertel

@chr-hertel chr-hertel commented Nov 9, 2026 •

Copy link
Copy Markdown
Member

The idle turn of StatelessResponder sleeps 250 ms, and the Windows job measured 0.2496 s on #713. The test only needs to show that the turn waits, so it now accepts 0.2 s.

FileSessionStoreTest::testConcurrentWritesNeverPublishAPartialPayload keeps failing on Windows because a writer's write() returns false, without saying why. The writers now log the store's warning to their error file, and the failure message includes it - the fix follows once CI shows the cause.

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.

🟢 Approval recommended

No active review issues are recorded for the StatelessResponder test’s timing-tolerance adjustment.

No open issues 🎉

What changed in this PR

Reduces timing-related flakiness in the server’s StatelessResponder unit test while preserving the check that an idle turn waits.

Changes:

  • Lowers the minimum elapsed-time assertion from 250 ms to 200 ms.
  • Adds a comment explaining that Windows can end the sleep slightly early.
File Description
tests/​Unit/​Server/​Transport/​Http/​StatelessResponderTest.php Relaxes the timing threshold and documents the Windows tolerance.

Review effort: Balanced

@chr-hertel chr-hertel changed the title [Server] Allow for a sleep that ends early in the StatelessResponder test [Server] Fix a flaky timing test and show why a concurrent session writer fails Nov 9, 2026
@chr-hertel
chr-hertel requested a balanced review from Copilot November 9, 2026 23:57

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.

🟢 Approval recommended

No active review issues are recorded for the test-timing and concurrent-write diagnostic changes.

No open issues 🎉

Review effort: Balanced

@chr-hertel
chr-hertel merged commit fe962d9 into modelcontextprotocol:main Nov 9, 2026
33 checks passed
@chr-hertel
chr-hertel deleted the fix-stateless-responder-timing branch November 9, 2026 23:59
@chr-hertel chr-hertel added the Server Issues & PRs related to the Server component label Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants