Fix SDK test completion races and timeout cleanup

[stephentoub]

Summary

Fix completion-subscription races across SDK test suites, repair a reproducible Python timeout-cleanup defect, and retain diagnostics for intermittent Python and .NET timeouts. SDK product code, dependency manifests, and snapshots are unchanged.

session.idle is ephemeral: subscribing after send returns can permanently miss completion, and durable history cannot recover idle. Starting a goroutine or scheduling an asyncio task is not itself a subscription fence.

SDK	Repair
.NET	Replace late waits with a shared send-and-wait helper, preserving the 120-second default and requiring a current-turn assistant message. Keep tests of SendAsync using pre-armed observation.
Go	Pre-arm 25 final-message waits and three abort/recovery waits. Separate live completion from reads of independently completed history; preserve caller contexts.
Node.js	Use sendAndWait for ordinary turns and a subscribe-before-trigger helper when testing send or releasing permission handlers. Clean up listeners on every terminal path.
Python	Use send_and_wait for ordinary turns; synchronously subscribe before scheduling waiter tasks. Preserve timeouts/error policies across abort, pending-work, mode, and todo tests.
Java / Rust	Audit completion APIs, subscriptions, abort/recovery, and pending work. No affected callers identified; no code changes.

Remove unsound idle-history fallback while preserving prompts, session setup, output assertions, and tests of nonblocking send. No forced already-idle flags, sleeps, timeout increases, or assertion weakening.

Confirmed Python cleanup defect

pytest-timeout can interrupt Runner.run outside the coroutine, leaving the timed-out test task alive and holding a session-disconnect lock. Module teardown then waits on that abandoned task, causing a second timeout.

The harness now preserves diagnostics and cancels exactly the interrupted runner's test task. It does not cancel unrelated tasks, force-stop the runtime, or turn the initial timeout into success. Three actual-plugin/module-fixture controls verify the mechanism: the old behavior reproduces the lock-only teardown stall; the fix removes it; a runtime that continues withholding replies still causes teardown failure.

The original session-config test's initial 300-second timeout is not conclusively attributed to this cleanup defect. Its exact stalled await was not captured in the original log, and no speculative SDK/native repair is claimed.

Diagnostics and reproduction

Python timeout reports capture coroutine/async-generator await chains, pending RPC identifiers/methods, lock/transport state, Python thread stacks, and a bounded macOS native sample before cleanup destroys evidence. RPC payloads and arbitrary frame locals are excluded.

An opt-in manual reproduce_timeout dispatch supports two full-suite invocations or ten session-config invocations with the usual pytest/xdist options. It stops at the first nonzero exit and retains the existing 20-minute job budget. Progress artifacts record each started/completed invocation and mark completion only when all planned invocations pass. Ordinary Python PR/reusable test invocations are unchanged.

The .NET macOS/default CAPI shard-1 job later exceeded its 20-minute limit without retrievable logs or artifacts. A watchdog now wraps that cell's unchanged test command and selection, reserving time to retain evidence before the outer job deadline. It records allowlisted phase progress, owned-process metadata, and bounded native call graphs, including when a descendant retains output pipes after dotnet exits. It preserves the first nonzero exit, reports watchdog expiry as failure, and terminates only its owned process group. The existing per-test blame timeout and outer job budget remain unchanged. Arbitrary console output and RPC payloads are not added to diagnostic artifacts.

The earlier .NET hang remains unattributed. The diagnostic run passed without watchdog intervention; this is verified instrumentation and non-reproduction evidence, not a claimed root-cause fix.

Validation

Published revision: 733232b44bcd6f7957399fce7955401df9dfed92.

• Current SDK run 34768280584, after two targeted infrastructure retries: 48/50 SDK test jobs passed; two timed out. All Node checks, including Node.js required, passed. Go required and .NET required remain failing because of the original Go macOS and .NET Windows timeouts described below. Full-matrix validation is not green.
• Two Node jobs originally failed before tests downloading the pinned CLI Linux x64 release asset with HTTP 500. The identical URL subsequently returned HEAD 200 and range-GET 206. Each affected job was then retried once, separately: build/validation passed in attempt 2, and Ubuntu/default tests passed in attempt 3. No Go or .NET test hang was rerun; GitHub carried those original results forward under new job IDs.
• .NET diagnostic job 103753104713: 555 passed, two existing skips, zero failures. All 452 method identities and case multiplicities match the prior healthy shard. The wrapped command finished in 207.214 seconds with exit 0; testhost shutdown and output drain completed normally, and artifact upload succeeded.
• All 11 watchdog controls passed on macOS, including real native sampling, POSIX signal handling, descendant cleanup, and retained-pipe cases preserving timeout/nonzero exit status. Seven Windows-compatible controls also passed locally; the other four are platform-specific controls, not newly skipped SDK tests.
• Cleanup repair: 19 focused local tests, Ruff, and SDK typechecking passed. All three cleanup controls also passed on real POSIX SIGALRM in macOS job 103745560405 and Linux job 103746971152. Earlier diagnostic work passed 25 focused tests.
• Targeted reproduction 34765982698: all ten session-config invocations passed. The retained reproduction-summary.txt confirms all ten completed and reproduction_complete=true.
• Pre-cleanup stress 34764284304: four full suites passed 925 tests each. The original five-invocation diagnostic configuration then exhausted the job budget at the start of the fifth invocation. That diagnostic-budget defect is corrected by the bounded scopes above; the interrupted run is not reported as successful.
• Diagnostic SDK run 34763599690 passed completely.
• SDK run 34765985408: 49/50 test jobs passed; .NET macOS/default CAPI shard 1 exceeded the 20-minute job limit, motivating the watchdog above.

Four full baseline passes and ten targeted passes are non-reproduction evidence, not proof that the initial Python timeout is repaired. Six SDK RPC probes and six proxy-control probes exercised withheld replies, early/late replies, cancellation, large/truncated responses, EOF/recovery, and concurrent shutdown. They confirmed the cleanup defect but did not establish the original timed-out await. get_exchanges() uses independent HTTPX requests with inactivity timeouts, not a shared stdout response-future map.

The expanded race repair used CI as requested. Local validation was limited to later diagnostic/cleanup additions where focused checks were needed. No test-failure reruns-to-green, manual CI cancellations, newly skipped existing SDK tests, or weakened checks were used. Only the two confirmed external download failures received targeted retries. Genuine pushes use the repository's existing PR concurrency behavior.

Outstanding CI timeouts

Both remaining failures are unattributed, not established as PR-caused, pre-existing, or transient infrastructure:

• Go macOS/inprocess, original job 103753104455: annotation confirms the 20-minute job limit. Logs contain no E2E test markers, panic, or goroutine stack identifying the stalled operation. The unchanged Go test timeout is also 20 minutes, so the enclosing job expires first. Identical Go inputs, CLI 1.0.84-5, Go 1.24.13, and runner image passed comparator job 103746971240, whose E2E package took 158.794 seconds. Seven other Go test configurations passed the current run. This establishes intermittency, not its cause.
• .NET Windows/default CAPI shard 1, original job 103753104714: annotation confirms the 20-minute job limit. Metadata spans 25m01s, but the additional elapsed time is not causally explained. Completed-job logs return BlobNotFound; the original-attempt log archive contains no entries for this shard, and no Windows diagnostic artifact exists. The combined build/test step began, but its stalled phase, framework, and test remain unknown. Healthy comparator 103746971275 used unchanged .NET source/build inputs and passed 978 tests with six existing skips, including the four new race-regression cases. The macOS-only watchdog does not run in this Windows job, whose command and filter are unchanged.

No speculative source or infrastructure changes were made for these two incidents. Further pre-timeout evidence capture is a follow-up; neither timeout is claimed repaired, and neither was retried to obtain a green result.

Preserved failure history and origin

Run 34761514877 passed 49/50 test jobs. Python macOS/inprocess reported 913 passed, one failed, nine existing skips, and one teardown error: unchanged test_should_apply_systemmessage_on_session_resume timed out at 300 seconds; module teardown timed out after the available-tools test body passed. The old log lacked actionable async/native stacks and artifacts. Identical Python code passed the preceding run.

First expanded run 34760106004 exposed five new Node fixture deadlocks: fake RPC completion preceded installation of its resolver. Commit df3457bd added the existing sendStarted fence; all Node configurations then passed. An earlier Go Windows/default 20-minute timeout lacked retained diagnostics; unchanged Go code passed subsequently, without claiming attribution.

This follows github/copilot-agent-runtime#20254 and its original optional CAPI C# SDK failure, using #2635's analogous abort-recovery fix as prior art. The original CI incident cannot conclusively be attributed to the reproduced subscription race because runtime logs were not retained.

Historical targeted validation at fa8fa0d4: both new ordering cases failed with old code, then all 22 focused cases and all 16 permission E2Es passed; net472 build, formatting, and its .NET CI matrix passed. Local E2E used an isolated verified copy of the supplied runtime build, not the exact failing CI binary.

Generated by Copilot

[Copilot]

Copilot review overview

🟢 Approval recommended

The test-only changes correctly eliminate the subscription race and cover both event orderings.

Review tier: Balanced
Findings: None

What changed in this PR

Fixes a race in the .NET approve-all permission E2E test by subscribing before sending.

Changes:

• Uses SendAndWaitAsync for race-free completion.
• Adds deterministic early/late idle regression coverage.
• Extracts a reusable fake-server event-delivery fence.

File	Description
dotnet/​test/​E2E/​PermissionE2ETests.cs	Updates and extracts the send/wait scenario.
dotnet/​test/​Unit/​ClientSessionLifetimeTests.cs	Adds regression cases and centralizes event fencing.

---

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

[github-actions]

SDK Consistency Review — PR #2642

Scope confirmed via API: All 49 changed files are test infrastructure only — E2E test suites, shared test harnesses/helpers (dotnet/test/Harness, go/internal/e2e/testharness, nodejs/test/e2e/harness, python/e2e/testharness, new python/_session_test_helpers.py), a new .NET CI watchdog script (dotnet/ci/test-watchdog.mjs), and related workflow files. No files under nodejs/src, python/copilot, go/*.go (non-test), dotnet/src, java/sdk/src/main, or rust/src are touched. Public SDK product APIs are unchanged.

Cross-SDK consistency assessment:

• The PR fixes the same underlying race (subscribing to session.idle/assistant.message after triggering the turn, which can miss the ephemeral session.idle event) consistently across the four affected SDKs: .NET (SendAndGetFinalAssistantMessageAsync), Go (SubscribeToFinalAssistantMessage/EventWaiter), Node.js (withFinalAssistantMessage), and Python (wait_for_event/get_next_event_of_type in the new shared _session_test_helpers.py).
• Each language's fix follows its own idiomatic pattern (subscribe-before-trigger) while preserving equivalent semantics (fail on unmatched session.error, require a current-turn assistant message before idle) — good parity across the ports.
• Java and Rust were explicitly audited per the PR description and already use the correct subscribe-before-trigger pattern (Rust: wait_for_final_assistant_message/wait_for_idle in rust/tests/e2e/support.rs; Java: session.sendAndWait(...) in E2E tests) — no changes needed there, and I found no discrepancy in the checked-out sources that would suggest otherwise.
• No new test-only helper method was added to only one SDK without an equivalent elsewhere; the four reworked helpers are analogous in name and behavior (accounting for language conventions), and the two unaffected SDKs are consistent with the target pattern already.

Conclusion: No cross-SDK API/feature-parity issues found. This is a coordinated, consistent test-infrastructure fix across languages with no public SDK surface changes.

Generated by SDK Consistency Review Agent for #2642 · copilot · sonnet50 · 49.4 AIC · ⌖ 12.2 AIC · ⊞ 8.3K · ◷