feat: URL-based stack bootstrapping and example Taskfile migration#15
feat: URL-based stack bootstrapping and example Taskfile migration#15kingwill101 merged 4 commits intomasterfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (36)
📝 WalkthroughWalkthroughThe PR adds URL-based bootstrap factories (fromUrl) for StemApp/StemClient/StemWorkflowApp, enhances StemStack adapter/error handling, renames memory result backend exports, adds tests for the new bootstrap flows, and migrates ~30 example projects from Justfile to Taskfile with supporting Taskfile orchestration and docs updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Caller
participant App as StemApp.fromUrl()
participant Stack as StemStack.fromUrl()
participant Adapter as AdapterRegistry
participant Factory as StoreFactory
participant Store as Broker/Backend/Lock/Revoke
Client->>App: call fromUrl(url, adapters, ...)
activate App
App->>Stack: StemStack.fromUrl(url, adapters, overrides)
activate Stack
Stack->>Adapter: resolve adapter for scheme
Adapter-->>Stack: provide broker/backend/lock/revoke factories
Stack->>Factory: invoke selected factory(...)
Factory-->>Store: return store instance
alt uniqueTasks enabled
Stack->>Factory: create lock store
Factory-->>Store: lock store
end
alt requireRevokeStore true
Stack->>Factory: create revoke store
Factory-->>Store: revoke store
end
Stack-->>App: return resolved stores/adapters
App->>Store: initialize app with stores
deactivate Stack
App-->>Client: StemApp (ready)
deactivate App
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fa9cce857
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 31
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/stem/example/progress_heartbeat/README.md (1)
35-41:⚠️ Potential issue | 🟡 MinorAdd blank lines around the fenced block.
markdownlint MD031 indicates the fence isn’t surrounded by blank lines.✍️ Minimal formatting fix
-```bash + +```bash export STEM_BROKER_URL=redis://localhost:6379/0 export STEM_RESULT_BACKEND_URL=redis://localhost:6379/1 task build-cli task stem observe workers
</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@packages/stem/example/progress_heartbeat/README.mdaround lines 35 - 41, The
fenced bash code block (the triple backticks starting with "bash") in README.md is not surrounded by blank lines; add a single blank line before the openingbash fence and a single blank line after the closing ``` fence so the
block is separated from surrounding text, ensuring markdownlint MD031 is
satisfied.</details> </blockquote></details> <details> <summary>packages/stem_memory/lib/src/memory_factories.dart (1)</summary><blockquote> `23-39`: _🧹 Nitpick_ | _🔵 Trivial_ **Consider a deprecated alias to ease migration.** Renaming the public factory is a breaking change. A deprecated wrapper can preserve compatibility while guiding users to the new name. <details> <summary>♻️ Suggested compatibility alias</summary> ```diff +@Deprecated('Use memoryResultBackendFactory instead.') +StemBackendFactory memoryBackendFactory({ + Duration defaultTtl = const Duration(days: 1), + Duration groupDefaultTtl = const Duration(days: 1), + Duration heartbeatTtl = const Duration(minutes: 1), +}) { + return memoryResultBackendFactory( + defaultTtl: defaultTtl, + groupDefaultTtl: groupDefaultTtl, + heartbeatTtl: heartbeatTtl, + ); +} + /// Creates a [StemBackendFactory] backed by [InMemoryResultBackend]. StemBackendFactory memoryResultBackendFactory({ ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/stem_memory/lib/src/memory_factories.dart` around lines 23 - 39, Add a deprecated compatibility alias for the renamed factory so existing callers keep working: create a top-level function with the old public name (e.g., the previous factory identifier) annotated with `@Deprecated` and forwarding to memoryResultBackendFactory(...) (passing through defaultTtl, groupDefaultTtl, heartbeatTtl), and include a deprecation message pointing users to memoryResultBackendFactory; update any doc comment to mention the alias is deprecated and will be removed in a future release. ``` </details> </blockquote></details> <details> <summary>packages/stem/test/bootstrap/stem_stack_test.dart (1)</summary><blockquote> `277-323`: _🧹 Nitpick_ | _🔵 Trivial_ **`_TestAdapter.name` is hardcoded to `'test'` regardless of the `scheme` argument.** In multi-adapter tests (e.g. `'honors overrides for specific stores'`), two adapters with different schemes (`'foo'`, `'bar'`) both report `name == 'test'`. If `StemStack.fromUrl` ever uses `name` for adapter deduplication, conflict detection, or diagnostic messages, this silently produces misleading output. <details> <summary>♻️ Proposed fix</summary> ```diff class _TestAdapter implements StemStoreAdapter { _TestAdapter({ required this.scheme, ... }); final String scheme; ... `@override` - String get name => 'test'; + String get name => scheme; ... } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/stem/test/bootstrap/stem_stack_test.dart` around lines 277 - 323, The _TestAdapter.name getter is hardcoded to 'test' which hides the adapter's actual scheme; change the name implementation in _TestAdapter to return a value derived from the instance's scheme (e.g., return scheme or 'test-$scheme') so each adapter created with different scheme values (constructor field scheme) exposes a unique name; update the getter for name in the _TestAdapter class (symbol: _TestAdapter.name) to use the instance field scheme instead of the literal 'test'. ``` </details> </blockquote></details> <details> <summary>packages/stem/example/docs_snippets/lib/workflows.dart (1)</summary><blockquote> `234-253`: _🧹 Nitpick_ | _🔵 Trivial_ **`main()` still uses `StemWorkflowApp.inMemory()` — inconsistent with the rest of the file's `fromUrl` migration.** Every other call site in this file has been migrated to `fromUrl`-based constructors, but `main()` at line 242 still uses the `inMemory()` factory. For documentation consistency, update it to use `StemWorkflowApp.fromUrl('memory://', flows: [demoFlow])`. <details> <summary>♻️ Proposed update</summary> ```diff - final app = await StemWorkflowApp.inMemory(flows: [demoFlow]); + final app = await StemWorkflowApp.fromUrl('memory://', flows: [demoFlow]); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/stem/example/docs_snippets/lib/workflows.dart` around lines 234 - 253, The main() function still constructs the app with StemWorkflowApp.inMemory(); replace that call with the fromUrl factory so it matches other examples — use StemWorkflowApp.fromUrl('memory://', flows: [demoFlow]) when creating app in main(), keep all other logic (start, startWorkflow, waitForCompletion, close) unchanged and ensure the variable name (app) and awaited creation remain the same. ``` </details> </blockquote></details> <details> <summary>packages/stem/example/stack_autowire.dart (1)</summary><blockquote> `26-55`: _⚠️ Potential issue_ | _🟡 Minor_ **Remove the const concern; clarify the dual-stack pattern as potentially wasteful.** 1. **`const redisAdapters`** is not an issue — `StemRedisAdapter` has a valid `const` constructor. 2. **Dual-stack pattern** (lines 27–55): `StemStack.fromUrl` and `StemApp.fromUrl` both target the same URL. `StemApp.fromUrl` creates its own internal `StemStack` regardless of the external one. The stores extracted from the first stack (`scheduleStore`/`lockStore`) are then unused since `StemApp.fromUrl` with `uniqueTasks: true` and `requireRevokeStore: true` creates fresh stores from its internal stack. To reuse stores, pass them via `StemWorkerConfig` to `StemApp.fromUrl`, or remove the redundant manual stack creation. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/stem/example/stack_autowire.dart` around lines 26 - 55, The example unnecessarily creates a separate StemStack via StemStack.fromUrl and extracts scheduleFactory/lockFactory/scheduleStore/lockStore which are never used because StemApp.fromUrl creates its own internal StemStack when called with uniqueTasks and requireRevokeStore; either remove the redundant StemStack.fromUrl block and related scheduleFactory/lockFactory/scheduleStore/lockStore code, or instead pass the created stores into StemApp.fromUrl using a StemWorkerConfig so the app reuses the existing stores; note that const redisAdapters is fine because StemRedisAdapter has a const ctor. ``` </details> </blockquote></details> </blockquote></details>♻️ Duplicate comments (3)
packages/stem/example/ops_health_suite/Taskfile.yml (1)
30-60: Same binary glob concern as in progress_heartbeat Taskfile.
Consider resolving a single binary before execution to avoid ambiguous matches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stem/example/ops_health_suite/Taskfile.yml` around lines 30 - 60, The run-worker and run-producer tasks use a bare glob ("$root/build/worker/bundle/bin/"* and "$root/build/producer/bundle/bin/"*) which can match multiple files or none; change each task to resolve exactly one binary before exec (e.g., expand the glob into an array, validate there is exactly one match, error if zero or >1, then exec that single path) and update the run-worker and run-producer command blocks to use that resolved single binary path instead of the raw glob.packages/stem/example/dlq_sandbox/Taskfile.yml (1)
30-60: Same binary glob concern as in progress_heartbeat Taskfile.
Consider resolving a single binary before execution to avoid ambiguous matches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stem/example/dlq_sandbox/Taskfile.yml` around lines 30 - 60, The run-worker and run-producer Taskfile commands use a glob ("$root/build/worker/bundle/bin/"* and "$root/build/producer/bundle/bin/"*) which can match multiple files; change each task to resolve the glob before execution, validate that exactly one binary was found (or fail with a clear error), and then exec that single path (e.g., collect matches into an array, check length, and use the first/only entry) so the tasks deterministically run a single binary instead of an ambiguous glob.packages/stem/example/signals_demo/Taskfile.yml (1)
30-58: Same binary glob concern as in progress_heartbeat Taskfile.
Consider resolving a single binary before execution to avoid ambiguous matches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stem/example/signals_demo/Taskfile.yml` around lines 30 - 58, The run-worker and run-producer tasks currently execute a glob ("$root/build/worker/bundle/bin/"* and "$root/build/producer/bundle/bin/"*) which can match multiple files; change each task to resolve the glob into a single binary before exec: expand the glob into an array/variable (e.g., bins=( "$root/build/.../bin/"* )), validate that exactly one match exists (or pick a deterministic one) and fail with a clear error if ambiguous, then exec that single path (replace the direct glob invocation in both run-worker and run-producer).🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed. Inline comments: In `@packages/dashboard/CHANGELOG.md`: - Around line 1-3: Add a top-level H1 heading above the existing "## Unreleased" entry to satisfy markdownlint MD041; for example insert a single-line "# Changelog" (or "# stem_dashboard Changelog") before the "## Unreleased" heading so the file begins with an H1 and retains the existing "## Unreleased" section and package note (`stem_dashboard`). In `@packages/stem_adapter_tests/CHANGELOG.md`: - Line 1: The top-level heading in CHANGELOG.md is currently an H2 ("## Unreleased") which violates MD041; change that first line to an H1 by replacing "## Unreleased" with "# Unreleased" so the file starts with a top-level H1 (keep the rest of the file unchanged). In `@packages/stem_cli/CHANGELOG.md`: - Around line 1-3: The changelog currently starts with an H2 which triggers markdownlint MD041; add a top-level H1 heading (for example "Changelog" or "CHANGELOG") as the very first line of packages/stem_cli/CHANGELOG.md so the existing "## Unreleased" becomes a valid secondary heading and MD041 is resolved. In `@packages/stem_memory/CHANGELOG.md`: - Around line 1-6: Add a top-level H1 heading above the existing "## Unreleased" section in the changelog so the file begins with a single H1 (for example "# Changelog") to satisfy markdownlint MD041; edit the current "Unreleased" section header (the "## Unreleased" line) by inserting the H1 before it and keep the existing bullet entries and renamed notes intact. In `@packages/stem_sqlite/CHANGELOG.md`: - Line 1: Add a top-level H1 heading above the existing "## Unreleased" heading to satisfy markdownlint MD041; edit CHANGELOG.md and insert a line such as "# Changelog" (or "# Stem SQLite Changelog") immediately before the "## Unreleased" line so the file begins with an H1 instead of an H2. In `@packages/stem/example/autoscaling_demo/README.md`: - Around line 23-29: The README mentions a non-existent task "task tmux"; either remove that line from README.md or add a corresponding tmux task to the project's Taskfile. If you choose to implement it, create a Taskfile entry named "tmux" that launches the demo in a tmux session (or calls existing tasks like "run-worker" and "run-producer" inside tmux), ensuring the task name is exactly "tmux" so the README command works; otherwise delete the `task tmux` reference from README.md and keep the documented alternatives ("task run-worker" / "task run-producer"). In `@packages/stem/example/canvas_patterns/Taskfile.yml`: - Around line 12-49: The three tasks run-chain, run-chord, and run-group duplicate the same shell block; create a parameterised internal task (e.g., run-canvas-demo with a parameter like example_path) that contains the shared bash script and accepts the final dart file as an argument, then make run-chain, run-chord, and run-group call that internal task with example_path set to "example/canvas_patterns/chain_example.dart", "example/canvas_patterns/chord_example.dart", and "example/canvas_patterns/group_example.dart" respectively; also fix the .env resolution inside the shared script to prefer the Taskfile root path (use "$root/$env_file" first) to avoid CWD-relative ambiguity before falling back to a top-level .env. In `@packages/stem/example/docs_snippets/lib/workflows.dart`: - Around line 11-22: The docs snippet for StemWorkflowApp.fromUrl hardcodes DB credentials in StemStoreOverrides.workflow ('postgresql://postgres:postgres@127.0.0.1:65432/stem'); update it to remove literal credentials by using a placeholder URL (e.g., 'postgresql://<user>:<password>@127.0.0.1:65432/stem') or construct the workflow URL from environment variables before passing to StemStoreOverrides so no username/password appear inline in the example. In `@packages/stem/example/encrypted_payload/Taskfile.yml`: - Around line 89-165: The three tasks run:worker, run:enqueuer and run:container-client duplicate the same env-loading preamble and export lines (env_file loading, REDIS_PORT, STEM_BROKER_URL, STEM_RESULT_BACKEND_URL, STEM_DEFAULT_QUEUE, PAYLOAD_SECRET); extract that shared logic into a single reusable Taskfile task (e.g., dotenv or env:load) or a sourced shell snippet and make each of the three tasks invoke that precondition before running their distinct binary paths, updating run:worker, run:enqueuer and run:container-client to remove the duplicated block and depend on or source the centralized env setup. - Line 111: The example Taskfile.yml currently hardcodes a fallback PAYLOAD_SECRET (e.g. in the export PAYLOAD_SECRET line used by the run:worker path) which allows accidental use of a committed key; remove the hardcoded default (don’t use "${PAYLOAD_SECRET:-...}") from all occurrences (the three export PAYLOAD_SECRET lines) and add a fail-fast guard in the run:worker worker startup logic that checks PAYLOAD_SECRET and prints a clear error + non-zero exit if unset (so users must run the init task to generate a key); reference PAYLOAD_SECRET and the run:worker task when making the change. In `@packages/stem/example/microservice/README.md`: - Around line 41-42: Add a blank line after the closing fenced code block (the triple backticks) so the fence is followed by an empty line before the next paragraph that starts with "Replace the placeholder secret in `.env`..."; update the README.md fenced-block to ensure there's a single blank line between the closing ``` and the following paragraph to satisfy MD031. In `@packages/stem/example/microservice/Taskfile.yml`: - Around line 263-266: The clean task currently deletes the repo-root dashboard build via the path "{{.TASKFILE_DIR}}/../../../dashboard/build"; update the Taskfile's clean task to avoid removing files outside the example tree by either (A) removing that dashboard path from the clean task so it only deletes "{{.TASKFILE_DIR}}/worker/build", "enqueuer/build" and "beat/build", or (B) extract the dashboard deletion into a separate opt-in task (e.g., clean:dashboard) that explicitly targets "{{.TASKFILE_DIR}}/../../../dashboard/build" so users must run it intentionally; modify the task named clean (and add clean:dashboard if choosing option B) accordingly. - Around line 159-161: Update the Taskfile.yml "build" task description to clarify that it compiles worker, enqueuer, and beat binaries but intentionally excludes the dashboard (which is built in a separate package), or alternatively add build:dashboard as an optional dependency; reference the "build" task and the "build:dashboard" target so reviewers can decide whether to modify the description or include the optional dep. In `@packages/stem/example/monolith_service/Taskfile.yml`: - Around line 13-21: The env lookup currently sets env_file to ".env" which makes the first check [[ -f "$env_file" ]] use the caller's CWD; change the default to be rooted at the Taskfile directory so the lookup is unambiguous by setting env_file="${ENV_FILE:-${root}/.env}" (or otherwise constructing "$root/.env" when ENV_FILE is empty) and then source that absolute path (keep the existing fallback logic only if you want to still support an explicit ENV_FILE pointing elsewhere); update the references to env_file in the if/elif so they check the absolute $env_file and $root/$env_file cases consistently. In `@packages/stem/example/postgres_tls/Taskfile.yml`: - Around line 70-74: The POSTGRES credentials are hardcoded in the default STEM_RESULT_BACKEND_URL; introduce and use environment variables for the DB username and password (e.g., POSTGRES_USER and POSTGRES_PASSWORD) and update the STEM_RESULT_BACKEND_URL default to interpolate those variables instead of "postgres:postgres"; do the same change for the duplicate block around lines 96-100 and ensure STEM_RESULT_BACKEND_URL falls back to sensible, non-secret defaults only for local demo use (or empty) so credentials are not embedded in the repo. In `@packages/stem/example/progress_heartbeat/README.md`: - Around line 45-56: The README references non-existent Taskfile targets "build-cli" and "tmux"; update the example to use the actual Taskfile targets by replacing "build-cli" with the correct build tasks (either the aggregate "build" or the specific "build-worker" and "build-producer") and remove or replace "tmux" with the real way to run concurrent processes (e.g., run separate terminals with "task run-worker" and "task run-producer"); ensure the retained task names match "deps-up", "build" (or "build-worker"/"build-producer"), "run-worker", and "run-producer" so the instructions align with the Taskfile. In `@packages/stem/example/progress_heartbeat/Taskfile.yml`: - Around line 30-60: The tasks run-worker and run-producer use an ambiguous glob ("$root/build/.../bin/"*) which can expand to zero or many files; change each task to explicitly resolve the glob into a single binary variable (e.g., collect matches into an array, enable nullglob so empty expands to zero entries), verify the array length is exactly one and fail with a clear error if not, then exec that single path (use the variable instead of the raw glob) so the script errors deterministically when no or multiple binaries are found. In `@packages/stem/example/rate_limit_delay/Taskfile.yml`: - Around line 34-45: The glob "$root/build/worker/bundle/bin/"* is fragile because if the bundle directory contains multiple files the shell will treat extras as arguments; update the Taskfile.yml worker and producer steps to either call the explicit binary name produced by your build (replace the glob with the actual executable filename in "$root/build/worker/bundle/bin/") or robustly select a single executable (e.g., use a find invocation that filters for a single -type f -executable and -exec it) so only one file is executed and extras are not passed as arguments. In `@packages/stem/example/security/ed25519_tls/Taskfile.yml`: - Around line 30-33: The sed in-place edit uses the non-portable flag pattern sed -i -E which fails on BSD/macOS; update the code that builds escaped_value and edits env_file (the lines referencing escaped_value, env_file, key, value and the sed -i -E invocation) to use a portable in-place invocation (e.g. sed -i'' -E ...) or otherwise supply an explicit empty backup suffix so the same command works on both GNU and BSD sed; make this change consistently across the example Taskfiles mentioned (signing_key_rotation, security/hmac, security/hmac_tls, microservice, encrypted_payload). In `@packages/stem/example/security/hmac_tls/README.md`: - Around line 9-11: Update the incorrect directory path in the README commands from "examples/" to "example/": find the two command snippets that say "cd examples/security/hmac_tls" and "task tls:certs" and change the cd invocation to "cd example/security/hmac_tls" (leave the task invocation alone), and repeat the same fix for the second occurrence around lines 16-18 so all example path references use "example/" not "examples/". In `@packages/stem/example/security/hmac_tls/Taskfile.yml`: - Around line 52-56: compose-up currently only runs tls:certs and misses rotating HMAC keys; update the compose-up task to also invoke keys:rotate before bringing up containers so both TLS certs and signing keys are ensured fresh, i.e., add keys:rotate as a prerequisite alongside tls:certs for the compose-up task (aligning with init and init:hmac-tls behavior used elsewhere). In `@packages/stem/example/security/hmac/Taskfile.yml`: - Around line 17-24: The sed -i usage in the STEM_SIGNING_KEYS / STEM_SIGNING_ACTIVE_KEY update blocks is not portable to BSD/macOS; modify the Taskfile logic that edits env_file (the code touching STEM_SIGNING_KEYS and STEM_SIGNING_ACTIVE_KEY and using escaped_secret/secret) to use a portable approach such as using sed -i.bak -E (and remove the backup after) or add a runtime detection branch (check sed --version to choose between sed -i -E and sed -i '' -E) or replace with perl -pi -e; ensure the replacement commands update STEM_SIGNING_KEYS and STEM_SIGNING_ACTIVE_KEY exactly as before and that any temporary backup file is cleaned up. In `@packages/stem/example/signing_key_rotation/Taskfile.yml`: - Around line 25-37: The sed in-place edits using `sed -i -E` are not portable to macOS/BSD; update the places that modify STEM_SIGNING_KEYS and STEM_SIGNING_ACTIVE_KEY (and other similar occurrences like the other signing/hmac/encrypted_payload Taskfile.yml entries) to detect GNU vs BSD sed and call sed with the appropriate -i arg (either `-i -E` for GNU or `-i '' -E` for BSD/macOS) before running the substitution that uses ${escaped_keys} and ${active}; apply the same pattern to each of the ~10 instances across the Taskfile.yml files so all sed in-place edits are portable. In `@packages/stem/example/task_context_mixed/README.md`: - Line 74: The README references a non-existent task "task tmux" which will fail because the Taskfile.yml only defines build-worker, build-enqueue, build, run-worker, run-enqueue, run-fail, run-overwrite, run, and clean; either add a "tmux" task entry to the Taskfile.yml that implements the intended behavior (e.g., starts the worker/enqueue targets in tmux) or remove/replace the "task tmux" line in the README with one of the actual tasks (for example "task run" or a new "tmux" task name you add) so the example matches the Taskfile.yml. In `@packages/stem/example/task_context_mixed/Taskfile.yml`: - Around line 31-32: The shell glob "$root/build/worker/bundle/bin/"* is unsafe because it may expand to multiple files; replace usages (the glob pattern in Taskfile tasks like run-enqueue, run-fail, run-overwrite and the Taskfiles for encrypted_payload and microservice) with an explicit binary name when stable, or else discover a single executable reliably (e.g., use a find invocation restricted to that directory such as find "$root/build/worker/bundle/bin" -maxdepth 1 -type f -executable and select the intended file) and execute only that path instead of the bare glob. - Around line 39-91: The three Taskfile targets run-enqueue, run-fail, and run-overwrite duplicate the same env-loading and binary-detection logic; refactor by extracting the shared setup into a single reusable fragment or parameterize the differing CLI flag via a variable (e.g., CLI_ARGS) and have one target (or a helper include) reuse the common block and append CLI_ARGS (empty, "--fail", "--overwrite") as needed so the logic in run-enqueue/run-fail/run-overwrite is replaced with a single DRY invocation that references the shared helper or variable. In `@packages/stem/example/unique_tasks/Taskfile.yml`: - Around line 12-21: The Taskfile's env-file lookup currently uses env_file="${ENV_FILE:-.env}" and checks the CWD first, which can load an ancestor .env when task is run elsewhere; update the conditional so it prefers the repository-relative path by testing "$root/$env_file" before "$env_file" (i.e., check if [[ -f "$root/$env_file" ]] then source that, elif [[ -f "$env_file" ]] then source that), leaving the env_file, root and stem_root variables and the rest of the script (including cd "$stem_root" and dart run unique_task_example.dart) unchanged. In `@packages/stem/lib/src/bootstrap/stem_app.dart`: - Around line 298-302: The positional call app._disposers.insertAll(1, autoDisposers) is fragile because it relies on create() always producing a specific ordering; change the API so disposers are placed explicitly: add a new parameter (e.g., preBackendDisposers) to create() or convert app._disposers into named phases (e.g., app._preBackendDisposers, app._backendDisposers, app._postBackendDisposers) and append autoDisposers to the appropriate named phase instead of using insertAll(1,...); update all call sites of create() and any code that iterates _disposers to respect the new phased structure and ensure shutdown order is explicit via the new symbol names rather than positional indices. In `@packages/stem/lib/src/bootstrap/workflow_app.dart`: - Around line 338-407: The code double-resolves adapters by calling StemStack.fromUrl and StemApp.fromUrl with the same url/adapters/overrides; change the flow so only one stack is built and reused: either (A) add an overload/parameter to StemApp.fromUrl to accept a pre-built StemStack (or StemStore factory) and call StemApp.fromUrl(stack: stack, ...) from fromUrl, or (B) have StemApp.fromUrl return or expose its built StemStack (e.g., a getter or field) and use that stack.workflowStore for the storeFactory instead of calling StemStack.fromUrl here; also remove or avoid forwarding encoderRegistry/resultEncoder/argsEncoder/additionalEncoders redundantly to create when passing an existing stemApp (ensure create only receives encoder params when it will construct a new StemApp). Reference: StemWorkflowApp.fromUrl, StemStack.fromUrl, StemApp.fromUrl, create. In `@packages/stem/test/bootstrap/stem_client_test.dart`: - Around line 64-98: Extract the duplicated test adapter implementations into a single shared test helper: create a common test class (e.g., TestStoreAdapter) that implements StemStoreAdapter and exposes constructor parameters for scheme, broker (StemBrokerFactory?), and backend (StemBackendFactory?), and implements the same methods used in both _ClientAdapter and _BootstrapAdapter; then replace both _ClientAdapter and _BootstrapAdapter usages with the new TestStoreAdapter to centralize scheme-based matching and optional factory fields so future changes to StemStoreAdapter only need one update. In `@packages/stem/test/bootstrap/stem_stack_test.dart`: - Around line 224-273: Add a symmetric sqlite guardrail test for workflows by following the existing pattern: add a test that calls StemStack.fromUrl('sqlite:///tmp/stem.db', workflows: true) and expects a StateError whose message contains both "do not provide a workflow store" and "Disable `workflows`"; mirror the structure and matchers used in the scheduling/uniqueTasks/requireRevokeStore tests so it fits alongside them in stem_stack_test.dart. --- Outside diff comments: In `@packages/stem_memory/lib/src/memory_factories.dart`: - Around line 23-39: Add a deprecated compatibility alias for the renamed factory so existing callers keep working: create a top-level function with the old public name (e.g., the previous factory identifier) annotated with `@Deprecated` and forwarding to memoryResultBackendFactory(...) (passing through defaultTtl, groupDefaultTtl, heartbeatTtl), and include a deprecation message pointing users to memoryResultBackendFactory; update any doc comment to mention the alias is deprecated and will be removed in a future release. In `@packages/stem/example/docs_snippets/lib/workflows.dart`: - Around line 234-253: The main() function still constructs the app with StemWorkflowApp.inMemory(); replace that call with the fromUrl factory so it matches other examples — use StemWorkflowApp.fromUrl('memory://', flows: [demoFlow]) when creating app in main(), keep all other logic (start, startWorkflow, waitForCompletion, close) unchanged and ensure the variable name (app) and awaited creation remain the same. In `@packages/stem/example/progress_heartbeat/README.md`: - Around line 35-41: The fenced bash code block (the triple backticks starting with "```bash") in README.md is not surrounded by blank lines; add a single blank line before the opening ```bash fence and a single blank line after the closing ``` fence so the block is separated from surrounding text, ensuring markdownlint MD031 is satisfied. In `@packages/stem/example/stack_autowire.dart`: - Around line 26-55: The example unnecessarily creates a separate StemStack via StemStack.fromUrl and extracts scheduleFactory/lockFactory/scheduleStore/lockStore which are never used because StemApp.fromUrl creates its own internal StemStack when called with uniqueTasks and requireRevokeStore; either remove the redundant StemStack.fromUrl block and related scheduleFactory/lockFactory/scheduleStore/lockStore code, or instead pass the created stores into StemApp.fromUrl using a StemWorkerConfig so the app reuses the existing stores; note that const redisAdapters is fine because StemRedisAdapter has a const ctor. In `@packages/stem/test/bootstrap/stem_stack_test.dart`: - Around line 277-323: The _TestAdapter.name getter is hardcoded to 'test' which hides the adapter's actual scheme; change the name implementation in _TestAdapter to return a value derived from the instance's scheme (e.g., return scheme or 'test-$scheme') so each adapter created with different scheme values (constructor field scheme) exposes a unique name; update the getter for name in the _TestAdapter class (symbol: _TestAdapter.name) to use the instance field scheme instead of the literal 'test'. --- Duplicate comments: In `@packages/stem/example/dlq_sandbox/Taskfile.yml`: - Around line 30-60: The run-worker and run-producer Taskfile commands use a glob ("$root/build/worker/bundle/bin/"* and "$root/build/producer/bundle/bin/"*) which can match multiple files; change each task to resolve the glob before execution, validate that exactly one binary was found (or fail with a clear error), and then exec that single path (e.g., collect matches into an array, check length, and use the first/only entry) so the tasks deterministically run a single binary instead of an ambiguous glob. In `@packages/stem/example/ops_health_suite/Taskfile.yml`: - Around line 30-60: The run-worker and run-producer tasks use a bare glob ("$root/build/worker/bundle/bin/"* and "$root/build/producer/bundle/bin/"*) which can match multiple files or none; change each task to resolve exactly one binary before exec (e.g., expand the glob into an array, validate there is exactly one match, error if zero or >1, then exec that single path) and update the run-worker and run-producer command blocks to use that resolved single binary path instead of the raw glob. In `@packages/stem/example/signals_demo/Taskfile.yml`: - Around line 30-58: The run-worker and run-producer tasks currently execute a glob ("$root/build/worker/bundle/bin/"* and "$root/build/producer/bundle/bin/"*) which can match multiple files; change each task to resolve the glob into a single binary before exec: expand the glob into an array/variable (e.g., bins=( "$root/build/.../bin/"* )), validate that exactly one match exists (or pick a deterministic one) and fail with a clear error if ambiguous, then exec that single path (replace the direct glob invocation in both run-worker and run-producer).
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (92)
README.mdTaskfile.ymlpackages/dashboard/CHANGELOG.mdpackages/stem/CHANGELOG.mdpackages/stem/README.mdpackages/stem/Taskfile.ymlpackages/stem/example/Taskfile.ymlpackages/stem/example/autoscaling_demo/README.mdpackages/stem/example/autoscaling_demo/Taskfile.ymlpackages/stem/example/autoscaling_demo/justfilepackages/stem/example/canvas_patterns/Taskfile.ymlpackages/stem/example/canvas_patterns/justfilepackages/stem/example/daemonized_worker/Taskfile.ymlpackages/stem/example/daemonized_worker/justfilepackages/stem/example/dlq_sandbox/README.mdpackages/stem/example/dlq_sandbox/Taskfile.ymlpackages/stem/example/dlq_sandbox/justfilepackages/stem/example/docs_snippets/lib/signals.dartpackages/stem/example/docs_snippets/lib/uniqueness.dartpackages/stem/example/docs_snippets/lib/workflows.dartpackages/stem/example/encrypted_payload/README.mdpackages/stem/example/encrypted_payload/Taskfile.ymlpackages/stem/example/encrypted_payload/justfilepackages/stem/example/microservice/README.mdpackages/stem/example/microservice/Taskfile.ymlpackages/stem/example/microservice/justfilepackages/stem/example/monolith_service/Taskfile.ymlpackages/stem/example/monolith_service/justfilepackages/stem/example/ops_health_suite/README.mdpackages/stem/example/ops_health_suite/Taskfile.ymlpackages/stem/example/ops_health_suite/justfilepackages/stem/example/postgres_tls/README.mdpackages/stem/example/postgres_tls/Taskfile.ymlpackages/stem/example/postgres_tls/docker-compose.ymlpackages/stem/example/postgres_tls/justfilepackages/stem/example/progress_heartbeat/README.mdpackages/stem/example/progress_heartbeat/Taskfile.ymlpackages/stem/example/progress_heartbeat/justfilepackages/stem/example/rate_limit_delay/README.mdpackages/stem/example/rate_limit_delay/Taskfile.ymlpackages/stem/example/rate_limit_delay/justfilepackages/stem/example/retry_task/README.mdpackages/stem/example/retry_task/Taskfile.ymlpackages/stem/example/retry_task/justfilepackages/stem/example/security/ed25519_tls/README.mdpackages/stem/example/security/ed25519_tls/Taskfile.ymlpackages/stem/example/security/ed25519_tls/justfilepackages/stem/example/security/hmac/README.mdpackages/stem/example/security/hmac/Taskfile.ymlpackages/stem/example/security/hmac/justfilepackages/stem/example/security/hmac_tls/README.mdpackages/stem/example/security/hmac_tls/Taskfile.ymlpackages/stem/example/security/hmac_tls/justfilepackages/stem/example/signals_demo/README.mdpackages/stem/example/signals_demo/Taskfile.ymlpackages/stem/example/signals_demo/justfilepackages/stem/example/signing_key_rotation/README.mdpackages/stem/example/signing_key_rotation/Taskfile.ymlpackages/stem/example/signing_key_rotation/justfilepackages/stem/example/stack_autowire.dartpackages/stem/example/task_context_mixed/README.mdpackages/stem/example/task_context_mixed/Taskfile.ymlpackages/stem/example/task_context_mixed/justfilepackages/stem/example/unique_tasks/Taskfile.ymlpackages/stem/example/unique_tasks/justfilepackages/stem/example/worker_control_lab/README.mdpackages/stem/example/worker_control_lab/Taskfile.ymlpackages/stem/example/worker_control_lab/justfilepackages/stem/example/workflows/Taskfile.ymlpackages/stem/example/workflows/custom_factories.dartpackages/stem/example/workflows/justfilepackages/stem/example/workflows/sqlite_store.dartpackages/stem/lib/src/bootstrap/stem_app.dartpackages/stem/lib/src/bootstrap/stem_client.dartpackages/stem/lib/src/bootstrap/stem_stack.dartpackages/stem/lib/src/bootstrap/workflow_app.dartpackages/stem/test/bootstrap/stem_app_test.dartpackages/stem/test/bootstrap/stem_client_test.dartpackages/stem/test/bootstrap/stem_stack_test.dartpackages/stem_adapter_tests/CHANGELOG.mdpackages/stem_adapter_tests/README.mdpackages/stem_builder/CHANGELOG.mdpackages/stem_cli/CHANGELOG.mdpackages/stem_memory/CHANGELOG.mdpackages/stem_memory/README.mdpackages/stem_memory/lib/src/memory_factories.dartpackages/stem_memory/lib/stem_memory.dartpackages/stem_postgres/CHANGELOG.mdpackages/stem_postgres/lib/src/stack/postgres_adapter.dartpackages/stem_postgres/lib/src/workflow/postgres_factories.dartpackages/stem_redis/CHANGELOG.mdpackages/stem_sqlite/CHANGELOG.md💤 Files with no reviewable changes (21)
- packages/stem/example/signing_key_rotation/justfile
- packages/stem/example/progress_heartbeat/justfile
- packages/stem/example/signals_demo/justfile
- packages/stem/example/workflows/justfile
- packages/stem/example/canvas_patterns/justfile
- packages/stem/example/security/ed25519_tls/justfile
- packages/stem/example/task_context_mixed/justfile
- packages/stem/example/rate_limit_delay/justfile
- packages/stem/example/daemonized_worker/justfile
- packages/stem/example/monolith_service/justfile
- packages/stem/example/postgres_tls/justfile
- packages/stem/example/encrypted_payload/justfile
- packages/stem/example/autoscaling_demo/justfile
- packages/stem/example/retry_task/justfile
- packages/stem/example/microservice/justfile
- packages/stem/example/security/hmac_tls/justfile
- packages/stem/example/worker_control_lab/justfile
- packages/stem/example/dlq_sandbox/justfile
- packages/stem/example/unique_tasks/justfile
- packages/stem/example/security/hmac/justfile
- packages/stem/example/ops_health_suite/justfile
📜 Review details
🧰 Additional context used
🪛 Checkov (3.2.334)
packages/stem/example/postgres_tls/Taskfile.yml
[medium] 73-74: Basic Auth Credentials
(CKV_SECRET_4)
🪛 markdownlint-cli2 (0.21.0)
packages/stem/example/postgres_tls/README.md
[warning] 16-16: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 18-18: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 20-20: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 24-24: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 26-26: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 28-28: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 30-30: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 32-32: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 34-34: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 36-36: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 41-41: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
packages/stem_sqlite/CHANGELOG.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
packages/stem/example/progress_heartbeat/README.md
[warning] 41-41: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
packages/stem_adapter_tests/CHANGELOG.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
packages/dashboard/CHANGELOG.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
packages/stem_cli/CHANGELOG.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
packages/stem_memory/CHANGELOG.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
packages/stem/example/microservice/README.md
[warning] 42-42: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🔇 Additional comments (60)
packages/stem_builder/CHANGELOG.md (1)
1-6: Changelog update looks good.packages/stem/Taskfile.yml (1)
6-10: Include block looks correct.packages/stem_redis/CHANGELOG.md (1)
1-4: Changelog update looks good.packages/stem/example/ops_health_suite/README.md (1)
33-44: Usejustinstead oftaskfor CLI health checks, or add missing tasks to the Taskfile.The commands shown use
task build-cliandtask stem ..., but neitherbuild-clinorstemtasks are defined in the ops_health_suite/Taskfile.yml. The Justfile.common defines both, so either:
- Change
tasktojust(preferred if using justfile like other examples), or- Add wrapper tasks to the Taskfile that invoke the compiled stem CLI binary.
Likely an incorrect or invalid review comment.
packages/stem/example/worker_control_lab/README.md (2)
37-60: Task-based CLI examples look consistent.The updated command examples align with the Taskfile migration.
69-80: Local workflow commands updated cleanly.The local build/run section reads correctly with Taskfile targets.
packages/stem/example/signals_demo/README.md (1)
32-42: Taskfile-based local build steps are clear.The updated commands are consistent and easy to follow.
packages/stem/example/dlq_sandbox/README.md (1)
104-113: Local Taskfile workflow update is consistent.The commands match the Taskfile-based setup described elsewhere.
packages/stem/example/retry_task/README.md (1)
51-60: Task-based local workflow is documented clearly.No issues with the updated commands.
packages/stem/example/daemonized_worker/Taskfile.yml (1)
1-31: Taskfile setup is solid and consistent.The task definitions and env loading pattern look good.
packages/stem/example/security/ed25519_tls/README.md (2)
10-14: Key/cert generation steps updated cleanly.The Taskfile commands are straightforward.
25-36: Local Taskfile workflow reads well.No issues with the updated local build/run guidance.
packages/stem/example/rate_limit_delay/README.md (1)
89-98: Task-based local workflow is consistent.The updated commands align with the Taskfile migration.
packages/stem/example/postgres_tls/docker-compose.yml (1)
1-45: The TLS certificates are properly baked into the postgres image. The Dockerfile atpackages/stem_cli/docker/testing/postgres/Dockerfileexplicitly copiesroot.crt,server.crt, andserver.keyto/etc/postgres/certs/with correct ownership and permissions set. No changes needed.packages/stem/example/security/hmac/README.md (1)
20-30: LGTM!The migration from Justfile to Taskfile command references is clean. The new section correctly leads with key rotation before dep startup.
packages/stem/CHANGELOG.md (1)
1-16: LGTM!Correctly promotes the in-progress section to
## Unreleasedrather than carrying a premature0.1.1tag, and the new bullets accurately describe the fail-fast bootstrap improvements and Taskfile migration.packages/stem_postgres/lib/src/stack/postgres_adapter.dart (1)
132-139: LGTM!The call site correctly migrates to the new positional argument, making
backendFactoryconsistent with all other factory method calls in this adapter (brokerFactory,workflowStoreFactory, etc.).packages/stem_postgres/CHANGELOG.md (1)
1-5: LGTM!The entry accurately documents the breaking signature change and the downstream adapter wiring update.
packages/stem_postgres/lib/src/workflow/postgres_factories.dart (1)
29-38: Verify no remaining callers use the old namedconnectionString:parameterThe signature change from an optional/named
connectionStringto a required positionaluriis a breaking public API change. Verification confirms only one call site exists (postgres_adapter.dartline 133), and it has been correctly updated to use the positional argument. No test files reference the old API style.Within this repository, the migration is complete. External consumers of the
stem_postgrespackage depending on the old named-parameter API will need to update their code.packages/stem/example/workflows/Taskfile.yml (1)
1-53: Solid Taskfile migration.
Tasks and env-loading patterns are consistent and clear.README.md (3)
31-61: Quick Start snippet looks consistent.
The client/worker lifecycle and enqueue/wait flow are clear.
135-145: Package list update looks good.
269-290: Targeted adapter task guidance is clear.packages/stem/lib/src/bootstrap/stem_stack.dart (4)
157-177: Fail-fast sqlite guardrails look solid.
193-235: Toggle-aware factory resolution wiring looks correct.
310-337: Improved adapter error messaging is clear.
354-365: Helper centralizes sqlite guard logic cleanly.packages/stem/example/signing_key_rotation/README.md (1)
15-47: Docs update looks consistent with Taskfile migration.packages/stem/example/autoscaling_demo/Taskfile.yml (1)
14-71: Task orchestration looks good.packages/stem/example/Taskfile.yml (1)
68-188: Central orchestration wiring looks solid.packages/stem_adapter_tests/README.md (1)
13-140: README revisions are clear and comprehensive.packages/stem_memory/lib/stem_memory.dart (1)
14-21: Export rename aligns with the new factory name.packages/stem_memory/README.md (1)
13-35: Updated quick‑start flow reads well and matches the new API.Also applies to: 41-44
packages/stem/example/retry_task/Taskfile.yml (1)
1-68: Taskfile workflow is cohesive and self‑contained.packages/stem/example/postgres_tls/README.md (1)
15-36: Taskfile‑based run instructions are clear and consistent.Also applies to: 40-42
packages/stem/lib/src/bootstrap/stem_client.dart (1)
79-122: fromUrl wiring is clean and consistent with the bootstrap stack.packages/stem/example/security/hmac_tls/Taskfile.yml (1)
1-86: LGTM — clean Taskfile structure with consistent shell hygiene.Tasks use
set -euo pipefail, template{{.TASKFILE_DIR}}for path resolution, and the sed escaping for&/|is correct for the|-delimited substitution (base64 output won't contain other sed-special replacement characters).packages/stem/lib/src/bootstrap/stem_app.dart (1)
206-305: OverallfromUrllogic is well-structured and correct.The conditional auto-provisioning of lock stores and revoke stores, the guard checks against
workerConfigfields, and the delegation tocreate()are all sound. Parameter resolution correctly avoids double-wrapping when the caller already provides a coordinator or revoke store.packages/stem/example/encrypted_payload/README.md (1)
110-122: LGTM — README correctly references all defined Taskfile tasks.All listed commands (
task init,task deps-up,task build,task run:worker,task run:enqueuer,task compose-up) have corresponding task definitions in the newTaskfile.yml.packages/stem/example/microservice/Taskfile.yml (1)
1-266: Overall structure is solid — consistent shell hygiene and well-described tasks.The init/secrets/TLS/keys workflow is well-organized, and the task descriptions make the intended usage clear.
packages/stem/example/microservice/README.md (1)
153-177: LGTM — local build section accurately reflects the Taskfile workflow.All referenced task names (
init:hmac,init:hmac-tls,init:ed25519-tls,deps-up,build,run:worker,run:enqueuer,run:beat) have corresponding definitions in the newTaskfile.yml.packages/stem/lib/src/bootstrap/workflow_app.dart (1)
1-36: LGTM on imports and constructor.New imports for
StemStack,RevokeStore, andUniqueTaskCoordinatorare needed by thefromUrlfactory and correctly added.packages/stem/example/worker_control_lab/Taskfile.yml (1)
1-73: LGTM — consistent with the other example Taskfiles.Structure, env-loading logic, and task definitions follow the same conventions as the other migrated examples. The additional
STEM_REVOKE_STORE_URL,WORKER_NAME, andWORKER_CONCURRENCYenvironment variables inrun-workerare appropriate for the worker-control-lab scenario.Taskfile.yml (2)
33-65: Well-structured centralized env bootstrap with clear error messaging.The
test:with-envinternal task cleanly encapsulates the pattern of checking required env vars → auto-sourcing the init script → re-checking → failing with actionable guidance. This eliminates duplication acrosstest,test:contract,test:redis,test:postgres, andcoverage.
89-101: Multi-lineRUNintest:contract— verify Taskfile template interpolation preserves newlines.The
RUNvalue (lines 96-101) is a multi-line YAML block that gets interpolated into the bash script via{{.RUN}}. Taskfile's Go template engine should preserve newlines in block scalars, but it's worth a quick smoke test to ensure all fivetaskinvocations execute sequentially rather than being concatenated into one line.packages/stem/test/bootstrap/stem_client_test.dart (1)
29-61: LGTM — good end-to-end coverage of thefromUrladapter resolution path.The test correctly exercises
StemClient.fromUrlwith a custom adapter, verifying that the adapter-backed broker/backend are wired and a task can execute to completion. Resource cleanup viatry/finallyis properly structured.packages/stem/test/bootstrap/stem_app_test.dart (3)
123-152: LGTM — solid adapter-backedStemApp.fromUrltest.Correctly validates that an adapter-backed broker/backend are resolved from the URL, and a task runs to completion. Clean resource management via
try/finally.
154-193: Good lifecycle verification for auto-wired stores.This test is particularly valuable — it validates that
uniqueTasks: trueandrequireRevokeStore: truetrigger automatic provisioning of the lock/revoke stores from the adapter, and thatshutdown()correctly invokes the dispose callbacks on both. The boolean-flag pattern for verifying disposal is clean and sufficient.
315-348: LGTM — end-to-endStemWorkflowApp.fromUrltest.Exercises the full
fromUrl→ workflow execution → completion path with an adapter-backed stack. Provides confidence that the newworkflow_app.dart:fromUrlfactory wires broker, backend, and workflow store correctly.packages/stem/README.md (2)
62-62: Good simplification of the Quick Start toStemClient.inMemory.Removing the dependency on external services from the first code sample lowers the barrier to entry. Users who need persistent adapters are correctly directed to the new
fromUrlsection immediately below.
75-103: Clear progressive disclosure: explicit factories → URL helper.The two-example pattern (explicit
StemClient.createwith factory functions, then the lower-boilerplateStemClient.fromUrl) gives users both the full-control and convenience paths. TheStemStoreOverridesusage for directing the backend to a different Redis DB is a practical example.packages/stem/example/workflows/sqlite_store.dart (1)
11-13: The code is correct. TheStemSqliteAdapterexplicitly handles the two-slash URI format in its_fileFromUrifunction: whenuri.pathis empty (which occurs withsqlite://workflow.sqlite), it falls back to usinguri.hostto construct the file path. The adapter supports bothsqlite://filenameandsqlite:///path/to/fileformats intentionally, making the example code functional and appropriate.packages/stem/example/docs_snippets/lib/signals.dart (1)
100-115: LGTM.Clean migration from
StemApp.createtoStemApp.fromUrl; resource lifecycle (start → enqueue → close → cancel) is correct.packages/stem/test/bootstrap/stem_stack_test.dart (2)
104-113: LGTM.Error message assertions for
uniqueTasksare correctly updated withallOf(contains(...), contains(...)).
139-201: LGTM on new revoke/workflow/broker tests; note the intentional asymmetry in broker's error assertion.The revoke store and workflow store tests correctly pair two
containsmatchers (error message + "Disable" hint). The broker test (line 193) uses only onecontainsmatcher — appropriate, since there's no "Disable X" option for a required broker.packages/stem/example/stack_autowire.dart (1)
10-11: LGTM —metadatagetter correctly fulfills theTaskHandlerinterface contract.packages/stem/example/docs_snippets/lib/uniqueness.dart (1)
101-110: LGTM.Clean migration;
uniqueTasks: truewithuniqueTaskDefaultTtlcorrectly replaces the explicit coordinator wiring, and thestem_redisimport is still justified bybuildRedisCoordinator.packages/stem/example/docs_snippets/lib/workflows.dart (2)
33-33: LGTM —StemClient.fromUrl('memory://')cleanly replacesStemClient.inMemory().
131-136: LGTM.
StemWorkflowApp.fromUrl('memory://', ...)with encoder configuration is consistent with the file's migration pattern;GzipPayloadEncoder's inlineconstconstructor is valid.packages/stem/example/workflows/custom_factories.dart (1)
8-23: No action needed — bothStemRedisAdapterandStemStoreOverridesproperly declareconstconstructors.
StemRedisAdapterdeclaresconst StemRedisAdapter({...})at packages/stem_redis/lib/src/stack/redis_adapter.dart:74, andStemStoreOverridesdeclaresconst StemStoreOverrides({...})at packages/stem/lib/src/bootstrap/stem_stack.dart:32. Theconstusage in the example code is valid.Likely an incorrect or invalid review comment.
packages/dashboard/CHANGELOG.md
Outdated
| ## Unreleased | ||
|
|
||
| - Initial release of the `stem_dashboard` package. |
There was a problem hiding this comment.
Fix markdownlint MD041 by adding a top-level heading.
The first line is an H2, which violates the configured lint rule.
Proposed fix
-## Unreleased
+# Changelog
+
+## Unreleased📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Unreleased | |
| - Initial release of the `stem_dashboard` package. | |
| # Changelog | |
| ## Unreleased | |
| - Initial release of the `stem_dashboard` package. |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dashboard/CHANGELOG.md` around lines 1 - 3, Add a top-level H1
heading above the existing "## Unreleased" entry to satisfy markdownlint MD041;
for example insert a single-line "# Changelog" (or "# stem_dashboard Changelog")
before the "## Unreleased" heading so the file begins with an H1 and retains the
existing "## Unreleased" section and package note (`stem_dashboard`).
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
packages/stem/lib/src/bootstrap/stem_app.dart (1)
299-302: Fragile positional insertion into the disposers list.This was already raised in prior review.
insertAll(1, autoDisposers)relies oncreate()always producing[workerShutdown, backendDispose, brokerDispose]at indices 0, 1, 2.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stem/lib/src/bootstrap/stem_app.dart` around lines 299 - 302, The code is fragile because app._disposers.insertAll(1, autoDisposers) assumes create() always returns [workerShutdown, backendDispose, brokerDispose]; instead locate the intended insertion point dynamically and insert there: find the index of the existing backend/broker disposal entry (e.g., by matching the backendDispose/brokerDispose callback or a unique marker in app._disposers using indexWhere) and call insertAll(foundIndex, autoDisposers); if neither is found, append autoDisposers to the end. Update the logic around app._disposers, autoDisposers and the create() consumers to use this index lookup rather than hard-coded 1.packages/stem/lib/src/bootstrap/workflow_app.dart (1)
366-388: Redundant adapter resolution:StemStack.fromUrlandStemApp.fromUrlboth resolve from the same URL/adapters.This was already raised in prior review.
StemStack.fromUrl(line 366) andStemApp.fromUrl(line 373) both perform full adapter resolution against the same URL, creating two separate stacks. Similarly, encoder parameters on lines 403-406 are silently ignored bycreate()whenstemAppis already provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/stem/lib/src/bootstrap/stem_app.dart`:
- Around line 306-317: The catch clause is shadowing the local StemStack
variable named stack; rename the catch's StackTrace variable from stack to
stackTrace in the try/catch that cleans up autoDisposers so it no longer hides
the StemStack, and update the subsequent Error.throwWithStackTrace call to pass
stackTrace (ensure autoDisposers.reversed loop and
Error.throwWithStackTrace(error, stackTrace) are adjusted accordingly).
In `@packages/stem/lib/src/bootstrap/stem_stack.dart`:
- Around line 372-379: _hasResolvedFactory currently duplicates work done by
_requireFactory by re-iterating adapters and calling the resolver twice; fix
this by changing the pre-check to return (and optionally cache) the resolved
factory instead of a boolean. Concretely, replace or augment _hasResolvedFactory
to call _optionalFactory(adapters, kind, uri, resolver) and return the resolved
T? (or a wrapper with the value) so callers can reuse that instance rather than
calling _requireFactory again; alternatively, refactor _requireFactory to accept
an optional pre-resolved factory or a toggle so it performs the sqlite/toggle
check internally. Update all callers (those using _hasResolvedFactory,
_optionalFactory, and _requireFactory) to consume the returned resolved factory
and avoid the second resolution loop.
- Around line 157-195: Add the same SQLite pre-check used for
scheduling/uniqueTasks/requireRevokeStore for workflows: check if workflows is
true and !_hasResolvedFactory<WorkflowStoreFactory>(registered,
StemStoreKind.workflow, workflowUri, (adapter) =>
adapter.workflowStoreFactory(workflowUri)) then call
_failIfSqliteStoreUnsupported(kind: StemStoreKind.workflow, uri: workflowUri,
toggle: 'workflows'); place this guard in the same block sequence (before the
subsequent _requireFactory/_requireFactory error for workflows) so users get the
sqlite-specific guidance when no workflow adapter is provided.
In `@packages/stem/lib/src/bootstrap/workflow_app.dart`:
- Around line 408-417: The catch clause `on Object catch (error, stack)` is
shadowing the earlier `StemStack stack` variable (declared above), hurting
readability; rename the catch's `stack` to e.g. `trace` (so `on Object catch
(error, trace)`) and update the subsequent call
`Error.throwWithStackTrace(error, stack)` to `Error.throwWithStackTrace(error,
trace)` to avoid the name collision while preserving behavior; leave the
`StemStack stack` identifier unchanged.
In `@packages/stem/test/bootstrap/stem_app_test.dart`:
- Around line 493-533: The _BootstrapAdapter currently always returns null from
scheduleStoreFactory; add an optional ScheduleStoreFactory? schedule field to
the _BootstrapAdapter constructor (mirroring
broker/backend/workflow/lock/revoke), store it as a final member, and return it
from the scheduleStoreFactory(Uri uri) override so fromUrl can support
scheduling when provided; update the constructor parameter list and the class
fields accordingly, referencing _BootstrapAdapter, scheduleStoreFactory, and
ScheduleStoreFactory.
In `@packages/stem/test/bootstrap/stem_stack_test.dart`:
- Line 315: There is an extra two-space indentation on the expect call asserting
stack.revokeStore is same(revokeFactory); adjust the line containing
expect(stack.revokeStore, same(revokeFactory)); to match the surrounding expect
calls (use the same 6-space indentation) so it aligns with the other assertions
in the test (symbols: expect, stack.revokeStore, revokeFactory).
---
Duplicate comments:
In `@packages/stem/lib/src/bootstrap/stem_app.dart`:
- Around line 299-302: The code is fragile because app._disposers.insertAll(1,
autoDisposers) assumes create() always returns [workerShutdown, backendDispose,
brokerDispose]; instead locate the intended insertion point dynamically and
insert there: find the index of the existing backend/broker disposal entry
(e.g., by matching the backendDispose/brokerDispose callback or a unique marker
in app._disposers using indexWhere) and call insertAll(foundIndex,
autoDisposers); if neither is found, append autoDisposers to the end. Update the
logic around app._disposers, autoDisposers and the create() consumers to use
this index lookup rather than hard-coded 1.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
packages/stem/lib/src/bootstrap/stem_app.dartpackages/stem/lib/src/bootstrap/stem_stack.dartpackages/stem/lib/src/bootstrap/workflow_app.dartpackages/stem/test/bootstrap/stem_app_test.dartpackages/stem/test/bootstrap/stem_stack_test.dart
📜 Review details
🔇 Additional comments (12)
packages/stem/lib/src/bootstrap/stem_stack.dart (2)
328-356: Improved error messages in_requireFactory— LGTM.The split between "adapter matched but didn't provide the store" and "no adapter registered at all" with toggle-specific hints is a nice UX improvement for bootstrap error diagnostics.
381-393:_failIfSqliteStoreUnsupportedonly checkssqliteandfileschemes.This is reasonable for now, but note that other file-backed database URLs (e.g.,
file+sqlite, custom scheme wrappers) would bypass this check silently. If additional schemes emerge, this will need updating.packages/stem/lib/src/bootstrap/workflow_app.dart (1)
390-407: Error cleanup correctly shuts down the app on workflow bootstrap failure — LGTM.The try/catch with swallowed shutdown errors ensures the original error surfaces while still cleaning up the auto-provisioned
StemApp.packages/stem/lib/src/bootstrap/stem_app.dart (3)
232-247:needsUniqueLockStore/needsRevokeStoreguard logic is sound.The conditions correctly avoid auto-provisioning when a coordinator or revoke store is already supplied by the caller or worker config.
249-278: Defensive null checks onstack.lockStoreandstack.revokeStoreare unreachable but acceptable.Since
StemStack.fromUrlwithuniqueTasks: trueorrequireRevokeStore: truealready throws if the factory can't be resolved, these null checks (lines 255 and 272) are effectively dead code. They're reasonable as defensive guards against future refactors ofStemStack.
280-317: Error cleanup on bootstrap failure properly disposes auto-provisioned stores in reverse order — LGTM.The reversed iteration and swallowed cleanup errors ensure orderly teardown while preserving the original failure.
packages/stem/test/bootstrap/stem_stack_test.dart (3)
224-273: Sqlite guardrail tests cover scheduling, lock, and revoke — good coverage.These tests align well with the
_failIfSqliteStoreUnsupportedpre-checks in production code. Note that a symmetric test forworkflows: truewith a sqlite URL is still absent (related to the missing sqlite pre-check inStemStack.fromUrlfor workflows).
275-317: Positive test for sqlite with custom adapter providing all toggle factories — LGTM.This test validates that the sqlite guardrails don't block legitimate adapters that explicitly handle sqlite stores.
152-200: New tests for missing workflow store and missing broker — LGTM.Good coverage of the enhanced
_requireFactoryerror paths, asserting both the store-kind-specific message and the toggle hint.packages/stem/test/bootstrap/stem_app_test.dart (3)
123-152:fromUrlresolution test — LGTM.Good basic coverage verifying adapter-backed broker/backend resolution and task execution through the URL-based bootstrap path.
154-238: Auto-wiring and disposal tests — thorough and well-structured.Both the happy-path disposal on shutdown and the error-path disposal on bootstrap failure are covered with proper tracking flags. This validates the resource lifecycle guarantees of
fromUrl.
360-441:StemWorkflowApp.fromUrlend-to-end and error cleanup tests — LGTM.The workflow completion test validates the full URL-based workflow bootstrap path, and the error cleanup test confirms that
StemAppauto-provisioned stores are properly disposed when downstreamStemWorkflowAppbootstrap fails (via theapp.shutdown()in the catch block).
| class _BootstrapAdapter implements StemStoreAdapter { | ||
| _BootstrapAdapter({ | ||
| required this.scheme, | ||
| this.broker, | ||
| this.backend, | ||
| this.workflow, | ||
| this.lock, | ||
| this.revoke, | ||
| }); | ||
|
|
||
| final String scheme; | ||
| final StemBrokerFactory? broker; | ||
| final StemBackendFactory? backend; | ||
| final WorkflowStoreFactory? workflow; | ||
| final LockStoreFactory? lock; | ||
| final RevokeStoreFactory? revoke; | ||
|
|
||
| @override | ||
| String get name => 'bootstrap-test-adapter'; | ||
|
|
||
| @override | ||
| bool supports(Uri uri, StemStoreKind kind) => uri.scheme == scheme; | ||
|
|
||
| @override | ||
| StemBrokerFactory? brokerFactory(Uri uri) => broker; | ||
|
|
||
| @override | ||
| StemBackendFactory? backendFactory(Uri uri) => backend; | ||
|
|
||
| @override | ||
| WorkflowStoreFactory? workflowStoreFactory(Uri uri) => workflow; | ||
|
|
||
| @override | ||
| ScheduleStoreFactory? scheduleStoreFactory(Uri uri) => null; | ||
|
|
||
| @override | ||
| LockStoreFactory? lockStoreFactory(Uri uri) => lock; | ||
|
|
||
| @override | ||
| RevokeStoreFactory? revokeStoreFactory(Uri uri) => revoke; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
_BootstrapAdapter.scheduleStoreFactory always returns null.
This is fine for current tests, but if future tests need scheduling support through fromUrl, this adapter will need extending. Consider adding an optional schedule parameter for symmetry with the other store kinds.
Proposed improvement
class _BootstrapAdapter implements StemStoreAdapter {
_BootstrapAdapter({
required this.scheme,
this.broker,
this.backend,
this.workflow,
this.lock,
this.revoke,
+ this.schedule,
});
final String scheme;
final StemBrokerFactory? broker;
final StemBackendFactory? backend;
final WorkflowStoreFactory? workflow;
final LockStoreFactory? lock;
final RevokeStoreFactory? revoke;
+ final ScheduleStoreFactory? schedule;
`@override`
- ScheduleStoreFactory? scheduleStoreFactory(Uri uri) => null;
+ ScheduleStoreFactory? scheduleStoreFactory(Uri uri) => schedule;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/stem/test/bootstrap/stem_app_test.dart` around lines 493 - 533, The
_BootstrapAdapter currently always returns null from scheduleStoreFactory; add
an optional ScheduleStoreFactory? schedule field to the _BootstrapAdapter
constructor (mirroring broker/backend/workflow/lock/revoke), store it as a final
member, and return it from the scheduleStoreFactory(Uri uri) override so fromUrl
can support scheduling when provided; update the constructor parameter list and
the class fields accordingly, referencing _BootstrapAdapter,
scheduleStoreFactory, and ScheduleStoreFactory.
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
1 participant
Summary
Validation
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores
Tests