Skip to content

Fix .NET in-process E2E transport coverage#1986

Open
roji wants to merge 2 commits into
mainfrom
roji-fix-inprocess-e2e-transport
Open

Fix .NET in-process E2E transport coverage#1986
roji wants to merge 2 commits into
mainfrom
roji-fix-inprocess-e2e-transport

Conversation

@roji

@roji roji commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

The .NET E2E fixture explicitly selected TCP even in the in-process matrix, so those tests never exercised the FFI transport and hid FFI-specific behavior.

This change:

  • Allows resuming the same session in .NET SDK, aligning it with the other SDK's behavior (.NET specifically had a check against doing that). This was a blocker for some .NET E2E tests in inproc mode.
  • selects RuntimeConnection.ForInProcess() for the in-process matrix while preserving TCP coverage in the other matrix
  • restores the native host environment before each test because the shared fixture outlives per-test environment isolation
  • aligns same-client session resume with the other SDKs by replacing the routed session wrapper before the resume RPC, without adding operation-wide locking or concurrent-resume rejection
  • keeps TCP-only multi-client behavior intact and adds fixture policy and session lifetime coverage
  • uses a checkpoint number valid for the native runtime's u32 domain

The complete .NET in-process suite passes through FFI with 672 tests passed and 3 existing skips. Targeted TCP resume coverage and all .NET target-framework builds also pass.

Make the in-process E2E matrix use the FFI runtime, restore the native host environment between tests, and align same-client session resume with the other SDKs.

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

Copilot-Session: c71188a1-1445-46aa-9faf-3b73cf6a6dd9
Copilot AI review requested due to automatic review settings July 14, 2026 16:44
@roji roji requested a review from a team as a code owner July 14, 2026 16:44
@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

Updates .NET E2E coverage to exercise the FFI transport and supports same-client session resumption.

Changes:

  • Selects in-process or TCP transport according to the test matrix.
  • Restores host environment state between in-process tests.
  • Adds same-client resume replacement behavior and related coverage.
Show a summary per file
File Description
dotnet/src/Client.cs Replaces tracked wrappers during resume.
dotnet/test/Harness/E2ETestFixture.cs Selects matrix-appropriate transport.
dotnet/test/Harness/E2ETestContext.cs Restores in-process environment state.
dotnet/test/Harness/E2ETestBase.cs Prepares tests and chooses resume client.
dotnet/test/E2E/SessionE2ETests.cs Covers same-client resume replacement.
dotnet/test/E2E/RpcWorkspaceCheckpointsE2ETests.cs Uses a native-compatible checkpoint value.
dotnet/test/Unit/E2ETestFixtureTests.cs Tests fixture transport selection.
dotnet/test/Unit/ClientSessionLifetimeTests.cs Tests replacement and failure lifetimes.

Review details

  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread dotnet/src/Client.cs Outdated
Capture the wrapper displaced before session.resume and restore it only if the failed replacement is still registered.

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

Copilot-Session: c71188a1-1445-46aa-9faf-3b73cf6a6dd9
@github-actions

Copy link
Copy Markdown
Contributor

✅ Cross-SDK Consistency Review

This PR touches only the .NET SDK (dotnet/). The key behavioral change — allowing ResumeSessionAsync to replace an already-tracked session instead of throwing — is consistent with how every other SDK handles this:

SDK Behavior when resuming a session the same client already tracks
Node.js this.sessions.set(sessionId, session) — silently replaces (client.ts:1714)
Python self._sessions[session_id] = session — silently replaces (client.py:3099)
Go c.sessions[sessionID] = session — silently replaces (client.go:1265)
Java sessions.put(sessionId, session) — silently replaces (CopilotClient.java:747)
.NET (before) Threw InvalidOperationException — was the only outlier
.NET (after PR) Replaces, restores previous on failure — now consistent ✅

No other SDKs need updates. The PR brings .NET into alignment with the established cross-SDK contract.

Generated by SDK Consistency Review Agent for issue #1986 · sonnet46 804.7K ·

@roji roji enabled auto-merge July 14, 2026 17:13
@roji

roji commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator Author

@SteveSandersonMS noticed this while working on other stuff, take a look - we weren't doing actual FFI in the .NET SDK, plus the .NET SDK was unique in not allowing the same session to be resumed with a new session object (which obstructed fixing the former).

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.

2 participants