Skip to content

feat: [4/6] add payload purge protocol and client lifecycle - #391

Open
wangbill (YunchuWang) wants to merge 1 commit into
yunchuwang-payload-phase-3-blobfrom
yunchuwang-payload-phase-4-client
Open

wangbill (YunchuWang) wants to merge 1 commit into
yunchuwang-payload-phase-3-blobfrom
yunchuwang-payload-phase-4-client

Conversation

@YunchuWang

@YunchuWang wangbill (YunchuWang) commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Summary

What changed?

  • Add the exact pinned DTS LargePayloadPurge protocol and four generated JS/TypeScript bindings, with reproducible codegen integration.
  • Add PascalCase-compatible DTOs/constants and authenticated purge calls on the effective existing sidecar channel, without importing or registering any future worker runner.
  • Implement setting-first enable/disable, unversioned deduplicated fixed runner identity/status verification and SetBatchSize signaling.
  • Correct single-instance purge routing independently of payload configuration, matching .NET management clients. Cover ordinary and entity IDs with payloads both disabled and enabled.

Why is this change needed?

  • Phase 4/6 of the requested large-payload implementation split. This PR's base is phase 3's branch, so its diff excludes previously reviewed phases.
  • Behavioral reference: durabletask-dotnet 91739687. The complete stack preserves the original implementation at c9b53fcb3ab8585c4b3b22f9c0500cc9dd122c5a, with the documented instance-kind routing correction.

Issues / work items

  • Resolves: N/A (user-requested phased feature review).
  • Related: original oversized PR #387. It is preserved while the coordinator verifies this replacement stack.

Project checklist

  • Release notes are not required for the next release
    • Otherwise: Notes added to CHANGELOG.md
  • Backport is not required
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • All required tests have been added/updated (unit tests, E2E tests)
  • Breaking change?
    • If yes:
      • Impact: Explicit ordinary instance IDs now set isOrchestration=true even when the management client has no payload store. Entity IDs remain false; timeouts/completion results are preserved.
      • Migration guidance: No blob credential is needed to manage the setting or route a purge. A compatible .NET worker or phase 5 JavaScript worker is needed for the cleanup job. Do not release partial phases.

AI-assisted code disclosure (required)

Was an AI tool used? (select one)

  • No
  • Yes, AI helped write parts of this PR (e.g., GitHub Copilot)
  • Yes, an AI agent generated most of this PR

If AI was used:

  • Tool(s): GitHub Copilot.
  • AI-assisted areas/files: This phase's source, regression tests, and directly related metadata/documentation.
  • What you changed after AI output: Split the implementation along independently buildable boundaries; retained focused regression evidence and removed unrelated formatting-only changes. Human review is still required.

AI verification (required if AI was used):

  • I understand the code and can explain it
  • I verified referenced APIs/types exist and are correct
  • I reviewed edge cases/failure paths (timeouts, retries, cancellation, exceptions)
  • I reviewed concurrency/async behavior
  • I checked for unintended breaking or behavior changes

These human attestations are intentionally left unchecked.


Testing

Automated tests

  • Result: Passed locally for this phase, without later-phase files.
  • npm ci --offline --no-audit --no-fund — passed after establishing the missing test runner.
  • npm run build and npm run lint — passed without the later runner or worker registration.
  • npm run test:unit -w @microsoft/durabletask-js -- --runTestsByPath test\payload-purge-client.spec.ts test\client-purge.spec.ts test\payload-transport.spec.ts test\client-wait-grpc.spec.ts — passed 53 tests in 4 suites.
  • The new no-payload ordinary-ID regression failed on the original conditional flag, then passed after the unconditional instance-kind correction. All 27 purge tests pass, including four enabled/disabled kind cases and preserved timeout/completion behavior.
  • Generated JS and pinned .NET protobuf descriptors compared byte-for-byte and matched SHA256 5DDA0563E9A4F61340C6A975AF8CB5994863766674F24C0B561AB037C3335569.
  • bash.exe -n tools\generate-grpc-javascript.sh — passed.
  • git diff --check yunchuwang-payload-phase-3-blob...HEAD — passed.
  • Hosted CI: pending/not inferred. Existing main-base filters mean some workflows will not run for stacked PRs; no workflow triggers were broadened.

Manual validation (only if runtime/behavior changed)

  • Environment (OS, Node.js version, components): Windows, Node.js v24.14.0; loopback gRPC servers with authenticated metadata assertions; pinned .NET descriptor comparison.
  • Steps + observed results:
    1. Materialized this phase on its declared predecessor, with no future-phase imports.
    2. Ran the exact owning checks above and inspected the incremental patch.
    3. Kept the original feature branch/history and primary user checkout unchanged.
  • Evidence (optional): Full local logs are session artifacts, not repository changes.

Notes for reviewers

Merge order Phase PR Base branch
1 Core contracts #388 main
2 SDK integration #389 yunchuwang-payload-phase-1-core
3 Azure Blob provider #390 yunchuwang-payload-phase-2-sdk
4 Purge protocol/client #391 yunchuwang-payload-phase-3-blob
5 Purge worker #392 yunchuwang-payload-phase-4-client
6 Release/usage/consumers #393 yunchuwang-payload-phase-5-worker

Merge 1 → 2 → 3 → 4 → 5 → 6. Each PR targets its predecessor; retarget the next PR only after its prerequisite merges. Do not publish packages or describe a partial phase as the complete feature.

  • Generated-code noise is isolated here: the four generated binding files account for 1,684 added lines. Start review with the 56-line .proto, the DTOs and client lifecycle; bindings were generated, not manually edited.
  • No worker runner is imported or registered. The management API can use an existing compatible .NET worker; the JavaScript runner follows in phase 5.
  • The routing correction is supported by the .NET transport and proto field contract; no runtime data-loss claim is made.
  • Full live DTS/mixed-.NET execution has not been run locally (Docker daemon unavailable). The final phase records aggregate local Blob/Azurite, source/wire interoperability and packed-consumer evidence without claiming absent hosted checks.

Phase 4 of 6: add the pinned DTS protocol and generated bindings, DTOs and authenticated setting/start/verify/configure lifecycle without importing the future worker runner. Determine single-instance purge kind independently of payload storage, with enabled/disabled orchestration/entity regressions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 81e20512-d3a2-4f8e-a574-f396140ec9c7
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.

1 participant