Skip to content

feat: replace stringly-typed closed-set params with enums#505

Merged
pratyush618 merged 22 commits into
masterfrom
feat/closed-set-enums
Jul 23, 2026
Merged

feat: replace stringly-typed closed-set params with enums#505
pratyush618 merged 22 commits into
masterfrom
feat/closed-set-enums

Conversation

@kartikeya-27

@kartikeya-27 kartikeya-27 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

All six splits of #503. Does not close it — a few inventory rows remain (see below).

Closed-set parameters that were typed str / String / &str now carry an enum. Most were
guarded by a hand-rolled membership check; a few by nothing at all —
Queue(disabled_proxies=["requests"]) silently left requests_session registered, which is
what prompted the issue.

Splits 1-5 are non-breaking: every wire and stored value is byte-identical and existing string
callers keep working. Split 6 is a breaking Python change — see below.

Commits

Commit Scope
feat(python): type the built-in proxy ids BuiltInProxy — an unknown id now raises listing the valid ones instead of leaving the handler registered
feat(java): take enums for dispatch order and log level DispatchOrder, TaskLogLevel, WorkflowState on listWorkflowRuns; matching Node unions
feat(python): type the closed-set task and workflow options InterceptionMode, PredicateAction, ConsumerErrorAction, GateAction, FanStrategy, DiagramFormat
feat: type the dashboard user role across the SDKs Role in all three SDKs, replacing three parallel VALID_ROLES sets
feat(core): type subscription mode and worker status SubscriptionMode, WorkerStatus on the storage trait
feat(python)!: rename resource scopes to match the other SDKs split 6 — see below
feat(node): add the request resource scope fresh per useResource(), disposed with the task

Split 6: the scope names meant different things

The issue filed this as "ResourceScope has three different shapes". It is worse than that —
the same name meant a different lifecycle per SDK, verified in the runtimes rather than the
enum docs:

Lifecycle Python (before) Java Node (before)
shared, worker lifetime WORKER WORKER worker
one per worker thread THREAD THREAD
fresh per task REQUEST TASK task
pool checkout per task TASK POOLED pooled
fresh per resolve REQUEST

resources/runtime.py built a ResourcePool for ResourceScope.TASK and did
pool.acquire() / pool.release(); Java's TASK is per-invocation and its pooling lives under
POOLED. So scope=TASK ported from Python to Java silently lost the pool — no bounded
capacity, no checkout, an instance per task instead.

Java's names win. Python renames TASKPOOLED and REQUESTTASK; Node gains request.

Migration. scope="request" now raises (the member is gone). scope="task" still resolves
but builds per task instead of pooling — the one path that could change behaviour quietly, so
ResourceDefinition rejects pool_size / pool_min on any non-POOLED scope, which is how
pooled use is always spelled. Move pooled resources to scope="pooled".

Two scopes stay platform-bound, now documented in all three SDKs: thread has no Node
equivalent (a worker runs its tasks on one event loop), and request has no Python one
(resources arrive by injection, resolved once per task).

Other notes for a reviewer

  • TaskLogLevel, not LogLevel. Java's logging/LogLevel and Node's utils LogLevel
    are the SDK's own logger level — an unrelated concept the issue called out.
  • dispatchOrder(String) still throws IllegalArgumentException, not the model enum's
    SerializationException — callers of that overload predate the enum.
  • Dashboard handlers stay on the lenient string path in Java and Node. An unknown
    ?level= / ?state= is untrusted input that must filter to nothing, not fail the request.
  • A stored role that no longer parses falls back to viewer in all three SDKs rather than
    being trusted. Same fail-closed treatment for an unrecognized mode / status in core.
  • No migration for the core enums. SubscriptionMode and WorkerStatus are converted at
    each persistence boundary, so the Redis JSON and every SQL column keep their existing strings
    and a legacy row still reads.

Verification

  • cargo test --workspace — 285 tests; --features postgres and --features redis check
    clean; cargo clippy --workspace --all-targets clean.
  • Python: 1304 passed / 7 skipped; ruff and mypy clean (source + tests).
  • Node: 472 tests across 77 files; typecheck and biome clean.
  • Java: ./gradlew build green — spotless, checkstyle, strict javadoc, tests.
  • Docs: biome clean; CHANGELOG.md carries the breaking-change entry.

Left for follow-ups

Inventory rows the issue's split list never reached: NodeStatus raw comparisons in the saga
orchestrator and tracker DAG, Strategy coercion in runtime_config, WorkflowCondition,
StorageBackend, and the webhook DeliveryStatus filter. Open registries (proxy handler names,
resource/task/queue names) stay strings, per the issue's non-goals.

Summary by CodeRabbit

  • New Features

    • Added typed options for subscription modes, worker statuses, task log levels, dispatch ordering, dashboard roles, workflow settings, and interception behavior across SDKs.
    • Added Node.js request-scoped resources with fresh instances per resolution and automatic disposal.
    • Added Python support for pooled resources and enum-based proxy, workflow, and interception configuration.
    • Added Java enum-based APIs for dispatch ordering and task log writing.
  • Bug Fixes

    • Invalid configuration values now produce clear errors instead of being silently accepted.
    • Unrecognized stored dashboard roles safely fall back to viewer access.
  • Documentation

    • Updated resource-scope guidance and Python proxy configuration examples.

disabled_proxies took bare strings, so a typo silently left the handler registered.
Node gains the matching DispatchOrder and TaskLogLevel unions. The string overloads stay, deprecated.
An unknown visualize format silently rendered Mermaid; the rest raised at a distance from the call that was wrong.
Replaces three parallel VALID_ROLES sets. A persisted role that no longer parses now falls back to viewer rather than being trusted.
Both were unvalidated strings threaded through the storage trait; the shells parse at the boundary so a legacy row still reads.
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This pull request replaces stringly typed closed-set values with enums and wire conversions across Rust storage, Java, Node, and Python APIs. It also updates Python resource scopes and pooling behavior, adds Node request-scoped resources, and revises related tests, documentation, and exports.

Changes

Rust storage and pub/sub

