Skip to content

docs(cue): multi-root pipelines guide; fix validator misfire on missing source_session - #987

Merged
pedramamini merged 3 commits into
RunMaestro:rcfrom
chr1syy:docs/cue-multi-root-pipelines
May 13, 2026
Merged

docs(cue): multi-root pipelines guide; fix validator misfire on missing source_session#987
pedramamini merged 3 commits into
RunMaestro:rcfrom
chr1syy:docs/cue-multi-root-pipelines

Conversation

@chr1syy

@chr1syy chr1syy commented May 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Documents the per-agent-cwd model for multi-root Cue pipelines and fixes a small validator misfire flagged by review bots on a related PR.

Two commits, 6 files, +139 / -32. Docs + one validator guard tightening + one regression test.

Multi-root pipelines docs (commit 1)

The Cue engine reads only <projectRoot>/.maestro/cue.yaml for each agent — no parent-directory walk, no ancestor fallback, no shared workspace file. A pipeline spanning agents at different project roots is physically multiple yaml files (one per participating agent's cwd), stitched at runtime via agent_id references in source_session_ids / fan_out_ids. The visual Pipeline Editor handles this automatically; agents authoring YAML by hand need explicit guidance.

Added:

  • A "Multi-Root Pipelines" section in the agent-facing prompt (src/prompts/_maestro-cue.md) and a mirrored "Multi-root pipelines" section in the user-facing configuration reference (docs/maestro-cue-configuration.md), both with a per-role placement table (trigger / fan-out / chain / fan-in / command) and a hand-authoring checklist.
  • A <Note> callout under File Location in maestro-cue-configuration.md linking to the new section.
  • An updated "Configuration File" paragraph in maestro-cue.md clarifying the per-agent-cwd model.
  • A <Note> on the CI-Style Pipeline example in maestro-cue-examples.md annotating it as the canonical multi-root pattern (three separate yamls) vs. the same-root case with agent_id routing in one file.

Validator misfire fix + schema-reference catch-up (commit 2)

Greptile and CodeRabbit flagged two issues downstream of PR #976's source_sub validator rule (already on rc):

  1. Doc/validator mismatch. The Multi-root docs originally said "prefer source_session_ids over source_session" — but the validator (correctly) requires source_session on every agent.completed sub. An agent reading the guidance literally would write invalid YAML. Reframed the prose to make the _ids fields companions (for rename stability) rather than replacements.
  2. Schema-reference gap. source_session_ids / source_sub / fan_out_ids were documented in prose but missing from the Full Schema block and the Optional Fields table. Added them.
  3. Misfiring type-shape guard in cue-config-validator.ts. When source_session was undefined (which the required-field check above already errors on), the type-shape check still emitted "source_sub" must be a string when "source_session" is a string — a misleading second error against an undefined value. Wrapped the shape check in if (sourceSession !== undefined && sourceSub !== undefined). Added a regression test that asserts only the required-field error fires in that case.
  4. Loader fixture refresh. The CLI-send fixture at cue-yaml-loader.test.ts:108-136 was an agent.completed + action: command config that became semantically invalid under Harden Cue YAML command-chain validation and authoring docs #976's source_sub requirement. Added source_sub: researcher-step to keep the fixture valid; asserts it round-trips (CodeRabbit's outside-diff Quick Win).

Files changed (6)

File Change
docs/maestro-cue-configuration.md Multi-root section + schema reference catch-up
docs/maestro-cue.md Configuration File copy linking to multi-root section
docs/maestro-cue-examples.md Note on the CI-Style Pipeline example
src/prompts/_maestro-cue.md Multi-Root Pipelines section + _ids companion-field framing
src/main/cue/config/cue-config-validator.ts Skip type-shape check when source_session undefined
src/__tests__/main/cue/cue-yaml-loader.test.ts Fixture update + regression case for the misfire fix

Notes

Test plan

  • npx vitest run src/__tests__/main/cue/cue-yaml-loader.test.ts — 143 pass, 0 fail (includes the new validator regression case).
  • npx vitest run src/__tests__/renderer/components/CuePipelineEditor/utils/ — 346 pass, 0 fail.
  • Prettier clean across all touched files.
  • CI lint-and-format + test green on the latest push.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Prevented misleading duplicate validation errors for certain multi-root subscription fields.
  • Documentation

    • Clarified that each agent project root has its own .maestro/cue.yaml (no parent/workspace fallback).
    • Expanded subscription schema with UUID-aware companion fields and fan-out/chain narrowing guidance.
    • Added a “Multi-root pipelines” section explaining runtime stitching across roots.
  • Tests

    • Added regression tests to cover the validation behavior above.

Review Change Stack

chr1syy and others added 2 commits May 11, 2026 13:46
The engine reads only <projectRoot>/.maestro/cue.yaml for each agent and
never walks parents, so a single root cue.yaml cannot manage a fleet of
agents living at distinct project roots. Document the correct authoring
pattern: each subscription lives in its owning agent's local cue.yaml,
cross-agent chains stitch at runtime via source_session_ids / fan_out_ids,
and orchestration / fan-in subs live with their target agent (which is
the workspace root only when the orchestrator's cwd happens to sit
there).

- src/prompts/_maestro-cue.md: add Multi-Root Pipelines section above
  Shared Workspaces with per-role placement table and hand-authoring
  checklist; update Configuration File and Authoring Guidance to point
  at it.
- docs/maestro-cue-configuration.md: add Note callout under File
  Location and a Multi-root pipelines section after Sharing a workspace
  across agents.
- docs/maestro-cue.md: clarify Configuration File copy and link to the
  new section.
- docs/maestro-cue-examples.md: add Note on the CI-Style Pipeline
  example calling out that its three yamls are the multi-root pattern
  (vs. a same-root setup with agent_id routing in one file).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ture

Resolves doc/validator mismatch flagged by Greptile + CodeRabbit, the
misfiring shape-check guard, and the now-invalid CLI-send loader fixture.

- Validator (cue-config-validator.ts): skip the source_sub/source_session
  type-shape check when source_session is undefined. The required-field
  check above already errors; re-emitting "source_sub must be a string
  when source_session is a string" against an undefined source_session
  was misleading noise.
- Validator test: add regression case asserting only the required-field
  error fires when source_session is missing (no misleading shape error).
- Loader fixture (cue-yaml-loader.test.ts:108-136): add
  source_sub: researcher-step to the CLI-send fixture so the YAML is
  semantically valid under the new agent.completed + action: command
  rule. Assert source_sub round-trips through the loader.
- Docs (_maestro-cue.md, maestro-cue-configuration.md): reframe
  source_session_ids / fan_out_ids as REQUIRED COMPANIONS to
  source_session / fan_out, not replacements. Validator requires
  source_session on every agent.completed sub; the _ids fields are
  additional UUID arrays for rename stability (dispatcher prefers ids
  at lookup time, falls back to names).
- Schema reference (maestro-cue-configuration.md): add
  source_session_ids, source_sub, and fan_out_ids to both the Full
  Schema YAML block and the Optional Fields table — they were
  documented in prose but missing from the schema reference.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 11, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8cee668a-944b-4b4b-b664-eb4c98c648bb

📥 Commits

Reviewing files that changed from the base of the PR and between e87a8db and 9fe4f17.

📒 Files selected for processing (3)
  • src/__tests__/main/cue/cue-yaml-loader.test.ts
  • src/main/cue/config/cue-config-validator.ts
  • src/prompts/_maestro-cue.md
✅ Files skipped from review due to trivial changes (1)
  • src/prompts/_maestro-cue.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/tests/main/cue/cue-yaml-loader.test.ts
  • src/main/cue/config/cue-config-validator.ts

📝 Walkthrough

Walkthrough

This PR clarifies Cue configuration multi-root scoping by documenting that the engine reads only <projectRoot>/.maestro/cue.yaml per agent with no parent-directory fallback, expands the subscription schema with UUID companion fields and source_sub for cross-root linking, and fixes the validator to avoid redundant errors when required fields are missing.

Changes

Cue Multi-Root Configuration and Validation

Layer / File(s) Summary
Multi-Root Scoping and Configuration
docs/maestro-cue-configuration.md, src/prompts/_maestro-cue.md, docs/maestro-cue.md
Establishes that the engine reads only <projectRoot>/.maestro/cue.yaml per agent project root with no parent-directory walk or workspace fallback; separate Cue engine instances per root.
Subscription Schema and Fan-Out Fields
docs/maestro-cue-configuration.md
Adds UUID companion fields (source_session_ids, fan_out_ids), introduces source_sub for command-action chain narrowing, documents multi-root pipeline splitting/stitching via source_session/fan_out plus their _ids companions, and updates Optional Fields table.
Authoring Guidance and Examples
src/prompts/_maestro-cue.md, docs/maestro-cue-examples.md
Clarifies authoring requirement to check each participating agent's cue.yaml when pipelines span multiple project roots; CI-style pipeline example notes the per-root assumption.
Validator Logic for Source Sessions
src/main/cue/config/cue-config-validator.ts
Validator skips source_sub/source_session shape alignment checks when source_session is undefined, preventing redundant/incorrect error reporting alongside the required-field check.
Tests and Regression Checks
src/__tests__/main/cue/cue-yaml-loader.test.ts
loadCueConfig test updated to assert source_sub: 'researcher-step' preservation; new regression tests verify missing or null source_session reports only the required-field error and not a shape-consistency error.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • RunMaestro/Maestro#976: Related — both PRs modify cue subscription chain handling (source_sub/source_session validator logic), related tests, yaml→pipeline chain resolution, and documentation about command-chain behavior.

Suggested labels

ready to merge

Poem

🐰 Multi-roots unite where gardens grow,
One .yaml per patch—no parent's flow,
Companions by UUID, chains intertwine,
Validation stays calm when fields align.
Pipelines split, stitched, and cross-root combined! 🌿

🚥 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the two main changes: documentation updates for multi-root pipelines and a validator fix for missing source_session.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@greptile-apps

greptile-apps Bot commented May 11, 2026

Copy link
Copy Markdown

Greptile Summary

This PR documents the per-agent-cwd model for multi-root Cue pipelines and fixes a misleading double-error in the cue-config-validator that fired a type-shape message against an already-errored undefined source_session. No behavioral changes are introduced — only documentation, one validator guard tightening, and a regression test.

  • Docs (5 files): Adds a "Multi-root pipelines" section to maestro-cue-configuration.md and _maestro-cue.md (with authoring table and discovery checklist), updates maestro-cue.md Configuration File paragraph, annotates the CI-Style Pipeline example, and fills in source_session_ids / source_sub / fan_out_ids in the Full Schema and Optional Fields table.
  • Validator fix (cue-config-validator.ts): Wraps the source_sub/source_session type-shape consistency check in if (sourceSession !== undefined && sourceSub !== undefined), silencing the misleading secondary error when source_session is absent; variable declarations are moved inside the guard as a tidy-up.
  • Test (cue-yaml-loader.test.ts): Updates the CLI-send fixture with the now-required source_sub field and adds a targeted regression case asserting only the required-field error surfaces when source_session is missing.

Confidence Score: 5/5

Safe to merge — the validator change is a narrowly scoped guard with direct test coverage, and all doc additions are additive.

The only code change is a two-line guard in the validator that prevents a redundant error message from firing when source_session is undefined; the existing required-field check above it already covers that path. The regression test directly exercises the fixed case and asserts both that the correct error fires and that the spurious one does not. Documentation additions are thorough and consistent across all three touched doc files and the agent prompt.

No files require special attention.

Important Files Changed

Filename Overview
src/main/cue/config/cue-config-validator.ts Wraps the source_sub/source_session shape-check in a guard preventing false-positive errors when source_session is undefined; moves variable declarations inside the guard for cleanliness. Logic is correct and the new code paths are well-covered.
src/tests/main/cue/cue-yaml-loader.test.ts Adds source_sub to the CLI-send fixture to keep it valid under #976's source_sub requirement, asserts round-trip, and adds a targeted regression test that confirms only the required-field error fires when source_session is missing.
docs/maestro-cue-configuration.md Adds a Note callout under File Location, expands the Full Schema YAML and Optional Fields table with source_session_ids / source_sub / fan_out_ids, and inserts a new Multi-root pipelines section with an authoring table and prose guidance.
src/prompts/_maestro-cue.md Adds per-agent-cwd model callout near the top, updates the authoring step 1 to include multi-root awareness, and appends a full Multi-Root Pipelines section with a role table and discovery checklist for agents hand-authoring YAML.
docs/maestro-cue.md Expands the Configuration File paragraph to clarify the per-agent-cwd model and links to the new Multi-root pipelines section in the configuration reference.
docs/maestro-cue-examples.md Adds a Note callout before the CI-Style Pipeline example identifying it as the canonical multi-root pattern and pointing same-root users to the agent_id routing alternative.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["validateEventSpecificFields\n(event = agent.completed)"]
    B{"source_session\npresent?"}
    C["Push required-field error\nsource_session is required"]
    D["Validate source_session\ntype/shape"]
    E{"source_sub\npresent?"}
    F["Validate source_sub\ntype/emptiness"]
    G{"source_session !== undefined\nAND source_sub !== undefined?\n(new guard)"}
    H["Check positional alignment\n(array vs string shape, length match)"]
    I["Skip shape check\n(avoids misleading secondary error)"]

    A --> B
    B -- "falsy" --> C
    B -- "truthy" --> D
    D --> E
    E -- "yes" --> F
    E -- "no" --> G
    F --> G
    G -- "both defined" --> H
    G -- "either undefined" --> I
Loading

Reviews (1): Last reviewed commit: "Address review feedback: align docs/vali..." | Re-trigger Greptile

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🤖 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 `@docs/maestro-cue-configuration.md`:
- Line 137: The docs entry for the poll interval currently uses lowercase
`github.*`; update that token to the official product capitalization `GitHub.*`
in the table row that documents `poll_minutes` so the line reads "Poll interval
for `GitHub.*` (default 5) and `task.pending` (default 1)"; ensure only the
token `github.*` is changed to `GitHub.*` and surrounding text/formatting for
`poll_minutes` remains unchanged.

In `@src/main/cue/config/cue-config-validator.ts`:
- Around line 376-389: The current guard only skips when sourceSession/sourceSub
are undefined, so null values can still produce misleading secondary shape
errors; update the conditional that decides whether to run the shape-comparison
block to treat null the same as undefined (e.g., check for sourceSession == null
or sourceSub == null or use !== null/!== undefined checks), and ensure you only
call Array.isArray(...) and push shape errors when both sourceSession and
sourceSub are non-null/non-undefined; reference the sourceSession, sourceSub,
errors and prefix variables in the existing block to apply this stricter
presence check before performing array/string shape validation.

In `@src/prompts/_maestro-cue.md`:
- Around line 15-16: The statement that the engine reads "ONLY
`<projectRoot>/.maestro/cue.yaml`" conflicts with the earlier canonical-first +
legacy-fallback guidance; update the wording in _maestro-cue.md to clarify that
the engine prefers the canonical `<projectRoot>/.maestro/cue.yaml` but still
supports legacy-fallback behavior described earlier (i.e., canonical-first then
legacy-fallback) so prompt-driven edits won't ignore valid legacy configs; keep
the single-root guidance for authoring pipelines but remove the absolute "ONLY"
wording and reference the canonical-first + legacy-fallback rule by name.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a1d456aa-d9ae-4025-92ad-cf60328324f5

📥 Commits

Reviewing files that changed from the base of the PR and between 56ece61 and e87a8db.

📒 Files selected for processing (6)
  • docs/maestro-cue-configuration.md
  • docs/maestro-cue-examples.md
  • docs/maestro-cue.md
  • src/__tests__/main/cue/cue-yaml-loader.test.ts
  • src/main/cue/config/cue-config-validator.ts
  • src/prompts/_maestro-cue.md

Comment thread docs/maestro-cue-configuration.md
Comment thread src/main/cue/config/cue-config-validator.ts Outdated
Comment thread src/prompts/_maestro-cue.md Outdated
…ng "ONLY" wording

- Validator (cue-config-validator.ts): widen the shape-check guard from
  `!== undefined` to `!= null` so explicit YAML `null` (e.g. `source_session: ~`)
  is treated the same as a missing key. The required-field check above
  errors on both shapes; emitting a secondary "must be a string when
  source_session is a string" error against null is the same misleading
  noise the original fix removed for `undefined`.
- Validator test: add regression case for the explicit-null shape with
  the same assertions as the undefined case.
- Prompt (_maestro-cue.md): replace "engine reads ONLY
  <projectRoot>/.maestro/cue.yaml" with wording that defers to the
  canonical-first + legacy-fallback rule documented earlier in the same
  doc. Keeps the per-agent-cwd point (no parent walk, no aggregation
  across roots) without contradicting the legacy-path support.

CodeRabbit's third inline (github.* -> GitHub.*) was skipped: `github.*`
in that table row is a glob over the literal event-type tokens
`github.pull_request` / `github.issue`, which are lowercase YAML
identifiers — not the GitHub product name. Capitalizing the token would
make the docs disagree with the schema.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@chr1syy

chr1syy commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @coderabbitai — pushed 9fe4f171 addressing the actionable findings:

  • cue-config-validator.ts null-equivalence ✓ — widened the guard from !== undefined to != null so a YAML source_session: ~ (which parses to null) is treated the same as an absent key. Added a regression test for the explicit-null shape alongside the existing undefined case (cue-yaml-loader.test.ts now 144 pass / 0 fail).
  • _maestro-cue.md "ONLY" wording ✓ — replaced the absolute "engine reads ONLY <projectRoot>/.maestro/cue.yaml" with wording that defers to the canonical-first + legacy-fallback rule documented earlier in the same file. The per-agent-cwd point (no parent walk, no aggregation across roots) is preserved, and the legacy fallback isn't contradicted.

Skipped with justification:

  • docs/maestro-cue-configuration.md:137 github.*GitHub.* — the token in that table row is a glob over the literal event-type identifiers github.pull_request and github.issue, which are lowercase YAML strings (see src/shared/cue/contracts.ts:CUE_EVENT_TYPES and the Event Types tables in maestro-cue.md / _maestro-cue.md). Capitalizing the token would make the docs disagree with the schema and with every other place those event types are referenced. Holding github.* as-is.

@coderabbitai

coderabbitai Bot commented May 11, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@chr1syy: All three items look good — thanks for the thorough follow-up.

  • Null-equivalence guard (!= null) — correct call. != null covers both undefined (absent key) and null (source_session: ~) in one idiomatic check, and the regression test for the explicit-null shape is exactly what's needed here. ✅

  • "ONLY" wording in _maestro-cue.md — the revised phrasing is cleaner: it preserves the no-parent-walk / no-cross-root-aggregation constraint while correctly deferring to the canonical-first + legacy-fallback rule already documented in the file. ✅

  • github.* capitalization skip — your reasoning is correct and I was wrong to flag it. Those tokens are literal lowercase event-type identifiers (github.pull_request, github.issue) defined in CUE_EVENT_TYPES; capitalizing them in the docs would introduce a factual inconsistency with the schema. Holding github.* as-is is the right call. ✅


✏️ Learnings added
Learnt from: chr1syy
URL: https://github.com/RunMaestro/Maestro/pull/987

Timestamp: 2026-05-11T12:25:10.999Z
Learning: In the RunMaestro/Maestro codebase, Cue event type identifiers such as `github.pull_request` and `github.issue` are defined as lowercase YAML strings in `src/shared/cue/contracts.ts` (`CUE_EVENT_TYPES`). The glob pattern `github.*` used in documentation tables (e.g., `docs/maestro-cue-configuration.md`) is intentionally lowercase to match these identifiers and should NOT be capitalized to `GitHub.*`, as that would contradict the schema and all other references.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@pedramamini

Copy link
Copy Markdown
Collaborator

Thanks @chr1syy — appreciate the careful follow-up on this one, especially the principled pushback on the github.* capitalization (you were right; those are literal lowercase event-type identifiers).

Reviewed the diff end-to-end:

  • Docs read clean and consistent across maestro-cue.md, maestro-cue-configuration.md, maestro-cue-examples.md, and _maestro-cue.md. The per-agent-cwd model is now explicit, the multi-root authoring table + checklist is genuinely useful for hand-authored YAML, and the source_session_ids / source_sub / fan_out_ids schema catch-up closes the prose/schema gap.
  • Validator guard in cue-config-validator.ts is correctly scoped — != null covers both undefined and source_session: ~ (explicit null) in one idiomatic check, and the type-shape comparison is properly skipped only when there's nothing meaningful to compare against. No risk of masking real shape mismatches.
  • Tests in cue-yaml-loader.test.ts — fixture refresh keeps the CLI-send case valid under Harden Cue YAML command-chain validation and authoring docs #976's source_sub requirement, and the two regression assertions (undefined and explicit-null source_session) lock in exactly the misfire being fixed.

Replacing #986 with a clean history was the right call too. Approving and labeling.

(CI lint-and-format + test are still in flight as I write this — assuming those go green, this is good to merge.)

@chr1syy chr1syy added the ready to merge This PR is ready to merge label May 11, 2026
@pedramamini
pedramamini merged commit c21fc20 into RunMaestro:rc May 13, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved ready to merge This PR is ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants