Skip to content

Stabilize session lifecycle E2E tests - #2687

Merged
stephentoub merged 1 commit into
mainfrom
mackinnonbuck-stabilize-merge-queue-tests
Sep 16, 2026
Merged

stephentoub merged 1 commit into
mainfrom
mackinnonbuck-stabilize-merge-queue-tests

Conversation

@MackinnonBuck

Copy link
Copy Markdown
Collaborator

Summary

  • run the Node handler-reentrancy detach check from session.idle, after the active turn has completed, so it tests disconnecting inside event dispatch without also racing active-turn cancellation
  • detach the Python session before resuming it with custom-agent configuration, matching persisted-session ownership semantics and the stable .NET coverage
  • leave replay snapshots unchanged

Root causes

The Windows Node failure was not an event-handler dispatch deadlock. The test detached on user.message while the turn was still active, so session.detach could wait on active-turn shutdown; when it remained pending, test teardown disposed the JSON-RPC connection and produced -32097.

The macOS Python failure resumed the same session while its original attachment was still live. Applying custom-agent configuration during that overlapping ownership transition could leave session.resume pending. The replay proxy's No stored conversations messages were interleaved output from another xdist worker, not a missing fixture for the timed-out request; timeout diagnostics showed the blocked RPC was session.resume.

Validation

  • Node focused test: 20/20 consecutive passes on Windows
  • Node session.e2e.test.ts: 37 passed, 2 skipped
  • Node Prettier, ESLint, and TypeScript checks passed
  • Python focused test: 20/20 consecutive passes for default transport and 20/20 for in-process
  • Python create-then-resume sequence: 10/10 passes for default transport and 10/10 for in-process
  • Python test_mcp_and_agents_e2e.py: 11/11 passes for default transport and 11/11 for in-process
  • Python Ruff format/lint and ty check copilot passed

This is based directly on current main and is independent of #2544.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 16, 2026 02:02
@MackinnonBuck
MackinnonBuck requested a review from a team as a code owner September 16, 2026 02:02

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.

Copilot review overview

🟢 Approval recommended

The targeted lifecycle sequencing changes are consistent with session semantics and introduce no unresolved issues.

Review tier: Balanced
Findings: None

What changed in this PR

Stabilizes session lifecycle E2E tests by avoiding overlapping session ownership and active-turn cancellation races.

Changes:

  • Node disconnect test now runs from session.idle.
  • Python resume test disconnects the original session first.
File Description
nodejs/​test/​e2e/​session.e2e.test.ts Tests handler-triggered disconnect after turn completion.
python/​e2e/​test_mcp_and_agents_e2e.py Detaches before resuming with custom-agent configuration.

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

@stephentoub
stephentoub added this pull request to the merge queue Sep 16, 2026
Merged via the queue into main with commit 062ee60 Sep 16, 2026
46 checks passed
@stephentoub
stephentoub deleted the mackinnonbuck-stabilize-merge-queue-tests branch September 16, 2026 04:24
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