From 9fef19f9d962f8fcbc70cba9bdc0d2a481414975 Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:38:59 +0530 Subject: [PATCH 01/22] feat(python): type the built-in proxy ids disabled_proxies took bare strings, so a typo silently left the handler registered. --- .../docs/python/api-reference/queue/index.mdx | 4 +- .../docs/python/guides/resources/proxies.mdx | 9 ++++- sdks/python/taskito/enums.py | 22 +++++++++++ sdks/python/taskito/proxies/__init__.py | 2 + sdks/python/taskito/proxies/built_in.py | 37 ++++++++++++++----- sdks/python/tests/resources/test_proxies.py | 23 +++++++++++- 6 files changed, 83 insertions(+), 14 deletions(-) create mode 100644 sdks/python/taskito/enums.py diff --git a/docs/content/docs/python/api-reference/queue/index.mdx b/docs/content/docs/python/api-reference/queue/index.mdx index a3a829cf..858b991d 100644 --- a/docs/content/docs/python/api-reference/queue/index.mdx +++ b/docs/content/docs/python/api-reference/queue/index.mdx @@ -34,7 +34,7 @@ Queue( recipe_signing_key: str | None = None, max_reconstruction_timeout: int = 10, file_path_allowlist: list[str] | None = None, - disabled_proxies: list[str] | None = None, + disabled_proxies: Sequence[BuiltInProxy | str] | None = None, async_concurrency: int = 100, event_workers: int = 4, scheduler_poll_interval_ms: int = 50, @@ -60,7 +60,7 @@ Queue( | `recipe_signing_key` | `str \| None` | `None` | HMAC-SHA256 key for proxy recipe integrity. Falls back to `TASKITO_RECIPE_SECRET` env var. | | `max_reconstruction_timeout` | `int` | `10` | Max seconds allowed for proxy reconstruction. | | `file_path_allowlist` | `list[str] \| None` | `None` | Allowed file path prefixes for the file proxy handler. | -| `disabled_proxies` | `list[str] \| None` | `None` | Handler names to skip when registering built-in proxy handlers. | +| `disabled_proxies` | `Sequence[BuiltInProxy \| str] \| None` | `None` | Built-in proxy handlers to skip registering. An unknown id raises rather than silently leaving the handler registered. | | `async_concurrency` | `int` | `100` | Maximum number of `async def` tasks running concurrently on the native async executor. | | `event_workers` | `int` | `4` | Thread pool size for the event bus. Increase for high event volume. | | `scheduler_poll_interval_ms` | `int` | `50` | Milliseconds between scheduler poll cycles. Lower values improve scheduling precision at the cost of CPU. | diff --git a/docs/content/docs/python/guides/resources/proxies.mdx b/docs/content/docs/python/guides/resources/proxies.mdx index d099bdbc..6284bcfe 100644 --- a/docs/content/docs/python/guides/resources/proxies.mdx +++ b/docs/content/docs/python/guides/resources/proxies.mdx @@ -90,12 +90,14 @@ reconstruction. Without an allowlist, any path is permitted. ### Disabling specific handlers -Disable individual handlers by name: +Disable individual handlers by id — `BuiltInProxy` members or their strings: ```python +from taskito import BuiltInProxy, Queue + queue = Queue( db_path="tasks.db", - disabled_proxies=["requests_session", "gcs_client"], + disabled_proxies=[BuiltInProxy.REQUESTS_SESSION, BuiltInProxy.GCS_CLIENT], ) ``` @@ -103,6 +105,9 @@ Disabled handlers are not registered. Arguments of those types fall through to the serializer — or are rejected if they would otherwise be PROXY-classified and interception is strict. +An id that isn't a built-in raises `ValueError` listing the valid ones, so a +typo can't quietly leave the handler enabled. + ## Cloud handlers ### AWS (boto3) diff --git a/sdks/python/taskito/enums.py b/sdks/python/taskito/enums.py new file mode 100644 index 00000000..b1193a81 --- /dev/null +++ b/sdks/python/taskito/enums.py @@ -0,0 +1,22 @@ +"""Coercion helper for the closed-set enums on public entry points.""" + +from __future__ import annotations + +import enum +from typing import TypeVar + +E = TypeVar("E", bound=enum.Enum) + + +def coerce_enum(enum_cls: type[E], value: E | str, *, param: str) -> E: + """Accept an enum member or its wire string, else raise naming the valid set. + + Public entry points keep taking plain strings, so a typo would otherwise + surface as a bare ``'x' is not a valid Y`` — or, where nothing validated, + not at all. + """ + try: + return enum_cls(value) + except ValueError: + valid = ", ".join(repr(member.value) for member in enum_cls) + raise ValueError(f"{param} must be one of {valid}, got {value!r}") from None diff --git a/sdks/python/taskito/proxies/__init__.py b/sdks/python/taskito/proxies/__init__.py index a477f485..ee6df73e 100644 --- a/sdks/python/taskito/proxies/__init__.py +++ b/sdks/python/taskito/proxies/__init__.py @@ -1,11 +1,13 @@ """Resource proxies — transparent deconstruction and reconstruction of objects.""" +from taskito.proxies.built_in import BuiltInProxy from taskito.proxies.handler import ProxyHandler from taskito.proxies.no_proxy import NoProxy from taskito.proxies.reconstruct import cleanup_proxies, reconstruct_proxies from taskito.proxies.registry import ProxyRegistry __all__ = [ + "BuiltInProxy", "NoProxy", "ProxyHandler", "ProxyRegistry", diff --git a/sdks/python/taskito/proxies/built_in.py b/sdks/python/taskito/proxies/built_in.py index cf532073..d3957363 100644 --- a/sdks/python/taskito/proxies/built_in.py +++ b/sdks/python/taskito/proxies/built_in.py @@ -2,33 +2,52 @@ from __future__ import annotations +import enum +from collections.abc import Sequence + +from taskito.enums import coerce_enum from taskito.proxies.handlers.file import FileHandler from taskito.proxies.handlers.logger import LoggerHandler from taskito.proxies.registry import ProxyRegistry +class BuiltInProxy(str, enum.Enum): + """Ids of the proxy handlers registered by default, for ``disabled_proxies``.""" + + FILE = "file" + LOGGER = "logger" + REQUESTS_SESSION = "requests_session" + HTTPX_CLIENT = "httpx_client" + BOTO3_CLIENT = "boto3_client" + GCS_CLIENT = "gcs_client" + + def register_builtin_handlers( registry: ProxyRegistry, *, - disabled_proxies: list[str] | None = None, + disabled_proxies: Sequence[BuiltInProxy | str] | None = None, file_path_allowlist: list[str] | None = None, ) -> None: """Register all built-in proxy handlers into the given registry. Args: registry: The proxy registry to register handlers with. - disabled_proxies: Handler names to skip registration for. + disabled_proxies: Handlers to skip registration for. An unknown id + raises rather than silently leaving the handler registered. file_path_allowlist: Allowed paths for the file handler. """ - disabled = set(disabled_proxies or []) + disabled = { + coerce_enum(BuiltInProxy, proxy, param="disabled_proxies entry") + for proxy in disabled_proxies or () + } - if "file" not in disabled: + if BuiltInProxy.FILE not in disabled: registry.register(FileHandler(path_allowlist=file_path_allowlist or [])) - if "logger" not in disabled: + if BuiltInProxy.LOGGER not in disabled: registry.register(LoggerHandler()) # Optional dependencies - if "requests_session" not in disabled: + if BuiltInProxy.REQUESTS_SESSION not in disabled: try: from taskito.proxies.handlers.requests_session import ( _HAS_REQUESTS, @@ -40,7 +59,7 @@ def register_builtin_handlers( except ImportError: pass - if "httpx_client" not in disabled: + if BuiltInProxy.HTTPX_CLIENT not in disabled: try: from taskito.proxies.handlers.httpx_client import ( _HAS_HTTPX, @@ -52,7 +71,7 @@ def register_builtin_handlers( except ImportError: pass - if "boto3_client" not in disabled: + if BuiltInProxy.BOTO3_CLIENT not in disabled: try: from taskito.proxies.handlers.boto3_client import ( _HAS_BOTO3, @@ -64,7 +83,7 @@ def register_builtin_handlers( except ImportError: pass - if "gcs_client" not in disabled: + if BuiltInProxy.GCS_CLIENT not in disabled: try: from taskito.proxies.handlers.gcs_client import ( _HAS_GCS, diff --git a/sdks/python/tests/resources/test_proxies.py b/sdks/python/tests/resources/test_proxies.py index 30c848a0..5c01beec 100644 --- a/sdks/python/tests/resources/test_proxies.py +++ b/sdks/python/tests/resources/test_proxies.py @@ -9,7 +9,7 @@ import pytest from taskito import ProxyReconstructionError, Queue -from taskito.proxies import ProxyRegistry, cleanup_proxies, reconstruct_proxies +from taskito.proxies import BuiltInProxy, ProxyRegistry, cleanup_proxies, reconstruct_proxies from taskito.proxies.built_in import register_builtin_handlers from taskito.proxies.handlers.file import FileHandler from taskito.proxies.handlers.logger import LoggerHandler @@ -500,3 +500,24 @@ def test_file_handler_rejects_sibling_prefix_bypass(tmp_path: Any) -> None: recipe = {"path": str(secret), "mode": "r"} with pytest.raises(ProxyReconstructionError, match="not in the allowed"): handler.reconstruct(recipe, 1) + + +class TestDisabledProxies: + """``disabled_proxies`` ids are a closed set — a typo must not pass silently.""" + + def test_enum_member_disables_handler(self) -> None: + registry = ProxyRegistry() + register_builtin_handlers(registry, disabled_proxies=[BuiltInProxy.LOGGER]) + assert registry.get("logger") is None + assert registry.get("file") is not None + + def test_wire_string_still_disables_handler(self) -> None: + """Existing string callers keep working.""" + registry = ProxyRegistry() + register_builtin_handlers(registry, disabled_proxies=["logger"]) + assert registry.get("logger") is None + + def test_unknown_id_raises_instead_of_silently_registering(self) -> None: + """The bug this replaces: 'requests' left requests_session registered.""" + with pytest.raises(ValueError, match="requests_session"): + register_builtin_handlers(ProxyRegistry(), disabled_proxies=["requests"]) From a6700cc2b62f6be8f23738295f3ec2acc9cd65c2 Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:39:24 +0530 Subject: [PATCH 02/22] feat(java): take enums for dispatch order and log level Node gains the matching DispatchOrder and TaskLogLevel unions. The string overloads stay, deprecated. --- .../org/byteveda/taskito/DefaultTaskito.java | 50 ++++++++++++++++--- .../java/org/byteveda/taskito/Taskito.java | 50 +++++++++++++++++-- .../taskito/dashboard/api/CoreHandlers.java | 3 ++ .../dashboard/api/WorkflowsHandlers.java | 3 ++ .../byteveda/taskito/model/DispatchOrder.java | 32 ++++++++++++ .../byteveda/taskito/model/TaskLogLevel.java | 38 ++++++++++++++ .../taskito/core/DispatchOrderTest.java | 4 +- .../org/byteveda/taskito/core/QueueTest.java | 3 +- .../dashboard/DashboardNativeTest.java | 3 +- .../taskito/model/ClosedSetEnumTest.java | 33 ++++++++++++ sdks/node/src/dashboard/handlers/core.ts | 5 +- sdks/node/src/queue.ts | 5 +- sdks/node/src/types.ts | 29 ++++++++++- 13 files changed, 242 insertions(+), 16 deletions(-) create mode 100644 sdks/java/src/main/java/org/byteveda/taskito/model/DispatchOrder.java create mode 100644 sdks/java/src/main/java/org/byteveda/taskito/model/TaskLogLevel.java create mode 100644 sdks/java/src/test/java/org/byteveda/taskito/model/ClosedSetEnumTest.java diff --git a/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java b/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java index a0da48a5..4a6ca72a 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java @@ -35,6 +35,7 @@ import org.byteveda.taskito.middleware.Middleware; import org.byteveda.taskito.model.CircuitBreakerState; import org.byteveda.taskito.model.DeadJob; +import org.byteveda.taskito.model.DispatchOrder; import org.byteveda.taskito.model.EffectiveRetention; import org.byteveda.taskito.model.Job; import org.byteveda.taskito.model.JobDag; @@ -46,6 +47,7 @@ import org.byteveda.taskito.model.ReplayEntry; import org.byteveda.taskito.model.Subscription; import org.byteveda.taskito.model.TaskLog; +import org.byteveda.taskito.model.TaskLogLevel; import org.byteveda.taskito.model.TaskMetric; import org.byteveda.taskito.model.Topic; import org.byteveda.taskito.model.TopicLogStat; @@ -79,6 +81,7 @@ import org.byteveda.taskito.workflows.Step; import org.byteveda.taskito.workflows.Workflow; import org.byteveda.taskito.workflows.WorkflowRun; +import org.byteveda.taskito.workflows.WorkflowState; import org.byteveda.taskito.workflows.WorkflowStatus; /** @@ -218,12 +221,20 @@ public Taskito codel(String queue, long targetMs, long intervalMs) { } @Override + public Taskito dispatchOrder(String queue, DispatchOrder order) { + dispatchOrders.put(queue, order.wire()); + return this; + } + + @Override + @Deprecated public Taskito dispatchOrder(String queue, String order) { + // Keeps throwing IllegalArgumentException rather than the model enum's + // SerializationException: callers of this overload predate the enum. if (!"fifo".equals(order) && !"lifo".equals(order)) { throw new IllegalArgumentException("order must be 'fifo' or 'lifo'"); } - dispatchOrders.put(queue, order); - return this; + return dispatchOrder(queue, DispatchOrder.fromWire(order)); } /** @@ -701,13 +712,25 @@ public Map listSettings() { // ── Logs ──────────────────────────────────────────────────────── @Override + public void writeTaskLog(String jobId, String taskName, TaskLogLevel level, String message) { + backend.writeTaskLog(jobId, taskName, level.wire(), message, null); + } + + @Override + public void writeTaskLog(String jobId, String taskName, TaskLogLevel level, String message, String extra) { + backend.writeTaskLog(jobId, taskName, level.wire(), message, extra); + } + + @Override + @Deprecated public void writeTaskLog(String jobId, String taskName, String level, String message) { - backend.writeTaskLog(jobId, taskName, level, message, null); + writeTaskLog(jobId, taskName, TaskLogLevel.fromWire(level), message); } @Override + @Deprecated public void writeTaskLog(String jobId, String taskName, String level, String message, String extra) { - backend.writeTaskLog(jobId, taskName, level, message, extra); + writeTaskLog(jobId, taskName, TaskLogLevel.fromWire(level), message, extra); } @Override @@ -721,8 +744,15 @@ public List getTaskLogsAfter(String jobId, String afterId) { } @Override + public List queryTaskLogs(String taskName, TaskLogLevel level, long sinceMs, long limit) { + String wire = level == null ? null : level.wire(); + return decodeList(backend.queryTaskLogsJson(taskName, wire, sinceMs, limit), TaskLog.class); + } + + @Override + @Deprecated public List queryTaskLogs(String taskName, String level, long sinceMs, long limit) { - return decodeList(backend.queryTaskLogsJson(taskName, level, sinceMs, limit), TaskLog.class); + return queryTaskLogs(taskName, level == null ? null : TaskLogLevel.fromWire(level), sinceMs, limit); } @Override @@ -1145,8 +1175,16 @@ public void cancelWorkflow(String runId) { } @Override + public List listWorkflowRuns( + String definitionName, WorkflowState state, long limit, long offset) { + String wire = state == null ? null : state.wire(); + return decodeList(backend.listWorkflowRunsJson(definitionName, wire, limit, offset), WorkflowRunInfo.class); + } + + @Override + @Deprecated public List listWorkflowRuns(String definitionName, String state, long limit, long offset) { - return decodeList(backend.listWorkflowRunsJson(definitionName, state, limit, offset), WorkflowRunInfo.class); + return listWorkflowRuns(definitionName, state == null ? null : WorkflowState.fromWire(state), limit, offset); } @Override diff --git a/sdks/java/src/main/java/org/byteveda/taskito/Taskito.java b/sdks/java/src/main/java/org/byteveda/taskito/Taskito.java index 4ac40d68..62082eae 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/Taskito.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/Taskito.java @@ -24,6 +24,7 @@ import org.byteveda.taskito.middleware.Middleware; import org.byteveda.taskito.model.CircuitBreakerState; import org.byteveda.taskito.model.DeadJob; +import org.byteveda.taskito.model.DispatchOrder; import org.byteveda.taskito.model.EffectiveRetention; import org.byteveda.taskito.model.Job; import org.byteveda.taskito.model.JobDag; @@ -35,6 +36,7 @@ import org.byteveda.taskito.model.ReplayEntry; import org.byteveda.taskito.model.Subscription; import org.byteveda.taskito.model.TaskLog; +import org.byteveda.taskito.model.TaskLogLevel; import org.byteveda.taskito.model.TaskMetric; import org.byteveda.taskito.model.Topic; import org.byteveda.taskito.model.TopicLogStat; @@ -61,6 +63,7 @@ import org.byteveda.taskito.worker.Worker; import org.byteveda.taskito.workflows.Workflow; import org.byteveda.taskito.workflows.WorkflowRun; +import org.byteveda.taskito.workflows.WorkflowState; import org.byteveda.taskito.workflows.WorkflowStatus; /** @@ -146,12 +149,21 @@ static Builder builder() { Taskito codel(String queue, long targetMs, long intervalMs); /** - * Set a queue's same-priority dispatch order. {@code "lifo"} runs - * newest-first under overload (a freshness lever); {@code "fifo"} (default) - * is the fair oldest-first ordering. Priority always dominates. Honored on + * Set a queue's same-priority dispatch order. {@link DispatchOrder#LIFO} runs + * newest-first under overload (a freshness lever); {@link DispatchOrder#FIFO} + * (default) is the fair oldest-first ordering. Priority always dominates. Honored on * SQLite/Postgres; the Redis backend is FIFO-only. Takes effect for workers * started after this call. Returns {@code this}. */ + Taskito dispatchOrder(String queue, DispatchOrder order); + + /** + * Set a queue's dispatch order from its wire form ({@code "fifo"}/{@code "lifo"}). + * + * @deprecated use {@link #dispatchOrder(String, DispatchOrder)}, which rejects a typo + * at compile time. + */ + @Deprecated Taskito dispatchOrder(String queue, String order); // ── Producer ──────────────────────────────────────────────────── @@ -331,8 +343,24 @@ static Builder builder() { // ── Logs ──────────────────────────────────────────────────────── + void writeTaskLog(String jobId, String taskName, TaskLogLevel level, String message); + + void writeTaskLog(String jobId, String taskName, TaskLogLevel level, String message, String extra); + + /** + * Write a task log at a wire-form level. + * + * @deprecated use {@link #writeTaskLog(String, String, TaskLogLevel, String)}. + */ + @Deprecated void writeTaskLog(String jobId, String taskName, String level, String message); + /** + * Write a task log at a wire-form level, with an extra JSON blob. + * + * @deprecated use {@link #writeTaskLog(String, String, TaskLogLevel, String, String)}. + */ + @Deprecated void writeTaskLog(String jobId, String taskName, String level, String message, String extra); List getTaskLogs(String jobId); @@ -341,6 +369,14 @@ static Builder builder() { List getTaskLogsAfter(String jobId, String afterId); /** Logs across jobs filtered by task/level, at or after {@code sinceMs}, capped at {@code limit}. */ + List queryTaskLogs(String taskName, TaskLogLevel level, long sinceMs, long limit); + + /** + * Logs across jobs filtered by a wire-form level. + * + * @deprecated use {@link #queryTaskLogs(String, TaskLogLevel, long, long)}. + */ + @Deprecated List queryTaskLogs(String taskName, String level, long sinceMs, long limit); // ── Locks ─────────────────────────────────────────────────────── @@ -551,6 +587,14 @@ Taskito logConsumer( void cancelWorkflow(String runId); /** Workflow run summaries, filtered by definition name and/or state, paged. Nulls mean no filter. */ + List listWorkflowRuns(String definitionName, WorkflowState state, long limit, long offset); + + /** + * Workflow run summaries filtered by a wire-form state. + * + * @deprecated use {@link #listWorkflowRuns(String, WorkflowState, long, long)}. + */ + @Deprecated List listWorkflowRuns(String definitionName, String state, long limit, long offset); /** A single workflow run summary, or empty if the run no longer exists. */ diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/CoreHandlers.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/CoreHandlers.java index d547af0b..10d68d42 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/CoreHandlers.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/CoreHandlers.java @@ -89,6 +89,9 @@ public Object resume(String name) { } /** Logs across jobs filtered by task/level; {@code since} is a lookback in seconds. */ + // Deliberately the string overload: an unknown ?level= is untrusted input that + // must filter to nothing, not fail the request. + @SuppressWarnings("deprecation") public Object logs(Map query) { long sinceSeconds = Http.longParam(query, "since", 3600); long sinceMs = System.currentTimeMillis() - sinceSeconds * 1000; diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/WorkflowsHandlers.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/WorkflowsHandlers.java index f8f1f381..fc3d8964 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/WorkflowsHandlers.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/WorkflowsHandlers.java @@ -25,6 +25,9 @@ public WorkflowsHandlers(Taskito queue) { this.queue = queue; } + // Deliberately the string overload: an unknown ?state= is untrusted input that + // must filter to nothing, not fail the request. + @SuppressWarnings("deprecation") public Object runs(Map query) { long limit = Http.longParam(query, "limit", DEFAULT_LIMIT); long offset = Http.longParam(query, "offset", 0); diff --git a/sdks/java/src/main/java/org/byteveda/taskito/model/DispatchOrder.java b/sdks/java/src/main/java/org/byteveda/taskito/model/DispatchOrder.java new file mode 100644 index 00000000..7d0963b7 --- /dev/null +++ b/sdks/java/src/main/java/org/byteveda/taskito/model/DispatchOrder.java @@ -0,0 +1,32 @@ +package org.byteveda.taskito.model; + +import java.util.Locale; +import org.byteveda.taskito.errors.SerializationException; + +/** + * Order in which same-priority jobs leave a queue. Priority always dominates; this only + * breaks ties. Wire form is the lowercase name, shared across SDKs. + */ +public enum DispatchOrder { + /** Oldest-first — the fair default a durable queue is expected to keep. */ + FIFO, + /** Newest-first — a freshness lever for same-priority ties under load. */ + LIFO; + + /** Lowercase wire form passed to the native layer. */ + public String wire() { + return name().toLowerCase(Locale.ROOT); + } + + /** Parse a wire form ({@code "fifo"}/{@code "lifo"}). */ + public static DispatchOrder fromWire(String wire) { + if (wire == null) { + throw new SerializationException("dispatch order is null"); + } + try { + return valueOf(wire.toUpperCase(Locale.ROOT)); + } catch (IllegalArgumentException e) { + throw new SerializationException("unknown dispatch order: " + wire, e); + } + } +} diff --git a/sdks/java/src/main/java/org/byteveda/taskito/model/TaskLogLevel.java b/sdks/java/src/main/java/org/byteveda/taskito/model/TaskLogLevel.java new file mode 100644 index 00000000..29062a2c --- /dev/null +++ b/sdks/java/src/main/java/org/byteveda/taskito/model/TaskLogLevel.java @@ -0,0 +1,38 @@ +package org.byteveda.taskito.model; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonValue; +import java.util.Locale; +import org.byteveda.taskito.errors.SerializationException; + +/** + * Severity of a structured task log. Distinct from the SDK's own logger level: this one + * is written to storage and read back by every SDK, so its wire form — the lowercase + * name — is part of the cross-SDK contract. + */ +public enum TaskLogLevel { + DEBUG, + INFO, + WARNING, + ERROR, + CRITICAL, + /** Partial results published from a running task, not a severity. */ + RESULT; + + @JsonValue + public String wire() { + return name().toLowerCase(Locale.ROOT); + } + + @JsonCreator + public static TaskLogLevel fromWire(String wire) { + if (wire == null) { + throw new SerializationException("task log level is null"); + } + try { + return valueOf(wire.toUpperCase(Locale.ROOT)); + } catch (IllegalArgumentException e) { + throw new SerializationException("unknown task log level: " + wire, e); + } + } +} diff --git a/sdks/java/src/test/java/org/byteveda/taskito/core/DispatchOrderTest.java b/sdks/java/src/test/java/org/byteveda/taskito/core/DispatchOrderTest.java index e96fc206..ad7a5584 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/core/DispatchOrderTest.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/core/DispatchOrderTest.java @@ -8,6 +8,7 @@ import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import org.byteveda.taskito.Taskito; +import org.byteveda.taskito.model.DispatchOrder; import org.byteveda.taskito.task.Task; import org.byteveda.taskito.worker.Worker; import org.junit.jupiter.api.Test; @@ -23,6 +24,7 @@ class DispatchOrderTest { @Test + @SuppressWarnings("deprecation") void validatesOrder(@TempDir Path dir) { try (Taskito queue = Taskito.builder().url(dir.resolve("v.db").toString()).open()) { @@ -54,7 +56,7 @@ private static List runAndRecord(Taskito queue, int total) throws Excep void lifoDispatchesNewestFirst(@TempDir Path dir) throws Exception { try (Taskito queue = Taskito.builder().url(dir.resolve("lifo.db").toString()).open()) { - queue.dispatchOrder("default", "lifo"); + queue.dispatchOrder("default", DispatchOrder.LIFO); List order = runAndRecord(queue, 6); assertEquals(List.of(5, 4, 3, 2, 1, 0), order, "LIFO runs newest-first"); } diff --git a/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java b/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java index c2e28e2d..b17f23fc 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java @@ -17,6 +17,7 @@ import org.byteveda.taskito.model.Job; import org.byteveda.taskito.model.JobFilter; import org.byteveda.taskito.model.JobStatus; +import org.byteveda.taskito.model.TaskLogLevel; import org.byteveda.taskito.model.TaskLog; import org.byteveda.taskito.task.EnqueueOptions; import org.byteveda.taskito.task.Task; @@ -154,7 +155,7 @@ void pauseAndResumeQueue(@TempDir Path dir) { void taskLogsRoundTrip(@TempDir Path dir) { try (Taskito queue = open(dir)) { String id = queue.enqueue("send_email", Collections.singletonMap("to", "a")); - queue.writeTaskLog(id, "send_email", "info", "hello"); + queue.writeTaskLog(id, "send_email", TaskLogLevel.INFO, "hello"); List logs = queue.getTaskLogs(id); assertEquals(1, logs.size()); assertEquals("hello", logs.get(0).message); diff --git a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardNativeTest.java b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardNativeTest.java index 1336a278..c8f07fa0 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardNativeTest.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardNativeTest.java @@ -8,6 +8,7 @@ import java.util.Collections; import java.util.Map; import org.byteveda.taskito.Taskito; +import org.byteveda.taskito.model.TaskLogLevel; import org.byteveda.taskito.model.JobFilter; import org.byteveda.taskito.task.EnqueueOptions; import org.byteveda.taskito.task.Task; @@ -39,7 +40,7 @@ void logsAndJobLogs(@TempDir Path dir) throws Exception { Taskito.builder().sqlite(dir.resolve("t.db").toString()).open(); DashboardServer server = DashboardServer.start(queue, 0)) { String jobId = seedJob(queue); - queue.writeTaskLog(jobId, "email.send", "info", "hello-log"); + queue.writeTaskLog(jobId, "email.send", TaskLogLevel.INFO, "hello-log"); DashboardClient client = new DashboardClient(server.port()).as(DashboardClient.seedAdmin(queue)); assertTrue(client.get("/api/jobs/" + jobId + "/logs").body().contains("hello-log")); assertTrue(client.get("/api/logs").body().contains("hello-log")); diff --git a/sdks/java/src/test/java/org/byteveda/taskito/model/ClosedSetEnumTest.java b/sdks/java/src/test/java/org/byteveda/taskito/model/ClosedSetEnumTest.java new file mode 100644 index 00000000..fce7ee3c --- /dev/null +++ b/sdks/java/src/test/java/org/byteveda/taskito/model/ClosedSetEnumTest.java @@ -0,0 +1,33 @@ +package org.byteveda.taskito.model; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.byteveda.taskito.errors.SerializationException; +import org.junit.jupiter.api.Test; + +/** The wire forms of the closed-set model enums are a cross-SDK contract. */ +class ClosedSetEnumTest { + + @Test + void dispatchOrderRoundTripsItsWireForm() { + assertEquals("fifo", DispatchOrder.FIFO.wire()); + assertEquals("lifo", DispatchOrder.LIFO.wire()); + assertEquals(DispatchOrder.LIFO, DispatchOrder.fromWire("lifo")); + assertEquals(DispatchOrder.FIFO, DispatchOrder.fromWire("FIFO")); + } + + @Test + void taskLogLevelRoundTripsItsWireForm() { + assertEquals("warning", TaskLogLevel.WARNING.wire()); + assertEquals("result", TaskLogLevel.RESULT.wire()); + assertEquals(TaskLogLevel.CRITICAL, TaskLogLevel.fromWire("critical")); + } + + @Test + void unknownWireFormIsRejected() { + assertThrows(SerializationException.class, () -> DispatchOrder.fromWire("sideways")); + assertThrows(SerializationException.class, () -> TaskLogLevel.fromWire("verbose")); + assertThrows(SerializationException.class, () -> TaskLogLevel.fromWire(null)); + } +} diff --git a/sdks/node/src/dashboard/handlers/core.ts b/sdks/node/src/dashboard/handlers/core.ts index 3e14f368..09928c11 100644 --- a/sdks/node/src/dashboard/handlers/core.ts +++ b/sdks/node/src/dashboard/handlers/core.ts @@ -4,6 +4,7 @@ import { randomBytes } from "node:crypto"; import type { EventName } from "../../events"; import type { Queue } from "../../index"; +import type { TaskLogLevel } from "../../types"; import type { WebhookInput } from "../../webhooks"; import type { WorkflowNode } from "../../workflows"; import { BadRequestError } from "../errors"; @@ -297,7 +298,9 @@ export async function logs(queue: Queue, url: URL) { const since = positiveOr(url.searchParams.get("since"), 3600); const found = await queue.queryLogs({ task: url.searchParams.get("task") ?? undefined, - level: url.searchParams.get("level") ?? undefined, + // Cast rather than validate: an unknown ?level= is untrusted input that must + // filter to nothing, which is exactly what the native filter already does. + level: (url.searchParams.get("level") ?? undefined) as TaskLogLevel | undefined, sinceMs: Date.now() - since * 1000, limit: num(url, "limit") ?? 100, }); diff --git a/sdks/node/src/queue.ts b/sdks/node/src/queue.ts index fba2e610..4d14fafb 100644 --- a/sdks/node/src/queue.ts +++ b/sdks/node/src/queue.ts @@ -80,6 +80,7 @@ import type { SubscriberOptions, Subscription, TaskLog, + TaskLogLevel, TaskMap, TaskOptions, TopicLogStat, @@ -1023,7 +1024,7 @@ export class Queue { * `sinceMs` is a Unix-ms lower bound (default: the last hour). */ queryLogs( - options: { task?: string; level?: string; sinceMs?: number; limit?: number } = {}, + options: { task?: string; level?: TaskLogLevel; sinceMs?: number; limit?: number } = {}, ): Promise { const sinceMs = options.sinceMs ?? Date.now() - 3_600_000; return this.native.queryTaskLogs(options.task, options.level, sinceMs, options.limit ?? 100); @@ -1415,7 +1416,7 @@ interface PendingSubscription { } /** Log level used for published partial results (matches the cross-SDK contract). */ -const STREAM_LEVEL = "result"; +const STREAM_LEVEL: TaskLogLevel = "result"; /** Job statuses at which a stream stops. */ const TERMINAL_STATUSES = new Set(["complete", "failed", "dead", "cancelled"]); diff --git a/sdks/node/src/types.ts b/sdks/node/src/types.ts index 9f70c0d6..a449391c 100644 --- a/sdks/node/src/types.ts +++ b/sdks/node/src/types.ts @@ -28,6 +28,33 @@ export type { MeshWorkerConfig, } from "./native"; +/** + * Same-priority dispatch order. `"lifo"` runs newest-first under overload (a + * freshness lever); `"fifo"` (default) is the fair oldest-first ordering. + */ +export type DispatchOrder = "fifo" | "lifo"; + +/** Every dispatch order, for runtime validation. */ +export const DISPATCH_ORDERS: readonly DispatchOrder[] = ["fifo", "lifo"]; + +/** + * Severity of a structured task log. `"result"` is not a severity — it carries + * partial results published from a running task. Distinct from the SDK's own + * logger level (`LogLevel` in `./utils`): this one is persisted and read back + * by every SDK. + */ +export type TaskLogLevel = "debug" | "info" | "warning" | "error" | "critical" | "result"; + +/** Every task-log level, for runtime validation. */ +export const TASK_LOG_LEVELS: readonly TaskLogLevel[] = [ + "debug", + "info", + "warning", + "error", + "critical", + "result", +]; + /** One message pulled from a log topic; `id` is the cursor token for `ackTopic`. */ export interface TopicMessage { /** Message id — pass to `ackTopic` to advance the cursor past it. */ @@ -226,7 +253,7 @@ export interface QueueLimits { * freshness lever); `"fifo"` (default) is the fair oldest-first ordering. * Priority always dominates. Honored on SQLite/Postgres; Redis is FIFO-only. */ - dispatchOrder?: "fifo" | "lifo"; + dispatchOrder?: DispatchOrder; } /** A task handler plus its registration options. */ From 5e4b3a9172e310a2728f9ccd199eed302d4ae28f Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:40:32 +0530 Subject: [PATCH 03/22] feat(python): type the closed-set task and workflow options An unknown visualize format silently rendered Mermaid; the rest raised at a distance from the call that was wrong. --- sdks/python/taskito/__init__.py | 7 +- sdks/python/taskito/app.py | 28 ++++--- sdks/python/taskito/interception/__init__.py | 2 + .../taskito/interception/interceptor.py | 27 +++--- sdks/python/taskito/interception/mode.py | 18 ++++ sdks/python/taskito/mixins/_log_consumer.py | 16 +++- sdks/python/taskito/mixins/decorators.py | 13 +-- sdks/python/taskito/mixins/predicates.py | 12 +-- sdks/python/taskito/mixins/pubsub.py | 15 ++-- sdks/python/taskito/predicates/__init__.py | 3 +- sdks/python/taskito/predicates/outcomes.py | 15 ++++ sdks/python/taskito/workflows/__init__.py | 13 ++- sdks/python/taskito/workflows/builder.py | 47 ++++++----- sdks/python/taskito/workflows/fan_out.py | 9 +- sdks/python/taskito/workflows/run.py | 11 ++- .../python/taskito/workflows/tracker/gates.py | 7 +- sdks/python/taskito/workflows/types.py | 27 ++++++ .../tests/python/test_closed_set_enums.py | 83 +++++++++++++++++++ 18 files changed, 275 insertions(+), 78 deletions(-) create mode 100644 sdks/python/taskito/interception/mode.py create mode 100644 sdks/python/tests/python/test_closed_set_enums.py diff --git a/sdks/python/taskito/__init__.py b/sdks/python/taskito/__init__.py index 97c02b95..622d4140 100644 --- a/sdks/python/taskito/__init__.py +++ b/sdks/python/taskito/__init__.py @@ -41,13 +41,15 @@ TaskTimeoutError, ) from taskito.inject import Inject -from taskito.interception import InterceptionError, InterceptionReport +from taskito.interception import InterceptionError, InterceptionMode, InterceptionReport from taskito.log_config import configure as configure_logging from taskito.mesh import MeshWorker from taskito.middleware import TaskMiddleware from taskito.mixins.periodic import PeriodicInfo from taskito.mixins.pubsub import TopicMessage from taskito.notes import MAX_NOTE_FIELDS +from taskito.predicates.outcomes import PredicateAction +from taskito.proxies.built_in import BuiltInProxy from taskito.proxies.no_proxy import NoProxy from taskito.result import JobResult from taskito.retention import EffectiveRetention, Retention @@ -70,6 +72,7 @@ "BatchItemResult", "BatchPartialFailureError", "BatchResultTypeError", + "BuiltInProxy", "CborSerializer", "CircuitBreakerOpenError", "CircularDependencyError", @@ -83,6 +86,7 @@ "HmacCodec", "Inject", "InterceptionError", + "InterceptionMode", "InterceptionReport", "JobNotFoundError", "JobResult", @@ -96,6 +100,7 @@ "NotesValidationError", "PayloadCodec", "PeriodicInfo", + "PredicateAction", "PredicateRejectedError", "ProxyCleanupError", "ProxyReconstructionError", diff --git a/sdks/python/taskito/app.py b/sdks/python/taskito/app.py index 359a39a6..d201b5cf 100644 --- a/sdks/python/taskito/app.py +++ b/sdks/python/taskito/app.py @@ -30,9 +30,10 @@ from taskito.async_support.mixins import AsyncQueueMixin from taskito.batching import BatchAccumulator, BatchConfig from taskito.codecs import CodecSerializer, PayloadCodec +from taskito.enums import coerce_enum from taskito.events import EventBus, EventType from taskito.exceptions import QueueFullError, SerializationError -from taskito.interception import ArgumentInterceptor +from taskito.interception import ArgumentInterceptor, InterceptionMode from taskito.interception.built_in import build_default_registry from taskito.interception.metrics import InterceptionMetrics from taskito.middleware import TaskMiddleware @@ -54,7 +55,7 @@ ) from taskito.notes import validate_and_encode_notes from taskito.proxies import ProxyRegistry -from taskito.proxies.built_in import register_builtin_handlers +from taskito.proxies.built_in import BuiltInProxy, register_builtin_handlers from taskito.proxies.metrics import ProxyMetrics from taskito.result import JobResult from taskito.retention import Retention @@ -138,12 +139,12 @@ def __init__( schema: str = "taskito", pool_size: int | None = None, drain_timeout: int = 30, - interception: str = "off", + interception: InterceptionMode | str = InterceptionMode.OFF, max_intercept_depth: int = 10, recipe_signing_key: str | None = None, max_reconstruction_timeout: int = 10, file_path_allowlist: list[str] | None = None, - disabled_proxies: list[str] | None = None, + disabled_proxies: Sequence[BuiltInProxy | str] | None = None, async_concurrency: int = 100, event_workers: int = 4, scheduler_poll_interval_ms: int = 50, @@ -197,17 +198,21 @@ def __init__( that have low connection limits. drain_timeout: Seconds to wait for in-flight jobs to finish during graceful shutdown. Defaults to 30. - interception: Argument interception mode — ``"strict"`` (reject - non-serializable args), ``"lenient"`` (warn and drop), or - ``"off"`` (disabled, default). See :mod:`taskito.interception`. + interception: Argument interception mode — an + :class:`~taskito.interception.InterceptionMode` or its string: + ``"strict"`` (reject non-serializable args), ``"lenient"`` (warn + and drop), or ``"off"`` (disabled, default). See + :mod:`taskito.interception`. max_intercept_depth: Maximum recursion depth for argument walking. Defaults to 10. recipe_signing_key: HMAC-SHA256 key for proxy recipe integrity. Falls back to ``TASKITO_RECIPE_SECRET`` env var. max_reconstruction_timeout: Max seconds for proxy reconstruction. file_path_allowlist: Allowed file paths for the file proxy handler. - disabled_proxies: Handler names to skip when registering built-in - proxy handlers. + disabled_proxies: Built-in proxy handlers to skip registering — + :class:`~taskito.proxies.built_in.BuiltInProxy` members or their + string ids. An unknown id raises rather than silently leaving + the handler registered. async_concurrency: Maximum number of async tasks running concurrently on the native async executor. Defaults to 100. event_workers: Thread pool size for the event bus (default 4). @@ -340,12 +345,13 @@ def __init__( # Argument interception self._interception_metrics: InterceptionMetrics | None = None - if interception != "off": + interception_mode = coerce_enum(InterceptionMode, interception, param="interception") + if interception_mode is not InterceptionMode.OFF: self._interception_metrics = InterceptionMetrics() registry = build_default_registry() self._interceptor: ArgumentInterceptor | None = ArgumentInterceptor( registry=registry, - mode=interception, + mode=interception_mode, max_depth=max_intercept_depth, proxy_registry=self._proxy_registry, metrics=self._interception_metrics, diff --git a/sdks/python/taskito/interception/__init__.py b/sdks/python/taskito/interception/__init__.py index aa4e3f02..71fdf8ac 100644 --- a/sdks/python/taskito/interception/__init__.py +++ b/sdks/python/taskito/interception/__init__.py @@ -10,6 +10,7 @@ from taskito.interception.errors import ArgumentFailure, InterceptionError from taskito.interception.interceptor import ArgumentInterceptor, InterceptionReport +from taskito.interception.mode import InterceptionMode from taskito.interception.reconstruct import reconstruct_args from taskito.interception.registry import TypeRegistry from taskito.interception.strategy import Strategy @@ -18,6 +19,7 @@ "ArgumentFailure", "ArgumentInterceptor", "InterceptionError", + "InterceptionMode", "InterceptionReport", "Strategy", "TypeRegistry", diff --git a/sdks/python/taskito/interception/interceptor.py b/sdks/python/taskito/interception/interceptor.py index 96a348e1..9c7b6fdc 100644 --- a/sdks/python/taskito/interception/interceptor.py +++ b/sdks/python/taskito/interception/interceptor.py @@ -8,7 +8,9 @@ from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any +from taskito.enums import coerce_enum from taskito.interception.errors import InterceptionError +from taskito.interception.mode import InterceptionMode from taskito.interception.registry import TypeRegistry from taskito.interception.strategy import Strategy from taskito.interception.walker import ArgumentWalker @@ -49,30 +51,29 @@ class ArgumentInterceptor: """Orchestrates argument interception before serialization. Three modes: - - ``"strict"``: REJECT raises immediately, unknown non-serializable types raise. - - ``"lenient"``: REJECT logs a warning and drops the argument, unknowns pass through. - - ``"off"``: No interception at all — passthrough to serializer. + - :attr:`~taskito.interception.InterceptionMode.STRICT`: REJECT raises immediately, + unknown non-serializable types raise. + - :attr:`~taskito.interception.InterceptionMode.LENIENT`: REJECT logs a warning and + drops the argument, unknowns pass through. + - :attr:`~taskito.interception.InterceptionMode.OFF`: No interception at all — + passthrough to serializer. """ def __init__( self, registry: TypeRegistry, - mode: str = "strict", + mode: InterceptionMode | str = InterceptionMode.STRICT, max_depth: int = 10, proxy_registry: ProxyRegistry | None = None, metrics: InterceptionMetrics | None = None, ) -> None: - if mode not in ("strict", "lenient", "off"): - raise ValueError( - f"Invalid interception mode: {mode!r}. Use 'strict', 'lenient', or 'off'." - ) self._registry = registry - self._mode = mode + self._mode = coerce_enum(InterceptionMode, mode, param="interception mode") self._walker = ArgumentWalker(registry, max_depth=max_depth, proxy_registry=proxy_registry) self._metrics = metrics @property - def mode(self) -> str: + def mode(self) -> InterceptionMode: return self._mode def intercept(self, args: tuple, kwargs: dict) -> tuple[tuple, dict]: @@ -84,7 +85,7 @@ def intercept(self, args: tuple, kwargs: dict) -> tuple[tuple, dict]: Raises: InterceptionError: In strict mode, if any arguments are rejected. """ - if self._mode == "off": + if self._mode is InterceptionMode.OFF: return args, kwargs start = time.monotonic() @@ -99,7 +100,7 @@ def intercept(self, args: tuple, kwargs: dict) -> tuple[tuple, dict]: ) if walk_result.failures: - if self._mode == "strict": + if self._mode is InterceptionMode.STRICT: raise InterceptionError(walk_result.failures) # Lenient mode: log warnings, strip rejected values for f in walk_result.failures: @@ -115,7 +116,7 @@ def intercept(self, args: tuple, kwargs: dict) -> tuple[tuple, dict]: def analyze(self, args: tuple, kwargs: dict) -> InterceptionReport: """Analyze arguments without transforming them. Returns a human-readable report.""" - if self._mode == "off": + if self._mode is InterceptionMode.OFF: return InterceptionReport() report = InterceptionReport() diff --git a/sdks/python/taskito/interception/mode.py b/sdks/python/taskito/interception/mode.py new file mode 100644 index 00000000..4c3f699a --- /dev/null +++ b/sdks/python/taskito/interception/mode.py @@ -0,0 +1,18 @@ +"""How strictly the interceptor treats an argument it cannot serialize.""" + +from __future__ import annotations + +import enum + + +class InterceptionMode(str, enum.Enum): + """Disposition for arguments the interceptor classifies as unsafe.""" + + STRICT = "strict" + """Reject the enqueue.""" + + LENIENT = "lenient" + """Warn and drop the argument.""" + + OFF = "off" + """Interception disabled (the default).""" diff --git a/sdks/python/taskito/mixins/_log_consumer.py b/sdks/python/taskito/mixins/_log_consumer.py index 2ad1384c..459e34f4 100644 --- a/sdks/python/taskito/mixins/_log_consumer.py +++ b/sdks/python/taskito/mixins/_log_consumer.py @@ -8,6 +8,7 @@ from __future__ import annotations +import enum import logging import threading from collections.abc import Callable @@ -18,9 +19,20 @@ if TYPE_CHECKING: from taskito.mixins.pubsub import QueuePubSubMixin, TopicMessage + logger = logging.getLogger("taskito") +class ConsumerErrorAction(str, enum.Enum): + """What a managed log consumer does when its handler raises.""" + + RETRY = "retry" + """Leave the message un-acked so the next poll re-reads the batch.""" + + SKIP = "skip" + """Ack past the failed message and continue.""" + + class LogConsumerThread(threading.Thread): """Poll one log subscription and drive its handler until asked to stop. @@ -43,7 +55,7 @@ def __init__( self._handler: Callable[..., Any] = config["handler"] self._poll_interval: float = config["poll_interval"] self._batch_size: int = config["batch_size"] - self._on_error: str = config["on_error"] + self._on_error: ConsumerErrorAction = config["on_error"] self._stop_event = stop_event def run(self) -> None: @@ -92,7 +104,7 @@ def _drain_batch(self, messages: list[TopicMessage]) -> tuple[str | None, bool]: self._name, message.id, ) - if self._on_error == "retry": + if self._on_error is ConsumerErrorAction.RETRY: return last_acked, True last_acked = message.id return last_acked, False diff --git a/sdks/python/taskito/mixins/decorators.py b/sdks/python/taskito/mixins/decorators.py index ae1d5843..a553c9c0 100644 --- a/sdks/python/taskito/mixins/decorators.py +++ b/sdks/python/taskito/mixins/decorators.py @@ -19,9 +19,11 @@ from taskito.codecs import CodecSerializer from taskito.context import _clear_context from taskito.dashboard.middleware_store import MiddlewareDisableStore +from taskito.enums import coerce_enum from taskito.inject import Inject, _InjectAlias from taskito.middleware import middleware_key from taskito.predicates.core import coerce_predicate +from taskito.predicates.outcomes import PredicateAction from taskito.task import TaskWrapper from taskito.task_lifecycle import run_lifecycle @@ -79,7 +81,7 @@ class QueueDecoratorMixin: _task_retry_filters: dict[str, dict[str, list[type[Exception]]]] _task_inject_map: dict[str, list[str]] _task_predicates: dict[str, Predicate] - _task_predicate_on_false: dict[str, str] + _task_predicate_on_false: dict[str, PredicateAction] _task_predicate_extras: dict[str, dict[str, Any]] _task_default_defer: dict[str, float] _task_predicate_serialized: dict[str, dict[str, Any] | None] @@ -196,7 +198,7 @@ def task( compensates: TaskWrapper | str | None = None, batch: bool | dict[str, Any] | None = None, predicate: Predicate | Callable[..., Any] | None = None, - on_false: str = "defer", + on_false: PredicateAction | str = PredicateAction.DEFER, predicate_extras: dict[str, Any] | None = None, default_defer_seconds: float = 60.0, # Appended, not slotted next to their related options: this signature is @@ -299,11 +301,10 @@ def task( ``.apply_async(args=..., kwargs=..., ...)`` for enqueueing. Raises: - ValueError: ``on_false`` is not ``"defer"`` or ``"cancel"``, or + ValueError: ``on_false`` is not a :class:`PredicateAction` value, or ``default_defer_seconds`` is negative. """ - if on_false not in {"defer", "cancel"}: - raise ValueError(f"on_false must be 'defer' or 'cancel', got {on_false!r}") + on_false_action = coerce_enum(PredicateAction, on_false, param="on_false") if default_defer_seconds < 0: raise ValueError("default_defer_seconds must be >= 0") batch_config = BatchConfig.normalize(batch) @@ -400,7 +401,7 @@ def decorator(fn: Callable) -> TaskWrapper: coerced = coerce_predicate(predicate) if coerced is not None: self._task_predicates[task_name] = coerced - self._task_predicate_on_false[task_name] = on_false + self._task_predicate_on_false[task_name] = on_false_action if predicate_extras: self._task_predicate_extras[task_name] = dict(predicate_extras) self._task_default_defer[task_name] = default_defer_seconds diff --git a/sdks/python/taskito/mixins/predicates.py b/sdks/python/taskito/mixins/predicates.py index 81c79dbd..8b66c52e 100644 --- a/sdks/python/taskito/mixins/predicates.py +++ b/sdks/python/taskito/mixins/predicates.py @@ -34,7 +34,7 @@ from taskito.predicates.core import Predicate as _Predicate from taskito.predicates.evaluate import evaluate_predicate from taskito.predicates.metrics import PredicateMetrics -from taskito.predicates.outcomes import Cancel, Defer +from taskito.predicates.outcomes import Cancel, Defer, PredicateAction from taskito.predicates.registry import PredicateValidationError from taskito.predicates.registry import ( register_predicate as _register_predicate, @@ -50,7 +50,7 @@ class QueuePredicateMixin: _inner: PyQueue _task_predicates: dict[str, _Predicate] - _task_predicate_on_false: dict[str, str] + _task_predicate_on_false: dict[str, PredicateAction] _task_predicate_extras: dict[str, dict[str, Any]] _task_default_defer: dict[str, float] _task_predicate_serialized: dict[str, dict[str, Any] | None] @@ -192,8 +192,8 @@ def _apply_dispatch_predicate( raise TaskCancelledError(f"predicate deferred job {job_id} by {outcome.seconds:.1f}s") # Plain False — branch on the task's on_false setting. - action = self._task_predicate_on_false.get(task_name, "defer") - if action == "cancel": + action = self._task_predicate_on_false.get(task_name, PredicateAction.DEFER) + if action is PredicateAction.CANCEL: self._emit_dispatch_cancel(task_name, job_id, queue_name, None) raise TaskCancelledError(f"predicate rejected job {job_id}") @@ -277,8 +277,8 @@ def _apply_enqueue_predicate( raise PredicateRejectedError(task_name, outcome.reason) # Plain False — branch on the task's on_false setting. - action = self._task_predicate_on_false.get(task_name, "defer") - if action == "cancel": + action = self._task_predicate_on_false.get(task_name, PredicateAction.DEFER) + if action is PredicateAction.CANCEL: self._emit_enqueue_reject(task_name, queue_name, None) raise PredicateRejectedError(task_name) diff --git a/sdks/python/taskito/mixins/pubsub.py b/sdks/python/taskito/mixins/pubsub.py index da791490..8d7141f9 100644 --- a/sdks/python/taskito/mixins/pubsub.py +++ b/sdks/python/taskito/mixins/pubsub.py @@ -9,7 +9,8 @@ from dataclasses import dataclass from typing import TYPE_CHECKING, Any -from taskito.mixins._log_consumer import LogConsumerThread +from taskito.enums import coerce_enum +from taskito.mixins._log_consumer import ConsumerErrorAction, LogConsumerThread from taskito.notes import validate_and_encode_notes from taskito.result import JobResult @@ -160,7 +161,7 @@ def log_consumer( *, poll_interval: float = 1.0, batch_size: int = 100, - on_error: str = "retry", + on_error: ConsumerErrorAction | str = ConsumerErrorAction.RETRY, ) -> Callable[[Callable[..., Any]], Callable[..., Any]]: """Decorator: register ``fn`` as a managed consumer of log ``topic``. @@ -180,11 +181,11 @@ def log_consumer( name: Stable subscription identity. Defaults to the handler name. poll_interval: Seconds to wait after an empty poll before re-reading. batch_size: Max messages pulled per poll. - on_error: ``"retry"`` (default) leaves the failed message un-acked so - the batch re-reads; ``"skip"`` acks past it and continues. + on_error: :class:`ConsumerErrorAction` (or its string) — + ``"retry"`` (default) leaves the failed message un-acked so the + batch re-reads; ``"skip"`` acks past it and continues. """ - if on_error not in ("retry", "skip"): - raise ValueError(f"on_error must be 'retry' or 'skip', got {on_error!r}") + on_error_action = coerce_enum(ConsumerErrorAction, on_error, param="on_error") # A non-finite/non-positive interval spins on empty polls; a non-positive # batch size never makes progress. if not (poll_interval > 0 and math.isfinite(poll_interval)): @@ -203,7 +204,7 @@ def decorator(fn: Callable[..., Any]) -> Callable[..., Any]: "handler": fn, "poll_interval": poll_interval, "batch_size": batch_size, - "on_error": on_error, + "on_error": on_error_action, } # Replace, don't append: re-decorating the same (topic, name) (module # reloads, test fixtures) must not spawn duplicate consumer threads. diff --git a/sdks/python/taskito/predicates/__init__.py b/sdks/python/taskito/predicates/__init__.py index 25713c67..508e4249 100644 --- a/sdks/python/taskito/predicates/__init__.py +++ b/sdks/python/taskito/predicates/__init__.py @@ -22,7 +22,7 @@ ) from taskito.predicates.evaluate import evaluate_predicate from taskito.predicates.metrics import PredicateMetrics -from taskito.predicates.outcomes import Cancel, Defer, PredicateOutcome +from taskito.predicates.outcomes import Cancel, Defer, PredicateAction, PredicateOutcome from taskito.predicates.parser import format_predicate, parse from taskito.predicates.providers import FeatureFlagProvider, env_feature_flag_provider from taskito.predicates.recipes import ( @@ -53,6 +53,7 @@ "NotPredicate", "OrPredicate", "Predicate", + "PredicateAction", "PredicateContext", "PredicateMetrics", "PredicateOutcome", diff --git a/sdks/python/taskito/predicates/outcomes.py b/sdks/python/taskito/predicates/outcomes.py index 9d207a87..040de722 100644 --- a/sdks/python/taskito/predicates/outcomes.py +++ b/sdks/python/taskito/predicates/outcomes.py @@ -2,9 +2,24 @@ from __future__ import annotations +import enum from dataclasses import dataclass +class PredicateAction(str, enum.Enum): + """What a task does when its predicate returns a plain ``False``. + + The richer :class:`Defer` / :class:`Cancel` sentinels say it per-evaluation; + this is the per-task default for the bare boolean. + """ + + DEFER = "defer" + """Re-enqueue the job after ``default_defer_seconds``.""" + + CANCEL = "cancel" + """Skip the job terminally.""" + + @dataclass(frozen=True, slots=True) class Defer: """Sentinel returned by a predicate to defer the job. diff --git a/sdks/python/taskito/workflows/__init__.py b/sdks/python/taskito/workflows/__init__.py index 9c34199e..e00eaaf4 100644 --- a/sdks/python/taskito/workflows/__init__.py +++ b/sdks/python/taskito/workflows/__init__.py @@ -10,9 +10,20 @@ from .builder import GateConfig, Workflow, WorkflowProxy from .context import WorkflowContext from .run import WorkflowRun -from .types import NodeSnapshot, NodeStatus, WorkflowState, WorkflowStatus +from .types import ( + DiagramFormat, + FanStrategy, + GateAction, + NodeSnapshot, + NodeStatus, + WorkflowState, + WorkflowStatus, +) __all__ = [ + "DiagramFormat", + "FanStrategy", + "GateAction", "GateConfig", "NodeSnapshot", "NodeStatus", diff --git a/sdks/python/taskito/workflows/builder.py b/sdks/python/taskito/workflows/builder.py index 564c5eb3..93f0a834 100644 --- a/sdks/python/taskito/workflows/builder.py +++ b/sdks/python/taskito/workflows/builder.py @@ -11,8 +11,10 @@ from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable from taskito._taskito import PyWorkflowBuilder +from taskito.enums import coerce_enum from . import analysis as _analysis +from .types import DiagramFormat, FanStrategy, GateAction from .visualization import nodes_and_edges_from_steps, render_dot, render_mermaid if TYPE_CHECKING: @@ -31,8 +33,6 @@ class HasTaskName(Protocol): def _task_name(self) -> str: ... -_VALID_FAN_OUT = frozenset({"each"}) -_VALID_FAN_IN = frozenset({"all"}) _VALID_CONDITIONS = frozenset({"on_success", "on_failure", "always"}) _VALID_ON_FAILURE = frozenset({"fail_fast", "continue"}) @@ -44,8 +44,8 @@ class GateConfig: timeout: float | None = None """Seconds until auto-resolve. ``None`` waits indefinitely.""" - on_timeout: str = "reject" - """Action on timeout: ``"approve"`` or ``"reject"``.""" + on_timeout: GateAction = GateAction.REJECT + """Action on timeout.""" message: str | Callable | None = None """Human-readable message shown to approvers.""" @@ -71,8 +71,8 @@ class _Step: max_retries: int | None = None timeout_ms: int | None = None priority: int | None = None - fan_out: str | None = None - fan_in: str | None = None + fan_out: FanStrategy | None = None + fan_in: FanStrategy | None = None condition: str | Callable | None = None gate_config: GateConfig | None = None sub_workflow: SubWorkflowRef | None = None @@ -137,8 +137,8 @@ def step( max_retries: int | None = None, timeout_ms: int | None = None, priority: int | None = None, - fan_out: str | None = None, - fan_in: str | None = None, + fan_out: FanStrategy | str | None = None, + fan_in: FanStrategy | str | None = None, condition: str | Callable | None = None, compensates: HasTaskName | str | None | _InheritCompensator = INHERIT_COMPENSATOR, ) -> Workflow: @@ -195,16 +195,21 @@ def step( ) if fan_out is not None: - if fan_out not in _VALID_FAN_OUT: - valid = sorted(_VALID_FAN_OUT) - raise ValueError(f"step '{name}': fan_out must be one of {valid}, got '{fan_out}'") + fan_out = coerce_enum(FanStrategy, fan_out, param=f"step '{name}': fan_out") + if fan_out is not FanStrategy.EACH: + raise ValueError( + f"step '{name}': fan_out must be {FanStrategy.EACH.value!r}, " + f"got {fan_out.value!r}" + ) if len(predecessors) != 1: raise ValueError(f"step '{name}': fan_out step must have exactly one predecessor") if fan_in is not None: - if fan_in not in _VALID_FAN_IN: + fan_in = coerce_enum(FanStrategy, fan_in, param=f"step '{name}': fan_in") + if fan_in is not FanStrategy.ALL: raise ValueError( - f"step '{name}': fan_in must be one of {sorted(_VALID_FAN_IN)}, got '{fan_in}'" + f"step '{name}': fan_in must be {FanStrategy.ALL.value!r}, " + f"got {fan_in.value!r}" ) if len(predecessors) != 1: raise ValueError(f"step '{name}': fan_in step must have exactly one predecessor") @@ -275,7 +280,7 @@ def gate( after: str | list[str] | None = None, condition: str | Callable | None = None, timeout: float | None = None, - on_timeout: str = "reject", + on_timeout: GateAction | str = GateAction.REJECT, message: str | Callable | None = None, ) -> Workflow: """Add an approval gate step. @@ -291,15 +296,14 @@ def gate( after: Predecessor step name(s). condition: Optional condition for entering the gate. timeout: Seconds until auto-resolve (``None`` = wait forever). - on_timeout: ``"approve"`` or ``"reject"`` when timeout fires. + on_timeout: :class:`GateAction` (or its string) when the timeout fires. message: Human-readable message for approvers. """ if name in self._steps: raise ValueError(f"step '{name}' already defined") if "[" in name: raise ValueError(f"step '{name}': names must not contain '['") - if on_timeout not in ("approve", "reject"): - raise ValueError(f"gate '{name}': on_timeout must be 'approve' or 'reject'") + timeout_action = coerce_enum(GateAction, on_timeout, param=f"gate '{name}': on_timeout") if after is None: predecessors: list[str] = [] @@ -318,7 +322,7 @@ def gate( condition=condition, gate_config=GateConfig( timeout=timeout, - on_timeout=on_timeout, + on_timeout=timeout_action, message=message, ), ) @@ -365,17 +369,18 @@ def bottleneck_analysis(self, costs: dict[str, float]) -> dict[str, Any]: """Identify the bottleneck node on the critical path.""" return _analysis.bottleneck_analysis(self._steps, costs) - def visualize(self, fmt: str = "mermaid") -> str: + def visualize(self, fmt: DiagramFormat | str = DiagramFormat.MERMAID) -> str: """Render the workflow DAG as a diagram string. Args: - fmt: Output format — ``"mermaid"`` or ``"dot"``. + fmt: Output format — a :class:`DiagramFormat` or its string. An + unknown one raises rather than silently falling back to Mermaid. Returns: The diagram string (no statuses — pre-execution view). """ nodes, edges = nodes_and_edges_from_steps(self._steps) - if fmt == "dot": + if coerce_enum(DiagramFormat, fmt, param="fmt") is DiagramFormat.DOT: return render_dot(nodes, edges) return render_mermaid(nodes, edges) diff --git a/sdks/python/taskito/workflows/fan_out.py b/sdks/python/taskito/workflows/fan_out.py index 7cc4e3fb..191d873c 100644 --- a/sdks/python/taskito/workflows/fan_out.py +++ b/sdks/python/taskito/workflows/fan_out.py @@ -4,14 +4,17 @@ from typing import Any +from taskito.enums import coerce_enum from taskito.serializers import Serializer +from taskito.workflows.types import FanStrategy -def apply_fan_out(strategy: str, result: Any) -> list[Any]: +def apply_fan_out(strategy: FanStrategy | str, result: Any) -> list[Any]: """Split a step's return value into individual fan-out items. Args: - strategy: The fan-out strategy (``"each"``). + strategy: The fan-out strategy — :attr:`FanStrategy.EACH`. Read back + from persisted step metadata, so a string is accepted. result: The predecessor step's return value. Returns: @@ -21,7 +24,7 @@ def apply_fan_out(strategy: str, result: Any) -> list[Any]: TypeError: If the result is not iterable for the given strategy. ValueError: If the strategy is unknown. """ - if strategy == "each": + if coerce_enum(FanStrategy, strategy, param="fan_out strategy") is FanStrategy.EACH: try: return list(result) except TypeError as exc: diff --git a/sdks/python/taskito/workflows/run.py b/sdks/python/taskito/workflows/run.py index 09b3002d..6c457ec3 100644 --- a/sdks/python/taskito/workflows/run.py +++ b/sdks/python/taskito/workflows/run.py @@ -12,7 +12,9 @@ import time from typing import TYPE_CHECKING -from .types import NodeSnapshot, NodeStatus, WorkflowState, WorkflowStatus +from taskito.enums import coerce_enum + +from .types import DiagramFormat, NodeSnapshot, NodeStatus, WorkflowState, WorkflowStatus from .visualization import nodes_and_edges_from_dag_bytes, render_dot, render_mermaid if TYPE_CHECKING: @@ -115,11 +117,12 @@ def cancel(self) -> None: """Cancel any pending steps and mark the run as cancelled.""" self._queue._inner.cancel_workflow_run(self.id) - def visualize(self, fmt: str = "mermaid") -> str: + def visualize(self, fmt: DiagramFormat | str = DiagramFormat.MERMAID) -> str: """Render the workflow DAG with live node statuses. Args: - fmt: Output format — ``"mermaid"`` or ``"dot"``. + fmt: Output format — a :class:`DiagramFormat` or its string. An + unknown one raises rather than silently falling back to Mermaid. """ dag_bytes = self._queue._inner.get_workflow_definition_dag(self.id) nodes, edges = nodes_and_edges_from_dag_bytes(dag_bytes) @@ -129,7 +132,7 @@ def visualize(self, fmt: str = "mermaid") -> str: for name, node in snapshot.nodes.items(): statuses[name] = node.status.value - if fmt == "dot": + if coerce_enum(DiagramFormat, fmt, param="fmt") is DiagramFormat.DOT: return render_dot(nodes, edges, statuses) return render_mermaid(nodes, edges, statuses) diff --git a/sdks/python/taskito/workflows/tracker/gates.py b/sdks/python/taskito/workflows/tracker/gates.py index de84916c..2bce9a26 100644 --- a/sdks/python/taskito/workflows/tracker/gates.py +++ b/sdks/python/taskito/workflows/tracker/gates.py @@ -7,6 +7,7 @@ from typing import TYPE_CHECKING from taskito.events import EventType +from taskito.workflows.types import GateAction if TYPE_CHECKING: from taskito.workflows.tracker.tracker import WorkflowTracker @@ -50,7 +51,9 @@ def enter_gate(tracker: WorkflowTracker, run_id: str, node_name: str, config: _R timer.start() -def on_gate_timeout(tracker: WorkflowTracker, run_id: str, node_name: str, action: str) -> None: +def on_gate_timeout( + tracker: WorkflowTracker, run_id: str, node_name: str, action: GateAction +) -> None: """Handle gate timeout expiry.""" with tracker._state_lock: # If the run was cleaned up (e.g., cancelled before timeout fired), @@ -58,6 +61,6 @@ def on_gate_timeout(tracker: WorkflowTracker, run_id: str, node_name: str, actio if (run_id, node_name) not in tracker._gate_timers: return tracker._gate_timers.pop((run_id, node_name), None) - approved = action == "approve" + approved = action is GateAction.APPROVE error = None if approved else "gate timeout" tracker.resolve_gate(run_id, node_name, approved=approved, error=error) diff --git a/sdks/python/taskito/workflows/types.py b/sdks/python/taskito/workflows/types.py index 6d05452a..1fd33c57 100644 --- a/sdks/python/taskito/workflows/types.py +++ b/sdks/python/taskito/workflows/types.py @@ -6,6 +6,33 @@ from dataclasses import dataclass, field +class GateAction(str, enum.Enum): + """What an approval gate does when its timeout elapses.""" + + APPROVE = "approve" + """Treat the gate as approved — the node completes and successors run.""" + + REJECT = "reject" + """Treat the gate as rejected — the node fails.""" + + +class FanStrategy(str, enum.Enum): + """How a step fans out over, or back in from, its predecessor's result.""" + + EACH = "each" + """Fan-out: one child job per item of the predecessor's result list.""" + + ALL = "all" + """Fan-in: collect every fan-out child's result into one list.""" + + +class DiagramFormat(str, enum.Enum): + """Render target for :meth:`Workflow.visualize`.""" + + MERMAID = "mermaid" + DOT = "dot" + + class WorkflowState(str, enum.Enum): """Terminal and intermediate states of a workflow run.""" diff --git a/sdks/python/tests/python/test_closed_set_enums.py b/sdks/python/tests/python/test_closed_set_enums.py new file mode 100644 index 00000000..f39bafd9 --- /dev/null +++ b/sdks/python/tests/python/test_closed_set_enums.py @@ -0,0 +1,83 @@ +"""Closed-set options accept an enum or its wire string, and reject anything else.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from taskito import InterceptionMode, PredicateAction, Queue +from taskito.mixins._log_consumer import ConsumerErrorAction +from taskito.workflows.types import DiagramFormat, FanStrategy, GateAction + + +def test_wire_values_are_the_cross_sdk_contract() -> None: + """The persisted/wire form of every closed set is its lowercase string.""" + assert InterceptionMode.STRICT.value == "strict" + assert PredicateAction.CANCEL.value == "cancel" + assert ConsumerErrorAction.SKIP.value == "skip" + assert GateAction.APPROVE.value == "approve" + assert FanStrategy.EACH.value == "each" + assert DiagramFormat.DOT.value == "dot" + # A (str, Enum) member carries its value as its string content, which is what + # the native layer reads — str() would render "FanStrategy.EACH" instead. + assert FanStrategy.EACH.encode() == b"each" + + +def test_interception_accepts_enum_and_string(tmp_path: Path) -> None: + enum_queue = Queue(db_path=str(tmp_path / "a.db"), interception=InterceptionMode.LENIENT) + string_queue = Queue(db_path=str(tmp_path / "b.db"), interception="lenient") + assert enum_queue._interceptor is not None + assert enum_queue._interceptor.mode is InterceptionMode.LENIENT + assert string_queue._interceptor is not None + assert string_queue._interceptor.mode is InterceptionMode.LENIENT + + +def test_interception_typo_raises_naming_the_valid_set(tmp_path: Path) -> None: + with pytest.raises(ValueError, match="'strict', 'lenient', 'off'"): + Queue(db_path=str(tmp_path / "c.db"), interception="strictt") + + +def test_on_false_accepts_enum_and_string(queue: Queue) -> None: + @queue.task(predicate=lambda: False, on_false=PredicateAction.CANCEL) + def cancels() -> None: + """Registered only to record its on_false setting.""" + + @queue.task(predicate=lambda: False, on_false="defer") + def defers() -> None: + """Registered only to record its on_false setting.""" + + stored = queue._task_predicate_on_false + assert stored[cancels._task_name] is PredicateAction.CANCEL + assert stored[defers._task_name] is PredicateAction.DEFER + + +def test_on_false_typo_raises(queue: Queue) -> None: + with pytest.raises(ValueError, match="'defer', 'cancel'"): + + @queue.task(predicate=lambda: False, on_false="canccel") + def bad() -> None: + """Never registered — the option is rejected first.""" + + +def test_log_consumer_on_error_typo_raises(queue: Queue) -> None: + with pytest.raises(ValueError, match="'retry', 'skip'"): + + @queue.log_consumer("topic", on_error="ignore") + def bad(message: object) -> None: + """Never registered — the option is rejected first.""" + + +def test_unknown_visualize_format_raises_instead_of_defaulting(queue: Queue) -> None: + """Previously anything that wasn't 'dot' silently rendered Mermaid.""" + from taskito.workflows import Workflow + + @queue.task() + def step() -> None: + """A single node, enough to render.""" + + workflow = Workflow("wf").step("a", step) + assert workflow.visualize(DiagramFormat.DOT).startswith("digraph") + assert "graph" in workflow.visualize("mermaid") + with pytest.raises(ValueError, match="'mermaid', 'dot'"): + workflow.visualize("graphviz") From cdcd0c054d63a2cf5df4d5d2b63a421093cdc697 Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:40:46 +0530 Subject: [PATCH 04/22] feat: type the dashboard user role across the SDKs Replaces three parallel VALID_ROLES sets. A persisted role that no longer parses now falls back to viewer rather than being trusted. --- .../taskito/dashboard/auth/AuthHandlers.java | 2 +- .../taskito/dashboard/auth/AuthStore.java | 38 +++++++-------- .../taskito/dashboard/auth/Policy.java | 2 +- .../byteveda/taskito/dashboard/auth/Role.java | 43 +++++++++++++++++ .../taskito/dashboard/auth/Session.java | 2 +- .../taskito/dashboard/auth/TokenAuth.java | 2 +- .../byteveda/taskito/dashboard/auth/User.java | 2 +- .../taskito/dashboard/DashboardAuthTest.java | 5 +- .../taskito/dashboard/DashboardClient.java | 5 +- .../taskito/dashboard/auth/AuthStoreTest.java | 47 +++++++++--------- .../dashboard/oauth/OAuthFlowTest.java | 3 +- sdks/node/src/dashboard/auth/store.ts | 33 +++++++++---- sdks/node/src/dashboard/index.ts | 2 + sdks/node/src/index.ts | 5 ++ sdks/python/taskito/dashboard/auth.py | 48 +++++++++++++------ 15 files changed, 161 insertions(+), 78 deletions(-) create mode 100644 sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Role.java diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthHandlers.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthHandlers.java index 90f6da1b..129fcc6a 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthHandlers.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthHandlers.java @@ -27,7 +27,7 @@ public Map setup(Map body) { } String username = requireField(body, "username"); String password = requireField(body, "password"); - User user = store.createUser(username, password, AuthStore.ROLE_ADMIN); + User user = store.createUser(username, password, Role.ADMIN); return Map.of("user", serializeUser(user)); } diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java index b82d656f..8d84252a 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java @@ -28,9 +28,6 @@ public final class AuthStore { public static final String SESSION_PREFIX = "auth:session:"; public static final long DEFAULT_SESSION_TTL_SECONDS = 24 * 60 * 60; - public static final String ROLE_ADMIN = "admin"; - public static final String ROLE_VIEWER = "viewer"; - public static final Set VALID_ROLES = Set.of(ROLE_ADMIN, ROLE_VIEWER); public static final String ENV_ADMIN_USER = "TASKITO_DASHBOARD_ADMIN_USER"; public static final String ENV_ADMIN_PASSWORD = "TASKITO_DASHBOARD_ADMIN_PASSWORD"; @@ -63,10 +60,12 @@ public Optional getUser(String username) { * cross-process CAS, so a second writer in another process is still a * theoretical race (bounded to first-run setup, which is single-shot). */ - public synchronized User createUser(String username, String password, String role) { + public synchronized User createUser(String username, String password, Role role) { validateUsername(username); validatePassword(password); - validateRole(role); + if (role == null) { + throw DashboardError.badRequest("invalid role"); + } if (rawUsers().containsKey(username)) { throw DashboardError.badRequest("user already exists"); } @@ -78,7 +77,7 @@ public synchronized User createUser(String username, String password, String rol long now = nowMillis(); Map row = new LinkedHashMap<>(); row.put("password_hash", hash); - row.put("role", role); + row.put("role", role.wire()); row.put("created_at", now); row.put("last_login_at", null); users.put(username, row); @@ -171,11 +170,11 @@ public synchronized User getOrCreateOauthUser( saveUsers(users); return toUser(username, row); } - String role = oauthBootstrapRole(email, emailVerified, adminEmails); + Role role = oauthBootstrapRole(email, emailVerified, adminEmails); long now = nowMillis(); Map row = new LinkedHashMap<>(); row.put("password_hash", PasswordHasher.oauthSentinel(slot)); - row.put("role", role); + row.put("role", role.wire()); row.put("created_at", now); row.put("last_login_at", now); row.put("email", email); @@ -190,30 +189,30 @@ public synchronized User getOrCreateOauthUser( * listed address. Everyone else — including the very first user — gets * viewer, so a stray first OAuth login can never win admin. */ - public static String oauthBootstrapRole(String email, boolean emailVerified, List adminEmails) { + public static Role oauthBootstrapRole(String email, boolean emailVerified, List adminEmails) { if (!emailVerified || email == null || email.isBlank()) { - return ROLE_VIEWER; + return Role.VIEWER; } String normalised = email.toLowerCase(Locale.ROOT); boolean listed = adminEmails != null && adminEmails.stream().anyMatch(e -> e.toLowerCase(Locale.ROOT).equals(normalised)); - return listed ? ROLE_ADMIN : ROLE_VIEWER; + return listed ? Role.ADMIN : Role.VIEWER; } // ---- sessions ---------------------------------------------------------- - public Session createSession(String username, String role) { + public Session createSession(String username, Role role) { return createSession(username, role, DEFAULT_SESSION_TTL_SECONDS); } - public Session createSession(String username, String role, long ttlSeconds) { + public Session createSession(String username, Role role, long ttlSeconds) { String token = Tokens.session(); String csrf = Tokens.session(); long now = nowSeconds(); long expires = now + ttlSeconds; Map row = new LinkedHashMap<>(); row.put("username", username); - row.put("role", role); + row.put("role", role.wire()); row.put("created_at", now); row.put("expires_at", expires); row.put("csrf_token", csrf); @@ -239,7 +238,7 @@ public Optional getSession(String token) { session = new Session( token, (String) data.get("username"), - (String) data.get("role"), + Role.orViewer((String) data.get("role")), asLong(data.get("created_at")), asLong(data.get("expires_at")), (String) data.get("csrf_token")); @@ -317,7 +316,7 @@ void bootstrapAdminFromEnv(Map env) { if (getUser(username).isPresent()) { return; } - createUser(username, password, ROLE_ADMIN); + createUser(username, password, Role.ADMIN); } // ---- helpers ----------------------------------------------------------- @@ -339,7 +338,7 @@ private static User toUser(String username, Map row) { return new User( username, (String) row.get("password_hash"), - (String) row.get("role"), + Role.orViewer((String) row.get("role")), asLong(row.get("created_at")), row.get("last_login_at") == null ? null : asLong(row.get("last_login_at")), (String) row.get("email"), @@ -379,9 +378,4 @@ private static void validatePassword(String password) { } } - private static void validateRole(String role) { - if (!VALID_ROLES.contains(role)) { - throw DashboardError.badRequest("invalid role"); - } - } } diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java index f8c3b56d..5c7c991c 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java @@ -65,7 +65,7 @@ public static void authorize(String path, String method, RequestContext ctx, Aut throw DashboardError.forbidden("csrf_failed"); } if (requiresAdmin(path, method) - && !AuthStore.ROLE_ADMIN.equals(ctx.session().role())) { + && ctx.session().role() != Role.ADMIN) { throw DashboardError.forbidden("forbidden"); } } diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Role.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Role.java new file mode 100644 index 00000000..04bf1480 --- /dev/null +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Role.java @@ -0,0 +1,43 @@ +package org.byteveda.taskito.dashboard.auth; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonValue; +import java.util.Locale; + +/** A dashboard user's access level. Wire form is the lowercase name, stored in settings. */ +public enum Role { + /** Full read/write access to jobs, queues, and users. */ + ADMIN, + /** Read-only access. */ + VIEWER; + + /** Lowercase wire form persisted in the settings store and sent over the API. */ + @JsonValue + public String wire() { + return name().toLowerCase(Locale.ROOT); + } + + /** Parse a wire form ({@code "admin"}/{@code "viewer"}); null for anything else. */ + @JsonCreator + public static Role fromWire(String wire) { + if (wire == null) { + return null; + } + for (Role role : values()) { + if (role.wire().equals(wire)) { + return role; + } + } + return null; + } + + /** + * Read a persisted role, failing closed. Rows predate this enum, so an + * unrecognized value yields the least-privileged role rather than locking the + * user out or granting more than was stored. + */ + public static Role orViewer(String wire) { + Role role = fromWire(wire); + return role == null ? VIEWER : role; + } +} diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Session.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Session.java index 320d78d3..889ed04a 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Session.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Session.java @@ -6,7 +6,7 @@ * this matches the reference wire contract exactly. The {@code token} is the KV * key suffix and is never serialised into the stored record. */ -public record Session(String token, String username, String role, long createdAt, long expiresAt, String csrfToken) { +public record Session(String token, String username, Role role, long createdAt, long expiresAt, String csrfToken) { public boolean isExpired(long nowSeconds) { return nowSeconds >= expiresAt; diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/TokenAuth.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/TokenAuth.java index 49c049ab..3f330158 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/TokenAuth.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/TokenAuth.java @@ -57,7 +57,7 @@ public static Map openStatus() { public static Map openWhoami() { Map user = new LinkedHashMap<>(); user.put("username", "viewer"); - user.put("role", AuthStore.ROLE_ADMIN); + user.put("role", Role.ADMIN.wire()); user.put("created_at", 0); user.put("last_login_at", null); Map out = new LinkedHashMap<>(); diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/User.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/User.java index 0d61ebbb..59693e27 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/User.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/User.java @@ -9,7 +9,7 @@ public record User( String username, String passwordHash, - String role, + Role role, long createdAt, Long lastLoginAt, String email, diff --git a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardAuthTest.java b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardAuthTest.java index 87b248e0..68b2b194 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardAuthTest.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardAuthTest.java @@ -11,6 +11,7 @@ import java.nio.file.Path; import java.util.List; import org.byteveda.taskito.Taskito; +import org.byteveda.taskito.dashboard.auth.Role; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.io.TempDir; @@ -130,7 +131,7 @@ void viewersCannotWrite(@TempDir Path dir) throws Exception { DashboardServer server = DashboardServer.start(queue, 0, true)) { DashboardClient.seedAdmin(queue); // keep setup satisfied DashboardClient viewer = - new DashboardClient(server.port()).as(DashboardClient.seedUser(queue, "read-only", "viewer")); + new DashboardClient(server.port()).as(DashboardClient.seedUser(queue, "read-only", Role.VIEWER)); assertEquals(200, viewer.get("/api/stats").statusCode()); assertEquals(403, viewer.post("/api/queues/emails/pause", null).statusCode()); } @@ -151,7 +152,7 @@ void changePasswordRotatesCredential(@TempDir Path dir) throws Exception { try (Taskito queue = open(dir); DashboardServer server = DashboardServer.start(queue, 0, true)) { int port = server.port(); - DashboardClient client = new DashboardClient(port).as(DashboardClient.seedUser(queue, "root", "admin")); + DashboardClient client = new DashboardClient(port).as(DashboardClient.seedUser(queue, "root", Role.ADMIN)); HttpResponse changed = client.post( "/api/auth/change-password", "{\"old_password\":\"password123\",\"new_password\":\"brand-new-1\"}"); assertEquals(200, changed.statusCode()); diff --git a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardClient.java b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardClient.java index ba481926..b78f5fd6 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardClient.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardClient.java @@ -6,6 +6,7 @@ import java.net.http.HttpResponse; import org.byteveda.taskito.Taskito; import org.byteveda.taskito.dashboard.auth.AuthStore; +import org.byteveda.taskito.dashboard.auth.Role; import org.byteveda.taskito.dashboard.auth.Session; import org.byteveda.taskito.dashboard.store.SettingsAccess; @@ -88,10 +89,10 @@ static AuthStore store(Taskito queue) { } static Session seedAdmin(Taskito queue) { - return seedUser(queue, "admin", "admin"); + return seedUser(queue, "admin", Role.ADMIN); } - static Session seedUser(Taskito queue, String username, String role) { + static Session seedUser(Taskito queue, String username, Role role) { AuthStore store = store(queue); store.createUser(username, "password123", role); return store.createSession(username, role); diff --git a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java index 66225799..9163fea5 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java @@ -23,9 +23,9 @@ private AuthStore store() { void createsAndAuthenticatesUsers() { AuthStore store = store(); assertEquals(0, store.countUsers()); - User user = store.createUser("alice", "password123", "admin"); + User user = store.createUser("alice", "password123", Role.ADMIN); assertEquals("alice", user.username()); - assertEquals("admin", user.role()); + assertEquals(Role.ADMIN, user.role()); assertNull(user.lastLoginAt()); assertEquals(1, store.countUsers()); @@ -42,30 +42,33 @@ void createsAndAuthenticatesUsers() { @Test void rejectsDuplicatesAndInvalidInput() { AuthStore store = store(); - store.createUser("bob", "password123", "admin"); - assertThrows(DashboardError.class, () -> store.createUser("bob", "password123", "admin")); - assertThrows(DashboardError.class, () -> store.createUser("bad name", "password123", "admin")); - assertThrows(DashboardError.class, () -> store.createUser("carol", "short", "admin")); - assertThrows(DashboardError.class, () -> store.createUser("carol", "password123", "superuser")); + store.createUser("bob", "password123", Role.ADMIN); + assertThrows(DashboardError.class, () -> store.createUser("bob", "password123", Role.ADMIN)); + assertThrows(DashboardError.class, () -> store.createUser("bad name", "password123", Role.ADMIN)); + assertThrows(DashboardError.class, () -> store.createUser("carol", "short", Role.ADMIN)); + assertThrows(DashboardError.class, () -> store.createUser("carol", "password123", null)); + // An unknown role is no longer representable at the call site; a stored one + // still has to fail closed. + assertEquals(Role.VIEWER, Role.orViewer("superuser")); } @Test void sessionsUseSecondsAndExpire() { AuthStore store = store(); - Session session = store.createSession("alice", "admin"); + Session session = store.createSession("alice", Role.ADMIN); assertEquals(AuthStore.DEFAULT_SESSION_TTL_SECONDS, session.expiresAt() - session.createdAt()); assertTrue(session.createdAt() < 1_000_000_000_000L); // seconds, not millis assertTrue(store.getSession(session.token()).isPresent()); - Session expired = store.createSession("alice", "admin", -10); + Session expired = store.createSession("alice", Role.ADMIN, -10); assertTrue(store.getSession(expired.token()).isEmpty()); } @Test void prunesExpiredSessions() { AuthStore store = store(); - Session valid = store.createSession("alice", "admin", 3600); - store.createSession("alice", "admin", -10); + Session valid = store.createSession("alice", Role.ADMIN, 3600); + store.createSession("alice", Role.ADMIN, -10); assertEquals(1, store.pruneExpiredSessions()); assertTrue(store.getSession(valid.token()).isPresent()); } @@ -73,8 +76,8 @@ void prunesExpiredSessions() { @Test void deletingUserClearsSessions() { AuthStore store = store(); - store.createUser("alice", "password123", "admin"); - Session session = store.createSession("alice", "admin"); + store.createUser("alice", "password123", Role.ADMIN); + Session session = store.createSession("alice", Role.ADMIN); store.deleteUser("alice"); assertEquals(0, store.countUsers()); assertTrue(store.getSession(session.token()).isEmpty()); @@ -83,13 +86,13 @@ void deletingUserClearsSessions() { @Test void oauthBootstrapRolePrecedence() { // Unverified / missing email never becomes admin. - assertEquals("viewer", AuthStore.oauthBootstrapRole("a@x.com", false, List.of())); - assertEquals("viewer", AuthStore.oauthBootstrapRole(null, true, List.of())); + assertEquals(Role.VIEWER, AuthStore.oauthBootstrapRole("a@x.com", false, List.of())); + assertEquals(Role.VIEWER, AuthStore.oauthBootstrapRole(null, true, List.of())); // Admin-email list is the only path to admin (case-insensitive). - assertEquals("admin", AuthStore.oauthBootstrapRole("A@X.com", true, List.of("a@x.com"))); - assertEquals("viewer", AuthStore.oauthBootstrapRole("b@x.com", true, List.of("a@x.com"))); + assertEquals(Role.ADMIN, AuthStore.oauthBootstrapRole("A@X.com", true, List.of("a@x.com"))); + assertEquals(Role.VIEWER, AuthStore.oauthBootstrapRole("b@x.com", true, List.of("a@x.com"))); // No admin list: everyone — including the first user — is a viewer. - assertEquals("viewer", AuthStore.oauthBootstrapRole("c@x.com", true, List.of())); + assertEquals(Role.VIEWER, AuthStore.oauthBootstrapRole("c@x.com", true, List.of())); } @Test @@ -97,12 +100,12 @@ void getOrCreateOauthUserProvisionsThenRefreshes() { AuthStore store = store(); User created = store.getOrCreateOauthUser("google", "123", "a@x.com", "Ann", true, List.of("a@x.com")); assertEquals("google:123", created.username()); - assertEquals("admin", created.role()); // allowlisted + assertEquals(Role.ADMIN, created.role()); // allowlisted assertTrue(created.isOauth()); User refreshed = store.getOrCreateOauthUser("google", "123", "new@x.com", "Ann N", true, List.of("z@x.com")); assertEquals("google:123", refreshed.username()); - assertEquals("admin", refreshed.role()); // role preserved + assertEquals(Role.ADMIN, refreshed.role()); // role preserved assertEquals("new@x.com", refreshed.email()); assertEquals(1, store.countUsers()); } @@ -127,8 +130,8 @@ void envBootstrapIsIdempotent() { @Test void changePasswordInvalidatesOldCredential() { AuthStore store = store(); - store.createUser("alice", "password123", "admin"); - Session existing = store.createSession("alice", "admin"); + store.createUser("alice", "password123", Role.ADMIN); + Session existing = store.createSession("alice", Role.ADMIN); store.updatePassword("alice", "new-password!"); assertNull(store.authenticate("alice", "password123")); assertNotNull(store.authenticate("alice", "new-password!")); diff --git a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/oauth/OAuthFlowTest.java b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/oauth/OAuthFlowTest.java index 3ac6c828..b891bd57 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/oauth/OAuthFlowTest.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/oauth/OAuthFlowTest.java @@ -9,6 +9,7 @@ import java.util.Map; import org.byteveda.taskito.dashboard.InMemorySettings; import org.byteveda.taskito.dashboard.auth.AuthStore; +import org.byteveda.taskito.dashboard.auth.Role; import org.byteveda.taskito.dashboard.auth.oauth.OAuthFlow; import org.byteveda.taskito.dashboard.auth.oauth.OAuthStateStore; import org.byteveda.taskito.dashboard.auth.oauth.config.OAuthConfig; @@ -58,7 +59,7 @@ void handleCallbackLandsSessionAsViewerWithoutAllowlist() { String state = fake.lastState.state(); OAuthFlow.CallbackResult result = flow.handleCallback("fake", "code", state, null); assertEquals("fake:subject-1", result.session().username()); - assertEquals("viewer", result.session().role()); // admin comes only from the allowlist + assertEquals(Role.VIEWER, result.session().role()); // admin comes only from the allowlist assertEquals("/next", result.nextUrl()); assertTrue(authStore.getUser("fake:subject-1").isPresent()); } diff --git a/sdks/node/src/dashboard/auth/store.ts b/sdks/node/src/dashboard/auth/store.ts index 1525fdc3..3d11a938 100644 --- a/sdks/node/src/dashboard/auth/store.ts +++ b/sdks/node/src/dashboard/auth/store.ts @@ -40,7 +40,11 @@ export const DEFAULT_SESSION_TTL_SECONDS = 24 * 60 * 60; const USERNAME_MAX_LEN = 64; const PASSWORD_MIN_LEN = 8; const PASSWORD_MAX_LEN = 256; -const VALID_ROLES = new Set(["admin", "viewer"]); +/** A dashboard user's access level; the wire form is stored in settings. */ +export type Role = "admin" | "viewer"; + +/** Every role, for runtime validation. */ +export const ROLES: readonly Role[] = ["admin", "viewer"]; const USERNAME_RE = /^[\p{L}\p{N}._-]+$/u; /** Sentinel `password_hash` prefix for OAuth-only users — password login always fails. */ @@ -100,7 +104,7 @@ export function generateSessionToken(): string { export interface DashboardUser { username: string; passwordHash: string; - role: string; + role: Role; createdAt: number; lastLoginAt: number | null; email: string | null; @@ -111,7 +115,7 @@ export interface DashboardUser { export interface DashboardSession { token: string; username: string; - role: string; + role: Role; createdAt: number; expiresAt: number; csrfToken: string; @@ -148,12 +152,17 @@ function validatePassword(password: string): void { } } -function validateRole(role: string): void { - if (!VALID_ROLES.has(role)) { - throw new ValidationError(`role must be one of ${[...VALID_ROLES].sort().join(", ")}`); +function validateRole(role: string): asserts role is Role { + if (!(ROLES as readonly string[]).includes(role)) { + throw new ValidationError(`role must be one of ${[...ROLES].sort().join(", ")}`); } } +/** Read a persisted role, failing closed on anything unrecognized. */ +function roleOrViewer(stored: unknown): Role { + return (ROLES as readonly unknown[]).includes(stored) ? (stored as Role) : "viewer"; +} + /** * Role for a freshly-created OAuth user. `admin` requires a verified email * AND membership in the admin list — everyone else, including the very first @@ -163,7 +172,7 @@ export function oauthBootstrapRole(options: { email: string | null; emailVerified: boolean; adminEmails: readonly string[]; -}): string { +}): Role { if (!options.emailVerified || !options.email) { return "viewer"; } @@ -229,7 +238,11 @@ export class AuthStore { return row ? rowToUser(username, row) : undefined; } - async createUser(username: string, password: string, role = "admin"): Promise { + async createUser( + username: string, + password: string, + role: Role | string = "admin", + ): Promise { validateUsername(username); validatePassword(password); validateRole(role); @@ -406,7 +419,7 @@ export class AuthStore { const session: DashboardSession = { token, username: row.username, - role: row.role, + role: roleOrViewer(row.role), createdAt: row.created_at ?? 0, expiresAt: row.expires_at, csrfToken: row.csrf_token, @@ -449,7 +462,7 @@ function rowToUser(username: string, row: UserRow): DashboardUser { return { username, passwordHash: row.password_hash, - role: row.role, + role: roleOrViewer(row.role), createdAt: Number(row.created_at) || 0, lastLoginAt: row.last_login_at === null || row.last_login_at === undefined diff --git a/sdks/node/src/dashboard/index.ts b/sdks/node/src/dashboard/index.ts index 6cb23ec7..e984acf2 100644 --- a/sdks/node/src/dashboard/index.ts +++ b/sdks/node/src/dashboard/index.ts @@ -92,6 +92,8 @@ export { type OAuthProvider, oauthConfigFromEnv, type ProviderIdentity, + ROLES, + type Role, } from "./auth"; export { createDashboardHandler, diff --git a/sdks/node/src/index.ts b/sdks/node/src/index.ts index 2ec7526a..01dcf817 100644 --- a/sdks/node/src/index.ts +++ b/sdks/node/src/index.ts @@ -12,6 +12,8 @@ export { type OAuthProvider, oauthConfigFromEnv, type ProviderIdentity, + ROLES, + type Role, serveDashboard, } from "./dashboard"; export { @@ -84,6 +86,7 @@ export type { CursorJobFilter, DeadJob, DetailedJobFilter, + DispatchOrder, EffectiveRetention, EnqueueOptions, Job, @@ -105,11 +108,13 @@ export type { Subscription, TaskHandler, TaskLog, + TaskLogLevel, TaskMap, TaskOptions, WorkerInfo, WorkerRunOptions, } from "./types"; +export { DISPATCH_ORDERS, TASK_LOG_LEVELS } from "./types"; export { createLogger, type Logger, diff --git a/sdks/python/taskito/dashboard/auth.py b/sdks/python/taskito/dashboard/auth.py index 018d6be7..8d10dfc2 100644 --- a/sdks/python/taskito/dashboard/auth.py +++ b/sdks/python/taskito/dashboard/auth.py @@ -18,6 +18,7 @@ from __future__ import annotations +import enum import hashlib import hmac import json @@ -28,6 +29,8 @@ from dataclasses import asdict, dataclass from typing import TYPE_CHECKING +from taskito.enums import coerce_enum + if TYPE_CHECKING: from taskito.app import Queue @@ -56,7 +59,17 @@ USERNAME_MAX_LEN = 64 PASSWORD_MIN_LEN = 8 PASSWORD_MAX_LEN = 256 -VALID_ROLES = frozenset({"admin", "viewer"}) + + +class Role(str, enum.Enum): + """A dashboard user's access level. The wire form is stored in settings.""" + + ADMIN = "admin" + """Full read/write access to jobs, queues, and users.""" + + VIEWER = "viewer" + """Read-only access.""" + # Sentinel prefix used in ``password_hash`` for OAuth-only users so # ``verify_password`` can short-circuit-reject any password attempt. @@ -124,7 +137,7 @@ class User: username: str password_hash: str - role: str + role: Role created_at: int last_login_at: int | None = None email: str | None = None @@ -141,7 +154,7 @@ class Session: token: str username: str - role: str + role: Role created_at: int expires_at: int csrf_token: str @@ -169,9 +182,16 @@ def _validate_password(password: str) -> None: raise ValueError(f"password must be <= {PASSWORD_MAX_LEN} chars") -def _validate_role(role: str) -> None: - if role not in VALID_ROLES: - raise ValueError(f"role must be one of {sorted(VALID_ROLES)}") +def _validate_role(role: Role | str) -> Role: + return coerce_enum(Role, role, param="role") + + +def _role_or_viewer(stored: object) -> Role: + """Read a persisted role, failing closed on anything unrecognized.""" + try: + return Role(stored) + except ValueError: + return Role.VIEWER def _oauth_bootstrap_role( @@ -179,7 +199,7 @@ def _oauth_bootstrap_role( email: str | None, email_verified: bool, admin_emails: tuple[str, ...], -) -> str: +) -> Role: """Decide the role for a freshly-created OAuth user. ``admin`` requires a verified email (defence against spoofed claims) @@ -189,10 +209,10 @@ def _oauth_bootstrap_role( first OAuth login can never win admin. """ if not email_verified or not email: - return "viewer" + return Role.VIEWER if email.lower() in {e.lower() for e in admin_emails}: - return "admin" - return "viewer" + return Role.ADMIN + return Role.VIEWER # ── Auth store ───────────────────────────────────────────────────────── @@ -230,17 +250,17 @@ def get_user(self, username: str) -> User | None: row = self._load_users().get(username) return self._row_to_user(username, row) if row else None - def create_user(self, username: str, password: str, role: str = "admin") -> User: + def create_user(self, username: str, password: str, role: Role | str = Role.ADMIN) -> User: _validate_username(username) _validate_password(password) - _validate_role(role) + validated_role = _validate_role(role) users = self._load_users() if username in users: raise ValueError(f"user '{username}' already exists") now_ms = int(time.time() * 1000) users[username] = { "password_hash": hash_password(password), - "role": role, + "role": validated_role, "created_at": now_ms, "last_login_at": None, } @@ -289,7 +309,7 @@ def _row_to_user(username: str, row: dict[str, object] | None) -> User: return User( username=username, password_hash=str(row["password_hash"]), - role=str(row["role"]), + role=_role_or_viewer(row.get("role")), created_at=int(created_raw) if isinstance(created_raw, (int, float, str)) else 0, last_login_at=(int(last_raw) if isinstance(last_raw, (int, float, str)) else None), email=str(email_raw) if isinstance(email_raw, str) and email_raw else None, From fb7febd808b4dd5b235eaf5c985e058c33c82304 Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:11:12 +0530 Subject: [PATCH 05/22] feat(core): type subscription mode and worker status Both were unvalidated strings threaded through the storage trait; the shells parse at the boundary so a legacy row still reads. --- crates/taskito-core/src/pubsub.rs | 33 +++-- .../src/storage/diesel_common/pubsub.rs | 12 +- .../src/storage/diesel_common/workers.rs | 8 +- crates/taskito-core/src/storage/mod.rs | 13 +- crates/taskito-core/src/storage/models.rs | 6 +- .../src/storage/postgres/pubsub.rs | 13 +- crates/taskito-core/src/storage/records.rs | 133 ++++++++++++++++-- .../src/storage/redis_backend/pubsub.rs | 29 ++-- .../src/storage/redis_backend/workers.rs | 8 +- .../taskito-core/src/storage/sqlite/pubsub.rs | 13 +- .../taskito-core/src/storage/sqlite/tests.rs | 2 +- crates/taskito-core/src/storage/traits.rs | 15 +- .../taskito-core/tests/rust/storage_tests.rs | 21 +-- crates/taskito-java/src/convert.rs | 2 +- crates/taskito-java/src/queue/pubsub.rs | 6 +- crates/taskito-java/src/worker.rs | 2 +- crates/taskito-node/src/convert/pubsub.rs | 2 +- crates/taskito-node/src/queue/pubsub.rs | 10 +- crates/taskito-python/src/py_queue/pubsub.rs | 14 +- crates/taskito-python/src/py_queue/worker.rs | 6 +- .../taskito/dashboard/auth/AuthStore.java | 3 - 21 files changed, 252 insertions(+), 99 deletions(-) diff --git a/crates/taskito-core/src/pubsub.rs b/crates/taskito-core/src/pubsub.rs index ac8dd28c..6e3be654 100644 --- a/crates/taskito-core/src/pubsub.rs +++ b/crates/taskito-core/src/pubsub.rs @@ -7,14 +7,17 @@ use crate::error::{QueueError, Result}; use crate::job::{now_millis, Job, NewJob}; -use crate::storage::records::{Subscription, SUBSCRIPTION_MODE_LOG}; +use crate::storage::records::{Subscription, SubscriptionMode}; use crate::storage::Storage; /// Validate a topic declaration before a backend persists it: only `"log"` /// topics are declarable, and a retention window must be non-negative (a /// negative one would expire messages immediately or overflow `now + retention`). -pub(crate) fn validate_topic_declaration(mode: &str, retention_ms: Option) -> Result<()> { - if mode != SUBSCRIPTION_MODE_LOG { +pub(crate) fn validate_topic_declaration( + mode: SubscriptionMode, + retention_ms: Option, +) -> Result<()> { + if !mode.is_log() { return Err(QueueError::Config(format!( "only \"log\" topics are declarable, got {mode:?}" ))); @@ -85,9 +88,7 @@ pub struct PublishRequest { /// on the unique index; unkeyed publishes use one batch insert. pub fn publish_to_topic(storage: &S, request: &PublishRequest) -> Result> { let subscriptions = storage.list_subscriptions_for_topic(&request.topic)?; - let has_log_sub = subscriptions - .iter() - .any(|s| s.mode == SUBSCRIPTION_MODE_LOG); + let has_log_sub = subscriptions.iter().any(|s| s.mode.is_log()); // A log topic stores one durable message that consumers pull via cursor; // fan-out subscribers still get one job each. A topic may mix both modes. @@ -121,7 +122,7 @@ pub fn publish_to_topic(storage: &S, request: &PublishRequest) -> Re let jobs: Vec = subscriptions .iter() - .filter(|sub| sub.mode != SUBSCRIPTION_MODE_LOG) + .filter(|sub| !sub.mode.is_log()) .map(|sub| delivery_job(request, sub)) .collect(); if jobs.is_empty() { @@ -271,7 +272,7 @@ mod tests { priority, max_retries, timeout_ms, - mode: crate::storage::records::SUBSCRIPTION_MODE_FANOUT.to_string(), + mode: SubscriptionMode::Fanout, }) .unwrap(); } @@ -297,7 +298,7 @@ mod tests { priority: None, max_retries: None, timeout_ms: None, - mode: SUBSCRIPTION_MODE_LOG.to_string(), + mode: SubscriptionMode::Log, }) .unwrap(); } @@ -446,7 +447,7 @@ mod tests { fn declared_log_topic_retains_with_zero_subscribers() { let storage = SqliteStorage::in_memory().unwrap(); storage - .declare_topic("events", SUBSCRIPTION_MODE_LOG, None) + .declare_topic("events", SubscriptionMode::Log, None) .unwrap(); // No subscriber at publish time, but the topic is declared → retained. @@ -478,7 +479,7 @@ mod tests { fn declared_topic_retention_sets_message_expiry() { let storage = SqliteStorage::in_memory().unwrap(); storage - .declare_topic("events", SUBSCRIPTION_MODE_LOG, Some(60_000)) + .declare_topic("events", SubscriptionMode::Log, Some(60_000)) .unwrap(); publish_to_topic(&storage, &request("events", None)).unwrap(); @@ -494,7 +495,7 @@ mod tests { assert!(storage.get_topic("events").unwrap().is_none()); storage - .declare_topic("events", SUBSCRIPTION_MODE_LOG, Some(1000)) + .declare_topic("events", SubscriptionMode::Log, Some(1000)) .unwrap(); let first = storage.get_topic("events").unwrap().unwrap(); assert_eq!(first.name, "events"); @@ -506,7 +507,7 @@ mod tests { // regression that overwrote created_at could still pass this assertion. std::thread::sleep(std::time::Duration::from_millis(2)); storage - .declare_topic("events", SUBSCRIPTION_MODE_LOG, Some(2000)) + .declare_topic("events", SubscriptionMode::Log, Some(2000)) .unwrap(); let second = storage.get_topic("events").unwrap().unwrap(); assert_eq!(second.retention_ms, Some(2000)); @@ -517,9 +518,11 @@ mod tests { #[test] fn declare_topic_rejects_bad_mode_and_negative_retention() { let storage = SqliteStorage::in_memory().unwrap(); - assert!(storage.declare_topic("events", "fanout", None).is_err()); assert!(storage - .declare_topic("events", SUBSCRIPTION_MODE_LOG, Some(-1)) + .declare_topic("events", SubscriptionMode::Fanout, None) + .is_err()); + assert!(storage + .declare_topic("events", SubscriptionMode::Log, Some(-1)) .is_err()); // Nothing was persisted on rejection. assert!(storage.get_topic("events").unwrap().is_none()); diff --git a/crates/taskito-core/src/storage/diesel_common/pubsub.rs b/crates/taskito-core/src/storage/diesel_common/pubsub.rs index 2bcc02f9..9684cabb 100644 --- a/crates/taskito-core/src/storage/diesel_common/pubsub.rs +++ b/crates/taskito-core/src/storage/diesel_common/pubsub.rs @@ -210,7 +210,7 @@ macro_rules! impl_diesel_pubsub_ops { .filter(topic_subscriptions::subscription_name.eq(subscription_name)) .filter( topic_subscriptions::mode - .eq($crate::storage::records::SUBSCRIPTION_MODE_LOG), + .eq($crate::storage::records::SubscriptionMode::Log.as_str()), ) .select(topic_subscriptions::cursor) .first(&mut conn) @@ -250,7 +250,7 @@ macro_rules! impl_diesel_pubsub_ops { .filter(topic_subscriptions::subscription_name.eq(subscription_name)) .filter( topic_subscriptions::mode - .eq($crate::storage::records::SUBSCRIPTION_MODE_LOG), + .eq($crate::storage::records::SubscriptionMode::Log.as_str()), ) .filter( topic_subscriptions::cursor @@ -271,7 +271,7 @@ macro_rules! impl_diesel_pubsub_ops { let subs: Vec<(String, String, Option)> = topic_subscriptions::table .filter( topic_subscriptions::mode - .eq($crate::storage::records::SUBSCRIPTION_MODE_LOG), + .eq($crate::storage::records::SubscriptionMode::Log.as_str()), ) .select(( topic_subscriptions::topic, @@ -322,7 +322,7 @@ macro_rules! impl_diesel_pubsub_ops { let subs: Vec<(String, Option)> = topic_subscriptions::table .filter( topic_subscriptions::mode - .eq($crate::storage::records::SUBSCRIPTION_MODE_LOG), + .eq($crate::storage::records::SubscriptionMode::Log.as_str()), ) .select((topic_subscriptions::topic, topic_subscriptions::cursor)) .load(&mut conn)?; @@ -393,7 +393,7 @@ macro_rules! impl_diesel_pubsub_ops { let log_subs: Vec<(String, String)> = topic_subscriptions::table .filter( topic_subscriptions::mode - .eq($crate::storage::records::SUBSCRIPTION_MODE_LOG), + .eq($crate::storage::records::SubscriptionMode::Log.as_str()), ) .select(( topic_subscriptions::topic, @@ -504,7 +504,7 @@ macro_rules! impl_diesel_pubsub_ops { .filter(topic_subscriptions::subscription_name.eq(subscription_name)) .filter( topic_subscriptions::mode - .eq($crate::storage::records::SUBSCRIPTION_MODE_LOG), + .eq($crate::storage::records::SubscriptionMode::Log.as_str()), ) .select(topic_subscriptions::subscription_name) .first::(&mut conn) diff --git a/crates/taskito-core/src/storage/diesel_common/workers.rs b/crates/taskito-core/src/storage/diesel_common/workers.rs index d56fe152..9e302ba7 100644 --- a/crates/taskito-core/src/storage/diesel_common/workers.rs +++ b/crates/taskito-core/src/storage/diesel_common/workers.rs @@ -22,12 +22,16 @@ macro_rules! impl_diesel_worker_ops { } /// Update the status of a worker. - pub fn update_worker_status(&self, worker_id: &str, status: &str) -> Result<()> { + pub fn update_worker_status( + &self, + worker_id: &str, + status: $crate::storage::records::WorkerStatus, + ) -> Result<()> { let mut conn = self.conn()?; diesel::update(workers::table) .filter(workers::worker_id.eq(worker_id)) - .set(workers::status.eq(status)) + .set(workers::status.eq(status.as_str())) .execute(&mut conn)?; Ok(()) diff --git a/crates/taskito-core/src/storage/mod.rs b/crates/taskito-core/src/storage/mod.rs index d67084a8..98daf184 100644 --- a/crates/taskito-core/src/storage/mod.rs +++ b/crates/taskito-core/src/storage/mod.rs @@ -731,7 +731,7 @@ macro_rules! impl_storage { fn declare_topic( &self, name: &str, - mode: &str, + mode: $crate::storage::records::SubscriptionMode, retention_ms: Option, ) -> $crate::error::Result<()> { self.declare_topic(name, mode, retention_ms) @@ -893,7 +893,7 @@ macro_rules! impl_storage { fn update_worker_status( &self, worker_id: &str, - status: &str, + status: $crate::storage::records::WorkerStatus, ) -> $crate::error::Result<()> { self.update_worker_status(worker_id, status) } @@ -1476,7 +1476,12 @@ impl Storage for StorageBackend { fn purge_topic_messages(&self, now: i64, limit: i64) -> Result { delegate!(self, purge_topic_messages, now, limit) } - fn declare_topic(&self, name: &str, mode: &str, retention_ms: Option) -> Result<()> { + fn declare_topic( + &self, + name: &str, + mode: records::SubscriptionMode, + retention_ms: Option, + ) -> Result<()> { delegate!(self, declare_topic, name, mode, retention_ms) } fn get_topic(&self, name: &str) -> Result> { @@ -1634,7 +1639,7 @@ impl Storage for StorageBackend { fn heartbeat(&self, worker_id: &str, resource_health: Option<&str>) -> Result<()> { delegate!(self, heartbeat, worker_id, resource_health) } - fn update_worker_status(&self, worker_id: &str, status: &str) -> Result<()> { + fn update_worker_status(&self, worker_id: &str, status: records::WorkerStatus) -> Result<()> { delegate!(self, update_worker_status, worker_id, status) } fn list_workers(&self) -> Result> { diff --git a/crates/taskito-core/src/storage/models.rs b/crates/taskito-core/src/storage/models.rs index 61e107c8..e2e68ba4 100644 --- a/crates/taskito-core/src/storage/models.rs +++ b/crates/taskito-core/src/storage/models.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize}; use super::records::{ CircuitBreakerState, JobError, LockInfo, PeriodicTask, RateLimitState, ReplayEntry, - Subscription, TaskLogEntry, TaskMetric, Topic, TopicMessage, WorkerInfo, + Subscription, SubscriptionMode, TaskLogEntry, TaskMetric, Topic, TopicMessage, WorkerInfo, }; use super::schema::{ archived_jobs, circuit_breakers, dashboard_settings, dead_letter, distributed_locks, @@ -740,7 +740,7 @@ impl From for Subscription { priority: r.priority, max_retries: r.max_retries, timeout_ms: r.timeout_ms, - mode: r.mode, + mode: SubscriptionMode::from_wire(&r.mode), cursor: r.cursor, } } @@ -764,7 +764,7 @@ impl From for Topic { fn from(r: TopicRow) -> Self { Topic { name: r.name, - mode: r.mode, + mode: SubscriptionMode::from_wire(&r.mode), retention_ms: r.retention_ms, created_at: r.created_at, } diff --git a/crates/taskito-core/src/storage/postgres/pubsub.rs b/crates/taskito-core/src/storage/postgres/pubsub.rs index a000b30d..02d1266b 100644 --- a/crates/taskito-core/src/storage/postgres/pubsub.rs +++ b/crates/taskito-core/src/storage/postgres/pubsub.rs @@ -1,7 +1,7 @@ use diesel::prelude::*; use super::super::models::*; -use super::super::records::NewSubscription; +use super::super::records::{NewSubscription, SubscriptionMode}; use super::super::schema::{topic_deliveries, topic_messages, topic_subscriptions, topics}; use super::PostgresStorage; use crate::error::Result; @@ -11,12 +11,17 @@ crate::storage::diesel_common::impl_diesel_pubsub_ops!(PostgresStorage); impl PostgresStorage { /// Declare a topic (idempotent upsert on `name`). `created_at` is preserved /// on re-declaration; only `mode`/`retention_ms` are updated. - pub fn declare_topic(&self, name: &str, mode: &str, retention_ms: Option) -> Result<()> { + pub fn declare_topic( + &self, + name: &str, + mode: SubscriptionMode, + retention_ms: Option, + ) -> Result<()> { crate::pubsub::validate_topic_declaration(mode, retention_ms)?; let mut conn = self.conn()?; let row = NewTopicRow { name, - mode, + mode: mode.as_str(), retention_ms, created_at: crate::job::now_millis(), }; @@ -53,7 +58,7 @@ impl PostgresStorage { priority: sub.priority, max_retries: sub.max_retries, timeout_ms: sub.timeout_ms, - mode: &sub.mode, + mode: sub.mode.as_str(), }; // `cursor` is omitted so a re-registration preserves a log consumer's diff --git a/crates/taskito-core/src/storage/records.rs b/crates/taskito-core/src/storage/records.rs index df0b13ca..c9798bdf 100644 --- a/crates/taskito-core/src/storage/records.rs +++ b/crates/taskito-core/src/storage/records.rs @@ -117,9 +117,8 @@ pub struct Subscription { pub max_retries: Option, /// Per-delivery timeout in milliseconds. `None` = queue default. pub timeout_ms: Option, - /// Delivery mode: `"fanout"` (one job per publish, the default) or `"log"` - /// (append one `topic_messages` row per publish; consumer pulls via cursor). - pub mode: String, + /// How this subscription is delivered to. + pub mode: SubscriptionMode, /// Log-mode read cursor: the last-acked message id. `None` = unread (start /// from the beginning). Ignored for fan-out subscriptions. pub cursor: Option, @@ -150,15 +149,86 @@ pub struct NewSubscription { pub max_retries: Option, /// Per-delivery timeout in milliseconds. `None` = queue default. pub timeout_ms: Option, - /// Delivery mode: `"fanout"` (default) or `"log"`. See [`Subscription::mode`]. - pub mode: String, + /// How this subscription is delivered to. See [`Subscription::mode`]. + pub mode: SubscriptionMode, } -/// Delivery mode marker for the `mode` column. `"log"` opts a subscription into -/// the append-once + cursor model; anything else is treated as fan-out. -pub const SUBSCRIPTION_MODE_LOG: &str = "log"; -/// Default fan-out delivery mode (one job per publish). -pub const SUBSCRIPTION_MODE_FANOUT: &str = "fanout"; +/// Lifecycle state a worker reports for itself. Stored as its lowercase wire +/// form in the `workers.status` column, so the persisted values are unchanged. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum WorkerStatus { + /// Registered and consuming — the state every worker starts in. + #[default] + Active, + /// Shutting down: finishing in-flight jobs, claiming no new ones. + Draining, +} + +impl WorkerStatus { + /// The wire form persisted in the `status` column. + pub fn as_str(&self) -> &'static str { + match self { + WorkerStatus::Active => "active", + WorkerStatus::Draining => "draining", + } + } + + /// Parse a persisted `status`; anything unrecognized reads as active, the + /// value every backend writes at registration. + pub fn from_wire(wire: &str) -> Self { + match wire { + "draining" => WorkerStatus::Draining, + _ => WorkerStatus::Active, + } + } +} + +impl std::fmt::Display for WorkerStatus { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + +/// How a topic delivers to a subscription. Stored as its lowercase wire form in +/// the `mode` column, so the persisted values are unchanged. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum SubscriptionMode { + /// One delivery job per publish, per subscription — the default. + #[default] + Fanout, + /// Append one `topic_messages` row per publish; consumers pull via cursor. + Log, +} + +impl SubscriptionMode { + /// The wire form persisted in the `mode` column. + pub fn as_str(&self) -> &'static str { + match self { + SubscriptionMode::Fanout => "fanout", + SubscriptionMode::Log => "log", + } + } + + /// Parse a persisted `mode`. Anything unrecognized reads as fan-out, which is + /// what the column's pre-enum readers did with a value that wasn't `"log"`. + pub fn from_wire(wire: &str) -> Self { + match wire { + "log" => SubscriptionMode::Log, + _ => SubscriptionMode::Fanout, + } + } + + /// Whether this mode is the append-once + cursor model. + pub fn is_log(&self) -> bool { + matches!(self, SubscriptionMode::Log) + } +} + +impl std::fmt::Display for SubscriptionMode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} /// One durable message in a log topic. Unlike fan-out delivery (one `jobs` row /// per subscriber), a log publish writes exactly one of these and each log @@ -189,9 +259,9 @@ pub struct TopicMessage { pub struct Topic { /// Topic name (primary key). pub name: String, - /// Delivery mode: `"log"` (the only declarable mode today) opts publishes - /// into the append-once store even without a subscriber. - pub mode: String, + /// Delivery mode: [`SubscriptionMode::Log`] (the only declarable mode today) + /// opts publishes into the append-once store even without a subscriber. + pub mode: SubscriptionMode, /// Retention window in milliseconds; `None` = keep until consumed/compacted. /// A published log row gets `expires_at = now + retention_ms` when the topic /// has no live log subscriber, so the retention sweep can reclaim it. @@ -203,7 +273,7 @@ pub struct Topic { impl Topic { /// Whether this is a log topic (publishes are retained without a subscriber). pub fn is_log(&self) -> bool { - self.mode == SUBSCRIPTION_MODE_LOG + self.mode.is_log() } } @@ -368,3 +438,38 @@ pub struct LockInfo { /// Unix-millisecond time the lock expires. pub expires_at: i64, } + +#[cfg(test)] +mod tests { + use super::{SubscriptionMode, WorkerStatus}; + + #[test] + fn subscription_mode_round_trips_its_stored_form() { + for mode in [SubscriptionMode::Fanout, SubscriptionMode::Log] { + assert_eq!(SubscriptionMode::from_wire(mode.as_str()), mode); + } + assert_eq!(SubscriptionMode::Fanout.as_str(), "fanout"); + assert_eq!(SubscriptionMode::Log.as_str(), "log"); + assert!(SubscriptionMode::Log.is_log()); + } + + #[test] + fn worker_status_round_trips_its_stored_form() { + for status in [WorkerStatus::Active, WorkerStatus::Draining] { + assert_eq!(WorkerStatus::from_wire(status.as_str()), status); + } + assert_eq!(WorkerStatus::Active.as_str(), "active"); + assert_eq!(WorkerStatus::Draining.as_str(), "draining"); + } + + #[test] + fn unrecognized_stored_values_read_as_the_default() { + // Rows predate the enums, so a value neither wrote must not panic or + // silently promote — it reads as what the old string compares implied. + assert_eq!( + SubscriptionMode::from_wire("broadcast"), + SubscriptionMode::Fanout + ); + assert_eq!(WorkerStatus::from_wire("paused"), WorkerStatus::Active); + } +} diff --git a/crates/taskito-core/src/storage/redis_backend/pubsub.rs b/crates/taskito-core/src/storage/redis_backend/pubsub.rs index b94e1d97..ebd7ee03 100644 --- a/crates/taskito-core/src/storage/redis_backend/pubsub.rs +++ b/crates/taskito-core/src/storage/redis_backend/pubsub.rs @@ -8,7 +8,7 @@ use super::{map_err, RedisStorage}; use crate::error::{QueueError, Result}; use crate::job::{Job, JobStatus}; use crate::storage::records::{ - NewSubscription, Subscription, Topic, TopicLogStats, TopicMessage, SUBSCRIPTION_MODE_LOG, + NewSubscription, Subscription, SubscriptionMode, Topic, TopicLogStats, TopicMessage, }; use crate::storage::SubscriptionBacklogStats; @@ -41,7 +41,7 @@ struct SubEntry { } fn default_mode() -> String { - crate::storage::records::SUBSCRIPTION_MODE_FANOUT.to_string() + SubscriptionMode::Fanout.as_str().to_string() } /// JSON blob stored in the `topics` hash, field = topic name (so the name is @@ -57,7 +57,7 @@ impl TopicEntry { fn into_topic(self, name: String) -> Topic { Topic { name, - mode: self.mode, + mode: SubscriptionMode::from_wire(&self.mode), retention_ms: self.retention_ms, created_at: self.created_at, } @@ -78,7 +78,7 @@ impl From for Subscription { priority: e.priority, max_retries: e.max_retries, timeout_ms: e.timeout_ms, - mode: e.mode, + mode: SubscriptionMode::from_wire(&e.mode), cursor: e.cursor, } } @@ -157,7 +157,7 @@ impl RedisStorage { priority: sub.priority, max_retries: sub.max_retries, timeout_ms: sub.timeout_ms, - mode: sub.mode.clone(), + mode: sub.mode.as_str().to_string(), cursor: None, }; let blob_key = self.key(&["sub", &sub.topic, &sub.subscription_name]); @@ -616,7 +616,7 @@ impl RedisStorage { }; let entry: SubEntry = serde_json::from_str(&data)?; // A fan-out sub must never read a mixed topic's log. - if entry.mode != SUBSCRIPTION_MODE_LOG { + if !SubscriptionMode::from_wire(&entry.mode).is_log() { return Ok(Vec::new()); } @@ -656,7 +656,7 @@ impl RedisStorage { }; let mut entry: SubEntry = serde_json::from_str(&data)?; // Only a log subscription has a cursor to advance. - if entry.mode != SUBSCRIPTION_MODE_LOG { + if !SubscriptionMode::from_wire(&entry.mode).is_log() { return Ok(false); } @@ -681,7 +681,7 @@ impl RedisStorage { let now = crate::job::now_millis(); let mut out = Vec::new(); - for sub in subs.into_iter().filter(|s| s.mode == SUBSCRIPTION_MODE_LOG) { + for sub in subs.into_iter().filter(|s| s.mode.is_log()) { let start = match &sub.cursor { Some(cursor) => format!("({cursor}"), None => "-".to_string(), @@ -721,7 +721,7 @@ impl RedisStorage { for sub in self .list_subscriptions()? .into_iter() - .filter(|s| s.mode == SUBSCRIPTION_MODE_LOG) + .filter(|s| s.mode.is_log()) { by_topic.entry(sub.topic.clone()).or_default().push(sub); } @@ -859,7 +859,12 @@ impl RedisStorage { /// Declare a topic (idempotent). Stored as a field in the `topics` hash; /// re-declaring preserves the original `created_at`. - pub fn declare_topic(&self, name: &str, mode: &str, retention_ms: Option) -> Result<()> { + pub fn declare_topic( + &self, + name: &str, + mode: SubscriptionMode, + retention_ms: Option, + ) -> Result<()> { crate::pubsub::validate_topic_declaration(mode, retention_ms)?; let mut conn = self.conn()?; let key = self.key(&["topics"]); @@ -873,7 +878,7 @@ impl RedisStorage { None => crate::job::now_millis(), }; let entry = TopicEntry { - mode: mode.to_string(), + mode: mode.as_str().to_string(), retention_ms, created_at, }; @@ -938,7 +943,7 @@ impl RedisStorage { let Some(data): Option = conn.get(&blob_key).map_err(map_err)? else { return Ok(Vec::new()); }; - if serde_json::from_str::(&data)?.mode != SUBSCRIPTION_MODE_LOG { + if !SubscriptionMode::from_wire(&serde_json::from_str::(&data)?.mode).is_log() { return Ok(Vec::new()); } diff --git a/crates/taskito-core/src/storage/redis_backend/workers.rs b/crates/taskito-core/src/storage/redis_backend/workers.rs index ba821a0b..2e498e13 100644 --- a/crates/taskito-core/src/storage/redis_backend/workers.rs +++ b/crates/taskito-core/src/storage/redis_backend/workers.rs @@ -60,11 +60,15 @@ impl RedisStorage { } /// Set a worker's status string. - pub fn update_worker_status(&self, worker_id: &str, status: &str) -> Result<()> { + pub fn update_worker_status( + &self, + worker_id: &str, + status: crate::storage::records::WorkerStatus, + ) -> Result<()> { let mut conn = self.conn()?; let wkey = self.key(&["worker", worker_id]); - conn.hset::<_, _, _, ()>(&wkey, "status", status) + conn.hset::<_, _, _, ()>(&wkey, "status", status.as_str()) .map_err(map_err)?; Ok(()) diff --git a/crates/taskito-core/src/storage/sqlite/pubsub.rs b/crates/taskito-core/src/storage/sqlite/pubsub.rs index cedf2748..9aad11e2 100644 --- a/crates/taskito-core/src/storage/sqlite/pubsub.rs +++ b/crates/taskito-core/src/storage/sqlite/pubsub.rs @@ -1,7 +1,7 @@ use diesel::prelude::*; use super::super::models::*; -use super::super::records::NewSubscription; +use super::super::records::{NewSubscription, SubscriptionMode}; use super::super::schema::{topic_deliveries, topic_messages, topic_subscriptions, topics}; use super::SqliteStorage; use crate::error::Result; @@ -11,12 +11,17 @@ crate::storage::diesel_common::impl_diesel_pubsub_ops!(SqliteStorage); impl SqliteStorage { /// Declare a topic (idempotent upsert on `name`). `created_at` is preserved /// on re-declaration; only `mode`/`retention_ms` are updated. - pub fn declare_topic(&self, name: &str, mode: &str, retention_ms: Option) -> Result<()> { + pub fn declare_topic( + &self, + name: &str, + mode: SubscriptionMode, + retention_ms: Option, + ) -> Result<()> { crate::pubsub::validate_topic_declaration(mode, retention_ms)?; let mut conn = self.conn()?; let row = NewTopicRow { name, - mode, + mode: mode.as_str(), retention_ms, created_at: crate::job::now_millis(), }; @@ -52,7 +57,7 @@ impl SqliteStorage { priority: sub.priority, max_retries: sub.max_retries, timeout_ms: sub.timeout_ms, - mode: &sub.mode, + mode: sub.mode.as_str(), }; // `cursor` is omitted so a re-registration preserves a log consumer's diff --git a/crates/taskito-core/src/storage/sqlite/tests.rs b/crates/taskito-core/src/storage/sqlite/tests.rs index 1582db69..df490370 100644 --- a/crates/taskito-core/src/storage/sqlite/tests.rs +++ b/crates/taskito-core/src/storage/sqlite/tests.rs @@ -1721,7 +1721,7 @@ fn make_sub( priority: None, max_retries: None, timeout_ms: None, - mode: "fanout".to_string(), + mode: crate::storage::records::SubscriptionMode::Fanout, } } diff --git a/crates/taskito-core/src/storage/traits.rs b/crates/taskito-core/src/storage/traits.rs index 68cf45cd..78ac1d50 100644 --- a/crates/taskito-core/src/storage/traits.rs +++ b/crates/taskito-core/src/storage/traits.rs @@ -2,8 +2,8 @@ use crate::error::Result; use crate::job::{Job, NewJob}; use crate::storage::records::{ CircuitBreakerState, JobError, LockInfo, NewPeriodicTask, NewSubscription, PeriodicTask, - RateLimitState, ReplayEntry, Subscription, TaskLogEntry, TaskMetric, Topic, TopicLogStats, - TopicMessage, WorkerInfo, + RateLimitState, ReplayEntry, Subscription, SubscriptionMode, TaskLogEntry, TaskMetric, Topic, + TopicLogStats, TopicMessage, WorkerInfo, WorkerStatus, }; use crate::storage::{DeadJob, DispatchOrder, QueueStats, SubscriptionBacklogStats}; @@ -295,7 +295,12 @@ pub trait Storage: Send + Sync + Clone { /// retained even with no subscriber (removing the late-join boundary). /// `mode` must be `"log"` (the only declarable mode) and `retention_ms`, if /// set, must be non-negative — backends reject anything else. - fn declare_topic(&self, name: &str, mode: &str, retention_ms: Option) -> Result<()>; + fn declare_topic( + &self, + name: &str, + mode: SubscriptionMode, + retention_ms: Option, + ) -> Result<()>; /// Fetch a declared topic by name, or `None` if it was never declared. fn get_topic(&self, name: &str) -> Result>; /// List every declared topic in the registry. @@ -416,8 +421,8 @@ pub trait Storage: Send + Sync + Clone { /// Refresh a worker's heartbeat timestamp, optionally updating its /// resource-health JSON. fn heartbeat(&self, worker_id: &str, resource_health: Option<&str>) -> Result<()>; - /// Set a worker's status string. - fn update_worker_status(&self, worker_id: &str, status: &str) -> Result<()>; + /// Set a worker's lifecycle status. + fn update_worker_status(&self, worker_id: &str, status: WorkerStatus) -> Result<()>; /// Every registered worker with its heartbeat state. fn list_workers(&self) -> Result>; /// Ids of workers whose heartbeat is at or after `cutoff_ms`. A narrow diff --git a/crates/taskito-core/tests/rust/storage_tests.rs b/crates/taskito-core/tests/rust/storage_tests.rs index 33e8d7ea..d1fab3d0 100644 --- a/crates/taskito-core/tests/rust/storage_tests.rs +++ b/crates/taskito-core/tests/rust/storage_tests.rs @@ -8,6 +8,7 @@ //! when all tests share a single storage instance. use taskito_core::job::{now_millis, JobCompletion, JobStatus, NewJob}; +use taskito_core::storage::records::{SubscriptionMode, WorkerStatus}; use taskito_core::storage::{DeadJob, Storage}; use taskito_core::SqliteStorage; @@ -456,7 +457,8 @@ fn test_workers(s: &impl Storage) { assert!(w.started_at.is_some()); // Test update_worker_status - s.update_worker_status("w-test-1", "draining").unwrap(); + s.update_worker_status("w-test-1", WorkerStatus::Draining) + .unwrap(); let workers = s.list_workers().unwrap(); let w = workers.iter().find(|w| w.worker_id == "w-test-1").unwrap(); assert_eq!(w.status, "draining"); @@ -1057,7 +1059,7 @@ fn test_topic_subscriptions_crud(s: &impl Storage) { priority: None, max_retries: None, timeout_ms: None, - mode: "fanout".to_string(), + mode: SubscriptionMode::Fanout, }; // Upsert idempotency: re-registering (topic, name) updates in place. @@ -1231,7 +1233,7 @@ fn test_topic_backlog_stats(s: &impl Storage) { priority: None, max_retries: None, timeout_ms: None, - mode: "fanout".to_string(), + mode: SubscriptionMode::Fanout, }; s.register_subscription(&sub("tbs-email", "tbs_send")) .unwrap(); @@ -1305,7 +1307,7 @@ fn log_sub(topic: &str, name: &str) -> taskito_core::NewSubscription { priority: None, max_retries: None, timeout_ms: None, - mode: "log".to_string(), + mode: SubscriptionMode::Log, } } @@ -1351,7 +1353,7 @@ fn test_topic_log_messages(s: &impl Storage) { // A fan-out subscription on the same topic can neither read the log nor // advance a cursor — the log is for log subscriptions only. let mut fan = log_sub(topic, "fan"); - fan.mode = "fanout".to_string(); + fan.mode = SubscriptionMode::Fanout; fan.task_name = "deliver".to_string(); s.register_subscription(&fan).unwrap(); assert!(s.read_topic_messages(topic, "fan", 10).unwrap().is_empty()); @@ -1368,7 +1370,8 @@ fn test_topic_registry(s: &impl Storage) { assert!(s.get_topic("treg-a").unwrap().is_none()); // Declare with a retention window; get_topic round-trips every field. - s.declare_topic("treg-a", "log", Some(1500)).unwrap(); + s.declare_topic("treg-a", SubscriptionMode::Log, Some(1500)) + .unwrap(); let a = s.get_topic("treg-a").unwrap().expect("declared topic"); assert_eq!(a.name, "treg-a"); assert!(a.is_log()); @@ -1376,13 +1379,15 @@ fn test_topic_registry(s: &impl Storage) { let created = a.created_at; // Re-declaring is idempotent: retention updates, created_at is preserved. - s.declare_topic("treg-a", "log", Some(3000)).unwrap(); + s.declare_topic("treg-a", SubscriptionMode::Log, Some(3000)) + .unwrap(); let a2 = s.get_topic("treg-a").unwrap().unwrap(); assert_eq!(a2.retention_ms, Some(3000)); assert_eq!(a2.created_at, created); // A topic can be declared with no retention (unbounded backlog). - s.declare_topic("treg-b", "log", None).unwrap(); + s.declare_topic("treg-b", SubscriptionMode::Log, None) + .unwrap(); assert_eq!(s.get_topic("treg-b").unwrap().unwrap().retention_ms, None); // Both declarations appear in the registry listing. diff --git a/crates/taskito-java/src/convert.rs b/crates/taskito-java/src/convert.rs index ef6f9769..1af3e88a 100644 --- a/crates/taskito-java/src/convert.rs +++ b/crates/taskito-java/src/convert.rs @@ -309,7 +309,7 @@ impl<'a> From<&'a Topic> for TopicView<'a> { fn from(t: &'a Topic) -> Self { Self { name: &t.name, - mode: &t.mode, + mode: t.mode.as_str(), retention_ms: t.retention_ms, created_at: t.created_at, } diff --git a/crates/taskito-java/src/queue/pubsub.rs b/crates/taskito-java/src/queue/pubsub.rs index 21980a4d..0592a9c1 100644 --- a/crates/taskito-java/src/queue/pubsub.rs +++ b/crates/taskito-java/src/queue/pubsub.rs @@ -9,7 +9,7 @@ use jni::sys::{jboolean, jint, jlong, jstring, JNI_FALSE}; use jni::JNIEnv; use taskito_core::job::now_millis; use taskito_core::pubsub::publish_to_topic; -use taskito_core::storage::records::{NewSubscription, SUBSCRIPTION_MODE_LOG}; +use taskito_core::storage::records::{NewSubscription, SubscriptionMode}; use taskito_core::Storage; use crate::backend; @@ -54,7 +54,7 @@ pub extern "system" fn Java_org_byteveda_taskito_internal_NativeQueue_registerSu let task_name = read_string(env, &task_name)?; let queue = read_string(env, &queue)?; let owner_worker_id = read_optional_string(env, &owner_worker_id)?; - let mode = read_string(env, &mode)?; + let mode = SubscriptionMode::from_wire(&read_string(env, &mode)?); // An ownerless ephemeral row would never be reaped (the reaper matches // on owner) yet keeps receiving deliveries — reject it up front. if durable == 0 && owner_worker_id.is_none() { @@ -355,7 +355,7 @@ pub extern "system" fn Java_org_byteveda_taskito_internal_NativeQueue_declareTop let retention = (retention_ms != jlong::MIN).then_some(retention_ms); queue .storage - .declare_topic(&name, SUBSCRIPTION_MODE_LOG, retention)?; + .declare_topic(&name, SubscriptionMode::Log, retention)?; Ok(()) }) } diff --git a/crates/taskito-java/src/worker.rs b/crates/taskito-java/src/worker.rs index 41b7bb7e..2a4fefbb 100644 --- a/crates/taskito-java/src/worker.rs +++ b/crates/taskito-java/src/worker.rs @@ -267,7 +267,7 @@ fn register_subscriptions( max_retries: spec.max_retries, timeout_ms: spec.timeout_ms, // Fan-out by default; the log-mode param is threaded in a later step. - mode: taskito_core::storage::records::SUBSCRIPTION_MODE_FANOUT.to_string(), + mode: taskito_core::storage::records::SubscriptionMode::Fanout, }; storage.register_subscription(&row)?; } diff --git a/crates/taskito-node/src/convert/pubsub.rs b/crates/taskito-node/src/convert/pubsub.rs index 62679ab1..1d1ae068 100644 --- a/crates/taskito-node/src/convert/pubsub.rs +++ b/crates/taskito-node/src/convert/pubsub.rs @@ -86,7 +86,7 @@ pub struct JsTopic { pub fn topic_to_js(topic: Topic) -> JsTopic { JsTopic { name: topic.name, - mode: topic.mode, + mode: topic.mode.as_str().to_string(), retention_ms: topic.retention_ms, created_at: topic.created_at, } diff --git a/crates/taskito-node/src/queue/pubsub.rs b/crates/taskito-node/src/queue/pubsub.rs index f0646700..980a0e16 100644 --- a/crates/taskito-node/src/queue/pubsub.rs +++ b/crates/taskito-node/src/queue/pubsub.rs @@ -6,7 +6,7 @@ use napi::bindgen_prelude::{spawn_blocking, Buffer, Result}; use napi_derive::napi; use taskito_core::job::now_millis; use taskito_core::pubsub::{publish_to_topic, DeliveryDefaults, PublishRequest}; -use taskito_core::storage::records::NewSubscription; +use taskito_core::storage::records::{NewSubscription, SubscriptionMode}; use taskito_core::Storage; use super::JsQueue; @@ -45,9 +45,9 @@ impl JsQueue { "an ephemeral subscription (durable=false) requires ownerWorkerId", )); } - let mode = mode.unwrap_or_else(|| { - taskito_core::storage::records::SUBSCRIPTION_MODE_FANOUT.to_string() - }); + let mode = mode + .map(|m| SubscriptionMode::from_wire(&m)) + .unwrap_or_default(); let storage = self.storage.clone(); spawn_blocking(move || { let row = NewSubscription { @@ -197,7 +197,7 @@ impl JsQueue { let storage = self.storage.clone(); spawn_blocking(move || { storage - .declare_topic(&name, &mode, retention_ms) + .declare_topic(&name, SubscriptionMode::from_wire(&mode), retention_ms) .map_err(to_napi_err) }) .await diff --git a/crates/taskito-python/src/py_queue/pubsub.rs b/crates/taskito-python/src/py_queue/pubsub.rs index 10b5de37..06436646 100644 --- a/crates/taskito-python/src/py_queue/pubsub.rs +++ b/crates/taskito-python/src/py_queue/pubsub.rs @@ -2,7 +2,7 @@ use pyo3::prelude::*; use taskito_core::job::now_millis; use taskito_core::pubsub::{publish_to_topic, DeliveryDefaults, PublishRequest}; -use taskito_core::storage::records::NewSubscription; +use taskito_core::storage::records::{NewSubscription, SubscriptionMode}; use taskito_core::storage::Storage; use super::PyQueue; @@ -80,7 +80,7 @@ impl PyQueue { priority, max_retries, timeout_ms, - mode: mode.to_string(), + mode: SubscriptionMode::from_wire(mode), }; self.storage .register_subscription(&row) @@ -272,6 +272,7 @@ impl PyQueue { retention_ms: Option, ) -> PyResult<()> { let storage = &self.storage; + let mode = SubscriptionMode::from_wire(mode); py.detach(|| storage.declare_topic(name, mode, retention_ms)) .map_err(|e| pyo3::exceptions::PyRuntimeError::new_err(e.to_string())) } @@ -283,7 +284,14 @@ impl PyQueue { .detach(|| storage.list_declared_topics()) .map_err(|e| pyo3::exceptions::PyRuntimeError::new_err(e.to_string()))? .into_iter() - .map(|t| (t.name, t.mode, t.retention_ms, t.created_at)) + .map(|t| { + ( + t.name, + t.mode.as_str().to_string(), + t.retention_ms, + t.created_at, + ) + }) .collect()) } diff --git a/crates/taskito-python/src/py_queue/worker.rs b/crates/taskito-python/src/py_queue/worker.rs index 87e59445..da446faa 100644 --- a/crates/taskito-python/src/py_queue/worker.rs +++ b/crates/taskito-python/src/py_queue/worker.rs @@ -9,6 +9,7 @@ use taskito_core::resilience::circuit_breaker::CircuitBreakerConfig; use taskito_core::resilience::rate_limiter::RateLimitConfig; use taskito_core::resilience::retry::RetryPolicy; use taskito_core::scheduler::{JobResult, ResultOutcome, Scheduler, SchedulerConfig, TaskConfig}; +use taskito_core::storage::records::WorkerStatus; use taskito_core::storage::Storage; use super::PyQueue; @@ -833,10 +834,11 @@ impl PyQueue { )) } - /// Update the status of a worker. + /// Update the status of a worker. An unrecognized status reads as active, + /// matching what every backend writes at registration. pub fn set_worker_status(&self, worker_id: &str, status: &str) -> PyResult<()> { self.storage - .update_worker_status(worker_id, status) + .update_worker_status(worker_id, WorkerStatus::from_wire(status)) .map_err(|e| pyo3::exceptions::PyRuntimeError::new_err(e.to_string())) } } diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java index 8d84252a..837163f7 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java @@ -5,7 +5,6 @@ import java.util.Locale; import java.util.Map; import java.util.Optional; -import java.util.Set; import org.byteveda.taskito.dashboard.store.SettingsAccess; import org.byteveda.taskito.dashboard.support.DashboardError; import org.byteveda.taskito.dashboard.support.Json; @@ -28,7 +27,6 @@ public final class AuthStore { public static final String SESSION_PREFIX = "auth:session:"; public static final long DEFAULT_SESSION_TTL_SECONDS = 24 * 60 * 60; - public static final String ENV_ADMIN_USER = "TASKITO_DASHBOARD_ADMIN_USER"; public static final String ENV_ADMIN_PASSWORD = "TASKITO_DASHBOARD_ADMIN_PASSWORD"; @@ -377,5 +375,4 @@ private static void validatePassword(String password) { throw DashboardError.badRequest("password must be 8-256 characters"); } } - } From f523d9db08903c38f67ddd93aebe7bea84b644b0 Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:11:52 +0530 Subject: [PATCH 06/22] style(java): apply spotless formatting --- .../src/main/java/org/byteveda/taskito/DefaultTaskito.java | 3 +-- .../main/java/org/byteveda/taskito/dashboard/auth/Policy.java | 3 +-- .../src/test/java/org/byteveda/taskito/core/QueueTest.java | 2 +- .../org/byteveda/taskito/dashboard/DashboardNativeTest.java | 2 +- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java b/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java index 4a6ca72a..84d47b65 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java @@ -1175,8 +1175,7 @@ public void cancelWorkflow(String runId) { } @Override - public List listWorkflowRuns( - String definitionName, WorkflowState state, long limit, long offset) { + public List listWorkflowRuns(String definitionName, WorkflowState state, long limit, long offset) { String wire = state == null ? null : state.wire(); return decodeList(backend.listWorkflowRunsJson(definitionName, wire, limit, offset), WorkflowRunInfo.class); } diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java index 5c7c991c..133c38d2 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java @@ -64,8 +64,7 @@ public static void authorize(String path, String method, RequestContext ctx, Aut if (isStateChanging(method) && !isCsrfExempt(path) && !ctx.csrfValid()) { throw DashboardError.forbidden("csrf_failed"); } - if (requiresAdmin(path, method) - && ctx.session().role() != Role.ADMIN) { + if (requiresAdmin(path, method) && ctx.session().role() != Role.ADMIN) { throw DashboardError.forbidden("forbidden"); } } diff --git a/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java b/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java index b17f23fc..8199a326 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java @@ -17,8 +17,8 @@ import org.byteveda.taskito.model.Job; import org.byteveda.taskito.model.JobFilter; import org.byteveda.taskito.model.JobStatus; -import org.byteveda.taskito.model.TaskLogLevel; import org.byteveda.taskito.model.TaskLog; +import org.byteveda.taskito.model.TaskLogLevel; import org.byteveda.taskito.task.EnqueueOptions; import org.byteveda.taskito.task.Task; import org.junit.jupiter.api.Test; diff --git a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardNativeTest.java b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardNativeTest.java index c8f07fa0..f3657265 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardNativeTest.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardNativeTest.java @@ -8,8 +8,8 @@ import java.util.Collections; import java.util.Map; import org.byteveda.taskito.Taskito; -import org.byteveda.taskito.model.TaskLogLevel; import org.byteveda.taskito.model.JobFilter; +import org.byteveda.taskito.model.TaskLogLevel; import org.byteveda.taskito.task.EnqueueOptions; import org.byteveda.taskito.task.Task; import org.junit.jupiter.api.Test; From 2adc76e54fc8240cb0bed9b56bd122ce0f1d6b2b Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:47:34 +0530 Subject: [PATCH 07/22] feat(python)!: rename resource scopes to match the other SDKs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../python/api-reference/queue/resources.mdx | 10 ++--- .../python/guides/resources/configuration.mdx | 19 +++++---- .../guides/resources/dependency-injection.mdx | 42 ++++++++++++------- sdks/python/taskito/mixins/resources.py | 14 ++++--- sdks/python/taskito/resources/definition.py | 36 +++++++++++++--- sdks/python/taskito/resources/runtime.py | 30 ++++++------- .../resources/test_resource_system_full.py | 10 +++-- .../tests/worker/test_lifecycle_parity.py | 6 +-- 8 files changed, 105 insertions(+), 62 deletions(-) diff --git a/docs/content/docs/python/api-reference/queue/resources.mdx b/docs/content/docs/python/api-reference/queue/resources.mdx index 8b84a27f..34763393 100644 --- a/docs/content/docs/python/api-reference/queue/resources.mdx +++ b/docs/content/docs/python/api-reference/queue/resources.mdx @@ -17,7 +17,7 @@ Methods for the worker resource system and distributed locking. health_check: Callable | None = None, health_check_interval: float = 0.0, max_recreation_attempts: int = 3, - scope: str = "worker", + scope: ResourceScope | str = ResourceScope.WORKER, pool_size: int | None = None, pool_min: int = 0, acquire_timeout: float = 10.0, @@ -38,9 +38,9 @@ Decorator to register a resource factory initialized at worker startup. | `health_check` | `Callable \| None` | `None` | Called periodically; returns truthy if healthy. | | `health_check_interval` | `float` | `0.0` | Seconds between health checks (0 = disabled). | | `max_recreation_attempts` | `int` | `3` | Max times to recreate on health failure. | -| `scope` | `str` | `"worker"` | Lifetime scope: `"worker"`, `"task"`, `"thread"`, or `"request"`. | -| `pool_size` | `int \| None` | `None` | Pool capacity for task-scoped resources (default = worker thread count). | -| `pool_min` | `int` | `0` | Pre-warmed instances for task-scoped resources. | +| `scope` | `ResourceScope \| str` | `WORKER` | Lifetime: `WORKER`, `THREAD`, `TASK` (fresh per task), or `POOLED` (pool checkout). | +| `pool_size` | `int \| None` | `None` | Pool capacity; `POOLED` scope only (default = worker thread count). | +| `pool_min` | `int` | `0` | Pre-warmed instances; `POOLED` scope only. | | `acquire_timeout` | `float` | `10.0` | Max seconds to wait for a pool instance. | | `max_lifetime` | `float` | `3600.0` | Max seconds a pooled instance can live. | | `idle_timeout` | `float` | `300.0` | Max idle seconds before pool eviction. | @@ -93,7 +93,7 @@ queue.resource_status() -> list[dict] ``` Return per-resource status. Each entry contains: `name`, `scope`, `health`, -`init_duration_ms`, `recreations`, `depends_on`. Task-scoped entries also +`init_duration_ms`, `recreations`, `depends_on`. Pooled entries also include `pool` stats. ### `queue.register_type()` diff --git a/docs/content/docs/python/guides/resources/configuration.mdx b/docs/content/docs/python/guides/resources/configuration.mdx index 9d7783f8..59b9063c 100644 --- a/docs/content/docs/python/guides/resources/configuration.mdx +++ b/docs/content/docs/python/guides/resources/configuration.mdx @@ -30,7 +30,7 @@ depends_on = ["config"] [resources.session] factory = "myapp.resources:create_session" -scope = "task" +scope = "pooled" pool_size = 20 pool_min = 5 acquire_timeout = 5.0 @@ -65,20 +65,21 @@ pip install tomli | `health_check` | string | — | Import path to the health check callable. | | `health_check_interval` | float | `0.0` | Seconds between health checks. `0` disables. | | `max_recreation_attempts` | int | `3` | Max recreation attempts on health failure. | -| `scope` | string | `"worker"` | `"worker"`, `"task"`, `"thread"`, or `"request"`. | +| `scope` | string | `"worker"` | `"worker"`, `"thread"`, `"task"`, or `"pooled"`. | | `depends_on` | list[string] | `[]` | Resource names this one depends on. | -| `pool_size` | int | `4` | Task scope: max concurrent instances. | -| `pool_min` | int | `0` | Task scope: pre-warmed instances at startup. | -| `acquire_timeout` | float | `10.0` | Task scope: seconds to wait for a pool instance. | -| `max_lifetime` | float | `3600.0` | Task scope: max seconds an instance lives. | -| `idle_timeout` | float | `300.0` | Task scope: max idle seconds before eviction. | +| `pool_size` | int | `4` | Pooled scope: max concurrent instances. | +| `pool_min` | int | `0` | Pooled scope: pre-warmed instances at startup. | +| `acquire_timeout` | float | `10.0` | Pooled scope: seconds to wait for a pool instance. | +| `max_lifetime` | float | `3600.0` | Pooled scope: max seconds an instance lives. | +| `idle_timeout` | float | `300.0` | Pooled scope: max idle seconds before eviction. | | `reloadable` | bool | `false` | Allow hot reload via SIGHUP or CLI. | | `frozen` | bool | `false` | Wrap instance in a read-only wrapper. | ## Pool configuration -Pool parameters apply only to task-scoped resources (`scope = "task"`). -They control the bounded pool that manages concurrent instances. +Pool parameters apply only to pooled resources (`scope = "pooled"`); setting +`pool_size` or `pool_min` on any other scope raises. They control the bounded +pool that manages concurrent instances. | Parameter | Default | Description | |---|---|---| diff --git a/docs/content/docs/python/guides/resources/dependency-injection.mdx b/docs/content/docs/python/guides/resources/dependency-injection.mdx index 5017229d..d257b4fe 100644 --- a/docs/content/docs/python/guides/resources/dependency-injection.mdx +++ b/docs/content/docs/python/guides/resources/dependency-injection.mdx @@ -1,6 +1,6 @@ --- title: Dependency Injection -description: "worker_resource decorator, scopes (worker/task/thread/request), dependencies, teardown, health checks." +description: "worker_resource decorator, scopes (worker/thread/task/pooled), dependencies, teardown, health checks." --- import { Tab, Tabs } from "fumadocs-ui/components/tabs"; @@ -13,7 +13,7 @@ entirely in the worker process and are never put in the queue. Celery has no first-class DI, so you typically open a connection at import time (`db = create_engine(...)`) and share that global. Worker resources replace that pattern: taskito creates one instance per worker, scopes it - (worker / task / thread / request), injects it by name, and tears it down on + (worker / thread / task / pooled), injects it by name, and tears it down on shutdown — no import-time side effects, and clean per-scope lifecycles. @@ -55,11 +55,11 @@ tasks. | `health_check_interval` | `0.0` | Seconds between health checks. `0` disables checking. | | `max_recreation_attempts` | `3` | Max times to recreate after consecutive health failures. | | `scope` | `"worker"` | Lifetime scope — see Resource scopes below. | -| `pool_size` | `None` | Task scope: max concurrent instances (default: 4). | -| `pool_min` | `0` | Task scope: pre-warmed instances at startup. | -| `acquire_timeout` | `10.0` | Task scope: seconds to wait for an available instance. | -| `max_lifetime` | `3600.0` | Task scope: max seconds an instance can live. | -| `idle_timeout` | `300.0` | Task scope: max idle seconds before eviction. | +| `pool_size` | `None` | Pooled scope: max concurrent instances (default: 4). | +| `pool_min` | `0` | Pooled scope: pre-warmed instances at startup. | +| `acquire_timeout` | `10.0` | Pooled scope: seconds to wait for an available instance. | +| `max_lifetime` | `3600.0` | Pooled scope: max seconds an instance can live. | +| `idle_timeout` | `300.0` | Pooled scope: max idle seconds before eviction. | | `reloadable` | `False` | Allow hot reload via SIGHUP or CLI. | | `frozen` | `False` | Wrap instance in a read-only wrapper (see [Configuration](/python/guides/resources/configuration#frozen-resources)). | @@ -105,13 +105,17 @@ over injection. | Scope | Lifetime | Use case | |---|---|---| | `"worker"` (default) | Entire worker process | Database connection pools, shared caches | -| `"task"` | Acquired per-task from a pool, returned after | Short-lived connections with limited concurrency | | `"thread"` | One instance per worker thread, created lazily | Thread-unsafe objects that must not be shared | -| `"request"` | Fresh instance per task, torn down after | Stateful per-request objects | +| `"task"` | Fresh instance per task, torn down after | Stateful per-task objects | +| `"pooled"` | Checked out of a bounded pool per task, returned after | Short-lived connections with limited concurrency | + +Scope names and their meanings are shared across SDKs. There is no `"request"` +scope (a fresh instance per *resolve*) here: resources arrive by injection, +resolved once per task, so there is no second resolve to build for. ```python -# Task scope: each task gets its own session from a pool of up to 10 -@queue.worker_resource("db_session", scope="task", pool_size=10) +# Pooled scope: each task checks out a session from a pool of up to 10 +@queue.worker_resource("db_session", scope="pooled", pool_size=10) def create_session(db): return db() # db must be a worker-scoped resource @@ -120,16 +124,26 @@ def create_session(db): def create_cache(): return {} -# Request scope: a fresh instance for every task, torn down right after -@queue.worker_resource("audit_context", scope="request") +# Task scope: a fresh instance for every task, torn down right after +@queue.worker_resource("audit_context", scope="task") def create_audit_context(): return AuditContext(started_at=time.time()) ``` Pool configuration parameters (`pool_size`, `pool_min`, `acquire_timeout`, -`max_lifetime`, `idle_timeout`) only apply to task-scoped resources. See +`max_lifetime`, `idle_timeout`) only apply to pooled resources — passing +`pool_size` or `pool_min` with any other scope raises. See [Configuration](/python/guides/resources/configuration) for details. + + **Breaking rename.** `"task"` used to mean *checked out of a pool* and + `"request"` meant *fresh per task*; they are now `"pooled"` and `"task"`, the + names the other SDKs already used. `scope="request"` raises. `scope="task"` + still resolves — but now builds per task instead of pooling, so move pooled + resources to `scope="pooled"`. Pool settings on a non-pooled scope raise, which + catches the common case. + + ## Dependencies Resources can declare other resources they depend on. Taskito resolves the diff --git a/sdks/python/taskito/mixins/resources.py b/sdks/python/taskito/mixins/resources.py index a6d9af0c..e20fe37c 100644 --- a/sdks/python/taskito/mixins/resources.py +++ b/sdks/python/taskito/mixins/resources.py @@ -5,6 +5,7 @@ from collections.abc import Callable from typing import TYPE_CHECKING, Any +from taskito.enums import coerce_enum from taskito.exceptions import CircularDependencyError from taskito.interception import InterceptionReport from taskito.resources.definition import ResourceDefinition, ResourceScope @@ -35,7 +36,7 @@ def worker_resource( health_check: Callable | None = None, health_check_interval: float = 0.0, max_recreation_attempts: int = 3, - scope: str = "worker", + scope: ResourceScope | str = ResourceScope.WORKER, pool_size: int | None = None, pool_min: int = 0, acquire_timeout: float = 10.0, @@ -53,10 +54,11 @@ def worker_resource( health_check: Optional callable that returns truthy if healthy. health_check_interval: Seconds between health checks (0 = disabled). max_recreation_attempts: Max times to recreate on health failure. - scope: Resource scope — ``"worker"``, ``"task"``, ``"thread"``, - or ``"request"``. - pool_size: Pool size for task-scoped resources. - pool_min: Minimum pre-warmed instances (task scope). + scope: :class:`~taskito.resources.ResourceScope` (or its string) — + ``WORKER`` (shared), ``THREAD`` (per worker thread), ``TASK`` + (fresh per task), or ``POOLED`` (checkout from a bounded pool). + pool_size: Pool size. ``POOLED`` scope only. + pool_min: Minimum pre-warmed instances. ``POOLED`` scope only. acquire_timeout: Max seconds to wait for pool instance. max_lifetime: Max seconds a pooled instance lives. idle_timeout: Max idle seconds before eviction. @@ -74,7 +76,7 @@ def decorator(factory: Callable[..., Any]) -> Callable[..., Any]: health_check=health_check, health_check_interval=health_check_interval, max_recreation_attempts=max_recreation_attempts, - scope=ResourceScope(scope), + scope=coerce_enum(ResourceScope, scope, param="scope"), pool_size=pool_size, pool_min=pool_min, acquire_timeout=acquire_timeout, diff --git a/sdks/python/taskito/resources/definition.py b/sdks/python/taskito/resources/definition.py index c3783609..be47dd5d 100644 --- a/sdks/python/taskito/resources/definition.py +++ b/sdks/python/taskito/resources/definition.py @@ -9,12 +9,24 @@ class ResourceScope(Enum): - """Lifecycle scope for a resource instance.""" + """Lifetime of a resource instance. Names and wire forms are shared across SDKs. - WORKER = "worker" # shared across all tasks, lives for worker lifetime - TASK = "task" # acquired per-task from a pool, returned after - THREAD = "thread" # one instance per worker thread, created lazily - REQUEST = "request" # fresh instance per task, torn down after + ``REQUEST`` (a fresh instance per *resolve*) has no Python equivalent: + resources arrive by injection, resolved once per task, so there is no second + resolve to build for. + """ + + WORKER = "worker" + """Built once, lazily, and shared by every task on the worker.""" + + THREAD = "thread" + """Built once per worker thread and shared by every task on that thread.""" + + TASK = "task" + """Built fresh per task and torn down when the task ends.""" + + POOLED = "pooled" + """Checked out of a bounded pool for the task's duration, returned at task end.""" @dataclass @@ -29,7 +41,7 @@ class ResourceDefinition: max_recreation_attempts: int = 3 scope: ResourceScope = ResourceScope.WORKER depends_on: list[str] = field(default_factory=list) - # Pool config (task scope only) + # Pool config (POOLED scope only) pool_size: int | None = None # None = worker thread count pool_min: int = 0 acquire_timeout: float = 10.0 @@ -38,3 +50,15 @@ class ResourceDefinition: # Behavior flags reloadable: bool = False frozen: bool = False + + def __post_init__(self) -> None: + # Pool tuning on a non-pooled scope is the one way the 0.21 rename can go + # wrong silently: `scope=TASK` used to mean "checkout from a pool", and now + # means "fresh per task". Reject it here so that misread fails loudly. + if self.scope is not ResourceScope.POOLED and ( + self.pool_size is not None or self.pool_min + ): + raise ValueError( + f"resource {self.name!r}: pool_size/pool_min require " + f"scope={ResourceScope.POOLED}, got scope={self.scope}" + ) diff --git a/sdks/python/taskito/resources/runtime.py b/sdks/python/taskito/resources/runtime.py index 363db7a8..79f915ea 100644 --- a/sdks/python/taskito/resources/runtime.py +++ b/sdks/python/taskito/resources/runtime.py @@ -26,7 +26,7 @@ class ResourceRuntime: """Manages the lifecycle of scoped resources. Worker-scoped resources are initialized eagerly in dependency order. - Task-scoped resources get a pool. Thread-scoped resources use thread-local storage. + Pooled resources get a checkout pool. Thread-scoped resources use thread-local storage. Request-scoped resources are created fresh per task. """ @@ -51,7 +51,7 @@ def initialize(self) -> None: self._create_resource(name) if defn.frozen and name in self._instances: self._instances[name] = FrozenResource(self._instances[name], name) - elif defn.scope == ResourceScope.TASK: + elif defn.scope == ResourceScope.POOLED: pool_size = defn.pool_size or 4 dep_kwargs = {dep: self._instances[dep] for dep in defn.depends_on} pool = ResourcePool( @@ -79,8 +79,8 @@ def initialize(self) -> None: dep_kwargs=dep_kwargs, ) self._thread_locals[name] = store - elif defn.scope == ResourceScope.REQUEST: - pass # created fresh in acquire_for_task + elif defn.scope == ResourceScope.TASK: + pass # built fresh per task in acquire_for_task self._init_duration[name] = time.monotonic() - start @@ -117,10 +117,10 @@ def resolve(self, name: str) -> Any: if name in self._thread_locals: return self._thread_locals[name].get_or_create() - # Task/request scope shouldn't use resolve() directly + # Pooled instances are checked out per task, never resolved as singletons. if name in self._pools: raise ResourceUnavailableError( - f"Resource '{name}' is task-scoped — use acquire_for_task() instead" + f"Resource '{name}' is pooled — use acquire_for_task() instead" ) raise ResourceNotFoundError(f"Resource '{name}' is not initialized") @@ -129,9 +129,9 @@ def acquire_for_task(self, name: str) -> tuple[Any, Callable[[], None] | None]: """Acquire a resource for a single task execution. Returns: - (instance, release_callback) where release_callback is None - for worker/thread scopes and must be called after task completion - for task/request scopes. + (instance, release_callback) where release_callback is None for + worker/thread scopes and must be called after task completion for + task (teardown) and pooled (return to pool) scopes. """ if name not in self._definitions and name not in self._instances: raise ResourceNotFoundError(f"Resource '{name}' is not registered") @@ -147,7 +147,7 @@ def acquire_for_task(self, name: str) -> tuple[Any, Callable[[], None] | None]: if scope == ResourceScope.THREAD: return self._thread_locals[name].get_or_create(), None - if scope == ResourceScope.TASK: + if scope == ResourceScope.POOLED: pool = self._pools[name] instance = pool.acquire() @@ -156,20 +156,20 @@ def release() -> None: return instance, release - if scope == ResourceScope.REQUEST: + if scope == ResourceScope.TASK: deps = defn.depends_on if defn else [] dep_kwargs = {dep: self._instances.get(dep) for dep in deps} instance = defn.factory(**dep_kwargs) if defn else None instance = run_maybe_async(instance) - def teardown_request() -> None: + def teardown_task() -> None: if defn and defn.teardown is not None and instance is not None: try: defn.teardown(instance) except Exception: - logger.exception("Error tearing down request resource '%s'", name) + logger.exception("Error tearing down task resource '%s'", name) - return instance, teardown_request + return instance, teardown_task return self._instances.get(name), None @@ -247,7 +247,7 @@ def status(self) -> list[dict[str, Any]]: "recreations": self._recreation_count.get(name, 0), "depends_on": defn.depends_on if defn else [], } - # Add pool stats for task-scoped resources + # Add pool stats for pooled resources if name in self._pools: entry["pool"] = self._pools[name].stats() result.append(entry) diff --git a/sdks/python/tests/resources/test_resource_system_full.py b/sdks/python/tests/resources/test_resource_system_full.py index 084339d3..30193b24 100644 --- a/sdks/python/tests/resources/test_resource_system_full.py +++ b/sdks/python/tests/resources/test_resource_system_full.py @@ -199,10 +199,12 @@ def test_unwrap_in_walker(self) -> None: class TestResourceScopes: def test_all_scopes_exist(self) -> None: + """Names and wire forms are the cross-SDK contract.""" assert ResourceScope.WORKER.value == "worker" - assert ResourceScope.TASK.value == "task" assert ResourceScope.THREAD.value == "thread" - assert ResourceScope.REQUEST.value == "request" + assert ResourceScope.TASK.value == "task" + assert ResourceScope.POOLED.value == "pooled" + assert [s.value for s in ResourceScope] == ["worker", "thread", "task", "pooled"] def test_pool_config(self) -> None: cfg = PoolConfig(pool_size=5, pool_min=2) @@ -386,7 +388,7 @@ def factory() -> int: defn = ResourceDefinition( name="req", factory=factory, - scope=ResourceScope.REQUEST, + scope=ResourceScope.TASK, ) rt = ResourceRuntime({"req": defn}) rt.initialize() @@ -452,7 +454,7 @@ def test_status_includes_pool(self) -> None: defn = ResourceDefinition( name="db", factory=lambda: {}, - scope=ResourceScope.TASK, + scope=ResourceScope.POOLED, pool_size=5, ) rt = ResourceRuntime({"db": defn}) diff --git a/sdks/python/tests/worker/test_lifecycle_parity.py b/sdks/python/tests/worker/test_lifecycle_parity.py index 7d40f72d..b4a051e2 100644 --- a/sdks/python/tests/worker/test_lifecycle_parity.py +++ b/sdks/python/tests/worker/test_lifecycle_parity.py @@ -116,9 +116,9 @@ def after(self, job: Any, result: Any, error: Any) -> None: db_path=str(tmp_path / "setup.db"), workers=2, default_retry=0, middleware=[Recorder()] ) try: - # Request scope, because its release calls `teardown` outright — a - # task-scoped resource goes back to a pool, where nothing observes it. - @q.worker_resource(name="db", scope="request", teardown=lambda conn: released.append(conn)) + # Task scope, because its release calls `teardown` outright — a pooled + # resource goes back to the pool, where nothing observes it. + @q.worker_resource(name="db", scope="task", teardown=lambda conn: released.append(conn)) def make_db() -> str: return "conn" From c3c5146194095a0acab3c9685c6611b3bde68e75 Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:47:47 +0530 Subject: [PATCH 08/22] feat(node): add the request resource scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fresh on every useResource() call, disposed with the task — the one scope Java had that Node could express. --- .../guides/resources/dependency-injection.mdx | 25 +++++++++++++- sdks/node/src/index.ts | 1 + sdks/node/src/resources/index.ts | 1 + sdks/node/src/resources/runtime.ts | 18 ++++++++++ sdks/node/src/resources/types.ts | 15 ++++++-- sdks/node/test/resources/resources.test.ts | 34 +++++++++++++++++++ 6 files changed, 90 insertions(+), 4 deletions(-) diff --git a/docs/content/docs/node/guides/resources/dependency-injection.mdx b/docs/content/docs/node/guides/resources/dependency-injection.mdx index 80499680..4c88b514 100644 --- a/docs/content/docs/node/guides/resources/dependency-injection.mdx +++ b/docs/content/docs/node/guides/resources/dependency-injection.mdx @@ -36,6 +36,11 @@ A resource's scope decides its lifetime: | `worker` (default) | Lazily, on first use | The worker — a shared singleton | Connection pools, HTTP clients, SDK clients | | `task` | Once per job invocation | That job — disposed when it finishes | Per-job transactions, request-scoped clients | | `pooled` | Lazily, up to `poolSize` at once | Checked out per job, returned when it finishes | Expensive clients that must stay bounded but be reused | +| `request` | On **every** resolve | That job — every instance disposed when it finishes | Short-lived handles a job needs several distinct copies of | + +Scope names and their meanings are shared across SDKs. There is no `thread` +scope here: a worker runs its tasks on one event loop, so there is no per-thread +identity to key an instance on. ```ts queue.resource("tx", async () => db.begin(), { @@ -135,7 +140,25 @@ queue.resource("db", async (ctx) => { Worker-scoped and pooled factories may only depend on worker-scoped resources; reaching for anything shorter-lived throws (it has no job to bind to). -Task-scoped factories may depend on any scope. +Task- and request-scoped factories may depend on any scope. + +## Request scope + +`task` caches its instance for the rest of the job, so every `useResource("x")` +inside one job returns the same object. `request` does not — each resolve builds +a fresh one, and all of them are disposed when the job ends: + +```ts +queue.resource("cursor", () => db.cursor(), { + scope: "request", + dispose: (cursor) => cursor.close(), +}); + +queue.task("scan", async () => { + const a = await useResource("cursor"); + const b = await useResource("cursor"); // a !== b +}); +``` ## Teardown diff --git a/sdks/node/src/index.ts b/sdks/node/src/index.ts index 01dcf817..470f65c2 100644 --- a/sdks/node/src/index.ts +++ b/sdks/node/src/index.ts @@ -57,6 +57,7 @@ export { mockResource, type PoolOptions, type PoolStats, + RESOURCE_SCOPES, type ResourceContext, type ResourceDefinition, type ResourceMetrics, diff --git a/sdks/node/src/resources/index.ts b/sdks/node/src/resources/index.ts index 7071bdc3..ece4fe93 100644 --- a/sdks/node/src/resources/index.ts +++ b/sdks/node/src/resources/index.ts @@ -13,3 +13,4 @@ export type { ResourceScope, ResourceStat, } from "./types"; +export { RESOURCE_SCOPES } from "./types"; diff --git a/sdks/node/src/resources/runtime.ts b/sdks/node/src/resources/runtime.ts index f751bb18..e8e45ca3 100644 --- a/sdks/node/src/resources/runtime.ts +++ b/sdks/node/src/resources/runtime.ts @@ -212,6 +212,24 @@ export class ResourceRuntime { }); return checkout; } + if (def.scope === "request") { + // Fresh on every resolve, never cached: N `useResource()` calls inside one + // task yield N instances, each disposed with the task (LIFO, like the rest). + const ctx: ResourceContext = { + scope: "request", + use: (dep: string) => resolve(dep) as Promise, + }; + const counter = this.counter(name); + counter.created += 1; + const built = Promise.resolve() + .then(() => startFactory(def, ctx)) + .catch((error) => { + counter.created -= 1; // a failed build is not a live resource + throw error; + }); + this.trackResource(taskTeardown, name, def, built); + return built; + } const cached = taskCache.get(name); if (cached) { return cached; diff --git a/sdks/node/src/resources/types.ts b/sdks/node/src/resources/types.ts index cce27ee7..ba2eb807 100644 --- a/sdks/node/src/resources/types.ts +++ b/sdks/node/src/resources/types.ts @@ -1,10 +1,19 @@ -/** Lifetime of a registered resource. */ -export type ResourceScope = "worker" | "task" | "pooled"; +/** + * Lifetime of a registered resource. Names and wire forms are shared across SDKs. + * + * `"thread"` has no Node equivalent — a worker runs its tasks on one event loop, + * so there is no per-thread identity to key an instance on. + */ +export type ResourceScope = "worker" | "task" | "pooled" | "request"; + +/** Every resource scope, for runtime validation. */ +export const RESOURCE_SCOPES: readonly ResourceScope[] = ["worker", "task", "pooled", "request"]; /** * Passed to a resource factory so it can depend on other resources. Worker- and * pooled-scoped factories may only depend on worker-scoped resources (their - * instances outlive any single task); a task-scoped factory may depend on any. + * instances outlive any single task); task- and request-scoped factories may + * depend on any. */ export interface ResourceContext { /** Scope of the resource currently being built. */ diff --git a/sdks/node/test/resources/resources.test.ts b/sdks/node/test/resources/resources.test.ts index 19deb810..81382b08 100644 --- a/sdks/node/test/resources/resources.test.ts +++ b/sdks/node/test/resources/resources.test.ts @@ -282,6 +282,40 @@ describe("ResourcePool", () => { }); }); +describe("request-scoped resources in the runtime", () => { + it("builds a fresh instance per resolve and disposes each with the task", async () => { + const rt = new ResourceRuntime(); + let builds = 0; + const disposed: number[] = []; + rt.register("cursor", { + scope: "request", + factory: () => ({ id: ++builds }), + dispose: (value) => { + disposed.push((value as { id: number }).id); + }, + }); + const scope = rt.createTaskScope(); + const first = await scope.resolver("cursor"); + const second = await scope.resolver("cursor"); + // Unlike task scope, a second resolve is not the cached first one. + expect(first).not.toBe(second); + expect(builds).toBe(2); + await scope.teardown(); + expect(disposed.sort()).toEqual([1, 2]); + expect(rt.metrics().cursor).toEqual({ created: 2, disposed: 2, active: 0 }); + }); + + it("caches a task-scoped resource for the whole task, unlike request scope", async () => { + const rt = new ResourceRuntime(); + let builds = 0; + rt.register("perTask", { scope: "task", factory: () => ({ id: ++builds }) }); + const scope = rt.createTaskScope(); + expect(await scope.resolver("perTask")).toBe(await scope.resolver("perTask")); + expect(builds).toBe(1); + await scope.teardown(); + }); +}); + describe("pooled resources in the runtime", () => { it("checks out one pooled instance per task and returns it on teardown", async () => { const rt = new ResourceRuntime(); From c31d8dccd55f4f3bb8a16d3507270316e54903aa Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:47:59 +0530 Subject: [PATCH 09/22] docs: record which resource scopes are platform-bound THREAD has no Node equivalent, REQUEST no Python one. Writing that down is what makes the shared names trustworthy. --- CHANGELOG.md | 26 +++++++++++++++++++ .../guides/resources/dependency-injection.mdx | 5 ++++ .../taskito/resources/ResourceScope.java | 6 ++++- 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a2497e6..da20da37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,32 @@ All notable changes to taskito are documented here. The format is based on [Semantic Versioning](https://semver.org/spec/v2.0.0.html). All SDKs (Python, Node, Java) and the underlying Rust crates are released together, in lock-step. +## Unreleased + +### Changed + +- **BREAKING (Python): resource scopes renamed to match the other SDKs.** + `ResourceScope.TASK` meant *checked out of a bounded pool* and `REQUEST` meant *fresh per + task* — the reverse of what Java and Node call those names, so the same `scope="task"` code + behaved differently per SDK. Python now uses `POOLED` (was `TASK`) and `TASK` (was `REQUEST`); + `REQUEST` is gone, and `scope="request"` raises. `scope="task"` still resolves but now builds + per task instead of pooling, so pooled resources must move to `scope="pooled"` — passing + `pool_size`/`pool_min` with any other scope now raises, which catches that case. +- Closed-set parameters across the core and all three SDKs take enums instead of bare strings: + built-in proxy ids, interception mode, predicate `on_false`, log-consumer `on_error`, gate + `on_timeout`, workflow diagram format, fan-out strategy, dispatch order, task-log level, + workflow run state, dashboard role, subscription mode, and worker status. Wire and stored + values are unchanged, and existing string callers keep working (Java keeps its `String` + overloads, deprecated). + +### Added + +- **Node `request` resource scope.** A fresh instance on every `useResource()` call, each + disposed when the task ends — matching the Java scope of the same name. +- Job outcome events report how long the task ran: `durationMs()` on Java's `OutcomeEvent`, + `durationMs` on Node's, and `duration_ms` on Python's job event payloads. Java also gains + `NodeSnapshot.durationMs()` / `compensationDurationMs()` and `TaskContext.elapsedMs()`. + ## 0.21.0 Overload-controls and retention release. The queue gains admission and load-shedding controls, diff --git a/docs/content/docs/java/guides/resources/dependency-injection.mdx b/docs/content/docs/java/guides/resources/dependency-injection.mdx index 4b332575..d41406a6 100644 --- a/docs/content/docs/java/guides/resources/dependency-injection.mdx +++ b/docs/content/docs/java/guides/resources/dependency-injection.mdx @@ -40,6 +40,11 @@ A resource's `ResourceScope` decides its lifetime: | `REQUEST` | Fresh on every `use()` | Disposed with the task; never cached | Values that must not be shared even within one job | | `POOLED` | Lazily, up to `poolSize` at once | Checked out per task, returned at task end | Expensive clients that must stay bounded but be reused | +Scope names and their meanings are shared across SDKs. Two are platform-bound: +`THREAD` has no Node equivalent (a Node worker runs its tasks on one event loop), +and `REQUEST` has no Python equivalent (Python resources arrive by injection, +resolved once per task, so there is no second resolve to build for). + ```java queue.resource("tx", ResourceScope.TASK, ctx -> beginTransaction(), diff --git a/sdks/java/src/main/java/org/byteveda/taskito/resources/ResourceScope.java b/sdks/java/src/main/java/org/byteveda/taskito/resources/ResourceScope.java index afbebb18..6f1d17ea 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/resources/ResourceScope.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/resources/ResourceScope.java @@ -1,6 +1,10 @@ package org.byteveda.taskito.resources; -/** Lifetime of a worker resource. */ +/** + * Lifetime of a worker resource. Names and wire forms are shared across SDKs; two are + * platform-bound — {@link #THREAD} has no Node equivalent (one event loop, so no per-thread + * identity) and {@link #REQUEST} has no Python equivalent (resources are injected once per task). + */ public enum ResourceScope { /** Built once, lazily, and shared across every task on the worker. */ WORKER, From dcef1405c944a377a779603ad16ba88cead29f9f Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:26:40 +0530 Subject: [PATCH 10/22] fix(java): keep log and run filters on a wire string 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. --- .../org/byteveda/taskito/DefaultTaskito.java | 19 ++---------- .../java/org/byteveda/taskito/Taskito.java | 23 ++++++--------- .../taskito/dashboard/api/CoreHandlers.java | 3 -- .../dashboard/api/WorkflowsHandlers.java | 3 -- .../org/byteveda/taskito/core/QueueTest.java | 29 +++++++++++++++++++ 5 files changed, 40 insertions(+), 37 deletions(-) diff --git a/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java b/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java index 84d47b65..4180ae6d 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java @@ -81,7 +81,6 @@ import org.byteveda.taskito.workflows.Step; import org.byteveda.taskito.workflows.Workflow; import org.byteveda.taskito.workflows.WorkflowRun; -import org.byteveda.taskito.workflows.WorkflowState; import org.byteveda.taskito.workflows.WorkflowStatus; /** @@ -744,15 +743,8 @@ public List getTaskLogsAfter(String jobId, String afterId) { } @Override - public List queryTaskLogs(String taskName, TaskLogLevel level, long sinceMs, long limit) { - String wire = level == null ? null : level.wire(); - return decodeList(backend.queryTaskLogsJson(taskName, wire, sinceMs, limit), TaskLog.class); - } - - @Override - @Deprecated public List queryTaskLogs(String taskName, String level, long sinceMs, long limit) { - return queryTaskLogs(taskName, level == null ? null : TaskLogLevel.fromWire(level), sinceMs, limit); + return decodeList(backend.queryTaskLogsJson(taskName, level, sinceMs, limit), TaskLog.class); } @Override @@ -1175,15 +1167,8 @@ public void cancelWorkflow(String runId) { } @Override - public List listWorkflowRuns(String definitionName, WorkflowState state, long limit, long offset) { - String wire = state == null ? null : state.wire(); - return decodeList(backend.listWorkflowRunsJson(definitionName, wire, limit, offset), WorkflowRunInfo.class); - } - - @Override - @Deprecated public List listWorkflowRuns(String definitionName, String state, long limit, long offset) { - return listWorkflowRuns(definitionName, state == null ? null : WorkflowState.fromWire(state), limit, offset); + return decodeList(backend.listWorkflowRunsJson(definitionName, state, limit, offset), WorkflowRunInfo.class); } @Override diff --git a/sdks/java/src/main/java/org/byteveda/taskito/Taskito.java b/sdks/java/src/main/java/org/byteveda/taskito/Taskito.java index 62082eae..d949b168 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/Taskito.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/Taskito.java @@ -368,15 +368,13 @@ static Builder builder() { /** Logs for a job with id after {@code afterId} (UUIDv7-ordered cursor); null = all. */ List getTaskLogsAfter(String jobId, String afterId); - /** Logs across jobs filtered by task/level, at or after {@code sinceMs}, capped at {@code limit}. */ - List queryTaskLogs(String taskName, TaskLogLevel level, long sinceMs, long limit); - /** - * Logs across jobs filtered by a wire-form level. - * - * @deprecated use {@link #queryTaskLogs(String, TaskLogLevel, long, long)}. + * Logs across jobs filtered by task/level, at or after {@code sinceMs}, capped at + * {@code limit}. {@code level} is the wire form ({@link TaskLogLevel#wire()}), not the + * enum: a filter is open by nature — {@code null} means no filter, and an unrecognized + * value must return nothing rather than throw, since it typically arrives from a query + * string. */ - @Deprecated List queryTaskLogs(String taskName, String level, long sinceMs, long limit); // ── Locks ─────────────────────────────────────────────────────── @@ -586,15 +584,12 @@ Taskito logConsumer( /** Cancel a workflow run: skip its pending nodes and mark it cancelled. */ void cancelWorkflow(String runId); - /** Workflow run summaries, filtered by definition name and/or state, paged. Nulls mean no filter. */ - List listWorkflowRuns(String definitionName, WorkflowState state, long limit, long offset); - /** - * Workflow run summaries filtered by a wire-form state. - * - * @deprecated use {@link #listWorkflowRuns(String, WorkflowState, long, long)}. + * Workflow run summaries, filtered by definition name and/or state, paged. Nulls mean no + * filter. {@code state} is the wire form ({@link WorkflowState#wire()}) rather than the enum, + * so that a bare {@code null} filter stays unambiguous; unlike the log level, an + * unrecognized state is rejected by the core. */ - @Deprecated List listWorkflowRuns(String definitionName, String state, long limit, long offset); /** A single workflow run summary, or empty if the run no longer exists. */ diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/CoreHandlers.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/CoreHandlers.java index 10d68d42..d547af0b 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/CoreHandlers.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/CoreHandlers.java @@ -89,9 +89,6 @@ public Object resume(String name) { } /** Logs across jobs filtered by task/level; {@code since} is a lookback in seconds. */ - // Deliberately the string overload: an unknown ?level= is untrusted input that - // must filter to nothing, not fail the request. - @SuppressWarnings("deprecation") public Object logs(Map query) { long sinceSeconds = Http.longParam(query, "since", 3600); long sinceMs = System.currentTimeMillis() - sinceSeconds * 1000; diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/WorkflowsHandlers.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/WorkflowsHandlers.java index fc3d8964..f8f1f381 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/WorkflowsHandlers.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/WorkflowsHandlers.java @@ -25,9 +25,6 @@ public WorkflowsHandlers(Taskito queue) { this.queue = queue; } - // Deliberately the string overload: an unknown ?state= is untrusted input that - // must filter to nothing, not fail the request. - @SuppressWarnings("deprecation") public Object runs(Map query) { long limit = Http.longParam(query, "limit", DEFAULT_LIMIT); long offset = Http.longParam(query, "offset", 0); diff --git a/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java b/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java index 8199a326..03b6d398 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java @@ -14,6 +14,7 @@ import java.util.Optional; import org.byteveda.taskito.Queue; import org.byteveda.taskito.Taskito; +import org.byteveda.taskito.TaskitoException; import org.byteveda.taskito.model.Job; import org.byteveda.taskito.model.JobFilter; import org.byteveda.taskito.model.JobStatus; @@ -161,4 +162,32 @@ void taskLogsRoundTrip(@TempDir Path dir) { assertEquals("hello", logs.get(0).message); } } + + @Test + void logFiltersTakeAWireStringAndStayLenient(@TempDir Path dir) { + // The filters take a wire string rather than an enum so that `null` (no filter) + // still compiles — an enum/String overload pair is ambiguous on a bare null. + // An unrecognized level filters to nothing, since it usually arrives from a + // dashboard query string. + try (Taskito queue = open(dir)) { + String id = queue.enqueue("send_email", Collections.singletonMap("to", "a")); + queue.writeTaskLog(id, "send_email", TaskLogLevel.INFO, "hello"); + + assertEquals(1, queue.queryTaskLogs(null, null, 0, 10).size()); + assertEquals( + 1, + queue.queryTaskLogs(null, TaskLogLevel.INFO.wire(), 0, 10).size()); + assertTrue(queue.queryTaskLogs(null, "verbose", 0, 10).isEmpty()); + } + } + + @Test + void unknownWorkflowStateFilterIsRejectedByTheCore(@TempDir Path dir) { + // Unlike the log level, the state filter is validated in the core, so an + // unrecognized value raises instead of filtering to nothing. + try (Taskito queue = open(dir)) { + assertEquals(0, queue.listWorkflowRuns(null, null, 10, 0).size()); + assertThrows(TaskitoException.class, () -> queue.listWorkflowRuns(null, "nonsense", 10, 0)); + } + } } From 981446b2c089b3d0ccc42d198730b68dc0decaed Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:26:50 +0530 Subject: [PATCH 11/22] fix: fail closed on an unusable dashboard role 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. --- .../taskito/dashboard/auth/AuthStore.java | 3 +++ .../taskito/dashboard/auth/AuthStoreTest.java | 1 + sdks/python/taskito/dashboard/auth.py | 2 +- sdks/python/tests/dashboard/test_auth.py | 23 +++++++++++++++++++ 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java index 837163f7..1327291d 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java @@ -204,6 +204,9 @@ public Session createSession(String username, Role role) { } public Session createSession(String username, Role role, long ttlSeconds) { + if (role == null) { + throw DashboardError.badRequest("invalid role"); + } String token = Tokens.session(); String csrf = Tokens.session(); long now = nowSeconds(); diff --git a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java index 9163fea5..6a3b7395 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java @@ -47,6 +47,7 @@ void rejectsDuplicatesAndInvalidInput() { assertThrows(DashboardError.class, () -> store.createUser("bad name", "password123", Role.ADMIN)); assertThrows(DashboardError.class, () -> store.createUser("carol", "short", Role.ADMIN)); assertThrows(DashboardError.class, () -> store.createUser("carol", "password123", null)); + assertThrows(DashboardError.class, () -> store.createSession("carol", null)); // An unknown role is no longer representable at the call site; a stored one // still has to fail closed. assertEquals(Role.VIEWER, Role.orViewer("superuser")); diff --git a/sdks/python/taskito/dashboard/auth.py b/sdks/python/taskito/dashboard/auth.py index 8d10dfc2..387179d7 100644 --- a/sdks/python/taskito/dashboard/auth.py +++ b/sdks/python/taskito/dashboard/auth.py @@ -399,7 +399,7 @@ def get_session(self, token: str) -> Session | None: except json.JSONDecodeError: return None try: - session = Session(token=token, **data) + session = Session(token=token, **{**data, "role": _role_or_viewer(data.get("role"))}) except TypeError: return None if session.is_expired(): diff --git a/sdks/python/tests/dashboard/test_auth.py b/sdks/python/tests/dashboard/test_auth.py index 4f6ba17d..2245348b 100644 --- a/sdks/python/tests/dashboard/test_auth.py +++ b/sdks/python/tests/dashboard/test_auth.py @@ -21,7 +21,9 @@ from taskito import Queue from taskito.dashboard import _make_handler from taskito.dashboard.auth import ( + SESSION_PREFIX, AuthStore, + Role, bootstrap_admin_from_env, hash_password, verify_password, @@ -142,6 +144,27 @@ def test_create_and_get_session(queue: Queue) -> None: assert not fetched.is_expired() +def test_session_role_is_the_enum(queue: Queue) -> None: + store = AuthStore(queue) + session = store.create_session(store.create_user("alice", "hunter2-secret")) + fetched = store.get_session(session.token) + assert fetched is not None + assert fetched.role is Role.ADMIN + + +def test_unreadable_persisted_role_falls_back_to_viewer(queue: Queue) -> None: + """A stored role nothing recognizes must not be handed back as-is.""" + store = AuthStore(queue) + session = store.create_session(store.create_user("alice", "hunter2-secret")) + raw = json.loads(queue.get_setting(SESSION_PREFIX + session.token) or "{}") + raw["role"] = "superuser" + queue.set_setting(SESSION_PREFIX + session.token, json.dumps(raw)) + + fetched = store.get_session(session.token) + assert fetched is not None + assert fetched.role is Role.VIEWER + + def test_get_session_unknown_token_returns_none(queue: Queue) -> None: assert AuthStore(queue).get_session("nope") is None assert AuthStore(queue).get_session("") is None From 6b2e1d416ae5d58eda33787c17f61923d2b305a9 Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:27:05 +0530 Subject: [PATCH 12/22] fix(python): coerce resource scope and rebuild pools on reload A wire-string scope matched no runtime branch, reload left pooled resources on a stale pool, and an async task teardown was dropped unawaited. --- .../python/guides/resources/configuration.mdx | 2 +- .../guides/resources/dependency-injection.mdx | 2 +- sdks/python/taskito/resources/definition.py | 12 +++- sdks/python/taskito/resources/runtime.py | 54 ++++++++++------ .../resources/test_resource_system_full.py | 63 +++++++++++++++++++ 5 files changed, 109 insertions(+), 24 deletions(-) diff --git a/docs/content/docs/python/guides/resources/configuration.mdx b/docs/content/docs/python/guides/resources/configuration.mdx index 59b9063c..e1bbc75b 100644 --- a/docs/content/docs/python/guides/resources/configuration.mdx +++ b/docs/content/docs/python/guides/resources/configuration.mdx @@ -92,7 +92,7 @@ pool that manages concurrent instances. ```python @queue.worker_resource( "session", - scope="task", + scope="pooled", pool_size=20, pool_min=5, acquire_timeout=5.0, diff --git a/docs/content/docs/python/guides/resources/dependency-injection.mdx b/docs/content/docs/python/guides/resources/dependency-injection.mdx index d257b4fe..40045f28 100644 --- a/docs/content/docs/python/guides/resources/dependency-injection.mdx +++ b/docs/content/docs/python/guides/resources/dependency-injection.mdx @@ -115,7 +115,7 @@ resolved once per task, so there is no second resolve to build for. ```python # Pooled scope: each task checks out a session from a pool of up to 10 -@queue.worker_resource("db_session", scope="pooled", pool_size=10) +@queue.worker_resource("db_session", scope="pooled", pool_size=10, depends_on=["db"]) def create_session(db): return db() # db must be a worker-scoped resource diff --git a/sdks/python/taskito/resources/definition.py b/sdks/python/taskito/resources/definition.py index be47dd5d..fb438bb2 100644 --- a/sdks/python/taskito/resources/definition.py +++ b/sdks/python/taskito/resources/definition.py @@ -7,6 +7,8 @@ from enum import Enum from typing import Any +from taskito.enums import coerce_enum + class ResourceScope(Enum): """Lifetime of a resource instance. Names and wire forms are shared across SDKs. @@ -52,9 +54,13 @@ class ResourceDefinition: frozen: bool = False def __post_init__(self) -> None: - # Pool tuning on a non-pooled scope is the one way the 0.21 rename can go - # wrong silently: `scope=TASK` used to mean "checkout from a pool", and now - # means "fresh per task". Reject it here so that misread fails loudly. + # Coerce first: a wire string is accepted here (TOML config, hand-built + # definitions), and the runtime dispatches on enum identity — a raw string + # would match no branch and leave the resource silently uninitialized. + self.scope = coerce_enum(ResourceScope, self.scope, param="scope") + # Pool tuning on a non-pooled scope is the one way the rename can go wrong + # silently: `scope=TASK` used to mean "checkout from a pool", and now means + # "fresh per task". Reject it here so that misread fails loudly. if self.scope is not ResourceScope.POOLED and ( self.pool_size is not None or self.pool_min ): diff --git a/sdks/python/taskito/resources/runtime.py b/sdks/python/taskito/resources/runtime.py index 79f915ea..187e73c2 100644 --- a/sdks/python/taskito/resources/runtime.py +++ b/sdks/python/taskito/resources/runtime.py @@ -52,24 +52,7 @@ def initialize(self) -> None: if defn.frozen and name in self._instances: self._instances[name] = FrozenResource(self._instances[name], name) elif defn.scope == ResourceScope.POOLED: - pool_size = defn.pool_size or 4 - dep_kwargs = {dep: self._instances[dep] for dep in defn.depends_on} - pool = ResourcePool( - name=name, - factory=defn.factory, - teardown=defn.teardown, - config=PoolConfig( - pool_size=pool_size, - pool_min=defn.pool_min, - acquire_timeout=defn.acquire_timeout, - max_lifetime=defn.max_lifetime, - idle_timeout=defn.idle_timeout, - ), - dep_kwargs=dep_kwargs, - ) - if defn.pool_min > 0: - pool.prewarm() - self._pools[name] = pool + self._build_pool(name) elif defn.scope == ResourceScope.THREAD: dep_kwargs = {dep: self._instances[dep] for dep in defn.depends_on} store = ThreadLocalStore( @@ -84,6 +67,35 @@ def initialize(self) -> None: self._init_duration[name] = time.monotonic() - start + def _build_pool(self, name: str) -> None: + """Build (or rebuild) a pooled resource's checkout pool. + + Replacing the pool object is how a reload swaps it: in-flight checkouts + release into the old, shut-down pool, which tears them down instead of + handing them out again. + """ + defn = self._definitions[name] + previous = self._pools.get(name) + dep_kwargs = {dep: self._instances[dep] for dep in defn.depends_on} + pool = ResourcePool( + name=name, + factory=defn.factory, + teardown=defn.teardown, + config=PoolConfig( + pool_size=defn.pool_size or 4, + pool_min=defn.pool_min, + acquire_timeout=defn.acquire_timeout, + max_lifetime=defn.max_lifetime, + idle_timeout=defn.idle_timeout, + ), + dep_kwargs=dep_kwargs, + ) + if defn.pool_min > 0: + pool.prewarm() + self._pools[name] = pool + if previous is not None: + previous.shutdown() + def _create_resource(self, name: str) -> None: """Invoke a resource factory, injecting its declared dependencies.""" defn = self._definitions[name] @@ -165,7 +177,7 @@ def release() -> None: def teardown_task() -> None: if defn and defn.teardown is not None and instance is not None: try: - defn.teardown(instance) + run_maybe_async(defn.teardown(instance)) except Exception: logger.exception("Error tearing down task resource '%s'", name) @@ -178,6 +190,10 @@ def recreate(self, name: str) -> bool: try: old = self._instances.get(name) defn = self._definitions[name] + if defn.scope == ResourceScope.POOLED: + self._build_pool(name) + 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) diff --git a/sdks/python/tests/resources/test_resource_system_full.py b/sdks/python/tests/resources/test_resource_system_full.py index 30193b24..cb61e311 100644 --- a/sdks/python/tests/resources/test_resource_system_full.py +++ b/sdks/python/tests/resources/test_resource_system_full.py @@ -675,3 +675,66 @@ def process(order_id: int, db: Inject["db"] = None) -> str: # type: ignore[type process.delay(1) assert results[0].return_value == "1:injected" + + +class TestScopeCoercionAndReload: + """Regressions from the scope rename (#503, split 6).""" + + def test_wire_string_scope_is_coerced_to_the_enum(self) -> None: + """The runtime dispatches on enum identity — a raw string would match no branch.""" + defn = ResourceDefinition(name="db", factory=lambda: {}, scope="thread") # type: ignore[arg-type] + assert defn.scope is ResourceScope.THREAD + + def test_unknown_scope_raises_naming_the_valid_set(self) -> None: + with pytest.raises(ValueError, match="'worker', 'thread', 'task', 'pooled'"): + ResourceDefinition(name="db", factory=lambda: {}, scope="request") # type: ignore[arg-type] + + def test_pool_settings_on_a_non_pooled_scope_raise(self) -> None: + """Catches the one case the TASK/POOLED swap could otherwise pass over quietly.""" + with pytest.raises(ValueError, match="pool_size/pool_min require"): + ResourceDefinition( + name="db", factory=lambda: {}, scope=ResourceScope.TASK, pool_size=4 + ) + + def test_reloading_a_pooled_resource_rebuilds_its_pool(self) -> None: + """recreate() used to add a singleton and leave task injection on the stale pool.""" + builds: list[int] = [] + + def factory() -> dict: + builds.append(1) + return {"n": len(builds)} + + defn = ResourceDefinition( + name="conn", factory=factory, scope=ResourceScope.POOLED, pool_size=2, reloadable=True + ) + runtime = ResourceRuntime({"conn": defn}) + runtime.initialize() + first, release = runtime.acquire_for_task("conn") + assert release is not None + release() + + assert runtime.reload() == {"conn": True} + second, release2 = runtime.acquire_for_task("conn") + assert release2 is not None + release2() + # A rebuilt pool hands out a new instance, and nothing leaked into the + # singleton map that resolve() would have returned instead. + assert second is not first + assert "conn" not in runtime._instances + + def test_async_task_teardown_is_awaited(self) -> None: + """A returned coroutine used to be dropped, so async teardowns never ran.""" + torn: list[str] = [] + + async def teardown(instance: str) -> None: + torn.append(instance) + + defn = ResourceDefinition( + name="ctx", factory=lambda: "value", teardown=teardown, scope=ResourceScope.TASK + ) + runtime = ResourceRuntime({"ctx": defn}) + runtime.initialize() + _, release = runtime.acquire_for_task("ctx") + assert release is not None + release() + assert torn == ["value"] From 68a874608b15268537b36db433a9c48f11d6e5f3 Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:44:37 +0530 Subject: [PATCH 13/22] fix: reject an unknown subscription mode at the binding boundary The lenient reader exists for persisted rows; caller input must not silently become a fan-out subscription. --- crates/taskito-core/src/storage/records.rs | 10 ++++++++++ crates/taskito-java/src/queue/pubsub.rs | 7 ++++++- crates/taskito-node/src/queue/pubsub.rs | 18 ++++++++++++++---- crates/taskito-python/src/py_queue/pubsub.rs | 14 ++++++++++++-- 4 files changed, 42 insertions(+), 7 deletions(-) diff --git a/crates/taskito-core/src/storage/records.rs b/crates/taskito-core/src/storage/records.rs index c9798bdf..9ee9e101 100644 --- a/crates/taskito-core/src/storage/records.rs +++ b/crates/taskito-core/src/storage/records.rs @@ -211,6 +211,7 @@ impl SubscriptionMode { /// Parse a persisted `mode`. Anything unrecognized reads as fan-out, which is /// what the column's pre-enum readers did with a value that wasn't `"log"`. + /// Use [`Self::parse`] for caller input, where a typo must not pass silently. pub fn from_wire(wire: &str) -> Self { match wire { "log" => SubscriptionMode::Log, @@ -218,6 +219,15 @@ impl SubscriptionMode { } } + /// Strictly parse caller-supplied input; `None` for anything outside the set. + pub fn parse(wire: &str) -> Option { + match wire { + "fanout" => Some(SubscriptionMode::Fanout), + "log" => Some(SubscriptionMode::Log), + _ => None, + } + } + /// Whether this mode is the append-once + cursor model. pub fn is_log(&self) -> bool { matches!(self, SubscriptionMode::Log) diff --git a/crates/taskito-java/src/queue/pubsub.rs b/crates/taskito-java/src/queue/pubsub.rs index 0592a9c1..40df802a 100644 --- a/crates/taskito-java/src/queue/pubsub.rs +++ b/crates/taskito-java/src/queue/pubsub.rs @@ -54,7 +54,12 @@ pub extern "system" fn Java_org_byteveda_taskito_internal_NativeQueue_registerSu let task_name = read_string(env, &task_name)?; let queue = read_string(env, &queue)?; let owner_worker_id = read_optional_string(env, &owner_worker_id)?; - let mode = SubscriptionMode::from_wire(&read_string(env, &mode)?); + let mode_wire = read_string(env, &mode)?; + let mode = SubscriptionMode::parse(&mode_wire).ok_or_else(|| { + crate::error::BindingError::new(format!( + "unknown subscription mode '{mode_wire}' (expected 'fanout' or 'log')" + )) + })?; // An ownerless ephemeral row would never be reaped (the reaper matches // on owner) yet keeps receiving deliveries — reject it up front. if durable == 0 && owner_worker_id.is_none() { diff --git a/crates/taskito-node/src/queue/pubsub.rs b/crates/taskito-node/src/queue/pubsub.rs index 980a0e16..3b5fc242 100644 --- a/crates/taskito-node/src/queue/pubsub.rs +++ b/crates/taskito-node/src/queue/pubsub.rs @@ -45,9 +45,14 @@ impl JsQueue { "an ephemeral subscription (durable=false) requires ownerWorkerId", )); } - let mode = mode - .map(|m| SubscriptionMode::from_wire(&m)) - .unwrap_or_default(); + let mode = match mode { + Some(m) => SubscriptionMode::parse(&m).ok_or_else(|| { + invalid_arg(format!( + "unknown subscription mode '{m}' (expected 'fanout' or 'log')" + )) + })?, + None => SubscriptionMode::default(), + }; let storage = self.storage.clone(); spawn_blocking(move || { let row = NewSubscription { @@ -194,10 +199,15 @@ impl JsQueue { mode: String, retention_ms: Option, ) -> Result<()> { + let parsed = SubscriptionMode::parse(&mode).ok_or_else(|| { + invalid_arg(format!( + "unknown subscription mode '{mode}' (expected 'fanout' or 'log')" + )) + })?; let storage = self.storage.clone(); spawn_blocking(move || { storage - .declare_topic(&name, SubscriptionMode::from_wire(&mode), retention_ms) + .declare_topic(&name, parsed, retention_ms) .map_err(to_napi_err) }) .await diff --git a/crates/taskito-python/src/py_queue/pubsub.rs b/crates/taskito-python/src/py_queue/pubsub.rs index 06436646..64f235f0 100644 --- a/crates/taskito-python/src/py_queue/pubsub.rs +++ b/crates/taskito-python/src/py_queue/pubsub.rs @@ -5,6 +5,16 @@ use taskito_core::pubsub::{publish_to_topic, DeliveryDefaults, PublishRequest}; use taskito_core::storage::records::{NewSubscription, SubscriptionMode}; use taskito_core::storage::Storage; +/// Strictly parse a caller-supplied subscription mode. Unlike the lenient reader +/// used for persisted rows, a typo here is a caller error, not a legacy value. +fn parse_mode(mode: &str) -> PyResult { + SubscriptionMode::parse(mode).ok_or_else(|| { + pyo3::exceptions::PyValueError::new_err(format!( + "unknown subscription mode '{mode}' (expected 'fanout' or 'log')" + )) + }) +} + use super::PyQueue; use crate::py_job::PyJob; @@ -80,7 +90,7 @@ impl PyQueue { priority, max_retries, timeout_ms, - mode: SubscriptionMode::from_wire(mode), + mode: parse_mode(mode)?, }; self.storage .register_subscription(&row) @@ -272,7 +282,7 @@ impl PyQueue { retention_ms: Option, ) -> PyResult<()> { let storage = &self.storage; - let mode = SubscriptionMode::from_wire(mode); + let mode = parse_mode(mode)?; py.detach(|| storage.declare_topic(name, mode, retention_ms)) .map_err(|e| pyo3::exceptions::PyRuntimeError::new_err(e.to_string())) } From c1862d8501f83ef3bf0a0252b7ace73b900e611b Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:44:50 +0530 Subject: [PATCH 14/22] fix(java): keep wire-form role overloads on the auth store AuthStore is public, so making it Role-only broke existing string callers; the wire-form overloads delegate through a strict parse. --- .../taskito/dashboard/auth/AuthStore.java | 24 +++++++++++++++++++ .../taskito/dashboard/auth/AuthStoreTest.java | 7 ++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java index 1327291d..3e704dd1 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java @@ -58,6 +58,11 @@ public Optional getUser(String username) { * cross-process CAS, so a second writer in another process is still a * theoretical race (bounded to first-run setup, which is single-shot). */ + public synchronized User createUser(String username, String password, String role) { + return createUser(username, password, parseRole(role)); + } + + /** Create a password user with a typed role. */ public synchronized User createUser(String username, String password, Role role) { validateUsername(username); validatePassword(password); @@ -203,6 +208,16 @@ public Session createSession(String username, Role role) { return createSession(username, role, DEFAULT_SESSION_TTL_SECONDS); } + /** Open a session from a wire-form role ({@code "admin"} / {@code "viewer"}). */ + public Session createSession(String username, String role) { + return createSession(username, parseRole(role), DEFAULT_SESSION_TTL_SECONDS); + } + + /** Open a session from a wire-form role, with an explicit TTL. */ + public Session createSession(String username, String role, long ttlSeconds) { + return createSession(username, parseRole(role), ttlSeconds); + } + public Session createSession(String username, Role role, long ttlSeconds) { if (role == null) { throw DashboardError.badRequest("invalid role"); @@ -373,6 +388,15 @@ private static void validateUsername(String username) { } } + /** Strictly parse a wire-form role; anything outside the set is a request error. */ + private static Role parseRole(String role) { + Role parsed = Role.fromWire(role); + if (parsed == null) { + throw DashboardError.badRequest("invalid role"); + } + return parsed; + } + private static void validatePassword(String password) { if (password == null || password.length() < PASSWORD_MIN_LEN || password.length() > PASSWORD_MAX_LEN) { throw DashboardError.badRequest("password must be 8-256 characters"); diff --git a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java index 6a3b7395..16a6c298 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java @@ -46,11 +46,14 @@ void rejectsDuplicatesAndInvalidInput() { assertThrows(DashboardError.class, () -> store.createUser("bob", "password123", Role.ADMIN)); assertThrows(DashboardError.class, () -> store.createUser("bad name", "password123", Role.ADMIN)); assertThrows(DashboardError.class, () -> store.createUser("carol", "short", Role.ADMIN)); - assertThrows(DashboardError.class, () -> store.createUser("carol", "password123", null)); - assertThrows(DashboardError.class, () -> store.createSession("carol", null)); + assertThrows(DashboardError.class, () -> store.createUser("carol", "password123", (Role) null)); + assertThrows(DashboardError.class, () -> store.createSession("carol", (Role) null)); // An unknown role is no longer representable at the call site; a stored one // still has to fail closed. assertEquals(Role.VIEWER, Role.orViewer("superuser")); + // The wire-form overloads reject an unknown role rather than coercing it. + assertThrows(DashboardError.class, () -> store.createUser("carol", "password123", "root")); + assertThrows(DashboardError.class, () -> store.createSession("carol", "root")); } @Test From f51ada2df71ad5e16a263886d0f6664b473c9c0c Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:45:01 +0530 Subject: [PATCH 15/22] fix(node): validate resource scope and reject request-scope cycles 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. --- sdks/node/src/resources/runtime.ts | 39 +++++++++++++++++++--- sdks/node/test/resources/resources.test.ts | 29 ++++++++++++++++ 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/sdks/node/src/resources/runtime.ts b/sdks/node/src/resources/runtime.ts index e8e45ca3..da166321 100644 --- a/sdks/node/src/resources/runtime.ts +++ b/sdks/node/src/resources/runtime.ts @@ -1,4 +1,9 @@ -import { ResourceNotFoundError, ResourceScopeError, ResourceUnavailableError } from "../errors"; +import { + ResourceError, + ResourceNotFoundError, + ResourceScopeError, + ResourceUnavailableError, +} from "../errors"; import { createLogger } from "../utils"; import { HealthChecker } from "./health"; import { ResourcePool } from "./pool"; @@ -9,9 +14,13 @@ import type { ResourceResolver, ResourceScope, } from "./types"; +import { RESOURCE_SCOPES } from "./types"; const log = createLogger("resources"); +/** Shared empty ancestry for a top-level resolve. */ +const EMPTY_CHAIN: ReadonlySet = new Set(); + /** A disposal thunk plus the resource name, for error context. */ interface Teardown { name: string; @@ -58,6 +67,11 @@ export class ResourceRuntime { /** Register (or replace) a resource definition. */ register(name: string, definition: ResourceDefinition): void { + if (!(RESOURCE_SCOPES as readonly string[]).includes(definition.scope)) { + throw new ResourceError( + `resource "${name}": unknown scope "${definition.scope}" — expected one of ${RESOURCE_SCOPES.join(", ")}`, + ); + } this.defs.set(name, definition as ResourceDefinition); // A replacement definition starts with a clean bill of health. this.unhealthy.delete(name); @@ -177,7 +191,10 @@ export class ResourceRuntime { const taskCache = new Map>(); const taskTeardown: Teardown[] = []; - const resolve: ResourceResolver = (name) => { + const resolve = ( + name: string, + building: ReadonlySet = EMPTY_CHAIN, + ): Promise => { const def = this.defs.get(name); if (!def) { return Promise.reject(unregistered(name)); @@ -215,9 +232,20 @@ export class ResourceRuntime { if (def.scope === "request") { // Fresh on every resolve, never cached: N `useResource()` calls inside one // task yield N instances, each disposed with the task (LIFO, like the rest). + // Being uncached is also why a cycle cannot resolve itself the way a task + // resource does — there is no pending promise to hand back, so track the + // chain and reject instead of recursing forever. + if (building.has(name)) { + return Promise.reject( + new ResourceError( + `resource "${name}": request-scope dependency cycle (${[...building, name].join(" -> ")})`, + ), + ); + } + const chain = new Set(building).add(name); const ctx: ResourceContext = { scope: "request", - use: (dep: string) => resolve(dep) as Promise, + use: (dep: string) => resolve(dep, chain) as Promise, }; const counter = this.counter(name); counter.created += 1; @@ -236,7 +264,7 @@ export class ResourceRuntime { } const ctx: ResourceContext = { scope: "task", - use: (dep: string) => resolve(dep) as Promise, + use: (dep: string) => resolve(dep, building) as Promise, }; const counter = this.counter(name); counter.created += 1; @@ -254,7 +282,8 @@ export class ResourceRuntime { return built; }; - return { resolver: resolve, teardown: () => runTeardown(taskTeardown) }; + const resolver: ResourceResolver = (name) => resolve(name); + return { resolver, teardown: () => runTeardown(taskTeardown) }; } /** Register a worker that shares this runtime's worker-scoped resources. */ diff --git a/sdks/node/test/resources/resources.test.ts b/sdks/node/test/resources/resources.test.ts index 81382b08..b9a7d974 100644 --- a/sdks/node/test/resources/resources.test.ts +++ b/sdks/node/test/resources/resources.test.ts @@ -11,6 +11,7 @@ import { } from "../../src/index"; import { ResourcePool } from "../../src/resources/pool"; import { ResourceRuntime } from "../../src/resources/runtime"; +import type { ResourceScope } from "../../src/resources/types"; let worker: Worker | undefined; @@ -316,6 +317,34 @@ describe("request-scoped resources in the runtime", () => { }); }); +describe("resource scope validation", () => { + it("rejects an unknown scope at registration", () => { + const rt = new ResourceRuntime(); + // TypeScript stops this at compile time; a plain-JS caller would otherwise + // land in the task branch and get different lifecycle semantics silently. + expect(() => + rt.register("bad", { scope: "per-request" as ResourceScope, factory: () => 1 }), + ).toThrow(/unknown scope/); + }); + + it("rejects a direct request-scope dependency cycle", async () => { + const rt = new ResourceRuntime(); + rt.register("a", { scope: "request", factory: (ctx) => ctx.use("a") }); + const scope = rt.createTaskScope(); + await expect(scope.resolver("a")).rejects.toThrow(/dependency cycle/); + await scope.teardown(); + }); + + it("rejects an indirect request-scope cycle through a task resource", async () => { + const rt = new ResourceRuntime(); + rt.register("a", { scope: "request", factory: (ctx) => ctx.use("b") }); + rt.register("b", { scope: "task", factory: (ctx) => ctx.use("a") }); + const scope = rt.createTaskScope(); + await expect(scope.resolver("a")).rejects.toThrow(/dependency cycle/); + await scope.teardown(); + }); +}); + describe("pooled resources in the runtime", () => { it("checks out one pooled instance per task and returns it on teardown", async () => { const rt = new ResourceRuntime(); From c2b201df3050e78fd39d7357f2e2588dceee8932 Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:45:05 +0530 Subject: [PATCH 16/22] fix(node): let the log-level filter take any string Typing it to the union broke existing callers and forced the dashboard to cast; an unknown filter must return nothing, not fail. --- sdks/node/src/dashboard/handlers/core.ts | 7 +++---- sdks/node/src/queue.ts | 3 ++- sdks/node/src/types.ts | 7 +++++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/sdks/node/src/dashboard/handlers/core.ts b/sdks/node/src/dashboard/handlers/core.ts index 09928c11..fa96e63d 100644 --- a/sdks/node/src/dashboard/handlers/core.ts +++ b/sdks/node/src/dashboard/handlers/core.ts @@ -4,7 +4,6 @@ import { randomBytes } from "node:crypto"; import type { EventName } from "../../events"; import type { Queue } from "../../index"; -import type { TaskLogLevel } from "../../types"; import type { WebhookInput } from "../../webhooks"; import type { WorkflowNode } from "../../workflows"; import { BadRequestError } from "../errors"; @@ -298,9 +297,9 @@ export async function logs(queue: Queue, url: URL) { const since = positiveOr(url.searchParams.get("since"), 3600); const found = await queue.queryLogs({ task: url.searchParams.get("task") ?? undefined, - // Cast rather than validate: an unknown ?level= is untrusted input that must - // filter to nothing, which is exactly what the native filter already does. - level: (url.searchParams.get("level") ?? undefined) as TaskLogLevel | undefined, + // Unvalidated on purpose: an unknown ?level= must filter to nothing, which is + // exactly what the native filter does. + level: url.searchParams.get("level") ?? undefined, sinceMs: Date.now() - since * 1000, limit: num(url, "limit") ?? 100, }); diff --git a/sdks/node/src/queue.ts b/sdks/node/src/queue.ts index 4d14fafb..90066842 100644 --- a/sdks/node/src/queue.ts +++ b/sdks/node/src/queue.ts @@ -81,6 +81,7 @@ import type { Subscription, TaskLog, TaskLogLevel, + TaskLogLevelFilter, TaskMap, TaskOptions, TopicLogStat, @@ -1024,7 +1025,7 @@ export class Queue { * `sinceMs` is a Unix-ms lower bound (default: the last hour). */ queryLogs( - options: { task?: string; level?: TaskLogLevel; sinceMs?: number; limit?: number } = {}, + options: { task?: string; level?: TaskLogLevelFilter; sinceMs?: number; limit?: number } = {}, ): Promise { const sinceMs = options.sinceMs ?? Date.now() - 3_600_000; return this.native.queryTaskLogs(options.task, options.level, sinceMs, options.limit ?? 100); diff --git a/sdks/node/src/types.ts b/sdks/node/src/types.ts index a449391c..87ef9090 100644 --- a/sdks/node/src/types.ts +++ b/sdks/node/src/types.ts @@ -45,6 +45,13 @@ export const DISPATCH_ORDERS: readonly DispatchOrder[] = ["fifo", "lifo"]; */ export type TaskLogLevel = "debug" | "info" | "warning" | "error" | "critical" | "result"; +/** + * A task-log level as a *filter*: any string is accepted, because an unrecognized + * value must filter to nothing rather than fail — filters usually arrive from a + * dashboard query string. The union still autocompletes the known levels. + */ +export type TaskLogLevelFilter = TaskLogLevel | (string & {}); + /** Every task-log level, for runtime validation. */ export const TASK_LOG_LEVELS: readonly TaskLogLevel[] = [ "debug", From 77b87626dcccf01a14ff295306e89cecec83fae5 Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:45:17 +0530 Subject: [PATCH 17/22] fix(python): coerce a gate action and guard a corrupt session blob 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. --- .../docs/python/api-reference/queue/resources.mdx | 2 +- sdks/python/taskito/__init__.py | 3 ++- sdks/python/taskito/dashboard/auth.py | 2 ++ sdks/python/taskito/mixins/decorators.py | 3 ++- sdks/python/taskito/workflows/builder.py | 8 +++++++- sdks/python/taskito/workflows/tracker/gates.py | 7 ++++--- sdks/python/tests/python/test_closed_set_enums.py | 12 ++++++++---- 7 files changed, 26 insertions(+), 11 deletions(-) diff --git a/docs/content/docs/python/api-reference/queue/resources.mdx b/docs/content/docs/python/api-reference/queue/resources.mdx index 34763393..0ad68b8a 100644 --- a/docs/content/docs/python/api-reference/queue/resources.mdx +++ b/docs/content/docs/python/api-reference/queue/resources.mdx @@ -39,7 +39,7 @@ Decorator to register a resource factory initialized at worker startup. | `health_check_interval` | `float` | `0.0` | Seconds between health checks (0 = disabled). | | `max_recreation_attempts` | `int` | `3` | Max times to recreate on health failure. | | `scope` | `ResourceScope \| str` | `WORKER` | Lifetime: `WORKER`, `THREAD`, `TASK` (fresh per task), or `POOLED` (pool checkout). | -| `pool_size` | `int \| None` | `None` | Pool capacity; `POOLED` scope only (default = worker thread count). | +| `pool_size` | `int \| None` | `None` | Pool capacity; `POOLED` scope only (defaults to 4). | | `pool_min` | `int` | `0` | Pre-warmed instances; `POOLED` scope only. | | `acquire_timeout` | `float` | `10.0` | Max seconds to wait for a pool instance. | | `max_lifetime` | `float` | `3600.0` | Max seconds a pooled instance can live. | diff --git a/sdks/python/taskito/__init__.py b/sdks/python/taskito/__init__.py index 622d4140..9d89e8d0 100644 --- a/sdks/python/taskito/__init__.py +++ b/sdks/python/taskito/__init__.py @@ -46,7 +46,7 @@ from taskito.mesh import MeshWorker from taskito.middleware import TaskMiddleware from taskito.mixins.periodic import PeriodicInfo -from taskito.mixins.pubsub import TopicMessage +from taskito.mixins.pubsub import ConsumerErrorAction, TopicMessage from taskito.notes import MAX_NOTE_FIELDS from taskito.predicates.outcomes import PredicateAction from taskito.proxies.built_in import BuiltInProxy @@ -78,6 +78,7 @@ "CircularDependencyError", "CloudpickleSerializer", "CodecSerializer", + "ConsumerErrorAction", "CryptoError", "EffectiveRetention", "EncryptedSerializer", diff --git a/sdks/python/taskito/dashboard/auth.py b/sdks/python/taskito/dashboard/auth.py index 387179d7..f9fdd6e9 100644 --- a/sdks/python/taskito/dashboard/auth.py +++ b/sdks/python/taskito/dashboard/auth.py @@ -398,6 +398,8 @@ def get_session(self, token: str) -> Session | None: data = json.loads(raw) except json.JSONDecodeError: return None + if not isinstance(data, dict): + return None try: session = Session(token=token, **{**data, "role": _role_or_viewer(data.get("role"))}) except TypeError: diff --git a/sdks/python/taskito/mixins/decorators.py b/sdks/python/taskito/mixins/decorators.py index a553c9c0..39af6ad5 100644 --- a/sdks/python/taskito/mixins/decorators.py +++ b/sdks/python/taskito/mixins/decorators.py @@ -301,7 +301,8 @@ def task( ``.apply_async(args=..., kwargs=..., ...)`` for enqueueing. Raises: - ValueError: ``on_false`` is not a :class:`PredicateAction` value, or + ValueError: ``on_false`` is neither a :class:`PredicateAction` nor one + of its wire strings (``"defer"``/``"cancel"``), or ``default_defer_seconds`` is negative. """ on_false_action = coerce_enum(PredicateAction, on_false, param="on_false") diff --git a/sdks/python/taskito/workflows/builder.py b/sdks/python/taskito/workflows/builder.py index 93f0a834..12889b49 100644 --- a/sdks/python/taskito/workflows/builder.py +++ b/sdks/python/taskito/workflows/builder.py @@ -45,11 +45,17 @@ class GateConfig: """Seconds until auto-resolve. ``None`` waits indefinitely.""" on_timeout: GateAction = GateAction.REJECT - """Action on timeout.""" + """Action on timeout. A wire string is accepted and coerced.""" message: str | Callable | None = None """Human-readable message shown to approvers.""" + def __post_init__(self) -> None: + # The timeout handler dispatches on enum identity, and this dataclass is + # public: a caller passing the pre-enum "approve" string would otherwise + # have the gate silently resolve as a rejection. + self.on_timeout = coerce_enum(GateAction, self.on_timeout, param="on_timeout") + class _InheritCompensator: """Sentinel — use the compensator declared on the task decorator.""" diff --git a/sdks/python/taskito/workflows/tracker/gates.py b/sdks/python/taskito/workflows/tracker/gates.py index 2bce9a26..e66bd14d 100644 --- a/sdks/python/taskito/workflows/tracker/gates.py +++ b/sdks/python/taskito/workflows/tracker/gates.py @@ -6,6 +6,7 @@ import threading from typing import TYPE_CHECKING +from taskito.enums import coerce_enum from taskito.events import EventType from taskito.workflows.types import GateAction @@ -52,15 +53,15 @@ def enter_gate(tracker: WorkflowTracker, run_id: str, node_name: str, config: _R def on_gate_timeout( - tracker: WorkflowTracker, run_id: str, node_name: str, action: GateAction + tracker: WorkflowTracker, run_id: str, node_name: str, action: GateAction | str ) -> None: - """Handle gate timeout expiry.""" + """Handle gate timeout expiry. Coerces in case a caller stored a wire string.""" with tracker._state_lock: # If the run was cleaned up (e.g., cancelled before timeout fired), # the timer entry was already removed by `_cleanup_run` — stop. if (run_id, node_name) not in tracker._gate_timers: return tracker._gate_timers.pop((run_id, node_name), None) - approved = action is GateAction.APPROVE + approved = coerce_enum(GateAction, action, param="on_timeout") is GateAction.APPROVE error = None if approved else "gate timeout" tracker.resolve_gate(run_id, node_name, approved=approved, error=error) diff --git a/sdks/python/tests/python/test_closed_set_enums.py b/sdks/python/tests/python/test_closed_set_enums.py index f39bafd9..913b83fe 100644 --- a/sdks/python/tests/python/test_closed_set_enums.py +++ b/sdks/python/tests/python/test_closed_set_enums.py @@ -27,10 +27,14 @@ def test_wire_values_are_the_cross_sdk_contract() -> None: def test_interception_accepts_enum_and_string(tmp_path: Path) -> None: enum_queue = Queue(db_path=str(tmp_path / "a.db"), interception=InterceptionMode.LENIENT) string_queue = Queue(db_path=str(tmp_path / "b.db"), interception="lenient") - assert enum_queue._interceptor is not None - assert enum_queue._interceptor.mode is InterceptionMode.LENIENT - assert string_queue._interceptor is not None - assert string_queue._interceptor.mode is InterceptionMode.LENIENT + try: + assert enum_queue._interceptor is not None + assert enum_queue._interceptor.mode is InterceptionMode.LENIENT + assert string_queue._interceptor is not None + assert string_queue._interceptor.mode is InterceptionMode.LENIENT + finally: + enum_queue.close() + string_queue.close() def test_interception_typo_raises_naming_the_valid_set(tmp_path: Path) -> None: From e2c4e51066195d265339927c8737356c4f97108b Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 16:29:00 +0530 Subject: [PATCH 18/22] fix: reject an unknown worker status at the binding boundary Coercing caller input to Active would silently un-drain a worker; the lenient reader stays for persisted rows. --- crates/taskito-core/src/storage/records.rs | 12 +++++++++++- crates/taskito-python/src/py_queue/worker.rs | 11 ++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/crates/taskito-core/src/storage/records.rs b/crates/taskito-core/src/storage/records.rs index 9ee9e101..ef12b7fc 100644 --- a/crates/taskito-core/src/storage/records.rs +++ b/crates/taskito-core/src/storage/records.rs @@ -174,13 +174,23 @@ impl WorkerStatus { } /// Parse a persisted `status`; anything unrecognized reads as active, the - /// value every backend writes at registration. + /// value every backend writes at registration. Use [`Self::parse`] for caller + /// input, where a typo must not pass silently. pub fn from_wire(wire: &str) -> Self { match wire { "draining" => WorkerStatus::Draining, _ => WorkerStatus::Active, } } + + /// Strictly parse caller-supplied input; `None` for anything outside the set. + pub fn parse(wire: &str) -> Option { + match wire { + "active" => Some(WorkerStatus::Active), + "draining" => Some(WorkerStatus::Draining), + _ => None, + } + } } impl std::fmt::Display for WorkerStatus { diff --git a/crates/taskito-python/src/py_queue/worker.rs b/crates/taskito-python/src/py_queue/worker.rs index da446faa..44f374a9 100644 --- a/crates/taskito-python/src/py_queue/worker.rs +++ b/crates/taskito-python/src/py_queue/worker.rs @@ -834,11 +834,16 @@ impl PyQueue { )) } - /// Update the status of a worker. An unrecognized status reads as active, - /// matching what every backend writes at registration. + /// Update the status of a worker. An unrecognized status is a caller error — + /// coercing it would silently un-drain a worker. pub fn set_worker_status(&self, worker_id: &str, status: &str) -> PyResult<()> { + let parsed = WorkerStatus::parse(status).ok_or_else(|| { + pyo3::exceptions::PyValueError::new_err(format!( + "unknown worker status '{status}' (expected 'active' or 'draining')" + )) + })?; self.storage - .update_worker_status(worker_id, WorkerStatus::from_wire(status)) + .update_worker_status(worker_id, parsed) .map_err(|e| pyo3::exceptions::PyRuntimeError::new_err(e.to_string())) } } From e3263fa05fd6c720d1222d5bec0669f9a7d819d8 Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 16:29:05 +0530 Subject: [PATCH 19/22] fix(node): catch task-scope dependency cycles too A task resource in a cycle handed back its own pending promise, so the build awaited itself forever instead of failing. --- sdks/node/src/resources/runtime.ts | 17 +++++++---- sdks/node/test/resources/resources.test.ts | 33 ++++++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/sdks/node/src/resources/runtime.ts b/sdks/node/src/resources/runtime.ts index da166321..8b8d274f 100644 --- a/sdks/node/src/resources/runtime.ts +++ b/sdks/node/src/resources/runtime.ts @@ -232,13 +232,10 @@ export class ResourceRuntime { if (def.scope === "request") { // Fresh on every resolve, never cached: N `useResource()` calls inside one // task yield N instances, each disposed with the task (LIFO, like the rest). - // Being uncached is also why a cycle cannot resolve itself the way a task - // resource does — there is no pending promise to hand back, so track the - // chain and reject instead of recursing forever. if (building.has(name)) { return Promise.reject( new ResourceError( - `resource "${name}": request-scope dependency cycle (${[...building, name].join(" -> ")})`, + `resource "${name}": dependency cycle (${[...building, name].join(" -> ")})`, ), ); } @@ -258,13 +255,23 @@ export class ResourceRuntime { this.trackResource(taskTeardown, name, def, built); return built; } + // A name already in the chain is a cycle: returning its still-pending promise + // would deadlock the build on itself rather than recurse, so reject instead. + if (building.has(name)) { + return Promise.reject( + new ResourceError( + `resource "${name}": dependency cycle (${[...building, name].join(" -> ")})`, + ), + ); + } const cached = taskCache.get(name); if (cached) { return cached; } + const taskChain = new Set(building).add(name); const ctx: ResourceContext = { scope: "task", - use: (dep: string) => resolve(dep, building) as Promise, + use: (dep: string) => resolve(dep, taskChain) as Promise, }; const counter = this.counter(name); counter.created += 1; diff --git a/sdks/node/test/resources/resources.test.ts b/sdks/node/test/resources/resources.test.ts index b9a7d974..d6faaa4e 100644 --- a/sdks/node/test/resources/resources.test.ts +++ b/sdks/node/test/resources/resources.test.ts @@ -343,6 +343,39 @@ describe("resource scope validation", () => { await expect(scope.resolver("a")).rejects.toThrow(/dependency cycle/); await scope.teardown(); }); + + it("rejects the reverse mixed-scope cycle, task through request", async () => { + // The task branch would otherwise hand back its own pending promise and the + // build would await itself forever. + const rt = new ResourceRuntime(); + rt.register("a", { scope: "task", factory: (ctx) => ctx.use("b") }); + rt.register("b", { scope: "request", factory: (ctx) => ctx.use("a") }); + const scope = rt.createTaskScope(); + await expect(scope.resolver("a")).rejects.toThrow(/dependency cycle/); + await scope.teardown(); + }); + + it("rejects a self-referential task resource", async () => { + const rt = new ResourceRuntime(); + rt.register("a", { scope: "task", factory: (ctx) => ctx.use("a") }); + const scope = rt.createTaskScope(); + await expect(scope.resolver("a")).rejects.toThrow(/dependency cycle/); + await scope.teardown(); + }); + + it("still shares one task instance across sibling dependents", async () => { + // A diamond is not a cycle: the chain is per-path, so the cache still serves + // the second dependent. + const rt = new ResourceRuntime(); + let builds = 0; + rt.register("shared", { scope: "task", factory: () => ({ n: ++builds }) }); + rt.register("x", { scope: "task", factory: (ctx) => ctx.use("shared") }); + rt.register("y", { scope: "task", factory: (ctx) => ctx.use("shared") }); + const scope = rt.createTaskScope(); + expect(await scope.resolver("x")).toBe(await scope.resolver("y")); + expect(builds).toBe(1); + await scope.teardown(); + }); }); describe("pooled resources in the runtime", () => { From 35ffb69472ff8ad063421ee915bd4b9e20bd3020 Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 16:29:08 +0530 Subject: [PATCH 20/22] test(python): tear down the runtimes the scope tests build --- .../resources/test_resource_system_full.py | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/sdks/python/tests/resources/test_resource_system_full.py b/sdks/python/tests/resources/test_resource_system_full.py index cb61e311..11a2bd33 100644 --- a/sdks/python/tests/resources/test_resource_system_full.py +++ b/sdks/python/tests/resources/test_resource_system_full.py @@ -709,18 +709,21 @@ def factory() -> dict: ) runtime = ResourceRuntime({"conn": defn}) runtime.initialize() - first, release = runtime.acquire_for_task("conn") - assert release is not None - release() - - assert runtime.reload() == {"conn": True} - second, release2 = runtime.acquire_for_task("conn") - assert release2 is not None - release2() - # A rebuilt pool hands out a new instance, and nothing leaked into the - # singleton map that resolve() would have returned instead. - assert second is not first - assert "conn" not in runtime._instances + try: + first, release = runtime.acquire_for_task("conn") + assert release is not None + release() + + assert runtime.reload() == {"conn": True} + second, release2 = runtime.acquire_for_task("conn") + assert release2 is not None + release2() + # A rebuilt pool hands out a new instance, and nothing leaked into the + # singleton map that resolve() would have returned instead. + assert second is not first + assert "conn" not in runtime._instances + finally: + runtime.teardown() def test_async_task_teardown_is_awaited(self) -> None: """A returned coroutine used to be dropped, so async teardowns never ran.""" @@ -734,7 +737,10 @@ async def teardown(instance: str) -> None: ) runtime = ResourceRuntime({"ctx": defn}) runtime.initialize() - _, release = runtime.acquire_for_task("ctx") - assert release is not None - release() - assert torn == ["value"] + try: + _, release = runtime.acquire_for_task("ctx") + assert release is not None + release() + assert torn == ["value"] + finally: + runtime.teardown() From d5b86fbc1070f0450f02355dce90cc952a3adf13 Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 16:41:32 +0530 Subject: [PATCH 21/22] fix(java): keep the wire-form role on the dashboard models MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../taskito/dashboard/auth/AuthStore.java | 8 ++--- .../taskito/dashboard/auth/Policy.java | 2 +- .../taskito/dashboard/auth/Session.java | 2 +- .../byteveda/taskito/dashboard/auth/User.java | 2 +- .../taskito/dashboard/auth/AuthStoreTest.java | 32 +++++++++++++++++-- .../dashboard/oauth/OAuthFlowTest.java | 2 +- 6 files changed, 37 insertions(+), 11 deletions(-) diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java index 3e704dd1..ad09f35f 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/AuthStore.java @@ -85,7 +85,7 @@ public synchronized User createUser(String username, String password, Role role) row.put("last_login_at", null); users.put(username, row); saveUsers(users); - return new User(username, hash, role, now, null, null, null); + return new User(username, hash, role.wire(), now, null, null, null); } /** Verify credentials; {@code null} on any failure. Updates last-login on success. */ @@ -233,7 +233,7 @@ public Session createSession(String username, Role role, long ttlSeconds) { row.put("expires_at", expires); row.put("csrf_token", csrf); settings.setSetting(SESSION_PREFIX + token, Json.toString(row)); - return new Session(token, username, role, now, expires, csrf); + return new Session(token, username, role.wire(), now, expires, csrf); } /** Resolve a session token; deletes and returns empty if expired/malformed. */ @@ -254,7 +254,7 @@ public Optional getSession(String token) { session = new Session( token, (String) data.get("username"), - Role.orViewer((String) data.get("role")), + Role.orViewer((String) data.get("role")).wire(), asLong(data.get("created_at")), asLong(data.get("expires_at")), (String) data.get("csrf_token")); @@ -354,7 +354,7 @@ private static User toUser(String username, Map row) { return new User( username, (String) row.get("password_hash"), - Role.orViewer((String) row.get("role")), + Role.orViewer((String) row.get("role")).wire(), asLong(row.get("created_at")), row.get("last_login_at") == null ? null : asLong(row.get("last_login_at")), (String) row.get("email"), diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java index 133c38d2..2b023ba9 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Policy.java @@ -64,7 +64,7 @@ public static void authorize(String path, String method, RequestContext ctx, Aut if (isStateChanging(method) && !isCsrfExempt(path) && !ctx.csrfValid()) { throw DashboardError.forbidden("csrf_failed"); } - if (requiresAdmin(path, method) && ctx.session().role() != Role.ADMIN) { + if (requiresAdmin(path, method) && Role.orViewer(ctx.session().role()) != Role.ADMIN) { throw DashboardError.forbidden("forbidden"); } } diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Session.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Session.java index 889ed04a..320d78d3 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Session.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/Session.java @@ -6,7 +6,7 @@ * this matches the reference wire contract exactly. The {@code token} is the KV * key suffix and is never serialised into the stored record. */ -public record Session(String token, String username, Role role, long createdAt, long expiresAt, String csrfToken) { +public record Session(String token, String username, String role, long createdAt, long expiresAt, String csrfToken) { public boolean isExpired(long nowSeconds) { return nowSeconds >= expiresAt; diff --git a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/User.java b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/User.java index 59693e27..0d61ebbb 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/User.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/dashboard/auth/User.java @@ -9,7 +9,7 @@ public record User( String username, String passwordHash, - Role role, + String role, long createdAt, Long lastLoginAt, String email, diff --git a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java index 16a6c298..69bd7bb6 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/auth/AuthStoreTest.java @@ -19,13 +19,39 @@ private AuthStore store() { return new AuthStore(new InMemorySettings()); } + @Test + void modelsExposeTheWireFormSoExistingCallersStillCompile() { + // A Java enum is not a String, so typing these accessors would break every + // `String role = session.role()` caller. The enum owns validation and the + // decisions; the models carry the wire form the other SDKs also expose. + AuthStore store = store(); + User user = store.createUser("alice", "password123", Role.ADMIN); + String userRole = user.role(); + String sessionRole = store.createSession("alice", Role.ADMIN).role(); + assertEquals("admin", userRole); + assertEquals("admin", sessionRole); + assertEquals(Role.ADMIN, Role.fromWire(userRole)); + } + + @Test + void storedRoleNothingRecognizesReadsBackAsViewer() { + InMemorySettings settings = new InMemorySettings(); + AuthStore store = new AuthStore(settings); + store.createUser("alice", "password123", Role.ADMIN); + String users = settings.getSetting(AuthStore.USERS_KEY).orElseThrow(); + settings.setSetting(AuthStore.USERS_KEY, users.replace("\"admin\"", "\"superuser\"")); + + // Fails closed on read rather than handing back an unusable role. + assertEquals(Role.VIEWER.wire(), store.getUser("alice").orElseThrow().role()); + } + @Test void createsAndAuthenticatesUsers() { AuthStore store = store(); assertEquals(0, store.countUsers()); User user = store.createUser("alice", "password123", Role.ADMIN); assertEquals("alice", user.username()); - assertEquals(Role.ADMIN, user.role()); + assertEquals(Role.ADMIN.wire(), user.role()); assertNull(user.lastLoginAt()); assertEquals(1, store.countUsers()); @@ -104,12 +130,12 @@ void getOrCreateOauthUserProvisionsThenRefreshes() { AuthStore store = store(); User created = store.getOrCreateOauthUser("google", "123", "a@x.com", "Ann", true, List.of("a@x.com")); assertEquals("google:123", created.username()); - assertEquals(Role.ADMIN, created.role()); // allowlisted + assertEquals(Role.ADMIN.wire(), created.role()); // allowlisted assertTrue(created.isOauth()); User refreshed = store.getOrCreateOauthUser("google", "123", "new@x.com", "Ann N", true, List.of("z@x.com")); assertEquals("google:123", refreshed.username()); - assertEquals(Role.ADMIN, refreshed.role()); // role preserved + assertEquals(Role.ADMIN.wire(), refreshed.role()); // role preserved assertEquals("new@x.com", refreshed.email()); assertEquals(1, store.countUsers()); } diff --git a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/oauth/OAuthFlowTest.java b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/oauth/OAuthFlowTest.java index b891bd57..443072f0 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/dashboard/oauth/OAuthFlowTest.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/dashboard/oauth/OAuthFlowTest.java @@ -59,7 +59,7 @@ void handleCallbackLandsSessionAsViewerWithoutAllowlist() { String state = fake.lastState.state(); OAuthFlow.CallbackResult result = flow.handleCallback("fake", "code", state, null); assertEquals("fake:subject-1", result.session().username()); - assertEquals(Role.VIEWER, result.session().role()); // admin comes only from the allowlist + assertEquals(Role.VIEWER.wire(), result.session().role()); // admin comes only from the allowlist assertEquals("/next", result.nextUrl()); assertTrue(authStore.getUser("fake:subject-1").isPresent()); } From ecac25f89035afc9ed9233fd114307fcdb029594 Mon Sep 17 00:00:00 2001 From: kartikeya <67143288+kartikeya-27@users.noreply.github.com> Date: Thu, 23 Jul 2026 19:41:31 +0530 Subject: [PATCH 22/22] fix(java): reject a null enum on the typed overloads 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. --- .../main/java/org/byteveda/taskito/DefaultTaskito.java | 8 +++++++- .../test/java/org/byteveda/taskito/core/QueueTest.java | 10 ++++++++++ sdks/python/taskito/resources/runtime.py | 4 ++-- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java b/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java index 4180ae6d..ef5f7414 100644 --- a/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java +++ b/sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java @@ -221,6 +221,9 @@ public Taskito codel(String queue, long targetMs, long intervalMs) { @Override public Taskito dispatchOrder(String queue, DispatchOrder order) { + if (order == null) { + throw new IllegalArgumentException("order must not be null"); + } dispatchOrders.put(queue, order.wire()); return this; } @@ -712,11 +715,14 @@ public Map listSettings() { @Override public void writeTaskLog(String jobId, String taskName, TaskLogLevel level, String message) { - backend.writeTaskLog(jobId, taskName, level.wire(), message, null); + writeTaskLog(jobId, taskName, level, message, null); } @Override public void writeTaskLog(String jobId, String taskName, TaskLogLevel level, String message, String extra) { + if (level == null) { + throw new IllegalArgumentException("level must not be null"); + } backend.writeTaskLog(jobId, taskName, level.wire(), message, extra); } diff --git a/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java b/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java index 03b6d398..70188113 100644 --- a/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java +++ b/sdks/java/src/test/java/org/byteveda/taskito/core/QueueTest.java @@ -15,6 +15,7 @@ import org.byteveda.taskito.Queue; import org.byteveda.taskito.Taskito; import org.byteveda.taskito.TaskitoException; +import org.byteveda.taskito.model.DispatchOrder; import org.byteveda.taskito.model.Job; import org.byteveda.taskito.model.JobFilter; import org.byteveda.taskito.model.JobStatus; @@ -181,6 +182,15 @@ void logFiltersTakeAWireStringAndStayLenient(@TempDir Path dir) { } } + @Test + void typedOverloadsRejectANullEnum(@TempDir Path dir) { + // Without the guard these dereference the enum and surface an incidental NPE. + try (Taskito queue = open(dir)) { + assertThrows(IllegalArgumentException.class, () -> queue.dispatchOrder("default", (DispatchOrder) null)); + assertThrows(IllegalArgumentException.class, () -> queue.writeTaskLog("j", "t", (TaskLogLevel) null, "m")); + } + } + @Test void unknownWorkflowStateFilterIsRejectedByTheCore(@TempDir Path dir) { // Unlike the log level, the state filter is validated in the core, so an diff --git a/sdks/python/taskito/resources/runtime.py b/sdks/python/taskito/resources/runtime.py index 187e73c2..7a2c3f73 100644 --- a/sdks/python/taskito/resources/runtime.py +++ b/sdks/python/taskito/resources/runtime.py @@ -27,7 +27,7 @@ class ResourceRuntime: Worker-scoped resources are initialized eagerly in dependency order. Pooled resources get a checkout pool. Thread-scoped resources use thread-local storage. - Request-scoped resources are created fresh per task. + Task-scoped resources are created fresh per task. """ def __init__(self, definitions: dict[str, ResourceDefinition]) -> None: @@ -110,7 +110,7 @@ def _create_resource(self, name: str) -> None: def resolve(self, name: str) -> Any: """Return a live resource instance by name (worker scope only). - For task/request scopes, use acquire_for_task() instead. + For task- and pooled-scoped resources, use acquire_for_task() instead. Raises: ResourceNotFoundError: If the name was never registered.