Layer / File(s) Summary
Typed records and storage APIs
crates/taskito-core/src/storage/...
Adds SubscriptionMode and WorkerStatus, updates storage traits and backends, and serializes enum values to existing wire strings.
Pub/sub routing
crates/taskito-core/src/pubsub.rs, crates/taskito-core/src/storage/diesel_common/pubsub.rs
Uses typed log-mode checks for declaration, publishing, fan-out, cursors, compaction, statistics, and leasing.
Binding boundaries
crates/taskito-{java,node,python}/src/...
Parses caller strings into typed modes/statuses and converts domain values back to strings.

Java SDK and dashboard

Layer / File(s) Summary
Typed SDK options
sdks/java/src/main/java/org/byteveda/taskito/{Taskito.java,DefaultTaskito.java}, sdks/java/src/main/java/org/byteveda/taskito/model/*
Adds enum-based dispatch-order and task-log APIs while retaining deprecated string overloads.
Dashboard roles
sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/*
Adds typed role persistence, strict input parsing, fail-closed stored-role conversion, and enum-based authorization paths.
Java tests
sdks/java/src/test/java/...
Updates SDK and dashboard tests for enum values, wire conversion, invalid inputs, and persisted-role fallback.

Node SDK and resources

Layer / File(s) Summary
Typed exports and filters
sdks/node/src/types.ts, sdks/node/src/index.ts, sdks/node/src/dashboard/*, sdks/node/src/queue.ts
Exports typed roles, resource scopes, dispatch orders, and task-log levels, and applies the task-log filter type.
Request-scoped resources
sdks/node/src/resources/*
Adds request-scope resolution with per-resolution instances, teardown disposal, metrics, scope validation, and dependency-cycle detection.
Node tests
sdks/node/test/resources/resources.test.ts
Covers request/task lifecycle differences, scope validation, dependency cycles, and diamond dependencies.

Python typed APIs and resources

Layer / File(s) Summary
Enum-backed APIs
sdks/python/taskito/{enums.py,app.py,interception/*,predicates/*,mixins/*,dashboard/auth.py,proxies/*}
Adds shared enum coercion and applies typed values to interception, predicates, consumers, proxies, and dashboard roles.
Workflow options
sdks/python/taskito/workflows/*
Uses typed gate actions, fan strategies, and diagram formats with strict invalid-value handling.
Resource scopes
sdks/python/taskito/resources/*, sdks/python/taskito/mixins/resources.py
Replaces Python REQUEST with POOLED, validates pool settings, and updates pooled/task acquisition and teardown behavior.
Python tests
sdks/python/tests/...
Adds coverage for enum coercion, invalid values, proxy disabling, role fallback, resource scope migration, pooled reloads, and async teardown.

Documentation and release notes

Layer / File(s) Summary
API and lifecycle documentation
docs/content/docs/..., CHANGELOG.md
Documents enum-compatible inputs, revised Python resource scopes, Node request scope, proxy validation, and cross-SDK scope differences.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

Possibly related PRs

  • ByteVeda/taskito#459 — Introduces the core log-and-cursor pub/sub paths whose mode checks are converted to SubscriptionMode.
  • ByteVeda/taskito#462 — Relies on the topic registry and log-retention behavior updated by the typed subscription-mode changes.

Suggested labels: dashboard

Suggested reviewers: pratyush618

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: replacing stringly typed closed-set parameters with enums.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/closed-set-enums

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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 `@sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java`:
- Around line 206-213: Update createSession to explicitly validate that role is
non-null before calling role.wire(), and return the same controlled “invalid
role” error behavior used by createUser. Preserve the existing session creation
flow for valid roles.

In `@sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java`:
- Around line 752-755: Update the deprecated queryTaskLogs(String taskName,
String level, long sinceMs, long limit) overload to preserve the raw wire-level
value for lenient dashboard filtering instead of eagerly calling
TaskLogLevel.fromWire(level), so unknown levels produce an empty result set
rather than throwing. Apply the same change to the corresponding overload near
the second referenced location, including the analogous state filter if present.

In `@sdks/java/src/main/java/org/byteveda/taskito/Taskito.java`:
- Around line 372-380: Resolve the ambiguous same-arity overloads in Taskito for
queryTaskLogs and listWorkflowRuns by giving the enum- and String-based APIs
distinct method names or a shared options/type wrapper. Preserve null as the
documented no-filter value so calls such as queryTaskLogs(task, null, since,
limit) and listWorkflowRuns(name, null, limit, offset) compile unambiguously,
and update deprecation references as needed.

In `@sdks/python/taskito/dashboard/auth.py`:
- Line 157: Update get_session() to normalize the persisted role with
_role_or_viewer() before constructing Session, matching the existing
_row_to_user() behavior. Ensure unknown serialized roles become the viewer role
while valid roles remain unchanged.
🪄 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: ce8469b1-e7b9-4967-b99c-5505fa530519

📥 Commits

Reviewing files that changed from the base of the PR and between 256f7c4 and f523d9d.

📒 Files selected for processing (72)
  • crates/taskito-core/src/pubsub.rs
  • crates/taskito-core/src/storage/diesel_common/pubsub.rs
  • crates/taskito-core/src/storage/diesel_common/workers.rs
  • crates/taskito-core/src/storage/mod.rs
  • crates/taskito-core/src/storage/models.rs
  • crates/taskito-core/src/storage/postgres/pubsub.rs
  • crates/taskito-core/src/storage/records.rs
  • crates/taskito-core/src/storage/redis_backend/pubsub.rs
  • crates/taskito-core/src/storage/redis_backend/workers.rs
  • crates/taskito-core/src/storage/sqlite/pubsub.rs
  • crates/taskito-core/src/storage/sqlite/tests.rs
  • crates/taskito-core/src/storage/traits.rs
  • crates/taskito-core/tests/rust/storage_tests.rs
  • crates/taskito-java/src/convert.rs
  • crates/taskito-java/src/queue/pubsub.rs
  • crates/taskito-java/src/worker.rs
  • crates/taskito-node/src/convert/pubsub.rs
  • crates/taskito-node/src/queue/pubsub.rs
  • crates/taskito-python/src/py_queue/pubsub.rs
  • crates/taskito-python/src/py_queue/worker.rs
  • docs/content/docs/python/api-reference/queue/index.mdx
  • docs/content/docs/python/guides/resources/proxies.mdx
  • sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java
  • sdks/java/src/main/java/org/byteveda/taskito/Taskito.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/CoreHandlers.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/WorkflowsHandlers.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthHandlers.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Role.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Session.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/TokenAuth.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/User.java
  • sdks/java/src/main/java/org/byteveda/taskito/model/DispatchOrder.java
  • sdks/java/src/main/java/org/byteveda/taskito/model/TaskLogLevel.java
  • sdks/java/src/test/java/org/byteveda/taskito/core/DispatchOrderTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardAuthTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardClient.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardNativeTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/oauth/OAuthFlowTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/model/ClosedSetEnumTest.java
  • sdks/node/src/dashboard/auth/store.ts
  • sdks/node/src/dashboard/handlers/core.ts
  • sdks/node/src/dashboard/index.ts
  • sdks/node/src/index.ts
  • sdks/node/src/queue.ts
  • sdks/node/src/types.ts
  • sdks/python/taskito/__init__.py
  • sdks/python/taskito/app.py
  • sdks/python/taskito/dashboard/auth.py
  • sdks/python/taskito/enums.py
  • sdks/python/taskito/interception/__init__.py
  • sdks/python/taskito/interception/interceptor.py
  • sdks/python/taskito/interception/mode.py
  • sdks/python/taskito/mixins/_log_consumer.py
  • sdks/python/taskito/mixins/decorators.py
  • sdks/python/taskito/mixins/predicates.py
  • sdks/python/taskito/mixins/pubsub.py
  • sdks/python/taskito/predicates/__init__.py
  • sdks/python/taskito/predicates/outcomes.py
  • sdks/python/taskito/proxies/__init__.py
  • sdks/python/taskito/proxies/built_in.py
  • sdks/python/taskito/workflows/__init__.py
  • sdks/python/taskito/workflows/builder.py
  • sdks/python/taskito/workflows/fan_out.py
  • sdks/python/taskito/workflows/run.py
  • sdks/python/taskito/workflows/tracker/gates.py
  • sdks/python/taskito/workflows/types.py
  • sdks/python/tests/python/test_closed_set_enums.py
  • sdks/python/tests/resources/test_proxies.py

Comment thread sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java Outdated
Comment thread sdks/java/src/main/java/org/byteveda/taskito/Taskito.java Outdated
Comment thread sdks/python/taskito/dashboard/auth.py
TASK meant pool checkout and REQUEST meant fresh-per-task — the reverse of every other SDK, so the same scope silently behaved differently. Pool settings on a non-pooled scope now raise, which catches the one case the rename could pass over quietly.
Fresh on every useResource() call, disposed with the task — the one scope Java had that Node could express.
THREAD has no Node equivalent, REQUEST no Python one. Writing that down is what makes the shared names trustworthy.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
sdks/python/taskito/resources/runtime.py (1)

54-72: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Make pooled-resource reload scope-aware.

reload() reaches recreate(), which always calls _create_resource(). For a pooled definition, that leaves the old pool in _pools while adding a singleton to _instances; resolve() returns the singleton, but task injection continues using the old pool. Recreate or replace the pool instead of creating a worker instance.

🤖 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/python/taskito/resources/runtime.py` around lines 54 - 72, Update the
reload/recreate flow to branch on the resource definition scope before calling
_create_resource: pooled definitions must recreate or replace their ResourcePool
in _pools, preserving pool configuration, dependencies, and prewarming behavior,
rather than creating an instance in _instances. Keep singleton/resource handling
unchanged and ensure subsequent resolve and task injection use the replacement
pool.
🤖 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 `@docs/content/docs/python/guides/resources/configuration.mdx`:
- Around line 80-82: Update the pool configuration example to use scope="pooled"
instead of scope="task" while retaining pool_size and pool_min, so it matches
the documented validation rule.

In `@docs/content/docs/python/guides/resources/dependency-injection.mdx`:
- Around line 116-120: Update the create_session resource declaration to include
its db dependency through the decorator’s depends_on configuration, while
preserving the pooled scope and pool_size=10 settings so the factory receives
the worker-scoped db resource.

In `@sdks/python/taskito/resources/definition.py`:
- Around line 54-64: Normalize or validate self.scope at the start of
ResourceDefinition.__post_init__, converting supported string values such as
"worker" into the corresponding ResourceScope member and rejecting unknown
values before pool_size/pool_min validation. Ensure subsequent runtime scope
branches receive a ResourceScope value and preserve the existing pooled-scope
validation behavior.

In `@sdks/python/taskito/resources/runtime.py`:
- Around line 165-170: Update teardown_task to invoke defn.teardown(instance)
through run_maybe_async(), matching the existing worker teardown behavior so
both synchronous and asynchronous teardowns execute before the function
completes. Preserve the current exception logging and resource/name checks.

---

Outside diff comments:
In `@sdks/python/taskito/resources/runtime.py`:
- Around line 54-72: Update the reload/recreate flow to branch on the resource
definition scope before calling _create_resource: pooled definitions must
recreate or replace their ResourcePool in _pools, preserving pool configuration,
dependencies, and prewarming behavior, rather than creating an instance in
_instances. Keep singleton/resource handling unchanged and ensure subsequent
resolve and task injection use the replacement pool.
🪄 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: e5625723-28a9-4b97-86d4-a1bceb1bd82c

📥 Commits

Reviewing files that changed from the base of the PR and between f523d9d and c31d8dc.

📒 Files selected for processing (17)
  • CHANGELOG.md
  • docs/content/docs/java/guides/resources/dependency-injection.mdx
  • docs/content/docs/node/guides/resources/dependency-injection.mdx
  • docs/content/docs/python/api-reference/queue/resources.mdx
  • docs/content/docs/python/guides/resources/configuration.mdx
  • docs/content/docs/python/guides/resources/dependency-injection.mdx
  • sdks/java/src/main/java/org/byteveda/taskito/resources/ResourceScope.java
  • sdks/node/src/index.ts
  • sdks/node/src/resources/index.ts
  • sdks/node/src/resources/runtime.ts
  • sdks/node/src/resources/types.ts
  • sdks/node/test/resources/resources.test.ts
  • sdks/python/taskito/mixins/resources.py
  • sdks/python/taskito/resources/definition.py
  • sdks/python/taskito/resources/runtime.py
  • sdks/python/tests/resources/test_resource_system_full.py
  • sdks/python/tests/worker/test_lifecycle_parity.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • sdks/node/src/index.ts

Comment thread docs/content/docs/python/guides/resources/configuration.mdx
Comment thread docs/content/docs/python/guides/resources/dependency-injection.mdx
Comment thread sdks/python/taskito/resources/definition.py
Comment thread sdks/python/taskito/resources/runtime.py
An enum/String overload pair is ambiguous on a bare null, so the documented no-filter call stopped compiling; the level filter also has to stay lenient for dashboard query params.
createSession dereferenced a null role, and a persisted session role was handed back as a raw string instead of being coerced like the user row already was.
A wire-string scope matched no runtime branch, reload left pooled resources on a stale pool, and an async task teardown was dropped unawaited.
@pratyush618
pratyush618 self-requested a review July 23, 2026 09:03
pratyush618
pratyush618 previously approved these changes Jul 23, 2026
@pratyush618

Copy link
Copy Markdown
Collaborator

@CodeRabbit full review

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
sdks/python/taskito/resources/runtime.py (1)

188-203: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

recreate() corrupts resolve() for reloadable TASK/THREAD resources.

The new POOLED branch is handled correctly, but TASK and THREAD scoped resources still fall through to the generic old/_create_resource() path. Since TASK/THREAD instances are never stored in self._instances (they live in acquire_for_task's per-call scope or self._thread_locals), this fallthrough calls the factory and writes the result into self._instances[name] — a singleton resolve() will now incorrectly return for a scope that should never be resolvable that way. Reachable whenever a TASK/THREAD-scoped resource has reloadable=True and reload()/recreate() runs.

🐛 Proposed fix
             if defn.scope == ResourceScope.POOLED:
                 self._build_pool(name)
                 self._recreation_count[name] = self._recreation_count.get(name, 0) + 1
                 return True
+            if defn.scope in (ResourceScope.TASK, ResourceScope.THREAD):
+                # Nothing persistent lives in `_instances` for these scopes;
+                # falling through to `_create_resource` would plant a stray
+                # singleton that `resolve()` would then incorrectly serve.
+                self._recreation_count[name] = self._recreation_count.get(name, 0) + 1
+                return True
             if old is not None and defn.teardown is not None:
                 run_maybe_async(defn.teardown(old))
             self._create_resource(name)
🤖 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/python/taskito/resources/runtime.py` around lines 188 - 203, Update
recreate() to handle TASK and THREAD-scoped definitions without routing them
through the generic old/_create_resource() path. After identifying defn.scope,
preserve the POOLED behavior and ensure TASK/THREAD resources are not created or
stored in self._instances, so resolve() remains invalid for those scopes; only
singleton-style resources should use the existing teardown and _create_resource
flow.
🤖 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 `@crates/taskito-node/src/queue/pubsub.rs`:
- Around line 48-50: The public subscription bindings currently use permissive
SubscriptionMode::from_wire parsing, allowing unknown modes to become Fanout. In
crates/taskito-node/src/queue/pubsub.rs lines 48-50, strictly accept only
"fanout" and "log", returning invalid_arg for other supplied values; in
crates/taskito-python/src/py_queue/pubsub.rs line 83, apply the same validation
and raise PyValueError for invalid values. Add binding regression tests covering
unknown modes at both sites.

In `@docs/content/docs/python/api-reference/queue/resources.mdx`:
- Around line 41-43: Update the pool_size row in the queue resources API
reference so its default description states that the default is 4, matching
_build_pool() and the sibling documentation. Leave the type, scope restriction,
and other pool fields unchanged.

In `@sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java`:
- Around line 61-66: Restore deprecated String overloads for createUser and both
createSession variants in AuthStore, delegating role conversion through
Role.fromWire(...) while retaining the Role-based methods as primary. Ensure
invalid string roles produce the existing controlled “invalid role” error, and
update null-role tests to cast null explicitly to Role to resolve overload
ambiguity.

In `@sdks/node/src/queue.ts`:
- Around line 1026-1030: Update queryLogs so its task filter accepts legacy
string values alongside the typed API, preserving compatibility for string-typed
callers. Retain TaskLogLevel typing for typed callers and ensure unrecognized
string filters continue producing no matching logs rather than causing
type-check failures.

In `@sdks/node/src/resources/runtime.ts`:
- Around line 215-230: Update request-scope resolution around the uncached
branch in the resource resolver to track the active dependency ancestry and
reject a resource name already present in that ancestry before invoking
startFactory. Propagate the ancestry through ResourceContext.use and nested
resolve calls so both direct and indirect request-scope cycles fail without
starting another build, while preserving normal independent resolutions and
teardown behavior. Add regression tests covering direct self-resolution and an
indirect cycle through another request resource.

In `@sdks/node/src/resources/types.ts`:
- Around line 7-10: Validate the scope argument at the Queue.resource() and/or
ResourceRuntime.register() registration boundary using the existing
RESOURCE_SCOPES collection. Reject unknown JavaScript scope values by throwing
before registration or lifecycle handling reaches
ResourceRuntime.createTaskScope(), while preserving behavior for all declared
ResourceScope values.

In `@sdks/python/taskito/__init__.py`:
- Around line 44-52: Add ConsumerErrorAction to the public exports in
taskito.__init__: import it from the module that defines _log_consumer’s
consumer error types and include it in __all__ alongside the other re-exported
enums, so typed callers can use the stable package API.

In `@sdks/python/taskito/dashboard/auth.py`:
- Line 402: Guard the decoded session data before the Session construction in
the authentication flow, ensuring only object/dictionary values proceed to
access data.get and create Session. Return None for lists, scalars, or other
non-object decoded values while preserving the existing TypeError handling.

In `@sdks/python/taskito/mixins/decorators.py`:
- Around line 304-305: Update the ValueError description in the decorator
documentation to state that on_false accepts PredicateAction members as well as
the wire strings "defer" and "cancel", while retaining the existing
default_defer_seconds negative-value condition.

In `@sdks/python/taskito/workflows/tracker/gates.py`:
- Around line 54-64: Update on_gate_timeout to accept GateAction | str and
normalize action with coerce_enum(GateAction, action) before the identity check,
preserving legacy "approve" strings as approvals while leaving timer cleanup
behavior unchanged.

In `@sdks/python/tests/python/test_closed_set_enums.py`:
- Around line 27-33: Update test_interception_accepts_enum_and_string to wrap
its assertions in a try/finally block and call close() on both enum_queue and
string_queue in the finally block, ensuring resources are released even when an
assertion fails.

---

Outside diff comments:
In `@sdks/python/taskito/resources/runtime.py`:
- Around line 188-203: Update recreate() to handle TASK and THREAD-scoped
definitions without routing them through the generic old/_create_resource()
path. After identifying defn.scope, preserve the POOLED behavior and ensure
TASK/THREAD resources are not created or stored in self._instances, so resolve()
remains invalid for those scopes; only singleton-style resources should use the
existing teardown and _create_resource flow.
🪄 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: 86497ee2-2d21-4b47-b0f5-fdd4ec468cc4

📥 Commits

Reviewing files that changed from the base of the PR and between 256f7c4 and 6b2e1d4.

📒 Files selected for processing (87)
  • CHANGELOG.md
  • crates/taskito-core/src/pubsub.rs
  • crates/taskito-core/src/storage/diesel_common/pubsub.rs
  • crates/taskito-core/src/storage/diesel_common/workers.rs
  • crates/taskito-core/src/storage/mod.rs
  • crates/taskito-core/src/storage/models.rs
  • crates/taskito-core/src/storage/postgres/pubsub.rs
  • crates/taskito-core/src/storage/records.rs
  • crates/taskito-core/src/storage/redis_backend/pubsub.rs
  • crates/taskito-core/src/storage/redis_backend/workers.rs
  • crates/taskito-core/src/storage/sqlite/pubsub.rs
  • crates/taskito-core/src/storage/sqlite/tests.rs
  • crates/taskito-core/src/storage/traits.rs
  • crates/taskito-core/tests/rust/storage_tests.rs
  • crates/taskito-java/src/convert.rs
  • crates/taskito-java/src/queue/pubsub.rs
  • crates/taskito-java/src/worker.rs
  • crates/taskito-node/src/convert/pubsub.rs
  • crates/taskito-node/src/queue/pubsub.rs
  • crates/taskito-python/src/py_queue/pubsub.rs
  • crates/taskito-python/src/py_queue/worker.rs
  • docs/content/docs/java/guides/resources/dependency-injection.mdx
  • docs/content/docs/node/guides/resources/dependency-injection.mdx
  • docs/content/docs/python/api-reference/queue/index.mdx
  • docs/content/docs/python/api-reference/queue/resources.mdx
  • docs/content/docs/python/guides/resources/configuration.mdx
  • docs/content/docs/python/guides/resources/dependency-injection.mdx
  • docs/content/docs/python/guides/resources/proxies.mdx
  • sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java
  • sdks/java/src/main/java/org/byteveda/taskito/Taskito.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthHandlers.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Role.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Session.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/TokenAuth.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/User.java
  • sdks/java/src/main/java/org/byteveda/taskito/model/DispatchOrder.java
  • sdks/java/src/main/java/org/byteveda/taskito/model/TaskLogLevel.java
  • sdks/java/src/main/java/org/byteveda/taskito/resources/ResourceScope.java
  • sdks/java/src/test/java/org/byteveda/taskito/core/DispatchOrderTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardAuthTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardClient.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardNativeTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/oauth/OAuthFlowTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/model/ClosedSetEnumTest.java
  • sdks/node/src/dashboard/auth/store.ts
  • sdks/node/src/dashboard/handlers/core.ts
  • sdks/node/src/dashboard/index.ts
  • sdks/node/src/index.ts
  • sdks/node/src/queue.ts
  • sdks/node/src/resources/index.ts
  • sdks/node/src/resources/runtime.ts
  • sdks/node/src/resources/types.ts
  • sdks/node/src/types.ts
  • sdks/node/test/resources/resources.test.ts
  • sdks/python/taskito/__init__.py
  • sdks/python/taskito/app.py
  • sdks/python/taskito/dashboard/auth.py
  • sdks/python/taskito/enums.py
  • sdks/python/taskito/interception/__init__.py
  • sdks/python/taskito/interception/interceptor.py
  • sdks/python/taskito/interception/mode.py
  • sdks/python/taskito/mixins/_log_consumer.py
  • sdks/python/taskito/mixins/decorators.py
  • sdks/python/taskito/mixins/predicates.py
  • sdks/python/taskito/mixins/pubsub.py
  • sdks/python/taskito/mixins/resources.py
  • sdks/python/taskito/predicates/__init__.py
  • sdks/python/taskito/predicates/outcomes.py
  • sdks/python/taskito/proxies/__init__.py
  • sdks/python/taskito/proxies/built_in.py
  • sdks/python/taskito/resources/definition.py
  • sdks/python/taskito/resources/runtime.py
  • sdks/python/taskito/workflows/__init__.py
  • sdks/python/taskito/workflows/builder.py
  • sdks/python/taskito/workflows/fan_out.py
  • sdks/python/taskito/workflows/run.py
  • sdks/python/taskito/workflows/tracker/gates.py
  • sdks/python/taskito/workflows/types.py
  • sdks/python/tests/dashboard/test_auth.py
  • sdks/python/tests/python/test_closed_set_enums.py
  • sdks/python/tests/resources/test_proxies.py
  • sdks/python/tests/resources/test_resource_system_full.py
  • sdks/python/tests/worker/test_lifecycle_parity.py

Comment thread crates/taskito-node/src/queue/pubsub.rs Outdated
Comment thread docs/content/docs/python/api-reference/queue/resources.mdx
Comment thread sdks/node/src/queue.ts
Comment thread sdks/node/src/resources/runtime.ts
Comment thread sdks/python/taskito/__init__.py
Comment thread sdks/python/taskito/dashboard/auth.py
Comment thread sdks/python/taskito/mixins/decorators.py Outdated
Comment thread sdks/python/taskito/workflows/tracker/gates.py Outdated
Comment thread sdks/python/tests/python/test_closed_set_enums.py Outdated
The lenient reader exists for persisted rows; caller input must not silently become a fan-out subscription.
AuthStore is public, so making it Role-only broke existing string callers; the wire-form overloads delegate through a strict parse.
An unknown scope from JS fell through to the task branch; a self-referential request resource recursed forever, since that scope is uncached by design.
Typing it to the union broke existing callers and forced the dashboard to cast; an unknown filter must return nothing, not fail.
GateConfig is public, so a legacy "approve" string resolved the timeout as a rejection. Also exports ConsumerErrorAction and rejects a session payload that is not an object.
@pratyush618

Copy link
Copy Markdown
Collaborator

@CodeRabbit full review

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
sdks/python/taskito/resources/runtime.py (1)

193-200: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Prevent TASK/THREAD scopes from falling through to worker recreation.

A reloadable TASK resource reaches _create_resource(), populates _instances, and is then returned by resolve() as a shared singleton—violating its per-task lifecycle. Give non-worker scopes explicit recreation behavior or reject recreation for them.

🤖 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/python/taskito/resources/runtime.py` around lines 193 - 200, Update the
recreation logic around _create_resource so TASK and THREAD-scoped resources do
not fall through to worker-level recreation or become shared entries in
_instances. Add explicit per-scope recreation behavior that preserves their
lifecycle, or reject recreation for these scopes; retain the existing POOLED and
worker-scoped behavior.
🤖 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 19-24: Update the changelog entry describing closed-set parameters
to qualify the Java nullable query-filter exception: caller-produced task-log
levels and workflow states use enums with deprecated String overloads, while
nullable Java filters retain a single wire-form String API. Document that
unknown task-log levels match nothing and unknown workflow states remain
rejected, without changing other listed parameters.

In `@crates/taskito-python/src/py_queue/worker.rs`:
- Around line 837-842: Update set_worker_status to strictly accept only the wire
values "active" and "draining" before calling storage.update_worker_status;
construct the corresponding WorkerStatus directly, and return PyValueError for
any other input instead of using WorkerStatus::from_wire. Preserve the existing
PyRuntimeError mapping for storage failures.

In `@sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Session.java`:
- Line 9: The public Session record is no longer source-compatible because its
role component changed from String to Role. In Session.java at lines 9-9 and
User.java at lines 12-12, retain the existing wire-form String role API (or
explicitly version the Java SDK as a breaking change), ensuring direct
construction and role() consumers remain compatible; update related conversions
only as needed.

In `@sdks/node/src/resources/runtime.ts`:
- Around line 261-267: Update the task-resource resolution flow around the task
cache and ResourceContext construction to pass a dependency chain containing the
current task name when resolving its dependencies, rather than preserving
building unchanged. Ensure mixed-scope cycles such as task a → request b → task
a are detected and rejected with the intended ResourceError, and add coverage
for this reverse mixed-scope cycle.

In `@sdks/python/tests/resources/test_resource_system_full.py`:
- Around line 710-723: Update the test around the ResourceRuntime instance to
wrap initialization, acquisition, reload, and assertions in a try/finally block,
then call runtime.teardown() in finally so the rebuilt pool and instances are
disposed even when assertions fail.

---

Outside diff comments:
In `@sdks/python/taskito/resources/runtime.py`:
- Around line 193-200: Update the recreation logic around _create_resource so
TASK and THREAD-scoped resources do not fall through to worker-level recreation
or become shared entries in _instances. Add explicit per-scope recreation
behavior that preserves their lifecycle, or reject recreation for these scopes;
retain the existing POOLED and worker-scoped behavior.
🪄 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: 3872594c-afc8-4b33-aaa1-ac7fcdc9e9ee

📥 Commits

Reviewing files that changed from the base of the PR and between 256f7c4 and 77b8762.

📒 Files selected for processing (87)
  • CHANGELOG.md
  • crates/taskito-core/src/pubsub.rs
  • crates/taskito-core/src/storage/diesel_common/pubsub.rs
  • crates/taskito-core/src/storage/diesel_common/workers.rs
  • crates/taskito-core/src/storage/mod.rs
  • crates/taskito-core/src/storage/models.rs
  • crates/taskito-core/src/storage/postgres/pubsub.rs
  • crates/taskito-core/src/storage/records.rs
  • crates/taskito-core/src/storage/redis_backend/pubsub.rs
  • crates/taskito-core/src/storage/redis_backend/workers.rs
  • crates/taskito-core/src/storage/sqlite/pubsub.rs
  • crates/taskito-core/src/storage/sqlite/tests.rs
  • crates/taskito-core/src/storage/traits.rs
  • crates/taskito-core/tests/rust/storage_tests.rs
  • crates/taskito-java/src/convert.rs
  • crates/taskito-java/src/queue/pubsub.rs
  • crates/taskito-java/src/worker.rs
  • crates/taskito-node/src/convert/pubsub.rs
  • crates/taskito-node/src/queue/pubsub.rs
  • crates/taskito-python/src/py_queue/pubsub.rs
  • crates/taskito-python/src/py_queue/worker.rs
  • docs/content/docs/java/guides/resources/dependency-injection.mdx
  • docs/content/docs/node/guides/resources/dependency-injection.mdx
  • docs/content/docs/python/api-reference/queue/index.mdx
  • docs/content/docs/python/api-reference/queue/resources.mdx
  • docs/content/docs/python/guides/resources/configuration.mdx
  • docs/content/docs/python/guides/resources/dependency-injection.mdx
  • docs/content/docs/python/guides/resources/proxies.mdx
  • sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java
  • sdks/java/src/main/java/org/byteveda/taskito/Taskito.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthHandlers.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Role.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Session.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/TokenAuth.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/User.java
  • sdks/java/src/main/java/org/byteveda/taskito/model/DispatchOrder.java
  • sdks/java/src/main/java/org/byteveda/taskito/model/TaskLogLevel.java
  • sdks/java/src/main/java/org/byteveda/taskito/resources/ResourceScope.java
  • sdks/java/src/test/java/org/byteveda/taskito/core/DispatchOrderTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardAuthTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardClient.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardNativeTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/oauth/OAuthFlowTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/model/ClosedSetEnumTest.java
  • sdks/node/src/dashboard/auth/store.ts
  • sdks/node/src/dashboard/handlers/core.ts
  • sdks/node/src/dashboard/index.ts
  • sdks/node/src/index.ts
  • sdks/node/src/queue.ts
  • sdks/node/src/resources/index.ts
  • sdks/node/src/resources/runtime.ts
  • sdks/node/src/resources/types.ts
  • sdks/node/src/types.ts
  • sdks/node/test/resources/resources.test.ts
  • sdks/python/taskito/__init__.py
  • sdks/python/taskito/app.py
  • sdks/python/taskito/dashboard/auth.py
  • sdks/python/taskito/enums.py
  • sdks/python/taskito/interception/__init__.py
  • sdks/python/taskito/interception/interceptor.py
  • sdks/python/taskito/interception/mode.py
  • sdks/python/taskito/mixins/_log_consumer.py
  • sdks/python/taskito/mixins/decorators.py
  • sdks/python/taskito/mixins/predicates.py
  • sdks/python/taskito/mixins/pubsub.py
  • sdks/python/taskito/mixins/resources.py
  • sdks/python/taskito/predicates/__init__.py
  • sdks/python/taskito/predicates/outcomes.py
  • sdks/python/taskito/proxies/__init__.py
  • sdks/python/taskito/proxies/built_in.py
  • sdks/python/taskito/resources/definition.py
  • sdks/python/taskito/resources/runtime.py
  • sdks/python/taskito/workflows/__init__.py
  • sdks/python/taskito/workflows/builder.py
  • sdks/python/taskito/workflows/fan_out.py
  • sdks/python/taskito/workflows/run.py
  • sdks/python/taskito/workflows/tracker/gates.py
  • sdks/python/taskito/workflows/types.py
  • sdks/python/tests/dashboard/test_auth.py
  • sdks/python/tests/python/test_closed_set_enums.py
  • sdks/python/tests/resources/test_proxies.py
  • sdks/python/tests/resources/test_resource_system_full.py
  • sdks/python/tests/worker/test_lifecycle_parity.py

Comment thread CHANGELOG.md
Comment thread crates/taskito-python/src/py_queue/worker.rs Outdated
Comment thread sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Session.java Outdated
Comment thread sdks/node/src/resources/runtime.ts Outdated
Comment thread sdks/python/tests/resources/test_resource_system_full.py Outdated
Coercing caller input to Active would silently un-drain a worker; the lenient reader stays for persisted rows.
A task resource in a cycle handed back its own pending promise, so the build awaited itself forever instead of failing.
A Java enum is not a String, so typing the record accessors broke every existing role() caller — unlike Python (str subclass) and Node (string union), where the typed role stays assignable. The enum keeps validation and the authorization decision.
@pratyush618

Copy link
Copy Markdown
Collaborator

@CodeRabbit full review

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
sdks/python/taskito/workflows/run.py (1)

120-135: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Validate fmt before reading workflow state.

visualize() currently fetches the DAG and status before coercing fmt, so an invalid format can surface an unrelated backend or missing-run error instead of the documented ValueError.

Proposed fix
     def visualize(self, fmt: DiagramFormat | str = DiagramFormat.MERMAID) -> str:
         """Render the workflow DAG with live node statuses.
@@
         """
+        diagram_format = coerce_enum(DiagramFormat, fmt, param="fmt")
         dag_bytes = self._queue._inner.get_workflow_definition_dag(self.id)
         nodes, edges = nodes_and_edges_from_dag_bytes(dag_bytes)
@@
-        if coerce_enum(DiagramFormat, fmt, param="fmt") is DiagramFormat.DOT:
+        if diagram_format is DiagramFormat.DOT:
             return render_dot(nodes, edges, statuses)
🤖 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/python/taskito/workflows/run.py` around lines 120 - 135, Update
visualize() to coerce and validate fmt before calling
get_workflow_definition_dag or status(), so invalid formats raise the documented
ValueError without reading workflow state; reuse the validated DiagramFormat
value for the subsequent rendering logic.
crates/taskito-core/src/pubsub.rs (1)

16-24: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Debug-formatting the enum leaks the Rust variant name into a user-facing error.

{mode:?} on SubscriptionMode now renders the bare derived-Debug variant name ("Fanout"/"Log"), not the wire string. Previously mode: &str made this message show the caller's quoted wire value (e.g. got "fanout"). This error propagates to end users through the Node/Python/Java declare_topic bindings, so callers now see "got Fanout" instead of the wire text they passed. mode already has a Display impl backed by as_str() — use it.

🐛 Proposed fix
     if !mode.is_log() {
         return Err(QueueError::Config(format!(
-            "only \"log\" topics are declarable, got {mode:?}"
+            "only \"log\" topics are declarable, got \"{mode}\""
         )));
     }

For enums, it will use the name of the variant and, if applicable, (, then the Debug values of the fields, then ). Derived Debug formats are not stable across Rust versions, confirming the plain variant-name output.

🤖 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 `@crates/taskito-core/src/pubsub.rs` around lines 16 - 24, Update the error
formatting in validate_topic_declaration to use SubscriptionMode’s Display
implementation instead of Debug formatting for mode. Preserve the existing
message structure so the reported value remains the caller-facing wire string,
such as "fanout" or "log".
🧹 Nitpick comments (1)
sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java (1)

61-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Mark legacy string role overloads as deprecated.

Lines 61 and 211-219 retain the wire-form compatibility APIs, but callers receive no migration signal. Add @Deprecated to all three String role overloads while retaining their strict parsing behavior.

Proposed fix
+    `@Deprecated`
     public synchronized User createUser(String username, String password, String role) {

+    `@Deprecated`
     public Session createSession(String username, String role) {

+    `@Deprecated`
     public Session createSession(String username, String role, long ttlSeconds) {

Based on learnings: keep deprecated wire-form String overloads for backward compatibility.

Also applies to: 211-219

🤖 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/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java`
around lines 61 - 63, Mark all three AuthStore createUser overloads that accept
a String role, including the overload near createUser(String username, String
password, String role), with `@Deprecated` while preserving their existing
parseRole-based strict parsing and compatibility behavior.

Source: Learnings

🤖 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 `@sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java`:
- Around line 223-226: Validate the enum parameters explicitly in the typed
dispatchOrder overloads and the writeTaskLog(..., extra) overload before calling
wire(). Reject null order or level using the API’s established documented
argument-validation exception, while preserving existing behavior for non-null
values.

In `@sdks/python/taskito/resources/runtime.py`:
- Around line 26-31: Update the ResourceScope documentation in the runtime
resource manager to replace stale “request-scoped” terminology with
“task-scoped,” including the lifecycle class docstring and the resolve()
docstring. Ensure the wording consistently describes TASK resources as created
fresh per task and does not reference the removed REQUEST scope.

---

Outside diff comments:
In `@crates/taskito-core/src/pubsub.rs`:
- Around line 16-24: Update the error formatting in validate_topic_declaration
to use SubscriptionMode’s Display implementation instead of Debug formatting for
mode. Preserve the existing message structure so the reported value remains the
caller-facing wire string, such as "fanout" or "log".

In `@sdks/python/taskito/workflows/run.py`:
- Around line 120-135: Update visualize() to coerce and validate fmt before
calling get_workflow_definition_dag or status(), so invalid formats raise the
documented ValueError without reading workflow state; reuse the validated
DiagramFormat value for the subsequent rendering logic.

---

Nitpick comments:
In `@sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java`:
- Around line 61-63: Mark all three AuthStore createUser overloads that accept a
String role, including the overload near createUser(String username, String
password, String role), with `@Deprecated` while preserving their existing
parseRole-based strict parsing and compatibility behavior.
🪄 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: 766cde90-b6b3-4816-849a-54b6459559ae

📥 Commits

Reviewing files that changed from the base of the PR and between 256f7c4 and d5b86fb.

📒 Files selected for processing (85)
  • CHANGELOG.md
  • crates/taskito-core/src/pubsub.rs
  • crates/taskito-core/src/storage/diesel_common/pubsub.rs
  • crates/taskito-core/src/storage/diesel_common/workers.rs
  • crates/taskito-core/src/storage/mod.rs
  • crates/taskito-core/src/storage/models.rs
  • crates/taskito-core/src/storage/postgres/pubsub.rs
  • crates/taskito-core/src/storage/records.rs
  • crates/taskito-core/src/storage/redis_backend/pubsub.rs
  • crates/taskito-core/src/storage/redis_backend/workers.rs
  • crates/taskito-core/src/storage/sqlite/pubsub.rs
  • crates/taskito-core/src/storage/sqlite/tests.rs
  • crates/taskito-core/src/storage/traits.rs
  • crates/taskito-core/tests/rust/storage_tests.rs
  • crates/taskito-java/src/convert.rs
  • crates/taskito-java/src/queue/pubsub.rs
  • crates/taskito-java/src/worker.rs
  • crates/taskito-node/src/convert/pubsub.rs
  • crates/taskito-node/src/queue/pubsub.rs
  • crates/taskito-python/src/py_queue/pubsub.rs
  • crates/taskito-python/src/py_queue/worker.rs
  • docs/content/docs/java/guides/resources/dependency-injection.mdx
  • docs/content/docs/node/guides/resources/dependency-injection.mdx
  • docs/content/docs/python/api-reference/queue/index.mdx
  • docs/content/docs/python/api-reference/queue/resources.mdx
  • docs/content/docs/python/guides/resources/configuration.mdx
  • docs/content/docs/python/guides/resources/dependency-injection.mdx
  • docs/content/docs/python/guides/resources/proxies.mdx
  • sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java
  • sdks/java/src/main/java/org/byteveda/taskito/Taskito.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthHandlers.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Role.java
  • sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/TokenAuth.java
  • sdks/java/src/main/java/org/byteveda/taskito/model/DispatchOrder.java
  • sdks/java/src/main/java/org/byteveda/taskito/model/TaskLogLevel.java
  • sdks/java/src/main/java/org/byteveda/taskito/resources/ResourceScope.java
  • sdks/java/src/test/java/org/byteveda/taskito/core/DispatchOrderTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardAuthTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardClient.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardNativeTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/dashboard/oauth/OAuthFlowTest.java
  • sdks/java/src/test/java/org/byteveda/taskito/model/ClosedSetEnumTest.java
  • sdks/node/src/dashboard/auth/store.ts
  • sdks/node/src/dashboard/handlers/core.ts
  • sdks/node/src/dashboard/index.ts
  • sdks/node/src/index.ts
  • sdks/node/src/queue.ts
  • sdks/node/src/resources/index.ts
  • sdks/node/src/resources/runtime.ts
  • sdks/node/src/resources/types.ts
  • sdks/node/src/types.ts
  • sdks/node/test/resources/resources.test.ts
  • sdks/python/taskito/__init__.py
  • sdks/python/taskito/app.py
  • sdks/python/taskito/dashboard/auth.py
  • sdks/python/taskito/enums.py
  • sdks/python/taskito/interception/__init__.py
  • sdks/python/taskito/interception/interceptor.py
  • sdks/python/taskito/interception/mode.py
  • sdks/python/taskito/mixins/_log_consumer.py
  • sdks/python/taskito/mixins/decorators.py
  • sdks/python/taskito/mixins/predicates.py
  • sdks/python/taskito/mixins/pubsub.py
  • sdks/python/taskito/mixins/resources.py
  • sdks/python/taskito/predicates/__init__.py
  • sdks/python/taskito/predicates/outcomes.py
  • sdks/python/taskito/proxies/__init__.py
  • sdks/python/taskito/proxies/built_in.py
  • sdks/python/taskito/resources/definition.py
  • sdks/python/taskito/resources/runtime.py
  • sdks/python/taskito/workflows/__init__.py
  • sdks/python/taskito/workflows/builder.py
  • sdks/python/taskito/workflows/fan_out.py
  • sdks/python/taskito/workflows/run.py
  • sdks/python/taskito/workflows/tracker/gates.py
  • sdks/python/taskito/workflows/types.py
  • sdks/python/tests/dashboard/test_auth.py
  • sdks/python/tests/python/test_closed_set_enums.py
  • sdks/python/tests/resources/test_proxies.py
  • sdks/python/tests/resources/test_resource_system_full.py
  • sdks/python/tests/worker/test_lifecycle_parity.py

Comment thread sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java
Comment thread sdks/python/taskito/resources/runtime.py
They dereferenced the argument, so a null surfaced as an incidental NPE rather than the documented IllegalArgumentException. Also drops the stale REQUEST wording left in the resource runtime docstrings.
@pratyush618
pratyush618 merged commit 3e1ba3c into master Jul 23, 2026
56 of 58 checks passed
@pratyush618
pratyush618 deleted the feat/closed-set-enums branch July 23, 2026 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants