docs(cue): multi-root pipelines guide; fix validator misfire on missing source_session - #987
Conversation
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>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR clarifies Cue configuration multi-root scoping by documenting that the engine reads only ChangesCue Multi-Root Configuration and Validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR documents the per-agent-cwd model for multi-root Cue pipelines and fixes a misleading double-error in the
Confidence Score: 5/5Safe 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
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
Reviews (1): Last reviewed commit: "Address review feedback: align docs/vali..." | Re-trigger Greptile |
There was a problem hiding this comment.
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
📒 Files selected for processing (6)
docs/maestro-cue-configuration.mddocs/maestro-cue-examples.mddocs/maestro-cue.mdsrc/__tests__/main/cue/cue-yaml-loader.test.tssrc/main/cue/config/cue-config-validator.tssrc/prompts/_maestro-cue.md
…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>
|
Thanks @coderabbitai — pushed
Skipped with justification:
|
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
|
|
Thanks @chr1syy — appreciate the careful follow-up on this one, especially the principled pushback on the Reviewed the diff end-to-end:
Replacing #986 with a clean history was the right call too. Approving and labeling. (CI |
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.yamlfor 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 viaagent_idreferences insource_session_ids/fan_out_ids. The visual Pipeline Editor handles this automatically; agents authoring YAML by hand need explicit guidance.Added:
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.<Note>callout under File Location inmaestro-cue-configuration.mdlinking to the new section.maestro-cue.mdclarifying the per-agent-cwd model.<Note>on the CI-Style Pipeline example inmaestro-cue-examples.mdannotating it as the canonical multi-root pattern (three separate yamls) vs. the same-root case withagent_idrouting in one file.Validator misfire fix + schema-reference catch-up (commit 2)
Greptile and CodeRabbit flagged two issues downstream of PR #976's
source_subvalidator rule (already onrc):source_session_idsoversource_session" — but the validator (correctly) requiressource_sessionon everyagent.completedsub. An agent reading the guidance literally would write invalid YAML. Reframed the prose to make the_idsfields companions (for rename stability) rather than replacements.source_session_ids/source_sub/fan_out_idswere documented in prose but missing from the Full Schema block and the Optional Fields table. Added them.cue-config-validator.ts. Whensource_sessionwas 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 inif (sourceSession !== undefined && sourceSub !== undefined). Added a regression test that asserts only the required-field error fires in that case.cue-yaml-loader.test.ts:108-136was anagent.completed + action: commandconfig that became semantically invalid under Harden Cue YAML command-chain validation and authoring docs #976'ssource_subrequirement. Addedsource_sub: researcher-stepto keep the fixture valid; asserts it round-trips (CodeRabbit's outside-diff Quick Win).Files changed (6)
docs/maestro-cue-configuration.mddocs/maestro-cue.mddocs/maestro-cue-examples.mdsrc/prompts/_maestro-cue.md_idscompanion-field framingsrc/main/cue/config/cue-config-validator.tssource_sessionundefinedsrc/__tests__/main/cue/cue-yaml-loader.test.tsNotes
b9f5b401,a8cb55f5) duplicating work already onrcvia Harden Cue YAML command-chain validation and authoring docs #976. Same final tree, clean history.source_subrequirement and the new validator/renderer rules pedram and bots discussed all live onrcalready via Harden Cue YAML command-chain validation and authoring docs #976.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.lint-and-format+testgreen on the latest push.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation
Tests