Skip to content

v0 Wave G: planner — Run Plan binding - #42

Merged
ThePlenkov merged 6 commits into
v0-f-enginefrom
v0-g-planner
Aug 13, 2026
Merged

ThePlenkov merged 6 commits into
v0-f-enginefrom
v0-g-planner

Conversation

@ThePlenkov

@ThePlenkov ThePlenkov commented Aug 13, 2026 •

Copy link
Copy Markdown
Contributor

User description

Summary

  • Rebuild @sverka/planner with bindRunPlan: binds a Definition Graph's Entry + user inputs into a concrete RunPlan for the native engine
  • computeReachableSteps: transitive closure from entry roots (both directions)
  • PlannerError: ENTRY_NOT_FOUND, ROOT_NOT_FOUND, MISSING_INPUT, INVALID_GRAPH
  • Reuses existing discovery + plan synthesis logic unchanged
  • Fixed cross-cutting bug: PipelineDefinition.inputs changed from Input[] to Record<string, Input> to preserve input names through synthesis. Updated core, IR validation, and test fixtures.

Test plan

  • planner: 58 tests pass (17 new bind tests + 41 existing)
  • All 7 affected packages green: 229 tests total
  • typecheck/lint/build clean
  • No any types

Generated with Devin


Summary by cubic

Produces a deterministic RunPlan from a selected entry in @sverka/planner, with fast-fail graph validation and secrets-safe plan IDs. Old: no binder. New: bindRunPlan computes reachable steps, binds inputs, and omits secret values from the plan and its ID.

  • Validates runtime graph shape up front, then runs @sverka/core validateGraph on the selected pipeline; malformed graphs, cycles, and unknown dependencies[].producer raise PlannerError("INVALID_GRAPH").
  • Finds the entry across all pipelines; unknown entry/root raise ENTRY_NOT_FOUND/ROOT_NOT_FOUND.
  • Reachability follows only dependencies[].producer backward from roots and excludes unrelated steps.
  • Inputs are Record<string, Input>; defaults merge with user overrides with strict type checks. Type mismatches raise INVALID_INPUT; missing required values raise MISSING_INPUT. Secret inputs are omitted from RunPlan.inputs and excluded from @sverka/ir computeRunPlanId.
  • Public API: Planner.bindRunPlan(...); @sverka/planner exports bindRunPlan, computeReachableSteps, and PlannerError (adds INVALID_INPUT). RunPlan includes graphId, a content-addressed id, and createdAt. @sverka/planner now depends on @sverka/core and @sverka/ir.
  • @sverka/core re-exports graph types. @sverka/ir validator enforces inputs object shape and validates type, required, secret, description, and default type consistency.

Migration

  • Replace PipelineDefinition.inputs: Input[] with Record<string, Input> and update array iterations to Object.entries/Object.keys.

Written for commit c06b5e0. Summary will update on new commits.

Review in cubic


CodeAnt-AI Description

Bind definition graphs into executable run plans

What Changed

  • Added Run Plan creation from a selected pipeline entry, including reachable steps, bound inputs, source graph identity, deterministic plan identity, and creation time
  • Includes connected dependencies and dependents while excluding unrelated steps
  • Applies default inputs, allows user values to override them, and reports missing required inputs with clear planner error codes
  • Exposes Run Plan binding and reachability utilities through the planner API, while preserving existing discovery and planning
  • Pipeline inputs are now named records, preserving input names through graph synthesis and validation

Impact

✅ Executable plans from selected pipeline entries
✅ Unrelated steps excluded from runs
✅ Clear errors for invalid entries and missing inputs

💡 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:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

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:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

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.

@codeant-ai

codeant-ai Bot commented Aug 13, 2026 •

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Incremental review completed adfa6db Aug 13, 2026 · 10:12 10:13
✅ Incremental review completed af258ed Aug 13, 2026 · 08:23 08:23
✅ Reviewed your PR 3e7610c Aug 13, 2026 · 00:36 00:39

@cubic-dev-ai

cubic-dev-ai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Running ultrareview automatically — This PR adds a new Run Plan binding feature and changes the public PipelineDefinition.inputs type from array to record, a cross-cutting contract change affecting core, IR, and all consumers — a subtle bug in reachability or input binding could break pipeline execution across the system.. I'll post findings when complete.

