Skip to content

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

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

wangbill (YunchuWang) wants to merge 7 commits 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. Record an existing-runner outcome only in _call's actual-RPC callback, not from arbitrary storage/metadata codes or error classes.
  • 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 stack also inherits the reviewed JavaScript carryover and whole-message size-preflight repairs described below; it does not claim unchanged .NET failure-priority semantics.

Issues / work items

  • Resolves: N/A (user-requested phased feature review).
  • Related: original oversized PR #387. It is closed without merging; its original branch/history remain preserved.

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

  • Current review head: 7c9d67f97c961c148a5fdd4809e7443c268d7162.
  • npm run test:unit -w @microsoft/durabletask-js -- --runTestsByPath test\payload-purge-client.spec.ts test\client-purge.spec.ts — 45 tests / 2 suites passed on exact 7c9d67f97c961c148a5fdd4809e7443c268d7162; npm run build:core and npm run lint passed.
  • Runner TDD reproduced two code6 upload/metadata failures with six passing controls before its source edit. Eight new fast cases also preserve same-class OrchestrationAlreadyExistsError failures, genuine duplicate verification/configuration once, unrelated RPC errors and cancellation.
  • This phase inherits the approved shared _call correction. Its extra lifecycle fix is only 9 added / 1 removed source lines; no additional pipeline, helper or error-wrapper class was introduced.
  • Owning receipts: runner-boundary-red.log, runner-boundary-green.log, runner-boundary-build-lint.log, runner-boundary-format-diff.log; scoped formatting and commit-hook ESLint passed.
  • git diff --check origin/yunchuwang-payload-phase-3-blob...HEAD — passed. Prior heads remain ancestors; predecessor bases, phase-local review scopes, release/cleanup/version notes and ready-for-review state are preserved.
  • Combined final on exact ae63cab8ecb6ec6b2b3e0e15067091a5b8a6b24e: npm run test:unit — 3,180 tests / 133 suites passed (core 2,028; Functions 226; Blob 39; Azure-managed 821; export-history 66). Ten opt-in service tests are skipped by this unit command.
  • On the same final head, npm run build passed all five workspaces and npm run lint passed. This is the true final aggregate containing both approved commits; the earlier stopped pre-lifecycle run is not a final pass.
  • Final structural/blob audit passed: one unary transport site and one upload/resolve site in _call; all five semantic callers delegate; phase4–6 client/lifecycle fixtures match the approved runner fix, and PR1 processor plus existing public-loopback fixtures are unchanged.
  • Final DTS live run: 39837123528 on exact ae63cab8ecb6ec6b2b3e0e15067091a5b8a6b24e — PENDING (in_progress) at 2026-10-22T19:07:38.5343134Z. The link is authoritative for later completion; no early pass is claimed.
  • PR388 source/body remain unchanged at 137bb547e8781fd84c2738c3e13ae0f65cd66174; its separately verified hosted evidence is 30 SUCCESS / 1 SKIPPED. Review approval is still required. Main-base-only PR filters are unchanged; absent stacked-PR jobs are not passes.
  • Scope limits remain: no live storage-outage or mixed .NET/JavaScript execution, no transactional rollback or generic orphan collector. Tombstone cleanup cannot discover blobs without a backend-recorded reference.

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

Shared unary boundary correction

  • f3ea69fc79eb594961ab4d57651f5e1360fb9127 restores one private _call pipeline: combine cancellation once; externalize once; generate fresh metadata outside each RPC catch; invoke only the operation's RPC policy; hydrate once after success. true retries the RPC, false rethrows unchanged, and throwing maps a genuine RPC error with its cause. The no-metadata path preserves its original dispatch timing.
  • Scheduling and completion waits no longer inline separate pipelines; rewind/restart mappings use the same RPC-only boundary. Errors are not mutated or tagged and no public API, helper signature, dependency or middleware framework was added.
  • 7c9d67f97c961c148a5fdd4809e7443c268d7162 handles runner dedupe through a per-invocation BUSINESS runnerAlreadyExists outcome set only inside that RPC policy. It is not an error tag or global marker; matching storage/metadata codes or semantic classes remain original failures, and cancellation takes precedence.
  • All earlier reviewed carryover, UTF-8 preflight, token/cancellation, rewind mapping and worker-release repairs remain ancestors. No PR merge, package publication or history rewrite was performed.
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 <[email protected]>
Copilot-Session: 81e20512-d3a2-4f8e-a574-f396140ec9c7
wangbill (YunchuWang) and others added 6 commits October 19, 2026 12:52
Retain both the management-client routing release note and all inherited carryover/preflight notes while propagating the reviewed core and provider documentation repairs.

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: 81e20512-d3a2-4f8e-a574-f396140ec9c7
Preserve lifecycle and instance-kind routing while inheriting exact reviewed work-item release and completion-wait deadline isolation. Retain both feature and repair changelog notes.

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: 81e20512-d3a2-4f8e-a574-f396140ec9c7
Preserve the existing phase head and all approved SDK repairs while inheriting aeb598f recognized-token skipping and bee1313 aborted-signal precedence. No unrelated behavior or release metadata changes.

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: 81e20512-d3a2-4f8e-a574-f396140ec9c7
Inherit only the approved rewind mapping and scheduling RPC error-domain repairs, preserving phase-specific lifecycle/routing/cleanup behavior and both directly related notes. Exact approved source/test blobs and stable client patch identity verified.

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: 81e20512-d3a2-4f8e-a574-f396140ec9c7
Inherit the exact approved central _call error-domain correction, owning regressions and directly related documentation. Preserve phase-specific lifecycle/routing/cleanup behavior and prior notes/ancestry; source/test patch identity verified without redundant per-hop suites.

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: 81e20512-d3a2-4f8e-a574-f396140ec9c7
Record a per-invocation duplicate-runner outcome only in the shared RPC-only error callback. Never infer dedupe from upload/metadata error codes or semantic classes; cancellation takes precedence. Preserve all setting, singleton verification and configuration steps.

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: 2e5a9c41-aa30-4a1e-b176-22049e28af91
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