Productize Samsarix Routine Engine and add its Samsarix roadmap - #1
Conversation
Summary by CodeRabbit
WalkthroughThe repository is repositioned around a packaged ChangesRoutine Engine productization
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant RoutineEngine
participant RegisteredAction
participant JsonStore
CLI->>RoutineEngine: Load and validate workflow
RoutineEngine->>RegisteredAction: Execute resolved action context
RegisteredAction-->>RoutineEngine: Return output or failure
RoutineEngine->>JsonStore: Save workflow and run result
RoutineEngine-->>CLI: Return serialized result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 15: Update both checkout steps in the test and build jobs to set
persist-credentials to false, ensuring the checkout action does not retain the
job token in the repository configuration.
In `@LICENSE`:
- Line 48: Update the LICENSE terms around the pricing-schedule reference so the
complete commercial pricing schedule is included with the distributed license or
replaced with a stable, versioned reference. Ensure downstream users can
determine applicable compliance requirements and fees without relying on an
unavailable document.
In `@ROADMAP.md`:
- Around line 30-38: Update the “Current hardening backlog” section in
ROADMAP.md by removing the stale claims that the candidate is uncommitted or
absent remotely, the license metadata is outdated, and CI or static
implementation claims remain unverified. Replace them with only the currently
outstanding, evidence-backed release gates, consistent with the documented BSL
update and the release status described earlier in the roadmap.
In `@routine_engine/helix_flow/api_routes.py`:
- Around line 41-43: Update _builder_guard handling in _builder_guard to cache a
distinct unavailable sentinel when importing require_builder raises ImportError,
then raise the existing 503 response for that cached state without retrying the
import or logging repeatedly.
In `@src/routine_engine/engine.py`:
- Around line 53-60: Update Engine.validate so direct Workflow inputs are
normalized through the same Workflow.from_dict validation path as mapping
inputs, using the existing to_dict conversion rather than trusting the instance.
Preserve the subsequent unregistered-action check and return the validated
Workflow so empty, invalid concurrency, and dependency violations raise
WorkflowValidationError before scheduling.
In `@src/routine_engine/models.py`:
- Around line 105-110: Update the workflow construction around the classmethod
returning cls to recursively freeze detached_params, converting nested
dictionaries and lists into immutable equivalents while preserving scalar
values. Update Workflow.to_dict() to recursively thaw those immutable containers
back into JSON-serializable dictionaries and lists before returning parameters.
- Around line 39-43: Update _require_json to recursively validate all mapping
keys before json.dumps, rejecting any key that is not a str with
WorkflowValidationError. Traverse nested dictionaries and containers so
non-string keys are rejected at every level, while preserving the existing
finite and JSON-compatibility validation behavior.
In `@src/routine_engine/storage.py`:
- Around line 65-70: Move the parent-directory creation in _write inside the
existing exception-handling scope, and catch the resulting OSError alongside
serialization failures so directory-creation errors are re-raised as
StorageError. Preserve the existing error message and exception chaining for
persistence failures.
In `@tests/conftest.py`:
- Around line 32-33: Remove the unused output helper function from
tests/conftest.py, including its definition and any associated unused import if
applicable. Do not alter other test configuration or fixtures.
In `@tests/test_execution.py`:
- Around line 98-99: Add asyncio_mode = "auto" to the [tool.pytest.ini_options]
pytest configuration, ensuring the `@pytest.mark.asyncio`
test_async_actions_run_in_parallel test is consistently awaited and plugin
absence is detected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 294780af-2ca5-4fe0-9621-760e44a77ac9
📒 Files selected for processing (43)
.github/workflows/ci.yml.gitignoreCHANGELOG.mdCODE_OF_CONDUCT.mdCONTRIBUTING.mdLICENSEREADME.mdROADMAP.mdSECURITY.mddocs/API_REFERENCE.mddocs/GETTING_STARTED.mddocs/PRODUCTIZATION.mdexamples/01_basic_workflow.pyexamples/02_parallel_workflow.pyexamples/02_scheduled_workflow.pyexamples/03_error_handling.pyexamples/input.jsonexamples/workflow.jsonpyproject.tomlpytest.inirequirements-test.txtroutine_engine/README.mdroutine_engine/helix_flow/api_routes.pyroutine_engine/helix_flow/pyproject.tomlroutine_engine/helix_flow/tools.pyroutine_engine/workflow_engine/README.mdroutine_engine/workflow_engine/core.pyroutine_engine/workflow_engine/integrations.pysetup.pysrc/routine_engine/__init__.pysrc/routine_engine/__main__.pysrc/routine_engine/builtins.pysrc/routine_engine/cli.pysrc/routine_engine/engine.pysrc/routine_engine/errors.pysrc/routine_engine/models.pysrc/routine_engine/py.typedsrc/routine_engine/storage.pytests/conftest.pytests/test_execution.pytests/test_scheduling.pytests/test_tasks.pytests/test_workflow.py
💤 Files with no reviewable changes (5)
- examples/02_scheduled_workflow.py
- routine_engine/helix_flow/pyproject.toml
- pytest.ini
- setup.py
- requirements-test.txt
| quality: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Set persist-credentials: false on both checkout steps.
actions/checkout writes the job token into .git/config by default, so any later step (or uploaded artifact containing .git) can leak it. Neither job pushes, so credentials are not needed.
🔒️ Proposed hardening
- - uses: actions/checkout@v4
+ - uses: actions/checkout@v4
+ with:
+ persist-credentials: falseApply the same change to the test job checkout at Line 44.
📝 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.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 15-15: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 15-15: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml at line 15, Update both checkout steps in the test
and build jobs to set persist-credentials to false, ensuring the checkout action
does not retain the job token in the repository configuration.
Source: Linters/SAST tools
| the monthly volume of such production use exceeds 1,000 API calls or equivalent | ||
| usage metric, then you must secure a commercial license key from Licensor or pay | ||
| the fees specified in the pricing schedule at https://helixcollective.dev/pricing. | ||
| the fees specified in the pricing schedule provided by Licensor. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== LICENSE excerpt and structure =="
if [ -f LICENSE ]; then
wc -l LICENSE
sed -n '1,120p' LICENSE | cat -n
else
fd -a '^LICENSE$' .
fi
echo
echo "== Repository references to pricing schedule / Additional Use Grant =="
rg -n --hidden --glob '!/.git/**' --glob '!/node_modules/**' \
'Additional Use Grant|pricing schedule|production use|BSL|License' . || true
echo
echo "== File diff stat/name-only if available =="
git diff --stat -- LICENSE 2>/dev/null || true
git show --stat -- LICENSE 2>/dev/null | sed -n '1,80p' || trueRepository: Deathcharge/routine-engine
Length of output: 9795
🌐 Web query:
MariaDB Business Source License 1.1 Additional Use Grant production use license text
💡 Result:
The Business Source License (BSL) 1.1 does not contain a single, universal "Additional Use Grant" text [1][2]. Instead, the Additional Use Grant is a placeholder section that the licensor must define for each specific product [3][4][5]. The core BSL 1.1 license text describes this section as follows: "The Licensor may make an Additional Use Grant, above, permitting limited production use." [1][2][6] In the license header or associated license file, the licensor is required to: "To either: (a) specify an additional grant of rights to use that does not impose any additional restriction on the right granted in this License, as the Additional Use Grant; or (b) insert the text 'None'." [1][2][6] Because this section is product-specific, its content varies depending on how the software vendor chooses to define their usage terms [4][5]. For example, in MariaDB Corporation’s use of the BSL for MaxScale, the Additional Use Grant has been defined as: "Additional Use Grant: You may use the Licensed Work when your application uses the Licensed Work with a total of less than three server instances in production." [7][8] In summary, if you are evaluating a BSL 1.1 licensed product, you must look at the specific license file provided with that software to find the actual text of the Additional Use Grant, as it determines the specific parameters under which you are permitted to use the software in production without a separate commercial license [4][5].
Citations:
- 1: https://mariadb.com/bsl11/
- 2: https://spdx.github.io/license-list-data/BUSL-1.1.html
- 3: https://mariadb.com/bsl-faq-mariadb.md
- 4: https://mariadb.com/bsl-faq-adopting/
- 5: https://mariadb.com/bsl-faq-mariadb/
- 6: https://github.com/spdx/license-list-data/blob/main/text/BUSL-1.1.txt
- 7: https://github.com/mariadb-corporation/MaxScale/blob/2.5/LICENSE25.TXT
- 8: https://github.com/mariadb-corporation/MaxScale/blob/24.02/licenses/LICENSE2402.TXT
🌐 Web query:
SPDX BUSL-1.1 BSL template SourceUseLimitation Additional Use Grant
💡 Result:
The Business Source License 1.1 (identified by the SPDX short identifier BUSL-1.1) is a source-available license that balances commercial protection for licensors with a guarantee of eventual open-source status [1][2][3]. It is not an open-source license, as it restricts production use [2][4]. The license operates as a template requiring the licensor to complete specific fields [5][3]. Central to its structure are three key components: 1. Additional Use Grant: This optional field allows the licensor to grant specific permissions that override the default prohibition on production use [2][3]. Licensors can define specific permitted use cases here (e.g., non-commercial use, internal use, or specific integrations) [6][7][8]. If no grant is provided, the licensor may simply insert "None" [5][9][10]. 2. Change Date: The licensor must specify a date or time period, capped at four years, after which the software automatically converts to the Change License [1][2][4]. 3. Change License: The licensor must specify an open-source license (which must be compatible with GPL v2.0 or later) that the software will adopt once the Change Date is reached [5][2][4]. The license explicitly grants users the right to copy, modify, create derivative works, and redistribute the software, provided they comply with the restrictions on production use as defined by the Additional Use Grant [1][5][8]. If a user's intended use falls outside the scope of the Additional Use Grant, they must purchase a separate commercial license from the copyright holder [1][2][3].
Citations:
- 1: https://spdx.org/licenses/BUSL-1.1.html
- 2: https://en.wikipedia.org/wiki/Business_Source_License
- 3: https://legalclarity.org/business-source-license-requirements-and-how-it-works/
- 4: https://fossa.com/blog/business-source-license-requirements-provisions-history/
- 5: https://akka.io/bsl/license
- 6: https://github.com/Yasuke2000/tachoviewer/blob/main/LICENSE
- 7: https://github.com/hashicorp/terraform/blob/054fdf93cf36f77d7da8f249c84e9b1432de2b28/LICENSE
- 8: https://www.tldrlegal.com/license/business-source-license-bsl-1-1
- 9: https://raw.githubusercontent.com/akka/akka-core/main/LICENSE
- 10: https://github.com/berachain/beacon-kit/blob/main/LICENSE
Publish or embed the pricing schedule referenced by BSL Section 2.
Additional Use Grant: None means the only production-use terms are the Section 2 modifications, but line 48 refers to a pricing schedule that is not attached or versioned here. Make the full commercial-license terms immediately available with the distributed license or reference a stable, versioned schedule; otherwise downstream users cannot determine compliance or cost.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LICENSE` at line 48, Update the LICENSE terms around the pricing-schedule
reference so the complete commercial pricing schedule is included with the
distributed license or replaced with a stable, versioned reference. Ensure
downstream users can determine applicable compliance requirements and fees
without relying on an unavailable document.
| Current hardening backlog: | ||
|
|
||
| - The entire candidate is uncommitted and absent remotely. | ||
| - No total workflow-file, nesting, persisted-record, output, or error-message size cap. | ||
| - Sync actions can block the event loop; actions have no timeout, isolation, idempotency key, compensation, or durable queue. | ||
| - Local JSON persistence is single-process and may retain secrets in plaintext. | ||
| - The market already has many DAG/workflow libraries; differentiation is weak without a portfolio consumer. | ||
| - License metadata is stale and internally risky: the BSL names Helix Collective and “Helix Licensing System,” has no Additional Use Grant, and does not clearly identify this package. | ||
| - Untracked CI and all static implementation claims remain unverified. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove the stale, contradictory release-state backlog.
Lines 32 and 38 contradict Line 16; Line 37 also conflicts with the documented BSL update. Replace these historical assertions with the actual outstanding, evidence-backed gates so release decisions are not based on mutually exclusive states.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ROADMAP.md` around lines 30 - 38, Update the “Current hardening backlog”
section in ROADMAP.md by removing the stale claims that the candidate is
uncommitted or absent remotely, the license metadata is outdated, and CI or
static implementation claims remain unverified. Replace them with only the
currently outstanding, evidence-backed release gates, consistent with the
documented BSL update and the release status described earlier in the roadmap.
| except ImportError: | ||
| logger.warning("require_builder unavailable — flows fall back to auth-only") | ||
| _builder_guard = None | ||
| if _builder_guard is None: | ||
| return await get_current_user(request=request, credentials=credentials) | ||
| logger.error("require_builder unavailable — protected flow execution is disabled") | ||
| raise HTTPException(status_code=503, detail="Flow authorization is unavailable") from None |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Cache the unavailable authorization state.
_builder_guard remains _UNSET after this failure, so every protected request re-imports and logs an error before returning 503. Cache a distinct unavailable sentinel, then raise the same 503 without repeated imports/log spam.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@routine_engine/helix_flow/api_routes.py` around lines 41 - 43, Update
_builder_guard handling in _builder_guard to cache a distinct unavailable
sentinel when importing require_builder raises ImportError, then raise the
existing 503 response for that cached state without retrying the import or
logging repeatedly.
| def validate(self, workflow: Workflow | Mapping[str, Any]) -> Workflow: | ||
| """Validate structure, dependency graph, and action registrations.""" | ||
|
|
||
| definition = workflow if isinstance(workflow, Workflow) else Workflow.from_dict(workflow) | ||
| missing = sorted({step.action for step in definition.steps} - self._actions.keys()) | ||
| if missing: | ||
| raise WorkflowValidationError(f"unregistered action(s): {', '.join(missing)}") | ||
| return definition |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Revalidate direct Workflow inputs.
Line 56 trusts every Workflow instance, but structural/resource checks occur in Workflow.from_dict(). A directly constructed empty workflow can return success without steps, while invalid concurrency/dependencies can reach scheduler errors instead of WorkflowValidationError. Enforce invariants at model construction or normalize Workflow instances through to_dict()/from_dict() here.
Proposed boundary normalization
- definition = workflow if isinstance(workflow, Workflow) else Workflow.from_dict(workflow)
+ definition = (
+ Workflow.from_dict(workflow.to_dict())
+ if isinstance(workflow, Workflow)
+ else Workflow.from_dict(workflow)
+ )📝 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.
| def validate(self, workflow: Workflow | Mapping[str, Any]) -> Workflow: | |
| """Validate structure, dependency graph, and action registrations.""" | |
| definition = workflow if isinstance(workflow, Workflow) else Workflow.from_dict(workflow) | |
| missing = sorted({step.action for step in definition.steps} - self._actions.keys()) | |
| if missing: | |
| raise WorkflowValidationError(f"unregistered action(s): {', '.join(missing)}") | |
| return definition | |
| def validate(self, workflow: Workflow | Mapping[str, Any]) -> Workflow: | |
| """Validate structure, dependency graph, and action registrations.""" | |
| definition = ( | |
| Workflow.from_dict(workflow.to_dict()) | |
| if isinstance(workflow, Workflow) | |
| else Workflow.from_dict(workflow) | |
| ) | |
| missing = sorted({step.action for step in definition.steps} - self._actions.keys()) | |
| if missing: | |
| raise WorkflowValidationError(f"unregistered action(s): {', '.join(missing)}") | |
| return definition |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routine_engine/engine.py` around lines 53 - 60, Update Engine.validate so
direct Workflow inputs are normalized through the same Workflow.from_dict
validation path as mapping inputs, using the existing to_dict conversion rather
than trusting the instance. Preserve the subsequent unregistered-action check
and return the validated Workflow so empty, invalid concurrency, and dependency
violations raise WorkflowValidationError before scheduling.
| def _require_json(value: Any, field_name: str) -> None: | ||
| try: | ||
| json.dumps(value, allow_nan=False) | ||
| except (TypeError, ValueError, OverflowError) as exc: | ||
| raise WorkflowValidationError(f"{field_name} must be finite, JSON-compatible data") from exc |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Reject non-string JSON object keys.
Line 41 accepts {1: "a", "1": "b"}; the round-trip at Line 105 serializes both keys as "1" and silently retains only one value. Recursively require str keys before serialization.
Proposed fix
def _require_json(value: Any, field_name: str) -> None:
+ def require_string_keys(item: Any) -> None:
+ if isinstance(item, Mapping):
+ if any(not isinstance(key, str) for key in item):
+ raise WorkflowValidationError(f"{field_name} object keys must be strings")
+ for nested in item.values():
+ require_string_keys(nested)
+ elif isinstance(item, list):
+ for nested in item:
+ require_string_keys(nested)
+
+ require_string_keys(value)
try:
json.dumps(value, allow_nan=False)📝 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.
| def _require_json(value: Any, field_name: str) -> None: | |
| try: | |
| json.dumps(value, allow_nan=False) | |
| except (TypeError, ValueError, OverflowError) as exc: | |
| raise WorkflowValidationError(f"{field_name} must be finite, JSON-compatible data") from exc | |
| def _require_json(value: Any, field_name: str) -> None: | |
| def require_string_keys(item: Any) -> None: | |
| if isinstance(item, Mapping): | |
| if any(not isinstance(key, str) for key in item): | |
| raise WorkflowValidationError(f"{field_name} object keys must be strings") | |
| for nested in item.values(): | |
| require_string_keys(nested) | |
| elif isinstance(item, list): | |
| for nested in item: | |
| require_string_keys(nested) | |
| require_string_keys(value) | |
| try: | |
| json.dumps(value, allow_nan=False) | |
| except (TypeError, ValueError, OverflowError) as exc: | |
| raise WorkflowValidationError(f"{field_name} must be finite, JSON-compatible data") from exc |
🧰 Tools
🪛 ast-grep (0.45.0)
[info] 40-40: use jsonify instead of json.dumps for JSON output
Context: json.dumps(value, allow_nan=False)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routine_engine/models.py` around lines 39 - 43, Update _require_json to
recursively validate all mapping keys before json.dumps, rejecting any key that
is not a str with WorkflowValidationError. Traverse nested dictionaries and
containers so non-string keys are rejected at every level, while preserving the
existing finite and JSON-compatibility validation behavior.
| detached_params = json.loads(json.dumps(params, allow_nan=False)) | ||
| return cls( | ||
| id=step_id, | ||
| action=action, | ||
| needs=tuple(raw_needs), | ||
| params=MappingProxyType(detached_params), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Deep-freeze validated workflow parameters.
MappingProxyType protects only params itself. Callers can still mutate nested dictionaries or lists in a supposedly immutable Workflow, changing the validated definition after construction. Recursively freeze JSON containers, and thaw them when serializing in to_dict().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routine_engine/models.py` around lines 105 - 110, Update the workflow
construction around the classmethod returning cls to recursively freeze
detached_params, converting nested dictionaries and lists into immutable
equivalents while preserving scalar values. Update Workflow.to_dict() to
recursively thaw those immutable containers back into JSON-serializable
dictionaries and lists before returning parameters.
| def _write(self, state: dict[str, Any]) -> None: | ||
| self.path.parent.mkdir(parents=True, exist_ok=True) | ||
| try: | ||
| payload = json.dumps(state, indent=2, sort_keys=True, allow_nan=False) + "\n" | ||
| except (TypeError, ValueError, OverflowError) as exc: | ||
| raise StorageError("workflow outputs must be finite, JSON-compatible data to persist them") from exc |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Wrap parent-directory failures in StorageError.
Line 66 is outside the try block. For example, when a path component is an existing file, mkdir() raises raw OSError rather than the documented StorageError for unwritable state.
Proposed fix
def _write(self, state: dict[str, Any]) -> None:
- self.path.parent.mkdir(parents=True, exist_ok=True)
try:
+ self.path.parent.mkdir(parents=True, exist_ok=True)
payload = json.dumps(state, indent=2, sort_keys=True, allow_nan=False) + "\n"
- except (TypeError, ValueError, OverflowError) as exc:
+ except (OSError, TypeError, ValueError, OverflowError) as exc:
raise StorageError("workflow outputs must be finite, JSON-compatible data to persist them") from exc📝 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.
| def _write(self, state: dict[str, Any]) -> None: | |
| self.path.parent.mkdir(parents=True, exist_ok=True) | |
| try: | |
| payload = json.dumps(state, indent=2, sort_keys=True, allow_nan=False) + "\n" | |
| except (TypeError, ValueError, OverflowError) as exc: | |
| raise StorageError("workflow outputs must be finite, JSON-compatible data to persist them") from exc | |
| def _write(self, state: dict[str, Any]) -> None: | |
| try: | |
| self.path.parent.mkdir(parents=True, exist_ok=True) | |
| payload = json.dumps(state, indent=2, sort_keys=True, allow_nan=False) + "\n" | |
| except (OSError, TypeError, ValueError, OverflowError) as exc: | |
| raise StorageError("workflow outputs must be finite, JSON-compatible data to persist them") from exc |
🧰 Tools
🪛 ast-grep (0.45.0)
[info] 67-67: use jsonify instead of json.dumps for JSON output
Context: json.dumps(state, indent=2, sort_keys=True, allow_nan=False)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routine_engine/storage.py` around lines 65 - 70, Move the
parent-directory creation in _write inside the existing exception-handling
scope, and catch the resulting OSError alongside serialization failures so
directory-creation errors are re-raised as StorageError. Preserve the existing
error message and exception chaining for persistence failures.
| def output(context: ActionContext) -> Any: | ||
| return context.params.get("value") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Any importer or direct caller of the conftest `output` helper?
rg -nP --type=py '\bfrom\s+(tests\.)?conftest\s+import\b|\bimport\s+conftest\b'
rg -nP --type=py -g 'tests/**' '(?<![\w.])output\s*\('Repository: Deathcharge/routine-engine
Length of output: 164
🏁 Script executed:
#!/bin/bash
set -u
echo "== Python files mentioning conftest output =="
rg -nP --type=py 'conftest|tests\.conftest|from .*tests.*conftest|import conftest|output\s*\('
echo
echo "== Candidate pytest config/conftest files =="
git ls-files | rg '(^|/)pytest.*\.ini$|(^|/)pyproject\.toml$|(^|/)setup\.cfg$|(^|/)tox\.ini$|^tests/conftest\.py$' || true
echo
echo "== tests/conftest.py =="
if [ -f tests/conftest.py ]; then
sed -n '1,120p' tests/conftest.py | cat -n
fi
echo
echo "== All Python files under tests =="
git ls-files 'tests/**/*.py' 'tests/*.py' | sortRepository: Deathcharge/routine-engine
Length of output: 505
🏁 Script executed:
#!/bin/bash
set -u
echo "== Python files mentioning output helper patterns =="
rg -nP --type=py 'conftest|tests\.conftest|from .*tests.*conftest|import conftest|^\s*output\s*\(' tests || true
echo
echo "== Candidate pytest config/conftest files =="
git ls-files | rg '(^|/)pytest.*\.ini$|(^|/)pyproject\.toml$|(^|/)setup\.cfg$|(^|/)tox\.ini$|^tests/conftest\.py$|^conftest\.py$' || true
echo
echo "== tests/conftest.py =="
if [ -f tests/conftest.py ]; then
cat -n tests/conftest.py
fi
echo
echo "== All Python files under tests =="
git ls-files 'tests/**/*.py' 'tests/*.py' | sort
echo
echo "== Direct AST/import probe for unqualified output helper usages =="
python3 - <<'PY'
import ast, pathlib
for p in pathlib.Path('tests').rglob('*.py'):
try:
tree = ast.parse(p.read_text())
except SyntaxError:
continue
for node in ast.walk(tree):
if isinstance(node, ast.Call):
fn = node.func
if isinstance(fn, ast.Name) and fn.id == 'output' and ast.dump(fn.annotation) != ast.dump(
ast.parse('ActionContext').body[0].value if isinstance(fn.annotation, ast.Attribute) and fn.annotation.value.id == 'ActionContext' else None
):
print(f"{p}:{node.lineno}: unqualified function call output(...)")
elif isinstance(fn, ast.Attribute) and fn.attr == 'output':
try:
print(f"{p}:{node.lineno}: attribute access ..{fn.attr}")
except Exception:
pass
PYRepository: Deathcharge/routine-engine
Length of output: 1619
Remove the unused output helper from tests/conftest.py.
It is not a pytest fixture and no test module calls or imports it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/conftest.py` around lines 32 - 33, Remove the unused output helper
function from tests/conftest.py, including its definition and any associated
unused import if applicable. Do not alter other test configuration or fixtures.
| @pytest.mark.asyncio | ||
| async def test_async_actions_run_in_parallel() -> None: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -H -t f 'pyproject.toml|setup.cfg|pytest.ini|tox.ini' -x rg -n 'pytest-asyncio|asyncio_mode|filterwarnings|\[tool\.pytest' {}Repository: Deathcharge/routine-engine
Length of output: 227
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== config files =="
fd -H -t f '(^pyproject\.toml$|setup\.cfg|pytest\.ini|tox\.ini)' -x sh -c 'echo "--- $1"; sed -n "1,120p" "$1"' sh {}
echo "== async tests in tests/test_execution.py =="
fd -H -t f 'test_execution.py' -x sh -c 'echo "--- $1"; grep -n "`@pytest.mark.asyncio`\|async def " "$1" || true' sh {}
echo "== pytest async config references =="
rg -n "pytest-asyncio|asyncio_mode|tool\.pytest|filterwarnings" pyproject.toml setup.cfg pytest.ini tox.ini 2>/dev/null || true
echo "== repo dependencies sections =="
sed -n '1,90p' pyproject.tomlRepository: Deathcharge/routine-engine
Length of output: 4918
Set asyncio_mode in pytest config.
pytest-asyncio is in the dev dependency set, but [tool.pytest.ini_options] does not declare asyncio_mode. Add asyncio_mode = "auto" so @pytest.mark.asyncio tests are awaited consistently and fail if the plugin is absent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_execution.py` around lines 98 - 99, Add asyncio_mode = "auto" to
the [tool.pytest.ini_options] pytest configuration, ensuring the
`@pytest.mark.asyncio` test_async_actions_run_in_parallel test is consistently
awaited and plugin absence is detected.
1 participant
Merge intent
This PR integrates the reviewed productization branch and its repository-specific roadmap. Merge is distinct from release, publication, deployment, or adoption as a canonical Samsarix Unified subsystem.
Portfolio contract
Verification
The portfolio merge-readiness pass compared this branch with its default, inspected its flagship relationship, and found no merge-blocking regression. Repository CI and any additional local verification remain recorded in the portfolio audit ledger.