Allow rules to execute ordered command lists (#550)

[leynos]

Summary

Allow a rule's command field to accept either the existing scalar string or
a non-empty ordered list of command strings. A command list runs its entries
in declaration order and stops at the first non-zero exit, so a reusable rule
can compose several distinct commands without a hand-written shell chain, a
script block, or a nested Netsuke invocation.

Closes #550

Manifest shape

A scalar command is unchanged:

rules:
  - name: lint
    command: cargo clippy --all-targets --all-features -- -D warnings

A list is now accepted too:

rules:
  - name: comprehensive-check
    description: Run the required checks sequentially
    command:
      - cargo fmt --all -- --check
      - cargo clippy --all-targets --all-features -- -D warnings
      - cargo nextest run --all-targets --all-features
      - cargo test --doc

Semantics

• Entries execute strictly in declaration order.
• The chain stops at the first non-zero exit and returns that failure.
• Every list entry is Jinja-rendered and gets {{ ins }}/{{ outs }}
  interpolation per entry during IR lowering.
• Entries share one shell process, so working directory, environment, and
  exit-code state carry forward like a script block.
• An empty command list is rejected during manifest deserialization with a
  localized diagnostic.
• The scalar form is unchanged: serialization, hashing, and Ninja output
  remain byte-identical.

Implementation

• Recipe::Command now holds a StringOrList; From<&str>, From<String>,
  and From<Vec<String>> keep existing construction sites compiling.
• render_recipe_string_or_list renders each list entry with the
  ins/outs placeholder injection.
• IR lowering interpolates each entry independently, preserving the
  scalar-vs-list shape.
• Ninja generation joins list entries with && into a single fail-fast chain.

Tests

Parsing, rendering, IR interpolation, and Ninja generation are covered for
both forms, plus ordering, fail-fast behaviour, Jinja rendering, empty-list
rejection, and a new multi_command.yml fixture with a Ninja snapshot. The
users' guide and design doc document command lists.

References

• https://lody.ai/leynos/sessions/e2c7837b-4836-47a6-b8fe-7e9e849cc4e4

Generated with Claude Code

Summary by Sourcery

Allow command recipes for rules and targets to be specified as either a scalar string or a non-empty ordered list, executed as a single fail-fast shell chain and rejected if empty.

New Features:

• Support ordered command lists in rule and target command recipes alongside the existing scalar command form, with each entry independently interpolated for inputs and outputs.
• Expose a localized manifest error when a command list is empty instead of silently accepting it.

Enhancements:

• Emit list-based command recipes to Ninja as a single &&-joined fail-fast chain while preserving existing scalar command behaviour and hashing.
• Extend the StringOrList AST helper with conversions, emptiness checks, and utility accessors used across manifest parsing, IR generation, and Ninja output.

Documentation:

• Document the command list syntax, execution semantics, and usage guidance in the users' guide and design document, including a tested example manifest.

Tests:

• Add unit, integration, IR, Jinja rendering, and snapshot tests covering scalar vs list command parsing, interpolation order, fail-fast behaviour, empty list rejection, and Ninja generation for multi-command manifests.

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Summary

• Support scalar strings and non-empty ordered lists in rule and target command fields.
• Render each list entry independently with Jinja.
• Execute entries in one shell process with shared state and fail-fast && semantics.
• Preserve scalar command serialization, hashing, and Ninja output.
• Reject empty command lists during manifest deserialization with localized diagnostics.
• Attribute failures by action and entry index without exposing command text or secrets.
• Reuse interpolation bindings across list entries.
• Update IR lowering, Ninja generation, documentation, fixtures, and tests.
• Document syntax and execution behaviour in docs/netsuke-design.md, docs/users-guide.md, and docs/developers-guide.md.
• Address issue #550 with parsing, rendering, ordering, interpolation, fail-fast, property-based, compatibility, UI, and Ninja execution coverage.

Walkthrough

Changes

Support scalar commands and ordered, non-empty command lists. Interpolate each list entry independently. Render lists as one fail-fast && shell chain. Reject empty lists during parsing. Add failure attribution, tests, snapshots, localisations, and documentation.

Ordered command list support

Layer / File(s)	Summary
Manifest command contract src/ast.rs, src/localization/*, docs/*, tests/ast_tests/*	Accept scalar strings and ordered string lists. Reject empty command content with localised diagnostics.
Command rendering and IR interpolation src/manifest/*, src/ir/*, tests/ir_from_manifest_tests.rs, tests/manifest_*	Render and interpolate each list entry independently. Reuse input and output bindings. Preserve order and placeholder handling.
Fail-fast Ninja generation src/ninja_gen.rs, src/ninja_gen_tests.rs, tests/ninja_*, tests/data/multi_command.yml	Quote list entries and join them with &&. Validate shell syntax and empty recipes. Test ordering, shared shell state, direct targets, and snapshots.
Failure attribution src/runner/process/*, tests/logging_stderr/*	Capture bounded failure markers and report the action and entry positions in human, JSON, and tracing diagnostics.

Sequence Diagram(s)

sequenceDiagram
  participant ManifestParser
  participant CommandRenderer
  participant IRConversion
  participant NinjaGenerator
  participant ProcessRunner
  ManifestParser->>CommandRenderer: provide scalar or ordered command list
  CommandRenderer->>IRConversion: render and interpolate each entry
  IRConversion->>NinjaGenerator: provide interpolated recipe
  NinjaGenerator->>NinjaGenerator: emit brace groups joined with &&
  NinjaGenerator->>ProcessRunner: execute generated Ninja command
  ProcessRunner->>ProcessRunner: capture action and entry failure marker

Loading

Poem

Commands run in order.
Each entry joins the chain.
Empty lists fail parsing.
&& stops at the first failure.
Shared shell state remains.

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (5 warnings, 4 inconclusive)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The .gitignore additions for .vtcode/ and vtcode.toml are unrelated to ordered command-list support.	Remove the unrelated .gitignore entries, or link an issue that requires the vtcode paths.
User-Facing Documentation	⚠️ Warning	The user guide documents command lists clearly, but the PR adds this new functionality without changing or adding the required n+1 migration document.	Add a pre-1.0.0 minor-version migration note for command lists and link it to the relevant user-guide section.
Observability	⚠️ Warning	Command lists add per-entry failure behaviour, but the PR adds no outcome/latency metrics and logs only generated action/entry indexes, not a stable rule or target identifier.	Add bounded Ninja command-list outcome and duration metrics. Propagate a stable rule/target/action identifier into the marker and structured warning. Exclude command text from telemetry.
Performance And Resource Use	⚠️ Warning	The PR wraps all stdout forwarding in FailureAttributionWriter, which scans every byte, although command-list markers are emitted only to stderr; this adds avoidable O(stdout bytes) work to every...	Keep FailureAttributionWriter on stderr only. Forward stdout through the existing writer and return None for stdout attribution.
Concurrency And State	⚠️ Warning	The generated eval/&& chain does not wait for backgrounded entries: modelling false & returned status 0 and ran entry 2, while the integration test covers only true &.	Add a boundary that waits for background jobs and propagates their failures, or reject background operators; test that a failing background entry stops later entries and fails the build.
Linked Issues check	❓ Inconclusive	The changes address issue #550, but the required Ninja snapshot content is excluded by the !**/*.snap path filter.	Review tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap to verify coverage for both action and target references.
Testing (Unit And Behavioural)	❓ Inconclusive	Ignore this interim call.	Continue investigation.
Domain Architecture	❓ Inconclusive	Investigation started; no verdict yet.	Await repository diff and architecture evidence.
Security And Privacy	❓ Inconclusive	Investigation is still in progress; no verdict evidence has been gathered yet.	Inspect the pull-request diff and verify changed shell, logging, deserialization, and fixture paths for explicit security or privacy failures.

✅ Passed checks (11 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title states the main change and includes the linked issue reference (#550).
Description check	✅ Passed	The description explains ordered command-list support, required semantics, implementation details, tests, and documentation.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Testing (Overall)	✅ Passed	Accept the coverage: tests assert parsing, rendering, IR order, real Ninja fail-fast and shared-shell behaviour, direct-target Jinja/interpolation, diagnostics, scalar compatibility, and empty-list...
Developer Documentation	✅ Passed	Documentation changes cover the new command-list architecture and internal boundaries in docs/developers-guide.md and docs/netsuke-design.md, with no localisation content changed beyond the existin...
Module-Level Documentation	✅ Passed	Accept the check: all changed Rust modules carry //! documentation, with headers describing their purpose, utility, and relevant relationships to IR, Ninja, or runner components.
Testing (Property / Proof)	✅ Passed	The PR introduces substantive proptest coverage for generated command-list ordering, fail-fast && joins, scalar compatibility, empty recipes, and per-entry interpolation.
Testing (Compile-Time / Ui)	✅ Passed	The PR adds an external Rust compile-pass fixture for StringOrList and Recipe::Command, plus a focused Ninja snapshot with semantic chain and target assertions.
Unit Architecture	✅ Passed	The changed paths keep rendering, interpolation, Ninja generation, process execution, and failure attribution in explicit command-side units with fallible results and focused tests.
Architectural Complexity And Maintainability	✅ Passed	The patch adds narrow, documented helpers for per-recipe binding reuse, shell rendering, and process attribution; it adds focused tests and no dependencies or speculative extension layers.
Rust Compiler Lint Integrity	✅ Passed	Mark PASS: the cumulative diff adds no lint suppressions, gates new support modules with cfg(test), and uses only justified context or test-setup clones; scalar command cloning is absent.

📋 Issue Planner

Built with CodeRabbit's Coding Plans for faster development and fewer bugs.

View plan used: #550

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-550-allow-rules-to-execute-ordered-command-lists

---

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

[sourcery-ai]

Reviewer's Guide

Extend Recipe::Command to support both scalar strings and non-empty ordered command lists, ensuring manifest parsing, Jinja rendering, IR lowering, Ninja generation, and documentation all understand and correctly execute fail-fast command chains while preserving existing scalar behaviour.

Flow diagram for command list processing from manifest to Ninja

flowchart LR
    ManifestCommand[StringOrList command in manifest]
    Render[render_recipe_string_or_list]
    IR[register_action interpolate_command]
    Ninja[ninja_gen write_recipe join with &&]

    ManifestCommand --> Render
    Render --> IR
    IR --> Ninja

    subgraph StringOrListVariants
      StringVariant[String]
      ListVariant[List]
      EmptyVariant[Empty]
    end

    ManifestCommand --> StringVariant
    ManifestCommand --> ListVariant
    ManifestCommand --> EmptyVariant

    ListVariant --> Render
    ListVariant --> IR
    ListVariant --> Ninja

    StringVariant --> Render
    StringVariant --> IR
    StringVariant --> Ninja

    EmptyVariant --> ManifestError[manifest.command_list_empty diagnostic]
    EmptyVariant --> NinjaGuard[reject_empty_command_recipe in debug]

Loading

File-Level Changes

Change	Details	Files
Recipe::Command now uses StringOrList, allowing scalar commands or non-empty ordered lists, with manifest deserialization rejecting empty lists.	Change Recipe::Command.command type from String to StringOrList and update RawRecipe to deserialize command as StringOrList. Implement StringOrList::is_empty_content plus From<&str>, From, and From<Vec> to preserve construction ergonomics. Update Recipe::Deserialize to emit a localized MANIFEST_COMMAND_LIST_EMPTY error when the command is Empty or an empty List. Adjust tests and helpers that previously assumed command was a plain String to use as_single(), to_string_vec(), or match on StringOrList variants.	src/ast.rs tests/ast_tests/string_or_list.rs tests/ast_tests/parsing.rs tests/ast_tests/recipe.rs tests/bdd/steps/manifest/targets.rs tests/bdd/steps/manifest/mod.rs tests/ir_tests.rs tests/hasher_tests.rs tests/manifest_env_tests.rs src/manifest/mod.rs src/manifest/tests/workspace.rs tests/command_escaping_tests.rs
Command rendering and IR lowering now handle lists by rendering/interpolating each entry independently while preserving the scalar vs list shape.	Add render_recipe_string_or_list utility that renders StringOrList commands entry-wise with ins/outs placeholders, computing the error label once. Use render_recipe_string_or_list when rendering rule and target Recipe::Command commands instead of render_recipe_str_with on a String. Update IR register_action to interpolate StringOrList commands, mapping interpolate_command over scalar and list variants and keeping Empty unchanged. Introduce tests to verify command lists render each entry with ins/outs, and IR interpolation preserves declaration order in lists.	src/manifest/render.rs src/ir/from_manifest_support.rs tests/manifest_jinja_tests.rs tests/ir_from_manifest_tests.rs
Ninja generation joins command lists into a single fail-fast && chain, rejects empty commands defensively, and adds tests for the new behaviour.	Update NamedAction::write_recipe to accept StringOrList, building command_line by joining List items with " && " and rejecting Empty via reject_empty_command_recipe. Add reject_empty_command_recipe debug-only panic helper to surface unexpected empty commands during Ninja generation. Refactor inline tests out of src/ninja_gen.rs into new src/ninja_gen_tests.rs, and add a test that command lists are emitted as echo one && echo two && echo three. Extend integration tests to cover fail-fast behaviour of command lists executed by ninja, ensuring later entries are skipped after a non-zero exit.	src/ninja_gen.rs src/ninja_gen_tests.rs tests/ninja_gen_integration_tests.rs
Documentation, examples, localization, and snapshots now describe and exercise command lists and their fail-fast semantics.	Update users guide to describe command lists, their execution semantics, and add a fenced guide-command-list example manifest. Update netsuke-design.md to document StringOrList-based command, list fail-fast behaviour, and rejection of empty lists. Add multi_command.yml manifest fixture and a corresponding Ninja snapshot test asserting joined fail-fast chains and references from both a target and an action. Register the new guide-command-list fenced example in documentation_examples_tests and ensure snapshot path includes the new ninja snapshot. Add MANIFEST_COMMAND_LIST_EMPTY localization key and messages across all locales, providing a consistent error string for empty command lists.	docs/users-guide.md docs/netsuke-design.md tests/documentation_examples_tests.rs tests/ninja_snapshot_tests.rs tests/data/multi_command.yml tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap src/localization/keys.rs locales/*/messages.ftl
Changelog entry documents the new command list feature and its fail-fast semantics.	Add a CHANGELOG.md entry describing the acceptance of non-empty ordered command lists for command recipes and their execution as a fail-fast && shell chain.	CHANGELOG.md

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#550	Extend the manifest, IR, and Ninja generation to allow a rule or target command field to be either the existing scalar string or a non-empty ordered list of command strings, with semantics: entries execute in declaration order, fail fast at first non-zero exit, share one shell process, empty lists rejected during manifest validation, Jinja rendering applied to each entry (including {{ ins }}/{{ outs }}), lists usable wherever rules are referenced, and existing scalar behavior preserved.	✅
#550	Add focused tests to cover both scalar and list command forms, including ordering and fail-fast behavior, Jinja rendering and interpolation per list entry, empty-list rejection, backwards compatibility for scalar commands, and Ninja snapshots demonstrating a multi-command rule referenced by both an action and a target.	✅
#550	Update the users guide and design documentation to describe command lists, their shell-state and fail-fast semantics, and guidance on when to prefer command lists versus script recipes.	✅

Possibly linked issues

• Allow rules to execute ordered command lists #550: PR adds scalar-or-list command support, fail-fast ordered execution, empty-list rejection, tests, and documentation as described.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Sorry @leynos, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ae55b27f3f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[wafflecat-df12]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 5

🤖 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 `@CHANGELOG.md`:
- Around line 26-27: Update the changelog sentence near the shell-chain
description by inserting a comma before “so” to separate the descriptive clause
from the result clause.

In `@docs/netsuke-design.md`:
- Line 719: Synchronize all command schema documentation with StringOrList: in
docs/netsuke-design.md lines 719-719, change the Mermaid RECIPE.command field
from string to StringOrList; in docs/netsuke-design.md lines 253-257, describe
scalar pass-through and list lowering into brace groups joined with &&, removing
the verbatim claim; in src/ast.rs lines 145-149, update the Recipe::Command
Rustdoc with the same scalar and list behavior.

In `@locales/ar/messages.ftl`:
- Line 152: Update the manifest.command_list_empty translation to specifically
state that the command list must not be empty, while still indicating that a
command string is an accepted alternative; do not imply that the scalar value
command: "" is rejected.

In `@src/ninja_gen.rs`:
- Around line 218-230: Update the StringOrList::List serialization in the
command_line construction to use a shell-safe boundary that remains valid when
an entry contains an inline comment or ends with &, while preserving brace-group
isolation and the fail-fast && chain. Add regression tests covering both
inline-comment entries and entries ending with &.

In `@tests/ninja_snapshot_tests.rs`:
- Around line 135-136: Update the fixture-loading code in the ninja snapshot
test to read multi_command.yml through a cap_std::fs_utf8::Dir capability
instead of std::fs::read_to_string, preserving the existing context error
handling and fixture path.

🪄 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: 8310c81e-5564-4147-9a10-e07b9a86c415

📥 Commits

Reviewing files that changed from the base of the PR and between 487f77e and 0203660.

⛔ Files ignored due to path filters (1)

• tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap is excluded by !**/*.snap

📒 Files selected for processing (62)

• CHANGELOG.md
• docs/netsuke-design.md
• docs/users-guide.md
• locales/ar/messages.ftl
• locales/cs/messages.ftl
• locales/cy/messages.ftl
• locales/da/messages.ftl
• locales/de/messages.ftl
• locales/el/messages.ftl
• locales/en-GB/messages.ftl
• locales/en-US/messages.ftl
• locales/es-419/messages.ftl
• locales/es-ES/messages.ftl
• locales/fa/messages.ftl
• locales/fi/messages.ftl
• locales/fr/messages.ftl
• locales/gd/messages.ftl
• locales/he/messages.ftl
• locales/hi/messages.ftl
• locales/hu/messages.ftl
• locales/id/messages.ftl
• locales/it/messages.ftl
• locales/ja/messages.ftl
• locales/ko/messages.ftl
• locales/nb/messages.ftl
• locales/nl/messages.ftl
• locales/pl/messages.ftl
• locales/pt-BR/messages.ftl
• locales/pt-PT/messages.ftl
• locales/ro/messages.ftl
• locales/ru/messages.ftl
• locales/sv/messages.ftl
• locales/th/messages.ftl
• locales/tr/messages.ftl
• locales/uk/messages.ftl
• locales/vi/messages.ftl
• locales/zh-Hans/messages.ftl
• locales/zh-Hant/messages.ftl
• src/ast.rs
• src/ir/from_manifest_support.rs
• src/localization/keys.rs
• src/manifest/mod.rs
• src/manifest/render.rs
• src/manifest/tests/workspace.rs
• src/ninja_gen.rs
• src/ninja_gen_tests.rs
• tests/ast_tests.rs
• tests/ast_tests/parsing.rs
• tests/ast_tests/recipe.rs
• tests/ast_tests/string_or_list.rs
• tests/bdd/steps/manifest/mod.rs
• tests/bdd/steps/manifest/targets.rs
• tests/command_escaping_tests.rs
• tests/data/multi_command.yml
• tests/documentation_examples_tests.rs
• tests/hasher_tests.rs
• tests/ir_from_manifest_tests.rs
• tests/ir_tests.rs
• tests/manifest_env_tests.rs
• tests/manifest_jinja_tests.rs
• tests/ninja_gen_integration_tests.rs
• tests/ninja_snapshot_tests.rs

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• leynos/rstest-bdd (auto-detected)
• leynos/ortho-config (auto-detected)
• leynos/whitaker (auto-detected)
• leynos/shared-actions (auto-detected)

[pandalump]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[leynos]

@coderabbitai Have the following failed checks now been resolved?

If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures.

Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat o11y, code safety, documentation and validation coverage as in scope).

❌ Failed checks (2 errors, 5 warnings)

Check name	Status	Explanation	Resolution
Testing (Overall)	❌ Error	Add target-recipe list tests: every list test uses a rule or a manually built Action; none parses/renders a target with a list command, so a broken render_target list branch would pass.	Add an end-to-end manifest test with a target command list containing Jinja and $in/$out, then assert the rendered IR and Ninja execution; test rejection of a programmatic empty list in Ninja generation.
Rust Compiler Lint Integrity	❌ Error	Flag the new StringOrList::String(cmd) => cmd.clone() in src/ninja_gen.rs:219; it copies every scalar command only to unify the match result, with no ownership need.	Remove the scalar copy by writing the borrowed command directly or by using Cow<str> for the borrowed scalar and owned list result.
Developer Documentation	⚠️ Warning	Document command-list internals in docs/developers-guide.md; it has no PR changes or relevant terms. Correct stale scalar-only command declarations in docs/netsuke-design.md.	Add the StringOrList, manifest→IR→Ninja lowering, rendering, shell-chain boundary, and target command-list semantics to the developer guide and design schema.
Testing (Property / Proof)	⚠️ Warning	The PR adds range-sensitive ordering and fail-fast invariants, but only fixed examples; its existing proptest block predates the PR and covers helper traversal, not command-list behaviour.	Add substantive proptest coverage for arbitrary non-empty list lengths and entries, including order, interpolation, fail-fast chaining, shell state, and empty-list rejection.
Testing (Compile-Time / Ui)	⚠️ Warning	The public Rust API changes Recipe::Command.command and adds From implementations, but the PR adds no trybuild or equivalent UI compile test; its focused Ninja snapshot does meet output expecta...	Add a Rust compile-time fixture using the changed Recipe::Command and StringOrList API, with a trybuild or existing direct-rustc UI harness and stable diagnostics where failures are intended.
Observability	⚠️ Warning	Runtime list failures have no entry index or per-entry context: Ninja emits one brace-chain command, while the PR only labels Jinja render errors.	Add bounded action/target and one-based entry context at runtime failure boundaries; expose it in human, JSON, and tracing output without logging secrets or raw payloads.
Performance And Resource Use	⚠️ Warning	The list loop clones the full Vars context for every entry, and register_action re-quotes all inputs and outputs for every entry, causing avoidable work to scale with list length.	Hoist recipe-context preparation out of the entry loop and cache quoted input/output bindings for the list; add a large-list benchmark or regression test.

[codescene-access]

Gates Failed
Enforce advisory code health rules (1 file with Code Duplication)

Our agent can fix these. Install it.

Gates Passed
5 Quality Gates Passed

Reason for failure

Enforce advisory code health rules	Violations	Code Health Impact
command_env_ui_tests.rs	1 advisory rule	10.00 → 9.39	Suppress

See analysis details in CodeScene

Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

[codescene-access]

❌ New issue: Code Duplication
The module contains 2 functions with similar structure: command_env_embedder_fixture_compiles,command_list_public_api_fixture_compiles

_Suppress

[codescene-access]

No quality gates enabled for this code.

[wafflecat-df12]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@src/ninja_gen.rs`:
- Around line 352-358: Update command_list_entry and the list-entry execution
flow so exit commands are handled before the eval-based failure wrapper can
bypass marker emission. Add a runtime regression covering a first entry of exit
23, asserting the next entry is not executed, the overall status is 23, and the
failure marker identifies entry 1.
- Around line 241-247: Update validate_action_recipe in src/ninja_gen.rs:241-247
to use command.is_empty_content() so programmatic empty command lists return
EmptyCommandRecipe instead of emitting an empty command. Add regression cases
covering empty command lists in src/ninja_gen_property_tests.rs:182-199 and
src/ninja_gen_tests.rs:135-155.

🪄 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: 1824da0e-ce56-4b69-ae99-efb8690cb694

📥 Commits

Reviewing files that changed from the base of the PR and between 0203660 and 21079dd.

⛔ Files ignored due to path filters (1)

• tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap is excluded by !**/*.snap

📒 Files selected for processing (26)

• .gitignore
• CHANGELOG.md
• docs/developers-guide.md
• docs/netsuke-design.md
• docs/users-guide.md
• locales/ar/messages.ftl
• src/ast.rs
• src/ir/cmd_interpolate.rs
• src/ir/from_manifest_support.rs
• src/ir/from_manifest_support_tests.rs
• src/manifest/render.rs
• src/manifest/render_command_list_tests.rs
• src/ninja_gen.rs
• src/ninja_gen_property_tests.rs
• src/ninja_gen_tests.rs
• src/runner/process/child_exit.rs
• src/runner/process/failure_attribution.rs
• src/runner/process/mod.rs
• src/runner/process/tests.rs
• tests/command_env_ui_tests.rs
• tests/documentation_examples_tests.rs
• tests/logging_stderr/command_list_failure.rs
• tests/logging_stderr_tests.rs
• tests/ninja_gen_command_list_integration_tests.rs
• tests/ninja_snapshot_tests.rs
• tests/ui/command_list_public_api_pass.rs

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• leynos/rstest-bdd (auto-detected)
• leynos/ortho-config (auto-detected)
• leynos/whitaker (auto-detected)
• leynos/shared-actions (auto-detected)

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '237,250p' src/ninja_gen.rs
sed -n '182,199p' src/ninja_gen_property_tests.rs
sed -n '135,155p' src/ninja_gen_tests.rs
rg -n -C 3 'is_empty_content|StringOrList::List\(Vec::new\(\)\)' src tests

Repository: leynos/netsuke

Length of output: 4248

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- StringOrList implementation ---'
sed -n '330,390p' src/ast.rs

printf '%s\n' '--- generation and recipe emission ---'
rg -n -C 6 'validate_action_recipe|Recipe::Command|command =' src/ninja_gen.rs src/ast.rs

printf '%s\n' '--- relevant tests and declarations ---'
rg -n -C 4 'EmptyCommandRecipe|StringOrList' src/ninja_gen.rs src/ninja_gen_tests.rs src/ninja_gen_property_tests.rs tests/ast_tests/string_or_list.rs

Repository: leynos/netsuke

Length of output: 25601

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

ast = Path("src/ast.rs").read_text()
method = re.search(
    r'pub const fn is_empty_content\(&self\) -> bool \{(?P<body>.*?)\n    \}',
    ast,
    re.S,
)
assert method, "is_empty_content implementation not found"
body = method.group("body")
assert "Self::Empty => true" in body
assert "Self::String(_) => false" in body
assert "Self::List(values) => values.is_empty()" in body

generator = Path("src/ninja_gen.rs").read_text()
validation = re.search(
    r'const fn validate_action_recipe\(.*?\n\}',
    generator,
    re.S,
)
assert validation, "validate_action_recipe implementation not found"
assert "StringOrList::Empty" in validation.group(0)
assert "is_empty_content" not in validation.group(0)

print("StringOrList::List(Vec::new()) is classified as empty content.")
print("validate_action_recipe checks only StringOrList::Empty, so an empty list bypasses validation.")
PY

Repository: leynos/netsuke

Length of output: 241

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '260,306p' src/ninja_gen.rs

python3 - <<'PY'
from pathlib import Path
import re

ast = Path("src/ast.rs").read_text()
method = re.search(
    r'pub const fn is_empty_content\(&self\) -> bool \{(?P<body>.*?)\n    \}',
    ast,
    re.S,
)
assert method, "is_empty_content implementation not found"
body = method.group("body")
assert "Self::Empty => true" in body
assert "Self::String(_) => false" in body
assert re.search(r"Self::List\(\w+\) => \w+\.is_empty\(\)", body)

generator = Path("src/ninja_gen.rs").read_text()
validation = re.search(
    r'const fn validate_action_recipe\(.*?\n\}',
    generator,
    re.S,
)
assert validation, "validate_action_recipe implementation not found"
validation_body = validation.group(0)
assert "StringOrList::Empty" in validation_body
assert "is_empty_content" not in validation_body

list_branch = re.search(
    r'Recipe::Command \{\s*command: StringOrList::List\(items\),(?P<body>.*?)\n\s*}\s*Recipe::Command',
    generator,
    re.S,
)
assert list_branch, "command-list branch not found"
assert '.join(" && ")' in list_branch.group("body")
assert 'writeln!(f, "  command = {command_line}")' in list_branch.group("body")

print("StringOrList::List(Vec::new()) is classified as empty content.")
print("validate_action_recipe checks only StringOrList::Empty.")
print("The command-list branch joins zero items to an empty command_line and writes it as a Ninja command.")
PY

Repository: leynos/netsuke

Length of output: 2615

---

Reject programmatic empty command lists.

Use command.is_empty_content() in validate_action_recipe. An empty list currently bypasses validation, joins to an empty command, and emits command =.

Add empty-list regression cases to both generator test sites.

📍 Affects 3 files

• src/ninja_gen.rs#L241-L247 (this comment)
• src/ninja_gen_property_tests.rs#L182-L199
• src/ninja_gen_tests.rs#L135-L155

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ninja_gen.rs` around lines 241 - 247, Update validate_action_recipe in
src/ninja_gen.rs:241-247 to use command.is_empty_content() so programmatic empty
command lists return EmptyCommandRecipe instead of emitting an empty command.
Add regression cases covering empty command lists in
src/ninja_gen_property_tests.rs:182-199 and src/ninja_gen_tests.rs:135-155.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

set +e
output=$(
  /bin/sh -c \
    '{ if eval '"'"'exit 23'"'"'; then :; else status=$?; printf "%s\n" "marker"; exit "$status"; fi; }' \
    2>&1
)
status=$?
set -e

test "$status" -eq 23
test "$output" != "marker"

Repository: leynos/netsuke

Length of output: 152

---

🏁 Script executed:

set -euo pipefail
printf '%s\n' '--- relevant definitions and call sites ---'
rg -n -A18 -B12 'fn command_list_entry|validate_action_recipe|command_list_entry|StringOrList|COMMAND_LIST_FAILURE_PREFIX' src tests || true
printf '%s\n' '--- module outline ---'
ast-grep outline src/ninja_gen.rs
printf '%s\n' '--- focused source ---'
sed -n '210,270p' src/ninja_gen.rs
sed -n '330,370p' src/ninja_gen.rs

Repository: leynos/netsuke

Length of output: 50373

---

🏁 Script executed:

set -euo pipefail
printf '%s\n' '--- command-list integration tests ---'
sed -n '185,285p' tests/ninja_gen_integration_tests.rs
printf '%s\n' '--- all command-list test names ---'
rg -n 'command_list|failure attribution|COMMAND_LIST_FAILURE_PREFIX|netsuke command-list failure' tests src --glob '*.rs'
printf '%s\n' '--- StringOrList definition and emptiness helpers ---'
rg -n -A80 -B15 'enum StringOrList|is_empty_content|impl.*StringOrList' src

Repository: leynos/netsuke

Length of output: 21919

---

🏁 Script executed:

set -euo pipefail
python3 - <<'PY'
import subprocess

command = r'''{ if eval 'exit 23'; then :; else _netsuke_command_status=$?; printf '%s\n' 'netsuke command-list failure: action 1, entry 1' >&2; exit "$_netsuke_command_status"; fi; } && { if eval 'printf second-entry-ran >&2'; then :; else _netsuke_command_status=$?; printf '%s\n' 'netsuke command-list failure: action 1, entry 2' >&2; exit "$_netsuke_command_status"; fi; }'''
p = subprocess.run(['/bin/sh', '-c', command], text=True, capture_output=True)
print(f"status={p.returncode}")
print(f"stdout={p.stdout!r}")
print(f"stderr={p.stderr!r}")
PY

Repository: leynos/netsuke

Length of output: 182

---

Handle exit commands before failure attribution is bypassed.

When a list entry executes exit, eval terminates the current shell before the failure marker is printed. Add a runtime regression for a first entry of exit 23. Verify that the next entry does not run, the status remains 23, and the marker identifies entry 1.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ninja_gen.rs` around lines 352 - 358, Update command_list_entry and the
list-entry execution flow so exit commands are handled before the eval-based
failure wrapper can bypass marker emission. Add a runtime regression covering a
first entry of exit 23, asserting the next entry is not executed, the overall
status is 23, and the failure marker identifies entry 1.