Skip to content

fix: validate explicit worker filters at startup - #379

Merged
wangbill (YunchuWang) merged 1 commit into
mainfrom
yunchuwang-validate-work-item-filters
Oct 5, 2026
Merged

wangbill (YunchuWang) merged 1 commit into
mainfrom
yunchuwang-validate-work-item-filters

Conversation

@YunchuWang

Copy link
Copy Markdown
Member

Summary

What changed?

  • Validate effective explicit work-item filter names in worker.start(), after the already-running guard and before creating the gRPC client or worker loop.
  • Report every missing name in one actionable error grouped by Orchestrations, Activities, and Entities. A failed validation leaves registration open so the caller can register missing tasks and retry.
  • Reuse existing registry name getters across all versions. Keep orchestration/activity names case-sensitive and entity names case-insensitive. Do not validate filter versions or change dispatch, wire serialization, default/auto/empty filters, or opt-in behavior.

Why is this change needed?

A worker can register ProcessOrder but explicitly filter for a misspelled or unregistered name. Previously start() succeeded and sent the invalid filter to the service; the local registration mistake was not reported at startup. Now start() rejects locally and tells the caller to register the missing names or remove the filters. Valid configurations behave as before.

Issues / work items


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 filter names absent from their task-kind registry now reject at startup instead of connecting.
      • Migration guidance: Register the names on this worker or remove them from the filters; retry start(). README and the upcoming breaking-change notes document this.

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 coding agent.
  • AI-assisted areas/files: Worker startup validation, core startup/filter tests, Azure-managed builder regression test, README, CHANGELOG, and this description.
  • What you changed after AI output: No human post-processing yet. Human review attestations below are intentionally unchecked.

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

Testing

Automated tests

  • TDD RED before production changes: 13 expected failures, 86 passes, 3 suites. Unknown names resolved start() instead of rejecting; the builder reached the guarded gRPC creation boundary.
  • GREEN: 706 tests passed, 9 suites, including all 23 new test cases. Covers each task kind, aggregation, empty/null/missing JavaScript names, wrong-kind registration, aliases/casing, versioned-only registrations, arbitrary filter versions, no startup side effects, retry after registration, and already-running precedence.

Exact commands from repository root (Windows PowerShell):

# RED
npx --no-install jest --runInBand --detectOpenHandles --runTestsByPath packages\durabletask-js\test\worker-startup.spec.ts packages\durabletask-js\test\work-item-filters.spec.ts packages\durabletask-js-azuremanaged\test\unit\worker-builder.spec.ts

# GREEN
npx --no-install jest --runInBand --detectOpenHandles --silent --runTestsByPath packages\durabletask-js\test\worker-startup.spec.ts packages\durabletask-js\test\work-item-filters.spec.ts packages\durabletask-js\test\versioned-dispatch.spec.ts packages\durabletask-js\test\versioned-dispatch-grpc.spec.ts packages\durabletask-js\test\worker-versioning.spec.ts packages\durabletask-js\test\concurrency-options.spec.ts packages\durabletask-js-azuremanaged\test\unit\worker-builder.spec.ts packages\durabletask-js-azuremanaged\test\unit\options.spec.ts packages\durabletask-js-azuremanaged\test\unit\resource-id.spec.ts

# Builds/type-checks and scoped lint: passed
npm run build:core
npm run build:azuremanaged
npx --no-install eslint packages\durabletask-js\src\worker\task-hub-grpc-worker.ts packages\durabletask-js\test\worker-startup.spec.ts packages\durabletask-js\test\work-item-filters.spec.ts packages\durabletask-js-azuremanaged\test\unit\worker-builder.spec.ts
git diff --check origin/main...HEAD

Formatting: scoped Prettier checking with --end-of-line crlf reports existing deviations in the worker, filter spec, and README. Comparing the exact formatter edits against base 5eca577aab8cf42631dc9afcc1606f1450a843d4 confirms no new formatting deviations in any changed file; checkout CRLF is preserved. This is not a blanket full-file Prettier pass.

