Fix system-message response test subscription races - #2638
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7e3dcf84-04c4-4bb7-8a42-7ca5904f3cf7
devm33
added
post-to-slack
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7e3dcf84-04c4-4bb7-8a42-7ca5904f3cf7
devm33
changed the title
SDK Consistency ReviewReviewed the changes in this PR against the authoritative file list from Files changed (all test-only,
Findings: No SDK production/client code ( Since no public API was added, changed, or removed, there is no cross-language feature-parity gap to flag for Node.js, Python, Go, Java, or Rust. This PR is internal-to-.NET-tests and does not require mirroring in other SDKs. No action needed — consistency is maintained.
|
1 participant
Summary
Fix two test-only lost-idle races:
SystemMessageSectionsE2ETests.Should_Use_Replaced_Preamble_Section_In_ResponseandSessionE2ETests.Should_Create_A_Session_With_Replaced_SystemMessage_Config. Both old scenarios awaitedSendAsyncbefore subscribing throughGetFinalAssistantMessageAsync. A fast turn can dispatch its assistant messages and ephemeralsession.idlebefore that subscription; history can recover assistant content but not idle, leaving the helper waiting until its deadline.Use the existing public
SendAndWaitAsyncin each response scenario so subscription precedes sending. Preserve final-not-intermediate response semantics and both explicit 120-second E2E budgets. The preamble's Botanica gardening configuration,Who are you?prompt, and response assertions remain unchanged. The replaced-message case retains its replacement configuration,What is your full name?prompt, NotNull/Testy/not-GitHub response assertions, nonempty actual exchanges, and exact wire-system-message equality.Two deterministic real-SDK loopback JSON-RPC regressions in
ClientSessionLifetimeTestscall those same E2E response scenarios. Each sends an intermediate assistant, tool activity, final assistant and idle before the send reply. A later delivered event affirmatively fences dispatch without sleeps. History contains both assistant messages but no idle. The published preamble scenario/regression is byte-identical; the new commit adds only the accepted replaced-case repair. Cumulative scope is three test files, with no production SDK, sharedTestHelper, framework, capture, deadline, retry, skip or concurrency change. The appended-message repair in #2636 remains separate and untouched.Original motivating occurrences, both attempt 1 with consumed SDK
f45c46fd1812f8bed5b4cbc250f47177c83068f0:ba1cd6cbad8d0606d2c3d06b41506a512678f8c342e1d40b7fa22a9fcc7740216a2a14c6c386db60Each original leg reported 894 passed, 1 failed, 4 skipped, with
Timeout waiting for assistant messageatTestHelper.cs:77/85(preamble caller line 65; replaced-message caller line 82). These are actual consumed checkouts, not assumed runtime PR heads.The live test-observation races are deterministically proven; both original CI causes remain UNKNOWN. Neither occurrence retained an event/RPC trace proving its interleaving. The separate original SDK automatic timeout and runtime publication-permission boundary are preserved below; this repair is not claimed to fix that timeout.
Validation
Timeout waiting for assistant message; idle drained before send reply: True; durable assistant messages: 2; durable idle events: 0, retaining the original helper stack. Each standalone GREEN passed 1/1, followed by its 126-case whole-unit validation. Those evidence seals remain unchanged.9f4b9e59ca7ce0516790ad18afc31f7bfd7ead01with sole parent7509bc154359e26b2aa55a0c6a92bda6ed6ecac6: wholeClientSessionLifetimeTests127/127, with named pass records proving both new regressions executed.SessionE2ETests38;SystemMessageSectionsE2ETests2), including both modified cases, unchanged identity, and unchanged appended case. All response assertions and replaced-message actual wire equality executed. No completed-suite failures/skips/cleanup errors.ee6465da61c6b394e40d3b5a2de78a485d500f60. All 114 build-output digests were rechecked before/after validation. It was previously staged through the runtime's vendored Bazel and CLI build with documented build-cache reuse; no runtime rebuild or unknown bundle was used for combined validation.GITHUB_ACTIONS=trueblocks live-CAPI fallback and snapshot writes;COPILOT_AUTO_UPDATE=falseandCopilotSkipCliDownload=truepreserve the supplied CLI. This is local Linux replay, not cross-platform/live-CI proof.tsxfixture failures are preserved. The latter affected all 40 selected cases before their bodies and emitted fixture-cleanupNullReferenceExceptiondiagnostics. Dependencies were restored only after those observed failures; no assertions/source/snapshots were weakened. Subsequent combined suites completed successfully.The preamble evidence, separate 48-file replaced-candidate seal and 41-file combined-validation seal remain intact. No original
7509automatic result, historical image-capacity proof, or old source-head proof transfers to the combined commit.9f4b9e59ca7ce0516790ad18afc31f7bfd7ead01The PR remains draft pending at least ten qualifying independent final-head workflow passes and parent review. This publication dispatched, reran and cancelled no CI jobs. The existing release-based .NET workflow is separate from the unpublished fixed-runtime validation branch described below.
Generated by Copilot
Original CI failure and validation gates
The original automatic run 34696477539, attempt 1 on
7509bc154359e26b2aa55a0c6a92bda6ed6ecac6, did not pass. The macOS default/CAPI shard 1 exceeded its configured 20-minute execution limit and was cancelled. The downstream.NET requiredfailure reflects that cancellation, not a second test failure. The other 26 configured .NET jobs succeeded, but those automatic results are not independent proof.The failed shard's log endpoint returned 404 and the run has no diagnostic artifacts. Its exact test, completed case count, and cleanup outcome remain unknown. The annotation identifies the configured limit; the 25-minute start-to-end wall interval does not redefine it. This loop did not cancel or rerun the job, and does not claim the pending combined repair fixes this timeout.
The separate fixed-runtime validation branch is not published. GitHub rejected its normal push because the OAuth app lacks
workflowscope. The validated local runtime commit390a22f5f48cefee7bfc6687e93c2b071e469215remains available, but no remote branch exists. Publishing it requires credentials authorized to modify workflows; no alternate-credential or API-write workaround was attempted.The SDK's existing full .NET workflow remains usable for separate release-based coverage. Its first manual wave will wait for the combined system-message candidate's validation and final pushed SDK SHA. All qualifying proof counts remain 0/10; no historical image-fixture run or automatic check transfers.
Independent manual proof: first workflow did not pass
9f4b9e59ca7ce0516790ad18afc31f7bfd7ead01workflow_dispatch2b-rpc-q-zexceeded the job execution limit. 0/10 qualifying full-workflow successes.Job 103565867411 completed as cancelled on September 12, 2026, at 14:34:27 UTC. Its annotation confirms the 20-minute execution limit; its 25-minute wall interval is a separate measurement. The original job log returned 404, one fallback failed, and the run has no artifacts. The active build/test/cleanup phase, individual case and cause remain unknown. Further proof waves are held; no duplicate dispatch or job rerun was made.
Two original-attempt, source-matched q-z comparison jobs completed in 130 and 165 seconds, each with 109 passes and one skip. They are not proof credits or a tail-latency estimate, and do not justify a generic timeout increase. Existing #2531 changes the general limit alongside production FFI behavior, but no causal link to this failure is established.
The completed original logs were audited: 25 test commands, 27 framework completions, 6,531 passed and 37 skipped, with no unclassified count gaps in those logs. Both repaired E2Es passed all nine expected framework executions. Each new unit regression passed all five eligible framework executions; the existing
NET8_0_OR_GREATERfile guard excludes net472, correcting the earlier six-framework estimate. No affected test was skipped in an eligible execution. These partial passes do not make the cancelled workflow a passing run.Completed jobs logged the exact SDK checkout. Runtime acquisition remains release-based CLI
1.0.84-5, not fixed-runtime-source proof. Release checksum enforcement, producer/consumer paths and runner cleanup are recorded; independent runtime binary hashes and globally leak-free cleanup are not claimed. The cancelled shard's execution population and cleanup have not been established.