@coderabbitai

coderabbitai Bot commented Aug 13, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Added Run Plan binding for selected pipeline entries, including dependency resolution, input defaults, overrides, and generated plan metadata.
    • Added reachability utilities and planner APIs for creating executable plans.
    • Added structured planner errors with codes for missing entries, roots, inputs, and invalid graphs.
    • Expanded public planner and graph type exports.
  • Validation
    • Pipeline inputs now use name-keyed objects with validation for supported types, required fields, secrets, descriptions, and defaults.
  • Documentation
    • Added comprehensive planner specification covering behavior, APIs, errors, and testing.

Walkthrough

The change converts pipeline inputs to keyed records and adds planner RunPlan binding. The planner resolves reachable steps, binds defaults and overrides, validates inputs, creates identifiers, exposes typed errors, and publishes the new API.

Changes

Graph input schema

Layer / File(s) Summary
Keyed pipeline input schema
packages/core/src/graph.ts, packages/core/src/index.ts, packages/core/src/synthesize.ts, packages/ir/src/validate.ts, packages/core/src/__tests__/*
Pipeline inputs now use keyed Input records. Core re-exports graph types. IR validation checks supported types, metadata, flags, and matching defaults. Fixtures and assertions use keyed inputs.

Planner API and RunPlan binding

Layer / File(s) Summary
Planner contracts and public API
specs/13-planner/spec.md, packages/planner/src/errors.ts, packages/planner/src/planner.ts, packages/planner/src/index.ts, packages/planner/package.json, packages/planner/src/__tests__/public-api.test.ts
The planner specification defines RunPlan binding and reachability. The public API exports binding functions, options, and planner errors. Planner delegates binding to the implementation.
RunPlan binding and validation
packages/planner/src/bind.ts, packages/planner/src/__tests__/bind.test.ts
bindRunPlan validates entries and dependencies, resolves reachable steps, binds inputs, excludes secrets, computes identifiers, and returns a timestamped RunPlan. Tests cover binding, reachability, errors, identifiers, and timestamps.

Estimated code review effort: 4 (Complex) | ~45 minutes

Mergeability Score: 🟠 High · up to 2d323

The planner can still produce invalid run plans or crash while handling malformed inputs, and cyclic graphs may reach execution without a schedulable plan. Several documented API and validation contracts also remain inconsistent, while a configured complexity check is failing; these issues should be resolved before merge.

Sequence Diagram(s)

sequenceDiagram
  participant Planner
  participant bindRunPlan
  participant DefinitionGraph
  participant RunPlan
  Planner->>bindRunPlan: bindRunPlan(options)
  bindRunPlan->>DefinitionGraph: locate entry and resolve reachable steps
  bindRunPlan->>DefinitionGraph: resolve and validate inputs
  bindRunPlan->>RunPlan: construct identifiers and timestamped plan
  RunPlan-->>Planner: return bound RunPlan
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely identifies the planner Run Plan binding change.
Description check ✅ Passed The description directly explains Run Plan binding, reachability, input handling, errors, API changes, and test results.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch v0-g-planner

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

@codeant-ai codeant-ai Bot added the size:XL This PR changes 500-999 lines, ignoring generated files label Aug 13, 2026
@cubic-dev-ai

cubic-dev-ai Bot commented Aug 13, 2026

Copy link
Copy Markdown

I can't run this ultrareview because you've reached your trial's review limit. Trial plans have lower review limits than paid plans. Upgrade now to resume reviews.

To help optimise your usage, you can tune cubic to get the most out of your usage limits:

Learn more →

@amazon-q-developer amazon-q-developer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

This PR introduces Run Plan binding functionality for the planner package, which binds Definition Graphs with entry points and inputs into concrete RunPlans for execution. The implementation includes transitive closure computation for reachable steps, input binding with validation, and comprehensive error handling.

Critical Issue Found

Performance Regression: The computeReachableSteps function contains a critical performance issue that must be fixed before merge. The array spread operators in lines 102-103 create unnecessary copies on every iteration, resulting in O(n²) complexity for the adjacency list construction.

Changes Reviewed

  • ✅ New bindRunPlan implementation with proper validation
  • ✅ Error handling with appropriate PlannerError codes
  • ✅ Cross-cutting fix: PipelineDefinition.inputs changed from array to object (maintains input names)
  • ✅ Updated IR validation to match new schema
  • ✅ Comprehensive test coverage (58 passing tests)

The implementation is well-structured with clear separation of concerns. Once the performance issue is resolved, this will be ready to merge.


You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

Comment thread packages/planner/src/bind.ts Outdated
@baz-reviewer

baz-reviewer Bot commented Aug 13, 2026 •

Copy link
Copy Markdown

Merger

Needs Review

This is a substantial planner/API and secret-handling change, but ci_ran=false; the required green CI verification is absent. Human review is needed before merging.

Commit c06b5e0 · Evaluated 2026-08-13 21:38 UTC

Review this PR on Baz | Customize your next review

@codacy-production

codacy-production Bot commented Aug 13, 2026 •

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 duplication

Metric Results
Duplication 0

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Planner: add bindRunPlan RunPlan binding + preserve pipeline input names

✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Add bindRunPlan to bind a DefinitionGraph entry + inputs into an executable RunPlan.
• Compute reachable steps via dependency transitive closure and validate missing entry/roots/inputs.
• Preserve pipeline input names by changing PipelineDefinition.inputs to a keyed object across
 core/IR/tests.
Diagram

graph TD
  U["Caller"] --> G[("DefinitionGraph")] --> P["bindRunPlan"] --> C["computeReachableSteps"] --> I["computeGraphId + computeRunPlanId"] --> O["RunPlan"]
  P --> BI["bindInputs"] --> I
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep `inputs: Input[]` and add `name` field to Input
  • ➕ Avoids a breaking structural change from array → map in PipelineDefinition
  • ➕ Maintains stable iteration ordering without additional sorting
  • ➖ Duplicates the key in each element and increases risk of name/value divergence
  • ➖ Still requires extra validation for uniqueness and non-empty names
2. Use `inputs: Array` (tuple list) instead of `Record`
  • ➕ Preserves names through synthesis while keeping deterministic ordering
  • ➕ Allows representing duplicate names (if ever needed) explicitly
  • ➖ More awkward consumer ergonomics than Record
  • ➖ IR/schema validation and JSON shape are less conventional than an object
3. Reachability only follows producers (backward) from roots
  • ➕ Smaller reachable set and simpler traversal logic
  • ➕ Matches 'dependencies[].producer' direction more strictly
  • ➖ Would exclude downstream dependents if roots are not true entry leaves
  • ➖ More brittle if entries/root semantics evolve (e.g., roots as mid-graph anchors)

Recommendation: The chosen approach is sound for v0: bindRunPlan is a clean boundary that produces an engine-ready RunPlan with deterministic IDs, and switching pipeline inputs to Record directly fixes the name-loss issue during synthesis. If ordering becomes important for ID determinism or UX, consider defining an explicit canonical key sort during ID computation rather than reverting the data model.

Files changed (15) +593 / -29

Enhancement (6) +226 / -2
graph.tsChange PipelineDefinition.inputs to Record and re-export constructs types +5/-1

Change PipelineDefinition.inputs to Record and re-export constructs types

• Updates 'PipelineDefinition.inputs' from 'Input[]' to 'Readonly<Record<string, Input>>' to preserve input names. Re-exports constructs types via '@sverka/core' to reduce downstream direct dependencies on '@sverka/constructs'.

packages/core/src/graph.ts

index.tsExpose graph schema types from @sverka/core public API +6/-0

Expose graph schema types from @sverka/core public API

• Exports 'Input', 'OutputDeclaration', 'OutputType', 'Reference', 'Runtime', and 'Trigger' from the core package entrypoint for consumer use.

packages/core/src/index.ts

bind.tsImplement RunPlan binding (bindRunPlan) and reachability traversal +182/-0

Implement RunPlan binding (bindRunPlan) and reachability traversal

• Adds 'bindRunPlan' to validate graph/entry/roots, select reachable steps, bind pipeline inputs, compute 'graphId'/plan 'id', and set 'createdAt'. Adds 'computeReachableSteps' BFS over dependency edges in both directions and input binding logic with required/default enforcement.

packages/planner/src/bind.ts

errors.tsIntroduce PlannerError and error codes for binding +20/-0

Introduce PlannerError and error codes for binding

• Adds a dedicated 'PlannerError' class and 'PlannerErrorCode' union for bind-time failures: entry/root missing, missing input, and invalid graph.

packages/planner/src/errors.ts

index.tsExport RunPlan binding APIs from planner entrypoint +6/-1

Export RunPlan binding APIs from planner entrypoint

• Updates the planner public API surface to export 'bindRunPlan', 'computeReachableSteps', 'BindRunPlanOptions', and 'PlannerError' alongside existing discovery/plan APIs.

packages/planner/src/index.ts

planner.tsAdd bindRunPlan to Planner interface and implementation +7/-0

Add bindRunPlan to Planner interface and implementation

• Extends the 'Planner' interface and 'PlannerImpl' to expose 'bindRunPlan', delegating to the new binding implementation and returning a 'RunPlan'.

packages/planner/src/planner.ts

Bug fix (2) +6 / -3
synthesize.tsPreserve input names during pipeline synthesis +4/-1

Preserve input names during pipeline synthesis

• Changes synthesis to emit pipeline inputs as a 'Record<string, Input>' by iterating pipeline input entries, preventing name loss during graph generation.

packages/core/src/synthesize.ts

validate.tsValidate pipeline inputs as an object (not array) +2/-2

Validate pipeline inputs as an object (not array)

• Updates IR validation to require 'inputs' be a non-null object and not an array, matching the new graph schema.

packages/ir/src/validate.ts

Tests (4) +228 / -13
graph.test.tsUpdate core graph tests for inputs-as-object +3/-3

Update core graph tests for inputs-as-object

• Migrates test fixtures from 'inputs: []' to 'inputs: {}' and adjusts assertions to reflect 'PipelineDefinition.inputs' becoming a keyed object.

packages/core/src/tests/graph.test.ts

validate.test.tsUpdate synthesis validation test fixtures for inputs-as-object +1/-1

Update synthesis validation test fixtures for inputs-as-object

• Updates pipeline fixtures to use 'inputs: {}' so validation/synthesis tests match the new 'PipelineDefinition.inputs' shape.

packages/core/src/tests/validate.test.ts

bind.test.tsAdd bindRunPlan/computeReachableSteps/PlannerError unit tests +199/-0

Add bindRunPlan/computeReachableSteps/PlannerError unit tests

• Introduces focused tests for RunPlan binding, reachability closure behavior, input default/override/required handling, deterministic IDs, and PlannerError code/cause semantics.

packages/planner/src/tests/bind.test.ts

public-api.test.tsVerify new planner public API exports +25/-9

Verify new planner public API exports

• Extends public API tests to assert 'bindRunPlan', 'computeReachableSteps', 'PlannerError', and related types/codes are exported and usable.

packages/planner/src/tests/public-api.test.ts

Documentation (1) +125 / -11
spec.mdActivate Spec 13 with RunPlan binding behavior and error model +125/-11

Activate Spec 13 with RunPlan binding behavior and error model

• Replaces the stub with an active specification describing 'bindRunPlan', reachability, input binding, ID computation, error codes, exports, and a concrete test plan aligned to the implementation.

specs/13-planner/spec.md

Other (2) +8 / -0
bun.lockAdd planner workspace dependencies in lockfile +4/-0

Add planner workspace dependencies in lockfile

• Records new workspace dependencies for '@sverka/planner' on '@sverka/core' and '@sverka/ir' to support RunPlan binding and ID computation.

bun.lock

package.jsonAdd planner runtime deps on core and IR +4/-0

Add planner runtime deps on core and IR

• Adds '@sverka/core' and '@sverka/ir' as dependencies so the planner can bind graphs and compute graph/run plan IDs.

packages/planner/package.json

Comment thread packages/ir/src/validate.ts
Comment thread packages/planner/src/bind.ts Outdated
Comment thread packages/planner/src/bind.ts
Comment thread packages/planner/src/bind.ts Outdated

@codacy-production codacy-production Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

The current implementation introduces a significant discrepancy with Spec 13 regarding reachability logic; the planner currently traverses both producers and dependents, whereas the specification mandates following only producer edges (backward traversal). Additionally, the reachability calculation lacks validation for referenced producers, which could lead to runtime failures if the graph contains broken references.

Codacy analysis indicates the PR is not up to standards, primarily due to high cyclomatic complexity in packages/planner/src/bind.ts. The computeReachableSteps function alone has a complexity of 18. Refactoring this logic into modular helpers is required to improve maintainability and ensure the core BFS logic is verifiable.

About this PR

  • Discrepancy in reachability logic: The implementation and PR description specify a 'both directions' (forward and backward) transitive closure, whereas the updated Spec 13 (§ Data models) specifies following only 'dependencies[].producer' edges (backward).
1 comment outside of the diff
packages/ir/src/validate.ts

line 118 ⚪ LOW RISK
Suggestion: This validation function is reaching the complexity threshold. While currently readable, consider using a loop for checking required array properties to reduce the branching factor.

function validatePipelineStructure(value: unknown): void {
  if (typeof value !== "object" || value === null) {
    throw new ValidationError("invalid pipeline: expected object");
  }
  const p = value as Record<string, unknown>;
  if (typeof p.id !== "string") throw new ValidationError("invalid pipeline: missing 'id'");
  if (typeof p.inputs !== "object" || p.inputs === null || Array.isArray(p.inputs)) {
    throw new ValidationError("invalid pipeline: missing 'inputs' object");
  }
  for (const key of ["entries", "steps", "outputs"]) {
    if (!Array.isArray(p[key])) throw new ValidationError(`invalid pipeline: missing '${key}' array`);
  }
  for (const step of p.steps as any[]) {
    validateStepStructure(step);
  }
}

Test suggestions

  • bindRunPlan produces a RunPlan for a single-step graph
  • bindRunPlan includes all reachable steps and excludes unrelated steps
  • bindRunPlan resolves inputs correctly using defaults and user overrides
  • bindRunPlan throws MISSING_INPUT for required inputs without a value
  • bindRunPlan throws ENTRY_NOT_FOUND or ROOT_NOT_FOUND for invalid references
  • bindRunPlan generates a deterministic ID excluding volatile fields
  • computeReachableSteps handles transitive closure and diamond dependencies without duplicates
  • computeReachableSteps returns empty list for empty roots

TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback

Comment thread packages/planner/src/bind.ts Outdated
Comment thread packages/planner/src/bind.ts
Comment thread packages/planner/src/bind.ts
@qodo-code-review

qodo-code-review Bot commented Aug 13, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Unknown producers pass binding ✓ Resolved 🐞 Bug ≡ Correctness
Description
Reachability queues nonexistent producer IDs and later filters them out, leaving dependent steps in
the RunPlan instead of rejecting the graph. The engine then ignores the missing edge during sorting
but cannot mark the dependent ready, and can report success without executing it.
Code

packages/planner/src/bind.ts[43]

+  const reachableSteps = computeReachableSteps(pipeline.steps, entry.roots);
Relevance

●●● Strong

PR/spec explicitly promises validation of producer IDs; missing-producer should be rejected as
INVALID_GRAPH.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The planner specification requires valid producers. The traversal accepts arbitrary producer IDs and
removes them only from the output; the scheduler ignores absent producers for indegree calculation
while readiness requires every producer to have succeeded, and the engine's no-work path retains
hasFailure === false.

specs/13-planner/spec.md[26-29]
packages/planner/src/bind.ts[100-125]
packages/engine-native/src/scheduler.ts[31-36]
packages/engine-native/src/scheduler.ts[96-105]
packages/engine-native/src/engine.ts[140-195]
packages/engine-native/src/engine.ts[215-216]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`bindRunPlan` can emit a RunPlan containing a step whose declared producer is absent.

## Issue Context
Validate producer existence for every reachable step and throw `PlannerError` with `INVALID_GRAPH` before computing or returning the plan.

## Fix Focus Areas
- packages/planner/src/bind.ts[39-46]
- packages/planner/src/bind.ts[90-125]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Malformed input definitions accepted ✓ Resolved 🐞 Bug ☼ Reliability
Description
The revised schema check accepts any non-array object as pipeline.inputs, including {env: null},
and asserts it is a valid graph. Binding such a deserialized graph dereferences input.default and
throws a raw TypeError rather than rejecting invalid input schema.
Code

packages/ir/src/validate.ts[R126-127]

+  if (typeof p.inputs !== "object" || p.inputs === null || Array.isArray(p.inputs)) {
+    throw new ValidationError("invalid pipeline: missing 'inputs' object");
Relevance

●●● Strong

They’ve accepted stricter schema/shape validation to avoid runtime crashes and silent bad data.

PR-#34

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Each input must have a valid Input shape, but deserialization returns immediately after validators
that never inspect the record values; the new binder then directly dereferences every value.

packages/constructs/src/model.ts[87-95]
packages/ir/src/serialize.ts[47-59]
packages/ir/src/validate.ts[118-141]
packages/core/src/validate.ts[161-168]
packages/planner/src/bind.ts[157-167]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Graph deserialization validates only the `inputs` container, not its values, allowing malformed input declarations into typed code.

## Issue Context
Validate every record value as an `Input`, including `type`, optional field primitive types, and default/type compatibility, before asserting `DefinitionGraph`.

## Fix Focus Areas
- packages/ir/src/validate.ts[118-141]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Bidirectional traversal pulls in unrelated sibling steps ✓ Resolved 🐞 Bug ≡ Correctness
Description
computeReachableSteps walks dependency edges in both directions from entry roots (producers AND
dependents), so once an ancestor of a root is added, all of that ancestor's other dependents
(unrelated sibling branches) are pulled into the RunPlan too. This contradicts the spec's stated
'transitive closure over dependencies[].producer edges' and can cause the native engine to receive
and execute steps that are not actually needed by the triggered entry.
Code

packages/planner/src/bind.ts[R114-121]

+    // Add dependents (forward).
+    for (const dep of dependents.get(id) ?? []) {
+      if (!reachable.has(dep)) queue.push(dep);
+    }
+    // Add producers (backward).
+    for (const prod of producers.get(id) ?? []) {
+      if (!reachable.has(prod)) queue.push(prod);
+    }
Relevance

●●● Strong

Spec says reachability follows dependencies[].producer only; current bidirectional walk includes
unrelated siblings, likely seen as a bug.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The spec (specs/13-planner/spec.md lines 84-87 in diff) states reachability is 'transitive closure
from Entry roots over dependencies[].producer edges' — i.e. producer-only (backward) traversal. The
implementation in bind.ts instead builds both a dependents map and a producers map and enqueues
both directions from every visited node (lines 114-121), so given steps B (root, depends on A) and C
(also depends on A, but not related to B), once A is reached via B's producer edge, C gets added as
a 'dependent' of A even though C is not needed to run B. The PR's own test 'includes all reachable
steps with dependencies' only exercises a linear chain so it doesn't catch this, but a
diamond/sibling-branch graph would incorrectly include C in the RunPlan for an entry rooted at B.

specs/13-planner/spec.md[84-87]
packages/planner/src/bind.ts[90-126]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
computeReachableSteps in packages/planner/src/bind.ts traverses dependency edges in both directions (producer edges AND dependent edges) starting from the entry's roots. This means that once an ancestor of a root is reached, all of that ancestor's other descendants (unrelated sibling steps not needed to execute the root) are also pulled into the RunPlan.

## Issue Context
The spec (specs/13-planner/spec.md) defines reachability as the transitive closure from Entry roots over `dependencies[].producer` edges only (i.e., an entry needs its roots and all of their transitive producers/ancestors — not unrelated dependents/descendants of those producers). The current implementation instead does a bidirectional BFS which over-includes steps.

## Fix Focus Areas
- packages/planner/src/bind.ts[90-126]
- packages/planner/src/bind.ts[97-105]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. bindRunPlan silently ignores non-first pipelines ✓ Resolved 🐞 Bug ≡ Correctness
Description
bindRunPlan hardcodes graph.project.pipelines[0] as the pipeline to bind against, so in graphs
with multiple pipelines a valid entryId that exists only in a later pipeline incorrectly throws
ENTRY_NOT_FOUND rather than being found or explicitly rejected. Although a comment acknowledges
this limitation, there is no validation to guard against multi-pipeline graphs and no mechanism to
select or search other pipelines, leaving multi-pipeline DefinitionGraphs only partially bindable.
Code

packages/planner/src/bind.ts[R33-37]

+  // For v0, we operate on the first (and typically only) pipeline.
+  const pipeline = graph.project.pipelines[0]!;
+
+  // Find the entry.
+  const entry = findEntry(pipeline, entryId);
Relevance

●● Moderate

Multi-pipeline support is explicitly deferred (“v0…first pipeline”); change is architectural and may
be postponed.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
In packages/planner/src/bind.ts, bindRunPlan always selects graph.project.pipelines[0]!
(around line 34) and passes it into findEntry, and findEntry only searches within that
pipeline’s entries (around line 129). Meanwhile, the graph model represents pipelines as
DefinitionGraph.project.pipelines: readonly PipelineDefinition[] (packages/core/src/graph.ts)
with no restriction to a single element, meaning multi-pipeline graphs are a valid representable
state; synthesis emits every pipeline and uses pipeline-qualified entry IDs, but the binder still
only checks the first pipeline. As a result, entries that exist in later pipelines are never
considered during binding and incorrectly surface as ENTRY_NOT_FOUND, which is misleading rather
than an explicit “unsupported multi-pipeline graph” or a correct match.

packages/planner/src/bind.ts[29-37]
packages/core/src/graph.ts[21-24]
packages/core/src/graph.ts[21-37]
packages/core/src/synthesize.ts[41-44]
packages/core/src/synthesize.ts[159-164]
packages/planner/src/bind.ts[33-37]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`bindRunPlan` unconditionally binds against `graph.project.pipelines[0]`, silently ignoring additional pipelines. This causes valid `entryId`s that belong to later pipelines to fail with a misleading `ENTRY_NOT_FOUND` instead of being found (e.g., by searching all pipelines) or being rejected with a clear error (e.g., invalid/unsupported multi-pipeline graph).

## Issue Context
`DefinitionGraph.project.pipelines` is an array (and not constrained to length 1 by the type system or validation), so multi-pipeline graphs are a legitimate state. Each pipeline owns its own entries, and synthesis emits every pipeline and uses pipeline-qualified entry IDs, but binding currently searches only the first pipeline; if you expand binding to search across pipelines, handle ambiguity explicitly if necessary.

## Fix Focus Areas
- packages/planner/src/bind.ts[29-37]
- packages/planner/src/bind.ts[33-37]
- packages/planner/src/bind.ts[128-137]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Input types not enforced ✓ Resolved 🐞 Bug ≡ Correctness
Description
bindInputs copies overrides and defaults without comparing their primitive type to Input.type,
so a declared string input can be bound to a number. The resulting concrete RunPlan violates its
input declaration and can deliver incorrect values to consumers.
Code

packages/planner/src/bind.ts[R159-162]

+    const userValue = userInputs?.[name];
+    if (userValue !== undefined) {
+      result[name] = userValue;
+      continue;
Relevance

●● Moderate

Adds runtime type enforcement and error semantics; could be deferred to later waves or treated as
out-of-scope.

PR-#34

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The input model carries an explicit primitive type, while binding never reads it and InputValue
permits all three primitive variants for every key.

packages/constructs/src/model.ts[87-95]
packages/ir/src/run-plan.ts[7-20]
packages/planner/src/bind.ts[151-178]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Input overrides and defaults are accepted regardless of the corresponding declaration's `type`.

## Issue Context
Require string, number, or boolean values to match `Input.type`. Validate graph defaults and return a structured planner error for invalid user overrides.

## Fix Focus Areas
- packages/planner/src/bind.ts[151-178]
- packages/ir/src/validate.ts[118-141]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

6. Special input names mishandled ✓ Resolved 🐞 Bug ≡ Correctness
Description
Synthesis writes arbitrary input names into a normal {} object, so assigning __proto__ invokes
its legacy setter rather than reliably creating an own property. Such a permitted input can be lost
or alter the record prototype, and binding repeats the same unsafe accumulation pattern.
Code

packages/core/src/synthesize.ts[R70-72]

+  const inputs: Record<string, Input> = {};
+  for (const [name, input] of pipeline.inputs) {
+    inputs[name] = input;
Relevance

●●● Strong

Team has accepted injection/unsafe-string fixes; using Object.create(null)/Map for arbitrary keys is
likely welcomed.

PR-#34

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The constructs API accepts arbitrary record keys and preserves them in a Map, but synthesis and
binding copy those keys via indexed assignment into ordinary objects.

packages/constructs/src/constructs.ts[30-51]
packages/core/src/synthesize.ts[70-73]
packages/planner/src/bind.ts[151-181]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Arbitrary input names are copied into prototype-bearing objects, which mishandles names such as `__proto__`.

## Issue Context
Use `Object.create(null)`, `Object.fromEntries`, or safely defined own properties consistently for synthesized and bound input records.

## Fix Focus Areas
- packages/core/src/synthesize.ts[70-73]
- packages/planner/src/bind.ts[151-181]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context
✅ Compliance rules (platform): 8 rules
Review mode: 🧠 Deep: This is a substantial behavioral planner/API change spanning multiple packages, with new graph traversal, input binding, validation, identity computation, and a cross-cutting schema change that creates several independent opportunities for subtle defects.

Grey Divider

Tip of the day
💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread packages/planner/src/bind.ts
Comment thread packages/ir/src/validate.ts
Comment thread packages/planner/src/bind.ts
Comment thread packages/core/src/synthesize.ts Outdated
Comment thread packages/planner/src/bind.ts Outdated
Comment thread packages/planner/src/bind.ts Outdated
@codeant-ai codeant-ai Bot added size:XL This PR changes 500-999 lines, ignoring generated files and removed size:XL This PR changes 500-999 lines, ignoring generated files labels Aug 13, 2026
@codeant-ai codeant-ai Bot removed the size:XL This PR changes 500-999 lines, ignoring generated files label Aug 13, 2026
@sonarqubecloud

Copy link
Copy Markdown

❌ The last analysis has failed.

See analysis details on SonarQube Cloud

ThePlenkov and others added 6 commits August 13, 2026 23:36
Rebuilt @sverka/planner with bindRunPlan: binds a Definition Graph's
Entry + user inputs into a concrete RunPlan for the native engine.

- bindRunPlan: selects reachable steps, binds inputs, computes IDs
- computeReachableSteps: transitive closure from entry roots (both directions)
- PlannerError: ENTRY_NOT_FOUND, ROOT_NOT_FOUND, MISSING_INPUT, INVALID_GRAPH
- Reuses existing discovery + plan synthesis logic unchanged

Changed PipelineDefinition.inputs from Input[] to Record<string, Input>
to preserve input names through synthesis. Updated core, IR validation,
and test fixtures accordingly.

58 planner tests pass (17 new bind tests + 41 existing). All 7 affected
packages green: 229 tests total. No any types.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Search all pipelines in findEntry (not just the first one).

- Validate dependency producers before building the RunPlan.

- Follow only producer/prerequisite edges backward in computeReachableSteps.

- Validate pipeline input descriptor fields (type, required, secret, description, default).

- Omit secret input values from RunPlan.inputs and from the content-addressed plan ID.

- Update graph and bind tests for record-based inputs and sink roots.

Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
- Validate runtime graph shape in bindRunPlan before dereferencing fields.
- Run core validateGraph on the selected pipeline to detect cycles and semantic errors.
- Use an index-based queue in computeReachableSteps and mark nodes reachable on enqueue.
- Validate input descriptor type for secret and optional inputs.
- Add INVALID_INPUT PlannerErrorCode and use it for type-mismatched user/default values.
- Update Spec 13 to document options-object contract, inputs record, INVALID_INPUT, and structural validation.

Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
…ptors

Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
Split validatePipelineInput (complexity 13→3) into per-field validators
and validatePipelineShape (complexity 12→3) into field/child helpers.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@ThePlenkov
ThePlenkov merged commit bf56c6a into main Aug 13, 2026
12 checks passed
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

baz: needs review size:XL This PR changes 500-999 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant