Fix SDK test completion races and timeout cleanup

[stephentoub]

Summary

Fix completion-subscription races in SDK tests and two Python test-lifecycle defects, preserving the APIs under test, caller budgets, and output assertions.

All workflow files are unchanged from the PR base. At the author's request, 7f5737e9 removes the investigation-only workflow edits, .NET/Go watchdogs and stack collectors, diagnostic Go TestMain, associated diagnostic-only tests/tool manifest, and reproduction documentation. Original CI commands, matrices, deadlines, and artifact behavior are restored. The actual completion fixes and Python interrupted-task cleanup remain.

SDK product sources, runtime pins, and snapshots are unchanged.

Current head: 7f5737e9087f3a1b74d53a6297f66ef504a80acd. SDK run 34789202978 is pending. The preceding revision 77127f86 passed all 61 SDK jobs and all 82 reported PR checks; those results are historical, not a current-head green claim.

Completion-subscription repair

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	Use 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 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 caller timeouts/error policies across abort, pending-work, mode, and todo tests.
Java / Rust	Audited completion APIs, subscriptions, abort/recovery, and pending work. No affected live callers identified; no code changes.

Fake-RPC regressions force early terminal events before send replies, with idle omitted from durable history. Preserve prompts, session setup, output assertions, and nonblocking-send coverage. No forced already-idle flags, sleeps, timeout increases, or assertion weakening.

Python test-lifecycle repairs

Abandoned task after pytest-timeout

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 preserves the timeout report 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. Actual-plugin/module-fixture controls prove that old behavior reproduces the lock-only teardown stall, the fix removes it, and a runtime that continues withholding replies still causes teardown failure.

The active Python harness still reports suspended coroutine await chains, pending RPC methods/identifiers, lock/transport state, and bounded thread diagnostics before cancellation. Payloads and arbitrary locals are excluded. These reports remain in pytest output/local files; the experimental workflow uploader and manual repetition modes have been removed.

The original session-config test's initial 300-second timeout is not conclusively attributed to this cleanup defect. Its exact stalled await was absent from the original log. Six SDK RPC and six proxy probes confirmed the cleanup defect but did not identify that primary await. The proxy's HTTPX calls do not share the SDK's stdout response-future map.

Complete the persisted-session fixture before cleanup

Python Alpine ARM64/inprocess job 103801413979 failed test_should_list_find_and_inspect_persisted_session_state. Diagnostics identify session.disconnect() in the test's finally, awaiting session.detach for 299.922 seconds. This test creates an explicit stdio client even in the inprocess matrix cell; the shared fixture's FFI state is not the blocked connection.

The fixture enqueued an uncontrolled model turn merely to make persisted-session listings nonempty, then inspected metadata and detached without observing completion. It now reuses existing synthetic inference-response helpers, awaits send_and_wait with the same prompt and a 60-second completion budget, verifies the synthetic assistant response, and then saves/lists metadata. All nonempty-list, exact-session, LocalSessionMetadataValue, and is_remote=False discriminator assertions remain. The obsolete persistence polling loop is removed. A nested finally also stops the per-test client if detach raises, without suppressing that detach failure.

Two controls invoke this exact E2E scenario with a controlled client: both fail against the original fixture and pass after repair, proving completion-before-save and cleanup-on-detach-failure. This fixes the fixture's unobserved inference/cleanup overlap; it does not claim to identify or repair the native reason that the original detach reply was withheld.

Validation

• Current cleanup: all workflow files, Go README, and Go ignore rules verified identical to the PR base; no dangling references to removed watchdog/dispatch infrastructure. All 21 retained Python timeout/fixture controls passed. Go E2Es compile without the diagnostic TestMain/package (empty test selection; not a full E2E run). Worktree clean and branch pushed.
• Pre-removal head 77127f86: SDK run 34787031867 passed all 61 jobs, including Python Alpine ARM64/inprocess. All 82 reported PR checks passed. No retry.
• Persisted-session fix: both shared-scenario controls fail against the original E2E source and pass after repair. Real Windows stdio E2E passed in 14.08 seconds with pinned CLI 1.0.84-5. Ruff and formatting checks passed; no snapshot edits.
• Python timeout cleanup: previously 19 focused tests, Ruff, and SDK typechecking passed; all three actual-plugin controls passed real POSIX SIGALRM on macOS and Linux.
• Original .NET repair at fa8fa0d4: both new ordering cases fail with old code; all 22 focused cases and all 16 permission E2Es pass after repair. net472 build, formatting, and .NET CI passed. Local E2E used an isolated verified copy of the supplied runtime build, not the exact failing CI binary.

Full-suite validation uses CI as requested. No existing SDK tests were disabled or assertions weakened. Diagnostic-only controls were removed together with the reverted investigation infrastructure. No test-failure reruns-to-green or manual CI cancellations were used; only the two confirmed external download failures below received targeted retries.

Preserved investigation history and limitations

The removed instrumentation and its evidence remain in commit history/session artifacts. Passing subsequent runs do not establish the primary cause of earlier unattributed hangs.

