refactor(plugins): apply plugin-dev best practices v2#96
Conversation
Fresh approach replacing stale PR #79. Three phases: A) Metadata compliance (color, version, examples) B) Reference extraction from bloated commands C) Superpowers-style task/team patterns Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
15 tasks across 3 phases: - Phase A (Tasks 1-4): Metadata compliance - Phase B (Tasks 5-12): Reference extraction and command slimming - Phase C (Tasks 13-14): Task/team patterns and sequential review - Task 15: Validation and PR Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…compliance Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ogic Extract procedural logic from bloated commands into references/ directory: - coordinator-pattern.md (732 lines) - task delegation from implement.md - failure-recovery.md (539 lines) - retry/fix logic from implement.md - verification-layers.md (198 lines) - 5-layer verification from implement.md - phase-rules.md (190 lines) - POC-first phases from implement.md/spec-executor - commit-discipline.md (110 lines) - commit conventions from spec-executor - intent-classification.md (241 lines) - goal detection from start.md - spec-scanner.md (306 lines) - spec discovery from start.md - branch-management.md (194 lines) - branch/worktree from start.md - parallel-research.md (157 lines) - multi-agent dispatch from research.md - quality-checkpoints.md (131 lines) - [VERIFY] rules from task-planner - quality-commands.md (115 lines) - command discovery from spec-executor These live outside skills/ to avoid bloating the / namespace. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lines from 1557) Replace inline procedural logic with Read references to coordinator-pattern.md, failure-recovery.md, verification-layers.md, phase-rules.md, and commit-discipline.md. Keep frontmatter, state initialization, and key behavioral summary inline. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…s from 1388) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…s from 1552) Replace inline procedural logic with Read references to extracted files: - goal-interview.md: brainstorming dialogue, exploration territory, approach proposals - quick-mode.md: full quick mode flow, validation, review loops, rollback - Existing refs: branch-management.md, intent-classification.md, spec-scanner.md, parallel-research.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rewrite research.md (672->188), requirements.md (480->173), design.md (508->176), tasks.md (510->185), and refactor.md (333->141) as thin orchestrators that reference extracted logic from references/. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…iewer, and research Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…p in quick mode) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a design-and-plan refresh for ralph-specum and ralph-speckit in three phases: Phase A (metadata/version front-matter updates), Phase B (extract command logic into a new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CLI as CLI/Start
participant Coordinator as Coordinator (docs/reference)
participant TaskTool as Task Tool (Task / Team spawn)
participant Subagent as Subagent (executor/reviewer/qa)
Note over CLI,Coordinator: Start / intent classification -> choose flow
User->>CLI: invoke start / --quick?
CLI->>Coordinator: read intent_classification, spec_scanner, branch_management
Coordinator->>TaskTool: create team / spawn tasks (uses templates)
TaskTool->>Subagent: dispatch task (executor/research/reviewer)
Subagent->>TaskTool: write artifact / signal (TASK_COMPLETE / REVIEW_FAIL)
TaskTool->>Coordinator: post results / progress
Coordinator->>CLI: update .ralph-state.json and .progress.md
(Colored rectangles not required for this simple sequence.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/ralph-speckit/.claude-plugin/plugin.json (1)
1-10:⚠️ Potential issue | 🔴 CriticalUpdate
marketplace.jsonto bumpralph-speckitversion to0.5.1.The
plugin.jsonwas bumped to0.5.1, but.claude-plugin/marketplace.jsonstill listsralph-speckitat version0.5.0. Both must be synchronized when plugin versions change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-speckit/.claude-plugin/plugin.json` around lines 1 - 10, The marketplace entry for the plugin "ralph-speckit" is out of sync: plugin.json was updated to version 0.5.1 but .claude-plugin/marketplace.json still lists 0.5.0; update the marketplace.json entry for "ralph-speckit" to version "0.5.1" so both manifests match, and verify the "name":"ralph-speckit" and "version" fields are identical across plugin.json and marketplace.json before committing.plugins/ralph-specum/agents/spec-executor.md (1)
449-455:⚠️ Potential issue | 🟠 Major
ALL_TASKS_COMPLETEsignal is missing — required by coding guideline.The output format only defines
TASK_COMPLETE, but the coding guideline for this file explicitly requires the executor to also outputALL_TASKS_COMPLETEwhen all tasks are done. Without this signal the coordinator has no way to distinguish "one more task done" from "the entire run is finished," which can cause the loop to stall or re-enter indefinitely.The natural location to emit this is step 9 of the Execution Flow and the Output Format section: after marking the task
[x], check whether every task intasks.mdis now checked, and if so emitALL_TASKS_COMPLETEin place of (or immediately after)TASK_COMPLETE.📋 Suggested additions
9. Output: TASK_COMPLETE + - If ALL tasks in tasks.md are now [x]: output ALL_TASKS_COMPLETE insteadOutput Format additions:
On successful completion:Task X.Y: [name] - DONE
Verify: PASSED
Commit: abc1234TASK_COMPLETE
+ +On final task completion (all tasks done): +``` +Task X.Y: [name] - DONE +Verify: PASSED +Commit: abc1234 + +ALL_TASKS_COMPLETE +```Completion Integrity
<mandatory>block:NEVER output TASK_COMPLETE unless the task is TRULY complete: + +Output ALL_TASKS_COMPLETE (instead of TASK_COMPLETE) when: +- The current task is TRULY complete (all criteria above met), AND +- All tasks in tasks.md are now marked [x]As per coding guidelines: "Task executor must output TASK_COMPLETE for coordinator to advance; output ALL_TASKS_COMPLETE when all tasks are done."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/agents/spec-executor.md` around lines 449 - 455, Update the Execution Flow (step 9) and Output Format sections to emit the ALL_TASKS_COMPLETE signal when the run finishes: after marking the current task as DONE (the logic that currently emits TASK_COMPLETE), add a check that inspects the task list (tasks.md) to verify every task is checked; if true, output ALL_TASKS_COMPLETE (in place of or immediately after TASK_COMPLETE) and include the new example block under Output Format showing ALL_TASKS_COMPLETE; reference the existing TASK_COMPLETE emission point in the spec-executor.md Execution Flow to locate where to add this check and output.
🟡 Minor comments (18)
plugins/ralph-speckit/.claude-plugin/plugin.json-3-3 (1)
3-3:⚠️ Potential issue | 🟡 MinorVersion bump should be minor (
0.6.0), not patch (0.5.1).This PR introduces new backward-compatible functionality across three phases: new
color/versionmetadata fields on 14 agents and 10 skills, 13 new reference files, 3 dispatch templates, and command restructuring. A minor version bump adds one or more new features; a patch version bump fixes bugs or security issues. None of the changes are bug fixes — bump to0.6.0.🔧 Suggested fix
- "version": "0.5.1", + "version": "0.6.0",As per coding guidelines: "patch for fixes, minor for features, major for breaking changes".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-speckit/.claude-plugin/plugin.json` at line 3, Update the plugin version value in plugin.json from "0.5.1" to "0.6.0" to reflect the addition of new, backward-compatible features (new metadata fields, reference files, dispatch templates, and command restructuring); locate the "version" key in plugins/ralph-speckit/.claude-plugin/plugin.json and change its string value to "0.6.0".plugins/ralph-specum/references/goal-interview.md-48-59 (1)
48-59:⚠️ Potential issue | 🟡 MinorHardcoded
./specs/in the awareness message will mislead users when their configured directory differs.The pseudocode at Line 55 outputs the literal string
./specs/even whendirs[0]might be a completely different path (e.g.,packages/api/specs). The message should interpolate the actual directory value.🛠️ Proposed fix
-4. If dirs.length == 1: OUTPUT awareness message (non-blocking): - "Spec will be created in ./specs/ - Tip: You can organize specs in multiple directories. - See /ralph-specum:help for multi-directory setup." +4. If dirs.length == 1: OUTPUT awareness message (non-blocking): + "Spec will be created in {dirs[0]} + Tip: You can organize specs in multiple directories. + See /ralph-specum:help for multi-directory setup."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/goal-interview.md` around lines 48 - 59, The awareness message currently prints a hardcoded "./specs/" which can mislead users; update the spec location logic that uses ralph_get_specs_dirs() (and the branch where dirs.length == 1) to interpolate the actual directory value (e.g., use dirs[0]) into the output message instead of the literal "./specs/". Locate the block that constructs the non-blocking awareness output (the code path used before storing specsDir for spec creation and near the AskUserQuestion handling) and replace the static string with a formatted message that includes the resolved specs directory variable so users see the real path.plugins/ralph-specum/references/spec-scanner.md-161-175 (1)
161-175:⚠️ Potential issue | 🟡 Minor
"./specs"hardcoded in the Resolution section conflicts with the Writing section's use ofralph_get_default_dir().Lines 173–174 compare
specsDir == "./specs"directly, but the Writing section (lines 191–194) correctly compares againstralph_get_default_dir(). If the project's default is configured to something other than./specs, the Resolution pseudocode would misclassify it, causing wrong.current-specwrite behavior.🔧 Proposed fix
-3. For .current-spec: - - If specsDir == "./specs" (default): Write bare name - - If specsDir != "./specs" (non-default): Write full path "$specsDir/$name" +3. For .current-spec: + defaultDir = ralph_get_default_dir() + - If specsDir == defaultDir: Write bare name + - If specsDir != defaultDir: Write full path "$specsDir/$name"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/spec-scanner.md` around lines 161 - 175, The pseudocode incorrectly compares specsDir to the hardcoded string "./specs"; change that to compare against the configured default by calling ralph_get_default_dir() instead (i.e., use ralph_get_default_dir() when deciding whether to write the bare name or full path for .current-spec). Update the Resolution logic that references "./specs" so it uses ralph_get_default_dir(), and ensure the variables/specsDir/basePath logic (specsDir, basePath, ralph_get_default_dir(), and the .current-spec write decision) are consistent across the document.plugins/ralph-specum/references/phase-rules.md-132-139 (1)
132-139:⚠️ Potential issue | 🟡 MinorLint requirement in Phase 1–3 checkpoint format contradicts the behavior table.
The Standard [VERIFY] checkpoint (lines 132–139) chains
<lint cmd>and marks the taskDone when: No lint errors, no type errors. But the behavior table at line 185 says Lint must pass: No for Phase 1, 2, and 3. An agent following the checkpoint format strictly cannot mark Phase 1–3 checkpoint tasks complete until lint is clean — effectively making lint mandatory in those phases despite the table saying otherwise.The same contradiction surfaces in
quality-checkpoints.mdline 43 (Phase 1 (POC): Lint + type check onlywith the sameDone when: No lint errors).Consider splitting the standard checkpoint by phase: Phase 1 typecheck-only, Phase 2–3 lint + typecheck, Phase 4+ full suite — or clarify in the behavior table that checkpoints do enforce lint as a gate.
🔧 One possible resolution — phase-aware checkpoint format
-Standard [VERIFY] checkpoint (every 2-3 tasks): -```markdown -- [ ] V1 [VERIFY] Quality check: <discovered lint cmd> && <discovered typecheck cmd> - - **Do**: Run quality commands and verify all pass - - **Verify**: All commands exit 0 - - **Done when**: No lint errors, no type errors - - **Commit**: `chore(scope): pass quality checkpoint` (if fixes needed) -``` +**Phase 1** [VERIFY] checkpoint (every 2-3 tasks — typecheck only): +```markdown +- [ ] V1 [VERIFY] Quality check: <discovered typecheck cmd> + - **Do**: Run type check and verify it passes + - **Verify**: Command exits 0 + - **Done when**: No type errors + - **Commit**: `chore(scope): pass quality checkpoint` (if fixes needed) +``` + +**Phase 2–3** [VERIFY] checkpoint (every 2-3 tasks — lint + typecheck): +```markdown +- [ ] V1 [VERIFY] Quality check: <discovered lint cmd> && <discovered typecheck cmd> + - **Do**: Run quality commands and verify all pass + - **Verify**: All commands exit 0 + - **Done when**: No lint errors, no type errors + - **Commit**: `chore(scope): pass quality checkpoint` (if fixes needed) +```Also applies to: 181-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/phase-rules.md` around lines 132 - 139, The Standard [VERIFY] checkpoint currently requires lint+typecheck and conflicts with the behavior table that sets "Lint must pass: No" for Phases 1–3; update the checkpoint wording in phase-rules.md (the "Standard [VERIFY] checkpoint" block) and the related block in quality-checkpoints.md (the "Phase 1 (POC): Lint + type check only" section) to be phase-aware: make Phase 1 checkpoints run only the discovered typecheck command (Done when: No type errors), make Phase 2–3 checkpoints run both discovered lint and typecheck commands (Done when: No lint errors, No type errors), and keep Phase 4+ as the full suite; ensure the behavior table ("Lint must pass") is either updated to match or left as-is if you adopt the phase-aware checkpoints so they no longer contradict the table.plugins/ralph-specum/references/verification-layers.md-70-70 (1)
70-70:⚠️ Potential issue | 🟡 MinorFenced code blocks at lines 70 and 128 are missing language specifiers.
The markdownlint rule MD040 flags both blocks. Use
```textsince the content is pseudocode/plain text.📝 Proposed fix
-``` +```text Set reviewIteration = 1-``` +```text Prior findings (from iteration $prevIteration):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/verification-layers.md` at line 70, Two fenced code blocks containing the literal lines "Set reviewIteration = 1" and "Prior findings (from iteration $prevIteration):" are missing language specifiers; update each triple-backtick fence to use the text language specifier (```text) so markdownlint MD040 is satisfied by changing the blocks around those exact strings to start with ```text and end with ``` respectively.plugins/ralph-specum/references/intent-classification.md-135-135 (1)
135-135:⚠️ Potential issue | 🟡 MinorMissing blank line before table at line 135 (markdownlint MD058).
Add a blank line between the
Examples:label and the table.📝 Proposed fix
Examples: + | Goal | Inferred Name |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/intent-classification.md` at line 135, The markdown file violates MD058: add a single blank line between the "Examples:" label and the following table header "| Goal | Inferred Name |" so the table is separated from the preceding paragraph; locate the "Examples:" label near that table and insert one empty line immediately after it to satisfy markdownlint.plugins/ralph-specum/references/branch-management.md-133-145 (1)
133-145:⚠️ Potential issue | 🟡 MinorEmpty
SPEC_NAMEproduces invalid git branchfeat/when no spec is active.If
ralph_resolve_currentfails (no active spec),SPEC_NAMEstays empty. Line 145 then runsgit worktree add "$WORKTREE_PATH" -b "feat/", which git will reject as an invalid branch name. The branch-decision text (line 39) already specifies a temp-name fallback but the script never implements it.🔧 Proposed fix — add fallback after spec resolution
if SPEC_PATH=$(ralph_resolve_current 2>/dev/null); then SPEC_NAME=$(basename "$SPEC_PATH") fi + +# Fallback when no active spec is known +if [ -z "$SPEC_NAME" ]; then + SPEC_NAME="spec-work-$(date +%s)" +fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/branch-management.md` around lines 133 - 145, After resolving the current spec with ralph_resolve_current, ensure SPEC_NAME is assigned a fallback when empty so you never construct an invalid branch "feat/". Update the block that sets SPEC_PATH and SPEC_NAME to check if SPEC_NAME is empty after resolution and, if so, set SPEC_NAME to the temp-name fallback used in your branch-decision text (or a safe default like "temp-name"); then compute WORKTREE_PATH using REPO_NAME and this final SPEC_NAME and call git worktree add with "-b feat/${SPEC_NAME}". This guarantees git worktree add and the branch name are always valid even when ralph_resolve_current fails.plugins/ralph-specum/references/failure-recovery.md-500-515 (1)
500-515:⚠️ Potential issue | 🟡 MinorFenced code blocks at lines 503 and 511 are missing language specifiers (MD040).
Both blocks contain plain-text log entry templates. Add
textto suppress the markdownlint warning.📝 Proposed fix
- ``` + ```text - Task $taskId: $attempts fixes attempted ($fixTaskIds) - Final: PASS ```- ``` + ```text - Task $taskId: $attempts fixes attempted ($fixTaskIds) - Final: FAIL (max limit) ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/failure-recovery.md` around lines 500 - 515, The two fenced code blocks used for log-entry templates (the PASS and FAIL lines that get appended to ".progress.md" under the "## Fix Task History" section) are missing a language specifier which triggers markdownlint MD040; update the fences that wrap the templates for "Task $taskId: $attempts fixes attempted ($fixTaskIds) - Final: PASS" and "Task $taskId: $attempts fixes attempted ($fixTaskIds) - Final: FAIL (max limit)" to use ```text so the blocks are explicitly plain-text and the lint warning is suppressed.plugins/ralph-specum/commands/design.md-83-83 (1)
83-83:⚠️ Potential issue | 🟡 MinorHardcoded
"architect-1"recipient is fragile.The
SendMessageshutdown call assumes the spawnedarchitect-revieweragent is named"architect-1". If Claude Code Teams assigns a different instance identifier, the message silently fails and the team is left orphaned. The same pattern inrequirements.mduses"pm-1". Consider capturing the actual instance name at team creation time, or confirm this naming convention is guaranteed by the Teams runtime.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/commands/design.md` at line 83, The shutdown SendMessage currently hardcodes recipient "architect-1" which can fail if the architect-reviewer instance receives a different identifier; update the flow so the actual agent instance name is captured at team creation and referenced for shutdown (e.g., store the created agent's name/ID when spawning the "architect-reviewer" in the team creation routine and use that stored identifier in the SendMessage shutdown call), and apply the same fix for the "pm-1" usage in requirements.md to read the stored instance name instead of a literal string.plugins/ralph-specum/commands/requirements.md-82-82 (1)
82-82:⚠️ Potential issue | 🟡 MinorHardcoded
"pm-1"recipient — same fragility asdesign.md's"architect-1".If the Teams runtime names the spawned
product-manageragent differently, this shutdown message silently fails and the team remains orphaned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/commands/requirements.md` at line 82, The shutdown example hardcodes recipient "pm-1" which will fail if the runtime names the product-manager differently; update the SendMessage example (SendMessage(type: "shutdown_request", recipient: "pm-1")) to use a dynamic reference or lookup instead (e.g., use a variable like productManagerId or a function getAgentIdByRole("product-manager") ), show the pattern to resolve the agent id at runtime, and add handling for a missing id (avoid sending if not found and surface an error/log). Ensure the example uses the symbol productManagerId or getAgentIdByRole("product-manager") instead of "pm-1".docs/plans/2026-02-20-plugin-best-practices-plan.md-404-406 (1)
404-406:⚠️ Potential issue | 🟡 MinorValidation expects 11 reference files, but 13 were created.
goal-interview.mdandquick-mode.mdwere added beyond the original 11 planned (both referenced bystart.md). Anyone running this validation script will see13where11is expected, which looks like a failure. The comment/expected value should be updated.🔧 Proposed fix
-Expected: 11 reference files total. +Expected: 13 reference files total.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-02-20-plugin-best-practices-plan.md` around lines 404 - 406, The validation expects 11 reference files but two new files (goal-interview.md and quick-mode.md) referenced by start.md mean the count is now 13; update the expected value in the validation text that uses "ls plugins/ralph-specum/references/ | wc -l" from 11 to 13 (or remove/adjust the hard-coded expectation) so the validation reflects the actual files and does not fail incorrectly.plugins/ralph-specum/commands/research.md-105-105 (1)
105-105:⚠️ Potential issue | 🟡 MinorUse
rm -fto handle the case where no partial files exist.
rm ./specs/$spec/.research-*.mdwill emit a "No such file or directory" error if no partial.research-*.mdfiles match (e.g., on a re-run after cleanup, or when agents wrote directly rather than via partials). Sinceallowed-tools: "*"means Claude can invoke Bash, this will surface as an error in the output.🔧 Proposed fix
-After merge, delete partial files: `rm ./specs/$spec/.research-*.md` +After merge, delete partial files: `rm -f ./specs/$spec/.research-*.md`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/commands/research.md` at line 105, Replace the literal removal command `rm ./specs/$spec/.research-*.md` with a force/no-error variant so shell won't fail when no matches exist; update the instruction that references the glob pattern `.research-*.md` (and the `$spec` variable) to use `rm -f` semantics (i.e., the force/ignore-missing option) so cleanup is idempotent and won’t surface a "No such file or directory" error.docs/plans/2026-02-20-plugin-best-practices-design.md-154-166 (1)
154-166:⚠️ Potential issue | 🟡 MinorReference file count is stale — 11 documented, 13 implemented.
The directory structure and success criteria (line 192) document 11 reference files, but 13 files actually exist in
./plugins/ralph-specum/references/. The missing files aregoal-interview.mdandquick-mode.md, which are actively referenced instart.md. Update both the directory listing and success criteria to reflect 13 files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-02-20-plugin-best-practices-design.md` around lines 154 - 166, The references count is stale: the document lists 11 files but the actual references/ folder contains 13 (including goal-interview.md and quick-mode.md which are referenced from start.md); update the directory listing block under references/ to include goal-interview.md and quick-mode.md and change the success criteria reference count from 11 to 13 so the plan and success criteria both reflect the actual 13 reference files.plugins/ralph-specum/commands/implement.md-135-136 (1)
135-136:⚠️ Potential issue | 🟡 MinorVerification layer count conflicts with the design doc.
Line 136 describes 5 verification layers, while
docs/plans/2026-02-20-plugin-best-practices-design.md(line 65) says "4-layer verification after each task completion." Theverification-layers.mdreference file is authoritative, but the design doc should be updated to reflect 5 layers (the 5th appears to be the artifact review viaspec-reviewer, likely added during implementation).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/commands/implement.md` around lines 135 - 136, Update the design doc that currently says "4-layer verification" (docs/plans/2026-02-20-plugin-best-practices-design.md) to reflect the authoritative verification-layers.md which defines 5 layers; explicitly add the artifact review via spec-reviewer as the 5th layer and make the wording consistent with plugins/ralph-specum/commands/implement.md and verification-layers.md, and search for any remaining "4-layer verification" text in the repo and replace it with the corrected "5-layer verification" language.plugins/ralph-specum/commands/start.md-13-19 (1)
13-19:⚠️ Potential issue | 🟡 MinorChecklist is missing the Quick Mode Flow step.
The checklist directive at line 13 says "Create a task for each item and complete in order." There are 5 checklist items, but the document has a Step 5 "Quick Mode Flow" (line 136) with no corresponding checklist entry. Step 2 in the document also merges two checklist items ("Parse input" + "Classify intent"). When
--quickis detected and execution jumps to Step 5, no task was created for it — contradicting the TaskCreate tracking goal.Add a 6th checklist item (or renumber to align):
🔧 Proposed fix
1. **Handle branch** -- check git branch, create/switch if needed -2. **Parse input** -- extract name, goal, flags from $ARGUMENTS -3. **Classify intent** -- determine what user wants (new spec, resume, quick mode) +2. **Parse input & classify intent** -- extract name, goal, flags; determine new/resume/quick 3. **Scan existing specs** -- find matching or related specs 4. **Route to action** -- invoke appropriate flow (new, resume, or quick mode) +5. **Quick mode flow** -- run all phases sequentially without interruption (skip if not --quick)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/commands/start.md` around lines 13 - 19, The checklist in start.md is missing an entry for the existing "Quick Mode Flow" step (Step 5); update the checklist so each major step has a corresponding task by either adding a sixth checklist item "6. Quick Mode Flow -- handle --quick execution path and create/switch tasks" or renumbering items so "Parse input" and "Classify intent" are separate entries and then adding the Quick Mode Flow entry; ensure the checklist items explicitly reference handling of --quick and the "Quick Mode Flow" section so TaskCreate tracking will create a task when execution jumps to that flow.plugins/ralph-specum/commands/tasks.md-87-87 (1)
87-87:⚠️ Potential issue | 🟡 MinorPhase count mismatch between delegation (5 phases) and walkthrough template (4 phases).
Step 3 line 87 instructs the task-planner to use
Phase 1-5 per phase-rules.md, but the walkthrough template at lines 133–140 says"across 4 phases"and enumerates only Phase 1–4. Ifphase-rules.mddefines 5 phases, the summary will silently omit the fifth phase and misreport the total task count.🛠️ Suggested fix — align walkthrough template to match phase-rules.md
-**Total**: [X] tasks across 4 phases +**Total**: [X] tasks across [N] phases ← derive from phase-rules.md (currently 5) **Phase Breakdown**: - Phase 1 (POC): [count] tasks - proves the idea works - Phase 2 (Refactor): [count] tasks - clean up - Phase 3 (Testing): [count] tasks - add coverage - Phase 4 (Quality): [count] tasks - CI/PR +- Phase 5 ([Name]): [count] tasks - [description] ← add if phase-rules.md defines itAlso applies to: 133-140
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/commands/tasks.md` at line 87, The walkthrough template currently says "across 4 phases" and lists Phase 1–4 while Step 3 instructs "Phase 1-5 per phase-rules.md"; update the walkthrough template text and enumeration so they match the delegation rule and include Phase 5. Concretely, replace the phrase "across 4 phases" with "across 5 phases" and extend the enumerated phases from "Phase 1–4" to "Phase 1–5" (ensure any summary or task-count language that references total phases is updated to 5) so the template and the "Phase 1-5 per phase-rules.md" directive are consistent.plugins/ralph-specum/commands/tasks.md-177-184 (1)
177-184:⚠️ Potential issue | 🟡 Minor
<mandatory>STOP block contradicted by embedded--quickexception.The
<mandatory>wrapper signals an unconditional constraint, but line 180 carves out--quickmode from inside it. An LLM executor parsing this block may either apply the exception too broadly (skipping the stop in non-quick mode) or be confused about which clause is truly mandatory.Move the
--quickcarve-out outside the mandatory block to keep the semantics clean:🛠️ Suggested restructuring
+If NOT in `--quick` mode: + <mandatory> **STOP HERE. DO NOT PROCEED TO IMPLEMENT.** - (Does not apply in `--quick` mode.) - 1. Display: `-> Next: Run /ralph-specum:implement to start execution` 2. End your response immediately 3. Wait for user to explicitly run `/ralph-specum:implement` </mandatory>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/commands/tasks.md` around lines 177 - 184, The `<mandatory>` block currently contains an exception for `--quick`, creating a contradiction; edit the tasks.md content to remove the `--quick` carve-out from inside the `<mandatory>` wrapper and place it immediately before or after that block as a separate paragraph or note so the mandatory stop semantics remain unconditional; update the messaging so the `<mandatory>` block only contains the stop instructions ("STOP HERE. DO NOT PROCEED TO IMPLEMENT." and the three numbered steps) and the `--quick` exception is clearly documented outside the `<mandatory>` wrapper (e.g., "Except in `--quick` mode: ...") to avoid confusion for LLM executors parsing the file.plugins/ralph-specum/commands/tasks.md-92-92 (1)
92-92:⚠️ Potential issue | 🟡 MinorCompletion criterion for "Monitor via TaskList" is undefined.
Item 5 says "Monitor via TaskList" with no signal or condition that marks the task-planner done. Without a stated termination signal (e.g.,
ALL_TASKS_COMPLETE, a specific task status, or a timeout), the coordinator has no deterministic rule for advancing to item 6. This creates an ambiguous polling loop. Based on learnings for this plugin, task executors signal completion — define the equivalent expected output here.🛠️ Suggested addition
-5. **Wait for completion**: Monitor via TaskList. +5. **Wait for completion**: Poll `TaskList` until the task-planner's task status is `done` + or until a `ALL_TASKS_COMPLETE` message is received. Timeout after [N] minutes and proceed with whatever `tasks.md` contains.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/commands/tasks.md` at line 92, Define a deterministic completion criterion for item 5 ("Monitor via TaskList") by specifying the exact signal or condition the coordinator should wait for (e.g., TaskList emitting ALL_TASKS_COMPLETE, all tasks reaching status COMPLETED, a specific terminal task state, or a configurable timeout), update the TaskList monitoring description to check that condition and proceed to item 6 only when it is satisfied, and mention expected task executor behavior (what signal/payload they emit) so the coordinator and task-planner (TaskList) have a clear, unambiguous termination rule.
🧹 Nitpick comments (11)
plugins/ralph-specum/references/goal-interview.md (1)
91-91:TaskCreateis inconsistent with theTasktool terminology used everywhere else.All other agent and skill files (
delegation-principle/SKILL.md,architect-reviewer.md,research-analyst.md,task-planner.md) uniformly use "Tasktool" or "Task tool".TaskCreateis undefined elsewhere and may confuse an agent following this reference.♻️ Proposed fix
-Each TaskCreate description should include: +Each Task tool invocation's `description` should include:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/goal-interview.md` at line 91, The documentation uses the undefined symbol "TaskCreate" which is inconsistent with the rest of the repo that calls this capability the "Task" tool; update the reference in goal-interview.md to use the existing "Task" tool terminology (replace "TaskCreate" with "Task" tool) and ensure any descriptions match phrasing used in other files (e.g., delegation-principle/SKILL.md, architect-reviewer.md, research-analyst.md, task-planner.md) so agents see a consistent "Task" tool name and behavior.plugins/ralph-specum/references/quality-commands.md (1)
96-100: Fenced code block missing language specifier (MD040).♻️ Proposed fix
-``` +```bash <lint cmd> && <typecheck cmd> # Phase 1 <lint cmd> && <typecheck cmd> && <test cmd> # Phase 2-3 <lint cmd> && <typecheck cmd> && <test cmd> && <e2e> && <build> # Phase 4</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@plugins/ralph-specum/references/quality-commands.mdaround lines 96 - 100,
The fenced code block containing the pipeline commands ( && ... ) lacks a language specifier; update the opening fence to include a
language (e.g., changetobash) so the block isbash <lint cmd> && <typecheck cmd> ..., ensuring the markdown linter MD040 is satisfied and
syntax highlighting applies to the code block containing the ,
, , , and lines.</details> </blockquote></details> <details> <summary>plugins/ralph-specum/references/phase-rules.md (1)</summary><blockquote> `78-80`: **Add language specifier to fenced code block.** <details> <summary>♻️ Proposed fix</summary> ```diff -``` +```text PR Creation -> CI Monitoring -> Review Check -> Fix Issues -> Push -> Repeat ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@plugins/ralph-specum/references/phase-rules.mdaround lines 78 - 80, The
fenced code block in the Phase Rules document is missing a language specifier;
update the triple-backtick fence that surrounds "PR Creation -> CI Monitoring ->
Review Check -> Fix Issues -> Push -> Repeat" to include a language hint (e.g.,
addtextafter the opening ````` ``) so the block becomes a labeled fenced
code block and renders correctly.</details> </blockquote></details> <details> <summary>plugins/ralph-specum/references/parallel-research.md (2)</summary><blockquote> `53-55`: **Fenced code blocks missing language specifier (MD040).** All three pseudo-API blocks lack a language tag. `text` would satisfy the lint rule and makes the intent clear. <details> <summary>♻️ Proposed fix (apply to all three blocks)</summary> ```diff -``` +```text TeamCreate(team_name: "research-$spec", ...) ``` ``` </details> Also applies to: 63-69, 75-96 <details> <summary>🤖 Prompt for AI Agents</summary> ```` Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/parallel-research.md` around lines 53 - 55, Add a language specifier to the fenced code blocks that contain the pseudo-API calls so the markdown linter rule MD040 is satisfied; for each block (e.g., the one with TeamCreate(team_name: "research-$spec", description: "Parallel research for $spec") and the other two pseudo-API examples in the same document) change the opening fence from ``` to ```text so the blocks read as ```text ... ``` and clearly mark them as text examples. ```` </details> --- `42-42`: **Hyphenate `rate-limiting` as a compound adjective.** ```diff -Example: "Add OAuth with rate limiting" becomes 3 research-analyst agents (OAuth patterns, rate limiting strategies, security best practices) +Example: "Add OAuth with rate limiting" becomes 3 research-analyst agents (OAuth patterns, rate-limiting strategies, security best practices) ``` <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/parallel-research.md` at line 42, Hyphenate "rate-limiting" as a compound adjective in the example string and any related list items: change the example phrase "Add OAuth with rate limiting" to "Add OAuth with rate-limiting" and update the list entry "rate limiting strategies" to "rate-limiting strategies" so the compound adjective is correctly formed (reference the example text and the parenthetical list in the same paragraph). ``` </details> </blockquote></details> <details> <summary>plugins/ralph-specum/references/commit-discipline.md (1)</summary><blockquote> `79-91`: **Unescaped `.` in sed pattern may corrupt sibling task IDs with the same prefix.** When `X.Y` is substituted with an actual task ID (e.g., `1.3`), the unescaped `.` matches any character. `sed 's/- \[ \] 1.3/- [x] 1.3/'` will also match `- [ ] 1.30 …`, partially replacing it and corrupting the line. <details> <summary>🔧 Suggested fix — anchor with trailing space and escape the dot</summary> ```diff - sed -i 's/- \[ \] X.Y/- [x] X.Y/' "<basePath>/tasks.md" + sed -i "s/- \\[ \\] ${TASK_ID//./\\.} /- [x] ${TASK_ID//./\\.} /" "<basePath>/tasks.md" ``` Or, more readably, pre-escape the ID before building the sed expression: ```bash ESCAPED_ID=$(printf '%s\n' "$TASK_ID" | sed 's/\./\\./g') sed -i "s/- \\[ \\] ${ESCAPED_ID} /- [x] ${ESCAPED_ID} /" "<basePath>/tasks.md" ``` The trailing space ensures you match exactly `1.3 ` and not the prefix of `1.30`. </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/commit-discipline.md` around lines 79 - 91, The sed substitution that replaces "- [ ] X.Y" uses an unescaped dot which matches any character and can corrupt sibling IDs like "1.30"; fix by pre-escaping dots in the task ID (e.g., build ESCAPED_ID by replacing '.' with '\.') and use that in the sed expression, and anchor the match with a trailing space so the pattern becomes "s/- \[ \] ${ESCAPED_ID} /- [x] ${ESCAPED_ID} /" (apply this change where the sed command is invoked in the commit-discipline block). ``` </details> </blockquote></details> <details> <summary>plugins/ralph-specum/references/quick-mode.md (1)</summary><blockquote> `68-70`: **Non-standard step label `4a` may confuse sequential step-following.** The `4a` notation is inconsistent with the surrounding numeric sequence. An AI agent executing these instructions might skip it or misplace it relative to step 5. <details> <summary>📝 Suggested fix — merge into step 4 or promote to own numbered step</summary> ```diff -4. Create spec directory: mkdir -p "$basePath" -4a. Ensure gitignore entries exist (.current-spec, .progress.md) +4. Create spec directory and configure gitignore: + - `mkdir -p "$basePath"` + - Ensure gitignore entries exist (`.current-spec`, `.progress.md`) ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/quick-mode.md` around lines 68 - 70, The step labeled "4a" in quick-mode.md is inconsistent and may be missed by an executor; either merge the "Ensure gitignore entries exist (.current-spec, .progress.md)" line into step 4's text or promote it to its own numbered step and renumber subsequent steps (adjust the current "5 Write .ralph-state.json" to the next number if you promote it). Update the label "4a" to a standard numeric step (e.g., "4." or "5.") and ensure the surrounding sequence remains consecutive so readers and agents follow the steps in order. ``` </details> </blockquote></details> <details> <summary>plugins/ralph-specum/commands/research.md (1)</summary><blockquote> `87-93`: **Add language specifiers to unlabeled code blocks.** Markdownlint (MD040) flags two fenced blocks in this file (reported at lines 41 and 127). Both the "Research topics identified" pre-step block and the walkthrough output template block lack a language identifier. Adding `text` resolves both warnings. <details> <summary>🔧 Proposed fix</summary> ```diff -``` +```text Research topics identified for parallel execution: ``` ```diff -``` +```text Research complete for '$spec'. ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ```` Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/commands/research.md` around lines 87 - 93, The two fenced code blocks that begin with "Research topics identified for parallel execution:" and "Research complete for '$spec'." are missing language specifiers (triggering MD040); update those blocks by adding the language identifier `text` to the opening fence for each block (i.e., change ``` to ```text for the block containing "Research topics identified for parallel execution:" and for the walkthrough output template that starts with "Research complete for '$spec'.") so markdownlint no longer flags them. ```` </details> </blockquote></details> <details> <summary>plugins/ralph-specum/commands/start.md (1)</summary><blockquote> `170-200`: **Two overlapping `<mandatory>` stop blocks could be consolidated.** Lines 170–188 and 190–200 both instruct the coordinator to stop after a subagent returns. The first covers the response pattern (output status, end immediately); the second adds the `awaitingApproval` state check. They share the same exception (`--quick` mode) and intent. Merging them into a single block reduces redundancy and the risk of the two descriptions drifting out of sync. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/commands/start.md` around lines 170 - 200, The two overlapping <mandatory> blocks that instruct the coordinator to stop after subagent returns should be consolidated into a single clear block that includes: the normal-mode stop behavior (output a brief status message and end the response immediately after any Task tool/subagent completes), the required state-file check (read $basePath/.ralph-state.json and if awaitingApproval: true STOP IMMEDIATELY), and the shared exception (allow --quick mode to run phases without stopping); remove the duplicate block and replace it with one merged <mandatory> section containing all three elements to avoid redundancy and drift. ``` </details> </blockquote></details> <details> <summary>docs/plans/2026-02-20-plugin-best-practices-design.md (1)</summary><blockquote> `147-147`: **Add language specifier to the directory tree code block.** As flagged by markdownlint (MD040), the fenced block is missing a language identifier. <details> <summary>🔧 Proposed fix</summary> ```diff -``` +```text plugins/ralph-specum/ ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ```` Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-02-20-plugin-best-practices-design.md` at line 147, The fenced code block that currently contains the directory tree string "plugins/ralph-specum/" is missing a language identifier; update that fenced block to include a language specifier (e.g., change ``` to ```text) so the block reads ```text followed by plugins/ralph-specum/ and the closing ```, which satisfies markdownlint MD040. ```` </details> </blockquote></details> <details> <summary>plugins/ralph-specum/commands/design.md (1)</summary><blockquote> `117-135`: **Add `text` language specifier to the walkthrough template block.** Flagged by markdownlint (MD040, line 132). <details> <summary>🔧 Proposed fix</summary> ```diff -``` +```text Design complete for '$spec'. ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ```` Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/commands/design.md` around lines 117 - 135, The markdown code block containing the walkthrough that starts with the text "Design complete for '$spec'." should have a language specifier added to the opening fence; update the opening triple-backtick fence from ``` to ```text so the block becomes a fenced code block with the text specifier (affect the block that wraps "Design complete for '$spec'." and the following lines) to satisfy markdownlint MD040. ```` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@plugins/ralph-specum/.claude-plugin/plugin.json:
- Line 3: Update the ralph-specum entry in marketplace.json to match the version
bump in plugin.json (set ralph-specum version from 3.10.0 to 3.11.0) so both
files are in sync; locate the "ralph-specum" object in marketplace.json and
change its "version" field to "3.11.0".In
@plugins/ralph-specum/commands/design.md:
- Around line 151-154: The "Update .ralph-state.json" step must perform a merge
rather than replace; change the wording in design.md to explicitly merge the two
fields into the existing .ralph-state.json (preserving existing keys like
source, name, basePath, commitSpec, relatedSpecs) using a jq merge that adds
{"phase":"design","awaitingApproval":true} into the file and write via a
temporary file then atomically mv it into place (same pattern used in
implement.md) so the state update never overwrites existing fields required
later.In
@plugins/ralph-specum/commands/tasks.md:
- Around line 83-95: The shutdown recipient is hardcoded as "planner-1", which
can mismatch the actual teammate ID and leave the planner running; modify the
spawn step (the teammate created during TaskCreate/spawn teammate) to capture
and return the assigned teammate ID (e.g., planner_id) when the planner is
created, persist that ID (in-memory or to ./specs/$spec/metadata) and use that
variable in the SendMessage call (replace SendMessage(... recipient:
"planner-1") with recipient: planner_id) and any subsequent cleanup checks
(TeamDelete/TaskList) so shutdown targets the actual teammate; alternatively, if
you intend to rely on a canonical ID, add explicit documentation asserting that
TaskCreate/spawn returns "planner-1" for the first teammate and include a
runtime assertion verifying the returned ID equals "planner-1".In
@plugins/ralph-specum/references/coordinator-pattern.md:
- Around line 236-251: The Sequential Review Pattern duplicates spec-reviewer
invocation already performed in Layer 5 (Artifact Review), causing up to 5
reviewer runs per task; update the flow to avoid double-invocation by either (A)
documenting the distinct purposes if both are intended (clarify that Layer 5 is
"artifact structural review" and Sequential Review is "execution-fidelity
reviewer" and keep both), or (B) gating one on quickMode or removing one: modify
the Sequential Review block that reads the reviewer template and dispatches the
spec-reviewersubagent so it only runs whenquickModeis false and Layer 5
was not executed, or remove the Sequential Review dispatch entirely and
consolidate retry logic into Layer 5 (which currently handles up to 3
iterations). Ensure references tospec-reviewer,quickMode(from
.ralph-state.json), Layer 5/Artifact Review, and the template
${CLAUDE_PLUGIN_ROOT}/templates/prompts/reviewer-prompt.mdare updated
accordingly.In
@plugins/ralph-specum/references/failure-recovery.md:
- Around line 98-147: The recovery loop breaks Layer 3 verification because
fix-task completions add extra checkmarks; update the coordinator logic so that
in the "After Delegation" path the verification layers are skipped when the
incoming TASK_COMPLETE originated from a fix-task (detect via fixTaskMap or a
fixTask flag on the delegation) and the flow jumps directly to the "Retry
original task" step (step 9), and simultaneously modify Layer 3's
expected-checkmark calculation in coordinator-pattern.md and
verification-layers.md so that when recoveryMode is true the expected value is
taskIndex + 1 + countOfActiveFixTasksInsertedForIndexes<=taskIndex (i.e.,
account for any inserted fix-task checkmarks), using the same
fixTaskMap/fixTaskIds bookkeeping to compute that count; ensure detection uses
the existing recoveryMode and fixTaskMap symbols and that taskIteration behavior
remains unchanged otherwise.In
@plugins/ralph-specum/references/intent-classification.md:
- Around line 88-92: The Quick Mode Input Detection currently references a
hardcoded ./specs/$name/plan.md and hardcoded path in its error message; update
that logic to use the existing ralph_find_spec(name) helper to resolve the spec
directory (same as the "Existing Plan Check" uses) and then check for a plan.md
inside the returned path, and if missing use the resolved path in the error
message (e.g., "No plan.md found in . Provide goal:
/ralph-specum:start 'your goal' --quick"). Ensure all lookups and
messages in the Quick Mode section reference the resolved path from
ralph_find_spec rather than ./specs/$name.In
@plugins/ralph-specum/references/verification-layers.md:
- Around line 24-44: Replace hardcoded ./specs/$spec/ paths with the resolved
$SPEC_PATH/ variable wherever the document runs git status or counts checkmarks
so non-default specs_dirs work; specifically update the Layer 2 git status
command (the git status --porcelain ./specs/$spec/tasks.md
./specs/$spec/.progress.md) to use $SPEC_PATH/tasks.md and
$SPEC_PATH/.progress.md, update the Layer 3 checkmark command (grep -c '-
[x]' ./specs/$spec/tasks.md) to grep $SPEC_PATH/tasks.md, and make the same
substitution for the two occurrences in Layer 5 (the two pairs of ./specs/$spec/
paths) so all references consistently use $SPEC_PATH/. Ensure the doc notes that
$SPEC_PATH is the resolved absolute path used elsewhere (e.g.,
coordinator-pattern.md).In
@plugins/ralph-specum/templates/prompts/research-prompt.md:
- Line 4: The template currently writes research partials without the required
path and uses {TOPIC} (which may contain spaces); update the prompt template so
the analyst writes to the coordinator-expected path using a new {BASE_PATH}
placeholder and a slugged topic placeholder {TOPIC_SLUG}: instruct output to be
written to {BASE_PATH}/specs/{SPEC_NAME}/.research-{TOPIC_SLUG}.md (replace any
occurrences of .research-{TOPIC}.md), and add guidance that callers must supply
{TOPIC_SLUG} (a URL/filename-safe slug of {TOPIC}) to ensure the coordinator's
merge and cleanup (rm ./specs/$spec/.research-*.md) find the files.
Outside diff comments:
In@plugins/ralph-speckit/.claude-plugin/plugin.json:
- Around line 1-10: The marketplace entry for the plugin "ralph-speckit" is out
of sync: plugin.json was updated to version 0.5.1 but
.claude-plugin/marketplace.json still lists 0.5.0; update the marketplace.json
entry for "ralph-speckit" to version "0.5.1" so both manifests match, and verify
the "name":"ralph-speckit" and "version" fields are identical across plugin.json
and marketplace.json before committing.In
@plugins/ralph-specum/agents/spec-executor.md:
- Around line 449-455: Update the Execution Flow (step 9) and Output Format
sections to emit the ALL_TASKS_COMPLETE signal when the run finishes: after
marking the current task as DONE (the logic that currently emits TASK_COMPLETE),
add a check that inspects the task list (tasks.md) to verify every task is
checked; if true, output ALL_TASKS_COMPLETE (in place of or immediately after
TASK_COMPLETE) and include the new example block under Output Format showing
ALL_TASKS_COMPLETE; reference the existing TASK_COMPLETE emission point in the
spec-executor.md Execution Flow to locate where to add this check and output.
Duplicate comments:
In@plugins/ralph-specum/commands/requirements.md:
- Around line 150-151: The step that updates .ralph-state.json is ambiguous
about merging and should use the same jq merge pattern from implement.md;
replace the plain assignmentUpdate .ralph-state.json: { "phase": "requirements", "awaitingApproval": true }with an explicit jq merge command
that reads the existing .ralph-state.json, merges/updates the phase and
awaitingApproval keys, and writes atomically (matching the pattern used in
implement.md) so concurrent state is preserved and only those fields are
changed.In
@plugins/ralph-specum/commands/research.md:
- Around line 165-166: The instruction in research.md to "Update
.ralph-state.json: { "phase": "research", ... }" is ambiguous about whether
to replace or merge; update the step to explicitly instruct a merge (not full
replace) and mirror the explicit guidance and jq pattern used in implement.md
(as noted in design.md Step 6). Reference .ralph-state.json and .progress.md in
the rewrite and include the same jq-style merge example and wording from
implement.md so readers know to update only the listed keys (phase,
awaitingApproval, relatedSpecs) while preserving other state fields.In
@plugins/ralph-specum/references/quality-checkpoints.md:
- Around line 41-51: The Phase 1 (POC) checkpoint "1.3 [VERIFY] Quality
checkpoint: && " incorrectly requires lint to exit 0,
conflicting with the phase-rules.md table which marks lint as not required for
Phase 1; update the Phase 1 checkpoint block (header "Phase 1 (POC)" and
checklist item "1.3 [VERIFY] Quality checkpoint") to align with phase-rules.md
by removing the lint command from the combined command (use only the typecheck
cmd) or explicitly marking lint as optional in the checklist and changing "Done
when" to reflect that only typecheck must pass.
Nitpick comments:
In@docs/plans/2026-02-20-plugin-best-practices-design.md:
- Line 147: The fenced code block that currently contains the directory tree
string "plugins/ralph-specum/" is missing a language identifier; update that
fenced block to include a language specifier (e.g., changetotext) so
the block readstext followed by plugins/ralph-specum/ and the closing,
which satisfies markdownlint MD040.In
@plugins/ralph-specum/commands/design.md:
- Around line 117-135: The markdown code block containing the walkthrough that
starts with the text "Design complete for '$spec'." should have a language
specifier added to the opening fence; update the opening triple-backtick fence
fromtotext so the block becomes a fenced code block with the text
specifier (affect the block that wraps "Design complete for '$spec'." and the
following lines) to satisfy markdownlint MD040.In
@plugins/ralph-specum/commands/research.md:
- Around line 87-93: The two fenced code blocks that begin with "Research topics
identified for parallel execution:" and "Research complete for '$spec'." are
missing language specifiers (triggering MD040); update those blocks by adding
the language identifiertextto the opening fence for each block (i.e., change
totext for the block containing "Research topics identified for parallel
execution:" and for the walkthrough output template that starts with "Research
complete for '$spec'.") so markdownlint no longer flags them.In
@plugins/ralph-specum/commands/start.md:
- Around line 170-200: The two overlapping blocks that instruct the
coordinator to stop after subagent returns should be consolidated into a single
clear block that includes: the normal-mode stop behavior (output a brief status
message and end the response immediately after any Task tool/subagent
completes), the required state-file check (read $basePath/.ralph-state.json and
if awaitingApproval: true STOP IMMEDIATELY), and the shared exception (allow
--quick mode to run phases without stopping); remove the duplicate block and
replace it with one merged section containing all three elements to
avoid redundancy and drift.In
@plugins/ralph-specum/references/commit-discipline.md:
- Around line 79-91: The sed substitution that replaces "- [ ] X.Y" uses an
unescaped dot which matches any character and can corrupt sibling IDs like
"1.30"; fix by pre-escaping dots in the task ID (e.g., build ESCAPED_ID by
replacing '.' with '.') and use that in the sed expression, and anchor the
match with a trailing space so the pattern becomes "s/- [ ] ${ESCAPED_ID} /-
[x] ${ESCAPED_ID} /" (apply this change where the sed command is invoked in the
commit-discipline block).In
@plugins/ralph-specum/references/goal-interview.md:
- Line 91: The documentation uses the undefined symbol "TaskCreate" which is
inconsistent with the rest of the repo that calls this capability the "Task"
tool; update the reference in goal-interview.md to use the existing "Task" tool
terminology (replace "TaskCreate" with "Task" tool) and ensure any descriptions
match phrasing used in other files (e.g., delegation-principle/SKILL.md,
architect-reviewer.md, research-analyst.md, task-planner.md) so agents see a
consistent "Task" tool name and behavior.In
@plugins/ralph-specum/references/parallel-research.md:
- Around line 53-55: Add a language specifier to the fenced code blocks that
contain the pseudo-API calls so the markdown linter rule MD040 is satisfied; for
each block (e.g., the one with TeamCreate(team_name: "research-$spec",
description: "Parallel research for $spec") and the other two pseudo-API
examples in the same document) change the opening fence fromtotext so
the blocks read astext ...and clearly mark them as text examples.- Line 42: Hyphenate "rate-limiting" as a compound adjective in the example
string and any related list items: change the example phrase "Add OAuth with
rate limiting" to "Add OAuth with rate-limiting" and update the list entry "rate
limiting strategies" to "rate-limiting strategies" so the compound adjective is
correctly formed (reference the example text and the parenthetical list in the
same paragraph).In
@plugins/ralph-specum/references/phase-rules.md:
- Around line 78-80: The fenced code block in the Phase Rules document is
missing a language specifier; update the triple-backtick fence that surrounds
"PR Creation -> CI Monitoring -> Review Check -> Fix Issues -> Push -> Repeat"
to include a language hint (e.g., addtextafter the opening ````` ``) so the
block becomes a labeled fenced code block and renders correctly.In
@plugins/ralph-specum/references/quality-commands.md:
- Around line 96-100: The fenced code block containing the pipeline commands
( && ... ) lacks a language specifier; update the
opening fence to include a language (e.g., changetobash) so the block
isbash <lint cmd> && <typecheck cmd> ..., ensuring the markdown linter
MD040 is satisfied and syntax highlighting applies to the code block containing
the , , , , and lines.In
@plugins/ralph-specum/references/quick-mode.md:
- Around line 68-70: The step labeled "4a" in quick-mode.md is inconsistent and
may be missed by an executor; either merge the "Ensure gitignore entries exist
(.current-spec, .progress.md)" line into step 4's text or promote it to its own
numbered step and renumber subsequent steps (adjust the current "5 Write
.ralph-state.json" to the next number if you promote it). Update the label "4a"
to a standard numeric step (e.g., "4." or "5.") and ensure the surrounding
sequence remains consecutive so readers and agents follow the steps in order.</details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| { | ||
| "name": "ralph-specum", | ||
| "version": "3.10.0", | ||
| "version": "3.11.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "marketplace.json" | head -20Repository: tzachbon/smart-ralph
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Find all marketplace.json files and show their version fields
fd -t f "marketplace.json" -x cat {} \; | jq '.version' 2>/dev/null || (fd -t f "marketplace.json" -x grep -E '"version"' {} \;)Repository: tzachbon/smart-ralph
Length of output: 46
🏁 Script executed:
#!/bin/bash
# List the path and version of all marketplace.json files with context
fd -t f "marketplace.json" -x sh -c 'echo "File: {}"; cat {} | jq ".version" 2>/dev/null || grep -A1 -B1 "version" {}'Repository: tzachbon/smart-ralph
Length of output: 46
🏁 Script executed:
cat ./.claude-plugin/marketplace.jsonRepository: tzachbon/smart-ralph
Length of output: 1226
Bump marketplace.json to version 3.11.0 for ralph-specum plugin.
The coding guideline requires both plugin.json and marketplace.json to be updated in lockstep. The plugin.json was bumped to 3.11.0, but marketplace.json still lists ralph-specum at 3.10.0 and must be updated to match.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/ralph-specum/.claude-plugin/plugin.json` at line 3, Update the
ralph-specum entry in marketplace.json to match the version bump in plugin.json
(set ralph-specum version from 3.10.0 to 3.11.0) so both files are in sync;
locate the "ralph-specum" object in marketplace.json and change its "version"
field to "3.11.0".
| 1. **Check orphaned team**: Read `~/.claude/teams/tasks-$spec/config.json`. If exists, `TeamDelete()`. | ||
| 2. **Create team**: `TeamCreate(team_name: "tasks-$spec")` | ||
| 3. **Create task**: `TaskCreate(subject: "Generate implementation tasks for $spec", activeForm: "Generating tasks")` | ||
| 4. **Spawn teammate**: Delegate to task-planner with requirements, design, and interview context. Instruct to: | ||
| - Break implementation into POC-first phases (Phase 1-5 per phase-rules.md) | ||
| - Create atomic, autonomous-ready tasks with Do/Files/Done when/Verify/Commit fields | ||
| - Insert quality checkpoints per quality-checkpoints.md | ||
| - Each task = one commit, tasks must be executable without human interaction | ||
| - Count total tasks, output to `./specs/$spec/tasks.md` | ||
| 5. **Wait for completion**: Monitor via TaskList. | ||
| 6. **Shutdown**: `SendMessage(type: "shutdown_request", recipient: "planner-1")` | ||
| 7. **Collect results**: Read `./specs/$spec/tasks.md`. | ||
| 8. **Clean up**: `TeamDelete()`. |
There was a problem hiding this comment.
Hardcoded "planner-1" recipient risks silent shutdown failure and team leak.
The spawn step (item 4) dispatches the task-planner teammate without explicitly naming it "planner-1", yet item 6 sends the shutdown to that hardcoded recipient. If the runtime assigns a different teammate ID (e.g. derived from the team name or an auto-incremented counter), the SendMessage is a no-op, the teammate never shuts down, and item 8's TeamDelete() may fail or leave an orphan.
Resolve by either:
- Capturing the teammate's assigned ID at spawn time and referencing it in the shutdown, or
- Documenting that
"planner-1"is the canonical first-teammate ID for this team configuration (and verifying that assumption holds).
🛠️ Suggested wording for items 4 & 6
-4. **Spawn teammate**: Delegate to task-planner with requirements, design, and interview context. Instruct to:
+4. **Spawn teammate**: `TeamAddMember(role: task-planner, name: "planner-1")` — delegate with requirements, design, and interview context. Instruct to:(Or capture the returned ID and use it symbolically in item 6.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/ralph-specum/commands/tasks.md` around lines 83 - 95, The shutdown
recipient is hardcoded as "planner-1", which can mismatch the actual teammate ID
and leave the planner running; modify the spawn step (the teammate created
during TaskCreate/spawn teammate) to capture and return the assigned teammate ID
(e.g., planner_id) when the planner is created, persist that ID (in-memory or to
./specs/$spec/metadata) and use that variable in the SendMessage call (replace
SendMessage(... recipient: "planner-1") with recipient: planner_id) and any
subsequent cleanup checks (TeamDelete/TaskList) so shutdown targets the actual
teammate; alternatively, if you intend to rely on a canonical ID, add explicit
documentation asserting that TaskCreate/spawn returns "planner-1" for the first
teammate and include a runtime assertion verifying the returned ID equals
"planner-1".
| ```text | ||
| 1. Task fails (no TASK_COMPLETE) | ||
| | | ||
| v | ||
| 2. Check recoveryMode in state | ||
| | | ||
| +-- false --> Normal retry/stop behavior | ||
| | | ||
| v (true) | ||
| 3. Parse failure output (see Parse Failure Output above) | ||
| Extract: taskId, error, attemptedFix | ||
| | | ||
| v | ||
| 4. Check fix limits | ||
| Read: fixTaskMap[taskId].attempts | ||
| | | ||
| +-- >= maxFixTasksPerOriginal --> STOP with error | ||
| | | ||
| v (under limit) | ||
| 5. Generate fix task | ||
| Create: X.Y.N [FIX X.Y] Fix: <error> | ||
| | | ||
| v | ||
| 6. Insert fix task into tasks.md | ||
| Position: immediately after original task | ||
| | | ||
| v | ||
| 7. Update state | ||
| - Increment fixTaskMap[taskId].attempts | ||
| - Add fix task ID to fixTaskMap[taskId].fixTaskIds | ||
| - Increment totalTasks | ||
| | | ||
| v | ||
| 8. Execute fix task | ||
| Delegate to spec-executor (same as standard delegation) | ||
| | | ||
| +-- TASK_COMPLETE --> Proceed to step 9 | ||
| | | ||
| +-- No completion --> Loop back to step 3 | ||
| (fix task becomes current, can spawn its own fixes) | ||
| | | ||
| v | ||
| 9. Retry original task | ||
| Delegate original task to spec-executor again | ||
| | | ||
| +-- TASK_COMPLETE --> Success! Verification layers | ||
| | | ||
| +-- No completion --> Loop back to step 3 | ||
| (generate another fix for original task) | ||
| ``` |
There was a problem hiding this comment.
Recovery mode is incompatible with Layer 3 checkmark verification — causes an infinite retry loop.
The Recovery Loop relies on spec-executor marking fix tasks [x] in tasks.md on completion (same as standard delegation, line 132). This creates two distinct failure modes against Layer 3 (Expected = taskIndex + 1):
Problem 1 — if "After Delegation" general rule runs verification layers after fix-task TASK_COMPLETE:
When fix task at position k+1 marks itself [x], checkmarks = k (prior tasks) + 1 (fix task) = k+1. This accidentally passes Layer 3 (expected = k+1). The coordinator then executes State Update and advances taskIndex past k, silently skipping the original task entirely.
Problem 2 — verification layers run after original task TASK_COMPLETE (step 9 success):
After original task at k also marks [x], checkmarks = k+1 (original) + 1 (fix task) = k+2. Layer 3 expects k+1 → rejects with "checkmark mismatch" → increments taskIteration → retries original → perpetual loop.
The root fix requires two aligned changes:
- In
coordinator-pattern.md"After Delegation": add an explicit exception — verification layers MUST be skipped when the TASK_COMPLETE comes from a fix-task delegation; proceed directly to step 9 (retry original). - In Layer 3 (both
coordinator-pattern.mdandverification-layers.md): when recovery mode is active, adjust the expected count to account for inserted fix-task checkmarks:
-Expected checkmark count = taskIndex + 1
+Expected checkmark count = taskIndex + 1 + sum of fixTaskMap[all task IDs ≤ taskIndex].attemptsWithout both fixes, enabling recoveryMode: true in .ralph-state.json will consistently corrupt execution state or stall indefinitely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/ralph-specum/references/failure-recovery.md` around lines 98 - 147,
The recovery loop breaks Layer 3 verification because fix-task completions add
extra checkmarks; update the coordinator logic so that in the "After Delegation"
path the verification layers are skipped when the incoming TASK_COMPLETE
originated from a fix-task (detect via fixTaskMap or a fixTask flag on the
delegation) and the flow jumps directly to the "Retry original task" step (step
9), and simultaneously modify Layer 3's expected-checkmark calculation in
coordinator-pattern.md and verification-layers.md so that when recoveryMode is
true the expected value is taskIndex + 1 +
countOfActiveFixTasksInsertedForIndexes<=taskIndex (i.e., account for any
inserted fix-task checkmarks), using the same fixTaskMap/fixTaskIds bookkeeping
to compute that count; ensure detection uses the existing recoveryMode and
fixTaskMap symbols and that taskIteration behavior remains unchanged otherwise.
…n-layers.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tive Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…path Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…view path - Bump marketplace.json ralph-specum version from 3.10.0 to 3.11.1 - Fix relative interview-framework path in goal-interview.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ommands Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ication Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rch-prompt.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/ralph-specum/commands/implement.md (1)
4-4:⚠️ Potential issue | 🟠 MajorAdd Team operation tools to
allowed-toolsin implement.md.The coordinator pattern used by implement.md calls
TeamCreate(),TaskCreate(),TaskList(),SendMessage(), andTeamDelete()to enable parallel execution of [P]-marked tasks. The currentallowed-tools: [Read, Write, Edit, Task, Bash, Skill]does not include these Team operations, causing the fallback path (sequential execution) to always trigger silently. All other commands using team-based parallelism (research.md, requirements.md, tasks.md, design.md) useallowed-tools: "*"for unrestricted access.Update:
-allowed-tools: [Read, Write, Edit, Task, Bash, Skill] +allowed-tools: "*"Or, if restrictive list is intentional:
-allowed-tools: [Read, Write, Edit, Task, Bash, Skill] +allowed-tools: [Read, Write, Edit, Task, Bash, Skill, TeamCreate, TeamDelete, TaskCreate, TaskList, SendMessage]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/commands/implement.md` at line 4, The allowed-tools list in implement.md is missing the Team operation tools used by the coordinator pattern; update the allowed-tools entry in implement.md to include TeamCreate, TaskCreate, TaskList, SendMessage, and TeamDelete (or set allowed-tools to "*" if you intend unrestricted tool access) so the coordinator can run parallel [P]-marked tasks; locate the allowed-tools line and add those exact symbols (TeamCreate, TaskCreate, TaskList, SendMessage, TeamDelete) or switch to "*" accordingly.plugins/ralph-specum/commands/requirements.md (1)
116-131:⚠️ Potential issue | 🟡 MinorAdd a language specifier to the walkthrough fenced code block (MD040).
🛠️ Proposed fix
-``` +```text Requirements complete for '$spec'. ... -``` +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/commands/requirements.md` around lines 116 - 131, The fenced code block in the requirements walkthrough is missing a language specifier (MD040); update the opening fence for the block that starts with "Requirements complete for '$spec'." to use a language tag (e.g., change the opening ``` to ```text) so the block becomes a ```text fenced code block and close it with the existing ```; locate the block in requirements.md by searching for the literal "Requirements complete for '$spec'." or the triple-backtick fence and update the start fence only.
🧹 Nitpick comments (1)
plugins/ralph-specum/references/verification-layers.md (1)
70-70: Add language identifiers to fenced code blocks (markdownlint MD040).Both fenced blocks at lines 70 and 128 are missing a language specifier. Use
`textfor the pseudo-code review loop and the prior-findings template.♻️ Proposed fix
-``` +```text Set reviewIteration = 1 ...-``` +```text Prior findings (from iteration $prevIteration): ...Also applies to: 128-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/verification-layers.md` at line 70, The fenced code blocks used for the pseudo-code review loop and the prior-findings template are missing language identifiers (markdownlint MD040); update the two backtick-fenced blocks that contain "Set reviewIteration = 1" (pseudo-code review loop) and "Prior findings (from iteration $prevIteration)" (prior-findings template) to include the language tag `text` after the opening ``` so they become ```text and satisfy the markdownlint 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 @.claude-plugin/marketplace.json:
- Line 13: The "version" field currently jumps from 3.10.0 to "3.11.1" which
skips 3.11.0; update the "version" key in the .claude-plugin/marketplace.json
diff (the "version" property) to "3.11.0" for a fresh minor release (or else add
a commit message/comment explaining why 3.11.1 is intentional), ensuring
semantic versioning is preserved.
In `@docs/plans/2026-02-21-review-fixes-plan.md`:
- Line 13: The markdown violates MD001 by jumping from an H1 to H3; insert an H2
section divider (e.g., add a heading like "## Tasks") immediately before the
first "### Task 1: Resolve Layer 5 duplication between coordinator-pattern.md
and verification-layers.md" so the heading hierarchy is H1 → H2 → H3 and ensure
subsequent "###" task headings remain nested under that new "## Tasks" section.
In `@plugins/ralph-specum/commands/design.md`:
- Around line 24-29: The design.md workflow hardcodes ./specs/$spec/ instead of
using the resolved spec directory from
ralph_find_spec()/ralph_resolve_current(), so define and use a $SPEC_PATH like
implement.md does: in Step 1 set $SPEC_PATH to the resolved spec directory
returned by ralph_find_spec()/ralph_resolve_current(), then replace all
hardcoded ./specs/$spec/ references (used in the Step 3 spawn/collect lines and
Step 5 state/read/write, and the commit block around line 160) to reference
$SPEC_PATH so specs in non-default specs_dirs (e.g.,
./packages/api/specs/my-feature) work reliably.
In `@plugins/ralph-specum/commands/requirements.md`:
- Around line 77-87: The workflow currently hardcodes the shutdown recipient
"pm-1" which can mismatch the runtime-assigned teammate ID and leak teams;
instead, when spawning the product-manager (item 4) either explicitly set the
teammate name using TeamAddMember(role: product-manager, name: "pm-1") or
capture the ID returned by TeamCreate/TeamAddMember and persist it (e.g., pmId)
and then use that variable in the SendMessage(type: "shutdown_request",
recipient: pmId) call (item 6); ensure any fallback path that directly calls
Task(subagent_type: product-manager) also captures the returned agent ID and
uses it for shutdown before calling TeamDelete().
In `@plugins/ralph-specum/commands/research.md`:
- Around line 87-92: The two fenced code blocks in research.md (the pre-step
output template and the walkthrough display block) lack language specifiers and
trigger markdownlint MD040; update each opening triple-backtick to declare a
language (use "text") so the blocks read ```text instead of ``` for both the
pre-step output template and the walkthrough display block (the block around
"Research topics identified for parallel execution..." and the block around
"Research complete for '$spec'. Output: ./specs/$spec/research.md ...").
In `@plugins/ralph-specum/references/coordinator-pattern.md`:
- Around line 246-251: The subagent_type used when dispatching the reviewer is
incorrectly namespaced as "ralph-specum:spec-reviewer"; change it to the bare
agent name "spec-reviewer" so the Task dispatch uses the same bare names used
elsewhere (e.g., "spec-executor" and other references to "spec-reviewer");
update the subagent_type value in the coordinator-pattern.md Task dispatch block
to "spec-reviewer" and ensure no plugin-prefix remains so the Task tool will
successfully resolve the subagent.
In `@plugins/ralph-specum/references/verification-layers.md`:
- Around line 37-51: Update Layer 3's expected-count logic to account for
parallel batches: instead of always using `taskIndex + 1`, when a parallel batch
is present use the batch representative index (the same approach as Layer 5:
`parallelGroup.startIndex` as the `$taskIndex`) to compute expected checkmarks;
add a short "Parallel Batch Note" to Layer 3 explaining that for a batch
[startIndex..endIndex] the coordinator runs Layer 3 with `taskIndex =
parallelGroup.startIndex`, and show the corrected expected-count formula and
rejection behavior when actual != expected.
---
Outside diff comments:
In `@plugins/ralph-specum/commands/implement.md`:
- Line 4: The allowed-tools list in implement.md is missing the Team operation
tools used by the coordinator pattern; update the allowed-tools entry in
implement.md to include TeamCreate, TaskCreate, TaskList, SendMessage, and
TeamDelete (or set allowed-tools to "*" if you intend unrestricted tool access)
so the coordinator can run parallel [P]-marked tasks; locate the allowed-tools
line and add those exact symbols (TeamCreate, TaskCreate, TaskList, SendMessage,
TeamDelete) or switch to "*" accordingly.
In `@plugins/ralph-specum/commands/requirements.md`:
- Around line 116-131: The fenced code block in the requirements walkthrough is
missing a language specifier (MD040); update the opening fence for the block
that starts with "Requirements complete for '$spec'." to use a language tag
(e.g., change the opening ``` to ```text) so the block becomes a ```text fenced
code block and close it with the existing ```; locate the block in
requirements.md by searching for the literal "Requirements complete for
'$spec'." or the triple-backtick fence and update the start fence only.
---
Duplicate comments:
In `@plugins/ralph-specum/commands/design.md`:
- Around line 151-153: The Update State step currently suggests writing `{
"phase": "design", "awaitingApproval": true }` to `.ralph-state.json`, which
risks overwriting existing keys (`source`, `name`, `basePath`, `commitSpec`,
`relatedSpecs`); change this to merge into the existing file (same approach as
implement.md Step 3) so you only add/update `phase` and `awaitingApproval`
without replacing other fields — use a jq merge (e.g., . + { "phase": "design",
"awaitingApproval": true }) writing to a temp file then atomically moving it
into place to avoid data loss.
In `@plugins/ralph-specum/commands/tasks.md`:
- Around line 83-95: The shutdown SendMessage recipient is hardcoded to
"planner-1" but the teammate spawned in step 4 isn't explicitly named or its ID
captured, so the shutdown can miss the real planner and leave an orphan; update
the spawn step (where you create/delegate to the planner in the
TaskCreate/TeamCreate flow) to either explicitly assign the member name/ID
"planner-1" when adding the teammate or return/capture the runtime-assigned
planner ID (e.g., plannerId) and replace the hardcoded "planner-1" in
SendMessage(type: "shutdown_request", recipient: "planner-1") with that captured
ID; ensure subsequent TeamDelete/TaskList/collection logic uses the same
captured ID or team reference to reliably shut down and clean up.
In `@plugins/ralph-specum/references/coordinator-pattern.md`:
- Around line 288-302: Update the "Layer 3: Checkmark Verification" logic so the
expected checkmark formula accounts for parallel batches: instead of always
using "Expected checkmark count = taskIndex + 1", detect when the current task
is part of a parallel batch and use "parallelGroup.endIndex + 1" as the expected
count; adjust the comparison that sets `expected` (and the rejection/retry flow
that logs "checkmark mismatch: expected $expected, found $actual" and increments
`taskIteration`) to use the parallel-aware value when a `parallelGroup` is
present.
---
Nitpick comments:
In `@plugins/ralph-specum/references/verification-layers.md`:
- Line 70: The fenced code blocks used for the pseudo-code review loop and the
prior-findings template are missing language identifiers (markdownlint MD040);
update the two backtick-fenced blocks that contain "Set reviewIteration = 1"
(pseudo-code review loop) and "Prior findings (from iteration $prevIteration)"
(prior-findings template) to include the language tag `text` after the opening
``` so they become ```text and satisfy the markdownlint rule.
.claude-plugin/marketplace.json
Outdated
| "name": "ralph-specum", | ||
| "description": "Spec-driven development with research, requirements, design, tasks, and autonomous execution. Fresh context per task.", | ||
| "version": "3.10.0", | ||
| "version": "3.11.1", |
There was a problem hiding this comment.
Version skips 3.11.0 — consider whether this is intentional.
The bump goes from 3.10.0 directly to 3.11.1, skipping 3.11.0. Under strict semantic versioning a new minor release should start at x.y.0; jumping straight to x.y.1 implies a prior 3.11.0 was published and then patched, which doesn't appear to be the case here.
If this is a fresh minor release for the Phase A–C feature additions, 3.11.0 would be the correct target.
🔧 Suggested correction
- "version": "3.11.1",
+ "version": "3.11.0",Based on learnings: "Use semantic versioning for plugin version bumps: patch for fixes, minor for features, major for breaking changes."
📝 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.
| "version": "3.11.1", | |
| "version": "3.11.0", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude-plugin/marketplace.json at line 13, The "version" field currently
jumps from 3.10.0 to "3.11.1" which skips 3.11.0; update the "version" key in
the .claude-plugin/marketplace.json diff (the "version" property) to "3.11.0"
for a fresh minor release (or else add a commit message/comment explaining why
3.11.1 is intentional), ensuring semantic versioning is preserved.
|
|
||
| --- | ||
|
|
||
| ### Task 1: Resolve Layer 5 duplication between coordinator-pattern.md and verification-layers.md |
There was a problem hiding this comment.
Heading skips from H1 to H3 — violates MD001.
The document opens with a single # (H1) and immediately uses ### (H3) for Task 1, with no intervening ## (H2). Add a ## section divider (e.g., ## Tasks) before the first ### to restore proper nesting.
📝 Proposed fix
---
+## Tasks
+
### Task 1: Resolve Layer 5 duplication...🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-02-21-review-fixes-plan.md` at line 13, The markdown violates
MD001 by jumping from an H1 to H3; insert an H2 section divider (e.g., add a
heading like "## Tasks") immediately before the first "### Task 1: Resolve Layer
5 duplication between coordinator-pattern.md and verification-layers.md" so the
heading hierarchy is H1 → H2 → H3 and ensure subsequent "###" task headings
remain nested under that new "## Tasks" section.
| 1. If `$ARGUMENTS` contains a spec name, use `ralph_find_spec()` to resolve it; otherwise use `ralph_resolve_current()` | ||
| 2. If no active spec, error: "No active spec. Run /ralph-specum:new <name> first." | ||
| 3. Check the resolved spec directory exists | ||
| 4. Check `requirements.md` exists. If not, error: "Requirements not found. Run /ralph-specum:requirements first." | ||
| 5. Read `.ralph-state.json`; clear approval flag: `awaitingApproval: false` | ||
| 6. Read context: `requirements.md` (required), `research.md` (if exists), `.progress.md` |
There was a problem hiding this comment.
Hardcoded ./specs/$spec/ paths break non-default specs_dirs — define $SPEC_PATH in Step 1 like implement.md does.
ralph_find_spec() / ralph_resolve_current() can return paths such as ./packages/api/specs/my-feature, but Step 1 never binds the result to a variable. All subsequent references fall back to the ./specs/$spec/ assumption (lines 81, 84, 115, 160), which breaks for any spec outside the default ./specs/ directory.
implement.md solves this with an explicit Step 1 binding (line 40: "Set $SPEC_PATH to the resolved spec directory path"). The same pattern should be applied here.
🐛 Proposed fix
Step 1 addition:
3. Check the resolved spec directory exists
+4. Set `$SPEC_PATH` to the resolved spec directory path. All subsequent file references use this variable.
4. Check `requirements.md` exists...Step 3 spawn instruction (line 81):
-Instruct to design architecture... Output to `./specs/$spec/design.md`.
+Instruct to design architecture... Output to `$SPEC_PATH/design.md`.Step 3 collect results (line 84):
-7. **Collect results**: Read `./specs/$spec/design.md`.
+7. **Collect results**: Read `$SPEC_PATH/design.md`.Step 5 walkthrough (line 115):
-Read `./specs/$spec/design.md` and display:
+Read `$SPEC_PATH/design.md` and display:Commit block (line 160):
-git add ./specs/$spec/design.md
+git add "$SPEC_PATH/design.md"Also applies to: 79-84, 115-115, 158-163
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/ralph-specum/commands/design.md` around lines 24 - 29, The design.md
workflow hardcodes ./specs/$spec/ instead of using the resolved spec directory
from ralph_find_spec()/ralph_resolve_current(), so define and use a $SPEC_PATH
like implement.md does: in Step 1 set $SPEC_PATH to the resolved spec directory
returned by ralph_find_spec()/ralph_resolve_current(), then replace all
hardcoded ./specs/$spec/ references (used in the Step 3 spawn/collect lines and
Step 5 state/read/write, and the commit block around line 160) to reference
$SPEC_PATH so specs in non-default specs_dirs (e.g.,
./packages/api/specs/my-feature) work reliably.
| ``` | ||
| Research topics identified for parallel execution: | ||
| 1. [Topic name] - [Agent type: research-analyst/Explore] | ||
| 2. [Topic name] - [Agent type: research-analyst/Explore] | ||
| 3. [Topic name] - [Agent type: research-analyst/Explore] (if applicable) | ||
| ... | ||
| ``` |
There was a problem hiding this comment.
Add language specifiers to fenced code blocks.
Both blocks are flagged by markdownlint (MD040). The pre-step output template (line 87) and the walkthrough display block (line 134) should declare a language — use text if they're plain-text output templates.
🛠️ Proposed fix
-```
+```text
Research topics identified for parallel execution:
1. [Topic name] - [Agent type: research-analyst/Explore]
...
-```
+```-```
+```text
Research complete for '$spec'.
Output: ./specs/$spec/research.md
...
-```
+```Also applies to: 134-148
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 87-87: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/ralph-specum/commands/research.md` around lines 87 - 92, The two
fenced code blocks in research.md (the pre-step output template and the
walkthrough display block) lack language specifiers and trigger markdownlint
MD040; update each opening triple-backtick to declare a language (use "text") so
the blocks read ```text instead of ``` for both the pre-step output template and
the walkthrough display block (the block around "Research topics identified for
parallel execution..." and the block around "Research complete for '$spec'.
Output: ./specs/$spec/research.md ...").
| ## Layer 3: Checkmark Verification | ||
|
|
||
| Count completed tasks in tasks.md: | ||
|
|
||
| ```bash | ||
| grep -c '\- \[x\]' $SPEC_PATH/tasks.md | ||
| ``` | ||
|
|
||
| Expected checkmark count = taskIndex + 1 (0-based index, so task 0 complete = 1 checkmark). | ||
|
|
||
| If actual count != expected: | ||
| - REJECT the completion | ||
| - Log: "checkmark mismatch: expected $expected, found $actual" | ||
| - This detects state manipulation or incomplete task marking | ||
| - Increment taskIteration and retry |
There was a problem hiding this comment.
Layer 3's expected-count formula breaks for parallel batches — add a Parallel Batch Note analogous to Layer 5's.
The formula Expected checkmark count = taskIndex + 1 is correct for sequential execution but fails for parallel batches. After a batch of tasks [startIndex..endIndex] all complete, the coordinator runs Layer 3 with taskIndex = startIndex (state not yet advanced), yielding:
| Scenario | Expected | Actual | Result |
|---|---|---|---|
| Sequential task N | N + 1 | N + 1 | ✓ |
| Parallel batch [2, 3, 4] | 3 | 5 | ✗ REJECT |
Layer 5 already documents the parallel case at line 162 ("use parallelGroup.startIndex as the representative $taskIndex"). Layer 3 needs the same treatment.
🐛 Proposed fix
Expected checkmark count = taskIndex + 1 (0-based index, so task 0 complete = 1 checkmark).
+
+**Parallel Batch**: When verifying after a parallel batch, use `parallelGroup.endIndex + 1` as the expected count (all tasks up to and including the last parallel task should be marked `[x]`).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/ralph-specum/references/verification-layers.md` around lines 37 - 51,
Update Layer 3's expected-count logic to account for parallel batches: instead
of always using `taskIndex + 1`, when a parallel batch is present use the batch
representative index (the same approach as Layer 5: `parallelGroup.startIndex`
as the `$taskIndex`) to compute expected checkmarks; add a short "Parallel Batch
Note" to Layer 3 explaining that for a batch [startIndex..endIndex] the
coordinator runs Layer 3 with `taskIndex = parallelGroup.startIndex`, and show
the corrected expected-count formula and rejection behavior when actual !=
expected.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
docs/plans/2026-02-21-ci-and-review-comments-plan.md (1)
28-28: Add language identifiers to fenced code blocks (MD040).The following lines contain fenced code blocks without a language specifier: 28, 67, 79, 93, 165, 170, 236. For the shell-output block at line 28 use
text; for the bash/markdown blocks usebashormarkdownaccordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-02-21-ci-and-review-comments-plan.md` at line 28, Add explicit language identifiers to all fenced code blocks in this document: change the shell-output block to use ```text and update the example command/output blocks and script snippets to use ```bash or ```markdown as appropriate so linters no longer flag MD040; locate the anonymous fenced blocks (the shell-output example and the various bash/markdown examples) and prepend the correct language token to each fence.plugins/ralph-specum/commands/design.md (1)
117-117: Add a language specifier to the walkthrough display block (MD040).♻️ Proposed fix
-``` +```text Design complete for '$spec'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/commands/design.md` at line 117, The Markdown code fence in the walkthrough display block currently lacks a language specifier (triggering MD040); update the fence that outputs the walkthrough message (the block surrounding the line "Design complete for '$spec'.") to use a language tag by replacing the opening ``` with ```text so the block reads as a text code block and satisfies MD040.plugins/ralph-specum/references/verification-layers.md (1)
86-86: Add language specifiers to fenced code blocks (MD040).The review loop block at line 86 and the
$priorFindingstemplate block at line 144 both lack language identifiers. Usetextfor these plain-text/pseudo-code blocks.♻️ Proposed fix
-``` +```text Set reviewIteration = 1 ...-``` +```text Prior findings (from iteration $prevIteration): ...Also applies to: 144-144
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/verification-layers.md` at line 86, Add explicit language specifiers to the two fenced code blocks that are plain text: the review loop block (the block starting with "Set reviewIteration = 1" referred to as the review loop) and the $priorFindings template block (the block titled "Prior findings (from iteration $prevIteration)"). Update both triple-backtick fences to use ```text so markdown lint rule MD040 is satisfied.plugins/ralph-specum/references/intent-classification.md (1)
135-141: Add a blank line before the Name Inference examples table.
markdownlintMD058 flags line 136: the table begins immediately after theExamples:label with no blank line.♻️ Proposed fix
Examples: + | Goal | Inferred Name |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/intent-classification.md` around lines 135 - 141, The Examples section violates markdownlint MD058 because the table starts immediately after the "Examples:" label; add a single blank line between the "Examples:" heading/label and the table so the table is separated (i.e., insert a blank line after the "Examples:" line before the table rows shown in the diff) to satisfy markdownlint.plugins/ralph-specum/references/failure-recovery.md (1)
505-516: Add language specifiers to fenced code blocks (MD040).Both inline code templates at lines 505 and 513 are bare triple-backtick blocks. Use
markdownsince they contain Markdown list syntax.♻️ Proposed fix
- ``` - - Task $taskId: $attempts fixes attempted ($fixTaskIds) - Final: PASS - ``` + ```markdown + - Task $taskId: $attempts fixes attempted ($fixTaskIds) - Final: PASS + ```- ``` - - Task $taskId: $attempts fixes attempted ($fixTaskIds) - Final: FAIL (max limit) - ``` + ```markdown + - Task $taskId: $attempts fixes attempted ($fixTaskIds) - Final: FAIL (max limit) + ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/failure-recovery.md` around lines 505 - 516, Change the two bare fenced code blocks that contain the list lines "- Task $taskId: $attempts fixes attempted ($fixTaskIds) - Final: PASS" and "- Task $taskId: $attempts fixes attempted ($fixTaskIds) - Final: FAIL (max limit)" to explicitly use the markdown language specifier by replacing the opening ``` with ```markdown for both occurrences (the PASS block and the FAIL (max limit) block) so the code fences are ```markdown ... ``` instead of plain ```.plugins/ralph-specum/references/coordinator-pattern.md (1)
319-342: State update sections lack an explicit jq merge command, unlike the Modification Request Handler.Lines 326 and 338 both say "Write updated state (merge, preserving all existing fields)" but provide no concrete jq invocation. The Modification Request Handler (lines 503–516) supplies a full jq pipeline as a guard against overwriting. Without the same guardrail here, the coordinator may write a bare JSON object containing only the three changed fields and silently clobber
source,name,basePath,commitSpec,relatedSpecs,modificationMap, etc.♻️ Suggested jq snippet for both sequential and parallel update steps
-5. Write updated state (merge, preserving all existing fields) +5. Write updated state (merge, preserving all existing fields): + ```bash + jq --argjson idx "$NEXT_TASK_INDEX" \ + --argjson iter 1 \ + --argjson gIter "$NEW_GLOBAL_ITERATION" \ + '.taskIndex = $idx | .taskIteration = $iter | .globalIteration = $gIter' \ + "$SPEC_PATH/.ralph-state.json" > "$SPEC_PATH/.ralph-state.json.tmp" && \ + mv "$SPEC_PATH/.ralph-state.json.tmp" "$SPEC_PATH/.ralph-state.json" + ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ralph-specum/references/coordinator-pattern.md` around lines 319 - 342, The sequential and parallel state-update steps must use an explicit jq merge pipeline (like the Modification Request Handler) instead of writing a bare object; update the "Write updated state (merge, preserving all existing fields)" steps to read "$SPEC_PATH/.ralph-state.json", run jq to set .taskIndex to the computed NEXT_TASK_INDEX (or parallelGroup.endIndex + 1), .taskIteration to 1, and .globalIteration to the new global iteration value, output to a temp file and atomically mv it back to "$SPEC_PATH/.ralph-state.json" so existing fields (source, name, basePath, commitSpec, relatedSpecs, modificationMap, etc.) are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/plans/2026-02-21-ci-and-review-comments-plan.md`:
- Around line 1-13: The top-level document heading "CI Fix + CodeRabbit Review
Comments Implementation Plan" is h1 while the subsequent task headings use h3,
violating MD001; update each task heading ("Task 1" through "Task 8") from ###
to ## so they follow h1 → h2 → h3 progression consistently (i.e., make "### Task
1" ... "### Task 8" into "## Task 1" ... "## Task 8") and scan the rest of the
markdown for any other skipped heading levels to ensure no further h-level
jumps.
- Around line 28-31: Update the stale expected output in Task 1's verify step so
it matches the final state after Task 8: change the expected version for the
symbol "ralph-specum" from 3.11.1 to 3.11.2 (leave "ralph-speckit" as-is unless
other tasks changed it); locate the verification block referenced as Task 1's
verify step and replace the old expected line to reflect "ralph-specum: 3.11.2"
to avoid future mismatches when the full plan is applied.
In `@plugins/ralph-specum/commands/tasks.md`:
- Around line 22-31: The Step 1 flow calls ralph_find_spec() /
ralph_resolve_current() but never stores the resolved path—bind the result to a
variable (e.g., $SPEC_PATH) immediately after resolution (as done in
implement.md Step 1) and use $SPEC_PATH for all subsequent file operations
instead of the hardcoded "./specs/$spec/"; update every use site referenced in
this diff (spawn/collect/walkthrough/git add and any checks for design.md,
requirements.md, .ralph-state.json, .progress.md, research.md) to reference
$SPEC_PATH so specs in non-default directories resolve correctly.
- Around line 159-163: The state update step currently overwrites
.ralph-state.json instead of merging — implement the same explicit jq merge used
in other phase commands: compute TOTAL_TASKS (as used near the tasks phase
logic), then run jq with '. +
{"phase":"tasks","totalTasks":$total,"awaitingApproval":true}' against
.ralph-state.json, write to a temp file (e.g. .ralph-state.json.tmp) and mv it
back to .ralph-state.json to preserve existing fields like source, name,
basePath, commitSpec and relatedSpecs.
In `@plugins/ralph-specum/references/coordinator-pattern.md`:
- Line 220: The current "Fix Task Bypass" detection is too permissive because it
matches any description containing "[FIX" (e.g., "[FIXME]" or "[FIXER]"); update
the check that inspects the task description to only recognize the canonical
marker "[FIX]" (exact match, ideally case-insensitive if required) before
skipping verification layers. Locate the logic that decides to "skip
verification layers" or "proceed directly to retry the original task" (the block
referencing "Fix Task Bypass" / "Execute Fix Task and Retry Original") and
replace the loose substring check with a strict equality/regex for the full
token "[FIX]" so only true fix tasks trigger the bypass. Ensure any test or
downstream branch that treats the task as intermediate uses the same strict
match.
- Around line 273-287: Layer 3's checkmark verification currently computes
expected as taskIndex + 1 which fails for parallel batches because taskIndex is
the startIndex; change the logic in the verification routine that reads tasks.md
(the check that computes expected vs actual) to detect when the current work is
a parallel group (use the parallelGroup or batch metadata used by the
coordinator/spec-executor) and compute expected = parallelGroup.endIndex + 1 for
parallel batches (fall back to taskIndex + 1 for sequential). Ensure the code
path that increments taskIteration and triggers a retry uses this adjusted
expected value so parallel batches aren’t endlessly retried.
In `@plugins/ralph-specum/references/failure-recovery.md`:
- Around line 262-268: The state update jq command already increments
.totalTasks, so remove the duplicate increment from the "Insert Fix Task into
tasks.md" algorithm's sub-step 6 (the step that currently also does `.totalTasks
+= 1`) to avoid double-counting; keep the increment only in the jq/state-update
block that modifies .fixTaskMap[$taskId] and .totalTasks, and ensure the insert
routine only adds the new fix entry and its id to
.fixTaskMap[$taskId].fixTaskIds without touching .totalTasks.
In `@plugins/ralph-specum/references/verification-layers.md`:
- Around line 190-195: Verification Summary item 3 incorrectly states a simple
"Checkmark count matches expected taskIndex + 1"; update the summary to reflect
Layer 3's full formula by replacing that line with a description that mentions
both modes: the standard expectation (taskIndex + 1) and the alternate
recovery-mode calculation defined in the Layer 3 block (the recovery-specific
checkmark logic referenced in the Layer 3 definition), ensuring the summary
mirrors the exact conditional rules in the Layer 3 section.
---
Duplicate comments:
In `@plugins/ralph-specum/commands/design.md`:
- Around line 22-29: The spec resolution result from
ralph_find_spec()/ralph_resolve_current() is never assigned to $SPEC_PATH, so
replace the hardcoded "./specs/$spec/" usage by binding the resolved path: set
$SPEC_PATH to the returned resolution (the same value used later in the jq merge
reference) immediately after calling ralph_find_spec()/ralph_resolve_current();
then update all file operations to use $SPEC_PATH (checks for directory
existence, checks for requirements.md, reading .ralph-state.json and updating
awaitingApproval:false, and reading requirements.md, research.md, .progress.md)
instead of the "./specs/$spec/" literal so specs outside the default root work
correctly.
In `@plugins/ralph-specum/commands/research.md`:
- Around line 86-92: The fenced code blocks that show the pre-step template
"Research topics identified for parallel execution:" and the walkthrough block
that prints "Research complete for '$spec'." are missing language specifiers
causing MD040/MD058 lint warnings; update both fenced blocks to use a language
tag (e.g., ```text) so they become ```text ... ``` rather than plain ```,
ensuring the pre-step block containing "Research topics identified for parallel
execution:" and the walkthrough block containing "Research complete for
'$spec'." are both corrected.
In `@plugins/ralph-specum/references/coordinator-pattern.md`:
- Around line 238-302: Summary: Remove duplicate spec-reviewer invocation and
replace namespaced subagent_type values with bare agent names. Fix: In the
Verification Layers section (Layer 5 and surrounding), remove any duplicate
invocation of "ralph-specum:spec-reviewer" so the review delegation only calls
the single referenced file
`${CLAUDE_PLUGIN_ROOT}/references/verification-layers.md`, and audit every
occurrence of the subagent_type field to change names like
"ralph-specum:spec-reviewer" to the bare agent name (e.g., "spec-reviewer") so
all subagent_type references are consistent; verify Layer 5 cleanly delegates to
the referenced verification-layers.md and that no other duplicate or namespaced
invocations remain.
In `@plugins/ralph-specum/references/verification-layers.md`:
- Around line 45-68: Layer 3's expected-checkmark calculation uses taskIndex
directly which fails for tasks that are part of a parallel batch; detect when
the current task has a parallelGroup (or similar parallel batch metadata) and
use the parallelGroup.startIndex (the same approach used in Layer 5) as the
representative index instead of taskIndex when computing EXPECTED and when
summing FIX_COUNT from .fixTaskMap; update the logic that sets EXPECTED (and the
RECOVERY_MODE adjustment) to use representativeIndex = (parallelGroup.startIndex
|| taskIndex) so the comparison uses endIndex-derived actual checkmarks and
avoids false REJECTs for completed parallel batches.
---
Nitpick comments:
In `@docs/plans/2026-02-21-ci-and-review-comments-plan.md`:
- Line 28: Add explicit language identifiers to all fenced code blocks in this
document: change the shell-output block to use ```text and update the example
command/output blocks and script snippets to use ```bash or ```markdown as
appropriate so linters no longer flag MD040; locate the anonymous fenced blocks
(the shell-output example and the various bash/markdown examples) and prepend
the correct language token to each fence.
In `@plugins/ralph-specum/commands/design.md`:
- Line 117: The Markdown code fence in the walkthrough display block currently
lacks a language specifier (triggering MD040); update the fence that outputs the
walkthrough message (the block surrounding the line "Design complete for
'$spec'.") to use a language tag by replacing the opening ``` with ```text so
the block reads as a text code block and satisfies MD040.
In `@plugins/ralph-specum/references/coordinator-pattern.md`:
- Around line 319-342: The sequential and parallel state-update steps must use
an explicit jq merge pipeline (like the Modification Request Handler) instead of
writing a bare object; update the "Write updated state (merge, preserving all
existing fields)" steps to read "$SPEC_PATH/.ralph-state.json", run jq to set
.taskIndex to the computed NEXT_TASK_INDEX (or parallelGroup.endIndex + 1),
.taskIteration to 1, and .globalIteration to the new global iteration value,
output to a temp file and atomically mv it back to
"$SPEC_PATH/.ralph-state.json" so existing fields (source, name, basePath,
commitSpec, relatedSpecs, modificationMap, etc.) are preserved.
In `@plugins/ralph-specum/references/failure-recovery.md`:
- Around line 505-516: Change the two bare fenced code blocks that contain the
list lines "- Task $taskId: $attempts fixes attempted ($fixTaskIds) - Final:
PASS" and "- Task $taskId: $attempts fixes attempted ($fixTaskIds) - Final: FAIL
(max limit)" to explicitly use the markdown language specifier by replacing the
opening ``` with ```markdown for both occurrences (the PASS block and the FAIL
(max limit) block) so the code fences are ```markdown ... ``` instead of plain
```.
In `@plugins/ralph-specum/references/intent-classification.md`:
- Around line 135-141: The Examples section violates markdownlint MD058 because
the table starts immediately after the "Examples:" label; add a single blank
line between the "Examples:" heading/label and the table so the table is
separated (i.e., insert a blank line after the "Examples:" line before the table
rows shown in the diff) to satisfy markdownlint.
In `@plugins/ralph-specum/references/verification-layers.md`:
- Line 86: Add explicit language specifiers to the two fenced code blocks that
are plain text: the review loop block (the block starting with "Set
reviewIteration = 1" referred to as the review loop) and the $priorFindings
template block (the block titled "Prior findings (from iteration
$prevIteration)"). Update both triple-backtick fences to use ```text so markdown
lint rule MD040 is satisfied.
| # CI Fix + CodeRabbit Review Comments Implementation Plan | ||
|
|
||
| > **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. | ||
|
|
||
| **Goal:** Fix the CI failure (marketplace.json version mismatch for ralph-speckit) and address 6 actionable CodeRabbit review comments on PR #96. | ||
|
|
||
| **Architecture:** All changes are to markdown plugin files. One CI-blocking fix, then editorial/logic fixes to reference files, commands, and templates. | ||
|
|
||
| **Tech Stack:** Markdown (Claude Code plugin files), git | ||
|
|
||
| --- | ||
|
|
||
| ### Task 1: Fix CI — bump ralph-speckit version in marketplace.json |
There was a problem hiding this comment.
Fix heading level skip: h1 → h3 without an h2 (MD001).
Line 1 is # CI Fix … (h1) and line 13 jumps to ### Task 1: (h3), skipping the h2 level. Markdownlint flags this as a heading-increment violation.
📄 Proposed fix
-### Task 1: Fix CI — bump ralph-speckit version in marketplace.json
+## Task 1: Fix CI — bump ralph-speckit version in marketplace.jsonApply the same ## level to Task 2 through Task 8 headings for consistency.
📝 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.
| # CI Fix + CodeRabbit Review Comments Implementation Plan | |
| > **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. | |
| **Goal:** Fix the CI failure (marketplace.json version mismatch for ralph-speckit) and address 6 actionable CodeRabbit review comments on PR #96. | |
| **Architecture:** All changes are to markdown plugin files. One CI-blocking fix, then editorial/logic fixes to reference files, commands, and templates. | |
| **Tech Stack:** Markdown (Claude Code plugin files), git | |
| --- | |
| ### Task 1: Fix CI — bump ralph-speckit version in marketplace.json | |
| # CI Fix + CodeRabbit Review Comments Implementation Plan | |
| > **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. | |
| **Goal:** Fix the CI failure (marketplace.json version mismatch for ralph-speckit) and address 6 actionable CodeRabbit review comments on PR `#96`. | |
| **Architecture:** All changes are to markdown plugin files. One CI-blocking fix, then editorial/logic fixes to reference files, commands, and templates. | |
| **Tech Stack:** Markdown (Claude Code plugin files), git | |
| --- | |
| ## Task 1: Fix CI — bump ralph-speckit version in marketplace.json |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~7-~7: Did you mean the formatting language “Markdown” (= proper noun)?
Context: .... Architecture: All changes are to markdown plugin files. One CI-blocking fix, then...
(MARKDOWN_NNP)
🪛 markdownlint-cli2 (0.21.0)
[warning] 13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-02-21-ci-and-review-comments-plan.md` around lines 1 - 13,
The top-level document heading "CI Fix + CodeRabbit Review Comments
Implementation Plan" is h1 while the subsequent task headings use h3, violating
MD001; update each task heading ("Task 1" through "Task 8") from ### to ## so
they follow h1 → h2 → h3 progression consistently (i.e., make "### Task 1" ...
"### Task 8" into "## Task 1" ... "## Task 8") and scan the rest of the markdown
for any other skipped heading levels to ensure no further h-level jumps.
| ``` | ||
| ralph-specum: 3.11.1 | ||
| ralph-speckit: 0.5.1 | ||
| ``` |
There was a problem hiding this comment.
Stale expected output in Task 1's verify step.
Line 29 expects ralph-specum: 3.11.1, but Task 8 later bumps it to 3.11.2. Any reader re-running the verification after the full plan has been applied will see a mismatch. Since this is an archived plan document, updating the comment to reflect the final state avoids confusion.
📄 Proposed fix
Expected:
-ralph-specum: 3.11.1
+ralph-specum: 3.11.2
ralph-speckit: 0.5.1🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-02-21-ci-and-review-comments-plan.md` around lines 28 - 31,
Update the stale expected output in Task 1's verify step so it matches the final
state after Task 8: change the expected version for the symbol "ralph-specum"
from 3.11.1 to 3.11.2 (leave "ralph-speckit" as-is unless other tasks changed
it); locate the verification block referenced as Task 1's verify step and
replace the old expected line to reflect "ralph-specum: 3.11.2" to avoid future
mismatches when the full plan is applied.
| ## Step 1: Gather Context | ||
|
|
||
| <mandatory> | ||
| **Skip interview if --quick flag detected in $ARGUMENTS.** | ||
|
|
||
| If NOT quick mode, conduct interview using AskUserQuestion before delegating to subagent. | ||
| </mandatory> | ||
| 1. If `$ARGUMENTS` contains a spec name, use `ralph_find_spec()` to resolve it; otherwise use `ralph_resolve_current()` | ||
| 2. If no active spec, error: "No active spec. Run /ralph-specum:new <name> first." | ||
| 3. Check the resolved spec directory exists | ||
| 4. Check `design.md` exists. If not, error: "Design not found. Run /ralph-specum:design first." | ||
| 5. Check `requirements.md` exists | ||
| 6. Read `.ralph-state.json`; clear approval flag: `awaitingApproval: false` | ||
| 7. Read context: `requirements.md`, `design.md`, `research.md` (if exists), `.progress.md` | ||
|
|
There was a problem hiding this comment.
$SPEC_PATH not bound in Step 1; all file operations use hardcoded ./specs/$spec/ paths.
Step 1 resolves the spec directory via ralph_find_spec() / ralph_resolve_current() (line 24) but never stores the result in a variable. All downstream file operations (spawn output line 91, collect line 94, walkthrough line 125, git add line 169) hardcode ./specs/$spec/, which breaks specs stored in non-default directories (e.g., ./packages/api/specs/my-feature). implement.md solves this with an explicit binding at its Step 1, line 40.
🐛 Proposed fix
Add to Step 1:
3. Check the resolved spec directory exists
+4. Set `$SPEC_PATH` to the resolved spec directory path. All subsequent file references use this variable.
4. Check `design.md` exists...Then replace hardcoded paths:
- Output to `./specs/$spec/tasks.md`.
+ Output to `$SPEC_PATH/tasks.md`.-7. **Collect results**: Read `./specs/$spec/tasks.md`.
+7. **Collect results**: Read `$SPEC_PATH/tasks.md`.-Read `./specs/$spec/tasks.md` and display:
+Read `$SPEC_PATH/tasks.md` and display:-git add ./specs/$spec/tasks.md
+git add "$SPEC_PATH/tasks.md"Also applies to: 91-95, 125-125, 168-172
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/ralph-specum/commands/tasks.md` around lines 22 - 31, The Step 1 flow
calls ralph_find_spec() / ralph_resolve_current() but never stores the resolved
path—bind the result to a variable (e.g., $SPEC_PATH) immediately after
resolution (as done in implement.md Step 1) and use $SPEC_PATH for all
subsequent file operations instead of the hardcoded "./specs/$spec/"; update
every use site referenced in this diff (spawn/collect/walkthrough/git add and
any checks for design.md, requirements.md, .ralph-state.json, .progress.md,
research.md) to reference $SPEC_PATH so specs in non-default directories resolve
correctly.
| ### Update State | ||
|
|
||
| **After update, repeat review questions.** Loop until user approves. | ||
| 1. Count total tasks from generated file | ||
| 2. Update `.ralph-state.json`: `{ "phase": "tasks", "totalTasks": <count>, "awaitingApproval": true }` | ||
| 3. Update `.progress.md`: mark design as implicitly approved, set current phase, update task count |
There was a problem hiding this comment.
State update at line 162 is missing the explicit jq merge — risks overwriting existing fields.
Every other phase command (design.md, requirements.md, research.md) uses an explicit jq '. + {...}' merge with a temp-file rename to preserve fields like source, name, basePath, commitSpec, and relatedSpecs. This step says only Update '.ralph-state.json': { "phase": "tasks", "totalTasks": ..., "awaitingApproval": true } with no path, no merge directive, and no jq command. If implemented naively it silently drops all fields set by earlier phases, causing implement.md to fail when it reads commitSpec or basePath.
Apply the same merge pattern used by the other phase commands:
🐛 Proposed fix
-1. Count total tasks from generated file
-2. Update `.ralph-state.json`: `{ "phase": "tasks", "totalTasks": <count>, "awaitingApproval": true }`
-3. Update `.progress.md`: mark design as implicitly approved, set current phase, update task count
+1. Count total tasks from generated file
+2. **Merge** into `.ralph-state.json` (preserve all existing fields):
+ ```bash
+ TOTAL_TASKS=$(grep -c '^\- \[' "$SPEC_PATH/tasks.md" || echo 0)
+ jq --argjson total "$TOTAL_TASKS" \
+ '. + {"phase": "tasks", "totalTasks": $total, "awaitingApproval": true}' \
+ "$SPEC_PATH/.ralph-state.json" > "$SPEC_PATH/.ralph-state.json.tmp" && \
+ mv "$SPEC_PATH/.ralph-state.json.tmp" "$SPEC_PATH/.ralph-state.json"
+ ```
+3. Update `.progress.md`: mark design as implicitly approved, set current phase, update task count🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/ralph-specum/commands/tasks.md` around lines 159 - 163, The state
update step currently overwrites .ralph-state.json instead of merging —
implement the same explicit jq merge used in other phase commands: compute
TOTAL_TASKS (as used near the tasks phase logic), then run jq with '. +
{"phase":"tasks","totalTasks":$total,"awaitingApproval":true}' against
.ralph-state.json, write to a temp file (e.g. .ralph-state.json.tmp) and mv it
back to .ralph-state.json to preserve existing fields like source, name,
basePath, commitSpec and relatedSpecs.
|
|
||
| ### After Delegation | ||
|
|
||
| **Fix Task Bypass**: If the just-completed task is a fix task (task description contains `[FIX`), skip verification layers entirely and proceed directly to retry the original task per `${CLAUDE_PLUGIN_ROOT}/references/failure-recovery.md` "Execute Fix Task and Retry Original" section. Fix tasks are intermediate — only the original task's completion triggers full verification. |
There was a problem hiding this comment.
Overly broad fix-task detection — [FIX without closing bracket can match [FIXME] or [FIXER].
If a task description contains [FIXME] or any other [FIX… prefix (e.g., carried in from a requirement), the coordinator will bypass all five verification layers for that task.
🐛 Tighten the match to the canonical marker
-If the just-completed task is a fix task (task description contains `[FIX`), skip verification layers entirely
+If the just-completed task is a fix task (task description matches `[FIX]` or `[FIX-`), skip verification layers entirely📝 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.
| **Fix Task Bypass**: If the just-completed task is a fix task (task description contains `[FIX`), skip verification layers entirely and proceed directly to retry the original task per `${CLAUDE_PLUGIN_ROOT}/references/failure-recovery.md` "Execute Fix Task and Retry Original" section. Fix tasks are intermediate — only the original task's completion triggers full verification. | |
| **Fix Task Bypass**: If the just-completed task is a fix task (task description matches `[FIX]` or `[FIX-`), skip verification layers entirely and proceed directly to retry the original task per `${CLAUDE_PLUGIN_ROOT}/references/failure-recovery.md` "Execute Fix Task and Retry Original" section. Fix tasks are intermediate — only the original task's completion triggers full verification. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/ralph-specum/references/coordinator-pattern.md` at line 220, The
current "Fix Task Bypass" detection is too permissive because it matches any
description containing "[FIX" (e.g., "[FIXME]" or "[FIXER]"); update the check
that inspects the task description to only recognize the canonical marker
"[FIX]" (exact match, ideally case-insensitive if required) before skipping
verification layers. Locate the logic that decides to "skip verification layers"
or "proceed directly to retry the original task" (the block referencing "Fix
Task Bypass" / "Execute Fix Task and Retry Original") and replace the loose
substring check with a strict equality/regex for the full token "[FIX]" so only
true fix tasks trigger the bypass. Ensure any test or downstream branch that
treats the task as intermediate uses the same strict match.
| **Layer 3: Checkmark Verification** | ||
|
|
||
| Count completed tasks in tasks.md: | ||
|
|
||
| ```bash | ||
| grep -c '\- \[x\]' $SPEC_PATH/tasks.md | ||
| ``` | ||
|
|
||
| Expected checkmark count = taskIndex + 1 (0-based index, so task 0 complete = 1 checkmark) | ||
|
|
||
| If actual count != expected: | ||
| - REJECT the completion | ||
| - Log: "checkmark mismatch: expected $expected, found $actual" | ||
| - This detects state manipulation or incomplete task marking | ||
| - Increment taskIteration and retry |
There was a problem hiding this comment.
Layer 3 checkmark formula breaks all parallel batch completions.
For sequential tasks taskIndex + 1 is correct — but for a parallel batch the verification layers run while taskIndex is still the startIndex (line 240: "BEFORE advancing taskIndex"). Every task in the batch has already been marked [x] by its own spec-executor, so the actual checkmark count is previousCompletedTasks + batchSize, not startIndex + 1.
Concrete example — batch tasks 3, 4, 5 (tasks 0–2 already done):
| Value | |
|---|---|
taskIndex at verification time |
3 (startIndex, not yet advanced) |
| Layer 3 expected | 3 + 1 = 4 |
Actual [x] count |
6 (tasks 0–5 all marked) |
| Result | FAIL → taskIteration is incremented → the entire batch is retried endlessly |
The expected count for parallel execution should be parallelGroup.endIndex + 1.
🐛 Proposed fix
-Expected checkmark count = taskIndex + 1 (0-based index, so task 0 complete = 1 checkmark)
+Expected checkmark count:
+- **Sequential**: `taskIndex + 1`
+- **Parallel batch**: `parallelGroup.endIndex + 1` (all tasks up to and including the last batch task should be marked `[x]`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/ralph-specum/references/coordinator-pattern.md` around lines 273 -
287, Layer 3's checkmark verification currently computes expected as taskIndex +
1 which fails for parallel batches because taskIndex is the startIndex; change
the logic in the verification routine that reads tasks.md (the check that
computes expected vs actual) to detect when the current work is a parallel group
(use the parallelGroup or batch metadata used by the coordinator/spec-executor)
and compute expected = parallelGroup.endIndex + 1 for parallel batches (fall
back to taskIndex + 1 for sequential). Ensure the code path that increments
taskIteration and triggers a retry uses this adjusted expected value so parallel
batches aren’t endlessly retried.
| .fixTaskMap[$taskId].attempts += 1 | | ||
| .fixTaskMap[$taskId].fixTaskIds += [$fixId] | | ||
| .fixTaskMap[$taskId].lastError = $error | | ||
|
|
||
| # Also increment totalTasks to account for inserted fix task | ||
| .totalTasks += 1 | ||
| ' "$SPEC_PATH/.ralph-state.json" > "$SPEC_PATH/.ralph-state.json.tmp" && \ |
There was a problem hiding this comment.
totalTasks increment is specified in two separate steps — risk of double-counting.
The jq command in "Update State After Fix Task Generation" (line 267) already includes .totalTasks += 1. The "Insert Fix Task into tasks.md" algorithm then repeats the same increment in its own sub-step 6 (lines 376–379). If the coordinator follows both sections sequentially (step 6 = insert, step 7 = update state), totalTasks is incremented twice per fix task.
Consolidate ownership: the increment belongs in the state-update step (step 7 / the jq command). Remove the duplicate sub-step 6 from the insert algorithm.
🐛 Proposed fix
-6. **Update state totalTasks**:
- - Read .ralph-state.json
- - Increment `totalTasks` by 1
- - Write updated state
-
-**Example Insertion**:
+**Example Insertion**:Also applies to: 376-379
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/ralph-specum/references/failure-recovery.md` around lines 262 - 268,
The state update jq command already increments .totalTasks, so remove the
duplicate increment from the "Insert Fix Task into tasks.md" algorithm's
sub-step 6 (the step that currently also does `.totalTasks += 1`) to avoid
double-counting; keep the increment only in the jq/state-update block that
modifies .fixTaskMap[$taskId] and .totalTasks, and ensure the insert routine
only adds the new fix entry and its id to .fixTaskMap[$taskId].fixTaskIds
without touching .totalTasks.
| All 5 layers must pass before advancing: | ||
| 1. No contradiction phrases with completion claim | ||
| 2. Spec files committed (no uncommitted changes) | ||
| 3. Checkmark count matches expected taskIndex + 1 | ||
| 4. Explicit TASK_COMPLETE signal present | ||
| 5. Artifact review passes (spec-reviewer REVIEW_PASS or max iterations with graceful degradation) |
There was a problem hiding this comment.
Verification Summary item 3 is now inconsistent with the actual Layer 3 formula.
Line 193 reads "Checkmark count matches expected taskIndex + 1", but Layer 3 (lines 45–61) now defines a more nuanced formula covering both standard and recovery modes. The summary should reflect this.
♻️ Proposed fix
-3. Checkmark count matches expected taskIndex + 1
+3. Checkmark count matches expected (taskIndex + 1, adjusted for recovery fix tasks and parallel batches per Layer 3)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/ralph-specum/references/verification-layers.md` around lines 190 -
195, Verification Summary item 3 incorrectly states a simple "Checkmark count
matches expected taskIndex + 1"; update the summary to reflect Layer 3's full
formula by replacing that line with a description that mentions both modes: the
standard expectation (taskIndex + 1) and the alternate recovery-mode calculation
defined in the Layer 3 block (the recovery-specific checkmark logic referenced
in the Layer 3 definition), ensuring the summary mirrors the exact conditional
rules in the Layer 3 section.
Add user-invocable: false to 4 internal skills that are background knowledge for agents, not user actions: delegation-principle, reality-verification, interview-framework, communication-style. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Fresh approach replacing stale PR #79. Applies plugin-dev best practices to both plugins across 3 phases:
colorto 14 agents,version: 0.1.0to 10 skills, examples to feedback.mdreferences/directory. Commands slimmed 60-88%--quick)Key stats
Design decisions
references/(notskills/) to avoid bloating the/namespace — Claude Code has nointernalskill mechanism${CLAUDE_PLUGIN_ROOT}/references/templates/prompts/with placeholder-based subagent dispatch (executor, reviewer, research)--quickflagDesign doc
docs/plans/2026-02-20-plugin-best-practices-design.mdTest plan
colorfieldversionfieldreferences/templates/prompts//skill namespace entries (still 6)claude --plugin-dir ./plugins/ralph-specum🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores