Await enclave attestation in asynchronous command execution by cheenamalhotra · Pull Request #4887 · dotnet/SqlClient

cheenamalhotra · 2026-10-21T06:15:05Z

Description

Integrate the async enclave delegate APIs from stack parent #4886 into command execution.

Functionality: await session lookup, attestation parameters, session creation, and retry invalidation across Reader, NonQuery, Scalar, XML, APM, and batch paths.
API changes/backward compatibility: no public API changes; synchronous provider dispatch is preserved.
Documentation/localization: document the supported async scope and remaining key-store work.
Bug fixes: propagate caller cancellation; release setup registrations and retry state before completing failed operations.
Code hygiene/refactoring: share describe-encryption parsing between sync and async paths.
Engineering/test coverage: add 45 deterministic regression cases and SQL-backed cold/warm-session API coverage.

Key-store operations and enclave-package generation remain synchronous here; their async implementation stays in #4585.

Issues

Part of #3672. Stacked on #4886; merge that PR first, then retarget this PR to main.

Testing

Passed 123 focused unit tests (45 new command cases, 58 inherited enclave cases, 20 existing binding/batch cases):

dotnet test src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj -f net10.0 --filter 'FullyQualifiedName~SqlCommandEnclaveAsyncTests|FullyQualifiedName~EnclaveDelegateAsyncTests|FullyQualifiedName~SqlColumnEncryptionEnclaveProviderAsyncShould|FullyQualifiedName~EnclaveSessionCacheTest|FullyQualifiedName~SqlCommandOptimizedBindingTests|FullyQualifiedName~SqlCommandSetTest' --blame-hang-timeout 2m --nologo -v:q

Passed 30 existing command functional tests:

dotnet test src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.FunctionalTests.csproj -f net10.0 --filter 'FullyQualifiedName~SqlCommandTest' --blame-hang-timeout 2m --nologo -v:q

Both builds passed with 0 warnings/errors:

dotnet build src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj -f net462 --nologo -v:q
dotnet build src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj -f net10.0 -c Release --nologo -v:q

git diff --check passed. Server-backed tests were not run: no reachable enclave-enabled SQL Server was available. Discovery listed the new test but emitted connection-refused errors from other server-dependent theories:

dotnet test src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj -f net10.0 -c Release --no-build --list-tests --filter 'FullyQualifiedName~TestEnclaveExecutionApis' --nologo

Tests added or updated
Public API changes documented (none; async behavior documented)
Verified against customer repro (requires enclave-enabled SQL Server)
Ensure no breaking changes introduced

Guidelines

Please review the contribution guidelines before submitting a pull request:

Copilot

🟡 Changes recommended

Describe processing introduces a connection-ownership gap and duplicate async-count cleanup after write failures.

2 open issues

What changed in this PR

Integrates asynchronous enclave attestation into command execution using the delegate APIs from #4886. Key-store and enclave-package work remains separate in #4585.

Changes:

Awaits enclave operations and forwards caller cancellation across shared execution paths.
Updates cleanup and shares describe-encryption parsing.
Adds deterministic regression tests, SQL-backed coverage, and scope documentation.

File	Description
`src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/SqlCommandEnclaveAsyncTests.cs`	Tests provider dispatch, cancellation, cleanup, and retries.
`src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs`	Adds cold/warm-session execution coverage.
`src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Xml.cs`	Threads cancellation through XML execution.
`src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs`	Integrates asynchronous preparation and retries.
`src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.NonQuery.cs`	Updates cancellation forwarding and setup cleanup.
`src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Encryption.cs`	Awaits enclave operations and shares metadata parsing.
`src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs`	Guards pending preparation and awaits retry invalidation.
`doc/always-encrypted-async.md`	Documents supported asynchronous scope and exclusions.

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

The no-preparation async path introduces a connection-sharing race, and enclave-server validation remains outstanding.

1 open issue

Prevent overlapping commands after parameterless async reader preparation · New

2 resolved since last review

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

Parameterless enclave-enabled batches can queue execution without reserving the connection, allowing conflicting non-MARS operations.

1 open issue

Parameterless enclave batches defer setup and race with competing readers · New

1 resolved since last review

Prevent overlapping commands after parameterless async reader preparation

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