Manual validation (only if runtime/behavior changed)

  • Environment (OS, Node.js version, components): Windows, Node.js 24.14.0, npm 11.9.0; locally built core and Azure-managed packages.
  • Steps + observed results:
    1. With an in-process fake transport, an Azure-managed builder's unknown activity filter rejected before client creation.
    2. Registering that activity with only a versioned implementation allowed retrying start() and produced the expected filter.
    3. Default unfiltered startup, and overwriting invalid explicit filters with auto or empty filters, retained the expected requests.
  • Evidence (optional): RED/GREEN, build, lint, formatting-baseline, compiled-package smoke, and normal Husky commit logs retained in the authoring session.

Notes for reviewers

  • Candidate base: 5eca577aab8cf42631dc9afcc1606f1450a843d4; candidate tree: 7f3b79f5317e16d322d0d1a766520ac4b78fc3e7. Six files, +294/-2; production change is confined to the existing worker.
  • Full-suite, Azure/DTS/emulator, and Functions E2E tests were not run. The targeted gRPC dispatch suite uses loopback locally; validation regression tests use mocks without a DTS connection. No Azure resources were provisioned.
  • Hosted CI is pending/not asserted as passed. No review requests, ready labels, or merge action are part of this PR.

Reject unknown filter names by task kind before connecting, preserving name-only and case-sensitive task registration semantics.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e0af01a5-0dfa-4e71-a660-c4186e65d7e0
Copilot AI balanced review requested due to automatic review settings October 2, 2026 22:15

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.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 High severity · 1 Medium severity · 1 Low severity

Open (3)
What changed in this PR

Adds startup-time validation for explicit work-item filters so workers fail fast when filters reference unregistered task names, preventing invalid filter configs from reaching the service.

Changes:

  • Validate explicit filter names in TaskHubGrpcWorker.start() before gRPC/client startup and worker loop initialization.
  • Add/adjust Jest coverage for grouped validation errors, retry-after-registration, already-running precedence, and Azure-managed builder behavior.
  • Update README + CHANGELOG to document the new startup validation behavior and migration guidance.
File Description
packages/​durabletask-js/​src/​worker/​task-hub-grpc-worker.ts Adds _validateWorkItemFilters() and calls it early in start() to reject unknown explicit filter names.
packages/​durabletask-js/​test/​worker-startup.spec.ts Adds comprehensive unit tests for validation behavior and “no side effects before validation” guarantees.
packages/​durabletask-js/​test/​work-item-filters.spec.ts Updates explicit-filter precedence test to register the explicit names (now required).
packages/​durabletask-js-azuremanaged/​test/​unit/​worker-builder.spec.ts Adds regression test ensuring builder registrations are applied before validation and gRPC startup is avoided on failure.
README.md Documents the new validation behavior, casing rules, and retry guidance.
CHANGELOG.md Records the breaking behavior change: explicit unknown filter names now fail at start().

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/durabletask-js/src/worker/task-hub-grpc-worker.ts
Comment thread packages/durabletask-js/src/worker/task-hub-grpc-worker.ts
Comment thread packages/durabletask-js/test/worker-startup.spec.ts
@YunchuWang

Copy link
Copy Markdown
Member Author

AI-assisted review of b8324674: no blocking correctness issues found. Validation runs after the already-running guard and before gRPC/lifecycle startup, uses the correct task-kind registrations across versions, and leaves registration open after rejection so startup can be retried. Default/auto/empty filters and valid wire filters remain unchanged.

I also assessed all three open review threads: the entity-casing warning is a false positive because registry keys are already lowercase; duplicate missing names are optional diagnostic polish; and the zero-timer assertion is valid with the current fresh fake-timer setup. None is a blocker. Thread resolution is left to the reviewer; this is not a human approval.

kaibocai (@kaibocai) could you review and approve this PR when satisfied?

@YunchuWang
wangbill (YunchuWang) merged commit 762d852 into main Oct 5, 2026
31 checks passed
@YunchuWang
wangbill (YunchuWang) deleted the yunchuwang-validate-work-item-filters branch October 5, 2026 22:06
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.

3 participants