feat(commands): add /plan-orchestrate — generate /orchestrate prompts from plan docs#1603
Closed
d0m999 wants to merge 4 commits into
Closed
feat(commands): add /plan-orchestrate — generate /orchestrate prompts from plan docs#1603d0m999 wants to merge 4 commits into
d0m999 wants to merge 4 commits into
Conversation
Reads a plan document, decomposes it into steps, designs a per-step agent chain, and emits ready-to-paste /orchestrate custom prompts. The command is generative only — it never invokes /orchestrate itself. Detects plugin vs legacy ECC install and emits matching slash-command and agent-name prefixes.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a new skill specification that converts a user plan into ready-to-paste Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Skill as Plan-Orchestrate Skill
participant ModeDetect as ECC Mode Detector
participant LangDetect as Language Detector
participant Decomposer
participant Tagger
participant Composer as Chain Composer
participant Renderer as Markdown Renderer
User->>Skill: submit plan path & options
Skill->>ModeDetect: detect ECC install mode
ModeDetect-->>Skill: mode (plugin|legacy) (frozen)
Skill->>LangDetect: analyze files / detect --lang (polyglot tie-break)
LangDetect-->>Skill: resolved language (or unknown)
Skill->>Decomposer: decompose plan into ordered steps
Decomposer-->>Skill: steps + metadata
Skill->>Tagger: tag steps via trigger-word matching
Tagger-->>Skill: tags per step
Skill->>Composer: build agent chains (dedupe, multi-tag rules, gating)
Composer-->>Skill: per-step chain definitions
Skill->>Renderer: render Markdown + bash blocks + self-check
Renderer-->>User: final output document (overview + per-step + batch)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
commands/plan-orchestrate.md (1)
132-136: Minor: consider rewording repeated “If ...” phrasing for style tools.Around line 134-135 there are multiple successive “If …” sentences. A style tool hint indicates this may trigger a “repeated sentence beginning” rule.
Not a functional blocker, but worth a quick polish if you care about style/lint noise.
Suggested fix (vary sentence beginnings)
- 4. If `--lang=auto` failed, `<lang>-reviewer` falls back to `code-reviewer`. - 5. If `<lang>-build-resolver` does not exist, fall back to `build-error-resolver`. + 4. When `--lang=auto` fails, `<lang>-reviewer` falls back to `code-reviewer`. + 5. When `<lang>-build-resolver` does not exist, fall back to `build-error-resolver`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@commands/plan-orchestrate.md` around lines 132 - 136, The list has repetitive sentence starts using "If ..." in items 4 and 5; edit the wording in the plan rules so sentence openings vary for style while preserving meaning — for example rephrase item 4 to "When `--lang=auto` fails, `<lang>-reviewer` falls back to `code-reviewer`" and item 5 to "When `<lang>-build-resolver` is missing, fall back to `build-error-resolver`" (keep items 2–6 semantics intact, including symbols `impl`, `db`, `tdd-guide,database-reviewer,<lang>-reviewer`, chain cap of 4, dropping `lookup`/`docs` first, `planner`/`architect` pairing rule).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@commands/plan-orchestrate.md`:
- Around line 217-218: Clarify the contradictory statements about count by
deciding and documenting exactly which count `--scope` affects: either the count
of rows in the overview table or the count of emitted per-step detail blocks;
update the lines referencing “The overview table and per-step sections agree on
count.” and the `--scope` note to state explicitly that `--scope` controls the
number of per-step detail blocks emitted while the overview table always lists
the full plan (or vice versa, pick the intended behavior), and add a short
parenthetical defining “count” (e.g., “count of detail blocks” vs “overview
rows”) so validators and generators have a single unambiguous interpretation of
`--scope` behavior.
- Around line 118-128: The doc’s invariant "Any step that produces code must end
the chain with a reviewer." conflicts with the default chains for the `test` and
`build` steps (currently `tdd-guide,e2e-runner` and `<lang>-build-resolver`);
fix by either appending a reviewer to those chains (e.g., change `test` to
`tdd-guide,e2e-runner,<lang>-reviewer` and `build` to
`<lang>-build-resolver,<lang>-reviewer`) or explicitly redefine "reviewer" in
the doc to include `e2e-runner` and `<lang>-build-resolver` and update the
invariant wording to reference that expanded definition so the invariant and
defaults remain consistent.
- Around line 130-137: Clarify deterministic chain composition by specifying how
to pick the primary tag and a full, ordered priority for dropping tags when
length >4: update the "primary tag" selection rule to pick the tag with highest
priority from a new tag-priority list (e.g., impl > design > security > db >
docs > lookup > lang-specific > build > test; tie-break by lexical order) and
extend rule 3 to drop tags in this explicit weakest-to-strongest order (with
lookup and docs first), also add fallback tie-break behavior (alphabetical) and
note that the primary tag wins when present; apply these changes to the "primary
tag" wording in Phase 2 and to the chain-length/drop rules so chain construction
is fully deterministic (reference terms: "primary tag", "drop weakest", rule 3,
Phase 2).
---
Nitpick comments:
In `@commands/plan-orchestrate.md`:
- Around line 132-136: The list has repetitive sentence starts using "If ..." in
items 4 and 5; edit the wording in the plan rules so sentence openings vary for
style while preserving meaning — for example rephrase item 4 to "When
`--lang=auto` fails, `<lang>-reviewer` falls back to `code-reviewer`" and item 5
to "When `<lang>-build-resolver` is missing, fall back to
`build-error-resolver`" (keep items 2–6 semantics intact, including symbols
`impl`, `db`, `tdd-guide,database-reviewer,<lang>-reviewer`, chain cap of 4,
dropping `lookup`/`docs` first, `planner`/`architect` pairing rule).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f0917f91-8ba1-44d2-95f2-9a5fd127eb41
📒 Files selected for processing (1)
commands/plan-orchestrate.md
Comment on lines
+118
to
+128
| | `impl` | implement, build, add, create, port / 实现、编码、落地、写 | `tdd-guide,<lang>-reviewer` | | ||
| | `test` | test, coverage, e2e, integration / 测试、覆盖率、单测、集成、E2E | `tdd-guide,e2e-runner` | | ||
| | `refactor` | refactor, cleanup, dedupe, split / 重构、清理、拆分、合并、去重 | `architect,refactor-cleaner,<lang>-reviewer` | | ||
| | `migration` | migrate, upgrade, rewrite, port / 迁移、重写、搬运、升级 | `architect,tdd-guide,<lang>-reviewer` | | ||
| | `db` | schema, migration, index, SQL, Postgres, alembic, sqlmodel | `database-reviewer,<lang>-reviewer` | | ||
| | `security` | encrypt, auth, secret, OWASP, PII / 安全、加密、密钥、认证、授权 | `security-reviewer,<lang>-reviewer` | | ||
| | `build` | build, compile, lint failure / 构建、编译、CI、打包错误 | `<lang>-build-resolver` (falls back to `build-error-resolver`) | | ||
| | `docs` | docs, readme, codemap, changelog / 文档、README | `doc-updater` | | ||
| | `lookup` | lookup, reference, API usage / 调研、文档查询 | `docs-lookup` | | ||
| | `review` | review, audit, verify / 评审、审查、check | `<lang>-reviewer,code-reviewer` | | ||
| | `loop` | loop, autonomous, watchdog / 长跑、循环、守护 | `loop-operator` | |
Contributor
There was a problem hiding this comment.
Make “code-producing steps end with a reviewer” consistent with test and build default chains.
Your invariant at line 137 (“Any step that produces code must end the chain with a reviewer.”) is enforced again in Phase 5 at line 216, but the default chains at:
test: line 119tdd-guide,e2e-runner(ends withe2e-runner, not a*-reviewer/code-reviewer/security-reviewer)build: line 124<lang>-build-resolver(ends with a build resolver, not a reviewer)
If the implementation literally treats “reviewer” as “<lang>-reviewer / code-reviewer / security-reviewer / etc.”, then the invariant is internally violated by the doc’s own default chain definitions.
Suggested fix (end chains with `-reviewer`)
| Tag | Trigger words (English + Chinese, since plans are often written in either language) | Default chain |
|---|---|---|
@@
-| `test` | test, coverage, e2e, integration / 测试、覆盖率、单测、集成、E2E | `tdd-guide,e2e-runner` |
+| `test` | test, coverage, e2e, integration / 测试、覆盖率、单测、集成、E2E | `tdd-guide,e2e-runner,<lang>-reviewer` |
@@
-| `build` | build, compile, lint failure / 构建、编译、CI、打包错误 | `<lang>-build-resolver` (falls back to `build-error-resolver`) |
+| `build` | build, compile, lint failure / 构建、编译、CI、打包错误 | `<lang>-build-resolver,<lang>-reviewer` (falls back to `build-error-resolver,<lang>-reviewer`) |If you instead intended that e2e-runner and *-build-resolver count as “reviewers” for the invariant, then you should explicitly redefine what “reviewer” means in this doc (and update the self-check wording) to remove ambiguity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@commands/plan-orchestrate.md` around lines 118 - 128, The doc’s invariant
"Any step that produces code must end the chain with a reviewer." conflicts with
the default chains for the `test` and `build` steps (currently
`tdd-guide,e2e-runner` and `<lang>-build-resolver`); fix by either appending a
reviewer to those chains (e.g., change `test` to
`tdd-guide,e2e-runner,<lang>-reviewer` and `build` to
`<lang>-build-resolver,<lang>-reviewer`) or explicitly redefine "reviewer" in
the doc to include `e2e-runner` and `<lang>-build-resolver` and update the
invariant wording to reference that expanded definition so the invariant and
defaults remain consistent.
Comment on lines
+130
to
+137
| Chain composition rules: | ||
| 1. `impl` + `security` → `tdd-guide,<lang>-reviewer,security-reviewer`. | ||
| 2. `impl` + `db` → `tdd-guide,database-reviewer,<lang>-reviewer`. | ||
| 3. Chain length is capped at 4. If exceeded, drop the weakest tag (`lookup` and `docs` go first). | ||
| 4. If `--lang=auto` failed, `<lang>-reviewer` falls back to `code-reviewer`. | ||
| 5. If `<lang>-build-resolver` does not exist, fall back to `build-error-resolver`. | ||
| 6. **Do not** put both `planner` and `architect` in an `impl` chain — that wastes tokens. Pair them only on `design` steps. | ||
| 7. Any step that produces code must end the chain with a reviewer. |
Contributor
There was a problem hiding this comment.
Clarify deterministic chain composition when multiple tags apply (primary tag + “drop weakest” beyond lookup/docs).
Phase 2 says a step “may carry multiple tags; the chain is built from the primary tag and stacked as needed” (line 112-113), but it never defines:
- How to choose the primary tag when multiple tags match.
- How to choose which tags are “weakest” if the chain is still too long after dropping
lookup/docs(line 133 only specifies that those go first).
This can lead to non-deterministic output or inconsistent chains across runs/LLMs.
Suggested fix (define tag priority + tie-breaks)
- 3. If exceeded, drop the weakest tag (`lookup` and `docs` go first).
+ 3. If exceeded, drop the weakest tag by this priority (highest drops first): `lookup`, `docs`, `plan`, `loop`, `review`, `design`, `refactor`, `migration`, `build`, `test`, `impl`, `db`, `security`.
+ - After dropping, re-apply the “end with a reviewer” rule for code-producing steps.
+ - If ties remain, prefer dropping earlier (leftmost) in the composed chain.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Chain composition rules: | |
| 1. `impl` + `security` → `tdd-guide,<lang>-reviewer,security-reviewer`. | |
| 2. `impl` + `db` → `tdd-guide,database-reviewer,<lang>-reviewer`. | |
| 3. Chain length is capped at 4. If exceeded, drop the weakest tag (`lookup` and `docs` go first). | |
| 4. If `--lang=auto` failed, `<lang>-reviewer` falls back to `code-reviewer`. | |
| 5. If `<lang>-build-resolver` does not exist, fall back to `build-error-resolver`. | |
| 6. **Do not** put both `planner` and `architect` in an `impl` chain — that wastes tokens. Pair them only on `design` steps. | |
| 7. Any step that produces code must end the chain with a reviewer. | |
| Chain composition rules: | |
| 1. `impl` + `security` → `tdd-guide,<lang>-reviewer,security-reviewer`. | |
| 2. `impl` + `db` → `tdd-guide,database-reviewer,<lang>-reviewer`. | |
| 3. Chain length is capped at 4. If exceeded, drop the weakest tag by this priority (highest drops first): `lookup`, `docs`, `plan`, `loop`, `review`, `design`, `refactor`, `migration`, `build`, `test`, `impl`, `db`, `security`. | |
| - After dropping, re-apply the "end with a reviewer" rule for code-producing steps. | |
| - If ties remain, prefer dropping earlier (leftmost) in the composed chain. | |
| 4. If `--lang=auto` failed, `<lang>-reviewer` falls back to `code-reviewer`. | |
| 5. If `<lang>-build-resolver` does not exist, fall back to `build-error-resolver`. | |
| 6. **Do not** put both `planner` and `architect` in an `impl` chain — that wastes tokens. Pair them only on `design` steps. | |
| 7. Any step that produces code must end the chain with a reviewer. |
🧰 Tools
🪛 LanguageTool
[style] ~135-~135: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ewerfalls back tocode-reviewer. 5. If -build-resolver` does not exist,...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@commands/plan-orchestrate.md` around lines 130 - 137, Clarify deterministic
chain composition by specifying how to pick the primary tag and a full, ordered
priority for dropping tags when length >4: update the "primary tag" selection
rule to pick the tag with highest priority from a new tag-priority list (e.g.,
impl > design > security > db > docs > lookup > lang-specific > build > test;
tie-break by lexical order) and extend rule 3 to drop tags in this explicit
weakest-to-strongest order (with lookup and docs first), also add fallback
tie-break behavior (alphabetical) and note that the primary tag wins when
present; apply these changes to the "primary tag" wording in Phase 2 and to the
chain-length/drop rules so chain construction is fully deterministic (reference
terms: "primary tag", "drop weakest", rule 3, Phase 2).
Comment on lines
+217
to
+218
| - [ ] The overview table and per-step sections agree on count. | ||
| - [ ] `--scope` is honoured (e.g. `step:3` emits one detail block, but the overview table still lists the full plan). |
Contributor
There was a problem hiding this comment.
Fix contradiction in --scope self-check vs overview behavior.
Line 217 says: “The overview table and per-step sections agree on count.”
But line 218 says: “--scope is honoured … but the overview table still lists the full plan.”
Those two statements can’t both be true at the same time unless you explicitly define what “count” refers to (e.g., count of emitted detail blocks vs count of rows in the overview table). This can cause the command generator (or a validator) to disagree with its own doc.
Suggested fix (clarify what’s counted)
- - [ ] The overview table and per-step sections agree on count.
+ - [ ] The overview table and per-step sections agree on which steps are covered by `--scope` (overview may list full plan; detail blocks must match emitted scope).
@@
- - [ ] `--scope` is honoured (e.g. `step:3` emits one detail block, but the overview table still lists the full plan).
+ - [ ] `--scope` is honoured (e.g. `step:3` emits one detail block; the overview table still lists the full plan).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - [ ] The overview table and per-step sections agree on count. | |
| - [ ] `--scope` is honoured (e.g. `step:3` emits one detail block, but the overview table still lists the full plan). | |
| - [ ] The overview table and per-step sections agree on which steps are covered by `--scope` (overview may list full plan; detail blocks must match emitted scope). | |
| - [ ] `--scope` is honoured (e.g. `step:3` emits one detail block; the overview table still lists the full plan). |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@commands/plan-orchestrate.md` around lines 217 - 218, Clarify the
contradictory statements about count by deciding and documenting exactly which
count `--scope` affects: either the count of rows in the overview table or the
count of emitted per-step detail blocks; update the lines referencing “The
overview table and per-step sections agree on count.” and the `--scope` note to
state explicitly that `--scope` controls the number of per-step detail blocks
emitted while the overview table always lists the full plan (or vice versa, pick
the intended behavior), and add a short parenthetical defining “count” (e.g.,
“count of detail blocks” vs “overview rows”) so validators and generators have a
single unambiguous interpretation of `--scope` behavior.
Contributor
Greptile SummaryAdds Confidence Score: 5/5Safe to merge — all findings are P2 documentation/logic-clarity issues with no runtime breakage. No P0 or P1 findings. The three issues (dead-code clause, underdefined 3-tag rule, shared 'build' trigger) affect output determinism and correctness in specific edge cases but do not crash or corrupt anything. The skill is generative-only, so worst-case a user pastes a slightly wrong chain and notices before running it. skills/plan-orchestrate/SKILL.md — Phase 2 chain composition rules and Phase 0 detection algorithm Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User invokes plan-orchestrate] --> B[Phase 0: Detect ECC mode + language]
B --> C{Plugin dir exists?}
C -->|Yes| D[ECC_MODE=plugin]
C -->|No| E{Legacy agents exist?}
E -->|Yes| F[ECC_MODE=legacy]
E -->|No| G[ECC_MODE=legacy + warning]
D & F & G --> H[Resolve --lang / polyglot tie-break]
H --> I[Phase 1: Decompose plan into steps]
I --> J[Phase 2: Tag each step + compose chain]
J --> K{Multi-tag?}
K -->|impl+security| L[tdd-guide, lang-reviewer, security-reviewer]
K -->|impl+db| M[tdd-guide, database-reviewer, lang-reviewer]
K -->|impl+security+db| N[underdefined — see Example 1]
K -->|Single tag| O[Default chain from table]
L & M & N & O --> P[Dedup + enforce length ≤ 4]
P --> Q[Phase 3: Compress task description]
Q --> R[Phase 4: Render output with ECC_MODE prefix]
R --> S[Phase 5: Self-check]
S --> T[Emit Markdown + Batch block]
Reviews (4): Last reviewed commit: "fix(plan-orchestrate): broaden reviewer-..." | Re-trigger Greptile |
Contributor
There was a problem hiding this comment.
4 issues found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="commands/plan-orchestrate.md">
<violation number="1" location="commands/plan-orchestrate.md:80">
P2: ECC install detection hardcodes Unix-style `~/.claude/...` paths, so Windows users can be misdetected and receive invalid legacy-form output.</violation>
<violation number="2" location="commands/plan-orchestrate.md:119">
P2: Internal inconsistency: rule 7 states "Any step that produces code must end the chain with a reviewer" but the default chains for `test` (`tdd-guide,e2e-runner`) and `build` (`<lang>-build-resolver`) both end with non-reviewer agents. Either append `<lang>-reviewer` to these default chains, or narrow the invariant to clarify which step tags are considered "code-producing" (excluding `test` and `build` if intended).</violation>
<violation number="3" location="commands/plan-orchestrate.md:196">
P3: Nested fenced examples reuse the same triple-backtick fence length, so the outer markdown code block closes early and the examples render incorrectly.</violation>
<violation number="4" location="commands/plan-orchestrate.md:218">
P2: Self-check contradiction: the checklist simultaneously requires the overview table and per-step sections to "agree on count" (previous item) while also stating the overview table lists the full plan even when `--scope` limits emitted detail blocks. These can't both be true unless you clarify what "count" refers to (e.g., the overview may list the full plan, but the detail blocks must match the emitted scope).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Address PR affaan-m#1603 review feedback: - P2: Move from commands/ to skills/plan-orchestrate/SKILL.md per the Workflow Surface Policy in AGENTS.md (skills/ is the canonical workflow surface; commands/ is for legacy compatibility shims). - P1: Phase 4 output template was hardcoded to plugin form (overview table, "# plugin form" comment, "strip the prefix" blockquote). Replaced with ECC_MODE-neutral {ORCH_CMD} / {AGENT(...)} placeholder notation; the runtime emits exactly one resolved form throughout the output, never both, never with viewer-facing instructions. - Polyglot --lang=auto: when multiple language markers match, pick the dominant language by source file count; on tie or below 60% threshold, fall back to code-reviewer. - Reformat as SKILL.md with required sections (When to Activate, How It Works, Examples).
0d1f94c to
4a09d25
Compare
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="skills/plan-orchestrate/SKILL.md">
<violation number="1">
P2: `pytorch-build-resolver` is documented in the catalogue but cannot be reached by the documented `--lang`/`<lang>-build-resolver` selection logic.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
skills/plan-orchestrate/SKILL.md (1)
203-204: Minor style suggestions from static analysis.Line 203: "Very large plan" could be simplified to "Large plan (>1500 lines)".
Line 204: "N.a / N.b" could use "N.a and N.b" for clarity.Both are optional style improvements.
✨ Optional style improvements
-- **Very large plan (>1500 lines)**: emit the overview table first and ask the user to narrow with `--scope` before generating details. +- **Large plan (>1500 lines)**: emit the overview table first and ask the user to narrow with `--scope` before generating details. -- **Step too broad** (e.g. "complete all backend work"): do not force a single chain. Suggest splitting into N.a / N.b and propose a split. +- **Step too broad** (e.g. "complete all backend work"): do not force a single chain. Suggest splitting into N.a and N.b and propose a split.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/plan-orchestrate/SKILL.md` around lines 203 - 204, Update the two bullet texts in SKILL.md: change "Very large plan (>1500 lines)" to "Large plan (>1500 lines)" and change occurrences of "N.a / N.b" to "N.a and N.b" so the guidance reads more concise and clearer; locate the bullets containing those exact phrases and edit them in-place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@skills/plan-orchestrate/SKILL.md`:
- Around line 203-204: Update the two bullet texts in SKILL.md: change "Very
large plan (>1500 lines)" to "Large plan (>1500 lines)" and change occurrences
of "N.a / N.b" to "N.a and N.b" so the guidance reads more concise and clearer;
locate the bullets containing those exact phrases and edit them in-place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 368c37f4-3b29-4f79-b31f-97f2c8359fef
📒 Files selected for processing (1)
skills/plan-orchestrate/SKILL.md
ECC install detection - Resolve ~/.claude paths via <claude-home> placeholder so detection works on Windows (%USERPROFILE%\.claude) (cubic P2) - Spell out the detection algorithm with priority order, mixed-install precedence (plugin wins), and a default-to-legacy fallback warning Language detection - lang=unknown is a sentinel, not the agent name code-reviewer. Phase 2 rules 5-6 resolve unknown to code-reviewer / build-error-resolver - Polyglot tie-break excludes vendor/test/dist/generated dirs - Add PyTorch sub-profile so pytorch-build-resolver is reachable when torch is a dep (cubic P2) Phase 2 chain composition - Define primary tag selection by table-order priority - Add deduplication rule (avoids code-reviewer,code-reviewer) - Add zero-tag default chain (code-reviewer with explicit rationale) - Narrow reviewer invariant to impl/refactor/migration (cubic P2) Output and self-check - Strip everything-claude-code: prefix from plan-declared agent names before catalogue validation (no double-prefix in plugin mode) - Out of scope clause is optional (only when plan declares one) - Overview-only mode (large plan) skips per-step blocks and Batch block - Self-check covers dedup, normalization, zero-tag, overview-only mode Other - Simplify trigger word table to a single canonical language; document multilingual matching by stem - Use 4-backtick outer fence for nested example (cubic P3) - Apply CodeRabbit nitpicks (Large plan; N.a and N.b) - Drop --mode future-extension hint that contradicted the no-invented-flag rule
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
You’re at about 93% of the monthly review limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="skills/plan-orchestrate/SKILL.md">
<violation number="1" location="skills/plan-orchestrate/SKILL.md:146">
P2: Rule 2 (`impl + security`) produces a chain ending with `security-reviewer`, but Rule 10 requires impl/refactor/migration steps to end with `<lang>-reviewer` or `code-reviewer`. These rules are internally contradictory — chains produced by Rule 2 will fail the Rule 10 / Phase 5 self-check.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
… conflict cubic flagged: rule 2 (impl+security) ends with security-reviewer, but rule 10 required impl/refactor/migration to end with <lang>-reviewer or code-reviewer — Phase 5 self-check would fail. Broaden rule 10 (and the matching self-check) so reviewer-class includes <lang>-reviewer, code-reviewer, security-reviewer, and database-reviewer. Domain-specific reviewers correctly win the tail position (security-reviewer closes security-tagged work).
Author
|
Thanks for the bot reviews — pushed two follow-up commits addressing the findings:
Greptile (
|
Owner
|
Thanks for the PR. This has been idle for a while and is not merge-ready against the moving main branch, so I am closing it to keep the queue tractable. Reopen is welcome with a current rebase and focused scope; maintainers may also port the useful parts into a fresh PR where appropriate. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
/plan-orchestrate, a command that reads a plan document, decomposes it into steps, picks a per-step agent chain from the existing ECC catalogue, and emits one ready-to-paste/orchestrate custominvocation per step.The command is generative only — it never invokes
/orchestrateitself. The user stays in the loop and decides when (and whether) to execute each step.Why this is high-frequency for everyone
security-reviewer, wheredatabase-reviewerbelongs) is the actual hard part of/orchestrate. They tend to either skip/orchestrateor ask Claude to "just figure it out", which loses determinism. This command makes the chain choice explicit and pasteable.What "AI-native" means here
The plan → orchestration handoff already exists conceptually in this repo (
commands/plan.md,commands/orchestrate.md,skills/dmux-workflows/). What's missing is the bridge that emits the literal command string a user pastes next. This PR closes that gap so the workflow stays in working memory end-to-end:plan doc → /plan-orchestrate → N pasteable lines → /orchestrate custom per stepWhat it does
everything-claude-code:prefix to both the slash command and every agent name. The two must stay in sync; under plugin install, bare agent names force fuzzy matching, which fails intermittently under parallel calls.pyproject.toml/package.json/go.mod/Cargo.toml/ etc.) to pick the right<lang>-reviewerand<lang>-build-resolver.design,impl,test,migration,db,security, ...) and composes a chain ≤ 4 agents long to keep token cost predictable.--mode/--gate/--agents=...); task descriptions are single-line and properly escaped.Type
Testing
node tests/run-all.js— passnpx markdownlint-cli commands/plan-orchestrate.md— pass/orchestrate customshape documented incommands/orchestrate.mdandskills/dmux-workflows/SKILL.mdChecklist
description:frontmatter + sections)Summary by cubic
Adds
plan-orchestrate(inskills/plan-orchestrate), a skill that reads a plan doc and emits ready-to-paste/orchestrate customcommands per step without executing them. It auto-detects ECC install (plugin vs legacy) and outputs a single consistent form for the slash command and agent names./orchestrateand agent names; normalizes any pre-prefixed agents from the plan to avoid double-prefixing.code-reviewer; ensuresimpl/refactor/migrationend with a reviewer-class agent (<lang>-reviewer,code-reviewer,security-reviewer, ordatabase-reviewer), letting domain-specific reviewers close when applicable.--lang=<...|auto>with a 60% polyglot tie-break andlang=unknownfallback (incl. PyTorch build resolver),--scope=all|step:n|range:a-b, and--dry-run; emits an overview, per-step one-liners with rationale, optional batch block, overview-only mode for very large plans, and runs strict self-checks (no invented flags).Written for commit af61c73. Summary will update on new commits. Review in cubic
Summary by CodeRabbit