feat(node): predicate recipes, decisions, and registry - #543
Merged
kartikeya-27 merged 11 commits intoJul 25, 2026
Conversation
A gate could only allow or reject, so a policy like "hold this until business hours" was inexpressible. Gates may now return an EnqueueDecision; booleans still mean allow/reject.
Nine ready-made gates matching the Java and Python surfaces. Time zones resolve through Intl, so no new dependency; a wall-clock target re-reads the zone offset at the candidate instant to survive DST shifts.
Gates can be registered by name and referenced as `gate(task, "name")`, so gating is configurable without a serializable predicate AST. predicateStats() reports what the gates decided.
Regenerating the docs page also picks up the autoscaler entry that landed in CHANGELOG.md without a sync.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughNode enqueue gates now support ChangesNode predicate gates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Producer
participant Queue
participant Registry
participant Gate
participant EventBus
Producer->>Queue: enqueue or tryEnqueue
Queue->>Registry: resolve named gate
Queue->>Gate: evaluate with taskName, args, now
Gate-->>Queue: allow, reject, skip, or defer
Queue->>EventBus: emit predicate outcome
Queue-->>Producer: job id, null, or error
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sdks/node/src/queue.ts (1)
928-951: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSkip reason is discarded before it reaches
enqueue()/enqueueMany()'s thrown error.
prepareEnqueue'scase "skip"(Lines 934-936) emitsdecision.reasonon thepredicate.skippedevent but thenreturn null;, discarding the reason. Downstream,enqueue()(Lines 551-556) throwsnew EnqueueSkippedError(name)with no reason at all, andenqueueMany()(Line 632) substitutes a fixed generic string instead of the gate's actual reason. This is inconsistent with therejectpath just above (Lines 931-933), which does threaddecision.reasonintoPredicateRejectedError. SinceEnqueueSkippedError's constructor already accepts an optionalreason, callers using the primaryenqueue()API lose the specific "why" unless they separately wire apredicate.skippedlistener or pollpredicateStats().♻️ Proposed fix: thread the skip reason through `prepareEnqueue` → `submit` → `enqueue`
private prepareEnqueue<Name extends keyof TTasks & string>( name: Name, args: Parameters<TTasks[Name]> | undefined, options: EnqueueOptions | undefined, mode?: { batch: boolean }, - ): { taskName: string; payload: Buffer; options: NativeEnqueueOptions } | null { + ): { taskName: string; payload: Buffer; options: NativeEnqueueOptions } | { skipped: true; reason: string } { ... case "skip": this.emitter.emit("predicate.skipped", predicateEvent(taskName, decision.reason)); - return null; + return { skipped: true, reason: decision.reason };private submit<Name extends keyof TTasks & string>( name: Name, args: Parameters<TTasks[Name]> | undefined, options: EnqueueOptions | undefined, - ): string | null { + ): { jobId: string | null; skipReason?: string } { this.rejectIfQueueFull(options?.queue ?? "default"); const prepared = this.prepareEnqueue(name, args, options); - if (prepared === null) { - return null; + if ("skipped" in prepared) { + return { jobId: null, skipReason: prepared.reason }; } const { taskName, payload, options: nativeOpts } = prepared; const jobId = this.native.enqueue(taskName, payload, nativeOpts); this.emitter.emit("job.enqueued", { jobId, taskName, queue: nativeOpts.queue ?? "default" }); - return jobId; + return { jobId }; }enqueue<Name extends keyof TTasks & string>(...): string { - const jobId = this.submit(name, args, options); - if (jobId === null) { - throw new EnqueueSkippedError(name); + const { jobId, skipReason } = this.submit(name, args, options); + if (jobId === null) { + throw new EnqueueSkippedError(name, skipReason || undefined); } return jobId; } tryEnqueue<Name extends keyof TTasks & string>(...): string | null { - return this.submit(name, args, options); + return this.submit(name, args, options).jobId; }
enqueueMany's guard would similarly checkif ("skipped" in entry)and could appendentry.reasonto its message.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/node/src/queue.ts` around lines 928 - 951, Thread the skip reason from prepareEnqueue through submit to the public enqueue and enqueueMany flows instead of returning an uninformative null. Preserve decision.reason in the skipped result, have enqueue pass it to EnqueueSkippedError, and have enqueueMany detect skipped entries and include their reason rather than using a fixed generic message.docs/content/docs/node/more/examples/predicate-gated-jobs.mdx (1)
63-84: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReject demo can't actually reject; the gate that can (
underQuota) is never shown.
sendPromo's gates areanyOf(urgent, duringBusinessHours)—urgentis a plain boolean andduringBusinessHoursonly ever allows or defers, so this call can never throwPredicateRejectedError, even though the surrounding text says "A rejection throws synchronously." MeanwhileunderQuota, the gate defined specifically to reject (line 33-38) and wired toexportData(line 60), is never enqueued anywhere in the doc.Consider swapping the reject example to
queue.enqueue("exportData", [tenantId])so the catch block demonstrates a real rejection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/docs/node/more/examples/predicate-gated-jobs.mdx` around lines 63 - 84, Update the rejection example to enqueue the reject-capable exportData task instead of sendPromo, passing the tenantId argument used by its underQuota gate. Keep the PredicateRejectedError handling and surrounding explanation intact, while leaving the tryEnqueue example unchanged.
🧹 Nitpick comments (1)
sdks/node/test/core/predicate-recipes.test.ts (1)
244-255: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard
TASKITO_TEST_FLAGcleanup withtry/finally.If the assertion at Line 252 fails,
delete process.env.TASKITO_TEST_FLAGon Line 253 never runs, leaking the env var into later tests. The siblingfeatureFlagenv test (Lines 199-208) already guards its cleanup withtry/finally— apply the same pattern here.♻️ Proposed fix
process.env.TASKITO_TEST_FLAG = "0"; - expect(gate(at(WED_NOON_UTC))).toBe(false); - delete process.env.TASKITO_TEST_FLAG; + try { + expect(gate(at(WED_NOON_UTC))).toBe(false); + } finally { + delete process.env.TASKITO_TEST_FLAG; + } expect(() => envVarTruthy("")).toThrow(PredicateValidationError);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/node/test/core/predicate-recipes.test.ts` around lines 244 - 255, Wrap the TASKITO_TEST_FLAG mutations and assertions in the envVarTruthy test with a try/finally block, and move delete process.env.TASKITO_TEST_FLAG into the finally block so cleanup always runs even when an expectation fails. Preserve the existing truthy, false, and validation assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 34-42: Update the older “Full lifecycle event taxonomy” changelog
entry to remove predicate.deferred from the Python-only event list, while
retaining predicate.cancelled and the rest of the existing wording.
In `@docs/content/docs/node/api-reference/queue/index.mdx`:
- Around line 46-47: Update the tryEnqueue documentation row to state that gate
skips return null while gate rejects still throw PredicateRejectedError,
preserving the existing enqueue behavior description.
In `@docs/content/docs/resources/changelog.mdx`:
- Around line 41-49: The changelog contains a contradictory lifecycle-event
statement about predicate.deferred. Update the “Full lifecycle event taxonomy”
bullet to remove predicate.deferred from the Python-only list, while keeping
predicate.cancelled there and preserving the event-bus statement in the Node
predicate recipes bullet.
In `@docs/content/docs/shared/guides/core/predicates.mdx`:
- Around line 235-266: Update the Decision table’s `false` /
`Decision.reject(reason)` row to explicitly state that `tryEnqueue` also throws
`PredicateRejectedError`, matching the documented Java behavior. Leave the
existing `enqueue` behavior and other decision rows unchanged.
- Around line 382-413: Update the introductory behavior summary to distinguish
ordinary Boolean recipes— isWeekend, before, and envVarTruthy—which allow or
reject, from recipes that skip silently. State that payloadMatches and
featureFlag skip, while dayOfWeek skips only when days is empty; preserve the
existing time-based defer behavior.
In `@sdks/node/src/predicates/combinators.ts`:
- Around line 12-22: Update the composed-gate handling in allOf and the
corresponding anyOf path to treat null or undefined outcomes as false before
accessing outcome.kind or otherwise inspecting the result. Preserve existing
boolean and decision handling, ensuring missing gate returns fail closed and
produce rejection behavior.
In `@sdks/node/src/predicates/decisions.ts`:
- Around line 90-96: Extend the decision validation immediately before the
return in the gate outcome validation flow to enforce payload invariants, not
just DECISION_KINDS membership. For defer outcomes, require a valid non-negative
delayMs consistent with Decision.defer(); for skip and reject outcomes, validate
their reason payloads using the existing decision constructors or validation
symbols. Keep unknown-kind errors unchanged and only return outcome after all
kind-specific payload checks pass.
In `@sdks/node/src/predicates/recipes.ts`:
- Around line 340-348: Update the object-property lookup in step to use
Object.hasOwn(record, segment) instead of the in operator, ensuring only own
payload properties match while preserving the existing MISSING behavior for
absent paths.
In `@sdks/node/src/predicates/tz.ts`:
- Around line 122-133: The zonedInstant conversion must resolve nonexistent
spring-forward wall times forward to the first valid local instant instead of
applying a correction that moves backward across the gap. Update zonedInstant’s
offset-estimation logic to detect when the requested wall-clock target falls in
a timezone gap and advance the result to the gap’s end, while preserving normal
conversion behavior for valid times and fall-back ambiguities.
---
Outside diff comments:
In `@docs/content/docs/node/more/examples/predicate-gated-jobs.mdx`:
- Around line 63-84: Update the rejection example to enqueue the reject-capable
exportData task instead of sendPromo, passing the tenantId argument used by its
underQuota gate. Keep the PredicateRejectedError handling and surrounding
explanation intact, while leaving the tryEnqueue example unchanged.
In `@sdks/node/src/queue.ts`:
- Around line 928-951: Thread the skip reason from prepareEnqueue through submit
to the public enqueue and enqueueMany flows instead of returning an
uninformative null. Preserve decision.reason in the skipped result, have enqueue
pass it to EnqueueSkippedError, and have enqueueMany detect skipped entries and
include their reason rather than using a fixed generic message.
---
Nitpick comments:
In `@sdks/node/test/core/predicate-recipes.test.ts`:
- Around line 244-255: Wrap the TASKITO_TEST_FLAG mutations and assertions in
the envVarTruthy test with a try/finally block, and move delete
process.env.TASKITO_TEST_FLAG into the finally block so cleanup always runs even
when an expectation fails. Preserve the existing truthy, false, and validation
assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 76064349-51ac-401f-ae43-0fc1cb2ae15d
📒 Files selected for processing (24)
CHANGELOG.mddocs/content/docs/node/api-reference/queue/index.mdxdocs/content/docs/node/api-reference/queue/resources.mdxdocs/content/docs/node/more/examples/predicate-gated-jobs.mdxdocs/content/docs/resources/changelog.mdxdocs/content/docs/shared/guides/core/predicates.mdxsdks/node/src/errors.tssdks/node/src/events.tssdks/node/src/index.tssdks/node/src/predicates.tssdks/node/src/predicates/combinators.tssdks/node/src/predicates/context.tssdks/node/src/predicates/decisions.tssdks/node/src/predicates/index.tssdks/node/src/predicates/metrics.tssdks/node/src/predicates/providers.tssdks/node/src/predicates/recipes.tssdks/node/src/predicates/registry.tssdks/node/src/predicates/tz.tssdks/node/src/queue.tssdks/node/test/core/predicate-recipes.test.tssdks/node/test/core/predicate-registry.test.tssdks/node/test/core/predicates.test.tssdks/node/test/core/webhooks.test.ts
💤 Files with no reviewable changes (1)
- sdks/node/src/predicates.ts
A gate returning nothing threw on `.kind` inside allOf/anyOf instead of
failing closed, and a hand-built `{ kind: "defer", delayMs: -1 }` — which
type-checks — bypassed the factory's invariant on its way to storage.
`segment in record` let a path resolve to an inherited member such as `constructor`, which is not present in the payload.
A wall clock inside a DST gap (02:30 in New York on 2026-03-08) resolved to 01:30 EST — an hour before the requested time — because the offset correction moved the instant back across the transition. Now the candidate the zone agrees with wins, and a gap target shifts forward past the gap, as java.time does.
tryEnqueue only swallows a skip, and the recipes are not uniformly skip-or-defer — three are plain booleans that reject.
Node emits it as of this branch.
Contributor
Author
CodeRabbit round 1 — all 9 findings addressedEvery one was valid; nothing skipped. Five commits, split by concern.
The DST one was the most substantive. It also falsified this branch's own docstring, which claimed Four new regression tests, each verified red on the preceding commit: Node suite 270 passed in |
kartikeya-27
approved these changes
Jul 25, 2026
This was referenced Jul 27, 2026
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
2 participants
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.
Closes #517.
Node's predicate surface was 28 lines — a boolean
PredicateplusallOf/anyOf/not. A gatecould only allow or reject, so a policy like "hold this until business hours" was inexpressible.
Java already had
EnqueueDecisionplus five recipes; Python adds a registry, outcome metrics, and afeature-flag provider seam. This closes the whole parity row rather than only the Java floor the
issue named as the minimum, since the issue title asks for the registry too.
src/predicates.tsbecomessrc/predicates/— a package alongsidelocks/,resources/,autoscale/. Existingfrom "./predicates"imports resolve unchanged.Decisions
EnqueueDecisionisallow | skip(reason) | defer(delayMs) | reject(reason), built with aDecisionfactory object copying theInterceptionidiom. Evaluated where gates already ran, aftermiddleware
onEnqueue; the first non-allowdecision wins, as in Java.reject→PredicateRejectedError(unchanged behaviour)skip→enqueuethrows the newEnqueueSkippedError; the newtryEnqueue()returnsnulldefer→ job created with the gate's delay, replacing anydelayMsthe caller passedBooleans keep working:
toDecisionnormalizestrue/false, and a barefalsecarries no reasonso the thrown message and the
predicate.rejectedpayload are byte-identical to before. A null-ishreturn (a JavaScript caller returning nothing by accident) fails closed.
Recipes
Nine, validated at construction so a bad zone or window throws at wiring time:
businessHours,timeWindow,dayOfWeek,payloadMatches,featureFlagisWeekend,after,before,envVarTruthyAlso exported as a
Recipesbundle, so the Java examples transliterate. Zones resolve throughIntl— no new dependency.Registry, metrics, events
registerPredicate(name, gate)plusqueue.gate(task, "name"), resolved when the gate isregistered so an unknown name throws at startup rather than on the first enqueue. This is what makes
config-driven gating possible without a serializable AST — Node gates are closures, and Python's AST
exists only to serialize one.
queue.predicateStats()returns{ allowed, skipped, deferred, rejected, errors }, one count pergated enqueue.
predicate.skippedandpredicate.deferredjoinpredicate.rejectedon the eventbus and are webhook-subscribable.
Three choices worth reviewing
allOf/anyOfare overloaded, not widened.(...Predicate[]) => Predicateand(...EnqueueGate[]) => EnqueueGate, and the body returns a real boolean when every memberreturned one. Widening the return type alone would have left
const p: Predicate = allOf(a, b)compiling while
if (p(ctx))became always-true — a silent break.notstays boolean-only;inverting a
deferhas no meaning.PredicateContextgainednow, read once per evaluation by the queue. Without it the timerecipes would call
new Date()internally and be untestable, which is the case for Java'sRecipestoday.deferbut refuseskip. Each entry carries its own options, so a per-entrydelay works (Java can't — one shared options object). A skip throws
EnqueueSkippedError,because the returned ids line up with the input and a dropped entry isn't expressible.
Closed sets follow the existing convention — literal union plus an
as constarray(
DECISION_KINDS,DAYS_OF_WEEK), andas const satisfies Record<…>for the decision→counter map,mirroring
OUTCOME_KIND_EVENTS. No TSenum, and the #505 contract that plain strings keep workingis preserved.
Time-zone math
tz.tsreads a wall clock withIntl.DateTimeFormatand resolves one back to an instant byguessing with the offset at a reference instant, then re-reading the offset at the guess and
correcting once. That second pass is what makes a target across a DST boundary land correctly —
regression-tested against New York's 2026-03-08 spring-forward, where the naive single-pass answer
is an hour off. A target inside a spring-forward gap resolves to just after it, documented.
Tests
test/core/predicates.test.ts7 → 19, pluspredicate-recipes.test.ts(33) andpredicate-registry.test.ts(6). Recipes are driven through a pinnednow, covering in/out ofwindow, midnight wrap, weekend rollover, the DST boundary, empty day sets, dotted-path misses, and
rejected zones.
webhooks.test.ts's event count goes 26 → 28 for the two new names.Node suite: 494 passed. The 10
test/dashboard/files fail onmastertoo —viteisn't installedin the
dashboard/workspace locally, sobuild:dashboardfails inbeforeAll.Smoke on a real clock (Saturday 05:42 UTC):
businessHours({ timeZone: "UTC" })scheduled the jobat
2026-07-27T09:00:00.002Z, the next weekday opening, andfeatureFlagwith the flag unsetreturned
nullfromtryEnqueuewith no row written.Docs
Rewrote the four
<SdkOnly sdk="node">blocks that said Node has no defer outcome, no recipelibrary, no inspection API, and no predicate metrics or events, plus the batch semantics, the node
example page, and two api-reference tables.
docs check:parityclean and the site builds.Summary by CodeRabbit
New Features
tryEnqueue(returnsnullon skipped),predicateStats(), and new predicate outcome events (predicate.skipped,predicate.deferred).Bug Fixes
Documentation