Wave 1: core package - #1
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 299 |
| Duplication | 0 |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
The PR establishes the core package and monorepo structure but contains several critical issues that should prevent merging in its current state. Most significantly, the Codacy analysis is 'not up to standards' due to 1,242 new issues and missing coverage data.
The implementation contains a major architectural divergence from ADR-006: operation IDs are currently generated using counters and name-concatenation instead of the required SHA-256 content-addressing. This breaks cache stability and plan reproducibility. Additionally, the condition evaluator contains a security risk related to unsafe property access and high cyclomatic complexity (34). Several implementation plan steps (6.2.7) and required files (canonical.ts) are also missing.
About this PR
- The planned file 'packages/core/src/internal/canonical.ts' is missing despite being listed as a requirement for stable ID generation in the Wave 1 plan.
- The PR scope significantly exceeds the 'core package' summary, including project-wide monorepo bootstrapping, website scaffolding, and extensive documentation tree setup.
Test suggestions
- run() composable creates a node with 'run' kind and preserves all spec fields
- pipeline() wires a linear dependency chain via predecessor references
- parallel() creates a synthetic join node with input operations as siblings
- matrix() produces a Cartesian product expansion of nodes with MATRIX_ env vars
- Safe condition evaluator handles operator precedence (NOT > AND > OR)
- Condition evaluator operates without eval() or Function constructor
- Planning process performs zero I/O during discovery and walk (verified via spies)
- Cycle detection identifies and rejects cyclic graphs with CompositionError
- Operation IDs are generated using SHA-256 content hashes as specified in ADR-006
- Planner identifies and rejects true duplicate operations sharing the same content
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Operation IDs are generated using SHA-256 content hashes as specified in ADR-006
2. Planner identifies and rejects true duplicate operations sharing the same content
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
The merge order was bottom-up (#1 first). This is wrong — stacks should be merged TOP-DOWN: 1. Rebase the TOP PR onto main (its diff now includes ALL stack changes) 2. /act --loop on the TOP PR until convergence 3. Squash merge the TOP PR → main gets everything in one commit 4. Close all lower PRs (their changes are included in the top PR's squash) 5. Retrospect, advance to next stack This is faster: one merge per stack (not N), one /act convergence per stack (on the top PR only), lower PRs are closed not merged. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
MergerCommit |
|
Skipping CodeAnt AI review — this PR changes more than 100 files, which usually means a migration, codemod, or vendored drop. Line-level review on diffs this large produces duplicate findings on the same rewrite pattern and drowns out anything that actually matters. If you still want a review, comment |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR establishes Sverka’s workflow foundation. It adds immutable operation composition, matrix expansion, conditional planning, DAG validation, runtime modes, public APIs, package tooling, agent orchestration, specifications, and documentation. ChangesSverka foundation
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
The merge order was bottom-up (#1 first). This is wrong — stacks should be merged TOP-DOWN: 1. Rebase the TOP PR onto main (its diff now includes ALL stack changes) 2. /act --loop on the TOP PR until convergence 3. Squash merge the TOP PR → main gets everything in one commit 4. Close all lower PRs (their changes are included in the top PR's squash) 5. Retrospect, advance to next stack This is faster: one merge per stack (not N), one /act convergence per stack (on the top PR only), lower PRs are closed not merged. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 102
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
specs/14-website/spec.md (1)
244-244: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the extra Markdown fence.
This fence starts an unintended code block at end of file.
🤖 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 `@specs/14-website/spec.md` at line 244, Remove the stray Markdown fence at the end of specs/14-website/spec.md so it does not start an unintended code block.
🤖 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 @.agents/skills/beads/SKILL.md:
- Line 38: Update the ordered-list markers in SKILL.md around “Inspect before
editing” and the other affected items, replacing each explicit 2., 3., 4., and
5. marker with 1. while preserving the existing item text and list structure.
In @.codacy.yml:
- Around line 36-40: Update the Codacy configuration for the ESLint 9 and
Markdownlint engines to remove their unsupported engines.*.enabled: true
settings, and activate both engines through Codacy Code patterns instead. Also
remove the markdown disablement from languages.markdown.enabled and the Markdown
exclusion from exclude_paths so Markdown analysis is enabled.
In @.gitignore:
- Around line 157-165: Update the .gitignore entries near website/dist/ and
website/node_modules/ to include website/.astro/. Remove any currently tracked
files under website/.astro/ from version control while leaving them locally
available.
In `@AGENTS.md`:
- Line 22: Fix the Markdown fence lint errors in the directory-tree and command
examples: specify a language such as text on the directory-tree fence to satisfy
MD040, and add blank lines before and after the command fence to satisfy MD031.
In `@agents/architect/prompt.template.md`:
- Around line 24-32: Update the skill-invocation instructions in the prompt
template so every mandatory skill is explicitly invoked, including deepwiki and
sourcegraph alongside spec-driven-development, minimalist, and
critical-thinking. Ensure the corresponding workflow section lists concrete
steps to use deepwiki for external-library research and sourcegraph for codebase
searches via the src CLI, preserving the existing invocation of the first three
skills.
In `@agents/builder/prompt.template.md`:
- Around line 23-34: Update the testing instruction in the Mandatory skills
section of prompt.template.md to require running the configured Vitest suite
with “bun run test” rather than “bun test”. Keep the existing skill list and
other testing guidance unchanged.
- Around line 57-59: Update the testing instructions in the prompt template to
replace the `bun test` command with the configured Vitest command, using `bun
run test` or the relevant Nx test target. Keep the requirement to confirm
failing tests for the correct reason before implementing.
In `@agents/mayor/prompt.template.md`:
- Around line 85-115: Update the “Stacked PRs to GitHub” procedure to require
explicit authorization under the active workflow profile before any git or
GitHub mutation, including checkout, add, commit, push, gh pr create, or
Git/Dolt synchronization. When authorization is absent, skip those commands and
report the completed wave without modifying repositories or GitHub; retain the
existing branch and commit steps only for authorized workflows.
- Around line 60-73: Update the wave progression instructions around “keep going
until the project is done” to require checking for an explicit user or
orchestrator stop request or scope change before closing, creating, or
dispatching subsequent wave work. Ensure that this human-control gate takes
precedence over the automatic continue-until-complete behavior, while preserving
monitoring and re-gating for active work when no stop or scope change is
requested.
In `@agents/reviewer/prompt.template.md`:
- Around line 52-64: Replace the markdown checkbox review checklist under
“Review checklist” with instructions to track each review task using bd.
Explicitly prohibit TodoWrite, TaskCreate, and markdown TODO lists, while
preserving the existing review requirements as bd-tracked tasks.
- Around line 43-57: Update the test command in the review instructions and
checklist from “bun test” to “bun run test” so the reviewer invokes the
configured root test script through the workspace Vitest and Nx target.
In `@CLAUDE.md`:
- Around line 43-51: Update the Markdown command fence in CLAUDE.md by adding
blank lines immediately before and after the indented fenced block containing
git status, git pull --rebase, and git push, resolving MD031 without changing
the commands or surrounding content.
- Around line 61-77: Replace the placeholder sections in CLAUDE.md with the
project’s actual Bun/Nx build, test, lint, and typecheck commands, using
AGENTS.md and the existing bun run test Vitest command as the source of truth.
Add a concise architecture overview and project-specific conventions so agents
can execute and modify the repository consistently, and remove the npm example
and placeholder text.
In `@engdocs/architecture/overview.md`:
- Around line 8-15: Label the system-flow code fence in the architecture
overview as text by updating its opening fence, while leaving the diagram
content unchanged.
- Around line 29-43: Update the architecture graph code fence to use text syntax
and match the dependency direction defined in dependencies.md: keep core
independent from ir, show dependencies pointing from executors such as
runtime-docker toward runtime, and correct the remaining package relationships
accordingly.
In `@engdocs/contributing/development-setup.md`:
- Around line 12-29: Update the development setup commands to invoke Vitest
through Nx: replace the top-level `bun test` command with `bun run test`, and
change the filtered `bun test --filter=`@sverka/core`` command to `bunx nx test
core`, preserving the surrounding build and test guidance.
In `@engdocs/contributing/guide.md`:
- Around line 5-10: Update the testing step in the contributing guide to use the
workspace script command bun run test instead of bun test, while leaving the
other setup, build, lint, and typecheck commands unchanged.
In `@engdocs/contributing/waves.md`:
- Around line 22-25: Update the Wave 1 row in the wave status table so the core
package reflects its post-merge status rather than remaining Pending, while
leaving the Wave 0 row unchanged.
In `@engdocs/README.md`:
- Around line 10-16: Label the directory-tree code fence in the README with the
text language by updating its opening fence, while leaving the documented
directory tree unchanged.
- Line 34: Add the missing engdocs ADR-006 document at the path referenced by
the README and the other linked document, or update both references to point to
the correct existing ADR. Ensure all links consistently resolve to the intended
SHA-256 content-addressed Plan and Operation IDs documentation.
In `@eslint.config.mjs`:
- Around line 7-24: Update the ignores array in the ESLint configuration to stop
excluding source-bearing TypeScript paths, including website files, test files,
__tests__ directories, and *.config.ts files. Retain ignores only for generated
or intentionally non-source output, ensuring all *.ts and *.tsx files remain
covered by the project’s ESLint and formatting checks.
- Around line 27-35: Add `@typescript-eslint/parser` as a development dependency
and configure it as languageOptions.parser in the TypeScript file configuration
block. Update the existing files: ["**/*.ts"] entry without changing its other
parser or ECMAScript settings.
In `@formulas/sverka-bootstrap.toml`:
- Around line 73-76: Update the finalize step identified by id "finalize" so it
requires explicit authorization before any Git or Dolt write operation,
including commit, push, or synchronization. Revise its description or
prerequisites to make this authorization gate explicit while preserving the
existing review dependency.
- Around line 64-67: Update the review instructions in the description block to
replace “bun test” with “bun run test”, ensuring the project’s root test script
is invoked instead of Bun’s native test runner.
In `@formulas/sverka-wave.toml`:
- Around line 27-43: Update the wave dependency flow around the review and
finalize steps so reviewer rejection cannot lead to finalize execution: add an
explicit approval gate or route rejection from reviewer back to implement, and
ensure finalize only runs after approval. Preserve the existing reviewer and
mayor roles, and verify the rejection path leaves the wave incomplete.
In `@nx.json`:
- Around line 22-24: Update the lint target’s inputs in the nx configuration to
track eslint.config.mjs instead of .eslintrc.json, preserving the existing
default input and caching behavior.
- Around line 13-17: Update the build target defaults in nx.json to declare the
generated output by adding an outputs entry for {projectRoot}/dist before
retaining build caching. Keep the existing dependsOn, inputs, and cache settings
unchanged.
In `@pack.toml`:
- Around line 5-11: Remove the packs.lock ignore rule from .gitignore, run gc
import install to generate the Pack V2 lockfile, and commit the resulting
packs.lock file. Do not create or use packs.lock.toml.
In `@packages/cli/src/index.ts`:
- Line 1: Define and export a package-specific CliError class from the public
API entry point marked by the existing `@sverka/cli` declaration. Make it extend
the standard Error type and preserve normal error behavior so consumers can
identify CLI-specific failures.
In `@packages/core/src/__tests__/conditions.test.ts`:
- Around line 87-95: Align the test name and assertion in the “malformed error
has code INVALID_CONDITION” case: use the required public error code
consistently, either renaming the test to COMPOSITION_ERROR or updating the
implementation and assertion to emit INVALID_CONDITION. Keep the existing
CompositionError and context checks unchanged.
In `@packages/core/src/__tests__/dag.test.ts`:
- Around line 20-23: Update both catch blocks in the DAG tests to narrow caught
errors with err instanceof CompositionError before accessing context; retain the
existing context assertions for matching errors and re-throw unexpected errors
instead of casting them.
In `@packages/core/src/__tests__/laziness.test.ts`:
- Line 40: Update the fetch spy setup in the laziness test to reject immediately
with an “Unexpected network call” error instead of preserving the real global
fetch implementation. Apply this mock rejection configuration to both fetch
spies.
In `@packages/core/src/__tests__/matrix.test.ts`:
- Around line 19-30: Update the “multi-dimension cartesian product with joined
id suffix” test to provide two values for both node and os, then assert four
operations covering every node/os combination in the expected IDs. Extend the
environment assertions to verify each operation’s MATRIX_NODE and MATRIX_OS
values match its corresponding Cartesian-product combination.
In `@packages/core/src/internal/ids.ts`:
- Around line 44-45: Update the matrix ID generation around formatMatrixValue
and the dims mapping to use a canonical, type-preserving encoding that escapes
delimiters, ensuring distinct values such as 1 and "1" produce different IDs.
After assigning IDs to matrix children, detect and reject duplicate IDs before
returning the planned children.
In `@packages/core/src/internal/node.ts`:
- Around line 33-45: Update the after and with methods to validate every added
Operation is a genuine private-branded OperationNode before storing it,
rejecting structurally valid public Operations without the required node fields.
Remove the unchecked casts and preserve existing predecessor and sibling
accumulation for valid nodes.
In `@packages/core/src/internal/plan.ts`:
- Around line 139-147: Update the matrix expansion logic around the child
creation loop and resolveEdges() to track each matrix template’s expanded child
nodes. Rewrite downstream predecessor references from the removed template to
all corresponding matrix children before resolving edges, so dependent
operations reference every expanded child and no unresolved template IDs remain.
- Around line 194-205: Update the node-rewriting flow around rewritten and idMap
to build an old-to-new mapping for every rebuilt node, then rewrite all
predecessor and sibling references through that mapping before filtering
artifacts and returning the graph. Ensure references to unchanged nodes remain
intact and later edge resolution can no longer target replaced node identities.
In `@packages/core/src/operation.ts`:
- Around line 18-31: Define a shared JSON-serializable scalar type and apply it
to both OperationSpec.matrix and the matrix() API. During planning, validate
every matrix value before String(v) is used for MATRIX_* environment variables
or matrix IDs, rejecting undefined, BigInt, and other non-scalar values to
prevent collisions and serialization failures.
In `@packages/core/src/runtime.ts`:
- Around line 12-19: Extend RuntimeResult with a required readonly outcomes
collection, then update planWorkflow to retain both evaluated OperationOutcome
values and synthetic skipped outcomes instead of discarding them. Propagate the
aggregate through every runtime implementation and test helper, preserving
existing outcome order and behavior.
In `@packages/ir/project.json`:
- Around line 11-16: Update the test target in packages/ir project.json to pass
Vitest’s empty-suite option, or add a valid IR test file; ensure nx test ir
succeeds when no tests are present.
In `@packages/policy/src/index.ts`:
- Line 1: Define a package-specific PolicyError class in the packages/policy
entry point and export it as part of the public API. Ensure it extends the
standard Error type and is available to consumers through the existing package
export surface.
In `@packages/runtime-podman/package.json`:
- Around line 18-24: Add eslint to the devDependencies of packages that invoke
it, including runtime-podman, so the package-local Nx lint target works with Bun
isolated installs. Keep the existing lint script unchanged and use the
workspace’s established eslint version.
In `@packages/runtime/project.json`:
- Around line 4-9: Update the build target in project.json to declare
packages/runtime/dist as its Nx output, unless nx.json already defines an
equivalent global build output. Ensure the target metadata identifies the
tsdown-generated dist directory so cached builds restore the package artifact.
- Around line 11-16: Update the runtime test configuration in the Nx "test"
target and the package test script to pass Vitest's --passWithNoTests option,
ensuring both invocation paths succeed when packages/runtime has no matching
test files.
In `@packages/runtime/src/index.ts`:
- Line 1: Define a package-specific `RuntimeError` class in the
`packages/runtime` public API entry point and export it alongside the existing
API. Make it an error type suitable for runtime-specific failures, preserving
standard Error behavior and package-level accessibility.
In `@README.md`:
- Around line 84-111: Add the `text` language identifier to the fenced Markdown
blocks containing the architecture diagram and project-structure block, ensuring
both fences satisfy Markdownlint MD040.
- Around line 1-5: Update the README’s opening structure so it begins with a
valid top-level heading recognized by Markdownlint: move the “# Sverka” heading
before the opening HTML div, or replace it with an HTML h1 and change the
tagline from “###” to a paragraph. Preserve the existing title and tagline
content while satisfying MD041 and MD001.
- Around line 139-143: Update the test command in the README quality-check
command list from bun test to bun run test, preserving the documented Vitest/Nx
test workflow and the surrounding commands unchanged.
In `@specs/00-overview/spec.md`:
- Around line 34-61: Add the text language tag to the fenced ASCII diagrams in
the overview specification, including the architecture and package-structure
fences, while preserving their existing diagram content unchanged.
- Around line 110-115: Update the “Test plan” section in spec.md to use “bun run
test” instead of “bun test,” and update contributing documentation and any other
instructions still referencing the old command so all test-running guidance
consistently invokes Nx and Vitest.
In `@specs/01-core/plan.md`:
- Around line 132-144: Resolve the error-code mismatch between the condition
parser and CompositionError: update the plan and conditions-related tests to
require the existing COMPOSITION_ERROR code, or introduce a separate
condition-specific error type that exposes INVALID_CONDITION and use it from
evaluateCondition. Keep malformed expressions mapped consistently to the chosen
contract.
- Around line 293-299: Update the root test command in the documented
verification commands to use “bun run test” instead of “bun test”, preserving
the existing typecheck, lint, and build commands.
In `@specs/01-core/spec.md`:
- Line 318: Update the Markdown fences in the data-model, ID-format, and
expression-grammar sections to specify the text language, resolving MD040.
Insert a blank line immediately before the commands fence to resolve MD031,
including the additionally referenced ranges.
- Around line 279-289: Update the matrix documentation around the matrix
operation description and related examples to use the content-addressed op- ID
contract instead of deterministic ID suffixes. Remove or revise any suffix-based
wording so it matches the later matrix rules, and ensure all affected examples
consistently demonstrate the same ID behavior.
- Around line 185-192: Align the PlanContext interface with its documented value
domain by either narrowing the comment to string arrays or expanding the index
signature to support readonly number and boolean arrays. Ensure the chosen type
and prose consistently describe the supported context values.
- Around line 342-345: Update the composition specification and corresponding
test plan to state that dependsOn and tags list merges concatenate values,
remove duplicates, and preserve their original order, matching concatDedupe in
merge.ts.
- Around line 514-522: Update the Test plan command references in the
documentation: replace `bun test` with `bun run test`, and specify `bunx nx run
core:test` for the core-specific test command. Keep the existing test coverage
descriptions unchanged.
- Around line 407-418: Resolve the operation-identity contract across the
specification, core, IR, and tests by deciding whether discovery index
participates in the computed ID; ensure the documented `{ kind, name, context }`
duplicate behavior matches that decision. Update core’s `computeOperationId`
implementation and remove any conflicting `kind:name/command/index` or
collision-suffix behavior, add the corresponding IR implementation, and revise
tests to validate identical operations and distinct indexed operations according
to the chosen contract.
- Around line 376-398: Define the complete operation identity projection,
explicitly deciding whether env, image, imageDigest, workingDir, timeoutSeconds,
retries, cache, network, credentials, artifacts, and condition affect identity;
include every identity-affecting field with deterministic canonicalization.
Update the operation-id implementation in computeOperationId and the `@sverka/ir`
package to use the same SHA-256 content-addressed projection, avoiding direct
spec.id or `${kind}:${name}` ids, and expose computeOperationId from the IR
package so both consumers remain consistent.
In `@specs/02-ir/spec.md`:
- Around line 125-126: Restrict the compiler metadata fields near compiler and
the corresponding metadata definition to a recursive JsonValue type instead of
unknown. Define JsonValue to allow only JSON primitives, arrays, and object
records whose values are JsonValue, and update validation to reject functions,
bigint, undefined, and cyclic structures so metadata can round-trip through
canonical JSON.
- Around line 339-376: Update the test execution instructions in the testing
section to use the workspace’s Vitest command with the IR package selector
instead of Bun’s test runner. Replace the `bun test packages/ir` entry while
leaving the typecheck and lint commands unchanged.
In `@specs/03-runtime/spec.md`:
- Around line 355-411: Update the test commands in the runtime specification to
use the project’s Vitest workspace command with the runtime package selector
instead of Bun’s built-in test runner. Keep the existing typecheck and lint
commands unchanged.
- Around line 191-199: Update the ExecutionState.outcomes representation to use
a JSON-serializable structure, such as a string-keyed record or entry array, so
StateStore persistence retains all operation outcomes. Ensure readers and
writers consistently use the new representation while preserving outcome lookup
semantics.
- Around line 73-106: Extend ExecuteRequest and the Executor contract with a
cancellation mechanism that the scheduler can invoke for in-progress operations.
Update the scheduler’s cancel() flow to propagate cancellation to the selected
executor and await its completion, ensuring ExecuteResult reports the cancelled
status while preserving existing state transitions.
In `@specs/04-runtime-docker/spec.md`:
- Around line 224-227: Update ContainerPolicyError and its call sites so each
policy violation preserves its rule-specific code: MISSING_TIMEOUT,
MISSING_DIGEST, UNDECLARED_SECRET, or DOCKER_SOCKET_DENIED. Either accept the
code in ContainerPolicyError’s constructor or introduce dedicated subclasses,
and ensure callers observe the required code instead of the generic
CONTAINER_POLICY_VIOLATION.
- Around line 259-261: Update the test plan in specs/04-runtime-docker/spec.md
to document the runtime-docker package or Nx test target that runs Vitest via
vitest run, replacing the bun test command in both referenced test-plan
sections.
- Around line 64-77: Align DockerExecutorConfig.runAs with its documented
default by making the property optional and normalizing omitted values to
"1000:1000" in the Docker executor constructor. Preserve explicitly supplied
runAs values.
- Around line 186-195: Validate or normalize the key at the start of
CacheManager.prepare before constructing any cache paths, rejecting absolute
paths and traversal segments; alternatively derive a safe hash/identifier from
the key. Ensure the same safe key is used consistently for /cache and persistent
cacheDir paths, preventing escape or collisions.
- Around line 143-159: Remove --timeout from the documented docker run
invocation and update the policy table and timeout rules to state that
timeoutSeconds is enforced by the parent process. In the parent execution flow,
track the container ID, monitor the deadline, stop then kill the container when
it expires, collect its output, and return a timeout failure.
In `@specs/05-runtime-host/spec.md`:
- Around line 51-55: Update the public entry point export block in src/index.ts
to re-export createAllowlist from the allowlist module, alongside
CommandAllowlist, so consumers can construct the documented allowlist through
the package root.
- Around line 61-63: Update HostExecutorConfig.enabled so its type and
documentation agree: either make enabled optional and normalize omitted values
to false, or keep it required and remove the statement that it defaults to
false. Apply the chosen contract consistently wherever HostExecutorConfig is
consumed.
- Around line 250-253: Update the test plan to document the runtime-host package
or Nx test target command that invokes vitest run instead of bun test. Preserve
the existing test location and Docker-daemon requirements, and apply the same
command correction to the additional test-plan section.
- Around line 145-148: Update the timeout handling requirements in the “Apply
timeout” execution flow to terminate the entire process tree, using
process-group termination on POSIX or job-object termination on Windows as
appropriate. Ensure descendants are cleaned up, then record the timeout failure
result and return only after termination cleanup completes.
- Around line 76-77: Update the runtime host construction and related validation
paths around runAsUid so an omitted value resolves to the current UID, but
rejects the configuration when that effective UID is 0; alternatively require an
explicitly configured non-root UID. Preserve the existing non-root behavior and
apply the same validation to the additional runAsUid handling paths.
- Around line 152-153: Update the artifact collection step to canonicalize each
declared artifact path and verify it remains contained within config.workspace
before copying. Reject traversal and symlink-resolved paths that escape the
workspace, while preserving copying for valid in-workspace artifacts.
- Around line 202-216: Align HostTimeoutError and the timeout execution path
with a single observable contract: either return an ExecuteResult with status
"failure" or consistently raise HostTimeoutError. Update the public API
documentation and all affected tests, including the timeout behavior around the
referenced execution flow, so they assert the chosen contract consistently.
- Around line 131-144: Update the operation validation and spawn flow to resolve
the allowlisted command to its approved absolute executable before applying
environment overrides. Reject loader/interpreter-related variables from
operation.env and the final environment, and explicitly spawn the executable
with shell: false while preserving the existing allowlist and environment merge
behavior.
- Around line 307-310: Expand the “Retry policy” section to state that Scheduler
owns retries and HostExecutor models one process attempt, using
retry.maxAttempts, retry.backoffSeconds, and retry.retryOn. Define how logs,
duration, exit data, errors, and artifacts are combined across attempts,
including artifact staging, cleanup, overwrite behavior, and repeated
non-idempotent workspace side effects. Update the success test to configure
retry.maxAttempts instead of an unscoped maxAttempts.
In `@specs/06-planner/spec.md`:
- Around line 406-459: Update the test execution instruction for planner tests
to use the project’s Vitest workspace command with the appropriate package
selector, replacing the `bun test` command while retaining the existing test
location and coverage scope.
- Around line 119-121: Update the ProjectContext contract and its construction
to remove raw provider tokens from the returned context, replacing credentials
with availability metadata only. Keep actual secrets in the private runtime
credential provider, and ensure PlanResult and the CLI inspect output cannot
expose them; apply the same change to the additional credential-handling
section.
In `@specs/07-findings/spec.md`:
- Around line 160-170: Align the Baseline model and resolved comparison result
so resolved findings are representable: either persist complete Finding
snapshots alongside fingerprints and use them in the compare operation, or
change resolvedFindings to return fingerprint values. Update the Baseline
interface and all affected compare/result definitions and serialization paths
consistently, including the referenced resolved-entry handling.
- Around line 450-506: Update the test execution instruction at the start of the
findings test plan to use the project’s Vitest workspace command with the
appropriate package selector instead of Bun’s built-in runner. Keep the
referenced test directory and all listed test scenarios unchanged.
In `@specs/08-policy/spec.md`:
- Around line 309-359: Update the test execution instruction to use the
project’s Vitest workspace command with the appropriate package selector instead
of Bun’s built-in runner. Preserve the existing test location and coverage list.
- Around line 64-67: Extend the policy evaluation input, using Finding or
PolicyContext, to carry suppression state or the set of suppressed fingerprints.
Propagate this state into the evaluator so excludeSuppressed filters matching
findings when true and retains them when false, including custom-rule evaluation
and all related policy specification sections.
In `@specs/09-sdk/spec.md`:
- Around line 216-229: Update the SDK workflow example’s test task in the
defineWorkflow configuration to invoke the project script with “bun run test”
instead of “bun test”, matching the existing lint and typecheck task
conventions.
- Around line 367-417: Update the test-running instruction in the SDK test plan
to use the workspace’s Vitest command with the appropriate package selector
instead of Bun’s built-in runner, while keeping the referenced test directory
and coverage unchanged.
- Around line 136-144: Update the ExecutionResult interface to preserve the
complete runtime execution result, including operations, artifacts, logs,
errors, and duration, in addition to the existing findings, policyResult,
verdict, and output fields. Prefer nesting the runtime result when an existing
runtime-result type is available, or otherwise expose all required fields
directly so SDK callers can inspect delegated execution details.
In `@specs/10-cli/spec.md`:
- Around line 183-185: Resolve the CLI contract inconsistency for inspect by
either defining inspect --json as an alias for the global --format json option,
including matching behavior and tests, or removing --json from the inspect
command documentation and related tests. Keep the command reference and option
definitions consistent.
- Around line 359-439: Update the test execution instruction in the CLI test
plan to use the project’s Vitest workspace command with the appropriate package
selector instead of “bun test.” Leave the listed CLI behavior and test location
unchanged.
In `@specs/11-checks/spec.md`:
- Line 384: Remove the stray Markdown code fence at the end of the specification
so the document does not start an unterminated code block.
- Around line 221-227: Update PluginProposeRule.image and the associated
resolve() flow so plugin images cannot remain mutable tags: either validate that
provided images use an `@sha256`: digest or resolve every image to a digest-pinned
ResolvedCheck.image before execution. Preserve optional image behavior while
ensuring any configured image is immutable.
In `@specs/12-compiler-github/spec.md`:
- Line 259: Remove the trailing Markdown fence at the end of the specification
so it no longer starts an unintended code block. Leave the preceding document
content unchanged.
- Around line 141-142: Update the workflow generation represented by the install
and execute steps to use the configured concrete Sverka version from
GithubCompilerConfig.sverkaVersion or PlanMetadata.sverkaVersion instead of
sverka@latest; if neither provides a version, fail compilation rather than
emitting an unpinned workflow.
- Around line 137-142: Add a pinned oven-sh/setup-bun action step before the bun
install command in the generated workflow, ensuring Bun is available regardless
of the runner label accepted by GithubCompilerConfig.runner.
In `@specs/13-compiler-gitlab/spec.md`:
- Around line 195-197: Update the custom config.stages handling to validate it
against all stages generated from plan check categories; when a derived stage is
missing, reject the configuration or deterministically append the required
stage, unless an explicit category-to-stage override defines the mapping. Ensure
native output never contains a job stage absent from the configured stages list.
- Around line 120-127: The generated GitLab job configuration around the image
and before_script must provide Bun for every thin and native output mode.
Replace node:24 with a Bun-capable image, or add Bun installation before the
existing global sverka installation, and validate both generated modes use the
declared image while retaining the bun-based commands.
In `@specs/14-website/spec.md`:
- Around line 137-161: The website navigation structure must not reference
undeclared routes. Update the links in the User Documentation and Agentic
Documentation sections to use declared or rendered documentation routes, or add
matching route declarations for each target such as /docs/workflow-api and
/docs/cli before publishing these links.
In `@specs/15-documentation/spec.md`:
- Line 348: Update the repository tree code fence in the documentation
specification to declare the text language by adding the text fence annotation,
while preserving the existing tree content.
- Around line 248-254: Update the taxonomy entries around “Package Map” and the
additional affected entries to use the repository’s actual documentation paths:
engdocs/architecture/dependencies.md, engdocs/adr/ADR-003-canonical-plan-ir.md,
and engdocs/contributing/development-setup.md. Keep each entry’s slug and
metadata consistent with its corrected path so the taxonomy builder resolves
every referenced page.
In `@website/src/layouts/Base.astro`:
- Line 28: Update the og:url meta tag in the Base layout to derive its value
from the configured site URL and current route, rather than using the hardcoded
https://sverka.dev value. Preserve the canonical origin while appending the
active route path.
In `@website/src/pages/getting-started.astro`:
- Around line 4-13: Ensure the documented install and commands are backed by an
implemented CLI contract: add a `bin` entry for `sverka` in the `@sverka/cli`
package and implement the documented `init`, `run`, `plan --explain`, and
`compile --target github|gitlab` commands in `packages/cli/src/index.ts`;
alternatively remove these examples from both pages if the CLI will not be
implemented.
In `@website/src/styles/global.css`:
- Line 8: Update the --font-sans declaration to use lowercase, unquoted
font-family keywords for BlinkMacSystemFont and Roboto, while preserving the
existing font stack order and fallback values.
---
Outside diff comments:
In `@specs/14-website/spec.md`:
- Line 244: Remove the stray Markdown fence at the end of
specs/14-website/spec.md so it does not start an unintended code block.
🪄 Autofix
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: 5393097b-4378-4911-8cb3-fde93f5703c5
⛔ Files ignored due to path filters (3)
bun.lockis excluded by!**/*.lockwebsite/bun.lockis excluded by!**/*.lockwebsite/public/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (179)
.agents/skills/beads/SKILL.md.agents/skills/beads/agents/openai.yaml.claude/settings.json.codacy.yml.codex/config.toml.codex/hooks.json.gitignoreAGENTS.mdCLAUDE.mdLICENSEREADME.mdagents/architect/agent.tomlagents/architect/prompt.template.mdagents/builder/agent.tomlagents/builder/prompt.template.mdagents/control-dispatcher/agent.tomlagents/mayor/agent.tomlagents/mayor/prompt.template.mdagents/reviewer/agent.tomlagents/reviewer/prompt.template.mdbunfig.tomlcity.tomlengdocs/README.mdengdocs/adr/ADR-000-typescript-nx-monorepo.mdengdocs/adr/ADR-001-bun-package-manager.mdengdocs/adr/ADR-002-tsdown-build.mdengdocs/adr/ADR-003-canonical-plan-ir.mdengdocs/adr/ADR-004-thin-wrapper-ci-compiler.mdengdocs/adr/ADR-005-predecessor-reference-resolution.mdengdocs/architecture/dependencies.mdengdocs/architecture/overview.mdengdocs/contributing/development-setup.mdengdocs/contributing/guide.mdengdocs/contributing/waves.mdeslint.config.mjsformulas/sverka-bootstrap.tomlformulas/sverka-wave.tomlnx.jsonpack.tomlpackage.jsonpackages/checks/package.jsonpackages/checks/project.jsonpackages/checks/src/index.tspackages/checks/tsconfig.jsonpackages/checks/tsdown.config.tspackages/cli/package.jsonpackages/cli/project.jsonpackages/cli/src/index.tspackages/cli/tsconfig.jsonpackages/cli/tsdown.config.tspackages/compiler-earthly/package.jsonpackages/compiler-earthly/project.jsonpackages/compiler-earthly/src/index.tspackages/compiler-earthly/tsconfig.jsonpackages/compiler-earthly/tsdown.config.tspackages/compiler-github/package.jsonpackages/compiler-github/project.jsonpackages/compiler-github/src/index.tspackages/compiler-github/tsconfig.jsonpackages/compiler-github/tsdown.config.tspackages/compiler-gitlab/package.jsonpackages/compiler-gitlab/project.jsonpackages/compiler-gitlab/src/index.tspackages/compiler-gitlab/tsconfig.jsonpackages/compiler-gitlab/tsdown.config.tspackages/core/package.jsonpackages/core/project.jsonpackages/core/src/__tests__/composables/parallel.test.tspackages/core/src/__tests__/composables/pipeline.test.tspackages/core/src/__tests__/composables/run.test.tspackages/core/src/__tests__/composables/when.test.tspackages/core/src/__tests__/composables/workflow.test.tspackages/core/src/__tests__/composition.test.tspackages/core/src/__tests__/conditions.test.tspackages/core/src/__tests__/dag.test.tspackages/core/src/__tests__/errors.test.tspackages/core/src/__tests__/helpers/runtime.tspackages/core/src/__tests__/laziness.test.tspackages/core/src/__tests__/matrix.test.tspackages/core/src/__tests__/public-api.test.tspackages/core/src/__tests__/runtime-modes.test.tspackages/core/src/composables/matrix.tspackages/core/src/composables/parallel.tspackages/core/src/composables/pipeline.tspackages/core/src/composables/run.tspackages/core/src/composables/when.tspackages/core/src/composables/workflow.tspackages/core/src/errors.tspackages/core/src/index.tspackages/core/src/internal/conditions.tspackages/core/src/internal/ids.tspackages/core/src/internal/merge.tspackages/core/src/internal/node.tspackages/core/src/internal/plan.tspackages/core/src/operation.tspackages/core/src/runtime.tspackages/core/tsconfig.jsonpackages/core/tsdown.config.tspackages/findings/package.jsonpackages/findings/project.jsonpackages/findings/src/index.tspackages/findings/tsconfig.jsonpackages/findings/tsdown.config.tspackages/ir/package.jsonpackages/ir/project.jsonpackages/ir/src/index.tspackages/ir/tsconfig.jsonpackages/ir/tsdown.config.tspackages/planner/package.jsonpackages/planner/project.jsonpackages/planner/src/index.tspackages/planner/tsconfig.jsonpackages/planner/tsdown.config.tspackages/policy/package.jsonpackages/policy/project.jsonpackages/policy/src/index.tspackages/policy/tsconfig.jsonpackages/policy/tsdown.config.tspackages/runtime-docker/package.jsonpackages/runtime-docker/project.jsonpackages/runtime-docker/src/index.tspackages/runtime-docker/tsconfig.jsonpackages/runtime-docker/tsdown.config.tspackages/runtime-host/package.jsonpackages/runtime-host/project.jsonpackages/runtime-host/src/index.tspackages/runtime-host/tsconfig.jsonpackages/runtime-host/tsdown.config.tspackages/runtime-podman/package.jsonpackages/runtime-podman/project.jsonpackages/runtime-podman/src/index.tspackages/runtime-podman/tsconfig.jsonpackages/runtime-podman/tsdown.config.tspackages/runtime-remote/package.jsonpackages/runtime-remote/project.jsonpackages/runtime-remote/src/index.tspackages/runtime-remote/tsconfig.jsonpackages/runtime-remote/tsdown.config.tspackages/runtime/package.jsonpackages/runtime/project.jsonpackages/runtime/src/index.tspackages/runtime/tsconfig.jsonpackages/runtime/tsdown.config.tspackages/sdk/package.jsonpackages/sdk/project.jsonpackages/sdk/src/index.tspackages/sdk/tsconfig.jsonpackages/sdk/tsdown.config.tsspecs/00-overview/spec.mdspecs/01-core/plan.mdspecs/01-core/spec.mdspecs/02-ir/spec.mdspecs/03-runtime/spec.mdspecs/04-runtime-docker/spec.mdspecs/05-runtime-host/spec.mdspecs/06-planner/spec.mdspecs/07-findings/spec.mdspecs/08-policy/spec.mdspecs/09-sdk/spec.mdspecs/10-cli/spec.mdspecs/11-checks/spec.mdspecs/12-compiler-github/spec.mdspecs/13-compiler-gitlab/spec.mdspecs/14-website/spec.mdspecs/15-documentation/spec.mdtsconfig.base.jsontsconfig.jsonwebsite/.astro/content-assets.mjswebsite/.astro/content-modules.mjswebsite/.astro/content.d.tswebsite/.astro/types.d.tswebsite/astro.config.mjswebsite/package.jsonwebsite/src/layouts/Base.astrowebsite/src/pages/docs.astrowebsite/src/pages/getting-started.astrowebsite/src/pages/index.astrowebsite/src/styles/global.csswebsite/tsconfig.json
d6449b9 to
2193d13
Compare
🤖 CodeAnt AI — Review Status
|
PR Summary by QodoWave 1: Add @sverka/core workflow graph, planner, and runtime modes
AI Description
Diagram
High-Level Assessment
Files changed (72)
|
There was a problem hiding this comment.
Pull Request Overview
The current implementation fails to meet the core architectural requirements defined in ADR-006. Specifically, the ID generation logic uses non-deterministic strings instead of the required content-addressed SHA-256 hashes, which compromises cache stability and plan reproducibility. Additionally, the PR is missing the internal/canonical.ts module required for stable serialization, as outlined in the implementation plan. Codacy analysis indicates this PR is not up to standards due to high cyclomatic complexity in the expression lexer and planning logic. While the functional requirements for workflow composition and execution planning (run, pipeline, matrix, etc.) are implemented, these architectural and quality issues must be resolved before merging.
About this PR
- ID generation significantly diverges from ADR-006 and the core specification. The use of human-readable strings with monotonic counters instead of content-addressed SHA-256 hashes breaks reproducibility and the distributed caching model.
- The implementation of
packages/core/src/internal/canonical.tsis missing from this PR. This module is essential for deterministic serialization of operations, which is a prerequisite for valid content-addressed ID generation.
Test suggestions
- Cyclic dependency detection in the workflow graph
- Matrix expansion into cartesian product nodes with injected environment variables
- Condition expression parsing and evaluation with operator precedence (NOT > AND > OR)
- Verification of laziness: no side effects (I/O) during workflow definition or planning
- Immutability of operations: composition methods return new instances and do not mutate originals
- Topological sorting respects dependency edges (predecessors before successors)
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
There was a problem hiding this comment.
Actionable comments posted: 17
🤖 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 `@packages/core/src/__tests__/conditions.test.ts`:
- Around line 87-96: Restructure the test around evaluateCondition so the
sentinel failure cannot be caught by the assertion block. Narrow the caught
value from unknown with an appropriate instanceof CompositionError check, then
assert its code and context without type casts; ensure the test fails clearly
when evaluateCondition does not throw.
In `@packages/core/src/__tests__/dag.test.ts`:
- Around line 19-25: Replace the unsafe CompositionError casts in both catch
blocks of the DAG tests with explicit narrowing from unknown, using an
appropriate CompositionError type guard or validated instanceof check before
accessing context. Ensure the tests fail for unrelated error types while
preserving the existing cycle-context assertions.
- Around line 9-40: Add a test in the “DAG validation” suite, before the cycle
or duplicate-ID cases, covering a workflow whose operation uses dependsOn with
an unresolved ID such as “ghost”; assert that wf.plan(makePlanRuntime()) rejects
with CompositionError and an error message containing the unknown dependency ID.
In `@packages/core/src/__tests__/matrix.test.ts`:
- Around line 31-34: Update the assertions in the matrix operations test to
verify each spec’s MATRIX_NODE and MATRIX_OS values match the expected pairing
encoded by its operation ID suffix, rather than only checking that they are
defined. Preserve the existing product-size assertions and ensure incorrect or
duplicated environment mappings fail.
- Around line 43-47: Update the test iterating over result.operations to first
collect entries whose IDs start with "run:test[" and assert that at least one
matching operation exists, then verify each matching operation depends on
["run:build"].
In `@packages/core/src/internal/ids.ts`:
- Around line 40-53: Update matrixChildId and formatMatrixValue so delimiter
characters in matrix keys and values are escaped before constructing the ID,
preventing commas and equals signs from changing component boundaries. Ensure
objects and null are serialized distinctly rather than both relying on ambiguous
String(v) output, while preserving existing IDs for values without delimiters.
- Around line 30-34: Update derivedBase() and its collision-suffix handling so
unnamed operations derive IDs from stable node content rather than the discovery
index, preserving IDs when unrelated roots alter traversal order; alternatively,
document this discovery-order limitation in the assignId documentation if
reproducibility is not required.
In `@packages/core/src/internal/plan.ts`:
- Around line 349-357: Remove the duplicated unknown-dependency validation from
topoSort, preserving the single CompositionError check in detectCycles. Update
the planner pipeline to validate dependencies once before cycle detection, using
a separate validateDeps(specs) step if needed, while keeping topological sorting
focused on ordering.
- Around line 160-174: Update cartesianProduct to enforce a configurable maximum
combination count before materializing combinations, throwing CompositionError
when the limit is exceeded. Reuse the existing configuration/error-context
mechanisms and include each dimension’s key and size in the error context;
preserve the current product behavior when within the limit.
- Around line 121-158: Reduce expandMatrices() cognitive complexity by
extracting dimension checks into validateDims(dims) and matrix child creation
into buildMatrixChild(node, combo). Update expandMatrices() to call these
helpers while preserving the existing validation errors, environment variables,
marker removal, and __matrixCombo metadata.
- Around line 330-362: Replace the recursive visit function in detectCycles with
an iterative depth-first traversal using an explicit stack of operation frames,
preserving gray/black coloring, cycle-path reporting, unknown-dependency
validation, and stack cleanup. Ensure chains of arbitrary supported length do
not use the JavaScript call stack and all graph validation failures remain
CompositionError instances.
- Around line 146-155: Remove the __matrixCombo mutation in expandMatrices and
have it return the expanded nodes together with a Map<OperationNode, readonly
[string, unknown][]> keyed by each child and its combo. Update assignIds and its
callers to accept and read this map instead of using the double-cast hidden
property, preserving matrix-specific ID assignment.
- Around line 47-64: Update the operation loop around runtime.evaluate to stop
after a "failure" or "cancelled" outcome unless the failed OperationSpec has
continueOnError enabled. For each unreached operation, append a cancelled
OperationOutcome rather than evaluating it, while preserving condition-based
"skipped" outcomes and normal execution for permitted continuation.
- Around line 62-73: Ensure planWorkflow always invokes runtime.finalize() when
any runtime.evaluate() call rejects. Wrap the evaluation loop and finalization
flow in try/finally, preserving rejection propagation while guaranteeing
cleanup; keep successful outcome collection and finalized return behavior
unchanged.
- Around line 294-324: Refactor buildSpec() to eliminate the repeated
conditional spreads for optional fields by copying those keys through a shared
list or equivalent iteration. Preserve all existing field names, omission
behavior for undefined values, and required fields; adding a new optional field
should require only one list entry.
In `@packages/core/src/runtime.ts`:
- Around line 12-20: Make outcomes required on the RuntimeResult interface, then
update every Runtime.finalize() implementation to always return an outcomes
array, using an empty array when there are no outcomes so planWorkflow() can
overwrite it.
- Around line 63-64: Define and export a dedicated type for the runtime-supplied
portion of RuntimeResult, excluding operations, outcomes, and durationMs. Update
the Runtime.finalize() method to return this narrower type, adjust
planWorkflow() to compose the final RuntimeResult from it, and re-export the new
public type through the package src/index.ts entry point.
🪄 Autofix
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: 0d9b1e07-1638-4eb9-b822-eb62e880b275
📒 Files selected for processing (19)
.codacy.yml.eslintrc.json.gitignoreCLAUDE.mdREADME.mdengdocs/README.mdengdocs/architecture/overview.mdpackages/cli/src/index.tspackages/core/src/__tests__/conditions.test.tspackages/core/src/__tests__/dag.test.tspackages/core/src/__tests__/matrix.test.tspackages/core/src/internal/ids.tspackages/core/src/internal/plan.tspackages/core/src/runtime.tspackages/policy/src/index.tspackages/runtime/src/index.tsspecs/00-overview/spec.mdspecs/01-core/spec.mdspecs/15-documentation/spec.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
specs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Write numbered, structured specifications before implementation as part of spec-driven development.
Files:
specs/00-overview/spec.mdspecs/15-documentation/spec.mdspecs/01-core/spec.md
**/*.{ts,tsx,js,jsx,json,md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier for formatting.
Files:
specs/00-overview/spec.mdpackages/core/src/__tests__/conditions.test.tspackages/policy/src/index.tspackages/core/src/__tests__/dag.test.tspackages/core/src/internal/ids.tsengdocs/architecture/overview.mdengdocs/README.mdpackages/core/src/__tests__/matrix.test.tspackages/cli/src/index.tsCLAUDE.mdpackages/runtime/src/index.tsspecs/15-documentation/spec.mdpackages/core/src/runtime.tspackages/core/src/internal/plan.tsREADME.mdspecs/01-core/spec.md
specs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Write specifications first in the
specs/directory using numbered, structured documents (SDD).
Files:
specs/00-overview/spec.mdspecs/15-documentation/spec.mdspecs/01-core/spec.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript and never useany; useunknownwith appropriate narrowing instead.
Use TypeScript with ESM-compatible module conventions.
**/*.{ts,tsx}: Do not useanyin TypeScript; useunknownand narrow it. Maintain strict TypeScript practices.
Use custom error classes for package-specific error handling.
Files:
packages/core/src/__tests__/conditions.test.tspackages/policy/src/index.tspackages/core/src/__tests__/dag.test.tspackages/core/src/internal/ids.tspackages/core/src/__tests__/matrix.test.tspackages/cli/src/index.tspackages/runtime/src/index.tspackages/core/src/runtime.tspackages/core/src/internal/plan.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{ts,tsx}: Write tests before implementation and use Vitest for testing.
Run tests with Vitest via the project'sbun run testcommand; do not confuse it with Bun's built-inbun testrunner.
Files:
packages/core/src/__tests__/conditions.test.tspackages/core/src/__tests__/dag.test.tspackages/core/src/__tests__/matrix.test.ts
**/src/index.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Export everything that is public from the package's
src/index.tsentry point.
Files:
packages/policy/src/index.tspackages/cli/src/index.tspackages/runtime/src/index.ts
**/src/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Export everything public from each package's
src/index.tsentry point.
Files:
packages/policy/src/index.tspackages/cli/src/index.tspackages/runtime/src/index.ts
engdocs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Create engineering documentation before implementing code when the work requires engineering documentation.
Files:
engdocs/architecture/overview.mdengdocs/README.md
engdocs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Create engineering documentation in
engdocs/before implementing code (document-first development).
Files:
engdocs/architecture/overview.mdengdocs/README.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: sverka-dev/sverka
Timestamp: 2026-08-10T22:43:09.173Z
Learning: Use Nx to orchestrate monorepo builds, tests, linting, and type-checking.
Learnt from: CR
Repo: sverka-dev/sverka
Timestamp: 2026-08-10T22:43:09.173Z
Learning: Organize work in waves using the architect, builder, and reviewer roles.
Learnt from: CR
Repo: sverka-dev/sverka
Timestamp: 2026-08-10T22:43:09.173Z
Learning: All work flows through the mayor agent, with multi-step orchestration defined in `formulas/`.
Learnt from: CR
Repo: sverka-dev/sverka
Timestamp: 2026-08-10T22:43:09.173Z
Learning: Use `bd` (Beads) for all task tracking; do not use TodoWrite, TaskCreate, markdown TODO lists, or ad hoc memory files.
Learnt from: CR
Repo: sverka-dev/sverka
Timestamp: 2026-08-10T22:43:09.173Z
Learning: Do not override the configured `DEVIN_MODEL=glm-5-2` with a paid model.
Learnt from: CR
Repo: sverka-dev/sverka
Timestamp: 2026-08-10T22:43:09.173Z
Learning: Run relevant quality gates after code changes, update issue status, and report changed files, validation, and blocked sync or commit steps at handoff.
Learnt from: CR
Repo: sverka-dev/sverka
Timestamp: 2026-08-10T22:43:09.173Z
Learning: Do not commit or push changes without explicit authority from the active profile or user request.
Learnt from: CR
Repo: sverka-dev/sverka
Timestamp: 2026-08-10T22:43:15.729Z
Learning: Use `bd` for all task tracking; do not use TodoWrite, TaskCreate, markdown TODO lists, or `MEMORY.md` files. Run `bd prime` for workflow details.
Learnt from: CR
Repo: sverka-dev/sverka
Timestamp: 2026-08-10T22:43:15.729Z
Learning: At session completion, file follow-up issues, run applicable quality gates, update issue status, and report changes, validation, and blocked sync steps. Do not commit or push without explicit authority.
Learnt from: CR
Repo: sverka-dev/sverka
Timestamp: 2026-08-10T22:43:15.729Z
Learning: Use the documented build and test commands: `bun install`, `bun run build`, `bun run test`, `bun run lint`, and `bun run typecheck` as applicable.
Learnt from: CR
Repo: sverka-dev/sverka
Timestamp: 2026-08-10T22:43:15.729Z
Learning: Write tests before implementation (TDD).
🪛 GitHub Check: SonarCloud Code Analysis
packages/core/src/internal/plan.ts
[warning] 285-285: Use .includes() instead of .some() when checking value existence.
[warning] 147-147: The empty object is useless.
[failure] 121-121: Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
[failure] 294-294: Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
🪛 markdownlint-cli2 (0.23.2)
specs/01-core/spec.md
[warning] 380-380: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 441-441: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (28)
packages/policy/src/index.ts (1)
2-16: LGTM!packages/cli/src/index.ts (1)
2-16: LGTM!packages/runtime/src/index.ts (1)
2-16: LGTM!packages/core/src/internal/plan.ts (2)
187-213: 🗄️ Data Integrity & IntegrationVerify predecessor identity after
flattenArtifacts()rebuilds a node.Line 209 returns a replacement node. Any other node that still holds the original node in its
predecessorsarray keeps the old identity, andidMapat line 36 contains only the replacement.resolveEdges()at line 284 then fails withunresolved predecessor reference. The previous review raised this and the author deferred the fix. The same concern applies to matrix templates removed at line 146.Run the script to confirm whether any current test reaches this path, so the deferral stays intentional and covered.
#!/bin/bash # Description: Find tests that chain operations after a join or a matrix template. set -eu rg -n -C 6 --type=ts 'join\(|parallel\(|matrix\(' packages/core/src/__tests__ | rg -n -C 6 '\.after\('
84-114: LGTM!Also applies to: 236-292
packages/core/src/runtime.ts (1)
26-30: LGTM!Also applies to: 38-46, 67-75
packages/core/src/internal/ids.ts (1)
14-28: LGTM!Also applies to: 56-71
packages/core/src/__tests__/conditions.test.ts (1)
6-85: LGTM!packages/core/src/__tests__/dag.test.ts (1)
42-72: LGTM!Also applies to: 74-97
packages/core/src/__tests__/matrix.test.ts (1)
8-17: LGTM!Also applies to: 50-65
specs/01-core/spec.md (7)
184-192: AlignPlanContextwith its documented value domain.The prose permits arrays of primitives, but
PlanContextpermits onlyreadonly string[]. Anumber[]orboolean[]context is rejected by the public type. Narrow the prose or expand the index signature. Keep the type and prose consistent.
369-372: Document list-merge deduplication.The composition section says arrays only concatenate, while predecessor resolution and
packages/core/src/internal/merge.tsremove duplicates. State concatenation, deduplication, and order preservation in both sections.Also applies to: 535-536
374-419: Resolve the operation identity contract.
contextincludes discoveryindex, so otherwise identical operations at different positions cannot produce the same{ kind, name, context }ID. The documented duplicate check cannot detect repeated operations across positions. The projection also omits fields such asenv,image,imageDigest,workingDir,timeoutSeconds,retries,cache,network,credentials,artifacts, andcondition. Define index participation and the complete projection, then align core, IR, and tests.Also applies to: 538-552
376-376: Resolve the ADR-006 link.The link at Line [376] targets
../../engdocs/adr/ADR-006-sha256-content-addressed-plan-ids.md, but the supplied stack does not contain that file. Add the ADR or update the link.
380-380: Add language tags to the remaining Markdown fences.Add
textto the ID-format fence at Line [380] and the expression-grammar fence at Line [441]. Static analysis still reports MD040.Also applies to: 441-441
Source: Linters/SAST tools
516-516: Use the configured Vitest commands.Replace
bun testat Lines [516] and [576] withbun run test. Usebunx nx run core:testfor the core-only command.bun testinvokes Bun’s built-in runner, not the repository’s Nx/Vitest command.Based on learnings, repository tests use
bun run testand Nx/Vitest commands.Also applies to: 574-579
Source: Learnings
49-62: LGTM!Also applies to: 168-182, 205-206, 318-318, 347-368, 420-433, 434-440, 442-462, 527-534, 554-568
engdocs/README.md (2)
33-34: Resolve the ADR-006 link before merging.The new entry at Line [34] points to
./adr/ADR-006-sha256-content-addressed-plan-ids.md, but the supplied stack does not contain that file. Add the ADR or update the link.
10-10: LGTM!.gitignore (2)
157-165: Verify the Astro metadata exclusion.The previous review found that
website/.astro/was still tracked. Confirm that the website exclusions now includewebsite/.astro/and that no files under that directory remain tracked.Verification
#!/usr/bin/env bash set -euo pipefail rg -n 'website/\.astro' .gitignore git ls-files -- 'website/.astro/**'
166-168: LGTM!specs/15-documentation/spec.md (1)
348-348: LGTM!specs/00-overview/spec.md (1)
65-65: LGTM!engdocs/architecture/overview.md (1)
8-8: LGTM!.codacy.yml (1)
1-70: LGTM!CLAUDE.md (1)
63-69: LGTM!Also applies to: 71-84
README.md (1)
84-84: LGTM!Also applies to: 141-141, 158-158
.eslintrc.json (1)
1-105: 🎯 Functional CorrectnessNo change is required in
.eslintrc.json. The active ESLint 9 runner useseslint.config.mjs, and Codacy excludes JavaScript and TypeScript source files. The rules in.eslintrc.jsoncannot cause an undefined-rule failure in these runners.> Likely an incorrect or invalid review comment.
… NaN/Infinity Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
|



User description
Summary
anyTest plan
Generated with Devin
CodeAnt-AI Description
Add the core workflow graph API for composing, planning, and executing workflows
What Changed
MATRIX_*environment values.Impact
✅ Lazy workflow definitions without setup-time side effects✅ Reliable sequential, parallel, conditional, and matrix plans✅ Clear skipped and cancelled operation outcomes🔄 Retrigger CodeAnt AI Review
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.