Run	Result and disposition
34785968413, c9aee7d9	49/50 SDK test jobs passed. Python Alpine: 933 passed, nine existing skips, one 300-second detach timeout, leading to the persisted-session fixture repair. All Go/.NET/Node gates passed. Artifacts confirm natural exit 0 for Go macOS in 127.533s and Windows .NET in 473.532s (978 passed, six existing skips), without watchdog intervention.
34768280584	Initially two Node jobs failed before tests downloading the pinned CLI asset with HTTP 500. The identical URL recovered (HEAD 200/range GET 206); one targeted retry each passed in attempts 2 and 3. Final 48/50 test jobs passed. Go/.NET hang results were carried forward, not rerun.
Go job 103753104455	macOS/inprocess hit the annotated 20-minute job limit without a test/goroutine trace. The per-package 20-minute timer lost to the enclosing deadline. Identical inputs passed 103746971240 in 158.794s. Primary cause remains unattributed.
Windows .NET job 103753104714	Annotated 20-minute timeout; metadata spans 25m01s, extra time unexplained. Logs return BlobNotFound; no shard log/artifact. Identical inputs passed 103746971275 with 978 passed/six existing skips. Primary cause remains unattributed. Aggregate failures reported these cancelled dependencies, not separate defects.
34765985408	49/50 test jobs passed. macOS .NET shard 1 reached the outer 20-minute limit without retained evidence. Later instrumented 103753104713 passed 555 tests/two existing skips with unchanged method identities and case multiplicities, naturally exiting in 207.214s; non-reproduction, not attribution.
34765982698, 34764284304	Ten targeted session-config invocations completed; the earlier stress run passed four full 925-test suites before its overambitious fifth invocation exhausted the job budget. Reproduction scopes were corrected before later being removed entirely. These were diagnostic non-reproductions, not proof of a primary timeout fix.
34763599690	Passed completely.
34761514877	49/50 test jobs passed. Python macOS/inprocess: 913 passed, one failed, nine existing skips, one teardown error. System-message resume timed out at 300s; module teardown timed out after the available-tools body passed. Initial stalled await was absent from logs; secondary cleanup defect reproduced and repaired separately.
34760106004	Five newly introduced Node fake-RPC fixtures deadlocked. df3457bd added the existing sendStarted fence before completing fake sends; subsequent Node configurations passed. An earlier Go Windows/default timeout later passed unchanged without attribution.

The removed diagnostics were independently validated before removal: 21 local Node watchdog controls (four existing POSIX-only controls inapplicable on Windows), seven Go focused test functions, Go vet, a zero-warning Windows helper build, and actual macOS/Windows CI checks. Their removal restores the original CI behavior rather than claiming the diagnostics themselves fixed an underlying runtime hang.

Origin

This follows github/copilot-agent-runtime#20254 and its original optional CAPI C# SDK failure, using #2635's 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.

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 of change: This PR touches only test infrastructure (E2E test harnesses/helpers and their call sites) across Node.js, Python, Go, and .NET, plus a Python README note and .gitignore entry. No production SDK client code (nodejs/src, python/copilot, go/*.go outside internal/e2e, dotnet/src, java/sdk/src/main, rust/src) is modified. Java and Rust have no changes in this PR at all.

What changed: Each touched language's E2E test helper (GetFinalAssistantMessage/getFinalAssistantMessage/get_final_assistant_message) was refactored to subscribe to session events before triggering the send (rather than racily backfilling from history after the fact), fixing a latent race where an ephemeral session.idle event could be missed. The pattern is applied consistently:

• Go: testharness.GetFinalAssistantMessage → SubscribeToFinalAssistantMessage/SubscribeToEvent + EventWaiter.Wait, plus GetFinalAssistantMessageFromHistory for the already-resumed-idle case.
• .NET: TestHelper.GetFinalAssistantMessageAsync → TestHelper.SendAndGetFinalAssistantMessageAsync, now delegating to the production session.SendAndWaitAsync API instead of reimplementing the subscribe/backfill race.
• Node.js: getFinalAssistantMessage → withFinalAssistantMessage(session, trigger), subscribing before invoking the trigger callback.
• Python: bespoke per-test-file waiters consolidated into shared _session_test_helpers.py (wait_for_event, get_next_event_of_type), plus new signal-safe async-timeout diagnostics for CI flake triage.

Cross-SDK consistency assessment: ✅ No parity issues found.

• The underlying production sendAndWait/SendAndWaitAsync/send_and_wait/SendAndWait APIs in all six SDKs (Node, Python, Go, .NET, Java, Rust) already implement the correct "subscribe before send" ordering and already skip autopilot-mode session.idle events — this PR only brings the test harnesses up to the same standard the production code already followed.
• Java and Rust E2E tests already call the production sendAndWait/send_and_wait methods directly (not a bespoke harness helper reimplementing the race-prone pattern), so they were not exposed to the bug being fixed here and require no corresponding change.
• No new public API surface was added to any SDK; naming, behavior, and error handling introduced (test-only) are internally consistent within each language's existing conventions.
• No stale references to the removed helper functions remain in any language.

Conclusion: This PR is a well-scoped, test-only race-condition fix applied uniformly to the languages that had the affected harness pattern. It does not introduce any single-language feature or behavior change that needs to be mirrored elsewhere. No inline review comments needed.

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