Core command-lifecycle and cancellation changes need final human review, and enclave-enabled SQL Server validation remains unrun.

No open issues 🎉

1 resolved since last review

Parameterless enclave batches defer setup and race with competing readers

_{Review effort: Balanced}

priyankatiwari08

One concern on the new async attestation window.

Copilot

🔵 Needs a closer look

Core command-lifecycle and cancellation changes need final human review, and enclave-enabled SQL Server validation remains outstanding.

No open issues 🎉

_{Review effort: Balanced}

Integrate asynchronous enclave session preparation, creation, and invalidation with command cancellation and retry cleanup. Preserve synchronous behavior and shared metadata parsing; add deterministic and SQL-backed regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47a925b3-e0e6-43ed-9fb7-519060a206c2

Give asynchronous describe processing sole ownership of its captured connection reservation. Preserve original write faults and cancellation after connection closure, retain describe mode through reconnect, and test delayed reader registration and setup failures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47a925b3-e0e6-43ed-9fb7-519060a206c2

Preserve null-task completion for parameterless Always Encrypted commands without an enclave workflow. Isolate simulated enclave fixtures by application name so cached attestation options cannot leak across reused server endpoints. Add deterministic regressions for both paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47a925b3-e0e6-43ed-9fb7-519060a206c2

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47a925b3-e0e6-43ed-9fb7-519060a206c2

paulmedynski

Needs compile errors fixed.

Fix Parser namespace imports after the stack rebase and cover provider faults and cancellation after successful session invalidation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47a925b3-e0e6-43ed-9fb7-519060a206c2

cheenamalhotra · 2026-10-26T19:36:37Z

Needs compile errors fixed.

Fixed in 7567986 by adding the Parser namespace import for RunBehavior and _SqlRPC. The net462 UnitTests build and 148 focused net10.0 tests in both Debug and Release pass locally; current-head CI is queued/running.

Copilot

🔵 Needs a closer look

Sensitive retry and connection-ownership changes need final human review and enclave-server validation, which has not been completed.

No open issues 🎉

1 resolved since last review

Release retry connection reservation when re-attestation is canceled

_{Review effort: Balanced}

codecov · 2026-10-26T20:52:23Z

Codecov Report

❌ Patch coverage is 79.05405% with 62 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.07%. Comparing base (7b1184b) to head (168a580).
⚠️ Report is 74 commits behind head on main.

Files with missing lines	Patch %	Lines
.../Microsoft/Data/SqlClient/SqlCommand.Encryption.cs	78.20%	34 Missing ⚠️
.../src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs	66.66%	22 Missing ⚠️
...lClient/src/Microsoft/Data/SqlClient/SqlCommand.cs	89.18%	4 Missing ⚠️
...rc/Microsoft/Data/SqlClient/SqlCommand.NonQuery.cs	95.00%	1 Missing ⚠️
...ent/src/Microsoft/Data/SqlClient/SqlCommand.Xml.cs	94.11%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4887      +/-   ##
==========================================
+ Coverage   67.94%   68.07%   +0.13%     
==========================================
  Files         311      307       -4     
  Lines       45399    69079   +23680     
==========================================
+ Hits        30845    47024   +16179     
- Misses      14554    22055    +7501

Flag	Coverage Δ
CI-SqlClient	`?`
PR-SqlClient-Project	`68.07% <79.05%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

priyankatiwari08

Minor concerns:

No test covers the new async retry path in RunExecuteReader (inline).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47a925b3-e0e6-43ed-9fb7-519060a206c2

Copilot

🔵 Needs a closer look

Async attestation and retry changes alter connection ownership and cleanup, but enclave-enabled SQL Server tests for cold/warm sessions, cancellation, retry, and connection reuse remain unrun, with current-head build/test checks still pending.

No open issues 🎉

_{Review effort: Balanced}

Gate the simulated response until overlap assertions finish and return metadata-free DONE for the fixture's negotiated encryption capabilities. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47a925b3-e0e6-43ed-9fb7-519060a206c2

Copilot

🔵 Needs a closer look

Async enclave changes affect connection ownership and cleanup, but end-to-end validation of retries, cancellation, and connection reuse against an enclave-enabled SQL Server remains unrun.

No open issues 🎉

_{Review effort: Balanced}

priyankatiwari08

Looks good - no concerns.

Compile errors have been resolved.

cheenamalhotra requested a review from a team as a code owner October 21, 2026 06:15

Copilot AI balanced review requested due to automatic review settings October 21, 2026 06:15

github-project-automation Bot added this to SqlClient Board Oct 21, 2026

github-project-automation Bot moved this to To triage in SqlClient Board Oct 21, 2026

Copilot started reviewing on behalf of cheenamalhotra October 21, 2026 06:15

View session

cheenamalhotra mentioned this pull request Oct 21, 2026

Feature | Async friendly Always Encrypted support #3672

Open

7 of 11 tasks

cheenamalhotra added this pull request to stack #4888 October 21, 2026 06:17

cheenamalhotra added this to the 8.0.0-preview1 milestone Oct 21, 2026

github-actions Bot assigned paulmedynski and priyankatiwari08 Oct 21, 2026

Copilot AI reviewed Oct 21, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Encryption.cs Resolved

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Encryption.cs Resolved

Copilot AI balanced review requested due to automatic review settings October 21, 2026 06:34

Copilot started reviewing on behalf of cheenamalhotra October 21, 2026 06:35

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Encryption.cs Resolved

Copilot AI balanced review requested due to automatic review settings October 21, 2026 17:20

Copilot started reviewing on behalf of cheenamalhotra October 21, 2026 17:20

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Encryption.cs Resolved

Copilot AI balanced review requested due to automatic review settings October 21, 2026 17:36

Copilot started reviewing on behalf of cheenamalhotra October 21, 2026 17:36

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

priyankatiwari08 reviewed Oct 22, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Encryption.cs Resolved

Copilot AI balanced review requested due to automatic review settings October 22, 2026 06:32

Copilot started reviewing on behalf of cheenamalhotra October 22, 2026 06:32

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

cheenamalhotra mentioned this pull request Oct 22, 2026

Use the async Always Encrypted key store APIs on async execution paths #4585

Open

Base automatically changed from dev/cheena/automation/async-enclave-delegate to main October 26, 2026 07:34

cheenamalhotra and others added 4 commits October 26, 2026 13:04

Keep completed enclave batch preparation inline

f847b92

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47a925b3-e0e6-43ed-9fb7-519060a206c2

cheenamalhotra modified the milestones: 8.0.0-preview1, 8.1.0-preview1 Oct 26, 2026

paulmedynski previously requested changes Oct 26, 2026

View reviewed changes

github-project-automation Bot moved this from To triage to Waiting for customer in SqlClient Board Oct 26, 2026

Copilot AI balanced review requested due to automatic review settings October 26, 2026 19:35

Copilot started reviewing on behalf of cheenamalhotra October 26, 2026 19:36

View session

Copilot AI reviewed Oct 26, 2026

View reviewed changes

priyankatiwari08 reviewed Nov 3, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs Resolved

Test inline asynchronous enclave retry cleanup

1b206b8

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47a925b3-e0e6-43ed-9fb7-519060a206c2

Copilot AI balanced review requested due to automatic review settings November 3, 2026 17:27

Copilot started reviewing on behalf of cheenamalhotra November 3, 2026 17:27

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings November 3, 2026 19:17

Copilot started reviewing on behalf of cheenamalhotra November 3, 2026 19:18

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

priyankatiwari08 approved these changes Nov 4, 2026

View reviewed changes

cheenamalhotra requested a review from paulmedynski November 13, 2026 08:37

cheenamalhotra moved this from Waiting for customer to In review in SqlClient Board Nov 13, 2026

cheenamalhotra added the Hotfix 8.0.0 PRs that must be backported to release/8.0 label Nov 13, 2026

cheenamalhotra modified the milestones: 8.1.0-preview1, 8.0.0 Nov 13, 2026

Conversation

cheenamalhotra commented Oct 21, 2026

Description

Issues

Testing

Guidelines

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

priyankatiwari08 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

paulmedynski left a comment

Choose a reason for hiding this comment

Uh oh!

cheenamalhotra commented Oct 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

codecov Bot commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

priyankatiwari08 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

priyankatiwari08 left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Oct 26, 2026 •

edited

Loading