feat: add auto-sidecar skills (review, unblock, security)#7
feat: add auto-sidecar skills (review, unblock, security)#7ellisjr wants to merge 13 commits intojrenaldi79:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates postinstall to discover and install multiple SKILL.md files, adds several new auto-skill SKILL.md documents and BMAD workflow documentation, and pluralizes a phrasing in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
scripts/postinstall.js (1)
21-21: Consider derivingAUTO_SKILLSfrom the filesystem.The hardcoded list is easy to forget when a new
skill/auto-*directory is added or renamed, which leaves installation behavior out of sync with the docs.Possible refactor
-const AUTO_SKILLS = ['auto-review', 'auto-unblock', 'auto-security']; +const AUTO_SKILLS = fs + .readdirSync(SKILL_DIR, { withFileTypes: true }) + .filter((entry) => entry.isDirectory() && entry.name.startsWith('auto-')) + .map((entry) => entry.name) + .sort();Also applies to: 66-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/postinstall.js` at line 21, Replace the hardcoded AUTO_SKILLS array with a dynamic derivation from the filesystem: use Node's fs (e.g., fs.readdirSync or asynchronous equivalent) to read the skills directory, filter entries that start with "auto-" and map to the same names currently used; assign that result to AUTO_SKILLS and reuse this derived constant wherever AUTO_SKILLS is referenced (including the logic at the other occurrence around lines 66-75) so adding/removing skill/auto-* directories automatically updates installation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skill/auto-review/SKILL.md`:
- Around line 40-49: The Markdown fences in SKILL.md (e.g., the block around
"Sidecar code review?" and the other fenced sections at the ranges shown) are
missing a language tag and trigger MD040; update each triple-backtick fence (the
``` blocks) to include a language tag (use "text") so they become ```text ...
``` for the blocks around the "Sidecar code review?" sample and the other fenced
regions (lines ~77-83 and ~95-122).
- Around line 61-63: Update the guidance in SKILL.md so the agent always runs
and unions both unstaged and staged diffs before deciding there is nothing to
review: replace the current single-command check with explicit steps to run `git
diff` and `git diff --cached`, merge their outputs into a single combined diff
block, and only treat the result as empty if that combined diff is empty (change
the wording around the "Run `git diff`..." section to instruct always running
both commands and combining results).
In `@skill/auto-security/SKILL.md`:
- Around line 42-51: Several fenced code blocks in SKILL.md lack a language tag
(triggering markdownlint MD040); update each triple-backtick block shown (the
example block around the "Sidecar security scan before commit?" text and the
other occurrences at the ranges mentioned) to include a language identifier such
as text. Locate the three code fences (the block starting near the "Sidecar
security scan before commit?" text and the other blocks covering lines
referenced 77-83 and 99-129) and change ``` to ```text so all fenced code blocks
include a language tag.
In `@skill/auto-unblock/SKILL.md`:
- Around line 48-63: The untyped fenced code blocks in SKILL.md trigger
markdownlint MD040; update each triple-backtick fence in the mentioned ranges
(including the block shown and those at 90-96, 108-139, 163-169, 177-202) to
include a language token (use "text") so they read ```text ... ```; locate the
fences in SKILL.md (the sidecar brainstorming sections and similar blocks) and
add "text" immediately after the opening backticks to satisfy MD040.
---
Nitpick comments:
In `@scripts/postinstall.js`:
- Line 21: Replace the hardcoded AUTO_SKILLS array with a dynamic derivation
from the filesystem: use Node's fs (e.g., fs.readdirSync or asynchronous
equivalent) to read the skills directory, filter entries that start with "auto-"
and map to the same names currently used; assign that result to AUTO_SKILLS and
reuse this derived constant wherever AUTO_SKILLS is referenced (including the
logic at the other occurrence around lines 66-75) so adding/removing
skill/auto-* directories automatically updates installation behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b9d0d651-3867-4359-80f7-67d828111bc3
📒 Files selected for processing (5)
CLAUDE.mdscripts/postinstall.jsskill/auto-review/SKILL.mdskill/auto-security/SKILL.mdskill/auto-unblock/SKILL.md
skill/auto-review/SKILL.md
Outdated
| ``` | ||
| Sidecar code review? | ||
|
|
||
| 1. Yes → send to: [default model] | ||
| (or specify models: e.g. "1 gemini gpt" for multi-model review) | ||
| 2. No → skip | ||
|
|
||
| Available models: <list aliases from sidecar_guide> | ||
| Full provider/model IDs also accepted (e.g., google/gemini-3.1-flash). | ||
| ``` |
There was a problem hiding this comment.
Add fence languages to satisfy markdownlint.
These fenced blocks are missing a language tag, which triggers MD040 in this file. text is enough here.
Minimal fix
-```
+```textAlso applies to: 77-83, 95-122
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 40-40: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skill/auto-review/SKILL.md` around lines 40 - 49, The Markdown fences in
SKILL.md (e.g., the block around "Sidecar code review?" and the other fenced
sections at the ranges shown) are missing a language tag and trigger MD040;
update each triple-backtick fence (the ``` blocks) to include a language tag
(use "text") so they become ```text ... ``` for the blocks around the "Sidecar
code review?" sample and the other fenced regions (lines ~77-83 and ~95-122).
There was a problem hiding this comment.
Already addressed — all opening code fences have text language tags in the current version. The bare `````` lines this flagged are closing fences, which are always bare per the CommonMark spec.
There was a problem hiding this comment.
@ellisjr, you're absolutely right — thank you for the clarification. Per the CommonMark spec, language info strings only belong on opening fences; closing fences are always bare ```. The markdownlint warning must have been a false positive targeting the closing delimiters. I'll note this for future reviews.
(◕ᴗ◕✿)
✏️ Learnings added
Learnt from: ellisjr
Repo: jrenaldi79/sidecar PR: 7
File: skill/auto-review/SKILL.md:0-0
Timestamp: 2026-03-12T14:18:34.518Z
Learning: In Markdown/CommonMark, language tags (info strings) are only valid on opening code fences (e.g., ```text). Closing fences are always bare (```). Flagging bare closing fences for missing language tags (MD040) is a false positive — do not raise this issue on closing fences.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Adds a skill that triggers after completing a feature/fix, offering to spawn a headless sidecar (Plan mode, read-only) to review changes for bugs, missed edge cases, and quality issues before claiming done. Key design decisions: - Dynamic model discovery via sidecar_guide (not hardcoded aliases) - Timeout omitted — uses sidecar platform default (15 min) - Polling cadence follows sidecar_status responses - Explicit terminal states: complete, timeout, crashed, error, aborted - Always unions staged + unstaged diffs before empty check - includeContext controls conversation history, not file access - Proof of failure required for each finding (reduces false positives) - git diff --stat for large diffs; Plan agent has read_file access - Briefing includes validation context and hardcoded-value checks - Dependency impact flagging for manifest/lockfile changes - Non-blocking: review runs in background while user gets completion msg Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ging Adds a skill that triggers after 5+ failed distinct approaches, offering an iterative brainstorming loop with a sidecar. The sidecar suggests ideas, Claude evaluates them against codebase knowledge, and feeds back what won't work — prompting deeper suggestions. Key design decisions: - Uses repeated sidecar_start (not sidecar_continue) to stay in Plan mode (headless sidecar_continue forces Build mode per mcp-server.js:275) - Dynamic model discovery via sidecar_guide (not hardcoded aliases) - Timeout omitted — uses sidecar platform default (15 min) - Polling cadence follows sidecar_status responses - Explicit terminal states: complete, timeout, crashed, error, aborted - Multi-model narrowing: all models round 1, best model only from round 2 - User check-in every 5 rounds to prevent theoretical spiraling - Rubber duck effect: abort sidecar if briefing prep solves the problem - Sidecar designs diagnostic probes, not just suggestions - Broadened context requests (dir listings, env vars, dep trees) - Token management guidance in follow-up briefings - Per-model task ID tracking for multi-model brainstorms - Max 20 iterations with clear termination conditions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a skill that triggers when user asks to commit/push/PR, offering
a headless sidecar security audit before changes enter the repository.
Sequential (blocks commit) to catch critical vulnerabilities first.
Key design decisions:
- Dynamic base branch resolution with full fallback chain
(origin/HEAD -> @{upstream} -> main -> master -> ask user)
- Audits union of staged + unstaged for commits; full branch diff for PRs
- Dynamic model discovery via sidecar_guide (not hardcoded aliases)
- Timeout omitted — uses sidecar platform default (15 min)
- Polling cadence follows sidecar_status responses
- Explicit terminal states: complete, timeout, crashed, error, aborted
- Attack vector required for each finding (reduces false positives)
- CI/CD files (.github/workflows/, etc.) in high-priority list
- Suspicious dependency detection (typosquatting, unmaintained packages)
- Security-relevant file prioritization for large diffs
- Critical/high findings block commit; medium findings are user's choice
- Does not fire on slash commands (/commit) — isolated context
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Updates postinstall.js to also install auto-review, auto-unblock, and auto-security skills to ~/.claude/skills/sidecar/<name>/SKILL.md alongside the main SKILL.md. Without this, npm install would not deploy the auto-skills to the user's skill directory. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6be1839 to
34aba52
Compare
|
Addressed the one actionable suggestion — |
Instead of hardcoding the list of auto-skills, scan the skill/ directory for auto-* subdirectories at install time. This ensures new auto-skills are picked up without updating the script. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…acts Adds a new auto-skill that offers second-opinion sidecar reviews at natural BMAD-METHOD workflow checkpoints. Covers 9 artifact types (PRD, architecture, epics, stories, etc.) with sequential multi-model review where each model sees improvements from prior rounds. Includes bmad-workflow.md reference doc. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (3)
docs/bmad-workflow.md (1)
35-35: Add language specifier to fenced code blocks.The opening code fences at lines 35 and 95 lack a language tag. For ASCII diagrams and directory listings, use
textto satisfy MD040 and improve rendering consistency.📝 Proposed fix
At line 35:
-``` +```text Brainstorming/Research ──> Product Brief ──> PRD ──> UX Design (optional)At line 95:
-``` +```text _bmad-output/Also applies to: 95-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/bmad-workflow.md` at line 35, Add the language specifier "text" to the two fenced code blocks that currently lack it: the block containing "Brainstorming/Research ──> Product Brief ──> PRD ──> UX Design (optional)" and the block starting with "_bmad-output/". Update the opening backtick lines for those code fences to use ```text so MD040 is satisfied and rendering is consistent.scripts/postinstall.js (1)
16-25: Consider defensive handling ifskill/directory is missing.
fs.readdirSync(SKILL_DIR, ...)runs at module load time before any try-catch. If theskill/directory is somehow missing from the package, the postinstall script will crash with an unhandled error. While unlikely in a properly published package, a defensive check could improve resilience.🛡️ Proposed defensive pattern
const SKILL_DIR = path.join(__dirname, '..', 'skill'); const SKILL_SOURCE = path.join(SKILL_DIR, 'SKILL.md'); const SKILL_DEST_DIR = path.join(os.homedir(), '.claude', 'skills', 'sidecar'); const SKILL_DEST = path.join(SKILL_DEST_DIR, 'SKILL.md'); -const AUTO_SKILLS = fs - .readdirSync(SKILL_DIR, { withFileTypes: true }) - .filter((entry) => entry.isDirectory() && entry.name.startsWith('auto-')) - .map((entry) => entry.name) - .sort(); +let AUTO_SKILLS = []; +try { + AUTO_SKILLS = fs + .readdirSync(SKILL_DIR, { withFileTypes: true }) + .filter((entry) => entry.isDirectory() && entry.name.startsWith('auto-')) + .map((entry) => entry.name) + .sort(); +} catch { + // skill/ directory missing — continue with empty list +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/postinstall.js` around lines 16 - 25, The code reads SKILL_DIR at module load with fs.readdirSync when computing AUTO_SKILLS, which will throw if the skill/ directory is missing; wrap the directory read in a defensive check (e.g., fs.existsSync(SKILL_DIR) or a try/catch around fs.readdirSync) and fall back to an empty array for AUTO_SKILLS when the directory is absent or unreadable so the rest of the script (references to SKILL_SOURCE, SKILL_DEST_DIR, SKILL_DEST) can continue safely.skill/auto-bmad-method-check/SKILL.md (1)
240-255: Minor: Consider varying sentence structure.Lines 240–248 have several sentences starting with "If". This is flagged by LanguageTool as repetitive. Consider rewording one or two for variety, though this is a stylistic nitpick.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skill/auto-bmad-method-check/SKILL.md` around lines 240 - 255, The reviewer flagged repetitive sentence starts in the SKILL.md section that begins "**If changes were minor (1-3 small edits):**" and the follow-up "**If changes were substantial (4+ edits, or structural changes across multiple sections):**"; edit those sentences in that block (the "If..." clauses around the minor/substantial guidance) to vary sentence openings—e.g., change one "If" to "When" or "For minor edits, ..." and rephrase another to an active-voice clause like "Perform a quick internal coherence check yourself" or "Re-read the full updated artifact to check for..."; keep the meaning identical, adjust 1–2 sentences to reduce repetition, and ensure the bullet list and the sidecar example text remain unchanged in intent and formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/bmad-workflow.md`:
- Line 35: Add the language specifier "text" to the two fenced code blocks that
currently lack it: the block containing "Brainstorming/Research ──> Product
Brief ──> PRD ──> UX Design (optional)" and the block starting with
"_bmad-output/". Update the opening backtick lines for those code fences to use
```text so MD040 is satisfied and rendering is consistent.
In `@scripts/postinstall.js`:
- Around line 16-25: The code reads SKILL_DIR at module load with fs.readdirSync
when computing AUTO_SKILLS, which will throw if the skill/ directory is missing;
wrap the directory read in a defensive check (e.g., fs.existsSync(SKILL_DIR) or
a try/catch around fs.readdirSync) and fall back to an empty array for
AUTO_SKILLS when the directory is absent or unreadable so the rest of the script
(references to SKILL_SOURCE, SKILL_DEST_DIR, SKILL_DEST) can continue safely.
In `@skill/auto-bmad-method-check/SKILL.md`:
- Around line 240-255: The reviewer flagged repetitive sentence starts in the
SKILL.md section that begins "**If changes were minor (1-3 small edits):**" and
the follow-up "**If changes were substantial (4+ edits, or structural changes
across multiple sections):**"; edit those sentences in that block (the "If..."
clauses around the minor/substantial guidance) to vary sentence openings—e.g.,
change one "If" to "When" or "For minor edits, ..." and rephrase another to an
active-voice clause like "Perform a quick internal coherence check yourself" or
"Re-read the full updated artifact to check for..."; keep the meaning identical,
adjust 1–2 sentences to reduce repetition, and ensure the bullet list and the
sidecar example text remain unchanged in intent and formatting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b88fd02-9e98-4ad7-b57f-9c67085dd4c7
📒 Files selected for processing (3)
docs/bmad-workflow.mdscripts/postinstall.jsskill/auto-bmad-method-check/SKILL.md
- Add sharded epics handling (concatenate shards with headers in briefing) - Remove Implementation Readiness from scope (gate, not persisted artifact) - Add partial pipeline failure handling (preserve changes, continue to next model) - Add explicit sleep 25 polling rule in Step 4a - Add prioritization guidance for large input document sets - Add scope rationale note explaining intentional artifact selectivity Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…catch - Add `text` language tags to bare code fences in bmad-workflow.md - Wrap AUTO_SKILLS readdirSync in try-catch for resilience Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/bmad-workflow.md`:
- Around line 67-71: A blank line is breaking the Markdown table and orphaning
the rows for `sprint-change-proposal-*.md`, `epic-*-retro-*.md`, and
`tech-spec.md`; remove the stray blank line that separates those rows from the
Artifact-to-Input Mapping table so the rows become part of the same table,
ensure the pipe-delimited rows (`| `sprint-change-proposal-*.md` | ... |`, `|
`epic-*-retro-*.md` | ... |`, `| `tech-spec.md` | ... |`) are contiguous with
the table header/separator and that the surrounding pipe alignment/row
separators remain consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4d1e4669-5a4a-42b5-8ce3-b24ac256142b
📒 Files selected for processing (3)
docs/bmad-workflow.mdscripts/postinstall.jsskill/auto-bmad-method-check/SKILL.md
Adds an "Auto-Skills" section to the main sidecar SKILL.md listing all four auto-skills with trigger conditions. Since the main skill appears in Claude's available skills list, this gives Claude passive awareness of auto-skill triggers without requiring new hook infrastructure. Also adds docs/auto-skill-invocation-research.md with findings on how Superpowers achieves reliable skill invocation and recommendations for improving sidecar auto-skill reliability (SessionStart hook, etc.). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/auto-skill-invocation-research.md (1)
149-149: Optional: Minor style improvement.Static analysis flagged this sentence for missing an explicit subject. While readable as-is, you could clarify by rephrasing: "This may not be possible without changes to how Claude Code lists skills."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/auto-skill-invocation-research.md` at line 149, The sentence lacks an explicit subject; replace "May not be possible without changes to how Claude Code lists skills." with "This may not be possible without changes to how Claude Code lists skills." to add a clear subject—update the sentence in the docs/auto-skill-invocation-research.md content where the cons list contains the clause about Claude Code listing skills.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/auto-skill-invocation-research.md`:
- Line 149: The sentence lacks an explicit subject; replace "May not be possible
without changes to how Claude Code lists skills." with "This may not be possible
without changes to how Claude Code lists skills." to add a clear subject—update
the sentence in the docs/auto-skill-invocation-research.md content where the
cons list contains the clause about Claude Code listing skills.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 84887f16-82f9-465b-8b7d-b549a190e9d2
📒 Files selected for processing (2)
docs/auto-skill-invocation-research.mdskill/SKILL.md
Install auto-skills as top-level skill directories (~/.claude/skills/sidecar-auto-*/) instead of nested under sidecar/. This makes them appear in Claude Code's available skills list, enabling both auto-fire on trigger conditions AND manual invocation via slash commands (/sidecar-auto-review, /sidecar-auto-security, etc.). - Rename frontmatter `name` fields to sidecar-auto-* prefix - Update postinstall.js to install to top-level with cleanup of old path - Update main SKILL.md reference to new location Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reflects that Options B and D are now implemented. Updates the gap analysis table, adds verified skills list output, documents the nesting depth discovery, and refocuses remaining recommendations on SessionStart hook and config framework. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds Claude Code hooks as a second triggering path for auto-skills, alongside existing description-matching. Phase 1 delivers shell hooks for auto-security (PreToolUse gate on git commit/push/PR) and auto-bmad-check (PostToolUse on _bmad-output/ writes), plus event collection for Phase 2 stuck-loop and review-completion detection. Includes: - autoSkills config namespace with master + per-skill enable/disable - `sidecar auto-skills` CLI command for managing auto-skill state - Shell hooks (pre-bash.sh, post-tool-use.sh, post-failure.sh) - Hook registration in postinstall with absolute path resolution - Cleanup on npm uninstall (preuninstall.js) - Step 0 config check in all 4 auto-skill SKILL.md files - Research doc analyzing 3 approaches (recommends Approach B) - 23 unit tests for config module Reviewed by Gemini Pro (9 findings) and ChatGPT (12 findings), all accepted fixes applied before commit. Builds on PR jrenaldi79#7 (feat/auto-sidecar-skills) which introduced the four auto-skills. Addresses jrenaldi79#8 (activity monitoring for auto-skill triggers). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Adds three skills that trigger sidecar conversations automatically at natural workflow boundaries, giving the user a second model's perspective without having to think to ask for it:
Also updates
postinstall.jsto copy auto-skill subdirectories to~/.claude/skills/sidecar/duringnpm install.Key design decisions (shared across all three)
sidecar_guide— no hardcoded alias listssidecar_startcalls — defers to sidecar platform default (15 min)sidecar_statusresponse hints (not hardcoded interval)running; treatscomplete|timeout|crashed|error|abortedas terminalincludeContextcorrectly documented as conversation context toggle, not file accesssidecar_start(notsidecar_continue) for auto-unblock rounds to stay in Plan mode (headlesssidecar_continueforces Build mode permcp-server.js:275)Skill-specific highlights
git diff --statfor large diffs, dependency impact flagging, validation context in briefing, hardcoded-value detectionFuture configurability
Each skill documents candidate settings under an
autoSkills.<name>.*namespace in~/.config/sidecar/config.jsonfor when sidecar adds config support for these.Test plan
npm installcopies all three auto-skill files to~/.claude/skills/sidecar/auto-*/SKILL.md🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
Documentation