-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Production-Tested Improvements: Mechanical Enforcement & Safety Gates #170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughForks plugin metadata to "superpowers-fork", adds a developer CLAUDE guide, introduces a write-plan wrapper and multiple enforcement/validation scripts (lock, rename, frontmatter validation, acceptance generation, pre-commit hook), and tightens several SKILL.md workflows for stronger TDD and PR-preview gating. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~50–70 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 16
🧹 Nitpick comments (32)
skills/writing-skills/anthropic-best-practices.md (2)
149-150: Fix unordered list indentation in Note and Warning blocks.The lists within the Note block (lines 149-150) and Warning block (lines 192-194) have inconsistent indentation. Markdownlint expects lists at the root level to have zero indentation; nested lists within blocks should follow the block structure. This affects rendering consistency.
Apply these diffs to correct the indentation:
- Note block (lines 146-153): <Note> **YAML Frontmatter**: The SKILL.md frontmatter supports two fields: - * `name` - Human-readable name of the Skill (64 characters maximum) - * `description` - One-line description of what the Skill does and when to use it (1024 characters maximum) + - `name` - Human-readable name of the Skill (64 characters maximum) + - `description` - One-line description of what the Skill does and when to use it (1024 characters maximum) For complete Skill structure details, see the [Skills overview](/en/docs/agents-and-tools/agent-skills/overview#skill-structure). </Note> - Warning block (lines 189-195): <Warning> **Always write in third person**. The description is injected into the system prompt, and inconsistent point-of-view can cause discovery problems. - * **Good:** "Processes Excel files and generates reports" - * **Avoid:** "I can help you process Excel files" - * **Avoid:** "You can use this to process Excel files" + - **Good:** "Processes Excel files and generates reports" + - **Avoid:** "I can help you process Excel files" + - **Avoid:** "You can use this to process Excel files" </Warning>Also applies to: 192-194
257-257: Specify language identifiers in fenced code blocks.Three code blocks are missing language specifications (lines 257, 301, 1040), which reduces syntax highlighting and clarity. Add language tags to each:
- Line 257: File tree structure → use
textorplaintext- Line 301: File tree structure → use
textorplaintext- Line 1040: File tree structure → use
textorplaintextExample fix for line 257:
-``` +```text pdf/ ├── SKILL.md # Main instructions (loaded when triggered) ├── FORMS.md # Form-filling guide (loaded as needed) ... -``` +```Repeat this pattern for lines 301 and 1040.
Also applies to: 301-301, 1040-1040
scripts/reload-plugin.sh (1)
1-20: Consider reading marketplace name from marketplace.json.The script hardcodes
superpowers-devon lines 17-18. If the marketplace name in.claude-plugin/marketplace.jsonchanges, this script would need manual updates.Apply this diff to make it more maintainable:
+MARKETPLACE=$(jq -r '.marketplaces[0].name // "superpowers-dev"' "$(dirname "$0")/../.claude-plugin/marketplace.json" 2>/dev/null || echo "superpowers-dev") + echo "==> Superpowers Plugin Reload Commands" echo "" echo "Copy and paste these into Claude Code (paste both lines at once):" echo "" -echo "/plugin uninstall superpowers@superpowers-dev" -echo "/plugin install superpowers@superpowers-dev" +echo "/plugin uninstall superpowers@${MARKETPLACE}" +echo "/plugin install superpowers@${MARKETPLACE}" echo "" echo "IMPORTANT: After reload, start a new session to see changes."hooks/run-hook.cmd (3)
7-11: Add validation for script existence.The Windows section doesn't validate whether the target script exists before attempting to execute it, which could lead to cryptic errors.
Apply this diff to add validation:
if "%~1"=="" ( echo run-hook.cmd: missing script name >&2 exit /b 1 ) +if not exist "%~dp0%~1" ( + echo run-hook.cmd: script not found: %~1 >&2 + exit /b 1 +) "C:\Program Files\Git\bin\bash.exe" -l "%~dp0%~1" %2 %3 %4 %5 %6 %7 %8 %9
11-11: Windows batch argument limitation: maximum 9 arguments.The
%2 %3 %4 %5 %6 %7 %8 %9syntax limits forwarding to 9 arguments. If hooks need more arguments, they'll be silently truncated.If more arguments are needed in the future, consider using
%*in a loop or upgrading to PowerShell.
16-19: Add validation for script existence in Unix section.The Unix section doesn't validate whether the target script exists before attempting to execute it.
Apply this diff:
# Unix shell runs from here SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" SCRIPT_NAME="$1" shift +if [ ! -f "${SCRIPT_DIR}/${SCRIPT_NAME}" ]; then + echo "run-hook.cmd: script not found: ${SCRIPT_NAME}" >&2 + exit 1 +fi "${SCRIPT_DIR}/${SCRIPT_NAME}" "$@"skills/brainstorming/SKILL.md (1)
19-19: Optional: Consider hyphenating "multiple-choice questions".Per grammar conventions, compound adjectives before nouns should be hyphenated.
Based on static analysis hints (LanguageTool).
CLAUDE.md (1)
43-46: Minor: Consider adding language identifier to code block.The static analysis tool suggests adding a language identifier to the fenced code block. While not critical, adding
bashwould improve syntax highlighting and clarity.Apply this diff:
- ``` + ```bash /plugin uninstall superpowers@superpowers-dev /plugin install superpowers@superpowers-dev - ``` + ```RELEASE-NOTES.md (1)
502-504: Optional: Format bare URLs as markdown links.The static analysis tool flags the bare URLs at the end. While functional, converting them to proper markdown links would improve readability and follow markdown best practices.
Apply this diff:
--- -**Full Changelog:** https://github.com/obra/superpowers/compare/dd013f6...main -**Skills Repository:** https://github.com/obra/superpowers-skills -**Issues:** https://github.com/obra/superpowers/issues +**Full Changelog:** [dd013f6...main](https://github.com/obra/superpowers/compare/dd013f6...main) +**Skills Repository:** [obra/superpowers-skills](https://github.com/obra/superpowers-skills) +**Issues:** [GitHub Issues](https://github.com/obra/superpowers/issues)commands/brainstorm.md (1)
5-5: Consider consistent wording across command files.This file uses "Use and follow the brainstorming skill exactly as written" while
commands/write-plan.mduses "Use the record-plans skill exactly as written" (without "and follow"). Consider standardizing the instruction format across command files for consistency.Apply this diff if you'd like to match the write-plan.md style:
-Use and follow the brainstorming skill exactly as written +Use the brainstorming skill exactly as writtendocs/README.opencode.md (2)
9-9: Optional: Add language identifiers to code blocks.Adding language identifiers to the fenced code blocks would enable syntax highlighting and improve readability.
Apply these diffs:
-``` +```text Clone https://github.com/obra/superpowers to ~/.config/opencode/superpowers, then create directory ~/.config/opencode/plugin, then symlink ~/.config/opencode/superpowers/.opencode/plugin/superpowers.js to ~/.config/opencode/plugin/superpowers.js, then restart opencode.```diff -``` +```text use find_skills tool```diff -``` +```text use use_skill tool with skill_name: "superpowers:brainstorming"Also applies to: 57-57, 65-65 --- `213-215`: **Optional: Format URLs as markdown links.** The bare URLs in the "Getting Help" section could be formatted as proper markdown links for better presentation. Apply this diff: ```diff -- Report issues: https://github.com/obra/superpowers/issues -- Main documentation: https://github.com/obra/superpowers -- OpenCode docs: https://opencode.ai/docs/ +- [Report issues](https://github.com/obra/superpowers/issues) +- [Main documentation](https://github.com/obra/superpowers) +- [OpenCode docs](https://opencode.ai/docs/).opencode/INSTALL.md (2)
39-39: Optional: Add language identifiers to code blocks.Adding language identifiers would enable syntax highlighting.
Apply these diffs:
-``` +```text use find_skills tool```diff -``` +```text use use_skill tool with skill_name: "superpowers:brainstorming"Also applies to: 47-47 --- `134-135`: **Optional: Format URLs as markdown links.** The bare URLs could be formatted as proper markdown links. Apply this diff: ```diff -- Report issues: https://github.com/obra/superpowers/issues -- Documentation: https://github.com/obra/superpowers +- [Report issues](https://github.com/obra/superpowers/issues) +- [Documentation](https://github.com/obra/superpowers)docs/README.codex.md (1)
9-11: Add language specifiers to code blocks for better rendering.Several code blocks are missing language specifiers, which prevents proper syntax highlighting and reduces readability.
Apply this diff to add language specifiers:
-``` +```text Fetch and follow instructions from https://raw.githubusercontent.com/obra/superpowers/refs/heads/main/.codex/INSTALL.md-
+text
Run ~/.codex/superpowers/.codex/superpowers-codex find-skills to show available skills-``` +```text Run ~/.codex/superpowers/.codex/superpowers-codex find-skills-
+text
Run ~/.codex/superpowers/.codex/superpowers-codex use-skill superpowers:brainstorming-``` +```text Run ~/.codex/superpowers/.codex/superpowers-codex bootstrapAlso applies to: 37-39, 47-49, 53-55, 59-61 </blockquote></details> <details> <summary>skills/sharing-skills/SKILL.md (2)</summary><blockquote> `85-100`: **Clarify the placeholder repository reference in the gh CLI command.** The `--repo` flag uses a placeholder `upstream-org/upstream-repo` that users need to replace with the actual repository. Consider adding a comment or note to make this clearer, or use a more specific placeholder like `obra/superpowers`. Apply this diff to clarify: ```diff # Create PR to upstream using gh CLI gh pr create \ - --repo upstream-org/upstream-repo \ + --repo obra/superpowers \ # Replace with actual upstream repo if different --title "Add ${skill_name} skill" \ --body "$(cat <<'EOF'
162-180: Convert bold text to proper headings and format URLs consistently.The troubleshooting section uses bold text for subsection titles instead of proper markdown headings. Additionally, bare URLs should be converted to proper links for consistency.
Apply this diff:
## Troubleshooting -**"gh: command not found"** +### "gh: command not found" + -- Install GitHub CLI: https://cli.github.com/ +- Install GitHub CLI: [https://cli.github.com/](https://cli.github.com/) - Authenticate: `gh auth login` -**"Permission denied (publickey)"** +### "Permission denied (publickey)" + - Check SSH keys: `gh auth status` -- Set up SSH: https://docs.github.com/en/authentication +- Set up SSH: [GitHub Authentication Docs](https://docs.github.com/en/authentication) -**"Skill already exists"** +### "Skill already exists" + - You're creating a modified version - Consider different skill name or coordinate with the skill's maintainer -**PR merge conflicts** +### PR merge conflicts + - Rebase on latest upstream: `git fetch upstream && git rebase upstream/main` - Resolve conflicts - Force push: `git push -f origin your-branch`skills/record-plan/scripts/check_lock.py (1)
14-16: Remove extraneous f-string prefixes.Lines 14 and 16 use f-string prefixes but don't contain any placeholders. These should be regular strings.
Apply this diff:
- print(f" python3 ${{CLAUDE_PLUGIN_ROOT}}/skills/record-plan/scripts/write_plan.py \\") + print(" python3 ${CLAUDE_PLUGIN_ROOT}/skills/record-plan/scripts/write_plan.py \\") print(f" --working-dir {working_dir} \\") - print(f" --plan-name <descriptive-name>") + print(" --plan-name <descriptive-name>")skills/record-plan/scripts/write_plan.py (2)
40-52: Remove extraneous f-string prefixes and verify staging path.Lines 45 and 51 use f-string prefixes without placeholders. Additionally, line 42 hardcodes
~/.claude/plansas the staging path, which appears inconsistent with other paths in the codebase that use~/.codex/or~/.config/.Apply this diff to remove extraneous f-strings:
if args.in_plan_mode: staging_path = os.path.expanduser("~/.claude/plans") target_file = f"{staging_path}/{args.plan_name}.md" final_dir = os.path.join(args.working_dir, args.target_dir) - print(f"📝 PLAN MODE DETECTED") + print("📝 PLAN MODE DETECTED") print(f"✓ Write plan to: {target_file}") print(f"✓ Then copy to: {final_dir}/{args.plan_name}.md") else: target_dir = os.path.join(args.working_dir, args.target_dir) target_file = f"{target_dir}/{args.plan_name}.md" - print(f"📝 REGULAR MODE") + print("📝 REGULAR MODE") print(f"✓ Write plan to: {target_file}")Please verify whether
~/.claude/plansis the intended staging path or if it should be~/.codex/plansto match the rest of the codebase.
62-77: Remove extraneous f-string prefixes and verify script paths.Multiple lines use f-string prefixes without placeholders (lines 62-68, 71, 73-75). Additionally, line 69 references
~/.claude/scripts/record-tools/which may be inconsistent with paths elsewhere in the codebase.Apply this diff to remove unnecessary f-strings:
- print(f"\n🔧 REQUIRED ACTIONS:") - print(f"1. Use Write tool to create file with:") - print(f" - First line: <!-- jot:md-rename -->") - print(f" - YAML frontmatter (title, date, type, status)") - print(f" - H1 heading with feature name") - print(f" - Implementation tasks following record-plan format") - print(f"\n2. After writing, run post-write workflow:") + print("\n🔧 REQUIRED ACTIONS:") + print("1. Use Write tool to create file with:") + print(" - First line: <!-- jot:md-rename -->") + print(" - YAML frontmatter (title, date, type, status)") + print(" - H1 heading with feature name") + print(" - Implementation tasks following record-plan format") + print("\n2. After writing, run post-write workflow:") print(f" - Validate: python3 ~/.claude/scripts/record-tools/validate-frontmatter.py {target_file}") print(f" - Rename: python3 ~/.claude/scripts/record-tools/rename_jot.py {target_file} (auto-tracks with file-track)") - print(f" ⚠️ IMPORTANT: Rename script will remove lock file") + print(" ⚠️ IMPORTANT: Rename script will remove lock file") - print(f"\n⚠️ CRITICAL: DO NOT just describe the plan!") - print(f"⚠️ You MUST use Write tool to create the actual file!") - print(f"⚠️ Describing = BUG. Writing = CORRECT.") + print("\n⚠️ CRITICAL: DO NOT just describe the plan!") + print("⚠️ You MUST use Write tool to create the actual file!") + print("⚠️ Describing = BUG. Writing = CORRECT.")Please verify whether
~/.claude/scripts/record-tools/is the correct path or if it should align with other paths in the codebase (e.g.,~/.codex/or similar).skills/finishing-a-development-branch/SKILL.md (1)
201-240: Excellent rationalization table - addresses common bypass patterns.The rationalization table and red flags section effectively preempt common ways an AI agent might rationalize skipping the approval step. The specific examples and rebuttals are clear and actionable.
Minor suggestion: Consider adding one more row to the rationalization table addressing the case where the AI might think "the commits already describe the changes, so preview is redundant" since Git commits != PR presentation.
Apply this diff if you want to add the additional rationalization:
| "This is different from Option 4" | PRs are as permanent as deleted code. Same approval rigor required. | | "I'll describe what I'm creating" | Description ≠ approval. Must show preview AND wait for "yes". | +| "Commits describe changes already" | Commits ≠ PR presentation. User needs to see title/body before publishing. |lib/skills-core.js (1)
148-170: Consider documenting the network timeout behavior more explicitly.The 3-second timeout is good for preventing delays, but the function silently returns
falseon timeout/error without distinguishing between "up-to-date" and "couldn't check". This is mentioned in the comment but might cause confusion.Consider whether callers need to distinguish between these cases. If not, the current behavior is fine.
If callers need to distinguish, consider returning an object:
-function checkForUpdates(repoDir) { +function checkForUpdates(repoDir, options = {}) { + const returnDetail = options.returnDetail || false; try { // Quick check with 3 second timeout to avoid delays if network is down const output = execSync('git fetch origin && git status --porcelain=v1 --branch', { @@ -162,10 +163,14 @@ function checkForUpdates(repoDir) { return true; // We're behind remote } } - return false; // Up to date + return returnDetail ? { hasUpdates: false, error: null } : false; } catch (error) { // Network down, git error, timeout, etc. - don't block bootstrap - return false; + if (returnDetail) { + return { hasUpdates: false, error: error.message }; + } + return false; } }However, for the "Chill" review mode and given the current usage is just for a banner, the existing implementation is perfectly fine.
skills/record-plan/SKILL.md (1)
287-306: Good template copy enforcement, but consider verifying template existence during testing.The IMPORTANT note correctly prevents AI agents from creating custom progress.md formats, which is critical for cross-session consistency. However, the error handling relies on human intervention if the template is missing.
Consider whether the record-plan skill itself should validate template existence during the post-write workflow and fail early if missing.
Add a validation step earlier in the workflow (perhaps right after wrapper invocation) that checks for template existence:
**Step 0.5: Initialize Progress Log (if needed)** **Check if progress.md exists:** ```bash test -f <working-directory>/llm/progress.md && echo "EXISTS" || echo "MISSING"+If output is "MISSING", verify template exists BEFORE attempting copy:
+bash +test -f ~/.claude/templates/progress.md && echo "TEMPLATE_OK" || echo "TEMPLATE_MISSING" +
+
+If template is missing, stop and report to user that setup Task 1 is incomplete.
+
If output is "MISSING", you MUST copy the template (do NOT create your own format):This prevents the agent from getting to the copy command only to encounter an error. </blockquote></details> <details> <summary>.codex/superpowers-codex (1)</summary><blockquote> `6-6`: **Module system inconsistency: Consider using ES modules.** This file uses CommonJS (`require`) while other new files in the PR (lib/skills-core.js, .opencode/plugin/superpowers.js) use ES modules (`import`). The skills-core.js file exports using ES module syntax but this file imports it with CommonJS. For consistency and to avoid potential module loading issues, consider converting this to ES modules or documenting why CommonJS is required for Codex compatibility. To convert to ES modules, change: ```diff #!/usr/bin/env node -const fs = require('fs'); -const path = require('path'); -const os = require('os'); -const skillsCore = require('../lib/skills-core'); +import fs from 'fs'; +import path from 'path'; +import os from 'os'; +import * as skillsCore from '../lib/skills-core.js';And add
"type": "module"to package.json if not already present, or rename the file to.mjs.However, if Codex specifically requires CommonJS, document that requirement.
README.md (1)
11-11: Optional: Consider more formal language.LanguageTool flags "gonna" as informal in "YAGNI (You Aren't Gonna Need It)". While this matches the established YAGNI acronym expansion, consider whether the README's tone should be more formal.
This is purely stylistic - "gonna" is widely accepted in tech writing for YAGNI.
If a more formal tone is preferred:
-After you've signed off on the design, your agent puts together an implementation plan that's clear enough for an enthusiastic junior engineer with poor taste, no judgement, no project context, and an aversion to testing to follow. It emphasizes true red/green TDD, YAGNI (You Aren't Gonna Need It), and DRY. +After you've signed off on the design, your agent puts together an implementation plan that's clear enough for an enthusiastic junior engineer with poor taste, no judgement, no project context, and an aversion to testing to follow. It emphasizes true red/green TDD, YAGNI (You Aren't Going to Need It), and DRY.skills/subagent-driven-development/SKILL.md (1)
1-3: Description is well-structured but quite long—consider tightening.The description at 140+ characters is at the upper end of guidance. Consider condensing to focus on the primary trigger (executing implementation plans with code review between tasks) rather than contrasting with alternatives.
docs/plans/2025-11-22-opencode-support-implementation.md (3)
40-76: YAML frontmatter parsing is functional but could be more robust.The
extractFrontmatterfunction at lines 40-76 uses a regex pattern/^(\w+):\s*(.*)$/that matches only alphanumeric + underscore characters in keys. While this works for the currentnameanddescriptionfields, it will fail silently on YAML keys containing hyphens (e.g.,my-field: value). If frontmatter fields are ever extended, this could cause hidden bugs.Consider updating the regex to be more explicit about supported keys or documenting the limitation:
for (const line of lines) { if (inFrontmatter) { - const match = line.match(/^(\w+):\s*(.*)$/); + const match = line.match(/^(name|description):\s*(.*)$/); if (match) { const [, key, value] = match;This makes the constraint explicit and prevents future confusion.
543-556: DRY violation: frontmatter stripping logic duplicated across tasks.The code to strip YAML frontmatter appears in both the
use_skilltool (lines 543-556) and thesession.startedhook (lines 677-695). This duplication creates maintenance burden and risk of divergence.Consider extracting this into a helper function in the core module:
// In lib/skills-core.js (add after stripFrontmatter if it exists, or create new): function stripFrontmatter(fullContent) { const lines = fullContent.split('\n'); let inFrontmatter = false; let frontmatterEnded = false; const contentLines = []; for (const line of lines) { if (line.trim() === '---') { if (inFrontmatter) { frontmatterEnded = true; continue; } inFrontmatter = true; continue; } if (frontmatterEnded || !inFrontmatter) { contentLines.push(line); } } return contentLines.join('\n').trim(); } module.exports = { extractFrontmatter, findSkillsInDir, resolveSkillPath, checkForUpdates, stripFrontmatter // Add this };Then in the plugin, use
skillsCore.stripFrontmatter()in both places.Also applies to: 677-695
757-977: Documentation phase looks solid; markdown formatting will need attention.The documentation tasks (Phase 4) are well-structured with clear installation steps, usage examples, and troubleshooting guidance. The commit messages are descriptive and follow conventions.
However, the static analysis hints flag multiple markdown issues (MD036 emphasis as heading, MD040 missing language on fenced code blocks, MD034 bare URLs). These should be addressed before merge to maintain consistency with project standards.
Key formatting updates needed in the plan:
- Lines 805, 815, 823, 1028: Add language identifiers to code blocks (e.g.,
```bashinstead of```)- Lines 881-882: Wrap bare URLs in markdown link syntax:
[URL](URL)instead of plain URLs- Review MD036 warnings: some emphasized text should be converted to headers (lines 21, 83, 88, etc. in original file—these appear to be in the implementation steps)
skills/writing-skills/SKILL.md (3)
1-35: Strong mandatory prerequisites align with PR objectives.The EXTREMELY-IMPORTANT block (lines 22-35) and REQUIRED BACKGROUND section (lines 18-20) effectively enforce mechanical requirements before skill modification. This aligns well with the PR's focus on "mechanical enforcement for objective requirements." The language is clear and uses strong directives ("MUST", "STOP") to prevent rationalization.
However, the frontmatter description (line 3) is quite long (~220 characters). While it's informative, consider whether it could be condensed for discoverability:
description: Use when creating, modifying, or testing skills - applies TDD to documentation by testing with subagents first, ensuring bulletproof against rationalization. REQUIRED before modifying any skill file.This maintains the critical "REQUIRED before modifying" constraint while being slightly more scannable.
108-151: SKILL.md structure guidance is clear and well-organized.The frontmatter constraints (lines 109-117) are explicit and align well with the shared core module's field validation from the implementation plan. The structure template (lines 118-150) provides a sensible framework without being overly prescriptive.
One minor note: the template includes a "## Skill Name" heading at line 124, but guidance earlier states to avoid emphasis as heading. Verify that markdown linting passes on this template (may be flagged by MD036).
1-640: Writing-Skills SKILL.md is comprehensive and well-structured overall.This document provides thorough guidance on skill authoring, testing, and deployment. The addition of mandatory prerequisites (EXTREMELY-IMPORTANT block) and REQUIRED BACKGROUND sections aligns well with the PR's focus on mechanical enforcement. The CSO section provides practical discovery optimization guidance, and the testing breakdown by skill type is valuable.
Static analysis flags many MD036 warnings, primarily for procedural step labels like "Step 1:" which are intentionally emphasized rather than heading-level. A few legitimate formatting improvements (adding language to code blocks per MD040, wrapping bare URLs per MD034) would improve consistency, but these are low priority.
Overall assessment: Strong alignment with PR objectives on mechanical enforcement and quality gates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
skills/collaboration/remembering-conversations/tool/package-lock.jsonis excluded by!**/package-lock.jsonskills/writing-skills/graphviz-conventions.dotis excluded by!**/*.dot
📒 Files selected for processing (107)
.claude-plugin/marketplace.json(1 hunks).claude-plugin/plugin.json(1 hunks).codex/INSTALL.md(1 hunks).codex/superpowers-bootstrap.md(1 hunks).codex/superpowers-codex(1 hunks).github/FUNDING.yml(1 hunks).gitignore(1 hunks).opencode/INSTALL.md(1 hunks).opencode/plugin/superpowers.js(1 hunks)CLAUDE.md(1 hunks)README.md(1 hunks)RELEASE-NOTES.md(1 hunks)agents/code-reviewer.md(1 hunks)commands/README.md(0 hunks)commands/brainstorm.md(1 hunks)commands/execute-plan.md(1 hunks)commands/write-plan.md(1 hunks)docs/README.codex.md(1 hunks)docs/README.opencode.md(1 hunks)docs/plans/2025-11-22-opencode-support-design.md(1 hunks)docs/plans/2025-11-22-opencode-support-implementation.md(1 hunks)docs/windows/polyglot-hooks.md(1 hunks)hooks/hooks.json(1 hunks)hooks/run-hook.cmd(1 hunks)hooks/session-start.sh(1 hunks)hooks/setup-personal-superpowers.sh(0 hunks)lib/skills-core.js(1 hunks)scripts/find-skills(0 hunks)scripts/reload-plugin.sh(1 hunks)scripts/skill-run(0 hunks)skills/REQUESTS.md(0 hunks)skills/brainstorming/SKILL.md(1 hunks)skills/collaboration/brainstorming/SKILL.md(0 hunks)skills/collaboration/executing-plans/SKILL.md(0 hunks)skills/collaboration/remembering-conversations/DEPLOYMENT.md(0 hunks)skills/collaboration/remembering-conversations/INDEXING.md(0 hunks)skills/collaboration/remembering-conversations/SKILL.md(0 hunks)skills/collaboration/remembering-conversations/tool/.gitignore(0 hunks)skills/collaboration/remembering-conversations/tool/hooks/sessionEnd(0 hunks)skills/collaboration/remembering-conversations/tool/index-conversations(0 hunks)skills/collaboration/remembering-conversations/tool/install-hook(0 hunks)skills/collaboration/remembering-conversations/tool/migrate-to-config.sh(0 hunks)skills/collaboration/remembering-conversations/tool/package.json(0 hunks)skills/collaboration/remembering-conversations/tool/prompts/search-agent.md(0 hunks)skills/collaboration/remembering-conversations/tool/search-conversations(0 hunks)skills/collaboration/remembering-conversations/tool/src/db.test.ts(0 hunks)skills/collaboration/remembering-conversations/tool/src/db.ts(0 hunks)skills/collaboration/remembering-conversations/tool/src/embeddings.ts(0 hunks)skills/collaboration/remembering-conversations/tool/src/index-cli.ts(0 hunks)skills/collaboration/remembering-conversations/tool/src/indexer.ts(0 hunks)skills/collaboration/remembering-conversations/tool/src/parser.ts(0 hunks)skills/collaboration/remembering-conversations/tool/src/paths.ts(0 hunks)skills/collaboration/remembering-conversations/tool/src/search-agent-template.test.ts(0 hunks)skills/collaboration/remembering-conversations/tool/src/search-cli.ts(0 hunks)skills/collaboration/remembering-conversations/tool/src/search.ts(0 hunks)skills/collaboration/remembering-conversations/tool/src/summarizer.ts(0 hunks)skills/collaboration/remembering-conversations/tool/src/types.ts(0 hunks)skills/collaboration/remembering-conversations/tool/src/verify.test.ts(0 hunks)skills/collaboration/remembering-conversations/tool/src/verify.ts(0 hunks)skills/collaboration/remembering-conversations/tool/test-deployment.sh(0 hunks)skills/collaboration/remembering-conversations/tool/test-install-hook.sh(0 hunks)skills/collaboration/remembering-conversations/tool/tsconfig.json(0 hunks)skills/collaboration/writing-plans/SKILL.md(0 hunks)skills/condition-based-waiting/SKILL.md(1 hunks)skills/defense-in-depth/SKILL.md(1 hunks)skills/dispatching-parallel-agents/SKILL.md(1 hunks)skills/executing-plans/SKILL.md(1 hunks)skills/finishing-a-development-branch/SKILL.md(4 hunks)skills/getting-started/SKILL.md(0 hunks)skills/meta/gardening-skills-wiki/SKILL.md(0 hunks)skills/meta/gardening-skills-wiki/analyze-search-gaps.sh(0 hunks)skills/meta/gardening-skills-wiki/check-index-coverage.sh(0 hunks)skills/meta/gardening-skills-wiki/check-links.sh(0 hunks)skills/meta/gardening-skills-wiki/check-naming.sh(0 hunks)skills/meta/gardening-skills-wiki/garden.sh(0 hunks)skills/meta/setting-up-personal-superpowers/SKILL.md(0 hunks)skills/meta/sharing-skills/SKILL.md(0 hunks)skills/receiving-code-review/SKILL.md(1 hunks)skills/record-plan/CHANGELOG.md(1 hunks)skills/record-plan/SKILL.md(1 hunks)skills/record-plan/scripts/check_lock.py(1 hunks)skills/record-plan/scripts/generate_acceptance.py(1 hunks)skills/record-plan/scripts/install-hook.sh(1 hunks)skills/record-plan/scripts/write_plan.py(1 hunks)skills/requesting-code-review/SKILL.md(4 hunks)skills/root-cause-tracing/SKILL.md(2 hunks)skills/root-cause-tracing/find-polluter.sh(1 hunks)skills/sharing-skills/SKILL.md(1 hunks)skills/subagent-driven-development/SKILL.md(4 hunks)skills/systematic-debugging/CREATION-LOG.md(1 hunks)skills/systematic-debugging/SKILL.md(4 hunks)skills/test-driven-development/SKILL.md(1 hunks)skills/testing-anti-patterns/SKILL.md(1 hunks)skills/testing-skills-with-subagents/SKILL.md(6 hunks)skills/testing-skills-with-subagents/examples/CLAUDE_MD_TESTING.md(2 hunks)skills/using-git-worktrees/SKILL.md(7 hunks)skills/using-superpowers/SKILL.md(1 hunks)skills/verification-before-completion/SKILL.md(1 hunks)skills/writing-skills/SKILL.md(14 hunks)skills/writing-skills/anthropic-best-practices.md(1 hunks)skills/writing-skills/automation-over-documentation.md(1 hunks)tests/opencode/run-tests.sh(1 hunks)tests/opencode/setup.sh(1 hunks)tests/opencode/test-plugin-loading.sh(1 hunks)tests/opencode/test-priority.sh(1 hunks)tests/opencode/test-skills-core.sh(1 hunks)tests/opencode/test-tools.sh(1 hunks)
💤 Files with no reviewable changes (45)
- skills/meta/gardening-skills-wiki/check-index-coverage.sh
- skills/meta/setting-up-personal-superpowers/SKILL.md
- skills/collaboration/remembering-conversations/SKILL.md
- skills/getting-started/SKILL.md
- skills/meta/gardening-skills-wiki/check-naming.sh
- skills/meta/sharing-skills/SKILL.md
- scripts/find-skills
- commands/README.md
- skills/collaboration/remembering-conversations/tool/src/types.ts
- skills/collaboration/executing-plans/SKILL.md
- skills/collaboration/writing-plans/SKILL.md
- skills/collaboration/remembering-conversations/tool/src/indexer.ts
- skills/collaboration/remembering-conversations/tool/src/summarizer.ts
- skills/collaboration/remembering-conversations/tool/src/index-cli.ts
- skills/collaboration/remembering-conversations/DEPLOYMENT.md
- skills/meta/gardening-skills-wiki/SKILL.md
- skills/collaboration/remembering-conversations/tool/src/paths.ts
- skills/meta/gardening-skills-wiki/garden.sh
- scripts/skill-run
- skills/collaboration/remembering-conversations/tool/tsconfig.json
- skills/collaboration/remembering-conversations/tool/src/search.ts
- skills/collaboration/remembering-conversations/tool/hooks/sessionEnd
- skills/collaboration/remembering-conversations/tool/install-hook
- skills/collaboration/remembering-conversations/tool/src/verify.test.ts
- skills/meta/gardening-skills-wiki/analyze-search-gaps.sh
- skills/collaboration/remembering-conversations/tool/src/verify.ts
- skills/collaboration/remembering-conversations/tool/.gitignore
- skills/collaboration/remembering-conversations/tool/src/search-cli.ts
- skills/REQUESTS.md
- skills/collaboration/brainstorming/SKILL.md
- skills/collaboration/remembering-conversations/tool/package.json
- skills/meta/gardening-skills-wiki/check-links.sh
- skills/collaboration/remembering-conversations/tool/test-install-hook.sh
- skills/collaboration/remembering-conversations/tool/src/db.ts
- skills/collaboration/remembering-conversations/tool/test-deployment.sh
- skills/collaboration/remembering-conversations/tool/index-conversations
- skills/collaboration/remembering-conversations/tool/src/db.test.ts
- skills/collaboration/remembering-conversations/INDEXING.md
- skills/collaboration/remembering-conversations/tool/search-conversations
- hooks/setup-personal-superpowers.sh
- skills/collaboration/remembering-conversations/tool/src/parser.ts
- skills/collaboration/remembering-conversations/tool/src/embeddings.ts
- skills/collaboration/remembering-conversations/tool/migrate-to-config.sh
- skills/collaboration/remembering-conversations/tool/prompts/search-agent.md
- skills/collaboration/remembering-conversations/tool/src/search-agent-template.test.ts
🧰 Additional context used
🧬 Code graph analysis (8)
tests/opencode/test-priority.sh (1)
tests/opencode/setup.sh (1)
cleanup_test_env(65-69)
skills/record-plan/scripts/generate_acceptance.py (1)
skills/record-plan/scripts/write_plan.py (1)
main(13-77)
tests/opencode/test-plugin-loading.sh (1)
tests/opencode/setup.sh (1)
cleanup_test_env(65-69)
.opencode/plugin/superpowers.js (1)
lib/skills-core.js (5)
content(18-18)actualSkillName(111-111)name(22-22)description(23-23)output(151-156)
hooks/session-start.sh (1)
lib/skills-core.js (1)
content(18-18)
tests/opencode/test-tools.sh (1)
tests/opencode/setup.sh (1)
cleanup_test_env(65-69)
tests/opencode/test-skills-core.sh (1)
tests/opencode/setup.sh (1)
cleanup_test_env(65-69)
skills/record-plan/scripts/write_plan.py (2)
skills/record-plan/scripts/generate_acceptance.py (1)
main(71-82)lib/skills-core.js (1)
description(23-23)
🪛 LanguageTool
skills/brainstorming/SKILL.md
[grammar] ~19-~19: Use a hyphen to join words.
Context: ...ime to refine the idea - Prefer multiple choice questions when possible, but open...
(QB_NEW_EN_HYPHEN)
CLAUDE.md
[grammar] ~202-~202: Use a hyphen to join words.
Context: ...- graphviz-conventions.dot - Flowchart style rules - persuasion-principles.md...
(QB_NEW_EN_HYPHEN)
README.md
[style] ~11-~11: The word ‘gonna’ is informal.
Context: ...s true red/green TDD, YAGNI (You Aren't Gonna Need It), and DRY. Next up, once you ...
(GONNA)
skills/testing-skills-with-subagents/SKILL.md
[grammar] ~330-~330: Ensure spelling is correct
Context: ...ed flags list - [ ] Updated description ith violation symptoms - [ ] Re-tested - ag...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
skills/writing-skills/anthropic-best-practices.md
[style] ~22-~22: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...Default assumption**: Claude is already very smart Only add context Claude doesn't alread...
(EN_WEAK_ADJECTIVE)
[style] ~140-~140: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...: Is the Skill clear and efficient? * Claude Opus (powerful reasoning): Does the S...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~802-~802: Try replacing commonly used words with synonyms to make your writing more engaging.
Context: ...encounter new scenarios. Each iteration improves the Skill based on real agent behavior,...
(IMPROVE_STRENGTHEN)
RELEASE-NOTES.md
[style] ~290-~290: The word ‘biggest’ tends to be overused. Consider an alternative.
Context: ...### Skills Repository Separation The biggest change: Skills no longer live in the plugin....
(BIG_DIFFERENCE)
[style] ~339-~339: This phrase is redundant. Consider writing “evolved”.
Context: ...ledge-lineages** - Understand how ideas evolved over time Architecture Skills (`skills/archi...
(EVOLVE_OVER_TIME)
🪛 markdownlint-cli2 (0.18.1)
skills/sharing-skills/SKILL.md
164-164: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
165-165: Bare URL used
(MD034, no-bare-urls)
168-168: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
170-170: Bare URL used
(MD034, no-bare-urls)
172-172: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
176-176: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
docs/windows/polyglot-hooks.md
54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
skills/record-plan/SKILL.md
182-182: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
187-187: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
194-194: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
199-199: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
205-205: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
343-343: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
413-413: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
skills/finishing-a-development-branch/SKILL.md
91-91: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
97-97: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
101-101: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
119-119: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
201-201: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
docs/README.opencode.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
213-213: Bare URL used
(MD034, no-bare-urls)
214-214: Bare URL used
(MD034, no-bare-urls)
215-215: Bare URL used
(MD034, no-bare-urls)
CLAUDE.md
43-43: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
75-75: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.opencode/INSTALL.md
39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
47-47: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
134-134: Bare URL used
(MD034, no-bare-urls)
135-135: Bare URL used
(MD034, no-bare-urls)
README.md
53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
64-64: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
74-74: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
102-102: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
113-113: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
docs/README.codex.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
37-37: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
47-47: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
147-147: Bare URL used
(MD034, no-bare-urls)
148-148: Bare URL used
(MD034, no-bare-urls)
149-149: Bare URL used
(MD034, no-bare-urls)
docs/plans/2025-11-22-opencode-support-design.md
200-200: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
skills/writing-skills/anthropic-best-practices.md
149-149: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
150-150: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
192-192: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
193-193: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
194-194: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
257-257: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
301-301: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1040-1040: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/plans/2025-11-22-opencode-support-implementation.md
21-21: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
83-83: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
88-88: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
103-103: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
154-154: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
165-165: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
170-170: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
185-185: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
234-234: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
244-244: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
249-249: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
264-264: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
306-306: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
317-317: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
322-322: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
338-338: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
346-346: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
351-351: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
365-365: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
369-369: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
375-375: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
380-380: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
394-394: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
398-398: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
402-402: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
407-407: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
421-421: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
425-425: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
429-429: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
434-434: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
450-450: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
454-454: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
485-485: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
490-490: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
504-504: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
575-575: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
580-580: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
594-594: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
641-641: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
646-646: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
660-660: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
744-744: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
749-749: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
765-765: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
805-805: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
815-815: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
823-823: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
881-881: Bare URL used
(MD034, no-bare-urls)
882-882: Bare URL used
(MD034, no-bare-urls)
883-883: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
904-904: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
922-922: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
927-927: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
941-941: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
966-966: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
971-971: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
987-987: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
992-992: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
997-997: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1002-1002: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1013-1013: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1024-1024: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1028-1028: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1035-1035: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1046-1046: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1051-1051: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1056-1056: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1065-1065: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
RELEASE-NOTES.md
83-83: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
111-111: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
126-126: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
148-148: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
164-164: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
173-173: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
182-182: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
197-197: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
205-205: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
213-213: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
220-220: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
229-229: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
247-247: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
322-322: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
346-346: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
354-354: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
359-359: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
369-369: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
374-374: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
380-380: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
387-387: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
486-486: Bare URL used
(MD034, no-bare-urls)
502-502: Bare URL used
(MD034, no-bare-urls)
503-503: Bare URL used
(MD034, no-bare-urls)
504-504: Bare URL used
(MD034, no-bare-urls)
🪛 Ruff (0.14.8)
skills/record-plan/scripts/generate_acceptance.py
19-19: Loop control variable idx not used within loop body
Rename unused idx to _idx
(B007)
skills/record-plan/scripts/write_plan.py
45-45: f-string without any placeholders
Remove extraneous f prefix
(F541)
51-51: f-string without any placeholders
Remove extraneous f prefix
(F541)
62-62: f-string without any placeholders
Remove extraneous f prefix
(F541)
63-63: f-string without any placeholders
Remove extraneous f prefix
(F541)
64-64: f-string without any placeholders
Remove extraneous f prefix
(F541)
65-65: f-string without any placeholders
Remove extraneous f prefix
(F541)
66-66: f-string without any placeholders
Remove extraneous f prefix
(F541)
67-67: f-string without any placeholders
Remove extraneous f prefix
(F541)
68-68: f-string without any placeholders
Remove extraneous f prefix
(F541)
71-71: f-string without any placeholders
Remove extraneous f prefix
(F541)
73-73: f-string without any placeholders
Remove extraneous f prefix
(F541)
74-74: f-string without any placeholders
Remove extraneous f prefix
(F541)
75-75: f-string without any placeholders
Remove extraneous f prefix
(F541)
skills/record-plan/scripts/check_lock.py
14-14: f-string without any placeholders
Remove extraneous f prefix
(F541)
16-16: f-string without any placeholders
Remove extraneous f prefix
(F541)
🪛 Shellcheck (0.11.0)
tests/opencode/setup.sh
[warning] 10-10: Declare and assign separately to avoid masking return values.
(SC2155)
|
|
||
| ## File Structure | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add language identifier to fenced code block.
The code block on line 200 should specify a language for proper syntax highlighting and markdown linting.
Apply this diff:
-```
+```text
superpowers/🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
200-200: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In docs/plans/2025-11-22-opencode-support-design.md around line 200, the fenced
code block is missing a language identifier; update the block to include a
language (e.g., "text") for proper syntax highlighting and linting by replacing
the opening triple backticks with ```text so the block reads as a text code
fence containing "superpowers/".
| - Report issues: https://github.com/obra/superpowers/issues | ||
| - Main documentation: https://github.com/obra/superpowers | ||
| - Blog post: https://blog.fsck.com/2025/10/27/skills-for-openai-codex/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Convert bare URLs to proper markdown links and verify blog post date.
The "Getting Help" section contains bare URLs that should be converted to proper markdown links. Additionally, the blog post URL references a future date (October 27, 2025) when the PR was created on December 16, 2025, which appears to be incorrect.
Apply this diff to convert URLs to proper links:
-- Report issues: https://github.com/obra/superpowers/issues
-- Main documentation: https://github.com/obra/superpowers
-- Blog post: https://blog.fsck.com/2025/10/27/skills-for-openai-codex/
+- Report issues: [GitHub Issues](https://github.com/obra/superpowers/issues)
+- Main documentation: [Superpowers Repository](https://github.com/obra/superpowers)
+- Blog post: [Skills for OpenAI Codex](https://blog.fsck.com/2025/10/27/skills-for-openai-codex/) *(verify date)*Please verify the blog post date is correct.
📝 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.
| - Report issues: https://github.com/obra/superpowers/issues | |
| - Main documentation: https://github.com/obra/superpowers | |
| - Blog post: https://blog.fsck.com/2025/10/27/skills-for-openai-codex/ | |
| - Report issues: [GitHub Issues](https://github.com/obra/superpowers/issues) | |
| - Main documentation: [Superpowers Repository](https://github.com/obra/superpowers) | |
| - Blog post: [Skills for OpenAI Codex](https://blog.fsck.com/2025/10/27/skills-for-openai-codex/) *(verify date)* |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
147-147: Bare URL used
(MD034, no-bare-urls)
148-148: Bare URL used
(MD034, no-bare-urls)
149-149: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In docs/README.codex.md around lines 147 to 149, the three bare URLs in the
"Getting Help" section should be converted into proper Markdown links (e.g.,
[Report issues](URL)), and the blog post URL's date (currently 2025-10-27) must
be verified and corrected to the actual publication date (or removed/annotated
if unknown); update the lines to use bracketed link text for each URL and fix
the blog post date to the verified value.
|
|
||
| ## File Structure | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add language specifier to fenced code block.
The file structure example on line 54 should have a language identifier for proper rendering.
Apply this diff:
-```
+```text
hooks/Based on static analysis hints.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In docs/windows/polyglot-hooks.md at line 54, the fenced code block lacks a
language specifier; update the opening fence to include "text" so it reads
```text before the block content (i.e., add the language identifier to the
fenced block containing "hooks/") to ensure correct rendering.
| /plugin marketplace add obra/superpowers-marketplace | ||
| /plugin install superpowers@superpowers-marketplace | ||
| ``` | ||
|
|
||
| That's it! On first session, the plugin automatically: | ||
| - Sets up `~/.config/superpowers/` as your personal skills repository | ||
| - Initializes git repo for version control | ||
| - Makes core skills searchable alongside your personal skills | ||
| - Adds `/brainstorm`, `/write-plan`, and `/execute-plan` commands | ||
| - Offers to create public GitHub repo for sharing your skills | ||
| Then install the plugin from this marketplace: | ||
|
|
||
| ```bash | ||
| /plugin install superpowers@superpowers-marketplace | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the plugin.json file
fd -t f "plugin.json" .Repository: obra/superpowers
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check the README.md file, specifically around lines 36-43
cat -n README.md | sed -n '30,50p'Repository: obra/superpowers
Length of output: 547
🏁 Script executed:
#!/bin/bash
# Get the full content of .claude-plugin/plugin.json if it exists
find . -path "*/.claude-plugin/plugin.json" -type f -exec cat -n {} \;Repository: obra/superpowers
Length of output: 733
Fix plugin name inconsistency between plugin.json and README.md installation command.
The plugin.json defines the plugin name as "superpowers-fork" (line 2), but the README.md installation instruction at line 42 tells users to install superpowers@superpowers-marketplace. The install command should match the name in plugin.json. Either update the README to /plugin install superpowers-fork@superpowers-marketplace or change the plugin.json name to "superpowers" to align with the marketplace entry.
🤖 Prompt for AI Agents
In README.md around lines 36 to 43, the install command references the wrong
plugin name (it instructs `/plugin install
superpowers@superpowers-marketplace`), which doesn't match plugin.json's
"superpowers-fork"; update the README to use the plugin name defined in
plugin.json by changing the install command to `/plugin install
superpowers-fork@superpowers-marketplace`, or alternatively change the "name"
field in plugin.json to "superpowers" so both files match—apply one of these two
consistent fixes and ensure the marketplace reference remains
`obra/superpowers-marketplace`.
| # Find all repos with llm/implementation-plans/ directory | ||
| installed_count=0 | ||
| for repo in "$REPOS_DIR"/*/; do | ||
| if [ -d "$repo/llm/implementation-plans" ] && [ -d "$repo/.git" ]; then | ||
| hook_path="$repo/.git/hooks/pre-commit" | ||
| echo "$HOOK_CONTENT" > "$hook_path" | ||
| chmod +x "$hook_path" | ||
| echo "✓ Installed hook: $hook_path" | ||
| installed_count=$((installed_count + 1)) | ||
| fi | ||
| done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Read the full install-hook.sh script to understand context
cat -n skills/record-plan/scripts/install-hook.shRepository: obra/superpowers
Length of output: 2608
🏁 Script executed:
# Check for any documentation or comments about hook handling in the file
grep -n "hook\|existing\|preserve\|merge\|overwrite" skills/record-plan/scripts/install-hook.sh -iRepository: obra/superpowers
Length of output: 546
🏁 Script executed:
# Check if there are other related files that might give context
ls -la skills/record-plan/scripts/Repository: obra/superpowers
Length of output: 434
Hook overwrite could silently remove existing pre-commit logic.
The script unconditionally overwrites .git/hooks/pre-commit (line 52) without checking if a hook already exists. While the targeted repos (those with llm/implementation-plans/) are likely specialized development repositories, this pattern could still cause silent data loss if repositories later accumulate additional pre-commit hooks for linting, formatting, or other workflows.
Recommend adding a check before overwriting:
- Warn users if a hook exists and show its content
- Optionally append this validation to existing hooks rather than replacing them
| **Required workflow skills:** | ||
| - **record-plans** - REQUIRED: Creates the plan that this skill executes | ||
| - **requesting-code-review** - REQUIRED: Review after each task (see Step 3) | ||
| - **finishing-a-development-branch** - REQUIRED: Complete development after all tasks (see Step 7) | ||
|
|
||
| **Alternative to:** | ||
| - skills/collaboration/executing-plans (parallel session) | ||
| **Subagents must use:** | ||
| - **test-driven-development** - Subagents follow TDD for each task | ||
|
|
||
| See code-reviewer template: skills/collaboration/requesting-code-review/code-reviewer.md | ||
| **Alternative workflow:** | ||
| - **executing-plans** - Use for parallel session instead of same-session execution | ||
|
|
||
| See code-reviewer template: requesting-code-review/code-reviewer.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, find and read the SKILL.md file at the specified location
cat -n skills/subagent-driven-development/SKILL.md | sed -n '175,195p'Repository: obra/superpowers
Length of output: 176
🏁 Script executed:
# Search for skill definitions in the repository
fd -type f -name "*.md" | head -20Repository: obra/superpowers
Length of output: 230
🏁 Script executed:
# Look for record-plan and record-plans references
rg "record-plan" --type md -iRepository: obra/superpowers
Length of output: 3032
🏁 Script executed:
# Search for the other required skills mentioned in the review
rg "requesting-code-review|finishing-a-development-branch|test-driven-development" --type md -lRepository: obra/superpowers
Length of output: 526
🏁 Script executed:
# Check what skill directories actually exist
fd "^SKILL.md$" --exec dirname {} \; | grep -E "(requesting-code-review|finishing-a-development|test-driven)" | head -20Repository: obra/superpowers
Length of output: 164
🏁 Script executed:
# Get more specific info about these skills
find skills -maxdepth 2 -name "SKILL.md" -exec grep -l "name:" {} \; | head -20Repository: obra/superpowers
Length of output: 809
🏁 Script executed:
# Look at actual skill names defined in their SKILL.md files
find skills -name "SKILL.md" -exec sh -c 'echo "=== {} ===" && head -5 {}' \; | grep -A 5 "name:"Repository: obra/superpowers
Length of output: 6744
🏁 Script executed:
# Search for how skills are invoked with superpowers: prefix
rg "superpowers:record-plan" --type mdRepository: obra/superpowers
Length of output: 159
🏁 Script executed:
# Check if there are any references with the correct singular form
rg "record-plan" --type md -B 1 -A 1 | head -30Repository: obra/superpowers
Length of output: 2068
🏁 Script executed:
# Look at the subagent-driven-development SKILL.md file to see the actual current state
cat -n skills/subagent-driven-development/SKILL.md | sed -n '175,195p'Repository: obra/superpowers
Length of output: 731
Naming inconsistency: "record-plans" should be "record-plan".
Line 179 references "record-plans" as a required workflow skill, but the actual skill is defined as "record-plan" (singular) in skills/record-plan/SKILL.md. This mismatch will cause skill lookup failures.
The other three required skills are correctly named:
requesting-code-review✓finishing-a-development-branch✓test-driven-development✓
Update line 179:
- **record-plans** - REQUIRED: Creates the plan that this skill executes
+ **record-plan** - REQUIRED: Creates the plan that this skill executes📝 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.
| **Required workflow skills:** | |
| - **record-plans** - REQUIRED: Creates the plan that this skill executes | |
| - **requesting-code-review** - REQUIRED: Review after each task (see Step 3) | |
| - **finishing-a-development-branch** - REQUIRED: Complete development after all tasks (see Step 7) | |
| **Alternative to:** | |
| - skills/collaboration/executing-plans (parallel session) | |
| **Subagents must use:** | |
| - **test-driven-development** - Subagents follow TDD for each task | |
| See code-reviewer template: skills/collaboration/requesting-code-review/code-reviewer.md | |
| **Alternative workflow:** | |
| - **executing-plans** - Use for parallel session instead of same-session execution | |
| See code-reviewer template: requesting-code-review/code-reviewer.md | |
| **Required workflow skills:** | |
| - **record-plan** - REQUIRED: Creates the plan that this skill executes | |
| - **requesting-code-review** - REQUIRED: Review after each task (see Step 3) | |
| - **finishing-a-development-branch** - REQUIRED: Complete development after all tasks (see Step 7) | |
| **Subagents must use:** | |
| - **test-driven-development** - Subagents follow TDD for each task | |
| **Alternative workflow:** | |
| - **executing-plans** - Use for parallel session instead of same-session execution | |
| See code-reviewer template: requesting-code-review/code-reviewer.md |
🤖 Prompt for AI Agents
In skills/subagent-driven-development/SKILL.md around lines 178 to 189, the
required workflow skill list uses "record-plans" (line 179) which doesn't match
the actual skill directory name "record-plan"; change the entry to "record-plan"
(singular) so the skill lookup matches the defined skill; ensure the rest of the
list remains unchanged and run a quick spell-check/lookup to confirm no other
naming mismatches.
| - [ ] Updated rationalization table | ||
| - [ ] Updated red flags list | ||
| - [ ] Updated when_to_use with violation symptoms | ||
| - [ ] Updated description ith violation symptoms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in checklist item.
The word "ith" should be "with". This typo is in a checklist item that users may copy, so it should be corrected.
Apply this diff:
-- [ ] Updated description ith violation symptoms
+- [ ] Updated description with violation symptoms📝 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.
| - [ ] Updated description ith violation symptoms | |
| - [ ] Updated description with violation symptoms |
🧰 Tools
🪛 LanguageTool
[grammar] ~330-~330: Ensure spelling is correct
Context: ...ed flags list - [ ] Updated description ith violation symptoms - [ ] Re-tested - ag...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In skills/testing-skills-with-subagents/SKILL.md around line 330, there's a typo
in the checklist item "- [ ] Updated description ith violation symptoms"; change
"ith" to "with" so the item reads "- [ ] Updated description with violation
symptoms". Ensure only the typo is corrected and spacing/punctuation remain
consistent with surrounding checklist formatting.
| 1. If relevant skill exists → Use the skill | ||
| 3. Announce you're using it | ||
| 4. Follow what it says |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix numbered list: missing item 2.
The list jumps from item 1 to item 3, skipping item 2. This appears to be a copy-paste error.
Apply this diff to fix the numbering:
**Starting any task:**
1. If relevant skill exists → Use the skill
-3. Announce you're using it
-4. Follow what it says
+2. Announce you're using it
+3. Follow what it says📝 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.
| 1. If relevant skill exists → Use the skill | |
| 3. Announce you're using it | |
| 4. Follow what it says | |
| 1. If relevant skill exists → Use the skill | |
| 2. Announce you're using it | |
| 3. Follow what it says |
🤖 Prompt for AI Agents
In skills/using-superpowers/SKILL.md around lines 96 to 98, the numbered list
skips item 2 (shows 1, then 3 and 4); update the numbering so it is sequential
by changing the current "3." to "2." and the current "4." to "3." (resulting in
a three-item list 1, 2, 3) to fix the missing item number.
| export TEST_HOME=$(mktemp -d) | ||
| export HOME="$TEST_HOME" | ||
| export XDG_CONFIG_HOME="$TEST_HOME/.config" | ||
| export OPENCODE_CONFIG_DIR="$TEST_HOME/.config/opencode" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address shellcheck warning: separate declaration and assignment.
The static analysis tool flags line 10 where TEST_HOME is declared and assigned in one line with command substitution. This can mask return values if mktemp fails.
Apply this diff to address the warning:
# Create temp home directory for isolation
-export TEST_HOME=$(mktemp -d)
+TEST_HOME=$(mktemp -d)
+export TEST_HOME
export HOME="$TEST_HOME"📝 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.
| export TEST_HOME=$(mktemp -d) | |
| export HOME="$TEST_HOME" | |
| export XDG_CONFIG_HOME="$TEST_HOME/.config" | |
| export OPENCODE_CONFIG_DIR="$TEST_HOME/.config/opencode" | |
| # Create temp home directory for isolation | |
| TEST_HOME=$(mktemp -d) | |
| export TEST_HOME | |
| export HOME="$TEST_HOME" | |
| export XDG_CONFIG_HOME="$TEST_HOME/.config" | |
| export OPENCODE_CONFIG_DIR="$TEST_HOME/.config/opencode" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 10-10: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
In tests/opencode/setup.sh around lines 10–13, split the combined
declaration+assignment of TEST_HOME into a separate declaration and an
assignment (e.g., declare TEST_HOME; TEST_HOME="$(mktemp -d)") and immediately
check the command exit status (if mktemp fails, print an error and exit
non‑zero) before exporting HOME, XDG_CONFIG_HOME and OPENCODE_CONFIG_DIR so
failures are not masked.
- Tested with RED-GREEN-REFACTOR using 4 pressure scenarios - Scenarios validate mechanical vs judgment decision-making - All tests pass: agents correctly identify automation opportunities - Word count optimized to 424 words (target: ≤500) - Integration points added to SKILL.md line 500 Test results: 4/4 scenarios pass (GREEN phase: first try) Test methodology: Baseline without doc showed documentation failures, tested version correctly guided agents to mechanical enforcement
- Use emoji library for production-ready approach - Include commented regex alternative for zero dependencies - Addresses CodeRabbit feedback: code examples must be runnable - Word count: 442 words (still under 500 target)
Add brief example showing automation-over-documentation applies to silent dependency failures, not just rationalization. Based on production incident where file-track registration was gated by Python module import, causing silent failures in subagent environments. Pattern: Remove dependency gates that hide failures - CLI registration should be independent of Python module availability.
Updates all skill and documentation references from the deprecated writing-plans to the new record-plans skill name following the rename in commit 8853d53. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Following TDD methodology for skills: - RED: Baseline tests showed agents skip ws consultation when modifying skills - GREEN: Enhanced triggers and added explicit enforcement blocks - REFACTOR: Extended rationalization tables with skill-modification scenarios Changes: - writing-skills: Enhanced description with explicit triggers (modifying, debugging, adding sections) - writing-skills: Added EXTREMELY-IMPORTANT block requiring consultation before any skill file edits - writing-skills: Extended rationalization table with 3 new skill-modification entries - using-superpowers: Added specific rationalization "Just editing a skill file" - CLAUDE.md: Added local plugin development workflow Testing revealed systemic limitation: subagents don't auto-invoke MANDATORY FIRST RESPONSE PROTOCOL. Content improvements help when agents do consult the skill. Co-Authored-By: Claude <noreply@anthropic.com>
…ecution Changes: - Execute all tasks continuously instead of stopping every 3 tasks - Only stop for actual blockers or ambiguous instructions - Add "Common Rationalizations" section to prevent early stopping - Update description, overview, and all process steps for consistency - Remove "batch" terminology throughout Testing: - Baseline: confirmed agent stopped after 3/6 tasks - Green: verified agent executes all 6/6 tasks continuously - Pressure: verified agent executes 10/10 tasks through milestone Addresses user feedback that checkpoint pausing interrupts flow when there are no blockers requiring input.
Wrapper script lives in user's .claude/scripts directory, not in skill directory. Changes: - write_plan.py path: ~/.claude/scripts/record-plan/ - check_lock.py path: ~/.claude/scripts/record-plan/ This separates skill documentation (in plugin) from user scripts (in ~/.claude).
CLAUDE.md changes: - Add blank lines for better markdown list formatting - Simplify plugin reload instructions Scripts added: - scripts/reload-plugin.sh: Helper to display plugin reload commands
- Add 4-step process: push → generate → preview → confirm - Match Option 4's typed confirmation pattern - Update Common Mistakes and Red Flags sections - Closes safety gap identified in conversation c989437e
- Add Rationalization Table with 5 common excuses - Enhance Red Flags with specific thought patterns - Explicitly counter permission system reliance
- Add PR Creation Safety section - Document 3-layer defense-in-depth approach - Cross-reference jira-publisher and Option 4 patterns
Change plugin name to superpowers-fork and update metadata to reflect this is a fork with record-plan and automation-over-documentation additions. Update homepage/repository URLs to point to fork. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
♻️ Duplicate comments (5)
skills/record-plan/CHANGELOG.md (1)
10-11: Remove changelog references to non-existent scripts.Lines 10-11 reference
scripts/track_with_filetrack.shandscripts/tests/test_track_integration.sh, but these files don't exist in the repository. The actual scripts present are:check_lock.py,generate_acceptance.py,install-hook.sh, andwrite_plan.py. Either add the missing files or update the changelog to reflect only the files included in this PR.skills/record-plan/scripts/install-hook.sh (1)
23-31: Hook installation overwrites existing pre-commit logic.The script unconditionally overwrites
.git/hooks/pre-commitwithout checking for existing hooks. This could silently remove other pre-commit validations (linting, formatting, etc.).Based on past review feedback, recommend adding a check before overwriting or supporting hook composition (e.g., appending to existing hooks).
skills/subagent-driven-development/SKILL.md (1)
179-179: Naming inconsistency: "record-plans" should be "record-plan".Line 179 uses "record-plans" (plural) but the actual skill name is "record-plan" (singular) as defined in
skills/record-plan/SKILL.md. This mismatch will cause skill lookup failures when the workflow tries to reference the required skill.Apply this diff:
-- **record-plans** - REQUIRED: Creates the plan that this skill executes +- **record-plan** - REQUIRED: Creates the plan that this skill executesskills/record-plan/scripts/generate_acceptance.py (1)
19-19: Remove unused loop variable.The loop control variable
idxis not used within the loop body. This issue has already been flagged in previous reviews.Apply this diff:
- for idx, line in enumerate(plan_content.split('\n')): + for line in plan_content.split('\n'): match = re.match(task_pattern, line)Since line numbers are not needed, simplify by removing
enumerateentirely.skills/finishing-a-development-branch/SKILL.md (1)
101-118: Strong approval gate implementation with code block issue.The preview-confirmation step is well-designed:
- Shows actual title and body content
- Requires explicit "yes" confirmation
- Handles cancellation gracefully
- Prevents accidental PR creation
However, the code block at line 104 should specify a language identifier.
Apply this diff to add language identifier:
**Step 3: Show preview and request confirmation** Present: -``` +```text Ready to create PR with: Title: <title>
🧹 Nitpick comments (5)
CLAUDE.md (1)
41-44: Add language specifiers to fenced code blocks.Code blocks at lines 41-44 and 73-80 are missing language specifiers. Adding them improves syntax highlighting and clarity.
Apply these diffs:
- ``` + ```bash /plugin uninstall superpowers@superpowers-dev /plugin install superpowers@superpowers-dev ```-``` +```text skills/ skill-name/ # lowercase-with-hyphens only (no special chars)Also applies to: 73-80
skills/record-plan/scripts/check_lock.py (1)
14-16: Remove unnecessary f-string prefixes.Lines 14 and 16 use f-string syntax but contain no placeholders. Remove the
fprefix for clarity.Apply this diff:
- print(f" python3 ~/.claude/skills/record-plan/scripts/write_plan.py \\") - print(f" --working-dir {working_dir} \\") - print(f" --plan-name <descriptive-name>") + print(" python3 ~/.claude/skills/record-plan/scripts/write_plan.py \\") + print(f" --working-dir {working_dir} \\") + print(" --plan-name <descriptive-name>")skills/record-plan/SKILL.md (1)
205-205: Add language specifiers to fenced code blocks.Fenced code blocks at lines 205, 343, and 413 are missing language specifiers. Adding
bash,text, or appropriate identifiers improves rendering and clarity.Also applies to: 343-343, 413-413
skills/record-plan/scripts/write_plan.py (2)
40-53: Consider adding directory creation.The path determination logic is correct, but the script doesn't ensure that the target directories exist. If
~/.claude/plans(plan mode) ortarget_dir(regular mode) don't exist, downstream Write operations might fail.Consider adding directory creation:
# Determine target path if args.in_plan_mode: staging_path = os.path.expanduser("~/.claude/plans") + os.makedirs(staging_path, exist_ok=True) target_file = f"{staging_path}/{args.plan_name}.md" final_dir = os.path.join(args.working_dir, args.target_dir) print(f"📝 PLAN MODE DETECTED") print(f"✓ Write plan to: {target_file}") print(f"✓ Then copy to: {final_dir}/{args.plan_name}.md") else: target_dir = os.path.join(args.working_dir, args.target_dir) + os.makedirs(target_dir, exist_ok=True) target_file = f"{target_dir}/{args.plan_name}.md" print(f"📝 REGULAR MODE") print(f"✓ Write plan to: {target_file}")
62-76: Remove unnecessary f-string prefixes.The guidance output is comprehensive and clear. However, multiple f-strings lack placeholders and should be converted to regular strings for consistency.
Apply this diff to address the Ruff warnings:
- print(f"\n🔧 REQUIRED ACTIONS:") - print(f"1. Use Write tool to create file with:") - print(f" - First line: <!-- jot:md-rename -->") - print(f" - YAML frontmatter (title, date, type, status)") - print(f" - H1 heading with feature name") - print(f" - Implementation tasks following record-plan format") - print(f"\n2. After writing, run post-write workflow:") + print("\n🔧 REQUIRED ACTIONS:") + print("1. Use Write tool to create file with:") + print(" - First line: <!-- jot:md-rename -->") + print(" - YAML frontmatter (title, date, type, status)") + print(" - H1 heading with feature name") + print(" - Implementation tasks following record-plan format") + print("\n2. After writing, run post-write workflow:") print(f" - Validate: python3 ~/.claude/scripts/record-tools/validate-frontmatter.py {target_file}") print(f" - Rename: python3 ~/.claude/scripts/record-tools/rename_jot.py {target_file} (auto-tracks with file-track)") - print(f" ⚠️ IMPORTANT: Rename script will remove lock file") + print(" ⚠️ IMPORTANT: Rename script will remove lock file") - print(f"\n⚠️ CRITICAL: DO NOT just describe the plan!") - print(f"⚠️ You MUST use Write tool to create the actual file!") - print(f"⚠️ Describing = BUG. Writing = CORRECT.") + print("\n⚠️ CRITICAL: DO NOT just describe the plan!") + print("⚠️ You MUST use Write tool to create the actual file!") + print("⚠️ Describing = BUG. Writing = CORRECT.")Similar changes should be applied to lines 45 and 51:
- print(f"📝 PLAN MODE DETECTED") + print("📝 PLAN MODE DETECTED") print(f"✓ Write plan to: {target_file}") print(f"✓ Then copy to: {final_dir}/{args.plan_name}.md") else: target_dir = os.path.join(args.working_dir, args.target_dir) target_file = f"{target_dir}/{args.plan_name}.md" - print(f"📝 REGULAR MODE") + print("📝 REGULAR MODE")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.claude-plugin/plugin.json(1 hunks)CLAUDE.md(1 hunks)README.md(2 hunks)commands/write-plan.md(1 hunks)scripts/reload-plugin.sh(1 hunks)skills/brainstorming/SKILL.md(1 hunks)skills/executing-plans/SKILL.md(3 hunks)skills/finishing-a-development-branch/SKILL.md(3 hunks)skills/record-plan/CHANGELOG.md(1 hunks)skills/record-plan/SKILL.md(1 hunks)skills/record-plan/scripts/check_lock.py(1 hunks)skills/record-plan/scripts/generate_acceptance.py(1 hunks)skills/record-plan/scripts/install-hook.sh(1 hunks)skills/record-plan/scripts/write_plan.py(1 hunks)skills/subagent-driven-development/SKILL.md(1 hunks)skills/using-superpowers/SKILL.md(1 hunks)skills/writing-skills/SKILL.md(4 hunks)skills/writing-skills/automation-over-documentation.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- skills/brainstorming/SKILL.md
- README.md
- commands/write-plan.md
- skills/using-superpowers/SKILL.md
🧰 Additional context used
🧬 Code graph analysis (1)
skills/record-plan/scripts/write_plan.py (1)
skills/record-plan/scripts/generate_acceptance.py (1)
main(71-82)
🪛 LanguageTool
CLAUDE.md
[grammar] ~200-~200: Use a hyphen to join words.
Context: ...- graphviz-conventions.dot - Flowchart style rules - persuasion-principles.md...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
73-73: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
skills/record-plan/SKILL.md
182-182: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
187-187: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
194-194: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
199-199: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
205-205: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
343-343: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
413-413: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.8)
skills/record-plan/scripts/write_plan.py
45-45: f-string without any placeholders
Remove extraneous f prefix
(F541)
51-51: f-string without any placeholders
Remove extraneous f prefix
(F541)
62-62: f-string without any placeholders
Remove extraneous f prefix
(F541)
63-63: f-string without any placeholders
Remove extraneous f prefix
(F541)
64-64: f-string without any placeholders
Remove extraneous f prefix
(F541)
65-65: f-string without any placeholders
Remove extraneous f prefix
(F541)
66-66: f-string without any placeholders
Remove extraneous f prefix
(F541)
67-67: f-string without any placeholders
Remove extraneous f prefix
(F541)
68-68: f-string without any placeholders
Remove extraneous f prefix
(F541)
71-71: f-string without any placeholders
Remove extraneous f prefix
(F541)
73-73: f-string without any placeholders
Remove extraneous f prefix
(F541)
74-74: f-string without any placeholders
Remove extraneous f prefix
(F541)
75-75: f-string without any placeholders
Remove extraneous f prefix
(F541)
skills/record-plan/scripts/generate_acceptance.py
19-19: Loop control variable idx not used within loop body
Rename unused idx to _idx
(B007)
skills/record-plan/scripts/check_lock.py
14-14: f-string without any placeholders
Remove extraneous f prefix
(F541)
16-16: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (25)
skills/writing-skills/automation-over-documentation.md (3)
36-48: Clear, actionable decision framework.The categorization (objective/detectable constraints → automate; judgment calls → document) is crisp and well-supported by examples. Framework generalizes well beyond the emoji case.
49-87: Red flags and ROI messaging are effective; cost-benefit argument is compelling.The red flags (lines 49-57) are practical indicators that you're using the wrong tool. The cost-benefit table (lines 75-81) is data-driven and persuasive. The bottom line—use TDD to decide which tool—is a sound, measurable heuristic.
The psychology insight references "persuasion-principles.md," which should be verified to exist and provide context for the Cialdini reference.
1-34: Strong foundation for mechanical enforcement philosophy; all external citations verified.The problem-solution framing is compelling: documentation approach failing (0/5 tests), automation succeeding (5/5 tests) is persuasive evidence. Both code solutions (emoji library and regex) are correct and well-commented. The concrete case study grounds the philosophy effectively.
The decision framework (lines 36–48) cleanly separates mechanical enforcement from judgment guidance. Cost-benefit table demonstrates clear ROI: 15 lines of code vs. multiple failed TDD cycles.
External citations are accurate: persuasion-principles.md contains full Cialdini (2021) reference and Meincke et al. (2025) citation with complete publication details, supporting the psychology insight on line 61.
skills/writing-skills/SKILL.md (4)
3-3: Strengthened enforcement messaging; REQUIRED marker and EXTREMELY-IMPORTANT section effectively close skill-editing loopholes.Line 3 adds crucial clarifications: scope now explicitly includes "modifying existing skills," "improving skill documentation," and includes the "REQUIRED before making ANY changes" marker. This closes a significant loophole where developers might skip this skill when editing existing documentation.
The new EXTREMELY-IMPORTANT section (lines 22-35) uses strong formatting and repetition ("STOP. Read this skill. Follow TDD process.") to interrupt automatic rationalizations. The comparison to "deploying untested code" is apt and reinforces consistency with development discipline.
Also applies to: 22-35
438-440: Rationalization table additions directly address documentation-editing loopholes.The three new rows (438-440) target high-risk rationalizations that would otherwise allow untested edits to documentation: "just adding a section," "just documentation," and "quick fix." The framing ("Skills ARE code") is consistent with the Iron Law and EXTREMELY-IMPORTANT section.
518-518: Cross-reference to automation-over-documentation.md is well-positioned; verify file path.The deep-dive reference links the philosophical framework (automate vs document) to the TDD-for-skills practice. Placement after red flags and before RED-GREEN-REFACTOR is logical. Verify that automation-over-documentation.md is discoverable at the relative path (both files are in skills/writing-skills/).
20-20: All external file references exist in the repository. The filesanthropic-best-practices.md,graphviz-conventions.dot, andpersuasion-principles.mdare present inskills/writing-skills/and the references are valid.Likely an incorrect or invalid review comment.
scripts/reload-plugin.sh (1)
1-20: LGTM - Clean helper script.The reload script is well-structured with clear instructions and appropriate commands for the plugin development workflow. The hardcoded plugin name
superpowers@superpowers-devaligns with the development workflow described in CLAUDE.md.CLAUDE.md (1)
1-205: Excellent comprehensive documentation.The CLAUDE.md file provides thorough guidance on project architecture, development workflow, TDD methodology, skill structure requirements, and contributing workflow. The content is well-organized and actionable.
.claude-plugin/plugin.json (1)
2-12: Fork metadata properly updated.The plugin metadata correctly reflects the fork with updated name, description, author, repository URLs, and keywords. The attribution to the original author (Jesse Vincent) is appropriately maintained in both the description and author name fields.
skills/record-plan/SKILL.md (2)
1-449: Comprehensive skill documentation with strong enforcement patterns.The record-plan skill documentation is thorough and well-structured, with:
- Clear scope boundaries and red flags section
- Detailed workflow steps with examples
- Mechanical enforcement approach with lock files
- Post-write validation workflow
- Production incident learning incorporated
The TDD-for-documentation approach and pressure testing validation demonstrate a mature skill development process.
19-21: Review premise incorrect: hardcoded paths follow established designThe codebase explicitly uses hardcoded
~/.claude/paths in documentation and bash commands. Commit b694992 documents this design decision: "CLAUDE_PLUGIN_ROOT is a template variable for documentation, not a bash environment variable." The record-plan skill uses hardcoded paths consistent with other skills in the codebase (e.g., testing-skills-with-subagents examples). There is no PR objective to replace these paths with environment variables; this contradicts the documented design pattern established in commit b694992.Likely an incorrect or invalid review comment.
skills/executing-plans/SKILL.md (4)
1-15: LGTM! Clear guidance on continuous execution model.The updated description and core principle effectively communicate the shift from batch-based to continuous execution. The guidance clearly specifies when to stop (blockers or ambiguity only).
24-48: Excellent anti-pattern guidance.The addition of "Common Rationalizations for Stopping" is particularly valuable. It proactively addresses likely misinterpretations of the continuous execution model by explicitly listing invalid reasons to pause. This should significantly reduce premature stops.
50-58: LGTM! Clear completion workflow.The completion step properly delegates to the finishing-a-development-branch skill and requires showing both implementation and verification output before transitioning.
59-83: LGTM! Comprehensive stopping and revisiting guidance.The distinction between stopping for immediate blockers versus revisiting earlier steps for fundamental changes is clear and actionable. The Remember section effectively reinforces the continuous execution model.
skills/record-plan/scripts/write_plan.py (3)
1-38: LGTM! Clean script structure and argument parsing.The imports are minimal and appropriate. The argument parser is well-structured with clear help text for each parameter. The docstring effectively explains the script's purpose.
54-61: LGTM! Simple and effective lock file mechanism.The lock file creation is straightforward and serves its purpose well. Writing the target path and timestamp provides useful context for debugging.
77-80: LGTM! Correct exit handling.The script correctly returns 0 and uses the standard Python entry point pattern.
skills/record-plan/scripts/generate_acceptance.py (2)
71-82: LGTM! Clean CLI with proper error handling.The main function properly validates input (plan file existence) and provides clear error messages. The CLI interface is simple and appropriate for this utility script.
84-85: LGTM! Standard entry point.skills/finishing-a-development-branch/SKILL.md (4)
1-16: LGTM! Clear emphasis on approval gates.The updated description and overview effectively communicate the addition of explicit approval gates for destructive operations, setting proper expectations for the workflow.
205-216: Excellent rationalization table.This table is a valuable addition that proactively addresses likely attempts to bypass the approval gate. By explicitly listing common excuses and explaining why they're insufficient, it significantly strengthens the enforcement of the approval workflow.
217-225: LGTM! Effective "pre-commit" mental checklist.The red flags section complements the rationalization table by helping catch approval-skipping attempts in real-time. The direct, imperative style ("STOP. Show preview.") is appropriate for preventing mistakes before they occur.
227-241: LGTM! Consistent integration with existing guidance.The additions to the Never/Always lists properly integrate the new PR approval requirements with the existing workflow guidance, maintaining consistency across the documentation.
|
|
||
| When developing superpowers locally and testing changes in Claude Code: | ||
|
|
||
| 1. **Edit skills** in `/Users/ethan.stark/dev/claude-code-resources/superpowers/skills/` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded absolute path in documentation example.
Line 38 contains a user-specific absolute path (/Users/ethan.stark/dev/claude-code-resources/superpowers/skills/) in the local plugin development section. While this is documentation rather than executable code, consider using a placeholder like /path/to/superpowers/skills/ or ${HOME}/dev/superpowers/skills/ to make the example more universally applicable.
Apply this diff to generalize the path:
-1. **Edit skills** in `/Users/ethan.stark/dev/claude-code-resources/superpowers/skills/`
+1. **Edit skills** in your local superpowers directory (e.g., `~/dev/superpowers/skills/`)📝 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.
| 1. **Edit skills** in `/Users/ethan.stark/dev/claude-code-resources/superpowers/skills/` | |
| 1. **Edit skills** in your local superpowers directory (e.g., `~/dev/superpowers/skills/`) |
🤖 Prompt for AI Agents
In CLAUDE.md around line 38, the documentation contains a hardcoded
user-specific absolute path
(/Users/ethan.stark/dev/claude-code-resources/superpowers/skills/) which is not
portable; replace it with a generic placeholder such as
/path/to/superpowers/skills/ or ${HOME}/dev/superpowers/skills/ (or similar) so
the example works for all users and update the line accordingly.
| # Check if lock authorizes this specific file | ||
| with open(lock_file) as f: | ||
| authorized_path = f.readline().strip() | ||
|
|
||
| if not file_path.endswith(os.path.basename(authorized_path)): | ||
| print(f"❌ ERROR: Lock file authorizes {authorized_path}, not {file_path}") | ||
| return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basename-only check is insufficient for path authorization.
Line 23 only validates that the target file path ends with the basename of the authorized path. This could allow unauthorized writes if multiple files share the same basename in different directories.
Example scenario:
- Lock authorizes:
/project/llm/implementation-plans/plan.md - Attack: write to
/project/other/subdir/plan.md✓ would pass (incorrect)
The check should compare full absolute paths or at least verify the complete relative path from the repository root.
Apply this diff to validate full paths:
- if not file_path.endswith(os.path.basename(authorized_path)):
+ authorized_abs = os.path.abspath(authorized_path)
+ file_abs = os.path.abspath(file_path)
+
+ if authorized_abs != file_abs:
print(f"❌ ERROR: Lock file authorizes {authorized_path}, not {file_path}")
return False📝 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.
| # Check if lock authorizes this specific file | |
| with open(lock_file) as f: | |
| authorized_path = f.readline().strip() | |
| if not file_path.endswith(os.path.basename(authorized_path)): | |
| print(f"❌ ERROR: Lock file authorizes {authorized_path}, not {file_path}") | |
| return False | |
| # Check if lock authorizes this specific file | |
| with open(lock_file) as f: | |
| authorized_path = f.readline().strip() | |
| authorized_abs = os.path.abspath(authorized_path) | |
| file_abs = os.path.abspath(file_path) | |
| if authorized_abs != file_abs: | |
| print(f"❌ ERROR: Lock file authorizes {authorized_path}, not {file_path}") | |
| return False |
🤖 Prompt for AI Agents
In skills/record-plan/scripts/check_lock.py around lines 19 to 25, the code
currently compares only basenames which allows mismatched paths with identical
filenames; change the check to resolve and compare full paths (e.g., absolute
paths or repository-root-relative canonical paths) for both the authorized_path
read from the lock file and the provided file_path, normalizing (os.path.abspath
and os.path.normpath) and then comparing the full strings (or using
os.path.samefile when appropriate) so that only the exact authorized file is
accepted; update the error message to print both resolved paths for clarity.
| def generate_acceptance(plan_file: Path, output_file: Path): | ||
| """Generate acceptance.json from plan file.""" | ||
| plan_content = plan_file.read_text() | ||
| features = extract_tasks(plan_content) | ||
|
|
||
| # Warn if no tasks were found | ||
| if not features: | ||
| print(f"WARNING: No tasks found in {plan_file.name}") | ||
| print(" Expected pattern: ### Task N: Description") | ||
| print(" Generating empty acceptance.json") | ||
|
|
||
| acceptance = { | ||
| "plan": str(plan_file.relative_to(Path.cwd())), | ||
| "generated": date.today().isoformat(), | ||
| "total_features": len(features), | ||
| "passing_features": 0, | ||
| "features": features, | ||
| "_rules": { | ||
| "immutable_fields": ["id", "category", "description", "steps"], | ||
| "mutable_fields": ["passes", "notes"], | ||
| "catastrophic_actions": [ | ||
| "remove features", | ||
| "edit descriptions", | ||
| "modify steps" | ||
| ] | ||
| } | ||
| } | ||
|
|
||
| output_file.write_text(json.dumps(acceptance, indent=2) + '\n') | ||
| print(f"✓ Generated {len(features)} acceptance criteria") | ||
| print(f" Output: {output_file}") | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for invocations and references to generate_acceptance
find . -type f \( -name "*.py" -o -name "*.sh" -o -name "*.md" -o -name "*.yml" -o -name "*.yaml" \) | head -50
rg -n "generate_acceptance" --type-listRepository: obra/superpowers
Length of output: 2429
🏁 Script executed:
# More targeted search for generate_acceptance invocations
rg -n "generate_acceptance" -A 3 -B 3Repository: obra/superpowers
Length of output: 1711
🏁 Script executed:
# Check if there's a main block or setup.py that might invoke this
rg -n "if __name__" skills/record-plan/scripts/ -A 5Repository: obra/superpowers
Length of output: 784
🏁 Script executed:
# Look for documentation about how to run this script
find skills/record-plan -name "*.md" -o -name "README*" | xargs cat 2>/dev/nullRepository: obra/superpowers
Length of output: 19196
Fix error handling for relative path computation.
Line 51 uses Path.cwd() to compute the relative path, which will raise a ValueError if the script is invoked from a directory that is not the repository root or an ancestor of the plan file path. Either wrap this in error handling with a fallback (e.g., return absolute path), or document and enforce that the script must be invoked from the repository root.
🤖 Prompt for AI Agents
In skills/record-plan/scripts/generate_acceptance.py around lines 39-70,
computing the plan path with plan_file.relative_to(Path.cwd()) can raise
ValueError if the current working directory is not a parent of plan_file; update
the code to handle that by wrapping the relative_to call in a try/except: try to
use plan_file.relative_to(Path.cwd()) and on ValueError fall back to
plan_file.resolve() (or plan_file.absolute()) converted to string, so
acceptance["plan"] is always a valid path string; ensure the rest of the code
uses that computed string.
| echo "❌ ERROR: Implementation plan does not follow YYMMDD-XX format" | ||
| echo "" | ||
| echo "Files must be created via wrapper script:" | ||
| echo " python3 ~/.claude/skills/record-plan/scripts/write_plan.py \\" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, read the install-hook.sh file to see context around line 11
cat -n skills/record-plan/scripts/install-hook.shRepository: obra/superpowers
Length of output: 1312
🏁 Script executed:
#!/bin/bash
# Search for CLAUDE_PLUGIN_ROOT or similar plugin root variables
rg -n "CLAUDE_PLUGIN_ROOT|PLUGIN_ROOT|PLUGIN_DIR|PLUGIN_PATH|\\\$\{.*PLUGIN.*\}" --type sh --type bash -iRepository: obra/superpowers
Length of output: 88
🏁 Script executed:
#!/bin/bash
# Search for how other hook/installation scripts handle paths
fd "install" -e sh -e bash | head -20 | xargs -I {} sh -c 'echo "=== {} ===" && head -30 {}'Repository: obra/superpowers
Length of output: 1141
🏁 Script executed:
#!/bin/bash
# Search for CLAUDE_PLUGIN_ROOT or similar variables with correct type
rg -n "CLAUDE_PLUGIN_ROOT|PLUGIN_ROOT|PLUGIN_DIR|PLUGIN_PATH" --type sh -iRepository: obra/superpowers
Length of output: 293
🏁 Script executed:
#!/bin/bash
# Search for write_plan.py references to understand usage context
rg -n "write_plan\.py"Repository: obra/superpowers
Length of output: 698
🏁 Script executed:
#!/bin/bash
# Search for environment variable patterns in shell scripts
rg -n "\\\${.*}" --type sh | head -30Repository: obra/superpowers
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Check if there's a setup or config script that defines plugin paths
fd -e sh -e bash | xargs grep -l "CLAUDE\|plugin\|skill" | head -10Repository: obra/superpowers
Length of output: 336
🏁 Script executed:
#!/bin/bash
# Check actual directory structure
find . -name "write_plan.py" -type fRepository: obra/superpowers
Length of output: 102
🏁 Script executed:
#!/bin/bash
# Look at the SKILL.md file to see the documented path
cat -n skills/record-plan/SKILL.md | head -50Repository: obra/superpowers
Length of output: 3090
🏁 Script executed:
#!/bin/bash
# Check check_lock.py to see more context
cat -n skills/record-plan/scripts/check_lock.py | head -30Repository: obra/superpowers
Length of output: 1301
🏁 Script executed:
#!/bin/bash
# Look at session-start.sh to understand the PLUGIN_ROOT pattern better
cat -n hooks/session-start.shRepository: obra/superpowers
Length of output: 2414
Hardcoded path in hook breaks when plugin is installed outside ~/.claude/skills.
Line 11 hardcodes ~/.claude/skills/record-plan/scripts/write_plan.py into HOOK_CONTENT, which gets written to git pre-commit hooks in multiple repositories. This will fail if the plugin is installed in a different location or uses a different directory structure.
Additionally, SKILL.md documents the path as ~/.claude/scripts/record-plan/write_plan.py (without "skills"), which conflicts with the hardcoded path in the hook—this inconsistency compounds the portability issue.
The codebase already uses dynamic path resolution via PLUGIN_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)" in hooks/session-start.sh. Apply the same pattern here: either compute the plugin root at hook install time and embed it, or make write_plan.py discoverable via environment variable (e.g., via ~/.bashrc or a wrapper in PATH).
| ' | ||
|
|
||
| # Find all repos with llm/implementation-plans/ directory | ||
| for repo in /Users/ethan.stark/dev/*/; do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Hardcoded user-specific path prevents portability.
Line 24 hardcodes /Users/ethan.stark/dev/*/ which makes this script unusable for any other user. This is a production-tested improvement intended for broader use, but the implementation is tied to a single developer's environment.
Solutions:
- Accept path parameter (recommended):
#!/bin/bash
# Install pre-commit hook for record-plan enforcement
+# Usage: install-hook.sh [base-directory]
+# Default: ~/dev
+BASE_DIR="${1:-$HOME/dev}"
+
HOOK_CONTENT='...'
# Find all repos with llm/implementation-plans/ directory
-for repo in /Users/ethan.stark/dev/*/; do
+for repo in "$BASE_DIR"/*/; do- Use environment variable:
+# Set SUPERPOWERS_DEV_DIR or default to ~/dev
+BASE_DIR="${SUPERPOWERS_DEV_DIR:-$HOME/dev}"
+
-for repo in /Users/ethan.stark/dev/*/; do
+for repo in "$BASE_DIR"/*/; do📝 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.
| for repo in /Users/ethan.stark/dev/*/; do | |
| #!/bin/bash | |
| # Install pre-commit hook for record-plan enforcement | |
| # Usage: install-hook.sh [base-directory] | |
| # Default: ~/dev | |
| BASE_DIR="${1:-$HOME/dev}" | |
| HOOK_CONTENT='...' | |
| # Find all repos with llm/implementation-plans/ directory | |
| for repo in "$BASE_DIR"/*/; do |
🤖 Prompt for AI Agents
In skills/record-plan/scripts/install-hook.sh around line 24, the for-loop
currently hardcodes a user-specific path (/Users/ethan.stark/dev/*/), breaking
portability; change the script to accept a repository path as a parameter
(preferred) or read from an environment variable (REPOS_DIR) and fall back to a
sensible default such as "$HOME/dev/*/"; validate the resolved path exists and
contains directories before iterating and fail with a clear error if not
provided or missing; update usage/help text to document the new parameter/env
and ensure the loop uses the resolved variable instead of the hardcoded path.
| - **In plan mode:** Write to `~/.claude/plans/<plan-name>.md` (staging area, as specified by plan mode system prompt) | ||
| - **In regular mode:** Write to `<working-directory>/<target-dir>/<plan-name>.md` | ||
| - Default: `<working-directory>/llm/implementation-plans/<plan-name>.md` | ||
| - Configurable via `--target-dir` parameter | ||
|
|
||
| **Note:** The target directory structure is workflow-specific. The default assumes an `llm/` directory pattern, but this can be customized for projects with different conventions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "SKILL.md" -type f 2>/dev/null | head -20Repository: obra/superpowers
Length of output: 42
🏁 Script executed:
git ls-files | grep -i "skill\.md"Repository: obra/superpowers
Length of output: 809
🏁 Script executed:
cat -n skills/record-plan/SKILL.md | sed -n '40,60p'Repository: obra/superpowers
Length of output: 1424
🏁 Script executed:
cat -n skills/record-plan/SKILL.md | sed -n '260,280p'Repository: obra/superpowers
Length of output: 1027
🏁 Script executed:
cat -n skills/record-plan/SKILL.md | sed -n '283,300p'Repository: obra/superpowers
Length of output: 825
🏁 Script executed:
grep -r "Task 1" skills/ README* CONTRIBUTING* 2>/dev/null | head -20Repository: obra/superpowers
Length of output: 414
🏁 Script executed:
grep -r "~/.claude/plans" . --include="*.md" --include="*.sh" 2>/dev/nullRepository: obra/superpowers
Length of output: 1023
🏁 Script executed:
grep -r "~/.claude/templates" . --include="*.md" --include="*.sh" 2>/dev/nullRepository: obra/superpowers
Length of output: 456
🏁 Script executed:
find . -name "setup*" -o -name "install*" -o -name "SETUP*" | grep -v node_modules | head -20Repository: obra/superpowers
Length of output: 130
🏁 Script executed:
cat -n README.md | head -100Repository: obra/superpowers
Length of output: 4911
🏁 Script executed:
cat -n skills/record-plan/scripts/install-hook.shRepository: obra/superpowers
Length of output: 1312
🏁 Script executed:
cat -n skills/subagent-driven-development/SKILL.md | grep -A 20 -B 5 "Task 1"Repository: obra/superpowers
Length of output: 1087
🏁 Script executed:
cat -n skills/subagent-driven-development/SKILL.md | head -150Repository: obra/superpowers
Length of output: 4950
🏁 Script executed:
grep -r "mkdir.*claude" . --include="*.md" --include="*.sh" 2>/dev/nullRepository: obra/superpowers
Length of output: 42
🏁 Script executed:
grep -r "\.claude/plans" . --include="*.md" -B 3 -A 3 2>/dev/null | head -40Repository: obra/superpowers
Length of output: 2928
🏁 Script executed:
cat -n skills/record-plan/SKILL.md | sed -n '1,50p'Repository: obra/superpowers
Length of output: 3090
🏁 Script executed:
cat -n skills/record-plan/SKILL.md | sed -n '250,310p'Repository: obra/superpowers
Length of output: 2814
🏁 Script executed:
cat -n .codex/INSTALL.md 2>/dev/null || echo "File not found"Repository: obra/superpowers
Length of output: 1229
🏁 Script executed:
cat -n .opencode/INSTALL.md 2>/dev/null || echo "File not found"Repository: obra/superpowers
Length of output: 3987
🏁 Script executed:
find . -name "*.md" -type f | xargs grep -l "Task 1" 2>/dev/nullRepository: obra/superpowers
Length of output: 235
🏁 Script executed:
cat -n docs/README.codex.md 2>/dev/null | head -200 || echo "File not found"Repository: obra/superpowers
Length of output: 4538
🏁 Script executed:
find . -name "INSTALL*" -o -name "setup*" | xargs cat 2>/dev/null | head -100Repository: obra/superpowers
Length of output: 2295
🏁 Script executed:
find . -type f \( -name "*.md" -o -name "*.sh" \) | xargs grep -l "claude/plans\|claude/templates\|claude/scripts" 2>/dev/null | head -20Repository: obra/superpowers
Length of output: 123
🏁 Script executed:
grep -r "\.claude" README.md docs/ .codex/ .opencode/ 2>/dev/nullRepository: obra/superpowers
Length of output: 42
🏁 Script executed:
cat -n skills/record-plan/SKILL.md | sed -n '15,25p'Repository: obra/superpowers
Length of output: 525
🏁 Script executed:
cat -n skills/record-plan/CHANGELOG.mdRepository: obra/superpowers
Length of output: 1789
🏁 Script executed:
grep -r "bootstrap\|setup\|Task 1" . --include="*.md" | grep -v ".git" | head -30Repository: obra/superpowers
Length of output: 3584
🏁 Script executed:
find . -name "write_plan.py" -o -name "*bootstrap*" | grep -v node_modulesRepository: obra/superpowers
Length of output: 136
🏁 Script executed:
cat -n skills/record-plan/scripts/write_plan.py | head -100Repository: obra/superpowers
Length of output: 3516
🏁 Script executed:
cat -n .codex/superpowers-bootstrap.md | head -150Repository: obra/superpowers
Length of output: 1857
🏁 Script executed:
grep -A 10 "\.claude\|setup\|mkdir" skills/record-plan/scripts/write_plan.pyRepository: obra/superpowers
Length of output: 1173
Document directory prerequisites and setup for ~/.claude/ paths, or add validation that creates them.
Lines 47, 268, and 295 reference ~/.claude/plans/, ~/.claude/templates/, and ~/.claude/scripts/record-tools/ without documenting setup. Line 288-291 includes a validation check that fails with "Contact your human partner - Task 1 must be completed first," but "Task 1" is never explained in the documentation. The installation instructions (README.md, .codex/INSTALL.md, .opencode/INSTALL.md) do not mention creating these directories.
Users following only the provided installation will encounter runtime failures when:
- Using plan mode (tries to write to non-existent ~/.claude/plans/)
- Initializing progress log (template validation fails at line 288)
- Running post-write workflow (scripts at ~/.claude/scripts/record-tools/ don't exist)
Either add explicit setup steps to installation docs (what creates these directories, when, and how), or add validation logic that creates missing directories with clear error messages if prerequisites cannot be met.
🤖 Prompt for AI Agents
In skills/record-plan/SKILL.md around lines 47, 268, 288-291 and 295, document
and/or programmatically ensure the ~/.claude directories referenced
(~/.claude/plans/, ~/.claude/templates/, ~/.claude/scripts/record-tools/) are
created before use: add a clear installation/setup subsection listing each
directory, when it should be created, and the exact commands (mkdir -p
~/.claude/plans ~/.claude/templates ~/.claude/scripts/record-tools) or update
the code paths to perform validation that creates missing directories at startup
and emits clear error messages; also clarify the ambiguous "Task 1" reference in
lines 288-291 by naming the required prereq step (or linking to the setup
instructions) so the validation message tells users exactly what to do.
…ded values Address 2 valid issues from CodeRabbit review: 1. Module system inconsistency: - Convert .codex/superpowers-codex from CommonJS to ES imports - Aligns with lib/skills-core.js and .opencode/plugin/superpowers.js 2. Hardcoded marketplace name: - Extract marketplace name from .claude-plugin/marketplace.json - Fall back to "superpowers-dev" if jq unavailable or file missing - Eliminates maintenance burden in scripts/reload-plugin.sh Both fixes improve mechanical enforcement (ws skill principle).
Response to CodeRabbit ReviewAfter evaluating all suggestions against the ✅ Implemented (commit 6ce3936)
❌ Rejected Suggestions1. Python f-string "Issues" (check_lock.py, write_plan.py)Claim: "Remove extraneous f-string prefixes" Rejection rationale:
2. Description Length (subagent-driven-development/SKILL.md)Claim: "Description is quite long—consider tightening" (181 chars) Rejection rationale:
3. DRY Violation in Plan Document (docs/plans/2025-11-22-opencode-support-implementation.md)Claim: "DRY violation: frontmatter stripping logic duplicated" Rejection rationale:
4. Path "Inconsistencies" (write_plan.py)Claim: Questions Rejection rationale:
📋 Deferred Style SuggestionsThese don't affect agent behavior or skill effectiveness:
Can be addressed in a dedicated polish pass if desired. Decision framework: All changes evaluated based on impact to (1) agent behavior, (2) skill effectiveness/CSO, (3) maintenance burden. Style preferences without functional impact are low priority. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (6)
skills/writing-plans/scripts/generate_acceptance.py (2)
19-19: Remove unused loop variable index.The loop uses
enumerate()but never references the index variableidx. This adds unnecessary overhead.Apply this diff to simplify:
- for idx, line in enumerate(plan_content.split('\n')): + for line in plan_content.split('\n'):
51-51: Fix potential ValueError when computing relative path.Line 51 uses
plan_file.relative_to(Path.cwd())which will raiseValueErrorif the current working directory is not a parent of the plan file. This breaks the script when invoked from different directories.Apply this diff to handle the error:
- "plan": str(plan_file.relative_to(Path.cwd())), + try: + plan_path = str(plan_file.relative_to(Path.cwd())) + except ValueError: + # Fallback to absolute path if cwd is not a parent + plan_path = str(plan_file.resolve()) + + acceptance = { + "plan": plan_path,skills/writing-plans/SKILL.md (1)
19-22: Document prerequisites for ~/.claude/ directory structure.Multiple references to
~/.claude/paths appear without documented setup instructions:
~/.claude/scripts/writing-plans/write_plan.py(line 19)~/.claude/plans/(line 47)~/.claude/templates/progress.md(line 288)~/.claude/scripts/record-tools/(line 295)Users will encounter runtime failures if these directories/files don't exist. The "Task 1" reference at line 288-291 is unclear without documentation.
Either add explicit setup documentation (with
mkdir -pcommands) or add validation logic that creates missing directories with clear error messages.Also applies to: 47-51, 268-268, 288-291, 295-295
skills/writing-plans/scripts/install-hook.sh (2)
24-24: Critical: Hardcoded user-specific path prevents portability.Line 24 hardcodes
/Users/ethan.stark/dev/*/making this script completely unusable for other users. This contradicts the PR's goal of production-tested improvements for broader use.Apply this diff to accept a path parameter:
#!/bin/bash # Install pre-commit hook for writing-plans enforcement +# Usage: install-hook.sh [base-directory] +# Default: ~/dev +BASE_DIR="${1:-$HOME/dev}" + HOOK_CONTENT='...' # Find all repos with llm/implementation-plans/ directory -for repo in /Users/ethan.stark/dev/*/; do +for repo in "$BASE_DIR"/*/; do
27-29: Hook overwrite silently removes existing pre-commit logic.Line 27 unconditionally overwrites
.git/hooks/pre-commitwithout checking if a hook already exists. This causes silent data loss if repositories have existing pre-commit hooks for linting, formatting, or other workflows.Add a check before overwriting:
for repo in "$BASE_DIR"/*/; do if [ -d "$repo/llm/implementation-plans" ] && [ -d "$repo/.git" ]; then hook_path="$repo/.git/hooks/pre-commit" + + # Warn if hook already exists + if [ -f "$hook_path" ]; then + echo "⚠️ WARNING: $hook_path already exists" + echo " Existing content:" + cat "$hook_path" | head -5 + echo " Backup: $hook_path.backup" + cp "$hook_path" "$hook_path.backup" + fi + echo "$HOOK_CONTENT" > "$hook_path"skills/writing-plans/scripts/check_lock.py (1)
23-25: Basename-only check allows unauthorized path writes.Line 23 validates only that the file path ends with the authorized path's basename. This could allow writes to unintended locations if multiple files share the same basename.
Example:
- Lock authorizes:
/project/llm/plans/plan.md- Attack: write to
/project/other/plan.md✓ passes (incorrect)Apply this diff to compare full paths:
- if not file_path.endswith(os.path.basename(authorized_path)): + # Compare normalized absolute paths + authorized_abs = os.path.abspath(authorized_path) + file_abs = os.path.abspath(file_path) + + if authorized_abs != file_abs: print(f"❌ ERROR: Lock file authorizes {authorized_path}, not {file_path}") + print(f" Authorized: {authorized_abs}") + print(f" Attempted: {file_abs}") return False
🧹 Nitpick comments (2)
skills/writing-plans/scripts/write_plan.py (2)
49-52: Consider creating target directory if it doesn't exist.Lines 49-50 construct
target_dirandtarget_filepaths, but don't verify the directory exists. Ifllm/implementation-plans/(or custom--target-dir) doesn't exist, subsequent file operations will fail.Add directory creation after line 50:
target_dir = os.path.join(args.working_dir, args.target_dir) target_file = f"{target_dir}/{args.plan_name}.md" + + # Ensure target directory exists + os.makedirs(target_dir, exist_ok=True) + print(f"📝 REGULAR MODE")Alternatively, document in the usage that target directories must pre-exist.
55-60: Add error handling for lock file creation.Lines 56-58 open and write the lock file without error handling. If
working_dirdoesn't exist or isn't writable, this will raise an unhandled exception with a cryptic error message.Add error handling:
# Create lock file to enable Write tool for this plan lock_file = os.path.join(args.working_dir, '.writing-plans-active') - with open(lock_file, 'w') as f: - f.write(f"{target_file}\n") - f.write(f"created: {datetime.now().isoformat()}\n") + + try: + with open(lock_file, 'w') as f: + f.write(f"{target_file}\n") + f.write(f"created: {datetime.now().isoformat()}\n") + except (OSError, IOError) as e: + print(f"❌ ERROR: Cannot create lock file at {lock_file}") + print(f" {e}") + print(f" Verify working directory exists and is writable") + return 1 print(f"🔓 Created lock file: {lock_file}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.claude-plugin/plugin.json(1 hunks)README.md(1 hunks)skills/writing-plans/CHANGELOG.md(1 hunks)skills/writing-plans/SKILL.md(4 hunks)skills/writing-plans/scripts/check_lock.py(1 hunks)skills/writing-plans/scripts/generate_acceptance.py(1 hunks)skills/writing-plans/scripts/install-hook.sh(1 hunks)skills/writing-plans/scripts/write_plan.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- skills/writing-plans/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🧬 Code graph analysis (1)
skills/writing-plans/scripts/write_plan.py (2)
skills/writing-plans/scripts/generate_acceptance.py (1)
main(71-82)lib/skills-core.js (1)
description(23-23)
🪛 markdownlint-cli2 (0.18.1)
skills/writing-plans/SKILL.md
343-343: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
413-413: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.8)
skills/writing-plans/scripts/check_lock.py
14-14: f-string without any placeholders
Remove extraneous f prefix
(F541)
16-16: f-string without any placeholders
Remove extraneous f prefix
(F541)
skills/writing-plans/scripts/generate_acceptance.py
19-19: Loop control variable idx not used within loop body
Rename unused idx to _idx
(B007)
skills/writing-plans/scripts/write_plan.py
45-45: f-string without any placeholders
Remove extraneous f prefix
(F541)
51-51: f-string without any placeholders
Remove extraneous f prefix
(F541)
62-62: f-string without any placeholders
Remove extraneous f prefix
(F541)
63-63: f-string without any placeholders
Remove extraneous f prefix
(F541)
64-64: f-string without any placeholders
Remove extraneous f prefix
(F541)
65-65: f-string without any placeholders
Remove extraneous f prefix
(F541)
66-66: f-string without any placeholders
Remove extraneous f prefix
(F541)
67-67: f-string without any placeholders
Remove extraneous f prefix
(F541)
68-68: f-string without any placeholders
Remove extraneous f prefix
(F541)
71-71: f-string without any placeholders
Remove extraneous f prefix
(F541)
73-73: f-string without any placeholders
Remove extraneous f prefix
(F541)
74-74: f-string without any placeholders
Remove extraneous f prefix
(F541)
75-75: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (1)
.claude-plugin/plugin.json (1)
2-12: LGTM! Proper fork attribution and metadata updates.The metadata changes correctly identify this as a fork while maintaining proper attribution to the original author (Jesse Vincent). The updated URLs and keywords accurately reflect the fork's identity and added capabilities.
Regression fixes: - Bundle rename_jot.py with .writing-plans-active lock file support - Bundle validate-frontmatter.py for self-contained operation - Update all script paths to use $CLAUDE_PLUGIN_ROOT for portability - Fix marketplace.json to use superpowers-fork name (not superpowers) - Default output: llm/implementation-plans/ (not docs/plans/) - Graceful file-track fallback for non-installed users Root cause: Plugin name mismatch caused upstream to load instead of fork
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
skills/writing-plans/SKILL.md (1)
277-318: Unresolved: Directory prerequisites still need documentation or auto-creation.This matches the previous review comment. Lines 288-291 reference "Task 1 must be completed first" without explaining what Task 1 is or how to complete it. The
~/.claude/templates/progress.mdpath is used without setup instructions.Consider either:
- Adding a prerequisites section documenting required directory setup
- Modifying scripts to auto-create directories with informative messages
🧹 Nitpick comments (4)
skills/writing-plans/scripts/rename_jot.py (1)
82-109: Consider narrowing exception handling for better diagnostics.The blanket
except Exceptionat line 108 is intentional for robustness, but it may hide unexpected errors during debugging. Consider catching more specific exceptions (IOError, OSError, UnicodeDecodeError) or logging the exception type in a debug mode.skills/writing-plans/scripts/validate-frontmatter.py (1)
78-81: Minor: Consider using.get()for cleaner date validation.The static analysis hint (RUF019) suggests simplifying the key check. However, the current form is explicit and readable.
- if 'date' in frontmatter and frontmatter['date']: - if not re.match(r'^\d{4}-\d{2}-\d{2}$', frontmatter['date']): + date_value = frontmatter.get('date') + if date_value and not re.match(r'^\d{4}-\d{2}-\d{2}$', date_value): errors.append(f"Invalid date format '{frontmatter['date']}'. Use YYYY-MM-DD format.")skills/writing-plans/scripts/write_plan.py (1)
54-60: Consider adding validation that working-dir exists before creating lock file.If
--working-dirpoints to a non-existent directory, theopen()at line 56 will fail with an unclear error. A pre-check could provide a better message.+ # Validate working directory exists + if not os.path.isdir(args.working_dir): + print(f"Error: Working directory does not exist: {args.working_dir}", file=sys.stderr) + return 1 + # Create lock file to enable Write tool for this plan lock_file = os.path.join(args.working_dir, '.writing-plans-active')skills/writing-plans/SKILL.md (1)
343-345: Add language specifiers to fenced code blocks.Static analysis (MD040) flagged code blocks at lines 343 and 413 as missing language specifiers. Use
textfor output/message blocks.-``` +```text ✓ Renamed plan-name.md → 251213-01-plan-name.md-``` +```text Plan complete: llm/implementation-plans/<filename>.mdAlso applies to: 413-419
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.claude-plugin/marketplace.json(1 hunks)skills/writing-plans/SKILL.md(4 hunks)skills/writing-plans/scripts/rename_jot.py(1 hunks)skills/writing-plans/scripts/validate-frontmatter.py(1 hunks)skills/writing-plans/scripts/write_plan.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
skills/writing-plans/scripts/validate-frontmatter.py (3)
lib/skills-core.js (3)
content(18-18)match(33-33)key(35-35)skills/writing-plans/scripts/rename_jot.py (1)
main(196-254)skills/writing-plans/scripts/write_plan.py (1)
main(13-77)
skills/writing-plans/scripts/rename_jot.py (2)
skills/writing-plans/scripts/validate-frontmatter.py (1)
main(86-113)skills/writing-plans/scripts/write_plan.py (1)
main(13-77)
skills/writing-plans/scripts/write_plan.py (4)
skills/writing-plans/scripts/rename_jot.py (1)
main(196-254)skills/writing-plans/scripts/validate-frontmatter.py (1)
main(86-113)skills/writing-plans/scripts/generate_acceptance.py (1)
main(71-82)lib/skills-core.js (1)
description(23-23)
🪛 markdownlint-cli2 (0.18.1)
skills/writing-plans/SKILL.md
343-343: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
413-413: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.8)
skills/writing-plans/scripts/validate-frontmatter.py
79-79: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
99-99: Do not catch blind exception: Exception
(BLE001)
skills/writing-plans/scripts/rename_jot.py
107-107: Consider moving this statement to an else block
(TRY300)
108-108: Do not catch blind exception: Exception
(BLE001)
173-173: Consider moving this statement to an else block
(TRY300)
224-224: Do not catch blind exception: Exception
(BLE001)
232-232: subprocess call: check for execution of untrusted input
(S603)
skills/writing-plans/scripts/write_plan.py
45-45: f-string without any placeholders
Remove extraneous f prefix
(F541)
51-51: f-string without any placeholders
Remove extraneous f prefix
(F541)
62-62: f-string without any placeholders
Remove extraneous f prefix
(F541)
63-63: f-string without any placeholders
Remove extraneous f prefix
(F541)
64-64: f-string without any placeholders
Remove extraneous f prefix
(F541)
65-65: f-string without any placeholders
Remove extraneous f prefix
(F541)
66-66: f-string without any placeholders
Remove extraneous f prefix
(F541)
67-67: f-string without any placeholders
Remove extraneous f prefix
(F541)
68-68: f-string without any placeholders
Remove extraneous f prefix
(F541)
71-71: f-string without any placeholders
Remove extraneous f prefix
(F541)
73-73: f-string without any placeholders
Remove extraneous f prefix
(F541)
74-74: f-string without any placeholders
Remove extraneous f prefix
(F541)
75-75: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (16)
.claude-plugin/marketplace.json (2)
10-11: Fork identity is clear and well-attributed.The plugin name and description clearly identify this as a fork while properly crediting the original author.
15-15: Attribution approach looks good.The author field clearly identifies the fork author while preserving credit to the original creator.
skills/writing-plans/scripts/rename_jot.py (5)
1-39: Well-structured fallback pattern with optional file-track integration.The graceful degradation approach is good - importing from
file_trackwhen available and falling back to local implementations. The warning message at line 188 appropriately informs users when fallback is active.
46-79: LGTM - slugify implementation is correct.Handles edge cases well: ASCII normalization, hyphen collapsing, truncation with trailing hyphen cleanup, and "untitled" fallback.
112-140: LGTM - sequence number logic is sound.Properly handles edge cases: non-existent directories, invalid sequence numbers, and graceful cap at 99 with user warning.
143-176: LGTM - rename logic is correct.The early return for already-formatted files (line 147) prevents unnecessary renames. The slug extraction priority (H1 title → filename) is sensible.
241-252: Git root detection could miss repositories at filesystem root.The loop termination condition (
working_dir != working_dir.parent) works for most cases, but if the file is at filesystem root/, it will exit without finding.git. This is an edge case unlikely to occur in practice.skills/writing-plans/scripts/validate-frontmatter.py (2)
20-43: LGTM - frontmatter extraction is well-designed.The regex correctly handles optional jot comments before frontmatter, and the simple YAML parser handles key-value pairs appropriately for this use case.
86-113: LGTM - CLI implementation follows good practices.Proper separation of success output (stdout) vs error output (stderr), appropriate exit codes, and specific exception handling for common errors (FileNotFoundError) with fallback to generic.
skills/writing-plans/scripts/write_plan.py (3)
13-38: LGTM - CLI argument parsing is well-structured.Clear argument definitions with helpful descriptions and sensible defaults. The
--in-plan-modeflag appropriately switches between staging and direct write modes.
40-52: LGTM - target path logic correctly implements the workflow.Plan mode uses staging area with subsequent copy, regular mode writes directly. Consistent with SKILL.md documentation.
62-77: Clear instructions enforce the mechanical workflow.The emphatic warnings ("DO NOT just describe the plan!") directly address the production incident documented in SKILL.md. The step-by-step guidance helps prevent workflow violations.
skills/writing-plans/SKILL.md (4)
3-3: Good - description is comprehensive and includes enforcement semantics.The description clearly states scope boundaries ("ONLY writes plans, never executes") and enforcement mechanism ("Mechanically enforced via lock file"). This aids discoverability and sets correct expectations.
16-43: Effective enforcement messaging with production context.The "DO NOT before invoking wrapper" list and production incident documentation (2025-12-13) effectively communicate the importance of following the workflow mechanically.
214-251: Excellent anti-rationalization section.The "Why you're reading this section: You already rationalized around skill boundaries" framing and the specific thought patterns to watch for ("I'll describe the plan structure first") directly address the documented production incidents.
400-449: LGTM - clear scope boundaries with testing evidence.The STOP section explicitly defines what's in-scope vs out-of-scope, and the testing verification section documents the enforcement approach with concrete results. This supports the "automation over documentation" philosophy.
| "name": "Jesse Vincent", | ||
| "email": "jesse@fsck.com" | ||
| "name": "Ethan Stark (original by Jesse Vincent)", | ||
| "email": "ethan.stark@renlearn.com" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid exposing personal/corporate email addresses in repository metadata.
The corporate email address is exposed in the plugin metadata, which creates privacy and security risks (spam, phishing, email harvesting). If this repository is or becomes public, the email will be permanently visible in git history.
Consider using a generic contact method or a GitHub-provided noreply address instead.
🤖 Prompt for AI Agents
In .claude-plugin/marketplace.json around line 16, the file currently exposes a
personal/corporate email ("ethan.stark@renlearn.com"); replace this with a
non-personal contact such as a generic team address, a maintainer role (e.g.,
support@yourdomain or contact@yourdomain), or a GitHub noreply/contact URL, and
update the commit to remove the personal email from future commits (e.g., amend
the commit or create a follow-up commit) and, if necessary, rotate or redact the
email from history using git tools (git rebase --interactive or git filter-repo)
before making the repository public.
- Replace hardcoded $CLAUDE_PLUGIN_ROOT variable (not set by Claude Code) - Use find command to auto-locate scripts in plugin cache - Fixes 'can't open file' error when invoking write_plan.py - Works with both marketplace and local-dev installations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
skills/writing-plans/SKILL.md (1)
47-55: Consider adding an explicit setup/prerequisites section addressing ~/.claude directory creation.The documentation now clearly explains where to write files (lines 47-55) and validates the progress template (lines 290-294), but does not document how the underlying ~/.claude directories are created or initialized. While lines 280-299 show validation that checks template existence, it doesn't explain:
- How ~/.claude/plans, ~/.claude/templates/, ~/.claude/scripts/record-tools/ are initially created
- When these should be created (before first use? by install-hook.sh? by plugin initialization?)
- Whether users must manually mkdir or if some tooling handles it
This leaves a gap for new users who see "Contact your human partner - Task 1 must be completed" without understanding what needs to be done.
Adding a "Setup / Prerequisites" section before "Bite-Sized Task Granularity" (before line 57) that explicitly documents directory creation would improve discoverability and reduce friction for first-time users.
Also applies to: 279-320
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
skills/writing-plans/SKILL.md(4 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
skills/writing-plans/SKILL.md
347-347: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
417-417: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
| ```bash | ||
| # Verify template exists first | ||
| if [ ! -f ~/.claude/templates/progress.md ]; then | ||
| echo "ERROR: Template not found at ~/.claude/templates/progress.md" | ||
| echo "Contact your human partner - Task 1 must be completed first" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
find . -name "SKILL.md" -type f 2>/dev/null | head -20Repository: obra/superpowers
Length of output: 851
🏁 Script executed:
fd "SKILL.md" -type fRepository: obra/superpowers
Length of output: 230
🏁 Script executed:
# Search for references to "Task 1" in the codebase
rg "Task 1" --type md -A 2 -B 2Repository: obra/superpowers
Length of output: 4939
🏁 Script executed:
# List files in skills/writing-plans directory
git ls-files "skills/writing-plans/" 2>/dev/null || find . -path "*/skills/writing-plans/*" -type f 2>/dev/nullRepository: obra/superpowers
Length of output: 402
🏁 Script executed:
# Read the SKILL.md file around lines 288-294 for context
sed -n '280,310p' skills/writing-plans/SKILL.mdRepository: obra/superpowers
Length of output: 1072
🏁 Script executed:
# Check what install-hook.sh contains
cat skills/writing-plans/scripts/install-hook.shRepository: obra/superpowers
Length of output: 1099
🏁 Script executed:
# Search for "Task 1" definition or explanation in the writing-plans skill
grep -n "Task 1" skills/writing-plans/SKILL.md -B 5 -A 5Repository: obra/superpowers
Length of output: 470
🏁 Script executed:
# Look for CLAUDE.md or other setup documentation
find . -name "CLAUDE.md" -o -name ".codex/INSTALL.md" -o -name "INSTALL.md" 2>/dev/null | head -10Repository: obra/superpowers
Length of output: 114
🏁 Script executed:
# Check what CLAUDE.md says about setup
grep -n "Task 1\|template\|progress.md" CLAUDE.md -A 3 -B 3 | head -50Repository: obra/superpowers
Length of output: 42
🏁 Script executed:
# Check .codex/INSTALL.md
head -100 .codex/INSTALL.mdRepository: obra/superpowers
Length of output: 984
🏁 Script executed:
# Search for any documentation about the template setup prerequisite
rg "progress.md\|Task 1" --type md docs/ --max-count 20 -B 2 -A 2Repository: obra/superpowers
Length of output: 42
🏁 Script executed:
# Look for references to "template" in the entire writing-plans skill
rg "template" skills/writing-plans/SKILL.md -B 2 -A 2Repository: obra/superpowers
Length of output: 1564
🏁 Script executed:
# Get full CLAUDE.md to see if there's any template setup instructions
head -100 CLAUDE.mdRepository: obra/superpowers
Length of output: 3811
🏁 Script executed:
# Search more broadly for template setup instructions
rg "\.claude/templates" --type md -B 3 -A 3Repository: obra/superpowers
Length of output: 1308
Clarify the undefined "Task 1" prerequisite reference (line 292).
Line 292 includes the error message "Contact your human partner - Task 1 must be completed first," but "Task 1" is never defined in this or any referenced documentation. This leaves users unable to self-serve when the template check fails.
Either:
- Reference a concrete setup doc (e.g., "See .codex/INSTALL.md or run ~/.codex/superpowers/.codex/superpowers-codex bootstrap")
- Name the actual prerequisite (e.g., "Template initialization must be completed first" or "Run: mkdir -p ~/.claude/templates && cp /path/to/template ~/.claude/templates/progress.md")
- Replace with actionable guidance (e.g., "Ensure ~/.claude/templates/progress.md exists. Copy the template from the superpowers repository or initialize via Codex bootstrap.")
The current message suggests an external setup step was already completed, but provides no way for users encountering this error to proceed.
🤖 Prompt for AI Agents
In skills/writing-plans/SKILL.md around lines 288-294, the error message
references an undefined "Task 1" and gives no actionable next steps; replace
that vague line with a concrete, actionable instruction that tells the user how
to create or obtain ~/.claude/templates/progress.md or where to find setup docs
— for example, change the message to instruct users to ensure
~/.claude/templates/progress.md exists and either run the Codex bootstrap (point
to .codex/INSTALL.md or the bootstrap path in the repo) or create the directory
and copy the progress.md template from the superpowers repository so users can
self-serve.
- Command substitution $(find...) doesn't work in Bash tool environment
- Split into two steps: find path, then use literal path
- Fixes parse error: (eval):1: parse error near `('
- Applies to all script invocations: write_plan, validate, rename, check_lock
- Based on user-confirmed workaround from production testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
skills/writing-plans/SKILL.md (2)
303-307: ** "Task 1" error message is still undefined—users cannot self-serve.**This is a duplicate of the past review comment (lines 301-307 in prior review). Line 305 still references "Task 1 must be completed first" without explaining what Task 1 is or how to proceed.
Users encountering this error have no actionable next steps. Replace the vague message with a concrete instruction, such as:
echo "ERROR: Template not found at ~/.claude/templates/progress.md" echo "Run the Codex bootstrap to initialize templates:" echo " bash /path/to/.codex/superpowers-codex bootstrap" exit 1Or, simpler:
echo "ERROR: Please create ~/.claude/templates/progress.md by copying from the superpowers repository or running setup." exit 1
365-368: ** Add language identifier to code block (MD040).**Line 366 is missing a language specification on the fenced code block.
Apply this diff:
Expected output: -``` +```bash ✓ Renamed plan-name.md → 251213-01-plan-name.md</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 603103e17a47f31a3212ac8760b7eb03fbfbba31 and 7332acec53e42ee0a15d98476cabca43131bad41. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `skills/writing-plans/SKILL.md` (4 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>skills/writing-plans/SKILL.md</summary> 18-18: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) --- 23-23: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) --- 366-366: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 436-436: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| **Report to user:** | ||
|
|
||
| ``` | ||
| Plan complete: llm/implementation-plans/<filename>.md | ||
| Next step: Use /superpowers:execute-plan OR open new session with executing-plans skill. | ||
| [STOP - writing-plans skill scope ends here] | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add language identifier to code block (MD040).
Lines 436-442 contain a fenced code block without a language identifier.
Apply this diff:
**Report to user:**
-```
+```markdown
Plan complete: llm/implementation-plans/<filename>.md
Next step: Use /superpowers:execute-plan OR open new session with executing-plans skill.
[STOP - writing-plans skill scope ends here]
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
436-436: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In skills/writing-plans/SKILL.md around lines 434 to 442 there is a fenced code
block lacking a language identifier (MD040); update the opening fence to include
the language by changing tomarkdown so the block becomes a
markdown‑annotated fenced code block, preserving the inner content and closing
fence unchanged.
</details>
<!-- fingerprinting:phantom:triton:mongoose -->
<!-- This is an auto-generated comment by CodeRabbit -->
- Document command substitution parse error as Common Mistake obra#5 - Add Troubleshooting section with script path and lock file issues - Explain root cause: Bash tool shell evaluation limitation - Based on production testing showing zsh parse error - Per ws skill: close loopholes explicitly with user-confirmed symptoms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
skills/writing-plans/SKILL.md (1)
268-334: Post-write workflow is comprehensive and actionable.The step-by-step process (copy from staging, initialize progress log with template check, validate frontmatter, rename, optional acceptance criteria) provides clear guardrails. The template copy instructions (lines 309–320) explicitly forbid custom formats, enforcing consistency. Error handling and branch-out paths are well-signaled.
One note: Line 305 retains the undefined "Task 1" error message from a prior review comment. While your author response indicates this path is intentional for plan-mode workflows, the error message still lacks actionable context for users encountering it. This may be acceptable for plan-mode-only usage, but flagging for awareness.
🧹 Nitpick comments (1)
skills/writing-plans/SKILL.md (1)
476-482: Add language identifier to code block at line 476 (MD040).The closing report block is missing a language identifier.
**Report to user:** -``` +```markdown Plan complete: llm/implementation-plans/<filename>.md Next step: Use /superpowers:execute-plan OR open new session with executing-plans skill. [STOP - writing-plans skill scope ends here]</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 7332acec53e42ee0a15d98476cabca43131bad41 and ddae0860995cbb8b837d86a8efe10f2ac12e9ce2. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `skills/writing-plans/SKILL.md` (4 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>skills/writing-plans/SKILL.md</summary> 18-18: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) --- 23-23: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) --- 366-366: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 476-476: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>skills/writing-plans/SKILL.md (1)</summary><blockquote> `16-62`: **Wrapper-driven workflow invocation is well-structured.** The new section clearly explains the mandatory wrapper script pattern, including the two-step invocation approach (avoiding command substitution gotchas) and enforcement via lock file. The production incident callback and "do not before invoking" guidelines reinforce the mechanical boundary. The clarification on absolute vs. relative paths (lines 118–155) and nested git repository handling (lines 131–155) effectively address practical edge cases. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
Closing this PR to resubmit from a clean branch that removes fork-specific configuration. The original PR from fork:main contained:
New PR will come from pr/170-upstream-clean branch with these files cleaned up for upstream submission. All functional changes remain intact. |
Production-Tested Improvements: Mechanical Enforcement & Safety Gates
This PR introduces several production-hardened improvements to the superpowers plugin, grounded in real incidents and test-driven documentation methodology.
Key Changes
1. Writing-Plans Automation Framework
Enhances the existing
writing-plansskill with executable wrapper scripts that enforce plan creation through lock files and git pre-commit hooks. This shifts from documentation-only guidance to mechanical constraints, addressing a December 13 incident where an agent rationalized away the plan-writing requirement despite explicit warnings.Technical implementation:
skills/writing-plans/scripts/- Python/shell automation for lock file management and plan validationKey insight: When requirements are 100% objective and violations are programmatically detectable, use automation over documentation.
2. Automation Over Documentation Framework
A new guidance document (
skills/writing-skills/automation-over-documentation.md) establishing when to use code vs. documentation for skill requirements. The framework provides clear decision criteria: objective, programmatically detectable requirements should trigger automation rather than endless documentation iterations.3. PR Creation Safety Gate
Adds mandatory user preview and approval before pull request creation in the
finishing-a-development-branchskill. This defense-in-depth approach prevents agents from bypassing approval workflows through rationalization.Implementation:
jira-publisherskill4. Continuous Execution Pattern
Removes artificial 3-task batching checkpoints from
executing-plans, allowing uninterrupted workflow through multiple tasks with only genuine blockers causing pauses. This improves development velocity while maintaining quality gates.5. Writing-Skills Improvements
Strengthens skill modification governance with:
Supporting Infrastructure
Statistics
Philosophy
This PR embodies the principle: automation over documentation for objective constraints, while preserving judgment-based guidance for subjective decisions. Skills are TDD for documentation—we test first, then write documentation that makes tests pass, iterating to close loopholes where agents rationalize around requirements.
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.