Repository navigation
Honor explicit abandonment in the in-process test host - #814
Open
wangbill (YunchuWang) wants to merge 2 commits into
Open
wangbill (YunchuWang) wants to merge 2 commits into
wangbill (YunchuWang) wants to merge 2 commits into
Conversation
Assign a fresh completion token to each activity and orchestration delivery. Coordinate completion, partial responses, explicit abandonment, and cleanup using that delivery's ownership so stale requests cannot settle a replacement. Cancel explicitly abandoned executions so the existing dispatcher requeues them, while preserving the stream-disconnect policy. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4a00a0f1-0b30-44f9-83a2-27dad922625f
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Comment on lines
+643
to
+655
| if (request.FailureDetails == null) | ||
| { | ||
| resultEvent = new TaskCompletedEvent(-1, request.TaskId, request.Result); | ||
| } | ||
| else | ||
| { | ||
| resultEvent = new TaskFailedEvent( | ||
| eventId: -1, | ||
| taskScheduledId: request.TaskId, | ||
| reason: null, | ||
| details: null, | ||
| failureDetails: ProtobufUtils.GetFailureDetails(request.FailureDetails)); | ||
| } |
| using CancellationTokenSource connectionCancellation = new(); | ||
| using AsyncServerStreamingCall<P.WorkItem> rejectingWorker = client.GetWorkItems( | ||
| new(), cancellationToken: connectionCancellation.Token); | ||
| IHost? compatibleWorker = null; |
| sealed class ServerSession : IAsyncDisposable | ||
| { | ||
| readonly CancellationTokenSource stopping = new(); | ||
| readonly CancellationTokenSource connectionCancellation = new(); |
|
|
||
| sealed class ServerSession : IAsyncDisposable | ||
| { | ||
| readonly CancellationTokenSource stopping = new(); |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A delayed stream-write failure can override an already-acknowledged completion and incorrectly requeue completed work.
1 open finding
What changed in this PR
Adds receiver-side support for the explicit abandonment introduced by #467, using per-delivery completion tokens to prevent stale settlements.
Changes:
- Cancels and requeues explicitly abandoned work.
- Coordinates completion, partial responses, snapshots, and abandonment by delivery token.
- Adds direct-server and localhost integration coverage plus documentation.
| File | Description |
|---|---|
src/InProcessTestHost/Sidecar/Grpc/TaskHubGrpcServer.cs |
Implements token-based delivery ownership and abandonment. |
src/InProcessTestHost/README.md |
Documents abandonment semantics and migration requirements. |
test/InProcessTestHost.Tests/WorkItemAbandonmentTests.cs |
Tests settlement, concurrency, and cleanup behavior. |
test/InProcessTestHost.Tests/WorkItemAbandonmentIntegrationTests.cs |
Tests end-to-end redelivery through gRPC. |
test/InProcessTestHost.Tests/WorkerHistorySnapshotTests.cs |
Updates snapshot tests for completion tokens. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Let send-failure cleanup claim delivery ownership under the same lock as completion and abandonment. When a successful completion already won, return its accepted result for both activity and orchestrator execution instead of requeueing it. Add deterministic completion-before-send-failure coverage for both dispatch paths and document the settlement policy. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5777cc4b-513e-47b7-a159-75ad5e8a7298
4 of 15 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What changed?
Why is this change needed?
The worker already sends explicit abandonment requests when it rejects or cannot process work. The test-host receiver acknowledged those requests without settling the pending execution, leaving the dispatcher waiting indefinitely instead of retrying the work.
Issues / work items
Project checklist
release_notes.mdcompletionTokenin completion, partial-response, and abandonment requests; use a replacement delivery's new token rather than an old token.AI-assisted code disclosure (required)
Was an AI tool used? (select one)
If AI was used:
AI verification (required if AI was used):
Testing
Automated tests
dotnet test test\InProcessTestHost.Tests\InProcessTestHost.Tests.csproj --configuration Release --no-build --no-restore --filter "FullyQualifiedName~WorkItemAbandonment|FullyQualifiedName~WorkerHistorySnapshotTests" --verbosity quietdotnet test test\InProcessTestHost.Tests\InProcessTestHost.Tests.csproj --configuration Release --no-restore --verbosity quietdotnet build src\InProcessTestHost\InProcessTestHost.csproj --configuration Release --no-restore --verbosity quietnet6.0,net8.0, andnet10.0; zero errors.dotnet pack src\InProcessTestHost\InProcessTestHost.csproj --configuration Release --no-build --no-restore --verbosity quietgit diff --check origin/main...HEADThe build has existing analyzer warnings. Hosted CI has not run; full-solution, emulator, and real Azure suites were not run for this test-host-only change.
Manual validation (only if runtime/behavior changed)
Notes for reviewers
Breaking Change
Type: behavioral (direct gRPC request acceptance and explicit-abandonment timing).
Impact: SDK signatures, protobuf wire fields, persisted JSON, and orchestrator replay code are unchanged, and current SDK workers already echo the delivered token. Direct gRPC callers that previously completed work using only instance/task identity must now supply a nonempty delivery token. Missing tokens return
InvalidArgument; unknown or already settled tokens returnNotFound; mismatched completion identity returnsInvalidArgumentwithout consuming the active delivery. A successful explicit abandonment now settles that execution and enables dispatcher retry instead of leaving it pending after acknowledgment.Migration: Echo the
WorkItem.completionTokenin activity completion, orchestrator partial/final completion, and abandonment requests. A replacement delivery has a new token; do not reuse a settled delivery's token to target the replacement.GetWorkItemsdoes not cancel or redispatch already delivered work. No activity timeout, heartbeat/lease policy, production DTS change, worker-policy change, entity support, or hub-lifecycle change is included.