Skip to content

Fix active Node and Rust replay E2E flakes - #2186

Merged
SteveSandersonMS merged 4 commits into
mainfrom
roji-fix-pure-ci-flakes
Aug 4, 2026
Merged

Fix active Node and Rust replay E2E flakes#2186
SteveSandersonMS merged 4 commits into
mainfrom
roji-fix-pure-ci-flakes

Conversation

@roji

@roji roji commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

July CI runs exposed two hermetic test failures: arbitrary Node E2E tests exhausted the fixed Windows timeout budget, and a repurposed Rust metadata test still selected an unrelated compaction snapshot.

This change:

  • gives Node tests and hooks a 60-second budget on Windows while retaining 30 seconds elsewhere, covering both default and in-process transports;
  • gives the Rust processing/context metadata test a dedicated snapshot matching its RUST_CONTEXT_INFO prompt, while preserving the shared compaction fixture used by Python.

The branch was rebased onto #2251. The earlier .NET target-framework split was removed: measured Windows critical-path time changed only from 21.9 to 21.8 minutes while doubling the Windows job count. The class-level process sharding merged in #2251 provides the meaningful latency reduction and continues to run both target frameworks in each shard.

Validation included workflow and TypeScript formatting, Node type checking, Rust formatting, Rust E2E test-target compilation, and commit whitespace checks.

Copilot AI balanced review requested due to automatic review settings July 31, 2026 20:00
@roji
roji requested a review from a team as a code owner July 31, 2026 20:00
@roji
roji force-pushed the roji-fix-pure-ci-flakes branch from 4e5f6d6 to ba7bf47 Compare July 31, 2026 20:02
@github-actions

This comment has been minimized.

@roji
roji enabled auto-merge July 31, 2026 20:09
@roji
roji force-pushed the roji-fix-pure-ci-flakes branch from ba7bf47 to daa55a7 Compare July 31, 2026 20:10
@github-actions

This comment has been minimized.

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.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Reduces CI flakiness across Node/.NET/Rust E2E coverage by adjusting timeouts, isolating Windows .NET target frameworks into separate jobs, and aligning a Rust E2E snapshot with its actual prompt/expectations.

Changes:

  • Adjust Node Vitest integration timeouts to allow longer execution on Windows while keeping other OS budgets unchanged.
  • Split .NET net8.0 and net472 runs into distinct Windows matrix jobs via a framework dimension and pass --framework to dotnet test.
  • Add a dedicated Rust snapshot and rename the Rust E2E test to match the new snapshot/intent.
Show a summary per file
File Description
nodejs/vitest.config.ts Introduces OS-specific test/hook timeouts to reduce Windows E2E flakiness.
.github/workflows/dotnet-sdk-tests.yml Adds a framework matrix dimension and runs dotnet test with explicit framework selection to avoid Windows runner contention.
rust/tests/e2e/rpc_session_state.rs Renames the Rust E2E test to match the new processing/context metadata behavior/snapshot.
test/snapshots/rpc_session_state/should_report_processing_and_context_metadata.yaml Adds a dedicated snapshot aligned with the Rust context metadata test prompt/expected output.

Review details

  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings July 31, 2026 20:17

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.

🟢 Ready to approve

The focused changes consistently address the three documented CI flakes without introducing unresolved issues.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@roji
roji added this pull request to the merge queue Aug 1, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 1, 2026
Copilot AI review requested due to automatic review settings August 4, 2026 14:14
@roji
roji force-pushed the roji-fix-pure-ci-flakes branch from daa55a7 to 824defe Compare August 4, 2026 14:14
@github-actions

This comment has been minimized.

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.

🟡 Not ready to approve

The Rust test still relies on observing a transient processing state that the immediate replay response can complete before polling begins.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

rust/tests/e2e/rpc_session_state.rs:1105

  • The dedicated fixture now completes a one-token response with no replay delay, but this test polls for a transient processing == true state only after send() returns. The replay proxy emits all chunks synchronously when slowStreaming is false, while wait_for_condition polls every 100 ms, so the turn can already be idle before the first poll and the test remains timing-dependent. Please make the in-flight state deterministic (for example, hold the turn open with a controlled tool/delayed stream) or avoid asserting that the transient state must be observed.
        "should_report_processing_and_context_metadata",
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

roji added 3 commits August 4, 2026 18:28
Use a 60-second Vitest test and hook budget on Windows runners while preserving the existing 30-second budget elsewhere. This covers both default and in-process transports without weakening individual assertions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 51fbb14d-022f-4acb-bdce-1753cba236f9
Run net8.0 and net472 in independent Windows matrix jobs instead of concurrently in one testhost job. This isolates each CLI and replay-proxy fixture graph without increasing the workflow's critical-path test duration.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 51fbb14d-022f-4acb-bdce-1753cba236f9
Rename the repurposed metadata test and route it to a snapshot that matches its RUST_CONTEXT_INFO prompt. Preserve the existing compaction snapshot used by Python instead of sharing an incompatible conversation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 51fbb14d-022f-4acb-bdce-1753cba236f9
Copilot AI review requested due to automatic review settings August 4, 2026 16:29
@roji
roji force-pushed the roji-fix-pure-ci-flakes branch from 824defe to 7b575e9 Compare August 4, 2026 16:29

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.

🟢 Ready to approve

The changes consistently address the identified CI flakes without introducing unresolved issues.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@github-actions

This comment has been minimized.

The framework split reduced the measured Windows critical path by only 0.1 minutes while doubling the number of Windows jobs. Preserve the class-level process sharding that reduced the critical path by roughly seven minutes instead.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 4, 2026 16:33
@SteveSandersonMS SteveSandersonMS changed the title Fix active Node and replay E2E flakes Fix active Node and Rust replay E2E flakes Aug 4, 2026

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.

🟢 Ready to approve

The focused changes correctly address both reported flakes without introducing unresolved issues.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review ✅

This PR makes no changes to SDK public APIs or client behavior. The changes are purely CI/test infrastructure:

  • Node.js (vitest.config.ts): Increases test timeout to 60s on Windows (30s elsewhere) — test config only
  • Rust (rpc_session_state.rs + snapshot YAML): Renames a test and adds a dedicated snapshot fixture
  • .NET (workflow YAML): Updates a CI comment

No cross-SDK consistency concerns. The changes are correctly scoped to the platforms where the flakes were observed.

Generated by SDK Consistency Review Agent for #2186 · sonnet46 8.74 AIC · ⌖ 5.34 AIC · ⊞ 6.6K ·

@SteveSandersonMS
SteveSandersonMS merged commit 70bb9a1 into main Aug 4, 2026
54 of 55 checks passed
@SteveSandersonMS
SteveSandersonMS deleted the roji-fix-pure-ci-flakes branch August 4, 2026 17:08
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.

4 participants