From 5b98935a19c45126a5bc23392017cef6c54d5861 Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 01:30:55 -0500 Subject: [PATCH 01/20] =?UTF-8?q?feat:=20add=20auto-review=20skill=20?= =?UTF-8?q?=E2=80=94=20post-implementation=20sidecar=20code=20review?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CLAUDE.md | 2 +- skill/auto-review/SKILL.md | 148 +++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 skill/auto-review/SKILL.md diff --git a/CLAUDE.md b/CLAUDE.md index 39a8918..f88293d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -163,7 +163,7 @@ scripts/ ├── generate-icon.js # Generate app icon PNG from SVG source. ├── integration-test.sh ├── list-models.js -├── postinstall.js # Install skill file to ~/.claude/skills/sidecar/ +├── postinstall.js # Install skill files to ~/.claude/skills/sidecar/ ├── test-tools.sh ├── validate-docs.js # * Main entry point. ├── validate-thinking.js diff --git a/skill/auto-review/SKILL.md b/skill/auto-review/SKILL.md new file mode 100644 index 0000000..add9ab1 --- /dev/null +++ b/skill/auto-review/SKILL.md @@ -0,0 +1,148 @@ +--- +name: auto-review +description: > + Use after completing a feature implementation, bug fix, or significant code change — before + claiming the work is done. Offers to spawn a sidecar with a different model to review the + changes for bugs, missed edge cases, and quality issues. TRIGGER when: you have finished + implementing changes, tests pass (or no tests to run), and you are about to tell the user + "done" or summarize the completed work. Do NOT trigger for trivial one-line changes, config + edits, or documentation-only changes. +--- + +# Auto-Review: Post-Implementation Sidecar Review + +## Purpose + +Before claiming work is complete, offer to spawn a headless sidecar to get a second opinion on the changes from a different model. This catches bugs, missed edge cases, and quality issues that you may have overlooked. + +## When This Skill Fires + +- You have finished implementing a feature, bug fix, or significant refactor +- Tests pass (or there are no relevant tests) +- You are about to tell the user the work is done + +**Skip this skill when:** +- Changes are trivial (single-line fix, config change, docs-only) +- You are in the middle of iterating and not yet done +- A sidecar review is already running or was just completed for these changes +- An auto-security sidecar is about to run for the same changes (avoid duplicate scans) +- The `mcp__sidecar__sidecar_start` tool is not available in this environment (sidecar not installed) +- The diff is empty (all changes already committed, nothing to review) + +## Procedure + +### Step 1: Discover available models and prompt the user + +Before presenting the prompt, call `mcp__sidecar__sidecar_guide` to get the configured model alias table. Extract the alias names (e.g., `gemini`, `gpt`, `opus`) from the guide output. If the guide call fails, fall back to: "your configured models (run `sidecar setup` to see them)". + +Then present this confirmation prompt: + +```text +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: +Full provider/model IDs also accepted (e.g., google/gemini-3.1-flash). +``` + +Wait for the user's response: +- **"1"** or **"yes"**: Proceed with the user's configured default model +- **"1 gemini"** or **"1 gpt opus"**: Proceed with the specified model(s). If multiple models given, spawn one sidecar per model (all headless, in parallel). +- **"2"** or **"no"** or **"skip"**: Skip this skill entirely. Deliver your completion message and stop. +- Any other bypass phrase ("skip sidecar", "no sidecars", "we're good", etc.): Skip. + +**Do not proceed past this step without the user's explicit choice.** + +### Step 2: Capture the diff + +Run both `git diff` (unstaged changes) and `git diff --cached` (staged changes). Combine them into a single diff block before checking whether the review diff is empty. + +**If the combined diff is empty**, tell the user there is nothing to review and skip. + +**If the diff exceeds ~500 lines**, do not paste the raw diff. Instead: +- Run `git diff --stat` and include the output — this gives the reviewer a compact map of the blast radius (which files changed and by how much) before it dives into specifics +- Set `includeContext: true` in the sidecar call to pass conversation context (what was implemented and why) +- Provide a summary of what changed in each file +- The sidecar in Plan mode has full `read_file` access to the repository, so this is sufficient + +**If the diff is under ~500 lines**, paste it directly into the briefing. + +### Step 3: Spawn the sidecar(s) + +For **each model** the user selected, call `mcp__sidecar__sidecar_start` with: + +```text +model: +agent: "Plan" +noUi: true +includeContext: false +prompt: +``` + +Notes on parameters: +- **model**: Use the model(s) the user selected in Step 1. If they just said "1" with no model specified, omit this parameter to use their configured default. +- **agent: "Plan"** — read-only and headless-safe. Do not change to Chat (stalls in headless mode). +- **timeout**: Omitted — sidecar uses its platform default (currently 15 minutes). Only override if the user requests a specific timeout. +- **includeContext: false** — briefing is self-contained. Override to `true` for large diffs (see Step 2) to pass conversation context about what was implemented. Note: the Plan agent always has `read_file` access to the repository regardless of this flag — `includeContext` controls conversation context, not file access. + +If spawning multiple sidecars, launch them all in parallel. Save each task ID. + +**Briefing template** — fill in the placeholders: + +```text +## Code Review Request + +**Objective:** Review these code changes for bugs, logic errors, missed edge cases, and code quality issues. + +**Changes:** + + +**Context:** These changes were made to . + +**Validation performed:** + + +**Focus on:** +- Logic errors and off-by-one mistakes +- Missing error handling at trust boundaries +- Edge cases not covered +- Security concerns (injection, auth bypass, data exposure) +- Race conditions or concurrency issues +- Test adequacy: are new/modified tests happy-path only, or do they exercise edge cases and error paths? +- Hardcoded values that should be configurable, dynamic, or derived (magic numbers, hardcoded lists, environment-specific paths) +- Dependency changes: if package.json, Cargo.toml, requirements.txt, or similar manifests are in the diff, check for unnecessary new dependencies, version conflicts, or overly broad version ranges +- Anything that looks wrong or fragile + +**Do NOT flag:** Style preferences, naming opinions, minor nits, or suggestions for additional features. Only report issues you're confident about. + +**Output format:** If issues found, list each with: severity (critical/high/medium), file and location, description, proof of failure (a concrete scenario — e.g., "if input X is null, line Y will throw TypeError" — that demonstrates how the bug manifests; if you cannot construct one, downgrade or drop the finding), and suggested fix. If no issues found, say "No issues identified." +``` + +### Step 4: Continue with your completion message + +Do NOT block on the sidecar. Tell the user the work is done and mention the review is running: + +> "I've completed the implementation. Sidecar review running — I'll share findings when it completes." + +### Step 5: When the sidecar(s) complete + +Poll each sidecar's status using `mcp__sidecar__sidecar_status` with the saved task ID, following the polling cadence indicated by `sidecar_status` responses. Continue polling while status is `running`; treat `complete`, `timeout`, `crashed`, `error`, and `aborted` as terminal. Once terminal, read the output using `mcp__sidecar__sidecar_read`. If multiple models were used, label each result (e.g., "Gemini review:", "GPT review:"). Then: + +- **If substantive issues found:** Surface them as "Second opinion from review sidecar:" and offer to fix. +- **If no issues:** Briefly note: "Sidecar review came back clean — no issues found." +- **If the sidecar failed/timed out:** Mention it briefly and move on. Don't retry. +- **If `mcp__sidecar__sidecar_read` fails:** The sidecar may not be installed or configured. Mention once and move on. + +## Future: Configurable Settings + +The following settings are currently hardcoded in this skill. If sidecar adds an `autoSkills` config namespace, they could be moved to `autoSkills.review.*` in `~/.config/sidecar/config.json`: + +| Setting | Current Default | Description | +|---------|----------------|-------------| +| `autoSkills.review.largeDiffThreshold` | 500 lines | Line count above which diff is summarized instead of pasted | +| `autoSkills.review.maxBriefingChars` | 100,000 | Upper bound on briefing prompt size | +| `autoSkills.review.agent` | Plan | Agent mode for review sidecar (Plan vs Build) | +| `autoSkills.review.timeout` | 15 min | Max time for sidecar to complete review (currently hardcoded in sidecar platform) | From 6c385fa84862ff442aac2e8c143d3b64a6e0d3c3 Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 01:31:08 -0500 Subject: [PATCH 02/20] =?UTF-8?q?feat:=20add=20auto-unblock=20skill=20?= =?UTF-8?q?=E2=80=94=20sidecar=20brainstorming=20when=20stuck=20debugging?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- skill/auto-unblock/SKILL.md | 233 ++++++++++++++++++++++++++++++++++++ 1 file changed, 233 insertions(+) create mode 100644 skill/auto-unblock/SKILL.md diff --git a/skill/auto-unblock/SKILL.md b/skill/auto-unblock/SKILL.md new file mode 100644 index 0000000..dd8d59a --- /dev/null +++ b/skill/auto-unblock/SKILL.md @@ -0,0 +1,233 @@ +--- +name: auto-unblock +description: > + Use when you have attempted 5 or more different approaches to fix a bug, pass a test, or + solve a problem and none have worked. Offers to spawn a sidecar brainstorming session with + a different model to get fresh debugging suggestions via an iterative "have you considered + trying..." loop. TRIGGER when: you are stuck in a debugging loop, have tried multiple fixes + that all failed, and are running out of ideas. Complements superpowers:systematic-debugging + — fires when systematic debugging has been exhausted. A "distinct approach" means a + meaningfully different strategy, not a minor variation (e.g., changing a type annotation + is not a distinct approach from the previous attempt). The attempt threshold (default: 5) + and max brainstorming iterations (default: 20) are configurable. +--- + +# Auto-Unblock: Escape Debugging Ruts with a Sidecar + +## Purpose + +When you're stuck — multiple approaches tried, all failing — offer to spawn a sidecar brainstorming session with a different model. Rather than trying to pass the full codebase (which rarely works well), this skill uses an iterative Q&A loop: the sidecar suggests ideas, Claude evaluates them against what it knows, and feeds back which ones won't work and why — prompting the sidecar to dig deeper. + +## Configuration + +| Setting | Default | Description | +|---------|---------|-------------| +| `attempt_threshold` | **5** | Number of distinct failed approaches before this skill offers to fire | +| `max_iterations` | **20** | Maximum brainstorming round-trips before stopping the loop | + +## When This Skill Fires + +- You have tried **5 or more meaningfully distinct approaches** to fix a bug or pass a test +- Each approach has failed (not just minor variations like tweaking a value) +- You recognize you are going in circles or running out of ideas + +**Skip this skill when:** +- You've only tried 4 or fewer approaches (keep debugging first) +- The failure is a simple typo or config issue you just spotted +- A sidecar is already running for this problem +- The `mcp__sidecar__sidecar_start` tool is not available in this environment (sidecar not installed) + +## Procedure + +### Step 1: Discover available models and prompt the user + +Before presenting the prompt, call `mcp__sidecar__sidecar_guide` to get the configured model alias table. Extract the alias names (e.g., `gemini`, `gpt`, `opus`) from the guide output. If the guide call fails, fall back to: "your configured models (run `sidecar setup` to see them)". + +Acknowledge you're stuck, then present the confirmation prompt: + +```text +I've tried [N] approaches and none have resolved this. + +Sidecar brainstorming session? + + 1. Yes → brainstorm with: [default model] + (or specify models: e.g. "1 gemini gpt" for multi-model brainstorm) + 2. No → skip (I'll keep debugging) + +The sidecar will suggest ideas in an iterative loop. I'll evaluate +each suggestion and feed back why it won't work, prompting deeper +ideas — up to 20 rounds. + +Available models: +Full provider/model IDs also accepted (e.g., google/gemini-3.1-flash). +``` + +Wait for the user's response: +- **"1"** or **"yes"**: Proceed with the user's configured default model +- **"1 gemini"** or **"1 gpt opus"**: Proceed with the specified model(s). If multiple models given, spawn one sidecar per model (all headless, in parallel). +- **"2"** or **"no"** or **"skip"**: Skip this skill. Continue debugging or ask the user for direction. +- Any other bypass phrase ("skip sidecar", "no sidecars", "I'll handle it", etc.): Skip. + +**Do not proceed past this step without the user's explicit choice.** + +### Step 2: Gather context for the initial briefing + +Collect: +- **The problem:** Error messages, failing test output, symptoms +- **Approaches tried:** Be concrete — reference actual commands you ran, files you edited, and the exact error output each attempt produced. Do not paraphrase from memory; review your tool history (shell commands, file edits) to provide accurate evidence of what was tried and what happened. +- **Relevant code:** Key file paths and brief descriptions (the sidecar may have limited local context — describe the relevant architecture concisely rather than assuming it can explore the codebase) +- **Your hypothesis:** What you suspect but haven't confirmed +- **Constraints:** Anything the sidecar should know (e.g., "this is a legacy system, can't upgrade dependencies") + +**Rubber duck effect:** The discipline of gathering and organizing this context often surfaces the answer. If you solve the problem while writing the briefing, skip the sidecar spawn — tell the user what clicked and proceed with the fix. + +**Truncation guidance:** If error output exceeds ~50 lines, include only the first and last 20 lines plus the core error message, and note that output was truncated. Keep the total briefing under ~100,000 characters. + +### Step 3: Spawn the sidecar(s) — initial brainstorm + +For **each model** the user selected, call `mcp__sidecar__sidecar_start` with: + +```text +model: +agent: "Plan" +noUi: true +includeContext: true +prompt: +``` + +Notes on parameters: +- **model**: Use the model(s) the user selected in Step 1. If they just said "1" with no model specified, omit this parameter to use their configured default. +- **agent: "Plan"** — headless-safe, read-only. The sidecar's role is to brainstorm, not to execute. +- **timeout**: Omitted — sidecar uses its platform default (currently 15 minutes). Only override if the user requests a specific timeout. +- **includeContext: true** — passes the parent conversation history to the sidecar, giving it visibility into prior debugging attempts, error output, and tool results. Note: `includeContext` controls conversation context, not file access — the Plan agent always has `read_file` access regardless. + +If spawning multiple sidecars, launch them all in parallel. Save each task ID per model. In subsequent rounds, track each model's task ID independently — each `sidecar_start` returns a new task ID. + +**Initial briefing template** — fill in the placeholders: + +```text +## Debugging Brainstorm — Fresh Ideas Needed + +I'm stuck on a bug and need your help brainstorming. I'll describe the situation and you suggest ideas. I'll tell you which ones won't work and why, and you can refine. You may have limited codebase context, so I'll provide relevant details below. + +**Problem:** + +**Tech stack / architecture:** + +**Approaches already tried** (be specific — include actual commands, edits, and error output): +1. : — Result: +2. : — Result: + + +**Error output:** + + +**Key files involved:** + + +**What I suspect but haven't confirmed:** + + +**Constraints:** + + +**What I need from you:** +1. **Challenge my assumptions.** What am I taking for granted that might be wrong? (e.g., "You assume this library is thread-safe — is it?" or "You assume the config is loaded before this runs — verify that.") List 2-3 hidden assumptions worth testing. +2. **Suggest 3-5 concrete debugging approaches** I haven't tried. For each, explain the reasoning and what it would confirm or rule out. Think laterally — the obvious approaches have failed. +3. **Design a diagnostic probe.** For your most promising suggestion, describe a minimal experiment to isolate the failure — e.g., a specific print/log statement to add, a value to hardcode temporarily, or a minimal reproduction case. Tell me exactly what to do and what the result would confirm or rule out. +4. **Ask me for specifics.** If you need to see a particular code snippet, config file, log output, directory listing, environment variables, or dependency tree to give better suggestions, tell me exactly what to provide in the next round. +``` + +Tell the user: + +> "Sidecar brainstorming session started with [model name(s)]. I'll share the first round of suggestions when it responds." + +### Step 4: Iterative brainstorming loop + +Poll each sidecar's status using `mcp__sidecar__sidecar_status` with the saved task ID, following the polling cadence indicated by `sidecar_status` responses. Continue polling while status is `running`; treat `complete`, `timeout`, `crashed`, `error`, and `aborted` as terminal. Once terminal, read the output using `mcp__sidecar__sidecar_read`. + +**For each suggestion the sidecar returns:** + +1. **Evaluate it** against your knowledge of the codebase and prior attempts +2. **If promising:** Present it to the user and try it. If it works, stop the loop. If it fails, continue to step 3. +3. **If already tried or clearly won't work:** Note why in a follow-up briefing +4. **If the sidecar requests code snippets or files:** Provide them in the next follow-up briefing. The sidecar may have limited codebase context — fulfilling its requests for specific code, config, or logs helps it give better-targeted suggestions. +5. **If needs more info:** Note what info would help + +**User check-in (every 5 rounds):** At rounds 5, 10, and 15, pause the loop and check in with the user before continuing. Briefly summarize what's been tried, what's been learned, and ask whether to continue, redirect, or stop. This prevents the two agents from spiraling into theoretical territory without human course correction. + +**Multi-model narrowing:** If the user selected multiple models, run all of them for round 1. From round 2 onward, continue with only the model that produced the most useful suggestions — this avoids combinatorial fan-out (N models × 20 rounds). Mention which model you're continuing with and why. + +**After evaluating all suggestions from a round**, if none solved the problem and you haven't hit the max iterations (20) and it's not a check-in round, spawn a fresh sidecar using `mcp__sidecar__sidecar_start` (not `sidecar_continue`): + +```text +model: +agent: "Plan" +noUi: true +includeContext: true +prompt: +``` + +**Why `sidecar_start` instead of `sidecar_continue`:** In the current sidecar implementation, headless `sidecar_continue` forces Build mode (full write access), which is inappropriate for a brainstorming-only workflow. Using fresh `sidecar_start` with `agent: "Plan"` keeps each round read-only. The follow-up briefing includes full context from prior rounds, so conversation continuity is preserved in the prompt itself. Save the new task ID returned by each round for status polling. + +After spawning, poll status using `mcp__sidecar__sidecar_status` (following the polling cadence indicated by `sidecar_status` responses) while `running`; treat `complete`, `timeout`, `crashed`, `error`, and `aborted` as terminal. Once terminal, read with `mcp__sidecar__sidecar_read`. + +**Follow-up briefing template:** + +```text +## Round [N] — Here's what happened + +**Suggestions that didn't work:** +- : Tried it — +- : Can't work because +- : Already tried in approach #3 (see above) + +**Code/config you requested:** + + +**New information uncovered:** + + +**What's still unexplained:** + + +**Important:** Be concise in your reasoning — focus on actionable suggestions rather than lengthy analysis. Long responses risk hitting output token limits. + +Please suggest 3-5 more approaches, avoiding the ones above. Go deeper — consider less obvious causes like: +- Environment/config issues +- Race conditions or timing +- Upstream dependencies behaving differently than expected +- Incorrect assumptions about how a library/framework works +- Data-related issues (encoding, format, edge cases) +``` + +**Loop termination conditions** (stop and report to user): +- A suggestion works (problem solved) +- The user says to stop (including during a check-in) +- Max iterations reached (default: 20) +- The sidecar starts repeating suggestions already tried +- The sidecar gives up or says it's out of ideas + +### Step 5: Report results + +After the loop ends, summarize for the user: + +- **If solved:** Report what worked and which round/suggestion cracked it. +- **If max iterations reached:** Summarize the most promising unexplored angles and ask the user for guidance. +- **If sidecar ran out of ideas:** Acknowledge it, list any partially-promising leads, and ask the user what to try next. +- **If the sidecar failed/timed out:** Mention it briefly and continue debugging or ask the user. +- **If `mcp__sidecar__sidecar_read` fails:** Sidecar may not be installed. Mention once and continue. + +## Future: Configurable Settings + +The following settings are currently hardcoded in this skill. If sidecar adds an `autoSkills` config namespace, they could be moved to `autoSkills.unblock.*` in `~/.config/sidecar/config.json`: + +| Setting | Current Default | Description | +|---------|----------------|-------------| +| `autoSkills.unblock.attemptThreshold` | 5 | Failed approaches before skill offers to fire | +| `autoSkills.unblock.maxIterations` | 20 | Maximum brainstorming round-trips | +| `autoSkills.unblock.userCheckinInterval` | 5 | Rounds between mandatory user check-ins | +| `autoSkills.unblock.suggestionsPerRound` | 3-5 | Number of suggestions requested per round | +| `autoSkills.unblock.maxBriefingChars` | 100,000 | Upper bound on briefing prompt size | +| `autoSkills.unblock.errorTruncationLines` | 50 | Max lines of error output in briefing | +| `autoSkills.unblock.timeout` | 15 min | Max time per sidecar round (currently hardcoded in sidecar platform) | From 9043a0dd924bc0b60a6d90e842badd8902f20ba1 Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 01:31:18 -0500 Subject: [PATCH 03/20] =?UTF-8?q?feat:=20add=20auto-security=20skill=20?= =?UTF-8?q?=E2=80=94=20pre-commit=20sidecar=20security=20scan?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- skill/auto-security/SKILL.md | 152 +++++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 skill/auto-security/SKILL.md diff --git a/skill/auto-security/SKILL.md b/skill/auto-security/SKILL.md new file mode 100644 index 0000000..ce9c181 --- /dev/null +++ b/skill/auto-security/SKILL.md @@ -0,0 +1,152 @@ +--- +name: auto-security +description: > + Use when the user asks to commit changes, push code, or create a pull request. Offers to + spawn a sidecar to audit the diff for security vulnerabilities before the commit/push/PR + proceeds. TRIGGER when: user says "commit", "push", "create a PR", "open a PR", or you are + about to run git commit, git push, or gh pr create in conversational flow. Does NOT trigger + on slash commands like /commit or /commit-push-pr (those execute in isolated contexts). + Complements commit-commands:commit — runs before the commit proceeds. +--- + +# Auto-Security: Pre-Commit Security Scan via Sidecar + +## Purpose + +Before committing, pushing, or creating a PR, offer to spawn a headless sidecar to audit the changes for security vulnerabilities. Catches issues like hardcoded secrets, injection flaws, and auth bypass before they reach the repository. + +## When This Skill Fires + +- The user asks to commit, push, or create a PR in conversation (e.g., "commit my changes", "let's push this", "create a PR") +- You are about to run `git commit`, `git push`, or `gh pr create` in normal conversational flow + +**This skill does NOT fire on:** +- Slash commands (`/commit`, `/commit-push-pr`) — those execute in their own isolated context and cannot be intercepted by skills + +**Skip this skill when:** +- Changes contain zero executable code (only `*.md` files, only code comments, only whitespace) +- Changes are trivially safe (bumping a version number in `package.json` with no other changes) +- A security sidecar already ran for these exact changes +- An auto-review sidecar already ran for these same changes (its briefing covers security concerns) +- The `mcp__sidecar__sidecar_start` tool is not available in this environment (sidecar not installed) +- The diff is empty (nothing staged or unstaged to commit) + +## Procedure + +### Step 1: Discover available models and prompt the user + +Before presenting the prompt, call `mcp__sidecar__sidecar_guide` to get the configured model alias table. Extract the alias names (e.g., `gemini`, `gpt`, `opus`) from the guide output. If the guide call fails, fall back to: "your configured models (run `sidecar setup` to see them)". + +Then present this confirmation prompt: + +```text +Sidecar security scan before commit? + + 1. Yes → send to: [default model] + (or specify models: e.g. "1 gemini gpt" for multi-model audit) + 2. No → skip, proceed with commit + +Available models: +Full provider/model IDs also accepted (e.g., google/gemini-3.1-flash). +``` + +Wait for the user's response: +- **"1"** or **"yes"**: Proceed with the security scan before committing +- **"1 gemini"** or **"1 gpt opus"**: Proceed with the specified model(s). If multiple models given, spawn one sidecar per model (all headless, in parallel). +- **"2"** or **"no"** or **"skip"**: Skip the scan and proceed directly with the commit/push/PR. +- Any other bypass phrase ("skip sidecar", "no sidecars", "just commit", "no scan", "commit without review", etc.): Skip. + +**Do not proceed past this step without the user's explicit choice.** If the user chooses to skip, go straight to the commit flow. + +### Step 2: Capture the diff + +Choose the right diff for the operation: +- **For commits:** Run both `git diff` (unstaged) and `git diff --cached` (staged). Combine them into a single diff for the audit — this audits all changes, not just staged ones, to avoid silently missing unstaged code. Note: this is intentionally broader than strict Git staging semantics; if the user wants to audit only staged changes, they can say so. +- **For PRs/pushes:** Resolve the base branch using this fallback order: (1) `git symbolic-ref refs/remotes/origin/HEAD` to get the remote's default branch, (2) `@{upstream}` if no `origin` remote exists, (3) check if `main` or `master` branches exist locally, (4) ask the user. Then run `git diff $(git merge-base HEAD )...HEAD` to capture the full branch diff, not just the current working tree. This catches security issues from earlier commits on the branch. + +**If the diff is empty**, tell the user there are no changes to audit and skip to the commit flow. For commits, this means both staged and unstaged diffs are empty. For PRs/pushes, this means the branch diff against the default branch is empty (no commits to push). + +**If the diff exceeds ~500 lines**, prioritize files by security relevance rather than truncating arbitrarily. Include in full: files handling authentication/authorization, user input processing, API route definitions, database queries, cryptographic operations, CI/CD configuration (`.github/workflows/`, `.gitlab-ci.yml`, `Jenkinsfile`, etc.), and any files containing secrets-adjacent patterns (environment config, credential setup). Deprioritize: tests, documentation, static assets, generated code. Truncate to ~100,000 characters max. For very large diffs, provide file paths and change summaries instead of raw diff — the Plan agent has `read_file` access and can read the source files directly. + +### Step 3: Spawn the sidecar(s) — BEFORE starting the commit + +**Important: Run the security scan FIRST, before executing the commit.** Do not start the commit in parallel. Wait for the scan to complete (or timeout) before proceeding. This ensures critical vulnerabilities are caught before they enter the repository. + +For **each model** the user selected, call `mcp__sidecar__sidecar_start` with: + +```text +model: +agent: "Plan" +noUi: true +includeContext: true +prompt: +``` + +Notes on parameters: +- **model**: Use the model(s) the user selected in Step 1. If they just said "1" with no model specified, omit this parameter to use their configured default. +- **agent: "Plan"** — read-only and headless-safe. Do not change to Chat (stalls in headless mode). +- **timeout**: Omitted — sidecar uses its platform default (currently 15 minutes). Only override if the user requests a specific timeout. +- **includeContext: true** — passes the parent conversation history to the sidecar, giving it visibility into prior discussion, error output, and what was implemented. Note: the Plan agent always has `read_file` access to the repository regardless of this flag — `includeContext` controls conversation context, not file access. + +If spawning multiple sidecars, launch them all in parallel. Save each task ID. + +Tell the user: + +> "Running security scan with [model name(s)]. This usually takes a minute or two." + +**Briefing template** — fill in the placeholders: + +```text +## Security Audit — Pre-Commit Review + +**Objective:** Audit these code changes for security vulnerabilities. + +**Changes:** + + +**Check for:** +- Injection vulnerabilities (SQL, command, XSS, template injection) +- Authentication/authorization bypass +- Hardcoded secrets, API keys, tokens, or credentials +- Insecure data handling (PII exposure, missing encryption, logging sensitive data) +- OWASP Top 10 issues +- Unsafe deserialization +- Path traversal +- Missing input validation at trust boundaries +- Insecure cryptographic practices +- SSRF, open redirects +- CI/CD pipeline tampering (modified workflow files, build scripts, or deployment configs) +- New dependencies that look suspicious (typosquatting, unnecessary permissions, unmaintained packages) + +**Output format:** List only confirmed or high-confidence issues. For each: +- **Severity:** critical / high / medium +- **File and line:** where the issue is +- **Description:** what's wrong +- **Attack vector:** how an attacker would exploit this (be specific — describe the input, the path through the code, and the impact). If you cannot articulate a concrete attack vector, downgrade or drop the finding. +- **Suggested fix:** how to resolve it + +If no security issues found, say: "No security issues identified." +``` + +### Step 4: Poll for completion, then proceed + +After spawning, poll each sidecar's status using `mcp__sidecar__sidecar_status` with the saved task ID, following the polling cadence indicated by `sidecar_status` responses. Continue polling while status is `running`; treat `complete`, `timeout`, `crashed`, `error`, and `aborted` as terminal. Only then read the output using `mcp__sidecar__sidecar_read`. If multiple models were used, label each result (e.g., "Gemini audit:", "GPT audit:"). Then: + +- **If critical/high severity issues found:** **Do not proceed with the commit.** Surface the findings: "Security scan found issues that should be addressed before committing:" — then list them and offer to fix. +- **If medium severity issues found:** Surface them as warnings and let the user decide: "Security scan flagged some medium-severity items. Want to address these before committing, or proceed?" +- **If no issues:** Proceed with the commit normally. Briefly note: "Security scan came back clean." +- **If the sidecar timed out:** Tell the user the scan timed out and ask if they want to proceed anyway or wait for a retry. +- **If `mcp__sidecar__sidecar_read` fails:** Sidecar may not be installed. Mention once and proceed with the commit. + +After resolving the scan results, proceed with the normal commit/push/PR flow. + +## Future: Configurable Settings + +The following settings are currently hardcoded in this skill. If sidecar adds an `autoSkills` config namespace, they could be moved to `autoSkills.security.*` in `~/.config/sidecar/config.json`: + +| Setting | Current Default | Description | +|---------|----------------|-------------| +| `autoSkills.security.largeDiffThreshold` | 500 lines | Line count above which diff is summarized by security relevance | +| `autoSkills.security.maxBriefingChars` | 100,000 | Upper bound on briefing prompt size | +| `autoSkills.security.blockingMode` | sequential | Whether scan blocks commit (sequential) or runs in parallel | +| `autoSkills.security.timeout` | 15 min | Max time for sidecar to complete scan (currently hardcoded in sidecar platform) | From 34aba520cdc378b618ad3419e425e0f100bc2a37 Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 01:31:27 -0500 Subject: [PATCH 04/20] fix: postinstall copies auto-skill subdirectories Updates postinstall.js to also install auto-review, auto-unblock, and auto-security skills to ~/.claude/skills/sidecar//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 --- scripts/postinstall.js | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/scripts/postinstall.js b/scripts/postinstall.js index 2f6b7d4..5056649 100644 --- a/scripts/postinstall.js +++ b/scripts/postinstall.js @@ -13,10 +13,13 @@ const path = require('path'); const os = require('os'); const { execFileSync } = require('child_process'); -const SKILL_SOURCE = path.join(__dirname, '..', 'skill', 'SKILL.md'); +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 = ['auto-review', 'auto-unblock', 'auto-security']; + const MCP_CONFIG = { command: 'npx', args: ['-y', 'claude-sidecar@latest', 'mcp'] }; /** @@ -50,7 +53,7 @@ function addMcpToConfigFile(configPath, name, config) { return status; } -/** Install skill file to ~/.claude/skills/sidecar/ */ +/** Install skill files to ~/.claude/skills/sidecar/ */ function installSkill() { try { fs.mkdirSync(SKILL_DEST_DIR, { recursive: true }); @@ -59,6 +62,18 @@ function installSkill() { } catch (err) { console.error(`[claude-sidecar] Warning: Could not install skill: ${err.message}`); } + + for (const name of AUTO_SKILLS) { + try { + const src = path.join(SKILL_DIR, name, 'SKILL.md'); + const destDir = path.join(SKILL_DEST_DIR, name); + fs.mkdirSync(destDir, { recursive: true }); + fs.copyFileSync(src, path.join(destDir, 'SKILL.md')); + console.log(`[claude-sidecar] Skill installed: ${name}`); + } catch (err) { + console.error(`[claude-sidecar] Warning: Could not install ${name} skill: ${err.message}`); + } + } } /** Register MCP server in Claude Code config */ From 020a28559d53ca55c2c596ddfb6cdea78bc46e0a Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 09:14:41 -0500 Subject: [PATCH 05/20] fix: derive AUTO_SKILLS dynamically from filesystem 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 --- scripts/postinstall.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/postinstall.js b/scripts/postinstall.js index 5056649..b9648eb 100644 --- a/scripts/postinstall.js +++ b/scripts/postinstall.js @@ -18,7 +18,11 @@ 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 = ['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(); const MCP_CONFIG = { command: 'npx', args: ['-y', 'claude-sidecar@latest', 'mcp'] }; From e4f257b03a30eec949c42c3982e252dd6e891310 Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 10:19:46 -0500 Subject: [PATCH 06/20] =?UTF-8?q?feat:=20add=20auto-bmad-method-check=20sk?= =?UTF-8?q?ill=20=E2=80=94=20sidecar=20review=20of=20BMAD=20artifacts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/bmad-workflow.md | 111 +++++++++++ skill/auto-bmad-method-check/SKILL.md | 276 ++++++++++++++++++++++++++ 2 files changed, 387 insertions(+) create mode 100644 docs/bmad-workflow.md create mode 100644 skill/auto-bmad-method-check/SKILL.md diff --git a/docs/bmad-workflow.md b/docs/bmad-workflow.md new file mode 100644 index 0000000..b303129 --- /dev/null +++ b/docs/bmad-workflow.md @@ -0,0 +1,111 @@ +# BMAD-METHOD Workflow Reference + +Reference for the auto-bmad-method-check skill. Captures all workflows, artifacts, dependencies, and checkpoints. + +## Phases & Workflows + +| Phase | Workflow | Agent | Input Dependencies | Output Artifact | Checkpoint | +|---|---|---|---|---|---| +| **1: Analysis** (optional) | Brainstorming | Mary (Analyst) | None | `brainstorming-report.md` | Human reviews report | +| | Market Research | Mary | None (needs web search) | `research-*.md` | Human reviews findings | +| | Domain Research | Mary | None | `research-*.md` | Human reviews findings | +| | Technical Research | Mary | None | `research-*.md` | Human reviews findings | +| | Create Product Brief | Mary | Brainstorming/research (optional) | `product-brief.md` | Human reviews brief | +| **2: Planning** (required) | Create PRD | John (PM) | Product brief (optional) | `PRD.md` | Human reviews PRD | +| | Validate PRD | John | `PRD.md` | Validation report | Human reviews findings | +| | Edit PRD | John | `PRD.md` | Updated `PRD.md` | Human reviews edits | +| | Create UX Design | Sally (UX) | `PRD.md` | `ux-design-specification.md` | Human reviews UX spec | +| **3: Solutioning** | Create Architecture | Winston (Architect) | `PRD.md` (+ UX spec optional) | `architecture.md` + ADRs | Human reviews arch decisions | +| | Create Epics & Stories | John | `PRD.md` + `architecture.md` | `epics.md` (or sharded) | Human reviews breakdown | +| | Check Implementation Readiness | Winston/John | PRD + Architecture + Epics (+ UX) | PASS / CONCERNS / FAIL | **Gate** — must pass before Phase 4 | +| **4: Implementation** | Sprint Planning | Bob (SM) | Epics files | `sprint-status.yaml` | One-time setup | +| | Sprint Status | Bob | `sprint-status.yaml` | Status summary + risk flags | Informational | +| | Create Story | Bob | Sprint status + Epics + all artifacts | `story-{e}-{s}-{slug}.md` | Human reviews story before dev | +| | Dev Story | Amelia (Dev) | Story file (status: ready-for-dev) | Working code + tests | HALTs on blockers | +| | Code Review | Amelia/Barry | Story file + git changes | Approved or Changes Requested | Human decides on action items | +| | Correct Course | Bob/John | PRD + Epics + sprint context | `sprint-change-proposal-*.md` | Human approves proposal | +| | Retrospective | Bob | All completed stories in epic | `epic-{N}-retro-{date}.md` | Significant Discovery Alert if assumptions shifted | +| **Quick Flow** | Quick Spec | Barry (Solo Dev) | None | `tech-spec.md` | Human reviews spec | +| | Quick Dev | Barry | `tech-spec.md` or direct instructions | Working code + tests | Self-review then human | +| **Cross-cutting** | Generate Project Context | Mary | Codebase scan | `project-context.md` | Used by 7+ workflows | +| | BMad Help | Any | Project state inspection | Next-step guidance | Runs after every workflow | + +## Dependency Chain + +``` +Brainstorming/Research ──> Product Brief ──> PRD ──> UX Design (optional) + ├──> Architecture + └──> Architecture ──> Epics/Stories + │ + Implementation Readiness <┘ (GATE) + │ + Sprint Planning (once) + │ + Create Story ──> Dev Story ──> Code Review + ^ │ + └─────── (next story) ────────┘ + │ + Epic complete ──> Retrospective ───┘ +``` + +## Artifact-to-Input Mapping + +Used by auto-bmad-method-check to determine which input documents to include in sidecar reviews. + +| Output Artifact | Input Documents | +|---|---| +| `brainstorming-report.md` | None (freeform ideation) | +| `research-*.md` | None (primary research) | +| `product-brief.md` | `brainstorming-report.md`, `research-*.md` (if they exist) | +| `PRD.md` | `product-brief.md` (if exists) | +| `ux-design-specification.md` | `PRD.md` | +| `architecture.md` | `PRD.md`, `ux-design-specification.md` (if exists) | +| `epics.md` | `PRD.md`, `architecture.md` | +| Implementation Readiness | `PRD.md`, `architecture.md`, `epics.md`, `ux-design-specification.md` (if exists) | +| `sprint-status.yaml` | `epics.md` | +| `story-*.md` | `epics.md`, `PRD.md`, `architecture.md`, `sprint-status.yaml` | + +| `sprint-change-proposal-*.md` | `PRD.md`, `epics.md`, affected `story-*.md` files | +| `epic-*-retro-*.md` | All `story-*.md` in that epic, previous retro (if exists) | +| `tech-spec.md` | None (Quick Flow — standalone) | + +## Agents + +| Agent | Name | Personality | Primary Workflows | +|---|---|---|---| +| Analyst | Mary | "Excited treasure hunter" | Brainstorming, Research, Product Brief, Project Context | +| Product Manager | John | "Asks WHY relentlessly like a detective" | PRD, Validate/Edit PRD, Epics, Readiness Check, Course Correction | +| Architect | Winston | "Calm, pragmatic tones" | Architecture, Readiness Check | +| Scrum Master | Bob | "Crisp, checklist-driven, zero ambiguity tolerance" | Sprint Planning/Status, Create Story, Retrospective, Course Correction | +| Developer | Amelia | "Ultra-succinct, speaks in file paths" | Dev Story, Code Review | +| UX Designer | Sally | "Paints pictures with words" | UX Design | +| Quick Flow Solo Dev | Barry | "Direct, no fluff, just results" | Quick Spec, Quick Dev, Code Review | +| Tech Writer | Paige | "Patient educator" | Document Project | + +## Key Design Principles + +- **Micro-file architecture**: Steps loaded one at a time to prevent LLM "lost in middle" issues +- **Human must approve** every step transition — no autonomous progression +- **Fresh conversations** per workflow to keep context clean +- **Scale-adaptive**: Quick Flow (1-15 stories), BMad Method (10-50+), Enterprise (30+) +- **`project-context.md`** acts as the "constitution" for consistent AI agent behavior + +## Standard Artifact Locations + +``` +_bmad-output/ + planning-artifacts/ + brainstorming-report.md + product-brief.md + research-*.md + PRD.md + ux-design-specification.md + architecture.md + epics.md (or epics/ directory) + sprint-change-proposal-*.md + implementation-artifacts/ + sprint-status.yaml + story-*.md + epic-*-retro-*.md + project-context.md +``` diff --git a/skill/auto-bmad-method-check/SKILL.md b/skill/auto-bmad-method-check/SKILL.md new file mode 100644 index 0000000..5eb3f7d --- /dev/null +++ b/skill/auto-bmad-method-check/SKILL.md @@ -0,0 +1,276 @@ +--- +name: auto-bmad-method-check +description: > + Use when a BMAD-METHOD workflow has just produced an output artifact (PRD, architecture doc, + epics, story file, etc.) and the user has not yet finalized it or moved to the next workflow + step. Offers to spawn sidecar(s) for a second-opinion review of the artifact along with its + input documents, then presents each model's suggestions sequentially for Claude to evaluate + and the user to approve. TRIGGER when: a BMAD planning or implementation artifact has just + been written or substantially updated in _bmad-output/, the artifact is substantive (not a + trivial edit), and the user is at a natural checkpoint before proceeding. Does NOT trigger + if _bmad/bmm/config.yaml is missing (BMAD-METHOD not installed), if the artifact is a minor + edit, or if a sidecar review already ran for this artifact version. +--- + +# Auto-BMAD-Method-Check: Artifact Review via Sidecar + +## Purpose + +When a BMAD-METHOD workflow produces a key artifact — PRD, architecture doc, epics, story file, or other checkpoint document — offer to send it to one or more sidecar models for a second opinion before the user finalizes and moves to the next workflow step. Claude evaluates each model's suggestions against the artifact and its input documents, presents an informed opinion, and the user decides what to apply. Multiple models are spawned and processed sequentially — each successive model reviews the artifact *after* changes from prior models have been applied, creating a genuine multi-model refinement pipeline. + +## Artifact Scope + +This skill covers planning and key implementation artifacts where a second opinion catches costly mistakes: + +| Artifact | Phase | Input Documents | +|---|---|---| +| `product-brief.md` | Analysis | `brainstorming-report.md`, `research-*.md` (if they exist) | +| `PRD.md` | Planning | `product-brief.md` (if exists) | +| `ux-design-specification.md` | Planning | `PRD.md` | +| `architecture.md` | Solutioning | `PRD.md`, `ux-design-specification.md` (if exists) | +| `epics.md` (or sharded `epics/` dir) | Solutioning | `PRD.md`, `architecture.md` | +| Implementation Readiness result | Solutioning | `PRD.md`, `architecture.md`, `epics.md`, `ux-design-specification.md` (if exists) | +| `story-*.md` | Implementation | `epics.md`, `PRD.md`, `architecture.md`, `sprint-status.yaml` | +| `sprint-change-proposal-*.md` | Implementation | `PRD.md`, `epics.md`, affected `story-*.md` files | +| `epic-*-retro-*.md` | Implementation | All `story-*.md` in that epic, previous retro (if exists) | + +## When This Skill Fires + +- A BMAD workflow has just written or substantially updated one of the artifacts above +- The user has not yet finalized the artifact or moved to the next workflow step +- The artifact represents a natural BMAD checkpoint where human review is expected + +**Skip this skill when:** +- `_bmad/bmm/config.yaml` does not exist in the project root (BMAD-METHOD not installed — this skill is designed specifically for the BMAD-METHOD framework at https://github.com/bmad-code-org/BMAD-METHOD) +- `_bmad-output/` directory does not exist (no artifact output location) +- The artifact change is trivial (typo fix, formatting, comment edit) +- A sidecar review already ran for this artifact version +- The user is in Quick Flow (`tech-spec.md`, `quick-dev`) — those have their own review cycle +- The `mcp__sidecar__sidecar_start` tool is not available in this environment (sidecar not installed) + +## Procedure + +### Step 0: Verify BMAD-METHOD installation + +Before anything else, check that the BMAD-METHOD is installed in the current project: + +1. Look for `_bmad/bmm/config.yaml` in the project root +2. Look for `_bmad-output/` directory + +If either is missing, do not fire this skill. Do not mention it to the user — silently skip. + +### Step 1: Discover available models and prompt the user + +Before presenting the prompt, call `mcp__sidecar__sidecar_guide` to get the configured model alias table. Extract the alias names (e.g., `gemini`, `gpt`, `opus`) from the guide output. If the guide call fails, fall back to: "your configured models (run `sidecar setup` to see them)". + +Identify the artifact that was just produced and present the confirmation prompt: + +```text +BMAD artifact review — [artifact name]? + +This [artifact type] is ready for second-opinion review before you finalize. + + 1. Yes → send to: [default model] + (or specify models: e.g. "1 gemini gpt" for multi-model sequential review) + 2. No → skip and finalize + +If multiple models selected, each reviews sequentially — later models +see improvements from earlier rounds. I'll evaluate each suggestion +and give my opinion before you decide. + +Available models: +Full provider/model IDs also accepted (e.g., google/gemini-3.1-flash). +``` + +Wait for the user's response: +- **"1"** or **"yes"**: Proceed with the user's configured default model +- **"1 gemini"** or **"1 gpt opus"**: Proceed with the specified model(s). If multiple models given, spawn and process them sequentially in the order specified — each model reviews the artifact after changes from prior models have been applied. +- **"2"** or **"no"** or **"skip"**: Skip this skill. Proceed with finalization. +- Any other bypass phrase ("skip sidecar", "no review", "finalize", etc.): Skip. + +**Do not proceed past this step without the user's explicit choice.** + +### Step 2: Gather context + +1. **Read the output artifact** in full. This is the primary document under review — never truncate it. + +2. **Resolve input documents.** Use this priority order: + - First, check if the artifact itself references its input documents (many BMAD artifacts cite their sources in headers, metadata, or "References" sections) + - If no references found, fall back to the Artifact Scope table above + - Scan `_bmad-output/planning-artifacts/` and `_bmad-output/implementation-artifacts/` for matching files + - For glob patterns (e.g., `research-*.md`, `story-*.md`), collect all matching files + +3. **Collect input document paths and descriptions.** For each input document, note its file path and a one-line description of what it contains (e.g., "`_bmad-output/planning-artifacts/PRD.md` — functional and non-functional requirements"). Do NOT read or paste their full content into the briefing — the sidecar has `read_file` access and will read them directly. This saves significant context in the parent session. + +4. **Truncation guidance:** The output artifact should always be included in full (it's what's being reviewed). If even the artifact alone exceeds ~80,000 characters, truncate to the most relevant sections and note what was omitted so the sidecar can use `read_file` for the rest. + +### Step 3: Spawn the first sidecar + +Spawn a sidecar for the **first** (or only) model the user selected. Call `mcp__sidecar__sidecar_start` with: + +```text +model: +agent: "Plan" +noUi: true +includeContext: true +prompt: +``` + +Notes on parameters: +- **model**: Use the first model the user selected. If they just said "1" with no model specified, omit this parameter to use their configured default. +- **agent: "Plan"** — read-only and headless-safe. The sidecar reviews but does not modify files. +- **timeout**: Omitted — sidecar uses its platform default (currently 15 minutes). Only override if the user requests a specific timeout. +- **includeContext: true** — passes the parent conversation history to the sidecar, giving it visibility into the workflow discussion that produced the artifact. The Plan agent also has `read_file` access to the repository for additional context. + +Save the task ID. If the user selected multiple models, subsequent models are spawned in Step 4e **after** the current model's approved changes have been applied to the artifact. This ensures each model reviews the improved version, not the original. + +**Briefing template** — fill in the placeholders: + +```text +## BMAD Artifact Review — [Artifact Type] + +**Objective:** Review this BMAD-METHOD artifact for completeness, internal consistency, +alignment with upstream documents, and quality. This artifact was produced by the +[workflow name] workflow and is at a checkpoint before the user finalizes it. + +**Artifact under review:** + + +**Input documents (upstream context) — read these with your file tools:** + +Use your read_file tool to examine these documents. They contain the upstream +decisions and requirements that this artifact must align with. + +**BMAD-METHOD context:** +This project uses the BMAD-METHOD framework (https://github.com/bmad-code-org/BMAD-METHOD). +The artifact above was produced by the [agent name] agent during the [phase name] phase. +The next workflow step will be [next step description]. + +**Review this artifact for:** + +1. **Completeness** — Are there gaps, missing sections, or requirements from the input + documents that aren't addressed? Are there implicit assumptions that should be explicit? + +2. **Internal consistency** — Does the artifact contradict itself? Are terms used + consistently? Do sections reference each other correctly? + +3. **Upstream alignment** — Does this artifact faithfully reflect decisions made in its + input documents? Flag any drift, contradiction, or silent omission of upstream + requirements. + +4. **Quality and clarity** — Is the writing clear and unambiguous? Would the next + agent/workflow in the BMAD pipeline be able to use this artifact effectively? + Are acceptance criteria testable? Are architecture decisions justified? + +5. **Risk and gaps** — What risks or edge cases does this artifact not address? + What questions should be resolved before moving to the next phase? + +**Do NOT flag:** Stylistic preferences, formatting opinions, or suggestions for +scope expansion beyond what the input documents require. + +**Output format:** For each finding: +- **Category:** completeness | consistency | alignment | quality | risk +- **Severity:** critical (blocks next phase) | important (should fix) | suggestion (nice to have) +- **Location:** section or line reference in the artifact +- **Finding:** what's wrong or missing +- **Evidence:** quote from input document or artifact that supports this finding +- **Suggested fix:** concrete recommendation + +If no issues found, say: "No issues identified — artifact is ready to finalize." +``` + +Tell the user: + +> "Sidecar review started with [model name(s)]. I'll present findings for each model as they complete." + +### Step 4: Sequential review loop + +Process models in the order the user specified them. For each model: + +**4a. Poll and read results** + +Poll the sidecar's status using `mcp__sidecar__sidecar_status` with the saved task ID, following the polling cadence indicated by `sidecar_status` responses. Continue polling while status is `running`; treat `complete`, `timeout`, `crashed`, `error`, and `aborted` as terminal. Once terminal, read the output using `mcp__sidecar__sidecar_read`. + +**4b. Claude evaluates each suggestion** + +For each finding the sidecar returned, evaluate it against: +- The artifact content — is the finding accurate? +- The input documents — does the suggestion align with upstream decisions? +- BMAD method conventions — does it follow the framework's patterns and expectations? +- Prior models' changes (if this is Model 2+) — does it conflict with already-applied changes? + +**4c. Present to user with Claude's opinion** + +Format the presentation as: + +```text +Review from [Model Name]: + +Finding 1: [summary] + Sidecar says: [the suggestion] + My assessment: [Agree / Disagree / Partially agree] — [reasoning] + Recommendation: [Apply / Skip / Modify to...] + +Finding 2: ... + ... + +Summary: [N] findings — I recommend applying [X], skipping [Y]. +Which changes would you like to apply? (all / none / list numbers, e.g. "1 3 5") +``` + +**4d. Apply approved changes** + +For each change the user approves: +- Update the artifact in place +- If a suggestion needs modification (user or Claude adjusted it), apply the modified version + +**4e. Spawn next model (if any)** + +If there are more models to process: +- Re-read the now-updated artifact (with all approved changes applied) +- Spawn the next model's sidecar using `mcp__sidecar__sidecar_start` with the same parameters as Step 3, but with the **updated artifact content** in the briefing +- This ensures the next model reviews the improved version, not the original +- Poll, read, and evaluate the next model's results the same way (repeat 4a–4d) +- When presenting: flag explicitly if this model raised something the previous model missed, or contradicts a change already applied + +### Step 5: Consolidation check + +After all models have been processed and changes applied: + +**If changes were minor (1-3 small edits):** Perform a quick internal coherence check yourself — re-read the updated artifact and verify the edits flow naturally. Report briefly: "Applied changes look coherent. Ready to finalize." + +**If changes were substantial (4+ edits, or structural changes across multiple sections):** Re-read the full updated artifact and check for: +- New contradictions or redundancies introduced by the edits +- Sections that no longer flow coherently after patching in changes +- Content that was inadvertently weakened or removed + +If you spot issues, fix them and note what you adjusted. If the artifact needs deeper review after heavy edits, offer a sidecar consolidation round: + +```text +This artifact had substantial edits from [N] reviews. Want a final +consolidation review with [model] to check coherence, or finalize as-is? +``` + +If the user wants a sidecar round, spawn one with the updated artifact only (no input documents) focused on internal consistency. Process results the same way: evaluate, present, user approves. + +### Step 6: Completion + +After the review process (with or without consolidation): + +> "Artifact review complete. [Summarize: N changes applied from M models, or no changes needed.] Ready to proceed with the next BMAD workflow step." + +**If all models failed/timed out:** Mention briefly and let the user decide whether to retry or finalize as-is. + +**If `mcp__sidecar__sidecar_read` fails:** Sidecar may not be installed or configured. Mention once and proceed. + +## Future: Configurable Settings + +The following settings are currently hardcoded in this skill. If sidecar adds an `autoSkills` config namespace, they could be moved to `autoSkills.bmadMethodCheck.*` in `~/.config/sidecar/config.json`: + +| Setting | Current Default | Description | +|---------|----------------|-------------| +| `autoSkills.bmadMethodCheck.maxBriefingChars` | 80,000 | Upper bound on total briefing content before summarizing inputs | +| `autoSkills.bmadMethodCheck.agent` | Plan | Agent mode for review sidecar | +| `autoSkills.bmadMethodCheck.timeout` | 15 min | Max time per sidecar round (currently hardcoded in sidecar platform) | +| `autoSkills.bmadMethodCheck.artifactDir` | `_bmad-output/` | Root directory for BMAD artifacts | +| `autoSkills.bmadMethodCheck.skipQuickFlow` | true | Whether to skip Quick Flow artifacts | From 5165ae23b3043f9c670283519cfca751c450e183 Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 10:30:16 -0500 Subject: [PATCH 07/20] fix: address ChatGPT review findings for auto-bmad-method-check - 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 --- skill/auto-bmad-method-check/SKILL.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/skill/auto-bmad-method-check/SKILL.md b/skill/auto-bmad-method-check/SKILL.md index 5eb3f7d..f4fb1dc 100644 --- a/skill/auto-bmad-method-check/SKILL.md +++ b/skill/auto-bmad-method-check/SKILL.md @@ -29,11 +29,12 @@ This skill covers planning and key implementation artifacts where a second opini | `ux-design-specification.md` | Planning | `PRD.md` | | `architecture.md` | Solutioning | `PRD.md`, `ux-design-specification.md` (if exists) | | `epics.md` (or sharded `epics/` dir) | Solutioning | `PRD.md`, `architecture.md` | -| Implementation Readiness result | Solutioning | `PRD.md`, `architecture.md`, `epics.md`, `ux-design-specification.md` (if exists) | | `story-*.md` | Implementation | `epics.md`, `PRD.md`, `architecture.md`, `sprint-status.yaml` | | `sprint-change-proposal-*.md` | Implementation | `PRD.md`, `epics.md`, affected `story-*.md` files | | `epic-*-retro-*.md` | Implementation | All `story-*.md` in that epic, previous retro (if exists) | +**Scope rationale:** This table is intentionally selective — it covers high-leverage checkpoint artifacts where a second opinion catches costly mistakes. Lower-leverage artifacts (`brainstorming-report.md`, `research-*.md`, `sprint-status.yaml`, `project-context.md`) and non-persisted gates (Implementation Readiness) are excluded. ADRs produced alongside `architecture.md` are reviewed as part of the architecture artifact, not separately. + ## When This Skill Fires - A BMAD workflow has just written or substantially updated one of the artifacts above @@ -99,10 +100,13 @@ Wait for the user's response: - If no references found, fall back to the Artifact Scope table above - Scan `_bmad-output/planning-artifacts/` and `_bmad-output/implementation-artifacts/` for matching files - For glob patterns (e.g., `research-*.md`, `story-*.md`), collect all matching files + - **Large input sets:** If a glob matches many files (e.g., 10+ stories for a retrospective), list all paths in the briefing but mark the most directly relevant ones (e.g., same-epic completed stories, previous retro). The sidecar can read additional files on demand via `read_file`. 3. **Collect input document paths and descriptions.** For each input document, note its file path and a one-line description of what it contains (e.g., "`_bmad-output/planning-artifacts/PRD.md` — functional and non-functional requirements"). Do NOT read or paste their full content into the briefing — the sidecar has `read_file` access and will read them directly. This saves significant context in the parent session. -4. **Truncation guidance:** The output artifact should always be included in full (it's what's being reviewed). If even the artifact alone exceeds ~80,000 characters, truncate to the most relevant sections and note what was omitted so the sidecar can use `read_file` for the rest. +4. **Sharded artifacts (e.g., `epics/` directory):** If the artifact is a directory of shard files rather than a single file, concatenate all shards into the briefing with clear `--- filename.md ---` headers between each. Apply approved changes to the specific shard file(s) they target, not to a single monolithic file. When listing the artifact in the briefing, note that it spans multiple files so the sidecar references the correct shard in its findings. + +5. **Truncation guidance:** The output artifact should always be included in full (it's what's being reviewed). If even the artifact alone exceeds ~80,000 characters, truncate to the most relevant sections and note what was omitted so the sidecar can use `read_file` for the rest. ### Step 3: Spawn the first sidecar @@ -189,7 +193,7 @@ Process models in the order the user specified them. For each model: **4a. Poll and read results** -Poll the sidecar's status using `mcp__sidecar__sidecar_status` with the saved task ID, following the polling cadence indicated by `sidecar_status` responses. Continue polling while status is `running`; treat `complete`, `timeout`, `crashed`, `error`, and `aborted` as terminal. Once terminal, read the output using `mcp__sidecar__sidecar_read`. +Run `sleep 25` in your shell before the first and every subsequent `mcp__sidecar__sidecar_status` call — this enforces the mandatory polling interval and prevents token waste. Continue polling while status is `running`; treat `complete`, `timeout`, `crashed`, `error`, and `aborted` as terminal. Once terminal, read the output using `mcp__sidecar__sidecar_read`. **4b. Claude evaluates each suggestion** @@ -232,6 +236,7 @@ If there are more models to process: - This ensures the next model reviews the improved version, not the original - Poll, read, and evaluate the next model's results the same way (repeat 4a–4d) - When presenting: flag explicitly if this model raised something the previous model missed, or contradicts a change already applied +- **If a model fails or times out mid-pipeline:** Preserve all changes already applied from prior models. Inform the user briefly (e.g., "[model] timed out after 15 minutes"). Continue to the next model in the queue using the current artifact state. The user can choose to retry the failed model later or proceed without it. ### Step 5: Consolidation check From d0da1db42264aab3eabdf497af1e520fa14b82a7 Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 10:33:33 -0500 Subject: [PATCH 08/20] =?UTF-8?q?fix:=20address=20CodeRabbit=20nitpicks=20?= =?UTF-8?q?=E2=80=94=20code=20fence=20tags=20and=20defensive=20try-catch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- docs/bmad-workflow.md | 4 ++-- scripts/postinstall.js | 15 ++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/docs/bmad-workflow.md b/docs/bmad-workflow.md index b303129..fde2bb5 100644 --- a/docs/bmad-workflow.md +++ b/docs/bmad-workflow.md @@ -32,7 +32,7 @@ Reference for the auto-bmad-method-check skill. Captures all workflows, artifact ## Dependency Chain -``` +```text Brainstorming/Research ──> Product Brief ──> PRD ──> UX Design (optional) ├──> Architecture └──> Architecture ──> Epics/Stories @@ -92,7 +92,7 @@ Used by auto-bmad-method-check to determine which input documents to include in ## Standard Artifact Locations -``` +```text _bmad-output/ planning-artifacts/ brainstorming-report.md diff --git a/scripts/postinstall.js b/scripts/postinstall.js index b9648eb..e729122 100644 --- a/scripts/postinstall.js +++ b/scripts/postinstall.js @@ -18,11 +18,16 @@ 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 +} const MCP_CONFIG = { command: 'npx', args: ['-y', 'claude-sidecar@latest', 'mcp'] }; From f01b2bf4f37961177971635d421174b9e98d56ac Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 10:53:15 -0500 Subject: [PATCH 09/20] feat: add auto-skills section to main SKILL.md + invocation research 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 --- docs/auto-skill-invocation-research.md | 177 +++++++++++++++++++++++++ skill/SKILL.md | 17 +++ 2 files changed, 194 insertions(+) create mode 100644 docs/auto-skill-invocation-research.md diff --git a/docs/auto-skill-invocation-research.md b/docs/auto-skill-invocation-research.md new file mode 100644 index 0000000..cef3187 --- /dev/null +++ b/docs/auto-skill-invocation-research.md @@ -0,0 +1,177 @@ +# Auto-Skill Invocation: Research & Recommendations + +Research into how Claude Code skills achieve reliable invocation, with specific recommendations for improving sidecar auto-skill trigger reliability. + +## Background + +Sidecar ships four auto-skills — `auto-review`, `auto-unblock`, `auto-security`, and `auto-bmad-method-check` — that fire contextually at key workflow moments. Unlike user-invocable skills (which have slash commands like `/commit`), auto-skills rely on Claude recognizing trigger conditions in the conversation and reading the skill file proactively. + +The question: **how do we ensure Claude actually fires these skills when conditions are met?** + +## How Claude Code Skills Work + +### Skill Discovery + +Skills live as `SKILL.md` files with YAML frontmatter in two locations: +- `~/.claude/skills//SKILL.md` — user/package skills +- `~/.claude/plugins/cache//skills//SKILL.md` — plugin skills + +Claude discovers skills via the **Skill tool**, which scans these directories and exposes all SKILL.md files. The `description` field in frontmatter serves as the **declarative trigger specification** — Claude evaluates it against conversation state to decide whether to invoke. + +### Invocation Pipeline + +```text +1. Skill tool scans filesystem → builds list of available skills +2. Skill descriptions appear in system reminders ("available skills" block) +3. Claude evaluates descriptions against conversation state each turn +4. If match → Claude reads the full SKILL.md and follows its procedure +``` + +### Key Mechanism: The "Available Skills" System Reminder + +Every turn, Claude sees a system reminder listing available skills with their descriptions. This is the primary discovery mechanism. **Skills that appear in this list are far more likely to be invoked** because Claude evaluates them on every turn. + +## How Superpowers Achieves Near-100% Reliability + +The Superpowers plugin (Claude's official skill framework) uses a three-layer reinforcement strategy: + +### Layer 1: SessionStart Hook (Most Critical) + +Superpowers registers a **synchronous SessionStart hook** that fires on every startup, resume, clear, and compact event. This hook: + +1. Reads `using-superpowers/SKILL.md` from disk +2. Injects its content into the system prompt via `experimental.chat.system.transform` +3. Wraps it in `` tags + +This means the meta-instruction is present **before any user input**, on every session. + +**Hook registration** (`hooks.json`): +```json +{ + "hooks": { + "SessionStart": [{ + "matcher": "startup|resume|clear|compact", + "hooks": [{ + "type": "command", + "command": "\"${CLAUDE_PLUGIN_ROOT}/hooks/run-hook.cmd\" session-start", + "async": false + }] + }] + } +} +``` + +### Layer 2: Meta-Instruction + +The injected content contains a forceful meta-instruction: + +> "If you think there is even a 1% chance a skill might apply to what you are doing, you ABSOLUTELY MUST invoke the skill. This is not negotiable. This is not optional. You cannot rationalize your way out of this." + +It also includes a "Red Flags" table of rationalizations Claude should watch for (e.g., "This is just a simple question" → "Questions are tasks. Check for skills."). + +### Layer 3: Available Skills List + +All Superpowers skills appear in the "available skills" system reminder with descriptions like: +- `systematic-debugging: Use when encountering any bug, test failure, or unexpected behavior` +- `brainstorming: You MUST use this before any creative work` + +Claude sees these every turn, making pattern matching automatic. + +### Why This Works + +The three layers create **redundant triggering paths**: +- Even if Claude skips the skill check, the meta-instruction reminds it +- Even if the meta-instruction is missed, the skills list surfaces matches +- Even if the skills list is scrolled past, the SessionStart hook re-injects on context reset + +## Where Sidecar Auto-Skills Stand Today + +### What We Have + +1. **SKILL.md files** with `TRIGGER when:` clauses in descriptions — installed to `~/.claude/skills/sidecar/auto-*/` +2. **Main sidecar skill** (`~/.claude/skills/sidecar/SKILL.md`) — appears in the available skills list +3. **MCP tools** — `sidecar_start`, `sidecar_status`, `sidecar_read` available when MCP server is running + +### The Gap + +| Mechanism | Superpowers | Sidecar Auto-Skills | +|-----------|-------------|---------------------| +| SessionStart hook | Yes — injects meta-instruction | No | +| System prompt injection | Yes — `` tags | No | +| Available skills list | Yes — all skills listed with descriptions | No — auto-skills not listed* | +| Meta-instruction forcing skill checks | Yes — "ABSOLUTELY MUST" language | No — relies on Superpowers being installed | +| Trigger specification | Description field | Description field (same format) | + +*Auto-skills are discoverable via the Skill tool filesystem scan but do **not** appear in the "available skills" system reminder that Claude sees every turn. This is the critical gap. + +### Practical Impact + +- If Superpowers is installed: auto-skills **may** fire because Superpowers forces Claude to check for skills. But Claude still has to discover them via filesystem scan rather than seeing them in the skills list. +- If Superpowers is NOT installed: auto-skills have **no mechanism** prompting Claude to check for them. They exist on disk but Claude has no reason to look. +- Auto-skills are **invisible** in the available skills reminder, so even an instruction-following Claude won't pattern-match against them unless something else prompts a skill check. + +## Recommendations + +### Immediate: Add Auto-Skills to Main SKILL.md (Option B) ✅ Done + +Add a section to the main `skill/SKILL.md` that lists all auto-skills with their trigger conditions. Since the main sidecar skill already appears in the available skills list, Claude will see the auto-skill triggers when reading the main skill. + +**Pros:** Simple one-file change, no new infrastructure needed, works today. +**Cons:** Relies on Claude reading the full main skill (which it does when sidecar is relevant, but not every turn). Only provides awareness when sidecar context is active. + +### Short-Term: SessionStart Hook (Option A) + +Register a lightweight SessionStart hook in sidecar's postinstall that injects a brief auto-skills reminder into the system prompt. This would mirror what Superpowers does but scoped to sidecar triggers. + +**Implementation:** +1. Create `hooks/hooks.json` with SessionStart matcher +2. Create a hook script that injects a compact reminder: + ```text + Sidecar auto-skills are available. Check trigger conditions: + - auto-review: after implementing changes, before telling user "done" + - auto-unblock: after 5+ failed fix attempts + - auto-security: before git commit/push/PR + - auto-bmad-method-check: after writing BMAD artifacts in _bmad-output/ + Read the full skill from ~/.claude/skills/sidecar//SKILL.md when triggered. + ``` +3. Register the hook during postinstall alongside MCP and skill file installation + +**Pros:** Matches Superpowers' proven pattern. Always present from session start. Works independent of Superpowers. +**Cons:** Requires hook infrastructure (hooks.json, hook script, postinstall changes). Adds to system prompt size every session. + +**Open question:** Does the Claude Code hooks API support `experimental.chat.system.transform` for third-party packages, or is this restricted to plugins? If restricted, the hook could use the simpler `command` type to emit the reminder, though this is less reliable than system prompt injection. + +### Medium-Term: Make Auto-Skills Appear in Skills List (Option D) + +Give auto-skills optional slash-command names (e.g., `/auto-review`, `/auto-security`) so they appear in the available skills system reminder. Keep the auto-fire behavior — the slash command would just be an alternative manual trigger. + +**Pros:** Skills appear in the list Claude checks every turn. Belt and suspenders with auto-fire. +**Cons:** May confuse users who see skills they didn't know about. Pollutes the skills namespace. May not be possible without changes to how Claude Code lists skills. + +### Long-Term: Auto-Skills Framework in Sidecar Config + +If sidecar adds an `autoSkills` config namespace, centralize trigger definitions and enable/disable switches: + +```json +{ + "autoSkills": { + "review": { "enabled": true, "model": "gemini" }, + "unblock": { "enabled": true, "attemptThreshold": 5 }, + "security": { "enabled": true, "scanOnCommit": true }, + "bmadMethodCheck": { "enabled": true, "artifactDir": "_bmad-output/" } + } +} +``` + +This would let users customize which auto-skills fire and with what defaults, without editing SKILL.md files. + +## Summary + +| Approach | Effort | Reliability | Independence from Superpowers | +|----------|--------|-------------|-------------------------------| +| **B: Main SKILL.md section** (done) | Low | Medium — works when sidecar context is active | Partial — still benefits from Superpowers | +| **A: SessionStart hook** | Medium | High — always present from session start | Full — self-contained | +| **D: Skills list appearance** | Low-Medium | High — Claude checks every turn | Full | +| **Config framework** | High | High — user-configurable | Full | + +**Recommended path:** B (done) → A (next PR) → Config framework (when sidecar adds autoSkills namespace). diff --git a/skill/SKILL.md b/skill/SKILL.md index b440434..6d00595 100644 --- a/skill/SKILL.md +++ b/skill/SKILL.md @@ -1005,6 +1005,23 @@ sidecar start --model gemini --prompt "Task" --- +## Auto-Skills: Contextual Sidecar Triggers + +Sidecar includes auto-skills that fire automatically at key workflow moments. These are NOT user-invocable — they trigger based on conversation context. When their conditions are met, you MUST check the corresponding skill and offer to run it. + +| Auto-Skill | Trigger Condition | What It Does | +|------------|-------------------|--------------| +| `auto-review` | You have finished implementing changes and are about to tell the user "done" | Offers sidecar code review of the diff before finalizing | +| `auto-unblock` | You have tried 5+ distinct approaches to fix a bug and all have failed | Offers sidecar brainstorming session with a different model | +| `auto-security` | User says "commit", "push", "create PR", or you are about to run git commit/push/gh pr create | Offers sidecar security scan of staged changes before commit | +| `auto-bmad-method-check` | A BMAD-METHOD artifact (PRD, architecture, epics, story, etc.) has just been written or updated in `_bmad-output/` | Offers sidecar second-opinion review of the artifact | + +**How to use:** When you recognize a trigger condition above, read the full skill from `~/.claude/skills/sidecar//SKILL.md` and follow its procedure. Each skill will prompt the user for confirmation before spawning any sidecars. + +**Skip conditions (common to all):** Skip if the `mcp__sidecar__sidecar_start` tool is not available, or if a sidecar review already ran for the same content. + +--- + ## Quick Start Checklist 1. [ ] Install sidecar: `npm install -g claude-sidecar` From 2cf996768e70d1775cde8b11b6896b3b96fb4666 Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 11:00:01 -0500 Subject: [PATCH 10/20] feat: make auto-skills user-invocable via top-level installation 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 --- CLAUDE.md | 2 +- scripts/postinstall.js | 20 +++++++++++++++++--- skill/SKILL.md | 2 +- skill/auto-bmad-method-check/SKILL.md | 2 +- skill/auto-review/SKILL.md | 2 +- skill/auto-security/SKILL.md | 2 +- skill/auto-unblock/SKILL.md | 2 +- 7 files changed, 23 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f88293d..d6bd1df 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -163,7 +163,7 @@ scripts/ ├── generate-icon.js # Generate app icon PNG from SVG source. ├── integration-test.sh ├── list-models.js -├── postinstall.js # Install skill files to ~/.claude/skills/sidecar/ +├── postinstall.js # Install skill files to ~/.claude/skills/ ├── test-tools.sh ├── validate-docs.js # * Main entry point. ├── validate-thinking.js diff --git a/scripts/postinstall.js b/scripts/postinstall.js index e729122..6f1b206 100644 --- a/scripts/postinstall.js +++ b/scripts/postinstall.js @@ -62,7 +62,7 @@ function addMcpToConfigFile(configPath, name, config) { return status; } -/** Install skill files to ~/.claude/skills/sidecar/ */ +/** Install skill files to ~/.claude/skills/ */ function installSkill() { try { fs.mkdirSync(SKILL_DEST_DIR, { recursive: true }); @@ -72,13 +72,27 @@ function installSkill() { console.error(`[claude-sidecar] Warning: Could not install skill: ${err.message}`); } + const skillsRoot = path.join(os.homedir(), '.claude', 'skills'); for (const name of AUTO_SKILLS) { try { const src = path.join(SKILL_DIR, name, 'SKILL.md'); - const destDir = path.join(SKILL_DEST_DIR, name); + // Install as top-level skill (e.g., ~/.claude/skills/sidecar-auto-review/) + // so Claude Code discovers it in the available skills list + const destDir = path.join(skillsRoot, `sidecar-${name}`); fs.mkdirSync(destDir, { recursive: true }); fs.copyFileSync(src, path.join(destDir, 'SKILL.md')); - console.log(`[claude-sidecar] Skill installed: ${name}`); + console.log(`[claude-sidecar] Skill installed: sidecar-${name}`); + + // Clean up old nested location (~/.claude/skills/sidecar//) + const oldDir = path.join(SKILL_DEST_DIR, name); + try { + if (fs.existsSync(path.join(oldDir, 'SKILL.md'))) { + fs.unlinkSync(path.join(oldDir, 'SKILL.md')); + fs.rmdirSync(oldDir); + } + } catch { + // Old location doesn't exist or already cleaned — ignore + } } catch (err) { console.error(`[claude-sidecar] Warning: Could not install ${name} skill: ${err.message}`); } diff --git a/skill/SKILL.md b/skill/SKILL.md index 6d00595..d377077 100644 --- a/skill/SKILL.md +++ b/skill/SKILL.md @@ -1016,7 +1016,7 @@ Sidecar includes auto-skills that fire automatically at key workflow moments. Th | `auto-security` | User says "commit", "push", "create PR", or you are about to run git commit/push/gh pr create | Offers sidecar security scan of staged changes before commit | | `auto-bmad-method-check` | A BMAD-METHOD artifact (PRD, architecture, epics, story, etc.) has just been written or updated in `_bmad-output/` | Offers sidecar second-opinion review of the artifact | -**How to use:** When you recognize a trigger condition above, read the full skill from `~/.claude/skills/sidecar//SKILL.md` and follow its procedure. Each skill will prompt the user for confirmation before spawning any sidecars. +**How to use:** When you recognize a trigger condition above, invoke the skill (e.g., `/sidecar-auto-review`) or read it from `~/.claude/skills/sidecar-/SKILL.md` and follow its procedure. Each skill will prompt the user for confirmation before spawning any sidecars. **Skip conditions (common to all):** Skip if the `mcp__sidecar__sidecar_start` tool is not available, or if a sidecar review already ran for the same content. diff --git a/skill/auto-bmad-method-check/SKILL.md b/skill/auto-bmad-method-check/SKILL.md index f4fb1dc..0517e4d 100644 --- a/skill/auto-bmad-method-check/SKILL.md +++ b/skill/auto-bmad-method-check/SKILL.md @@ -1,5 +1,5 @@ --- -name: auto-bmad-method-check +name: sidecar-auto-bmad-method-check description: > Use when a BMAD-METHOD workflow has just produced an output artifact (PRD, architecture doc, epics, story file, etc.) and the user has not yet finalized it or moved to the next workflow diff --git a/skill/auto-review/SKILL.md b/skill/auto-review/SKILL.md index add9ab1..c56d51c 100644 --- a/skill/auto-review/SKILL.md +++ b/skill/auto-review/SKILL.md @@ -1,5 +1,5 @@ --- -name: auto-review +name: sidecar-auto-review description: > Use after completing a feature implementation, bug fix, or significant code change — before claiming the work is done. Offers to spawn a sidecar with a different model to review the diff --git a/skill/auto-security/SKILL.md b/skill/auto-security/SKILL.md index ce9c181..1e74e58 100644 --- a/skill/auto-security/SKILL.md +++ b/skill/auto-security/SKILL.md @@ -1,5 +1,5 @@ --- -name: auto-security +name: sidecar-auto-security description: > Use when the user asks to commit changes, push code, or create a pull request. Offers to spawn a sidecar to audit the diff for security vulnerabilities before the commit/push/PR diff --git a/skill/auto-unblock/SKILL.md b/skill/auto-unblock/SKILL.md index dd8d59a..5c6b382 100644 --- a/skill/auto-unblock/SKILL.md +++ b/skill/auto-unblock/SKILL.md @@ -1,5 +1,5 @@ --- -name: auto-unblock +name: sidecar-auto-unblock description: > Use when you have attempted 5 or more different approaches to fix a bug, pass a test, or solve a problem and none have worked. Offers to spawn a sidecar brainstorming session with From e0b284b9e7c9fae9ea264dbba10910da88dbdd6b Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 11:06:30 -0500 Subject: [PATCH 11/20] docs: update auto-skill invocation research with completed work 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 --- docs/auto-skill-invocation-research.md | 108 ++++++++++++++----------- 1 file changed, 61 insertions(+), 47 deletions(-) diff --git a/docs/auto-skill-invocation-research.md b/docs/auto-skill-invocation-research.md index cef3187..ab932ea 100644 --- a/docs/auto-skill-invocation-research.md +++ b/docs/auto-skill-invocation-research.md @@ -4,7 +4,7 @@ Research into how Claude Code skills achieve reliable invocation, with specific ## Background -Sidecar ships four auto-skills — `auto-review`, `auto-unblock`, `auto-security`, and `auto-bmad-method-check` — that fire contextually at key workflow moments. Unlike user-invocable skills (which have slash commands like `/commit`), auto-skills rely on Claude recognizing trigger conditions in the conversation and reading the skill file proactively. +Sidecar ships four auto-skills — `auto-review`, `auto-unblock`, `auto-security`, and `auto-bmad-method-check` — that fire contextually at key workflow moments. Unlike user-invocable skills (which have slash commands like `/commit`), auto-skills were originally designed to rely solely on Claude recognizing trigger conditions in the conversation and reading the skill file proactively. The question: **how do we ensure Claude actually fires these skills when conditions are met?** @@ -31,6 +31,10 @@ Claude discovers skills via the **Skill tool**, which scans these directories an Every turn, Claude sees a system reminder listing available skills with their descriptions. This is the primary discovery mechanism. **Skills that appear in this list are far more likely to be invoked** because Claude evaluates them on every turn. +### Critical Discovery: Nesting Depth Matters + +Claude Code only discovers skills at the **top level** of `~/.claude/skills/`. A skill at `~/.claude/skills/my-skill/SKILL.md` appears in the available skills list; a skill nested at `~/.claude/skills/parent/child/SKILL.md` does **not**. This was the root cause of the original visibility gap — auto-skills were installed as nested subdirectories under the main sidecar skill. + ## How Superpowers Achieves Near-100% Reliability The Superpowers plugin (Claude's official skill framework) uses a three-layer reinforcement strategy: @@ -84,42 +88,52 @@ The three layers create **redundant triggering paths**: - Even if the meta-instruction is missed, the skills list surfaces matches - Even if the skills list is scrolled past, the SessionStart hook re-injects on context reset -## Where Sidecar Auto-Skills Stand Today +## What We've Done -### What We Have +### Step 1: Add Auto-Skills to Main SKILL.md (Option B) ✅ Done -1. **SKILL.md files** with `TRIGGER when:` clauses in descriptions — installed to `~/.claude/skills/sidecar/auto-*/` -2. **Main sidecar skill** (`~/.claude/skills/sidecar/SKILL.md`) — appears in the available skills list -3. **MCP tools** — `sidecar_start`, `sidecar_status`, `sidecar_read` available when MCP server is running +Added an "Auto-Skills: Contextual Sidecar Triggers" section to the main `skill/SKILL.md` listing all four auto-skills with their trigger conditions in a table. Since the main sidecar skill appears in the available skills list, Claude gets passive awareness of auto-skill triggers when it reads the main skill. -### The Gap +**Pros:** Simple one-file change, no new infrastructure needed. +**Cons:** Only provides awareness when sidecar context is active — Claude has to read the main skill first. -| Mechanism | Superpowers | Sidecar Auto-Skills | -|-----------|-------------|---------------------| -| SessionStart hook | Yes — injects meta-instruction | No | -| System prompt injection | Yes — `` tags | No | -| Available skills list | Yes — all skills listed with descriptions | No — auto-skills not listed* | -| Meta-instruction forcing skill checks | Yes — "ABSOLUTELY MUST" language | No — relies on Superpowers being installed | -| Trigger specification | Description field | Description field (same format) | +### Step 2: Top-Level Installation for Skills List Visibility (Option D) ✅ Done -*Auto-skills are discoverable via the Skill tool filesystem scan but do **not** appear in the "available skills" system reminder that Claude sees every turn. This is the critical gap. +Moved auto-skills from nested directories (`~/.claude/skills/sidecar/auto-*/`) to top-level directories (`~/.claude/skills/sidecar-auto-*/`). This makes them appear in Claude Code's "available skills" system reminder every turn. -### Practical Impact +**Changes made:** +- `postinstall.js` — installs auto-skills to `~/.claude/skills/sidecar-auto-review/` etc. (top-level), cleans up old nested path +- All 4 SKILL.md frontmatters — `name` field updated to `sidecar-auto-review`, `sidecar-auto-unblock`, `sidecar-auto-security`, `sidecar-auto-bmad-method-check` +- Main SKILL.md — updated reference path, mentions slash-command invocation -- If Superpowers is installed: auto-skills **may** fire because Superpowers forces Claude to check for skills. But Claude still has to discover them via filesystem scan rather than seeing them in the skills list. -- If Superpowers is NOT installed: auto-skills have **no mechanism** prompting Claude to check for them. They exist on disk but Claude has no reason to look. -- Auto-skills are **invisible** in the available skills reminder, so even an instruction-following Claude won't pattern-match against them unless something else prompts a skill check. +**Result:** All four auto-skills now appear in the available skills list with their full trigger descriptions. They support both: +- **Auto-fire** — Claude pattern-matches trigger conditions from the skills list every turn +- **Manual invocation** — user can type `/sidecar-auto-review`, `/sidecar-auto-security`, etc. + +**Verified:** After local installation, the system reminder now includes entries like: +```text +- sidecar-auto-review: Use after completing a feature implementation, bug fix, or significant code change... +- sidecar-auto-unblock: Use when you have attempted 5 or more different approaches to fix a bug... +- sidecar-auto-security: Use when the user asks to commit changes, push code, or create a pull request... +- sidecar-auto-bmad-method-check: Use when a BMAD-METHOD workflow has just produced an output artifact... +``` -## Recommendations +### Current State -### Immediate: Add Auto-Skills to Main SKILL.md (Option B) ✅ Done +| Mechanism | Superpowers | Sidecar Auto-Skills | +|-----------|-------------|---------------------| +| SessionStart hook | Yes — injects meta-instruction | Not yet | +| System prompt injection | Yes — `` tags | Not yet | +| Available skills list | Yes — all skills listed with descriptions | **Yes** — all 4 auto-skills listed ✅ | +| Meta-instruction forcing skill checks | Yes — "ABSOLUTELY MUST" language | No — benefits from Superpowers if installed | +| Trigger specification | Description field | Description field (same format) ✅ | +| Manual invocation fallback | Yes — slash commands | **Yes** — `/sidecar-auto-*` commands ✅ | -Add a section to the main `skill/SKILL.md` that lists all auto-skills with their trigger conditions. Since the main sidecar skill already appears in the available skills list, Claude will see the auto-skill triggers when reading the main skill. +The critical gap (skills list visibility) is now closed. The remaining gap is the SessionStart hook — without it, sidecar auto-skills rely on either (a) Superpowers being installed to force skill checking, or (b) Claude independently deciding to check skills based on the available skills list. -**Pros:** Simple one-file change, no new infrastructure needed, works today. -**Cons:** Relies on Claude reading the full main skill (which it does when sidecar is relevant, but not every turn). Only provides awareness when sidecar context is active. +## Remaining Recommendations -### Short-Term: SessionStart Hook (Option A) +### Next: SessionStart Hook (Option A) Register a lightweight SessionStart hook in sidecar's postinstall that injects a brief auto-skills reminder into the system prompt. This would mirror what Superpowers does but scoped to sidecar triggers. @@ -127,28 +141,21 @@ Register a lightweight SessionStart hook in sidecar's postinstall that injects a 1. Create `hooks/hooks.json` with SessionStart matcher 2. Create a hook script that injects a compact reminder: ```text - Sidecar auto-skills are available. Check trigger conditions: - - auto-review: after implementing changes, before telling user "done" - - auto-unblock: after 5+ failed fix attempts - - auto-security: before git commit/push/PR - - auto-bmad-method-check: after writing BMAD artifacts in _bmad-output/ - Read the full skill from ~/.claude/skills/sidecar//SKILL.md when triggered. + Sidecar auto-skills are installed. When you recognize these trigger conditions, + invoke the corresponding skill: + - /sidecar-auto-review — after implementing changes, before telling user "done" + - /sidecar-auto-unblock — after 5+ failed fix attempts + - /sidecar-auto-security — before git commit/push/PR + - /sidecar-auto-bmad-method-check — after writing BMAD artifacts in _bmad-output/ ``` 3. Register the hook during postinstall alongside MCP and skill file installation -**Pros:** Matches Superpowers' proven pattern. Always present from session start. Works independent of Superpowers. +**Pros:** Matches Superpowers' proven pattern. Always present from session start. Works independent of Superpowers. Combined with skills list visibility (already done), this would give two independent triggering paths. **Cons:** Requires hook infrastructure (hooks.json, hook script, postinstall changes). Adds to system prompt size every session. **Open question:** Does the Claude Code hooks API support `experimental.chat.system.transform` for third-party packages, or is this restricted to plugins? If restricted, the hook could use the simpler `command` type to emit the reminder, though this is less reliable than system prompt injection. -### Medium-Term: Make Auto-Skills Appear in Skills List (Option D) - -Give auto-skills optional slash-command names (e.g., `/auto-review`, `/auto-security`) so they appear in the available skills system reminder. Keep the auto-fire behavior — the slash command would just be an alternative manual trigger. - -**Pros:** Skills appear in the list Claude checks every turn. Belt and suspenders with auto-fire. -**Cons:** May confuse users who see skills they didn't know about. Pollutes the skills namespace. May not be possible without changes to how Claude Code lists skills. - -### Long-Term: Auto-Skills Framework in Sidecar Config +### Future: Auto-Skills Framework in Sidecar Config If sidecar adds an `autoSkills` config namespace, centralize trigger definitions and enable/disable switches: @@ -165,13 +172,20 @@ If sidecar adds an `autoSkills` config namespace, centralize trigger definitions This would let users customize which auto-skills fire and with what defaults, without editing SKILL.md files. +### Future: Community Auto-Skills + +The top-level installation pattern (`~/.claude/skills/sidecar-auto-*/`) and dynamic discovery in postinstall (`fs.readdirSync` for `auto-*` directories) means third-party auto-skills could be contributed by following the same convention: +1. Add a `skill/auto-/SKILL.md` to the repo +2. Postinstall automatically discovers and installs it as `~/.claude/skills/sidecar-auto-/` +3. No hardcoded lists to maintain + ## Summary -| Approach | Effort | Reliability | Independence from Superpowers | -|----------|--------|-------------|-------------------------------| -| **B: Main SKILL.md section** (done) | Low | Medium — works when sidecar context is active | Partial — still benefits from Superpowers | -| **A: SessionStart hook** | Medium | High — always present from session start | Full — self-contained | -| **D: Skills list appearance** | Low-Medium | High — Claude checks every turn | Full | -| **Config framework** | High | High — user-configurable | Full | +| Approach | Effort | Reliability | Status | +|----------|--------|-------------|--------| +| **B: Main SKILL.md section** | Low | Medium — works when sidecar context is active | ✅ Done | +| **D: Top-level installation** | Low | High — Claude checks skills list every turn | ✅ Done | +| **A: SessionStart hook** | Medium | Very high — always present from session start | Recommended next | +| **Config framework** | High | Very high — user-configurable | Future | -**Recommended path:** B (done) → A (next PR) → Config framework (when sidecar adds autoSkills namespace). +**Current reliability:** With options B and D both implemented, auto-skills have strong visibility through the available skills list. The main remaining improvement is a SessionStart hook (Option A) for environments where Superpowers is not installed, ensuring skill awareness is always injected at session start regardless of other plugins. From 66f937d6c8adcede9257d540b59eadd6ae99b5d2 Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 11:07:31 -0500 Subject: [PATCH 12/20] chore: remove internal research doc from PR Co-Authored-By: Claude Opus 4.6 --- docs/auto-skill-invocation-research.md | 191 ------------------------- 1 file changed, 191 deletions(-) delete mode 100644 docs/auto-skill-invocation-research.md diff --git a/docs/auto-skill-invocation-research.md b/docs/auto-skill-invocation-research.md deleted file mode 100644 index ab932ea..0000000 --- a/docs/auto-skill-invocation-research.md +++ /dev/null @@ -1,191 +0,0 @@ -# Auto-Skill Invocation: Research & Recommendations - -Research into how Claude Code skills achieve reliable invocation, with specific recommendations for improving sidecar auto-skill trigger reliability. - -## Background - -Sidecar ships four auto-skills — `auto-review`, `auto-unblock`, `auto-security`, and `auto-bmad-method-check` — that fire contextually at key workflow moments. Unlike user-invocable skills (which have slash commands like `/commit`), auto-skills were originally designed to rely solely on Claude recognizing trigger conditions in the conversation and reading the skill file proactively. - -The question: **how do we ensure Claude actually fires these skills when conditions are met?** - -## How Claude Code Skills Work - -### Skill Discovery - -Skills live as `SKILL.md` files with YAML frontmatter in two locations: -- `~/.claude/skills//SKILL.md` — user/package skills -- `~/.claude/plugins/cache//skills//SKILL.md` — plugin skills - -Claude discovers skills via the **Skill tool**, which scans these directories and exposes all SKILL.md files. The `description` field in frontmatter serves as the **declarative trigger specification** — Claude evaluates it against conversation state to decide whether to invoke. - -### Invocation Pipeline - -```text -1. Skill tool scans filesystem → builds list of available skills -2. Skill descriptions appear in system reminders ("available skills" block) -3. Claude evaluates descriptions against conversation state each turn -4. If match → Claude reads the full SKILL.md and follows its procedure -``` - -### Key Mechanism: The "Available Skills" System Reminder - -Every turn, Claude sees a system reminder listing available skills with their descriptions. This is the primary discovery mechanism. **Skills that appear in this list are far more likely to be invoked** because Claude evaluates them on every turn. - -### Critical Discovery: Nesting Depth Matters - -Claude Code only discovers skills at the **top level** of `~/.claude/skills/`. A skill at `~/.claude/skills/my-skill/SKILL.md` appears in the available skills list; a skill nested at `~/.claude/skills/parent/child/SKILL.md` does **not**. This was the root cause of the original visibility gap — auto-skills were installed as nested subdirectories under the main sidecar skill. - -## How Superpowers Achieves Near-100% Reliability - -The Superpowers plugin (Claude's official skill framework) uses a three-layer reinforcement strategy: - -### Layer 1: SessionStart Hook (Most Critical) - -Superpowers registers a **synchronous SessionStart hook** that fires on every startup, resume, clear, and compact event. This hook: - -1. Reads `using-superpowers/SKILL.md` from disk -2. Injects its content into the system prompt via `experimental.chat.system.transform` -3. Wraps it in `` tags - -This means the meta-instruction is present **before any user input**, on every session. - -**Hook registration** (`hooks.json`): -```json -{ - "hooks": { - "SessionStart": [{ - "matcher": "startup|resume|clear|compact", - "hooks": [{ - "type": "command", - "command": "\"${CLAUDE_PLUGIN_ROOT}/hooks/run-hook.cmd\" session-start", - "async": false - }] - }] - } -} -``` - -### Layer 2: Meta-Instruction - -The injected content contains a forceful meta-instruction: - -> "If you think there is even a 1% chance a skill might apply to what you are doing, you ABSOLUTELY MUST invoke the skill. This is not negotiable. This is not optional. You cannot rationalize your way out of this." - -It also includes a "Red Flags" table of rationalizations Claude should watch for (e.g., "This is just a simple question" → "Questions are tasks. Check for skills."). - -### Layer 3: Available Skills List - -All Superpowers skills appear in the "available skills" system reminder with descriptions like: -- `systematic-debugging: Use when encountering any bug, test failure, or unexpected behavior` -- `brainstorming: You MUST use this before any creative work` - -Claude sees these every turn, making pattern matching automatic. - -### Why This Works - -The three layers create **redundant triggering paths**: -- Even if Claude skips the skill check, the meta-instruction reminds it -- Even if the meta-instruction is missed, the skills list surfaces matches -- Even if the skills list is scrolled past, the SessionStart hook re-injects on context reset - -## What We've Done - -### Step 1: Add Auto-Skills to Main SKILL.md (Option B) ✅ Done - -Added an "Auto-Skills: Contextual Sidecar Triggers" section to the main `skill/SKILL.md` listing all four auto-skills with their trigger conditions in a table. Since the main sidecar skill appears in the available skills list, Claude gets passive awareness of auto-skill triggers when it reads the main skill. - -**Pros:** Simple one-file change, no new infrastructure needed. -**Cons:** Only provides awareness when sidecar context is active — Claude has to read the main skill first. - -### Step 2: Top-Level Installation for Skills List Visibility (Option D) ✅ Done - -Moved auto-skills from nested directories (`~/.claude/skills/sidecar/auto-*/`) to top-level directories (`~/.claude/skills/sidecar-auto-*/`). This makes them appear in Claude Code's "available skills" system reminder every turn. - -**Changes made:** -- `postinstall.js` — installs auto-skills to `~/.claude/skills/sidecar-auto-review/` etc. (top-level), cleans up old nested path -- All 4 SKILL.md frontmatters — `name` field updated to `sidecar-auto-review`, `sidecar-auto-unblock`, `sidecar-auto-security`, `sidecar-auto-bmad-method-check` -- Main SKILL.md — updated reference path, mentions slash-command invocation - -**Result:** All four auto-skills now appear in the available skills list with their full trigger descriptions. They support both: -- **Auto-fire** — Claude pattern-matches trigger conditions from the skills list every turn -- **Manual invocation** — user can type `/sidecar-auto-review`, `/sidecar-auto-security`, etc. - -**Verified:** After local installation, the system reminder now includes entries like: -```text -- sidecar-auto-review: Use after completing a feature implementation, bug fix, or significant code change... -- sidecar-auto-unblock: Use when you have attempted 5 or more different approaches to fix a bug... -- sidecar-auto-security: Use when the user asks to commit changes, push code, or create a pull request... -- sidecar-auto-bmad-method-check: Use when a BMAD-METHOD workflow has just produced an output artifact... -``` - -### Current State - -| Mechanism | Superpowers | Sidecar Auto-Skills | -|-----------|-------------|---------------------| -| SessionStart hook | Yes — injects meta-instruction | Not yet | -| System prompt injection | Yes — `` tags | Not yet | -| Available skills list | Yes — all skills listed with descriptions | **Yes** — all 4 auto-skills listed ✅ | -| Meta-instruction forcing skill checks | Yes — "ABSOLUTELY MUST" language | No — benefits from Superpowers if installed | -| Trigger specification | Description field | Description field (same format) ✅ | -| Manual invocation fallback | Yes — slash commands | **Yes** — `/sidecar-auto-*` commands ✅ | - -The critical gap (skills list visibility) is now closed. The remaining gap is the SessionStart hook — without it, sidecar auto-skills rely on either (a) Superpowers being installed to force skill checking, or (b) Claude independently deciding to check skills based on the available skills list. - -## Remaining Recommendations - -### Next: SessionStart Hook (Option A) - -Register a lightweight SessionStart hook in sidecar's postinstall that injects a brief auto-skills reminder into the system prompt. This would mirror what Superpowers does but scoped to sidecar triggers. - -**Implementation:** -1. Create `hooks/hooks.json` with SessionStart matcher -2. Create a hook script that injects a compact reminder: - ```text - Sidecar auto-skills are installed. When you recognize these trigger conditions, - invoke the corresponding skill: - - /sidecar-auto-review — after implementing changes, before telling user "done" - - /sidecar-auto-unblock — after 5+ failed fix attempts - - /sidecar-auto-security — before git commit/push/PR - - /sidecar-auto-bmad-method-check — after writing BMAD artifacts in _bmad-output/ - ``` -3. Register the hook during postinstall alongside MCP and skill file installation - -**Pros:** Matches Superpowers' proven pattern. Always present from session start. Works independent of Superpowers. Combined with skills list visibility (already done), this would give two independent triggering paths. -**Cons:** Requires hook infrastructure (hooks.json, hook script, postinstall changes). Adds to system prompt size every session. - -**Open question:** Does the Claude Code hooks API support `experimental.chat.system.transform` for third-party packages, or is this restricted to plugins? If restricted, the hook could use the simpler `command` type to emit the reminder, though this is less reliable than system prompt injection. - -### Future: Auto-Skills Framework in Sidecar Config - -If sidecar adds an `autoSkills` config namespace, centralize trigger definitions and enable/disable switches: - -```json -{ - "autoSkills": { - "review": { "enabled": true, "model": "gemini" }, - "unblock": { "enabled": true, "attemptThreshold": 5 }, - "security": { "enabled": true, "scanOnCommit": true }, - "bmadMethodCheck": { "enabled": true, "artifactDir": "_bmad-output/" } - } -} -``` - -This would let users customize which auto-skills fire and with what defaults, without editing SKILL.md files. - -### Future: Community Auto-Skills - -The top-level installation pattern (`~/.claude/skills/sidecar-auto-*/`) and dynamic discovery in postinstall (`fs.readdirSync` for `auto-*` directories) means third-party auto-skills could be contributed by following the same convention: -1. Add a `skill/auto-/SKILL.md` to the repo -2. Postinstall automatically discovers and installs it as `~/.claude/skills/sidecar-auto-/` -3. No hardcoded lists to maintain - -## Summary - -| Approach | Effort | Reliability | Status | -|----------|--------|-------------|--------| -| **B: Main SKILL.md section** | Low | Medium — works when sidecar context is active | ✅ Done | -| **D: Top-level installation** | Low | High — Claude checks skills list every turn | ✅ Done | -| **A: SessionStart hook** | Medium | Very high — always present from session start | Recommended next | -| **Config framework** | High | Very high — user-configurable | Future | - -**Current reliability:** With options B and D both implemented, auto-skills have strong visibility through the available skills list. The main remaining improvement is a SessionStart hook (Option A) for environments where Superpowers is not installed, ensuring skill awareness is always injected at session start regardless of other plugins. From 464de2155ced1ab258662c87d4dd2df99fb94bd9 Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 11:26:28 -0500 Subject: [PATCH 13/20] fix: remove blank line breaking markdown table in bmad-workflow.md Co-Authored-By: Claude Opus 4.6 --- docs/bmad-workflow.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/bmad-workflow.md b/docs/bmad-workflow.md index fde2bb5..f6b0b02 100644 --- a/docs/bmad-workflow.md +++ b/docs/bmad-workflow.md @@ -64,7 +64,6 @@ Used by auto-bmad-method-check to determine which input documents to include in | Implementation Readiness | `PRD.md`, `architecture.md`, `epics.md`, `ux-design-specification.md` (if exists) | | `sprint-status.yaml` | `epics.md` | | `story-*.md` | `epics.md`, `PRD.md`, `architecture.md`, `sprint-status.yaml` | - | `sprint-change-proposal-*.md` | `PRD.md`, `epics.md`, affected `story-*.md` files | | `epic-*-retro-*.md` | All `story-*.md` in that epic, previous retro (if exists) | | `tech-spec.md` | None (Quick Flow — standalone) | From 4212dcc1fd85fd1dc84d8f108ddb58e0cd04d255 Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 17:02:59 -0500 Subject: [PATCH 14/20] feat: add activity-monitoring hooks for auto-skill triggers (Phase 1) 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 #7 (feat/auto-sidecar-skills) which introduced the four auto-skills. Addresses #8 (activity monitoring for auto-skill triggers). Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 5 +- bin/sidecar.js | 5 +- docs/activity-monitoring-research.md | 446 +++++++++++++++++++++++++ docs/auto-skill-invocation-proposal.md | 140 ++++++++ hooks/hooks.json | 38 +++ hooks/post-failure.sh | 57 ++++ hooks/post-tool-use.sh | 100 ++++++ hooks/pre-bash.sh | 70 ++++ package.json | 5 +- scripts/postinstall.js | 85 ++++- scripts/preuninstall.js | 65 ++++ skill/auto-bmad-method-check/SKILL.md | 6 +- skill/auto-review/SKILL.md | 6 + skill/auto-security/SKILL.md | 6 + skill/auto-unblock/SKILL.md | 6 + src/cli-handlers.js | 62 +++- src/cli.js | 10 +- src/utils/auto-skills-config.js | 152 +++++++++ tests/auto-skills-config.test.js | 208 ++++++++++++ 19 files changed, 1464 insertions(+), 8 deletions(-) create mode 100644 docs/activity-monitoring-research.md create mode 100644 docs/auto-skill-invocation-proposal.md create mode 100644 hooks/hooks.json create mode 100755 hooks/post-failure.sh create mode 100755 hooks/post-tool-use.sh create mode 100755 hooks/pre-bash.sh create mode 100644 scripts/preuninstall.js create mode 100644 src/utils/auto-skills-config.js create mode 100644 tests/auto-skills-config.test.js diff --git a/CLAUDE.md b/CLAUDE.md index d6bd1df..4e6e559 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -103,6 +103,7 @@ src/ │ ├── api-key-store.js # Maps provider IDs to environment variable names │ ├── api-key-validation.js # Validation endpoints per provider │ ├── auth-json.js # Known provider IDs that map to sidecar's PROVIDER_ENV_MAP +│ ├── auto-skills-config.js # Valid auto-skill names (keys in autoSkills config) │ ├── config.js # Default model alias map — short names to full OpenRouter model identifiers │ ├── logger.js # Structured Logger Module │ ├── mcp-discovery.js # MCP Discovery - Discovers MCP servers from parent LLM configuration @@ -164,6 +165,7 @@ scripts/ ├── integration-test.sh ├── list-models.js ├── postinstall.js # Install skill files to ~/.claude/skills/ +├── preuninstall.js # Sidecar hook script filenames — used to identify our hooks regardless of install path ├── test-tools.sh ├── validate-docs.js # * Main entry point. ├── validate-thinking.js @@ -190,7 +192,7 @@ evals/ | Module | Purpose | Key Exports | |--------|---------|-------------| -| `cli-handlers.js` | CLI Command Handlers | `handleSetup()`, `handleAbort()`, `handleUpdate()`, `handleMcp()` | +| `cli-handlers.js` | CLI Command Handlers | `handleSetup()`, `handleAbort()`, `handleUpdate()`, `handleMcp()`, `handleAutoSkills()` | | `cli.js` | * Default values per spec §4.1 | `parseArgs()`, `validateStartArgs()`, `getUsage()`, `DEFAULTS()` | | `conflict.js` | File Conflict Detection Module | `detectConflicts()`, `formatConflictWarning()` | | `context-compression.js` | Context Compression Module | `compressContext()`, `estimateTokenCount()`, `buildPreamble()`, `DEFAULT_TOKEN_LIMIT()` | @@ -223,6 +225,7 @@ evals/ | `utils/api-key-store.js` | Maps provider IDs to environment variable names | `getEnvPath()`, `readApiKeys()`, `readApiKeyHints()`, `readApiKeyValues()`, `saveApiKey()` | | `utils/api-key-validation.js` | Validation endpoints per provider | `validateApiKey()`, `validateOpenRouterKey()`, `VALIDATION_ENDPOINTS()` | | `utils/auth-json.js` | Known provider IDs that map to sidecar's PROVIDER_ENV_MAP | `readAuthJsonKeys()`, `importFromAuthJson()`, `checkAuthJson()`, `removeFromAuthJson()`, `AUTH_JSON_PATH()` | +| `utils/auto-skills-config.js` | Valid auto-skill names (keys in autoSkills config) | `VALID_SKILL_NAMES()`, `SKILL_LABELS()`, `getAutoSkillsConfig()`, `isSkillEnabled()`, `isMonitoringEnabled()` | | `utils/config.js` | Default model alias map — short names to full OpenRouter model identifiers | `getConfigDir()`, `getConfigPath()`, `loadConfig()`, `saveConfig()`, `getDefaultAliases()` | | `utils/logger.js` | Structured Logger Module | `logger()`, `LOG_LEVELS()` | | `utils/mcp-discovery.js` | MCP Discovery - Discovers MCP servers from parent LLM configuration | `discoverParentMcps()`, `discoverClaudeCodeMcps()`, `discoverCoworkMcps()`, `normalizeMcpJson()` | diff --git a/bin/sidecar.js b/bin/sidecar.js index 5b288f5..01f6791 100755 --- a/bin/sidecar.js +++ b/bin/sidecar.js @@ -21,7 +21,7 @@ if (process.env.GEMINI_API_KEY && !process.env.GOOGLE_GENERATIVE_AI_API_KEY) { const { parseArgs, validateStartArgs, getUsage } = require('../src/cli'); const { validateTaskId } = require('../src/utils/validators'); const { resolveModelFromArgs, validateFallbackModel } = require('../src/utils/start-helpers'); -const { handleSetup, handleAbort, handleUpdate, handleMcp } = require('../src/cli-handlers'); +const { handleSetup, handleAbort, handleUpdate, handleMcp, handleAutoSkills } = require('../src/cli-handlers'); const VERSION = require('../package.json').version; @@ -103,6 +103,9 @@ async function main() { case 'update': await handleUpdate(); break; + case 'auto-skills': + handleAutoSkills(args); + break; default: console.error(`Unknown command: ${command}`); console.log(getUsage()); diff --git a/docs/activity-monitoring-research.md b/docs/activity-monitoring-research.md new file mode 100644 index 0000000..620e8d2 --- /dev/null +++ b/docs/activity-monitoring-research.md @@ -0,0 +1,446 @@ +# Activity-Monitoring Auto-Skill Triggers: Research + +## Problem Statement + +Sidecar's four auto-skills (`auto-unblock`, `auto-review`, `auto-security`, `auto-bmad-check`) currently rely on a single triggering mechanism: Claude pattern-matching skill descriptions against conversation state every turn. Each skill's `description` field in its SKILL.md frontmatter contains trigger conditions (e.g., "5 or more different approaches"), and Claude evaluates these against its perception of the conversation. + +This mechanism has three fundamental weaknesses: + +1. **Self-assessment is unreliable.** Claude must count its own failed attempts, judge when "implementation is complete," or recognize it's looping — but it often doesn't count accurately, misses the threshold, or rationalizes that it's making progress when it isn't. + +2. **Description-matching is probabilistic.** Whether Claude invokes a skill depends on how well it pattern-matches the description against the current conversational state. This varies with context window pressure, conversation length, and model behavior. There's no guarantee a skill fires at the right moment. + +3. **No objective evidence.** The trigger decision is entirely internal to Claude's reasoning. There is no external signal — like "Bash has failed 5 times in a row on the same file" — feeding into the decision. The skill description says "5 or more approaches" but nothing actually counts them. + +**The consequence:** Auto-skills fire inconsistently. `auto-unblock` might never trigger despite 10 failed attempts. `auto-security` might miss a `git commit` because Claude was focused on the commit message, not the security scan. `auto-review` might not fire because Claude doesn't consider its work "significant enough." + +**The goal:** Add a second, objective triggering path that uses behavioral evidence from tool call patterns to recommend skill invocation — working alongside the existing description-matching mechanism, not replacing it. + +--- + +## Hook System Capabilities + +Claude Code's [hooks system](https://code.claude.com/docs/en/hooks) provides lifecycle event handlers that can observe and influence Claude's behavior. This section documents the capabilities relevant to activity monitoring. + +### Hook Events Available + +The table below lists hook events relevant to activity monitoring. Claude Code supports 17 hook events total — see the [full reference](https://code.claude.com/docs/en/hooks) for the complete list including `PermissionRequest`, `Notification`, `SubagentStart/Stop`, `TeammateIdle`, `TaskCompleted`, `ConfigChange`, `WorktreeCreate/Remove`, `PreCompact`, `InstructionsLoaded`, and `SessionEnd`. + +| Event | When It Fires | Matcher Support | Can Block? | +|-------|--------------|-----------------|------------| +| `PreToolUse` | Before a tool call executes | Tool name regex | Yes — deny/allow/ask | +| `PostToolUse` | After a tool call succeeds | Tool name regex | Feedback only (`decision: "block"` shows reason to Claude, but tool already ran) | +| `PostToolUseFailure` | After a tool call fails | Tool name regex | Feedback only (tool already failed) | +| `Stop` | When Claude finishes responding | No matcher (always fires) | Yes — force continuation | +| `SessionStart` | When a session begins/resumes | Source type | No | +| `SessionEnd` | When a session terminates | Session end reason | No (side effects only) | +| `UserPromptSubmit` | When the user submits a prompt | No matcher (always fires) | Yes — block prompt | + +### Hook Input Contract + +All hooks receive JSON via stdin with these common fields: + +| Field | Description | +|-------|-------------| +| `session_id` | Current session identifier | +| `transcript_path` | Path to conversation JSONL file | +| `cwd` | Current working directory | +| `permission_mode` | Current permission mode | +| `hook_event_name` | Name of the event that fired | + +**Tool events** (`PreToolUse`, `PostToolUse`, `PostToolUseFailure`) additionally include: + +| Field | Description | +|-------|-------------| +| `tool_name` | Name of the tool (e.g., `Bash`, `Edit`, `Write`) | +| `tool_input` | Tool-specific input object | +| `tool_response` | (PostToolUse only) Result from the tool | +| `tool_use_id` | Unique ID for the tool call | +| `error` | (PostToolUseFailure only) Error description | +| `is_interrupt` | (PostToolUseFailure only) Whether the failure was caused by user interruption | + +**Stop** hooks additionally include: + +| Field | Description | +|-------|-------------| +| `stop_hook_active` | Whether Claude is already continuing from a stop hook | +| `last_assistant_message` | Text of Claude's final response | + +### Hook Output Contract + +Hooks communicate back via exit codes and JSON stdout: + +**Exit codes:** +- `0` — success; stdout parsed for JSON output +- `2` — blocking error (PreToolUse blocks the tool, Stop forces continuation) +- Other — non-blocking error, execution continues + +**JSON output fields** (on exit 0): + +| Field | Description | +|-------|-------------| +| `systemMessage` | Warning message shown to the user | +| `decision` | `"block"` to prevent the action (Stop, PostToolUse) | +| `reason` | Explanation when decision is "block" | +| `continue` | `false` to stop Claude entirely | +| `hookSpecificOutput` | Event-specific structured output | + +**Key capability for auto-skills:** The `Stop` hook can return `{ "decision": "block", "reason": "..." }` to prevent Claude from stopping and force it to continue with the reason as context. This is the primary mechanism for injecting auto-skill recommendations — the `reason` field tells Claude what skill to invoke and why. + +### Hook Registration + +Hooks are defined in JSON settings files at multiple scopes: + +| Location | Scope | +|----------|-------| +| `~/.claude/settings.json` | All projects (user-level) | +| `.claude/settings.json` | Single project | +| `.claude/settings.local.json` | Single project, gitignored | +| Plugin `hooks/hooks.json` | When plugin is enabled | +| Skill/agent frontmatter | While component is active | + +### Constraints + +- **Synchronous execution:** Command hooks run synchronously; Claude waits for the result before proceeding. Must be fast. +- **Default timeout:** 600 seconds for command hooks. +- **`async: true`:** Hooks can run in the background without blocking, but then they cannot return decisions. +- **Environment variables:** `$CLAUDE_PROJECT_DIR` for project root, `$CLAUDE_PLUGIN_ROOT` for plugin root. +- **Transcript access:** All hooks receive `transcript_path` — the full conversation JSONL, which can be parsed for historical analysis. +- **Hook types beyond shell:** Claude Code also supports `type: "prompt"` (single-turn LLM evaluation returning yes/no) and `type: "agent"` (subagent with tool access) hooks. These could theoretically replace custom Node.js analysis — e.g., a Stop prompt hook asking "Is Claude stuck in a loop?" However, they add per-turn LLM API costs and latency, making them better suited as a future refinement than as the primary mechanism. See [Future Extensions](#future-extensions) in the design spec. +- **Exit code 2 varies by event:** Exit code 2 means different things for different hooks — e.g., PreToolUse blocks the tool call, Stop forces continuation, UserPromptSubmit blocks and erases the prompt. Not all events support blocking via exit code 2. + +--- + +## Three Approaches + +### Approach A: Pure Hook Accumulator (Shell-based) + +**Architecture:** Shell scripts handle all logic — event collection, pattern detection, and decision output. + +``` +PostToolUse → post-tool-use.sh → append event to JSONL + inline checks +PostToolUseFailure → post-failure.sh → append event to JSONL + inline checks +Stop → stop-hook.sh → read accumulated JSONL + pattern analysis via jq/awk +PreToolUse(Bash) → pre-bash.sh → check for git commit/push patterns +``` + +**How it works:** +1. `PostToolUse` and `PostToolUseFailure` hooks append structured events to `$TMPDIR/sidecar-monitor-$SESSION_ID.jsonl` using `jq` one-liners +2. Same hooks do inline pattern checks for immediate triggers (e.g., BMAD artifact writes) +3. `Stop` hook reads the accumulated event file, counts patterns with `jq`/`awk`, and decides whether to block stopping +4. `PreToolUse` hook on Bash checks if the command matches `git commit|push|gh pr create` + +**Example PostToolUse hook:** +```bash +#!/bin/bash +INPUT=$(cat) +TOOL=$(echo "$INPUT" | jq -r '.tool_name') +FILE=$(echo "$INPUT" | jq -r '.tool_input.file_path // empty') +SESSION=$(echo "$INPUT" | jq -r '.session_id') +EVENTS="$TMPDIR/sidecar-monitor-$SESSION.jsonl" + +# Append event +echo "$INPUT" | jq -c "{ts: now | todate, tool: .tool_name, file: (.tool_input.file_path // null), success: true}" >> "$EVENTS" + +# Quick check: BMAD artifact write +if [ "$TOOL" = "Write" ] || [ "$TOOL" = "Edit" ]; then + if echo "$FILE" | grep -q '_bmad-output/'; then + jq -n '{ hookSpecificOutput: { hookEventName: "PostToolUse", additionalContext: "IMPORTANT: A BMAD artifact was just written. Consider invoking sidecar-auto-bmad-method-check." } }' + fi +fi +exit 0 +``` + +**Pros:** +- Zero dependencies beyond `jq` (standard on most systems) +- Self-contained — no Node.js subprocess, no daemon +- Uses existing hook infrastructure directly +- Fast for simple immediate triggers + +**Cons:** +- Complex pattern detection (stuck loops, revert-like edits) is extremely fragile in shell/jq +- String comparison for error deduplication requires careful escaping +- No access to structured transcript parsing — would need to parse JSONL with jq +- Maintenance burden: shell scripts with complex logic are hard to test and debug +- Runs synchronously on every tool call — must stay fast + +**Best for:** Simple, immediate triggers where the pattern is a single event (git commit → auto-security, BMAD write → auto-bmad-check). + +--- + +### Approach B: Hook Collector + Node.js Analyzer (Recommended) + +**Architecture:** Lightweight shell hooks for event collection and immediate triggers; Node.js module for complex analysis at natural decision points (Stop, PreToolUse). + +``` +PostToolUse → post-tool-use.sh → fast event append + quick BMAD check +PostToolUseFailure → post-failure.sh → fast event append +Stop → stop-hook.sh → node hooks/analyze-patterns.js → block/allow +PreToolUse(Bash) → pre-bash.sh → check for git commit/push/pr +``` + +**How it works:** +1. `PostToolUse` / `PostToolUseFailure` hooks: fast shell scripts that append a structured event to the session's event file. Also handle immediate triggers (BMAD artifact writes) with quick regex checks. +2. `Stop` hook: shell script invokes `node hooks/analyze-patterns.js` with the transcript path and event file. The Node.js analyzer reads both files, runs pattern detection functions, and outputs a JSON decision. +3. `PreToolUse` (Bash): shell script checks if the command matches commit/push/PR patterns. Pure shell — no Node.js needed for this fast path. + +**Why analysis at Stop, not on every tool call:** +- The Stop hook fires when Claude finishes responding — the natural decision point for "should Claude do something else before it stops?" +- Analysis happens once per Claude turn, not once per tool call, so a ~200ms Node.js startup is acceptable +- The Stop hook has access to `last_assistant_message` (Claude's final response) which provides additional signal +- For patterns like "stuck loop" and "implementation complete," the relevant question is always "should Claude stop now, or should it invoke a skill first?" + +**Example Stop hook:** +```bash +#!/bin/bash +INPUT=$(cat) +SESSION=$(echo "$INPUT" | jq -r '.session_id') +TRANSCRIPT=$(echo "$INPUT" | jq -r '.transcript_path') +STOP_ACTIVE=$(echo "$INPUT" | jq -r '.stop_hook_active') +EVENTS="$TMPDIR/sidecar-monitor-$SESSION.jsonl" + +# Prevent infinite loops: if we already forced continuation, don't do it again +if [ "$STOP_ACTIVE" = "true" ]; then + exit 0 +fi + +# Run Node.js analyzer +node "$(dirname "$0")/analyze-patterns.js" "$TRANSCRIPT" "$EVENTS" "$SESSION" +``` + +**Node.js analyzer outputs:** +```json +{ + "decision": "block", + "reason": "IMPORTANT: You appear to be stuck in a debugging loop (5 edit→fail cycles on src/auth.js). Consider invoking the sidecar-auto-unblock skill to get fresh ideas from a different model." +} +``` + +**Pros:** +- Rich analysis: Node.js can parse JSONL transcripts, do fuzzy string matching, detect edit/revert cycles +- Runs at natural decision points (Stop = "should Claude keep going?") +- Shell hooks for fast-path triggers stay lightweight (<10ms per tool call) +- Testable: Node.js module with unit tests, mock transcripts +- Extensible: adding a new pattern is adding a function, not rewriting shell logic + +**Cons:** +- ~200ms Node.js startup for the analyzer (acceptable at Stop, not on every tool call) +- Requires Node.js on the system (safe assumption for sidecar users) +- Analysis only at Stop/PreToolUse points — not continuous +- Two languages (shell + Node.js) to maintain + +**Best for:** The full spectrum of auto-skill triggers. Simple patterns (git commit, BMAD writes) are handled by fast shell hooks. Complex patterns (stuck loops, implementation completion) are handled by the Node.js analyzer at Stop. + +--- + +### Approach C: Background Watcher Daemon + +**Architecture:** A persistent Node.js process runs alongside Claude Code, tailing the transcript file and maintaining real-time behavioral models. + +``` +SessionStart → start-watcher.sh → spawn background Node.js process +PostToolUse → post-tool-use.sh → append event (watcher also reads transcript) +Stop → stop-hook.sh → read watcher's recommendation file +SessionEnd → cleanup-watcher.sh → kill the daemon +``` + +**How it works:** +1. `SessionStart` hook spawns a background Node.js process that watches the transcript file (`fs.watch` on `transcript_path`) +2. The watcher maintains in-memory models: edit history per file, error frequency, tool call sequences +3. When patterns are detected, the watcher writes recommendations to `$TMPDIR/sidecar-recommendations-$SESSION_ID.json` +4. `Stop` hook reads the recommendations file and returns decisions +5. `SessionEnd` hook cleans up the daemon process + +**Pros:** +- Real-time detection — patterns are identified as they happen, not only at Stop +- Stateful models — can track complex sequences across many turns +- No startup cost at Stop — analysis is pre-computed +- Could enable cross-session learning (persist models between sessions) +- Future potential: proactive mid-turn coaching + +**Cons:** +- Daemon lifecycle management: must handle crashes, restarts, orphaned processes +- Still needs hooks for delivery — the daemon can't inject messages directly +- Most complex to implement and debug +- Resource usage: persistent Node.js process per session +- `fs.watch` behavior varies across OS/filesystem +- Overkill for the current set of patterns + +**Best for:** A hypothetical future with real-time coaching, proactive mid-turn suggestions, and cross-session pattern learning. Not justified for the current four auto-skills. + +--- + +## Pattern Catalog + +Specific detection patterns for each auto-skill, mapping to hook events and signal types. + +### auto-unblock + +| Hook | Pattern | Signal | Detection Method | +|------|---------|--------|-----------------| +| Stop | Edit→Bash(fail) cycles | 3+ cycles editing the same files followed by failing Bash commands | Parse transcript for interleaved Edit/Write→Bash sequences where Bash exits non-zero. Group by target file. | +| Stop | Same error repeated | Error string appears 3+ times in Bash results | Extract error strings from PostToolUseFailure events and Bash tool responses. Fuzzy-match for repeated patterns (strip line numbers, paths). | +| Stop | Revert-like edits | Edit `new_string` ≈ earlier `old_string` on same file | Compare accumulated Edit events: if a later edit's `new_string` closely matches an earlier edit's `old_string` on the same file, Claude is reverting. | +| Stop | Growing transcript without progress | Many tool calls, few new files/changes | Count unique file paths in Edit/Write events vs. total tool call count. High ratio = thrashing. | + +### auto-review + +| Hook | Pattern | Signal | Detection Method | +|------|---------|--------|-----------------| +| Stop | Implementation complete | Test pass after 5+ Edit/Write calls, Claude about to stop | Count Edit/Write calls in transcript. Check last Bash results for test-passing patterns (`passing`, `✓`, exit 0 after `npm test`/`pytest`). Claude's `last_assistant_message` contains completion language ("done", "implemented", "complete"). | +| Stop | Large change set | Many Edit/Write calls, shift from writes to reads | Track the ratio of write-tools to read-tools over recent turns. A shift from predominantly Edit/Write to Read/Grep suggests implementation is winding down. | +| Stop | Branch with uncommitted changes | `git status` shows modified files, Claude stopping | Check if recent Bash results contain `git status` output with modified files. | + +### auto-security + +| Hook | Pattern | Signal | Detection Method | +|------|---------|--------|-----------------| +| PreToolUse | Pre-commit gate | Bash command matches `git commit`, `git push`, `gh pr create` | Regex on `tool_input.command`: `/^\s*(git\s+(commit|push)|gh\s+pr\s+create)/`. Fast shell check. | +| PostToolUse | Staged files | `git add` detected | Regex on Bash `tool_input.command` for `git add`. Set a flag in the event file so the Stop hook knows files were staged. | + +### auto-bmad-check + +| Hook | Pattern | Signal | Detection Method | +|------|---------|--------|-----------------| +| PostToolUse | Artifact written | Write/Edit to `_bmad-output/` path | Check `tool_input.file_path` against `_bmad-output/` prefix. Immediate `additionalContext` trigger — no need to wait for Stop. | +| PostToolUse | Substantial artifact update | Edit with large `new_string` to `_bmad-output/` | Same path check, plus heuristic on content size (>500 chars of new content suggests substantive change, not a typo fix). | + +--- + +## Comparison Matrix + +| Dimension | A: Pure Shell | B: Hook + Node.js (Rec.) | C: Background Daemon | +|-----------|--------------|--------------------------|---------------------| +| **Implementation complexity** | Low | Medium | High | +| **Pattern detection reliability** | Low for complex patterns | High — full Node.js capabilities | Highest — stateful models | +| **Latency impact** | <10ms per hook (fast) | <10ms per tool hook; ~200ms at Stop | <10ms per hook (pre-computed) | +| **Maintenance burden** | High (shell complexity grows) | Medium (two languages, but testable) | High (daemon lifecycle) | +| **Extensibility** | Hard to add complex patterns | Easy — add a function | Easy — add a model | +| **Testability** | Hard (shell scripts) | Good (Node.js unit tests) | Good but integration is complex | +| **Dependencies** | `jq` only | Node.js + `jq` | Node.js + `jq` + process management | +| **Failure mode** | Silent (no output = allow) | Silent (analyzer crash = allow) | Daemon crash = no recommendations | +| **Resource usage** | Negligible | Negligible (Node.js only at Stop) | Persistent process per session | +| **Transcript analysis** | Fragile (jq on JSONL) | Native (Node.js JSONL parsing) | Native + real-time | +| **Cross-session learning** | Not feasible | Possible with state files | Natural fit | +| **Current auto-skill coverage** | 2/4 reliable (security, bmad) | 4/4 reliable | 4/4 reliable | + +--- + +## Recommendation: Approach B + +**Approach B (Hook Collector + Node.js Analyzer)** is recommended for the following reasons: + +1. **Right tool for the job.** Simple triggers (git commit, BMAD writes) use fast shell hooks. Complex triggers (stuck loops, implementation completion) use Node.js at the Stop hook — where a 200ms startup is negligible compared to Claude's response time. + +2. **Natural decision point.** The Stop hook answers exactly the right question: "Should Claude stop, or should it invoke an auto-skill first?" This is when all four auto-skills need to be evaluated. + +3. **Testable and maintainable.** The Node.js analyzer can be unit-tested with mock transcripts. Each pattern detector is an independent function. Adding a new pattern means adding a function and a test, not rewriting shell logic. + +4. **Incremental deployment.** Start with `PreToolUse` (auto-security) and `PostToolUse` (auto-bmad-check) — these are pure shell, zero risk. Then add the Stop hook with the Node.js analyzer for auto-unblock and auto-review. + +5. **Avoids daemon complexity.** Approach C's benefits (real-time detection, stateful models) aren't needed for the current four auto-skills. The Stop hook fires frequently enough — every time Claude finishes responding — to catch patterns in time. + +6. **Builds toward Approach C.** If future auto-skills need real-time detection, the event file and analyzer module from Approach B are directly reusable as the foundation for a daemon. + +### Migration path + +- **Phase 1:** Shell hooks for PreToolUse (security) and PostToolUse (BMAD) — immediate, no Node.js +- **Phase 2:** Stop hook with Node.js analyzer for unblock and review patterns +- **Phase 3 (future):** If needed, promote the analyzer to a persistent daemon (Approach C) + +--- + +## Phase 1 Must-Haves + +Beyond hook scripts and pattern detection, Phase 1 requires user-facing controls, configuration, and clear communication about what gets installed. This section covers the five must-haves that ship alongside the hooks themselves. + +### 1. Disabling Auto-Skills + +Two trigger paths exist, and both need disable controls: + +**Description-matching path** (SKILL.md-based): Each SKILL.md gains a Step 0 that checks `autoSkills..enabled` in sidecar config before doing anything. If the skill is disabled, it silently skips — no output, no side effects. + +**Activity monitoring path** (shell hooks): The hooks themselves check `monitoring.enabled` and per-pattern `enabled` flags in config before collecting events or making decisions. A disabled hook exits immediately with code 0. + +**Master kill switch:** Setting `autoSkills.enabled: false` disables both paths entirely — no SKILL.md Step 0 proceeds, no shell hook collects events or fires triggers. + +**Per-project override (future):** A `.sidecar.json` in the project root could override user-level config (e.g., disable auto-security for a trusted internal repo). Not in Phase 1, but the config namespace is designed to support it. + +**In-session:** Users can say "don't use auto-skills" in conversation. This works today — all auto-skills already ask for confirmation before proceeding, so Claude simply won't invoke them if told not to. No config change needed for this path. + +### 2. Config Namespace + +All auto-skill configuration lives under `autoSkills` in `~/.config/sidecar/config.json`: + +```json +{ + "autoSkills": { + "enabled": true, + "review": { "enabled": true }, + "unblock": { "enabled": true }, + "security": { "enabled": true }, + "bmadMethodCheck": { "enabled": true } + }, + "monitoring": { "enabled": true, "patterns": { "..." : "..." } } +} +``` + +Each SKILL.md's Step 0 reads this config via `sidecar_guide` or a dedicated config-check mechanism to determine whether to proceed. Shell hooks read the same config at the shell level (parsed with `jq` from the JSON file) to decide whether to collect events or fire triggers. + +The `monitoring` key controls the activity-monitoring path independently — it can be enabled while individual skills under `autoSkills` are disabled, or vice versa. This separation allows event collection to continue (for debugging or future analysis) even when specific skill triggers are turned off. + +### 3. CLI Command (`sidecar auto-skills`) + +A new CLI command provides the primary interface for managing auto-skill state: + +| Command | Effect | +|---------|--------| +| `sidecar auto-skills` | Lists status of all auto-skills (enabled/disabled) | +| `sidecar auto-skills --off` | Disables all auto-skills | +| `sidecar auto-skills --on` | Enables all auto-skills | +| `sidecar auto-skills --off review security` | Disables specific skills by name | +| `sidecar auto-skills --on unblock` | Enables a specific skill by name | + +All commands read and write `~/.config/sidecar/config.json`. The status display shows both the master switch and per-skill state, so users can see at a glance what's active. Invalid skill names produce a clear error listing valid options. + +### 4. First-Run Notice + +When `scripts/postinstall.js` registers hooks during installation, it prints a clear message to the terminal: + +- **What was installed:** Which hooks were added and where (e.g., "Registered PreToolUse and PostToolUse hooks in `~/.claude/settings.json`") +- **What they do:** One-sentence summary of the auto-skill system ("Auto-skills monitor tool usage patterns and suggest security scans, code reviews, and unblock assistance") +- **How to disable:** The exact command to turn everything off (`sidecar auto-skills --off`) +- **Where config lives:** The path to `~/.config/sidecar/config.json` + +This notice fires once — postinstall only runs on `npm install`. Users who install via global npm (`npm install -g claude-sidecar`) see it in their terminal. Users who clone and `npm install` locally see it there. + +### 5. Phase 1 vs Phase 2 Scope + +**Phase 1 (this PR):** The minimal viable set of hooks, config, and controls: + +- Shell hooks for `PreToolUse` (auto-security gate on git commit/push/PR creation) +- Shell hooks for `PostToolUse` (BMAD artifact trigger + event collection to session JSONL) +- `autoSkills` config namespace in `~/.config/sidecar/config.json` +- `sidecar auto-skills` CLI command for enable/disable management +- SKILL.md Step 0 checks in each auto-skill's frontmatter +- First-run notice in `postinstall.js` + +**Phase 2 (future PR):** The Node.js analyzer and complex pattern detection: + +- `Stop` hook invoking `node hooks/analyze-patterns.js` for decision-point analysis +- Auto-unblock pattern detection (edit→fail cycles, repeated errors, revert detection) +- Auto-review pattern detection (implementation completion, large change sets, test passing) +- Transcript parsing and fuzzy matching in the Node.js analyzer module +- Unit tests with mock transcripts for each pattern detector + +This split follows the recommendation in [Approach B](#approach-b-hook-collector--nodejs-analyzer-recommended): start with the fast shell hooks that carry zero risk, then layer in the sophisticated analysis once the foundation is proven. + +--- + +## References + +- [Claude Code Hooks Reference](https://code.claude.com/docs/en/hooks) — official hooks API documentation +- [docs/auto-skill-invocation-proposal.md](auto-skill-invocation-proposal.md) — existing proposal (Proposals A/B/C for skill discovery) +- `skill/auto-*/SKILL.md` — current trigger descriptions and thresholds +- `scripts/postinstall.js` — hook registration would be added here +- `src/mcp-tools.js` — MCP tool definitions for context diff --git a/docs/auto-skill-invocation-proposal.md b/docs/auto-skill-invocation-proposal.md new file mode 100644 index 0000000..1c08e74 --- /dev/null +++ b/docs/auto-skill-invocation-proposal.md @@ -0,0 +1,140 @@ +# Proposal: Improving Auto-Skill Invocation Reliability + +## Context + +PR #7 (`feat/auto-sidecar-skills`) introduces four auto-skills for sidecar: `auto-review`, `auto-unblock`, `auto-security`, and `auto-bmad-method-check`. These are contextual skills that fire automatically at key workflow moments (post-implementation review, stuck debugging, pre-commit security scan, BMAD artifact checkpoints). + +During development of the PR, we researched how Claude Code discovers and invokes skills, and found a significant reliability issue with how skills are installed. This proposal documents the findings and the changes made in the PR to address them, plus suggestions for further improvements that could be made in sidecar independently. + +## Summary + +Auto-skills need to be reliably discovered and invoked by Claude Code when their trigger conditions are met. The key finding: **Claude Code only scans top-level `~/.claude/skills/*/SKILL.md`** for its available skills list. Skills nested deeper than one level are invisible. + +## The Problem + +The initial version of the PR installed auto-skills as nested subdirectories under the main sidecar skill: + +```text +~/.claude/skills/sidecar/ ← discovered ✅ +~/.claude/skills/sidecar/auto-review/ ← NOT discovered ❌ +~/.claude/skills/sidecar/auto-unblock/ ← NOT discovered ❌ +~/.claude/skills/sidecar/auto-security/ ← NOT discovered ❌ +``` + +Nested skills don't appear in the system reminder Claude evaluates every turn, so Claude has no reason to invoke them unless something else prompts a skill check. + +## How Claude Code Skill Discovery Works + +Every turn, Claude sees a system reminder listing available skills with their `description` fields from SKILL.md frontmatter. This is the primary mechanism for contextual skill invocation — Claude pattern-matches descriptions against conversation state and invokes matching skills. + +The pipeline: +1. Skill tool scans `~/.claude/skills/*/SKILL.md` and plugin skill directories +2. Skill names + descriptions appear in the "available skills" system reminder +3. Claude evaluates descriptions against conversation state each turn +4. If a description matches → Claude reads the full SKILL.md and follows its procedure + +Skills not in this list are effectively invisible unless something else (like the Superpowers plugin) forces Claude to do a broader filesystem scan. + +## What's Been Addressed in PR #7 + +The following changes are included in the PR to address the discovery issue. + +### 1. Top-level installation + +Auto-skills are installed as top-level skill directories: + +```text +~/.claude/skills/sidecar-auto-review/ ← discovered ✅ +~/.claude/skills/sidecar-auto-unblock/ ← discovered ✅ +~/.claude/skills/sidecar-auto-security/ ← discovered ✅ +~/.claude/skills/sidecar-auto-bmad-method-check/ ← discovered ✅ +``` + +The `sidecar-` prefix keeps them namespaced. `postinstall.js` handles the installation and cleans up the old nested location on upgrade. + +### 2. Dual invocation: auto-fire + manual + +Each auto-skill now supports both: +- **Auto-fire** — Claude sees trigger descriptions in the skills list every turn and invokes when conditions match +- **Manual invocation** — users can type `/sidecar-auto-review`, `/sidecar-auto-security`, etc. to force invocation + +### 3. Cross-reference in main SKILL.md + +The main `skill/SKILL.md` now includes an "Auto-Skills" section with a table listing all four auto-skills and their trigger conditions. This gives Claude a second discovery path when reading the main sidecar skill. + +### Verified result + +After installing locally with these changes, the system reminder includes all four auto-skills: + +```text +- sidecar-auto-review: Use after completing a feature implementation, bug fix, or + significant code change — before claiming the work is done... +- sidecar-auto-unblock: Use when you have attempted 5 or more different approaches + to fix a bug, pass a test, or solve a problem and none have worked... +- sidecar-auto-security: Use when the user asks to commit changes, push code, or + create a pull request... +- sidecar-auto-bmad-method-check: Use when a BMAD-METHOD workflow has just produced + an output artifact... +``` + +## Suggestions for Further Improvement + +The following are ideas for future work in sidecar — separate from PR #7 — that could further improve auto-skill reliability. + +### Proposal A: SessionStart Hook + +**What:** Register a lightweight SessionStart hook that injects a compact auto-skills reminder into the system prompt on every session start, resume, clear, and compact. + +**Why:** This is the pattern used by the Superpowers plugin to achieve near-100% invocation reliability. Superpowers registers a synchronous SessionStart hook that injects skill-checking instructions into the system prompt. The skills list visibility from PR #7 is good, but a SessionStart hook would add a second independent triggering path — particularly valuable for users who don't have Superpowers installed. + +**Sketch:** A `hooks/hooks.json` registered during postinstall, with a SessionStart hook that injects a compact reminder listing the four auto-skills and their trigger conditions. The hook would fire on startup, resume, clear, and compact events. + +**Open question:** Does the Claude Code hooks API support system prompt injection for third-party packages, or only for plugins? This would affect the implementation approach. + +**Trade-off:** Adds ~200 bytes to the system prompt on every session. Minimal cost for significant reliability improvement. + +### Proposal B: Auto-Skills Config Namespace + +**What:** Add an `autoSkills` section to `~/.config/sidecar/config.json` for user-configurable enable/disable and defaults. + +**Why:** Users may want to disable specific auto-skills (e.g., skip security scans on personal projects) or set default models per skill (e.g., always use `gemini-pro` for code review). + +**Example config:** +```json +{ + "autoSkills": { + "review": { "enabled": true, "model": "gemini" }, + "unblock": { "enabled": true, "attemptThreshold": 5 }, + "security": { "enabled": true }, + "bmadMethodCheck": { "enabled": true, "artifactDir": "_bmad-output/" } + } +} +``` + +**How it would work:** Each auto-skill's SKILL.md would check config on invocation (via `sidecar_guide` or a new MCP tool) and skip if disabled. Default models from config would be used when the user doesn't specify one. + +**Trade-off:** Higher implementation effort. Probably best deferred until auto-skills have been used in practice and real user preferences emerge. + +### Proposal C: Community Auto-Skills + +**What:** Document the auto-skill convention so third parties can contribute their own. + +**Why:** The infrastructure already supports it — `postinstall.js` uses `fs.readdirSync` to dynamically discover `skill/auto-*` directories, so adding a new auto-skill is just adding a directory with a `SKILL.md`. No hardcoded lists to update. + +**Convention:** +1. Create `skill/auto-/SKILL.md` with standard frontmatter +2. Use `name: sidecar-auto-` in frontmatter +3. Include `TRIGGER when:` clause in description +4. Postinstall automatically discovers and installs it as `~/.claude/skills/sidecar-auto-/` + +**Trade-off:** Mainly a documentation effort. Could be a section in the README or a `CONTRIBUTING.md` addition. + +## Recommended Priority + +| Proposal | Effort | Impact | Suggested Timeline | +|----------|--------|--------|-------------------| +| **A: SessionStart hook** | Medium | High — guarantees awareness every session | Next PR | +| **B: Config namespace** | High | Medium — useful once patterns are established | After community feedback | +| **C: Community auto-skills** | Low | Medium — enables ecosystem growth | Anytime (docs only) | + +Happy to discuss any of these or help with implementation. The auto-skills PR (#7) is independent of these proposals — they're suggestions for future sidecar development regardless of whether the PR is merged as-is or modified. diff --git a/hooks/hooks.json b/hooks/hooks.json new file mode 100644 index 0000000..d977780 --- /dev/null +++ b/hooks/hooks.json @@ -0,0 +1,38 @@ +{ + "description": "Sidecar activity monitoring hooks for auto-skill triggers", + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "__HOOKS_DIR__/pre-bash.sh" + } + ] + } + ], + "PostToolUse": [ + { + "matcher": "Edit|Write|Bash|MultiEdit", + "hooks": [ + { + "type": "command", + "command": "__HOOKS_DIR__/post-tool-use.sh" + } + ] + } + ], + "PostToolUseFailure": [ + { + "matcher": "Bash|Edit|Write|MultiEdit", + "hooks": [ + { + "type": "command", + "command": "__HOOKS_DIR__/post-failure.sh" + } + ] + } + ] + } +} diff --git a/hooks/post-failure.sh b/hooks/post-failure.sh new file mode 100755 index 0000000..f969a10 --- /dev/null +++ b/hooks/post-failure.sh @@ -0,0 +1,57 @@ +#!/usr/bin/env bash +# PostToolUseFailure hook: failure event collection. +# +# Reads JSON from stdin (Claude Code hook contract). +# Appends a failure event to session JSONL for Phase 2 analysis. +# No triggers in Phase 1 — purely collecting data for the Stop hook analyzer. + +set -euo pipefail + +# Safety guard +[ -f "$0" ] || exit 0 + +# ── Config check ────────────────────────────────────────────────────── +# Note: jq's // operator treats false as falsy, so we use explicit type checks +CONFIG_PATH="${HOME}/.config/sidecar/config.json" +if [ -f "$CONFIG_PATH" ] && command -v jq >/dev/null 2>&1; then + MASTER=$(jq -r '.autoSkills.enabled | if type == "boolean" then . else true end' "$CONFIG_PATH" 2>/dev/null || echo "true") + MONITORING=$(jq -r '.monitoring.enabled | if type == "boolean" then . else true end' "$CONFIG_PATH" 2>/dev/null || echo "true") + if [ "$MASTER" = "false" ] || [ "$MONITORING" = "false" ]; then + exit 0 + fi +fi + +# ── Read stdin to temp file (avoid ARG_MAX on large payloads) ───────── +if ! command -v jq >/dev/null 2>&1; then + exit 0 +fi + +TMP_JSON=$(mktemp) +trap 'rm -f "$TMP_JSON"' EXIT +cat > "$TMP_JSON" + +SESSION_ID=$(jq -r '.session_id // ""' "$TMP_JSON" 2>/dev/null || echo "") +if [ -z "$SESSION_ID" ]; then + exit 0 +fi + +# ── Append failure event ───────────────────────────────────────────── +EVENT_FILE="${TMPDIR:-/tmp}/sidecar-monitor-${SESSION_ID}.jsonl" + +EVENT=$(jq -rc --arg ts "$(date -u +%Y-%m-%dT%H:%M:%SZ)" ' + { + ts: $ts, + tool: (.tool_name // "unknown"), + file: (if .tool_name == "MultiEdit" then (.tool_input.edits[0].file_path // "") + else (.tool_input.file_path // "") end), + success: false, + command: (if .tool_name == "Bash" then (.tool_input.command // "") else "" end), + errorSnippet: ((.error // .tool_response.error // .tool_response.stderr // "")[:500]) + }' "$TMP_JSON" 2>/dev/null || echo "") + +if [ -n "$EVENT" ]; then + if [ ! -f "$EVENT_FILE" ]; then + touch "$EVENT_FILE" && chmod 600 "$EVENT_FILE" + fi + echo "$EVENT" >> "$EVENT_FILE" +fi diff --git a/hooks/post-tool-use.sh b/hooks/post-tool-use.sh new file mode 100755 index 0000000..642d462 --- /dev/null +++ b/hooks/post-tool-use.sh @@ -0,0 +1,100 @@ +#!/usr/bin/env bash +# PostToolUse hook: event collection + BMAD artifact trigger. +# +# Reads JSON from stdin (Claude Code hook contract). +# 1. Appends a structured event to session JSONL file (for Phase 2 analysis). +# 2. If a Write/Edit/MultiEdit targets _bmad-output/, injects additionalContext. +# +# Phase 1: shell-only, no Node.js dependency. + +set -euo pipefail + +# Safety guard +[ -f "$0" ] || exit 0 + +# ── Config check ────────────────────────────────────────────────────── +# Note: jq's // operator treats false as falsy, so we use explicit type checks +CONFIG_PATH="${HOME}/.config/sidecar/config.json" +MONITORING_ON="true" +BMAD_ON="true" +MASTER_ON="true" + +if [ -f "$CONFIG_PATH" ] && command -v jq >/dev/null 2>&1; then + MASTER_ON=$(jq -r '.autoSkills.enabled | if type == "boolean" then . else true end' "$CONFIG_PATH" 2>/dev/null || echo "true") + BMAD_ON=$(jq -r '.autoSkills.bmadMethodCheck.enabled | if type == "boolean" then . else true end' "$CONFIG_PATH" 2>/dev/null || echo "true") + MONITORING_ON=$(jq -r '.monitoring.enabled | if type == "boolean" then . else true end' "$CONFIG_PATH" 2>/dev/null || echo "true") +fi + +# If master auto-skills switch or monitoring is off, skip everything +if [ "$MASTER_ON" = "false" ] || [ "$MONITORING_ON" = "false" ]; then + exit 0 +fi + +# ── Read stdin to temp file (avoid ARG_MAX on large payloads) ───────── +if ! command -v jq >/dev/null 2>&1; then + exit 0 +fi + +TMP_JSON=$(mktemp) +trap 'rm -f "$TMP_JSON"' EXIT +cat > "$TMP_JSON" + +TOOL_NAME=$(jq -r '.tool_name // ""' "$TMP_JSON" 2>/dev/null || echo "") +SESSION_ID=$(jq -r '.session_id // ""' "$TMP_JSON" 2>/dev/null || echo "") + +# ── Event collection ───────────────────────────────────────────────── +# Append structured event to session-specific JSONL file +if [ -n "$SESSION_ID" ]; then + EVENT_FILE="${TMPDIR:-/tmp}/sidecar-monitor-${SESSION_ID}.jsonl" + + EVENT=$(jq -rc --arg ts "$(date -u +%Y-%m-%dT%H:%M:%SZ)" ' + { + ts: $ts, + tool: (.tool_name // "unknown"), + file: (if .tool_name == "Bash" then "" + elif .tool_name == "MultiEdit" then (.tool_input.edits[0].file_path // "") + else (.tool_input.file_path // "") end), + success: (if .tool_name == "Bash" then ((.tool_response.exit_code // 0) == 0) else true end), + command: (if .tool_name == "Bash" then (.tool_input.command // "") else "" end) + }' "$TMP_JSON" 2>/dev/null || echo "") + + if [ -n "$EVENT" ]; then + # Create with restrictive permissions if new; append otherwise + if [ ! -f "$EVENT_FILE" ]; then + touch "$EVENT_FILE" && chmod 600 "$EVENT_FILE" + fi + echo "$EVENT" >> "$EVENT_FILE" + fi +fi + +# ── BMAD artifact trigger ──────────────────────────────────────────── +# Only fire if master + bmadMethodCheck are enabled +if [ "$MASTER_ON" = "false" ] || [ "$BMAD_ON" = "false" ]; then + exit 0 +fi + +# Check if a Write, Edit, or MultiEdit targeted _bmad-output/ +# MultiEdit uses .tool_input.edits[].file_path; Write/Edit use .tool_input.file_path +if [ "$TOOL_NAME" = "Write" ] || [ "$TOOL_NAME" = "Edit" ] || [ "$TOOL_NAME" = "MultiEdit" ]; then + HAS_BMAD=$(jq -r ' + [.tool_input.file_path, (.tool_input.edits[]?.file_path)] + | map(select(. != null and contains("_bmad-output/"))) + | length > 0' "$TMP_JSON" 2>/dev/null || echo "false") + + if [ "$HAS_BMAD" = "true" ]; then + # Extract a representative file path for the message + BMAD_FILE=$(jq -r ' + [.tool_input.file_path, (.tool_input.edits[]?.file_path)] + | map(select(. != null and contains("_bmad-output/"))) + | first' "$TMP_JSON" 2>/dev/null || echo "_bmad-output/") + + cat </dev/null 2>&1; then + MASTER=$(jq -r '.autoSkills.enabled | if type == "boolean" then . else true end' "$CONFIG_PATH" 2>/dev/null || echo "true") + SECURITY=$(jq -r '.autoSkills.security.enabled | if type == "boolean" then . else true end' "$CONFIG_PATH" 2>/dev/null || echo "true") + MONITORING=$(jq -r '.monitoring.enabled | if type == "boolean" then . else true end' "$CONFIG_PATH" 2>/dev/null || echo "true") + if [ "$MASTER" = "false" ] || [ "$SECURITY" = "false" ] || [ "$MONITORING" = "false" ]; then + exit 0 + fi +fi + +# ── Read stdin to temp file (avoid ARG_MAX on large payloads) ───────── +TMP_JSON=$(mktemp) +trap 'rm -f "$TMP_JSON"' EXIT +cat > "$TMP_JSON" + +# Extract the Bash command from tool_input.command +COMMAND="" +if command -v jq >/dev/null 2>&1; then + COMMAND=$(jq -r '.tool_input.command // ""' "$TMP_JSON" 2>/dev/null || echo "") +fi + +# If jq not available or command empty, allow through +if [ -z "$COMMAND" ]; then + exit 0 +fi + +# ── Pattern match ───────────────────────────────────────────────────── +# Check for git commit, git push, gh pr create +# Uses printf (not echo) to avoid flag injection with commands starting with -n/-e +# Includes semicolons in boundary pattern to catch "git add .; git commit" +IS_COMMIT=false +if printf '%s\n' "$COMMAND" | grep -qE '(^|[;&|]+\s*)git\s+commit(\s|$)'; then + IS_COMMIT=true +fi +if printf '%s\n' "$COMMAND" | grep -qE '(^|[;&|]+\s*)git\s+push(\s|$)'; then + IS_COMMIT=true +fi +if printf '%s\n' "$COMMAND" | grep -qE '(^|[;&|]+\s*)gh\s+pr\s+create(\s|$)'; then + IS_COMMIT=true +fi + +if [ "$IS_COMMIT" = "false" ]; then + exit 0 +fi + +# ── Output recommendation ──────────────────────────────────────────── +# Use hookSpecificOutput.additionalContext to reach Claude's context +cat <<'HOOK_OUTPUT' +{ + "hookSpecificOutput": { + "additionalContext": "IMPORTANT: A git commit, push, or PR creation was detected. Before proceeding, consider running the sidecar-auto-security skill to scan these changes for security vulnerabilities (hardcoded secrets, injection flaws, auth bypass). You can invoke it with: use the Skill tool with skill 'sidecar-auto-security'. If the user has explicitly declined a security scan for these changes, proceed without one." + } +} +HOOK_OUTPUT diff --git a/package.json b/package.json index 268c667..58de514 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,9 @@ "src/", "electron/", "skill/", - "scripts/postinstall.js" + "hooks/", + "scripts/postinstall.js", + "scripts/preuninstall.js" ], "scripts": { "start": "node --experimental-top-level-await --experimental-vm-modules bin/sidecar.js", @@ -44,6 +46,7 @@ "posttest": "git rev-parse HEAD > .test-passed 2>/dev/null || true", "lint": "eslint src/", "postinstall": "node scripts/postinstall.js", + "preuninstall": "node scripts/preuninstall.js", "test:thinking": "node scripts/benchmark-thinking.js", "test:thinking:quick": "MODELS=gemini node scripts/benchmark-thinking.js", "refresh-models": "node scripts/refresh-model-capabilities.js", diff --git a/scripts/postinstall.js b/scripts/postinstall.js index 6f1b206..0abe7dd 100644 --- a/scripts/postinstall.js +++ b/scripts/postinstall.js @@ -148,11 +148,94 @@ function registerClaudeDesktop() { } } +/** + * Register activity monitoring hooks in ~/.claude/settings.json. + * Resolves absolute paths to hook scripts at install time. + * Merges sidecar hooks without overwriting existing user hooks. + */ +function registerHooks() { + const hooksDir = path.join(__dirname, '..', 'hooks'); + const hooksConfigPath = path.join(hooksDir, 'hooks.json'); + if (!fs.existsSync(hooksConfigPath)) { return; } + + let hooksConfig; + try { + const raw = fs.readFileSync(hooksConfigPath, 'utf-8'); + // Escape backslashes for Windows paths before injecting into JSON + const safeHooksDir = hooksDir.replace(/\\/g, '\\\\'); + hooksConfig = JSON.parse(raw.replace(/__HOOKS_DIR__/g, safeHooksDir)); + } catch (err) { + console.error(`[claude-sidecar] Warning: Could not read hooks config: ${err.message}`); + return; + } + + const settingsPath = path.join(os.homedir(), '.claude', 'settings.json'); + let settings = {}; + try { + settings = JSON.parse(fs.readFileSync(settingsPath, 'utf-8')); + } catch { + // File doesn't exist or invalid — start fresh + } + + if (!settings.hooks) { settings.hooks = {}; } + + // Merge each hook event, appending sidecar hooks after existing ones + let registered = 0; + for (const [event, matchers] of Object.entries(hooksConfig.hooks || {})) { + if (!settings.hooks[event]) { settings.hooks[event] = []; } + + for (const matcher of matchers) { + const cmd = (matcher.hooks && matcher.hooks[0] && matcher.hooks[0].command) || ''; + // Skip if this exact hook command is already registered + const alreadyExists = settings.hooks[event].some((existing) => { + return existing.hooks && existing.hooks.some((h) => h.command === cmd); + }); + if (!alreadyExists) { + settings.hooks[event].push(matcher); + registered++; + } + } + } + + if (registered > 0) { + const settingsDir = path.dirname(settingsPath); + if (!fs.existsSync(settingsDir)) { + fs.mkdirSync(settingsDir, { recursive: true, mode: 0o700 }); + } + fs.writeFileSync(settingsPath, JSON.stringify(settings, null, 2), { mode: 0o600 }); + } + + // First-run notice + console.log(''); + console.log('[claude-sidecar] Activity monitoring hooks registered:'); + console.log(' - PreToolUse: auto-security gate (git commit/push/PR)'); + console.log(' - PostToolUse: BMAD artifact trigger + event collection'); + console.log(' - PostToolUseFailure: failure event collection'); + console.log(''); + console.log(' Auto-skills suggest security scans, code reviews, and unblock'); + console.log(' assistance at key workflow moments.'); + console.log(''); + console.log(' To disable: sidecar auto-skills --off'); + console.log(' Config: ~/.config/sidecar/config.json'); +} + function main() { console.log('[claude-sidecar] Installing...'); installSkill(); registerClaudeCode(); registerClaudeDesktop(); + registerHooks(); + + // Warn if jq is not available (hooks degrade gracefully but lose functionality) + try { + execFileSync('jq', ['--version'], { stdio: 'pipe', timeout: 5000 }); + } catch { + console.log(''); + console.log('[claude-sidecar] Warning: `jq` is not installed.'); + console.log(' Activity monitoring hooks require jq for JSON parsing.'); + console.log(' Install it: brew install jq (macOS) or apt install jq (Linux)'); + console.log(' Without jq, hooks will degrade gracefully (auto-skills still work via description-matching).'); + } console.log(''); console.log('[claude-sidecar] Setup:'); @@ -165,4 +248,4 @@ if (require.main === module) { main(); } -module.exports = { addMcpToConfigFile }; +module.exports = { addMcpToConfigFile, registerHooks }; diff --git a/scripts/preuninstall.js b/scripts/preuninstall.js new file mode 100644 index 0000000..ee3c007 --- /dev/null +++ b/scripts/preuninstall.js @@ -0,0 +1,65 @@ +#!/usr/bin/env node + +/** + * Pre-uninstall script for claude-sidecar + * + * Removes activity monitoring hooks from ~/.claude/settings.json + * that were registered by postinstall.js. Leaves other user hooks intact. + */ + +const fs = require('fs'); +const path = require('path'); +const os = require('os'); + +/** Sidecar hook script filenames — used to identify our hooks regardless of install path */ +const SIDECAR_HOOK_SCRIPTS = ['pre-bash.sh', 'post-tool-use.sh', 'post-failure.sh', 'stop-hook.sh']; + +function isSidecarHookCommand(command) { + if (!command || typeof command !== 'string') { return false; } + const basename = path.basename(command); + return SIDECAR_HOOK_SCRIPTS.includes(basename); +} + +function removeHooks() { + const settingsPath = path.join(os.homedir(), '.claude', 'settings.json'); + let settings; + try { + settings = JSON.parse(fs.readFileSync(settingsPath, 'utf-8')); + } catch { + return; // No settings file — nothing to clean up + } + + if (!settings.hooks) { return; } + + let removed = 0; + for (const [event, matchers] of Object.entries(settings.hooks)) { + if (!Array.isArray(matchers)) { continue; } + const filtered = matchers.filter((matcher) => { + const hooks = matcher.hooks || []; + const isSidecar = hooks.some((h) => isSidecarHookCommand(h.command)); + if (isSidecar) { removed++; } + return !isSidecar; + }); + if (filtered.length === 0) { + delete settings.hooks[event]; + } else { + settings.hooks[event] = filtered; + } + } + + // Clean up empty hooks object + if (Object.keys(settings.hooks).length === 0) { + delete settings.hooks; + } + + if (removed > 0) { + fs.writeFileSync(settingsPath, JSON.stringify(settings, null, 2), { mode: 0o600 }); + console.log(`[claude-sidecar] Removed ${removed} hook(s) from ~/.claude/settings.json`); + } +} + +if (require.main === module) { + removeHooks(); +} + +module.exports = { removeHooks }; diff --git a/skill/auto-bmad-method-check/SKILL.md b/skill/auto-bmad-method-check/SKILL.md index 0517e4d..c97d656 100644 --- a/skill/auto-bmad-method-check/SKILL.md +++ b/skill/auto-bmad-method-check/SKILL.md @@ -51,9 +51,11 @@ This skill covers planning and key implementation artifacts where a second opini ## Procedure -### Step 0: Verify BMAD-METHOD installation +### Step 0: Check config and verify BMAD-METHOD installation -Before anything else, check that the BMAD-METHOD is installed in the current project: +First, call `mcp__sidecar__sidecar_guide` with the question "Is auto-skill 'bmadMethodCheck' enabled?" If the guide response indicates the skill is disabled (either master switch off or per-skill disabled), **silently skip this entire skill** — produce no output, no prompt, no side effects. Just stop here. If `sidecar_guide` is unavailable, proceed (assume enabled by default). + +Then check that the BMAD-METHOD is installed in the current project: 1. Look for `_bmad/bmm/config.yaml` in the project root 2. Look for `_bmad-output/` directory diff --git a/skill/auto-review/SKILL.md b/skill/auto-review/SKILL.md index c56d51c..cbf4052 100644 --- a/skill/auto-review/SKILL.md +++ b/skill/auto-review/SKILL.md @@ -31,6 +31,12 @@ Before claiming work is complete, offer to spawn a headless sidecar to get a sec ## Procedure +### Step 0: Check if this skill is enabled + +Call `mcp__sidecar__sidecar_guide` with the question "Is auto-skill 'review' enabled?" If the guide response indicates the skill is disabled (either master switch off or per-skill disabled), **silently skip this entire skill** — produce no output, no prompt, no side effects. Just stop here. + +If `sidecar_guide` is unavailable, proceed (assume enabled by default). + ### Step 1: Discover available models and prompt the user Before presenting the prompt, call `mcp__sidecar__sidecar_guide` to get the configured model alias table. Extract the alias names (e.g., `gemini`, `gpt`, `opus`) from the guide output. If the guide call fails, fall back to: "your configured models (run `sidecar setup` to see them)". diff --git a/skill/auto-security/SKILL.md b/skill/auto-security/SKILL.md index 1e74e58..149f789 100644 --- a/skill/auto-security/SKILL.md +++ b/skill/auto-security/SKILL.md @@ -33,6 +33,12 @@ Before committing, pushing, or creating a PR, offer to spawn a headless sidecar ## Procedure +### Step 0: Check if this skill is enabled + +Call `mcp__sidecar__sidecar_guide` with the question "Is auto-skill 'security' enabled?" If the guide response indicates the skill is disabled (either master switch off or per-skill disabled), **silently skip this entire skill** — produce no output, no prompt, no side effects. Just stop here. + +If `sidecar_guide` is unavailable, proceed (assume enabled by default). + ### Step 1: Discover available models and prompt the user Before presenting the prompt, call `mcp__sidecar__sidecar_guide` to get the configured model alias table. Extract the alias names (e.g., `gemini`, `gpt`, `opus`) from the guide output. If the guide call fails, fall back to: "your configured models (run `sidecar setup` to see them)". diff --git a/skill/auto-unblock/SKILL.md b/skill/auto-unblock/SKILL.md index 5c6b382..74d696d 100644 --- a/skill/auto-unblock/SKILL.md +++ b/skill/auto-unblock/SKILL.md @@ -39,6 +39,12 @@ When you're stuck — multiple approaches tried, all failing — offer to spawn ## Procedure +### Step 0: Check if this skill is enabled + +Call `mcp__sidecar__sidecar_guide` with the question "Is auto-skill 'unblock' enabled?" If the guide response indicates the skill is disabled (either master switch off or per-skill disabled), **silently skip this entire skill** — produce no output, no prompt, no side effects. Just stop here. + +If `sidecar_guide` is unavailable, proceed (assume enabled by default). + ### Step 1: Discover available models and prompt the user Before presenting the prompt, call `mcp__sidecar__sidecar_guide` to get the configured model alias table. Extract the alias names (e.g., `gemini`, `gpt`, `opus`) from the guide output. If the guide call fails, fall back to: "your configured models (run `sidecar setup` to see them)". diff --git a/src/cli-handlers.js b/src/cli-handlers.js index 5e125e2..3b99111 100644 --- a/src/cli-handlers.js +++ b/src/cli-handlers.js @@ -5,6 +5,7 @@ * under the 300-line limit. */ +/* eslint-disable no-console */ const fs = require('fs'); const path = require('path'); const { validateTaskId, safeSessionDir } = require('./utils/validators'); @@ -105,7 +106,7 @@ async function handleUpdate() { } const result = await performUpdate(); if (result.success) { - console.log(`Updated successfully! Run 'sidecar --version' to verify.`); + console.log('Updated successfully! Run \'sidecar --version\' to verify.'); } else { console.error(`Update failed: ${result.error}`); process.exit(1); @@ -121,9 +122,68 @@ async function handleMcp() { await startMcpServer(); } +/** + * Handle 'sidecar auto-skills' command + * Lists status or enables/disables auto-skills + */ +function handleAutoSkills(args) { + const { + getAutoSkillsStatus, + setAutoSkillsEnabled, + resolveSkillNames, + VALID_SKILL_NAMES, + SKILL_LABELS, + } = require('./utils/auto-skills-config'); + + const on = args.on; + const off = args.off; + const skillArgs = args._.slice(1); + + // Reject mutually exclusive flags + if (on && off) { + console.error('Error: --on and --off cannot be used together'); + process.exit(1); + } + + // Reject positional args without --on/--off + if (!on && !off && skillArgs.length > 0) { + console.error('Error: specify --on or --off with skill names'); + console.error('Usage: sidecar auto-skills --off review security'); + process.exit(1); + } + + // No flags — show status + if (!on && !off) { + console.log(getAutoSkillsStatus()); + return; + } + + const enabled = !!on; + + if (skillArgs.length === 0) { + // Master switch + setAutoSkillsEnabled(enabled); + console.log(`Auto-skills ${enabled ? 'enabled' : 'disabled'}.`); + return; + } + + const { valid, invalid } = resolveSkillNames(skillArgs); + if (invalid.length > 0) { + const validNames = VALID_SKILL_NAMES.map((k) => SKILL_LABELS[k]).join(', '); + console.error(`Unknown skill(s): ${invalid.join(', ')}`); + console.error(`Valid names: ${validNames}`); + process.exit(1); + } + + setAutoSkillsEnabled(enabled, valid); + const labels = valid.map((k) => SKILL_LABELS[k]).join(', '); + console.log(`${labels}: ${enabled ? 'enabled' : 'disabled'}.`); +} + module.exports = { handleSetup, handleAbort, handleUpdate, handleMcp, + handleAutoSkills, }; diff --git a/src/cli.js b/src/cli.js index f928b68..99ab4e7 100644 --- a/src/cli.js +++ b/src/cli.js @@ -95,7 +95,9 @@ function isBooleanFlag(key) { 'version', 'help', 'api-keys', - 'validate-model' + 'validate-model', + 'on', + 'off' ]; return booleanFlags.includes(key); } @@ -284,6 +286,7 @@ Commands: abort Abort a running sidecar session setup Configure default model and aliases --api-keys Open API key setup window + auto-skills List/enable/disable auto-skills update Update to latest version mcp Start MCP server (stdio transport) @@ -326,6 +329,11 @@ Options for 'read': --summary Show summary (default) --conversation Show full conversation +Options for 'auto-skills': + --on [skill ...] Enable all or specific auto-skills + --off [skill ...] Disable all or specific auto-skills + (no flags) Show current status + OpenCode Agent Types: Chat Reads auto, writes/bash ask permission (interactive default) Build Full tool access (headless default) diff --git a/src/utils/auto-skills-config.js b/src/utils/auto-skills-config.js new file mode 100644 index 0000000..2e22ad9 --- /dev/null +++ b/src/utils/auto-skills-config.js @@ -0,0 +1,152 @@ +/** + * Auto-Skills Configuration Module + * + * Manages the autoSkills namespace in ~/.config/sidecar/config.json. + * Provides enable/disable controls and status reporting for auto-skills. + */ + +const { loadConfig, saveConfig } = require('./config'); + +/** Valid auto-skill names (keys in autoSkills config) */ +const VALID_SKILL_NAMES = ['review', 'unblock', 'security', 'bmadMethodCheck']; + +/** Display labels for CLI output */ +const SKILL_LABELS = { + review: 'auto-review', + unblock: 'auto-unblock', + security: 'auto-security', + bmadMethodCheck: 'auto-bmad-method-check', +}; + +/** + * Get the autoSkills section from config, with defaults applied. + * @param {object|null} [config] - Config object (loaded if not provided) + * @returns {object} autoSkills config with defaults + */ +function getAutoSkillsConfig(config) { + const cfg = config || loadConfig() || {}; + const autoSkills = cfg.autoSkills || {}; + return { + enabled: autoSkills.enabled !== false, + review: { enabled: (autoSkills.review || {}).enabled !== false }, + unblock: { enabled: (autoSkills.unblock || {}).enabled !== false }, + security: { enabled: (autoSkills.security || {}).enabled !== false }, + bmadMethodCheck: { enabled: (autoSkills.bmadMethodCheck || {}).enabled !== false }, + }; +} + +/** + * Check if a specific auto-skill is enabled (master + per-skill). + * @param {string} skillName - One of VALID_SKILL_NAMES + * @param {object|null} [config] - Config object (loaded if not provided) + * @returns {boolean} + */ +function isSkillEnabled(skillName, config) { + const as = getAutoSkillsConfig(config); + if (!as.enabled) { return false; } + const skill = as[skillName]; + return skill ? skill.enabled !== false : true; +} + +/** + * Check if activity monitoring is enabled. + * @param {object|null} [config] - Config object (loaded if not provided) + * @returns {boolean} + */ +function isMonitoringEnabled(config) { + const cfg = config || loadConfig() || {}; + const monitoring = cfg.monitoring || {}; + return monitoring.enabled !== false; +} + +/** + * Enable or disable auto-skills and save to config. + * @param {boolean} enabled - Whether to enable or disable + * @param {string[]} [skillNames] - Specific skills, or empty for master switch + */ +function setAutoSkillsEnabled(enabled, skillNames) { + const config = loadConfig() || {}; + if (!config.autoSkills) { config.autoSkills = {}; } + + if (!skillNames || skillNames.length === 0) { + config.autoSkills.enabled = enabled; + } else { + for (const name of skillNames) { + if (!config.autoSkills[name]) { config.autoSkills[name] = {}; } + config.autoSkills[name].enabled = enabled; + } + } + + saveConfig(config); +} + +/** + * Get formatted status of all auto-skills for CLI display. + * @returns {string} Multi-line status string + */ +function getAutoSkillsStatus() { + const as = getAutoSkillsConfig(); + const lines = []; + const masterLabel = as.enabled ? 'enabled' : 'disabled'; + lines.push(`Auto-skills: ${masterLabel}`); + lines.push(''); + + for (const name of VALID_SKILL_NAMES) { + const label = SKILL_LABELS[name]; + const skillEnabled = as[name].enabled; + const effective = as.enabled && skillEnabled; + let status = effective ? 'enabled' : 'disabled'; + if (as.enabled && !skillEnabled) { status = 'disabled (per-skill)'; } + if (!as.enabled && skillEnabled) { status = 'disabled (master off)'; } + lines.push(` ${label}: ${status}`); + } + + return lines.join('\n'); +} + +/** + * Validate skill names from CLI input, mapping display names to config keys. + * @param {string[]} names - Skill names from CLI (e.g., 'review', 'auto-review') + * @returns {{ valid: string[], invalid: string[] }} + */ +function resolveSkillNames(names) { + // Build case-insensitive lookup from display labels to config keys + const reverseLabels = {}; + for (const [key, label] of Object.entries(SKILL_LABELS)) { + reverseLabels[label] = key; + reverseLabels[label.replace('auto-', '')] = key; + } + // Also accept 'bmad' shorthand + reverseLabels['bmad'] = 'bmadMethodCheck'; + + // Build case-insensitive lookup for VALID_SKILL_NAMES (handles camelCase input) + const validLower = VALID_SKILL_NAMES.map((n) => n.toLowerCase()); + + const valid = []; + const invalid = []; + + for (const name of names) { + const lower = name.toLowerCase(); + const validIdx = validLower.indexOf(lower); + if (validIdx !== -1) { + valid.push(VALID_SKILL_NAMES[validIdx]); + } else if (reverseLabels[lower]) { + valid.push(reverseLabels[lower]); + } else { + invalid.push(name); + } + } + + return { valid, invalid }; +} + +module.exports = { + VALID_SKILL_NAMES, + SKILL_LABELS, + getAutoSkillsConfig, + isSkillEnabled, + isMonitoringEnabled, + setAutoSkillsEnabled, + getAutoSkillsStatus, + resolveSkillNames, +}; diff --git a/tests/auto-skills-config.test.js b/tests/auto-skills-config.test.js new file mode 100644 index 0000000..1fb1285 --- /dev/null +++ b/tests/auto-skills-config.test.js @@ -0,0 +1,208 @@ +/** + * Auto-Skills Config Module Tests + * + * Tests for auto-skill enable/disable, status reporting, and name resolution. + */ + +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +describe('Auto-Skills Config Module', () => { + let tempDir; + let originalEnv; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sidecar-autoskills-test-')); + originalEnv = { ...process.env }; + process.env.SIDECAR_CONFIG_DIR = tempDir; + jest.resetModules(); + }); + + afterEach(() => { + process.env = originalEnv; + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + function loadModule() { + return require('../src/utils/auto-skills-config'); + } + + function writeConfig(data) { + fs.writeFileSync( + path.join(tempDir, 'config.json'), + JSON.stringify(data, null, 2) + ); + } + + describe('getAutoSkillsConfig', () => { + test('returns all enabled by default when no config exists', () => { + const { getAutoSkillsConfig } = loadModule(); + const config = getAutoSkillsConfig(); + expect(config.enabled).toBe(true); + expect(config.review.enabled).toBe(true); + expect(config.unblock.enabled).toBe(true); + expect(config.security.enabled).toBe(true); + expect(config.bmadMethodCheck.enabled).toBe(true); + }); + + test('respects master disable', () => { + writeConfig({ autoSkills: { enabled: false } }); + const { getAutoSkillsConfig } = loadModule(); + const config = getAutoSkillsConfig(); + expect(config.enabled).toBe(false); + }); + + test('respects per-skill disable', () => { + writeConfig({ autoSkills: { security: { enabled: false } } }); + const { getAutoSkillsConfig } = loadModule(); + const config = getAutoSkillsConfig(); + expect(config.enabled).toBe(true); + expect(config.security.enabled).toBe(false); + expect(config.review.enabled).toBe(true); + }); + }); + + describe('isSkillEnabled', () => { + test('returns true when no config', () => { + const { isSkillEnabled } = loadModule(); + expect(isSkillEnabled('review')).toBe(true); + }); + + test('returns false when master is off', () => { + writeConfig({ autoSkills: { enabled: false } }); + const { isSkillEnabled } = loadModule(); + expect(isSkillEnabled('review')).toBe(false); + }); + + test('returns false when per-skill is off', () => { + writeConfig({ autoSkills: { review: { enabled: false } } }); + const { isSkillEnabled } = loadModule(); + expect(isSkillEnabled('review')).toBe(false); + expect(isSkillEnabled('security')).toBe(true); + }); + + test('returns false when both master and per-skill are off', () => { + writeConfig({ autoSkills: { enabled: false, review: { enabled: false } } }); + const { isSkillEnabled } = loadModule(); + expect(isSkillEnabled('review')).toBe(false); + }); + }); + + describe('isMonitoringEnabled', () => { + test('returns true when no config', () => { + const { isMonitoringEnabled } = loadModule(); + expect(isMonitoringEnabled()).toBe(true); + }); + + test('returns false when monitoring disabled', () => { + writeConfig({ monitoring: { enabled: false } }); + const { isMonitoringEnabled } = loadModule(); + expect(isMonitoringEnabled()).toBe(false); + }); + }); + + describe('setAutoSkillsEnabled', () => { + test('sets master switch off', () => { + const { setAutoSkillsEnabled, getAutoSkillsConfig } = loadModule(); + setAutoSkillsEnabled(false); + const config = getAutoSkillsConfig(); + expect(config.enabled).toBe(false); + }); + + test('sets master switch on', () => { + writeConfig({ autoSkills: { enabled: false } }); + const { setAutoSkillsEnabled, getAutoSkillsConfig } = loadModule(); + setAutoSkillsEnabled(true); + const config = getAutoSkillsConfig(); + expect(config.enabled).toBe(true); + }); + + test('disables specific skills', () => { + const { setAutoSkillsEnabled, isSkillEnabled } = loadModule(); + setAutoSkillsEnabled(false, ['review', 'security']); + expect(isSkillEnabled('review')).toBe(false); + expect(isSkillEnabled('security')).toBe(false); + expect(isSkillEnabled('unblock')).toBe(true); + }); + + test('enables specific skills', () => { + writeConfig({ autoSkills: { review: { enabled: false } } }); + const { setAutoSkillsEnabled, isSkillEnabled } = loadModule(); + setAutoSkillsEnabled(true, ['review']); + expect(isSkillEnabled('review')).toBe(true); + }); + + test('creates config file if missing', () => { + const { setAutoSkillsEnabled } = loadModule(); + setAutoSkillsEnabled(false, ['unblock']); + const raw = JSON.parse(fs.readFileSync(path.join(tempDir, 'config.json'), 'utf-8')); + expect(raw.autoSkills.unblock.enabled).toBe(false); + }); + }); + + describe('getAutoSkillsStatus', () => { + test('shows all enabled by default', () => { + const { getAutoSkillsStatus } = loadModule(); + const status = getAutoSkillsStatus(); + expect(status).toContain('Auto-skills: enabled'); + expect(status).toContain('auto-review: enabled'); + expect(status).toContain('auto-security: enabled'); + }); + + test('shows master off with per-skill detail', () => { + writeConfig({ autoSkills: { enabled: false } }); + const { getAutoSkillsStatus } = loadModule(); + const status = getAutoSkillsStatus(); + expect(status).toContain('Auto-skills: disabled'); + expect(status).toContain('disabled (master off)'); + }); + + test('shows per-skill disabled', () => { + writeConfig({ autoSkills: { security: { enabled: false } } }); + const { getAutoSkillsStatus } = loadModule(); + const status = getAutoSkillsStatus(); + expect(status).toContain('auto-security: disabled (per-skill)'); + }); + }); + + describe('resolveSkillNames', () => { + test('resolves canonical names', () => { + const { resolveSkillNames } = loadModule(); + const result = resolveSkillNames(['review', 'security']); + expect(result.valid).toEqual(['review', 'security']); + expect(result.invalid).toEqual([]); + }); + + test('resolves display names (auto- prefix)', () => { + const { resolveSkillNames } = loadModule(); + const result = resolveSkillNames(['auto-review', 'auto-unblock']); + expect(result.valid).toEqual(['review', 'unblock']); + }); + + test('resolves bmad shorthand', () => { + const { resolveSkillNames } = loadModule(); + const result = resolveSkillNames(['bmad']); + expect(result.valid).toEqual(['bmadMethodCheck']); + }); + + test('resolves camelCase config key (bmadMethodCheck)', () => { + const { resolveSkillNames } = loadModule(); + const result = resolveSkillNames(['bmadMethodCheck']); + expect(result.valid).toEqual(['bmadMethodCheck']); + }); + + test('resolves case-insensitively', () => { + const { resolveSkillNames } = loadModule(); + const result = resolveSkillNames(['REVIEW', 'Auto-Security']); + expect(result.valid).toEqual(['review', 'security']); + }); + + test('reports invalid names', () => { + const { resolveSkillNames } = loadModule(); + const result = resolveSkillNames(['review', 'nonexistent']); + expect(result.valid).toEqual(['review']); + expect(result.invalid).toEqual(['nonexistent']); + }); + }); +}); From e89f7ee6d6536a4632b901650d81d2ef00346f55 Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 17:35:23 -0500 Subject: [PATCH 15/20] fix: address code review findings in activity-monitoring hooks - Use jq for safe JSON construction in BMAD trigger (prevent injection from filenames with quotes/backslashes) - Make mktemp failure non-fatal in all hooks (exit 0 instead of blocking git operations) - Fix CLAUDE.md auto-generated description for preuninstall.js - Update postinstall.js JSDoc to mention hook registration - Gate first-run notice on hooks actually being registered Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 2 +- hooks/post-failure.sh | 2 +- hooks/post-tool-use.sh | 17 +++++------------ hooks/pre-bash.sh | 4 +++- scripts/postinstall.js | 26 ++++++++++++++------------ scripts/preuninstall.js | 2 +- 6 files changed, 25 insertions(+), 28 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4e6e559..4599fe6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -165,7 +165,7 @@ scripts/ ├── integration-test.sh ├── list-models.js ├── postinstall.js # Install skill files to ~/.claude/skills/ -├── preuninstall.js # Sidecar hook script filenames — used to identify our hooks regardless of install path +├── preuninstall.js # Pre-uninstall script for claude-sidecar ├── test-tools.sh ├── validate-docs.js # * Main entry point. ├── validate-thinking.js diff --git a/hooks/post-failure.sh b/hooks/post-failure.sh index f969a10..7bf7bb5 100755 --- a/hooks/post-failure.sh +++ b/hooks/post-failure.sh @@ -26,7 +26,7 @@ if ! command -v jq >/dev/null 2>&1; then exit 0 fi -TMP_JSON=$(mktemp) +TMP_JSON=$(mktemp 2>/dev/null) || exit 0 trap 'rm -f "$TMP_JSON"' EXIT cat > "$TMP_JSON" diff --git a/hooks/post-tool-use.sh b/hooks/post-tool-use.sh index 642d462..12ea223 100755 --- a/hooks/post-tool-use.sh +++ b/hooks/post-tool-use.sh @@ -35,7 +35,7 @@ if ! command -v jq >/dev/null 2>&1; then exit 0 fi -TMP_JSON=$(mktemp) +TMP_JSON=$(mktemp 2>/dev/null) || exit 0 trap 'rm -f "$TMP_JSON"' EXIT cat > "$TMP_JSON" @@ -82,19 +82,12 @@ if [ "$TOOL_NAME" = "Write" ] || [ "$TOOL_NAME" = "Edit" ] || [ "$TOOL_NAME" = " | length > 0' "$TMP_JSON" 2>/dev/null || echo "false") if [ "$HAS_BMAD" = "true" ]; then - # Extract a representative file path for the message - BMAD_FILE=$(jq -r ' + # Build JSON safely via jq to avoid injection from filenames with quotes/backslashes + jq -n --arg file "$(jq -r ' [.tool_input.file_path, (.tool_input.edits[]?.file_path)] | map(select(. != null and contains("_bmad-output/"))) - | first' "$TMP_JSON" 2>/dev/null || echo "_bmad-output/") - - cat </dev/null || echo "_bmad-output/")" \ + '{hookSpecificOutput:{additionalContext:("A BMAD-METHOD artifact was just written or updated at " + $file + ". Consider running the sidecar-auto-bmad-method-check skill to get a second-opinion review from another model before finalizing this artifact. You can invoke it with: use the Skill tool with skill \u0027sidecar-auto-bmad-method-check\u0027. If the user has already reviewed this artifact or explicitly declined a check, proceed without one.")}}' exit 0 fi fi diff --git a/hooks/pre-bash.sh b/hooks/pre-bash.sh index 7e9def8..be25364 100755 --- a/hooks/pre-bash.sh +++ b/hooks/pre-bash.sh @@ -25,7 +25,9 @@ if [ -f "$CONFIG_PATH" ] && command -v jq >/dev/null 2>&1; then fi # ── Read stdin to temp file (avoid ARG_MAX on large payloads) ───────── -TMP_JSON=$(mktemp) +# If mktemp fails (disk full, permissions), allow the command through +# rather than blocking git operations with a non-zero exit. +TMP_JSON=$(mktemp 2>/dev/null) || exit 0 trap 'rm -f "$TMP_JSON"' EXIT cat > "$TMP_JSON" diff --git a/scripts/postinstall.js b/scripts/postinstall.js index 0abe7dd..d8e5440 100644 --- a/scripts/postinstall.js +++ b/scripts/postinstall.js @@ -6,6 +6,7 @@ * 1. Copies SKILL.md to ~/.claude/skills/sidecar/ * 2. Registers MCP server in Claude Code (~/.claude.json) * 3. Registers MCP server in Claude Desktop/Cowork config + * 4. Registers activity monitoring hooks in ~/.claude/settings.json */ const fs = require('fs'); @@ -205,18 +206,19 @@ function registerHooks() { fs.writeFileSync(settingsPath, JSON.stringify(settings, null, 2), { mode: 0o600 }); } - // First-run notice - console.log(''); - console.log('[claude-sidecar] Activity monitoring hooks registered:'); - console.log(' - PreToolUse: auto-security gate (git commit/push/PR)'); - console.log(' - PostToolUse: BMAD artifact trigger + event collection'); - console.log(' - PostToolUseFailure: failure event collection'); - console.log(''); - console.log(' Auto-skills suggest security scans, code reviews, and unblock'); - console.log(' assistance at key workflow moments.'); - console.log(''); - console.log(' To disable: sidecar auto-skills --off'); - console.log(' Config: ~/.config/sidecar/config.json'); + if (registered > 0) { + console.log(''); + console.log('[claude-sidecar] Activity monitoring hooks registered:'); + console.log(' - PreToolUse: auto-security gate (git commit/push/PR)'); + console.log(' - PostToolUse: BMAD artifact trigger + event collection'); + console.log(' - PostToolUseFailure: failure event collection'); + console.log(''); + console.log(' Auto-skills suggest security scans, code reviews, and unblock'); + console.log(' assistance at key workflow moments.'); + console.log(''); + console.log(' To disable: sidecar auto-skills --off'); + console.log(' Config: ~/.config/sidecar/config.json'); + } } function main() { diff --git a/scripts/preuninstall.js b/scripts/preuninstall.js index ee3c007..540a8bc 100644 --- a/scripts/preuninstall.js +++ b/scripts/preuninstall.js @@ -11,7 +11,7 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); -/** Sidecar hook script filenames — used to identify our hooks regardless of install path */ +// Hook script basenames registered by postinstall — used to identify sidecar hooks during removal const SIDECAR_HOOK_SCRIPTS = ['pre-bash.sh', 'post-tool-use.sh', 'post-failure.sh', 'stop-hook.sh']; function isSidecarHookCommand(command) { From 54b85a3862003b64ab521870ff538306b1f4414e Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 17:48:32 -0500 Subject: [PATCH 16/20] fix: address sub-threshold code review findings - Split registerHooks into mergeHooks + registerHooks (both under 50 lines) - Deduplicate hooks by basename (not full path) to prevent accumulation on upgrades - Fix TOCTOU race: use umask for atomic file creation with restrictive permissions - Extract getUsage() to cli-usage.js (cli.js now 289 lines, under 300 limit) - Handle --key=value syntax in CLI arg parser - Sanitize SESSION_ID to prevent path traversal in event file paths - Fix jq slice syntax ([:500] -> [0:500]) for broader compatibility - Cap event JSONL files at 5MB to prevent unbounded growth - Add Phase 2 forward-reference comment for stop-hook.sh in preuninstall Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 2 + hooks/post-failure.sh | 11 ++-- hooks/post-tool-use.sh | 42 +++++++++------- scripts/postinstall.js | 62 +++++++++++------------ scripts/preuninstall.js | 3 +- src/cli-usage.js | 93 ++++++++++++++++++++++++++++++++++ src/cli.js | 108 +++++++--------------------------------- 7 files changed, 177 insertions(+), 144 deletions(-) create mode 100644 src/cli-usage.js diff --git a/CLAUDE.md b/CLAUDE.md index 4599fe6..fd56b8d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -117,6 +117,7 @@ src/ │ ├── updater.js # @type {import('update-notifier').UpdateNotifier|null} │ └── validators.js # * Provider to API key mapping ├── cli-handlers.js # CLI Command Handlers +├── cli-usage.js # CLI Usage Text ├── cli.js # * Default values per spec §4.1 ├── conflict.js # File Conflict Detection Module ├── context-compression.js # Context Compression Module @@ -193,6 +194,7 @@ evals/ | Module | Purpose | Key Exports | |--------|---------|-------------| | `cli-handlers.js` | CLI Command Handlers | `handleSetup()`, `handleAbort()`, `handleUpdate()`, `handleMcp()`, `handleAutoSkills()` | +| `cli-usage.js` | CLI Usage Text | `getUsage()` | | `cli.js` | * Default values per spec §4.1 | `parseArgs()`, `validateStartArgs()`, `getUsage()`, `DEFAULTS()` | | `conflict.js` | File Conflict Detection Module | `detectConflicts()`, `formatConflictWarning()` | | `context-compression.js` | Context Compression Module | `compressContext()`, `estimateTokenCount()`, `buildPreamble()`, `DEFAULT_TOKEN_LIMIT()` | diff --git a/hooks/post-failure.sh b/hooks/post-failure.sh index 7bf7bb5..4a14ede 100755 --- a/hooks/post-failure.sh +++ b/hooks/post-failure.sh @@ -36,7 +36,12 @@ if [ -z "$SESSION_ID" ]; then fi # ── Append failure event ───────────────────────────────────────────── -EVENT_FILE="${TMPDIR:-/tmp}/sidecar-monitor-${SESSION_ID}.jsonl" +# Sanitize SESSION_ID to alphanumeric/hyphens only (prevent path traversal) +SAFE_SID=$(printf '%s' "$SESSION_ID" | tr -cd 'a-zA-Z0-9_-') +if [ -z "$SAFE_SID" ]; then + exit 0 +fi +EVENT_FILE="${TMPDIR:-/tmp}/sidecar-monitor-${SAFE_SID}.jsonl" EVENT=$(jq -rc --arg ts "$(date -u +%Y-%m-%dT%H:%M:%SZ)" ' { @@ -46,12 +51,12 @@ EVENT=$(jq -rc --arg ts "$(date -u +%Y-%m-%dT%H:%M:%SZ)" ' else (.tool_input.file_path // "") end), success: false, command: (if .tool_name == "Bash" then (.tool_input.command // "") else "" end), - errorSnippet: ((.error // .tool_response.error // .tool_response.stderr // "")[:500]) + errorSnippet: ((.error // .tool_response.error // .tool_response.stderr // "")[0:500]) }' "$TMP_JSON" 2>/dev/null || echo "") if [ -n "$EVENT" ]; then if [ ! -f "$EVENT_FILE" ]; then - touch "$EVENT_FILE" && chmod 600 "$EVENT_FILE" + (umask 177 && : > "$EVENT_FILE") fi echo "$EVENT" >> "$EVENT_FILE" fi diff --git a/hooks/post-tool-use.sh b/hooks/post-tool-use.sh index 12ea223..ed7bd1d 100755 --- a/hooks/post-tool-use.sh +++ b/hooks/post-tool-use.sh @@ -44,26 +44,34 @@ SESSION_ID=$(jq -r '.session_id // ""' "$TMP_JSON" 2>/dev/null || echo "") # ── Event collection ───────────────────────────────────────────────── # Append structured event to session-specific JSONL file -if [ -n "$SESSION_ID" ]; then - EVENT_FILE="${TMPDIR:-/tmp}/sidecar-monitor-${SESSION_ID}.jsonl" +# Sanitize SESSION_ID to alphanumeric/hyphens only (prevent path traversal) +SAFE_SID=$(printf '%s' "$SESSION_ID" | tr -cd 'a-zA-Z0-9_-') +if [ -n "$SAFE_SID" ]; then + EVENT_FILE="${TMPDIR:-/tmp}/sidecar-monitor-${SAFE_SID}.jsonl" - EVENT=$(jq -rc --arg ts "$(date -u +%Y-%m-%dT%H:%M:%SZ)" ' - { - ts: $ts, - tool: (.tool_name // "unknown"), - file: (if .tool_name == "Bash" then "" - elif .tool_name == "MultiEdit" then (.tool_input.edits[0].file_path // "") - else (.tool_input.file_path // "") end), - success: (if .tool_name == "Bash" then ((.tool_response.exit_code // 0) == 0) else true end), - command: (if .tool_name == "Bash" then (.tool_input.command // "") else "" end) - }' "$TMP_JSON" 2>/dev/null || echo "") + # Cap event file at 5MB to prevent unbounded growth in long sessions + MAX_SIZE=5242880 + if [ -f "$EVENT_FILE" ] && [ "$(wc -c < "$EVENT_FILE" 2>/dev/null || echo 0)" -gt "$MAX_SIZE" ]; then + : # Skip collection — file too large + else + EVENT=$(jq -rc --arg ts "$(date -u +%Y-%m-%dT%H:%M:%SZ)" ' + { + ts: $ts, + tool: (.tool_name // "unknown"), + file: (if .tool_name == "Bash" then "" + elif .tool_name == "MultiEdit" then (.tool_input.edits[0].file_path // "") + else (.tool_input.file_path // "") end), + success: (if .tool_name == "Bash" then ((.tool_response.exit_code // 0) == 0) else true end), + command: (if .tool_name == "Bash" then (.tool_input.command // "") else "" end) + }' "$TMP_JSON" 2>/dev/null || echo "") - if [ -n "$EVENT" ]; then - # Create with restrictive permissions if new; append otherwise - if [ ! -f "$EVENT_FILE" ]; then - touch "$EVENT_FILE" && chmod 600 "$EVENT_FILE" + if [ -n "$EVENT" ]; then + # Create with restrictive permissions atomically (umask prevents TOCTOU window) + if [ ! -f "$EVENT_FILE" ]; then + (umask 177 && : > "$EVENT_FILE") + fi + echo "$EVENT" >> "$EVENT_FILE" fi - echo "$EVENT" >> "$EVENT_FILE" fi fi diff --git a/scripts/postinstall.js b/scripts/postinstall.js index d8e5440..60649d2 100644 --- a/scripts/postinstall.js +++ b/scripts/postinstall.js @@ -149,10 +149,36 @@ function registerClaudeDesktop() { } } +/** + * Merge sidecar hook matchers into settings, deduplicating by script basename. + * @param {object} settings - Claude settings object (mutated in place) + * @param {object} hooksConfig - Parsed hooks.json with resolved paths + * @returns {number} Count of newly registered hooks + */ +function mergeHooks(settings, hooksConfig) { + if (!settings.hooks) { settings.hooks = {}; } + let registered = 0; + + for (const [event, matchers] of Object.entries(hooksConfig.hooks || {})) { + if (!settings.hooks[event]) { settings.hooks[event] = []; } + + for (const matcher of matchers) { + const cmd = (matcher.hooks && matcher.hooks[0] && matcher.hooks[0].command) || ''; + const basename = path.basename(cmd); + // Deduplicate by basename so upgrades (which change install path) replace old entries + settings.hooks[event] = settings.hooks[event].filter((existing) => { + return !(existing.hooks && existing.hooks.some((h) => path.basename(h.command) === basename)); + }); + settings.hooks[event].push(matcher); + registered++; + } + } + return registered; +} + /** * Register activity monitoring hooks in ~/.claude/settings.json. * Resolves absolute paths to hook scripts at install time. - * Merges sidecar hooks without overwriting existing user hooks. */ function registerHooks() { const hooksDir = path.join(__dirname, '..', 'hooks'); @@ -162,7 +188,6 @@ function registerHooks() { let hooksConfig; try { const raw = fs.readFileSync(hooksConfigPath, 'utf-8'); - // Escape backslashes for Windows paths before injecting into JSON const safeHooksDir = hooksDir.replace(/\\/g, '\\\\'); hooksConfig = JSON.parse(raw.replace(/__HOOKS_DIR__/g, safeHooksDir)); } catch (err) { @@ -178,46 +203,19 @@ function registerHooks() { // File doesn't exist or invalid — start fresh } - if (!settings.hooks) { settings.hooks = {}; } - - // Merge each hook event, appending sidecar hooks after existing ones - let registered = 0; - for (const [event, matchers] of Object.entries(hooksConfig.hooks || {})) { - if (!settings.hooks[event]) { settings.hooks[event] = []; } - - for (const matcher of matchers) { - const cmd = (matcher.hooks && matcher.hooks[0] && matcher.hooks[0].command) || ''; - // Skip if this exact hook command is already registered - const alreadyExists = settings.hooks[event].some((existing) => { - return existing.hooks && existing.hooks.some((h) => h.command === cmd); - }); - if (!alreadyExists) { - settings.hooks[event].push(matcher); - registered++; - } - } - } - + const registered = mergeHooks(settings, hooksConfig); if (registered > 0) { const settingsDir = path.dirname(settingsPath); if (!fs.existsSync(settingsDir)) { fs.mkdirSync(settingsDir, { recursive: true, mode: 0o700 }); } fs.writeFileSync(settingsPath, JSON.stringify(settings, null, 2), { mode: 0o600 }); - } - - if (registered > 0) { console.log(''); - console.log('[claude-sidecar] Activity monitoring hooks registered:'); + console.log(`[claude-sidecar] Activity monitoring hooks registered (${registered}):`); console.log(' - PreToolUse: auto-security gate (git commit/push/PR)'); console.log(' - PostToolUse: BMAD artifact trigger + event collection'); console.log(' - PostToolUseFailure: failure event collection'); - console.log(''); - console.log(' Auto-skills suggest security scans, code reviews, and unblock'); - console.log(' assistance at key workflow moments.'); - console.log(''); console.log(' To disable: sidecar auto-skills --off'); - console.log(' Config: ~/.config/sidecar/config.json'); } } @@ -250,4 +248,4 @@ if (require.main === module) { main(); } -module.exports = { addMcpToConfigFile, registerHooks }; +module.exports = { addMcpToConfigFile, mergeHooks, registerHooks }; diff --git a/scripts/preuninstall.js b/scripts/preuninstall.js index 540a8bc..69dda8e 100644 --- a/scripts/preuninstall.js +++ b/scripts/preuninstall.js @@ -11,7 +11,8 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); -// Hook script basenames registered by postinstall — used to identify sidecar hooks during removal +// Hook script basenames registered by postinstall — used to identify sidecar hooks during removal. +// Includes stop-hook.sh (Phase 2) so future installs are cleaned up correctly on uninstall. const SIDECAR_HOOK_SCRIPTS = ['pre-bash.sh', 'post-tool-use.sh', 'post-failure.sh', 'stop-hook.sh']; function isSidecarHookCommand(command) { diff --git a/src/cli-usage.js b/src/cli-usage.js new file mode 100644 index 0000000..7f88a39 --- /dev/null +++ b/src/cli-usage.js @@ -0,0 +1,93 @@ +/** + * CLI Usage Text + * + * Extracted from cli.js to keep files under the 300-line limit. + */ + +/** + * Get usage text + * @returns {string} Formatted usage/help string + */ +function getUsage() { + return ` +Usage: sidecar [options] + +Commands: + start Launch a new sidecar + list Show previous sidecars + resume Reopen a previous sidecar + continue New sidecar building on previous + read Output sidecar summary/conversation + abort Abort a running sidecar session + setup Configure default model and aliases + --api-keys Open API key setup window + auto-skills List/enable/disable auto-skills + update Update to latest version + mcp Start MCP server (stdio transport) + +Options for 'start': + --model Optional (uses config default). Model to use: + - Short aliases: gemini, opus, gpt (see 'sidecar setup') + - Direct API: google/gemini-2.5-flash + - OpenRouter: openrouter/google/gemini-2.5-flash + --prompt Required. Task description + --agent OpenCode agent to use (see Agent Types below) + --session-id Session ID to pull context from (default: current) + --cwd Project directory (default: cwd) + --no-ui Run without GUI (autonomous mode) + --no-context Skip parent conversation history context + --timeout Headless timeout (default: 15) + --client Client type: code-local, code-web, cowork + --session-dir Explicit session data directory + --setup Force open configuration + --fold-shortcut Customize fold shortcut + --opencode-port Port override for OpenCode server + --context-turns Max conversation turns (default: 50) + --context-since Time filter (e.g., 2h). Overrides turns. + --context-max-tokens Max context tokens (default: 80000) + --summary-length Summary verbosity: brief, normal (default), verbose + --mcp Add MCP server. Formats: + - name=url (remote server) + - name=command (local server) + --mcp-config Path to opencode.json with MCP config + --no-mcp Don't inherit MCP servers from parent LLM + --exclude-mcp Exclude specific MCP server (repeatable) + --validate-model Verify model exists on provider API (opt-in) + --position Window position: right (default), left, center + +Options for 'list': + --status Filter by status (running, complete) + --all Show all projects + --json Output as JSON + +Options for 'read': + --summary Show summary (default) + --conversation Show full conversation + +Options for 'auto-skills': + --on [skill ...] Enable all or specific auto-skills + --off [skill ...] Disable all or specific auto-skills + (no flags) Show current status + +OpenCode Agent Types: + Chat Reads auto, writes/bash ask permission (interactive default) + Build Full tool access (headless default) + Plan Read-only analysis and planning + + NOTE: --agent chat is interactive-only (incompatible with --no-ui). + Headless mode defaults to build agent. + +Custom agents defined in ~/.config/opencode/agents/ or +.opencode/agents/ are also supported. + +Examples: + sidecar start --model google/gemini-2.5 --prompt "Debug auth issue" + sidecar start --model openai/o3 --prompt "Generate tests" --no-ui + sidecar start --model gemini --prompt "Review code" --agent Plan + sidecar list + sidecar resume abc123 + sidecar read abc123 --conversation +`; +} + +module.exports = { getUsage }; diff --git a/src/cli.js b/src/cli.js index 99ab4e7..1cd80c8 100644 --- a/src/cli.js +++ b/src/cli.js @@ -47,25 +47,34 @@ function parseArgs(argv) { const arg = argv[i]; if (arg.startsWith('--')) { - const key = arg.slice(2); - const next = argv[i + 1]; + let key = arg.slice(2); + let inlineValue; + const eqIdx = key.indexOf('='); + if (eqIdx !== -1) { + inlineValue = key.slice(eqIdx + 1); + key = key.slice(0, eqIdx); + } + const next = inlineValue !== undefined ? undefined : argv[i + 1]; - // Boolean flags (no value expected) + // Boolean flags (no value expected) — ignore inline values (--on=x is invalid) if (isBooleanFlag(key)) { result[key] = true; continue; } // Array accumulation flags - if (key === 'exclude-mcp' && next && !next.startsWith('--')) { + const val = inlineValue !== undefined ? inlineValue : next; + if (key === 'exclude-mcp' && val && !val.startsWith('--')) { result['exclude-mcp'] = result['exclude-mcp'] || []; - result['exclude-mcp'].push(next); - i++; + result['exclude-mcp'].push(val); + if (inlineValue === undefined) { i++; } continue; } // Options with values - if (next && !next.startsWith('--')) { + if (inlineValue !== undefined) { + result[key] = parseValue(key, inlineValue); + } else if (next && !next.startsWith('--')) { result[key] = parseValue(key, next); i++; } else { @@ -270,90 +279,7 @@ function isValidDurationFormat(duration) { return /^\d+[mhd]$/.test(duration); } -/** - * Get usage text - */ -function getUsage() { - return ` -Usage: sidecar [options] - -Commands: - start Launch a new sidecar - list Show previous sidecars - resume Reopen a previous sidecar - continue New sidecar building on previous - read Output sidecar summary/conversation - abort Abort a running sidecar session - setup Configure default model and aliases - --api-keys Open API key setup window - auto-skills List/enable/disable auto-skills - update Update to latest version - mcp Start MCP server (stdio transport) - -Options for 'start': - --model Optional (uses config default). Model to use: - - Short aliases: gemini, opus, gpt (see 'sidecar setup') - - Direct API: google/gemini-2.5-flash - - OpenRouter: openrouter/google/gemini-2.5-flash - --prompt Required. Task description - --agent OpenCode agent to use (see Agent Types below) - --session-id Session ID to pull context from (default: current) - --cwd Project directory (default: cwd) - --no-ui Run without GUI (autonomous mode) - --no-context Skip parent conversation history context - --timeout Headless timeout (default: 15) - --client Client type: code-local, code-web, cowork - --session-dir Explicit session data directory - --setup Force open configuration - --fold-shortcut Customize fold shortcut - --opencode-port Port override for OpenCode server - --context-turns Max conversation turns (default: 50) - --context-since Time filter (e.g., 2h). Overrides turns. - --context-max-tokens Max context tokens (default: 80000) - --summary-length Summary verbosity: brief, normal (default), verbose - --mcp Add MCP server. Formats: - - name=url (remote server) - - name=command (local server) - --mcp-config Path to opencode.json with MCP config - --no-mcp Don't inherit MCP servers from parent LLM - --exclude-mcp Exclude specific MCP server (repeatable) - --validate-model Verify model exists on provider API (opt-in) - --position Window position: right (default), left, center - -Options for 'list': - --status Filter by status (running, complete) - --all Show all projects - --json Output as JSON - -Options for 'read': - --summary Show summary (default) - --conversation Show full conversation - -Options for 'auto-skills': - --on [skill ...] Enable all or specific auto-skills - --off [skill ...] Disable all or specific auto-skills - (no flags) Show current status - -OpenCode Agent Types: - Chat Reads auto, writes/bash ask permission (interactive default) - Build Full tool access (headless default) - Plan Read-only analysis and planning - - NOTE: --agent chat is interactive-only (incompatible with --no-ui). - Headless mode defaults to build agent. - -Custom agents defined in ~/.config/opencode/agents/ or -.opencode/agents/ are also supported. - -Examples: - sidecar start --model google/gemini-2.5 --prompt "Debug auth issue" - sidecar start --model openai/o3 --prompt "Generate tests" --no-ui - sidecar start --model gemini --prompt "Review code" --agent Plan - sidecar list - sidecar resume abc123 - sidecar read abc123 --conversation -`; -} +const { getUsage } = require('./cli-usage'); module.exports = { parseArgs, From efd18f430493e578b7cdf274485084bc9ac5deca Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 18:43:28 -0500 Subject: [PATCH 17/20] fix: address all code review findings in activity-monitoring hooks Fixes 14 issues found during structured code review: High-confidence fixes (score 100): - Replace console.error with logger.error in start-helpers.js (lint failure) - Fix JSON injection in registerHooks via JSON.stringify escaping - Prevent mergeHooks from unconditionally rewriting settings.json - Scope basename dedup to claude-sidecar paths to protect user hooks Sub-threshold fixes (score 50-75): - Move cli-handlers.js no-console override to .eslintrc.js - Extract parseAutoSkillsArgs to bring handleAutoSkills under 50 lines - Decouple monitoring.enabled from auto-skill triggers in hooks - Wrap saveConfig in try/catch in setAutoSkillsEnabled - Consolidate double progress.json read into single parse - Remove phantom stop-hook.sh from preuninstall.js - Remove broken docs/plans/index.md link from CLAUDE.md - Add unit tests for start-helpers, cli-handlers, and mergeHooks Co-Authored-By: Claude Opus 4.6 --- .eslintrc.js | 2 +- CLAUDE.md | 1 - hooks/post-tool-use.sh | 11 ++-- hooks/pre-bash.sh | 3 +- scripts/postinstall.js | 21 ++++++-- scripts/preuninstall.js | 3 +- src/cli-handlers.js | 40 +++++++------- src/sidecar/progress.js | 28 +++------- src/utils/auto-skills-config.js | 7 ++- src/utils/start-helpers.js | 6 ++- tests/cli-handlers.test.js | 96 +++++++++++++++++++++++++++++++++ tests/postinstall.test.js | 71 ++++++++++++++++++++++++ tests/start-helpers.test.js | 88 ++++++++++++++++++++++++++++++ 13 files changed, 322 insertions(+), 55 deletions(-) create mode 100644 tests/cli-handlers.test.js create mode 100644 tests/start-helpers.test.js diff --git a/.eslintrc.js b/.eslintrc.js index 92d364a..328af2c 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -47,7 +47,7 @@ module.exports = { { // CLI output files use console.log for user-facing output (not logging) // These display results to the user, not debug info - files: ['src/sidecar/read.js', 'src/sidecar/session-utils.js'], + files: ['src/sidecar/read.js', 'src/sidecar/session-utils.js', 'src/cli-handlers.js'], rules: { 'no-console': 'off' } diff --git a/CLAUDE.md b/CLAUDE.md index fd56b8d..3ad6445 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -374,4 +374,3 @@ GEMINI.md and AGENTS.md are symlinks to CLAUDE.md -- no sync needed. - [docs/electron-testing.md](docs/electron-testing.md) - CDP patterns - [docs/jsdoc-setup.md](docs/jsdoc-setup.md) - JSDoc, `.d.ts` generation - [evals/README.md](evals/README.md) - Agentic eval system -- [docs/plans/index.md](docs/plans/index.md) - Design plans diff --git a/hooks/post-tool-use.sh b/hooks/post-tool-use.sh index ed7bd1d..2519926 100755 --- a/hooks/post-tool-use.sh +++ b/hooks/post-tool-use.sh @@ -25,8 +25,8 @@ if [ -f "$CONFIG_PATH" ] && command -v jq >/dev/null 2>&1; then MONITORING_ON=$(jq -r '.monitoring.enabled | if type == "boolean" then . else true end' "$CONFIG_PATH" 2>/dev/null || echo "true") fi -# If master auto-skills switch or monitoring is off, skip everything -if [ "$MASTER_ON" = "false" ] || [ "$MONITORING_ON" = "false" ]; then +# If master auto-skills switch is off, skip everything +if [ "$MASTER_ON" = "false" ]; then exit 0 fi @@ -43,9 +43,12 @@ TOOL_NAME=$(jq -r '.tool_name // ""' "$TMP_JSON" 2>/dev/null || echo "") SESSION_ID=$(jq -r '.session_id // ""' "$TMP_JSON" 2>/dev/null || echo "") # ── Event collection ───────────────────────────────────────────────── -# Append structured event to session-specific JSONL file +# Append structured event to session-specific JSONL file (controlled by monitoring.enabled) # Sanitize SESSION_ID to alphanumeric/hyphens only (prevent path traversal) -SAFE_SID=$(printf '%s' "$SESSION_ID" | tr -cd 'a-zA-Z0-9_-') +SAFE_SID="" +if [ "$MONITORING_ON" != "false" ]; then + SAFE_SID=$(printf '%s' "$SESSION_ID" | tr -cd 'a-zA-Z0-9_-') +fi if [ -n "$SAFE_SID" ]; then EVENT_FILE="${TMPDIR:-/tmp}/sidecar-monitor-${SAFE_SID}.jsonl" diff --git a/hooks/pre-bash.sh b/hooks/pre-bash.sh index be25364..a28597a 100755 --- a/hooks/pre-bash.sh +++ b/hooks/pre-bash.sh @@ -18,8 +18,7 @@ CONFIG_PATH="${HOME}/.config/sidecar/config.json" if [ -f "$CONFIG_PATH" ] && command -v jq >/dev/null 2>&1; then MASTER=$(jq -r '.autoSkills.enabled | if type == "boolean" then . else true end' "$CONFIG_PATH" 2>/dev/null || echo "true") SECURITY=$(jq -r '.autoSkills.security.enabled | if type == "boolean" then . else true end' "$CONFIG_PATH" 2>/dev/null || echo "true") - MONITORING=$(jq -r '.monitoring.enabled | if type == "boolean" then . else true end' "$CONFIG_PATH" 2>/dev/null || echo "true") - if [ "$MASTER" = "false" ] || [ "$SECURITY" = "false" ] || [ "$MONITORING" = "false" ]; then + if [ "$MASTER" = "false" ] || [ "$SECURITY" = "false" ]; then exit 0 fi fi diff --git a/scripts/postinstall.js b/scripts/postinstall.js index 60649d2..9e77b20 100644 --- a/scripts/postinstall.js +++ b/scripts/postinstall.js @@ -151,9 +151,11 @@ function registerClaudeDesktop() { /** * Merge sidecar hook matchers into settings, deduplicating by script basename. + * Only counts a hook as "registered" if it actually changed. + * Uses full command path comparison to avoid removing unrelated user hooks. * @param {object} settings - Claude settings object (mutated in place) * @param {object} hooksConfig - Parsed hooks.json with resolved paths - * @returns {number} Count of newly registered hooks + * @returns {number} Count of newly registered or changed hooks */ function mergeHooks(settings, hooksConfig) { if (!settings.hooks) { settings.hooks = {}; } @@ -164,10 +166,20 @@ function mergeHooks(settings, hooksConfig) { for (const matcher of matchers) { const cmd = (matcher.hooks && matcher.hooks[0] && matcher.hooks[0].command) || ''; + // Check if this exact hook already exists (by full command path) + const alreadyExists = settings.hooks[event].some((existing) => { + return existing.hooks && existing.hooks.some((h) => h.command === cmd); + }); + if (alreadyExists) { continue; } + // Remove old sidecar entries (same basename, different path) for upgrades const basename = path.basename(cmd); - // Deduplicate by basename so upgrades (which change install path) replace old entries settings.hooks[event] = settings.hooks[event].filter((existing) => { - return !(existing.hooks && existing.hooks.some((h) => path.basename(h.command) === basename)); + if (!existing.hooks) { return true; } + return !existing.hooks.some((h) => { + const hBase = path.basename(h.command); + // Only remove if it looks like a sidecar hook (lives in a node_modules path) + return hBase === basename && h.command.includes('claude-sidecar'); + }); }); settings.hooks[event].push(matcher); registered++; @@ -188,7 +200,8 @@ function registerHooks() { let hooksConfig; try { const raw = fs.readFileSync(hooksConfigPath, 'utf-8'); - const safeHooksDir = hooksDir.replace(/\\/g, '\\\\'); + // Use JSON.stringify to properly escape all special chars (quotes, backslashes, etc.) + const safeHooksDir = JSON.stringify(hooksDir).slice(1, -1); hooksConfig = JSON.parse(raw.replace(/__HOOKS_DIR__/g, safeHooksDir)); } catch (err) { console.error(`[claude-sidecar] Warning: Could not read hooks config: ${err.message}`); diff --git a/scripts/preuninstall.js b/scripts/preuninstall.js index 69dda8e..0b1695c 100644 --- a/scripts/preuninstall.js +++ b/scripts/preuninstall.js @@ -12,8 +12,7 @@ const path = require('path'); const os = require('os'); // Hook script basenames registered by postinstall — used to identify sidecar hooks during removal. -// Includes stop-hook.sh (Phase 2) so future installs are cleaned up correctly on uninstall. -const SIDECAR_HOOK_SCRIPTS = ['pre-bash.sh', 'post-tool-use.sh', 'post-failure.sh', 'stop-hook.sh']; +const SIDECAR_HOOK_SCRIPTS = ['pre-bash.sh', 'post-tool-use.sh', 'post-failure.sh']; function isSidecarHookCommand(command) { if (!command || typeof command !== 'string') { return false; } diff --git a/src/cli-handlers.js b/src/cli-handlers.js index 3b99111..b4b690a 100644 --- a/src/cli-handlers.js +++ b/src/cli-handlers.js @@ -5,7 +5,6 @@ * under the 300-line limit. */ -/* eslint-disable no-console */ const fs = require('fs'); const path = require('path'); const { validateTaskId, safeSessionDir } = require('./utils/validators'); @@ -123,45 +122,50 @@ async function handleMcp() { } /** - * Handle 'sidecar auto-skills' command - * Lists status or enables/disables auto-skills + * Parse and validate auto-skills CLI arguments. + * @param {object} args - Parsed CLI arguments + * @returns {{ enabled: boolean, skillArgs: string[] }} */ -function handleAutoSkills(args) { - const { - getAutoSkillsStatus, - setAutoSkillsEnabled, - resolveSkillNames, - VALID_SKILL_NAMES, - SKILL_LABELS, - } = require('./utils/auto-skills-config'); - +function parseAutoSkillsArgs(args) { const on = args.on; const off = args.off; const skillArgs = args._.slice(1); - // Reject mutually exclusive flags if (on && off) { console.error('Error: --on and --off cannot be used together'); process.exit(1); } - // Reject positional args without --on/--off if (!on && !off && skillArgs.length > 0) { console.error('Error: specify --on or --off with skill names'); console.error('Usage: sidecar auto-skills --off review security'); process.exit(1); } - // No flags — show status + return { on, off, enabled: !!on, skillArgs }; +} + +/** + * Handle 'sidecar auto-skills' command + * Lists status or enables/disables auto-skills + */ +function handleAutoSkills(args) { + const { + getAutoSkillsStatus, + setAutoSkillsEnabled, + resolveSkillNames, + VALID_SKILL_NAMES, + SKILL_LABELS, + } = require('./utils/auto-skills-config'); + + const { on, off, enabled, skillArgs } = parseAutoSkillsArgs(args); + if (!on && !off) { console.log(getAutoSkillsStatus()); return; } - const enabled = !!on; - if (skillArgs.length === 0) { - // Master switch setAutoSkillsEnabled(enabled); console.log(`Auto-skills ${enabled ? 'enabled' : 'disabled'}.`); return; diff --git a/src/sidecar/progress.js b/src/sidecar/progress.js index 44feda2..b7a164a 100644 --- a/src/sidecar/progress.js +++ b/src/sidecar/progress.js @@ -146,8 +146,12 @@ function readProgress(sessionDir) { ? computeLastActivity(convStat.mtime) : 'never'; - // Read progress.json for lifecycle stage info + // Read progress.json for lifecycle stage info and stall detection let stage; + let lastActivityMs = null; + if (convStat) { + lastActivityMs = Date.now() - convStat.mtime.getTime(); + } if (fs.existsSync(progressPath)) { try { @@ -170,35 +174,19 @@ function readProgress(sessionDir) { messages = progress.messagesReceived; } - // Use progress updatedAt for lastActivity if more recent + // Use progress updatedAt for lastActivity/lastActivityMs if more recent if (progress.updatedAt) { const progressTime = new Date(progress.updatedAt); if (!convStat || progressTime > convStat.mtime) { lastActivity = computeLastActivity(progressTime); } - } - } catch { - // Ignore malformed progress file - } - } - - // Compute raw lastActivityMs for stall detection - let lastActivityMs = null; - if (convStat) { - lastActivityMs = Date.now() - convStat.mtime.getTime(); - } - // Use progress.json updatedAt if more recent - if (fs.existsSync(progressPath)) { - try { - const progress = JSON.parse(fs.readFileSync(progressPath, 'utf-8')); - if (progress.updatedAt) { - const progressMs = Date.now() - new Date(progress.updatedAt).getTime(); + const progressMs = Date.now() - progressTime.getTime(); if (lastActivityMs === null || progressMs < lastActivityMs) { lastActivityMs = progressMs; } } } catch { - // Ignore — already handled above + // Ignore malformed progress file } } diff --git a/src/utils/auto-skills-config.js b/src/utils/auto-skills-config.js index 2e22ad9..36b08e4 100644 --- a/src/utils/auto-skills-config.js +++ b/src/utils/auto-skills-config.js @@ -77,7 +77,12 @@ function setAutoSkillsEnabled(enabled, skillNames) { } } - saveConfig(config); + try { + saveConfig(config); + } catch (err) { + const { logger } = require('./logger'); + logger.warn(`Could not save config: ${err.message}`); + } } /** diff --git a/src/utils/start-helpers.js b/src/utils/start-helpers.js index 2f3e233..5a6459e 100644 --- a/src/utils/start-helpers.js +++ b/src/utils/start-helpers.js @@ -5,6 +5,8 @@ * to keep the CLI entry point under the 300-line limit. */ +const { logger } = require('./logger'); + /** * Resolve model from args: resolve alias or config default. * Returns { model, alias } or calls process.exit(1) on error. @@ -18,7 +20,7 @@ function resolveModelFromArgs(args) { try { model = resolveModel(args.model); } catch (err) { - console.error(err.message); + logger.error(err.message); process.exit(1); } @@ -51,7 +53,7 @@ async function validateFallbackModel(args, alias) { headless: args['no-ui'] || !process.stdin.isTTY }); } catch (err) { - console.error(err.message); + logger.error(err.message); process.exit(1); } } diff --git a/tests/cli-handlers.test.js b/tests/cli-handlers.test.js new file mode 100644 index 0000000..639b003 --- /dev/null +++ b/tests/cli-handlers.test.js @@ -0,0 +1,96 @@ +/** + * CLI Handlers Module Tests + * + * Tests for handleAutoSkills and argument validation. + */ + +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +describe('CLI Handlers Module', () => { + let tempDir; + let originalEnv; + let originalExit; + let consoleLogSpy; + let consoleErrorSpy; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sidecar-cli-handlers-test-')); + originalEnv = { ...process.env }; + process.env.SIDECAR_CONFIG_DIR = tempDir; + originalExit = process.exit; + process.exit = jest.fn(); + consoleLogSpy = jest.spyOn(console, 'log').mockImplementation(); + consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); + jest.resetModules(); + }); + + afterEach(() => { + process.env = originalEnv; + process.exit = originalExit; + consoleLogSpy.mockRestore(); + consoleErrorSpy.mockRestore(); + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + function writeConfig(data) { + fs.writeFileSync( + path.join(tempDir, 'config.json'), + JSON.stringify(data, null, 2) + ); + } + + describe('handleAutoSkills', () => { + test('shows status when no flags provided', () => { + writeConfig({}); + const { handleAutoSkills } = require('../src/cli-handlers'); + handleAutoSkills({ _: ['auto-skills'] }); + expect(consoleLogSpy).toHaveBeenCalledWith(expect.stringContaining('Auto-skills:')); + }); + + test('enables master switch with --on', () => { + writeConfig({ autoSkills: { enabled: false } }); + const { handleAutoSkills } = require('../src/cli-handlers'); + handleAutoSkills({ _: ['auto-skills'], on: true }); + expect(consoleLogSpy).toHaveBeenCalledWith('Auto-skills enabled.'); + const config = JSON.parse(fs.readFileSync(path.join(tempDir, 'config.json'), 'utf-8')); + expect(config.autoSkills.enabled).toBe(true); + }); + + test('disables master switch with --off', () => { + writeConfig({}); + const { handleAutoSkills } = require('../src/cli-handlers'); + handleAutoSkills({ _: ['auto-skills'], off: true }); + expect(consoleLogSpy).toHaveBeenCalledWith('Auto-skills disabled.'); + }); + + test('enables specific skill with --on and skill name', () => { + writeConfig({ autoSkills: { security: { enabled: false } } }); + const { handleAutoSkills } = require('../src/cli-handlers'); + handleAutoSkills({ _: ['auto-skills', 'security'], on: true }); + expect(consoleLogSpy).toHaveBeenCalledWith('auto-security: enabled.'); + }); + + test('rejects --on and --off together', () => { + const { handleAutoSkills } = require('../src/cli-handlers'); + handleAutoSkills({ _: ['auto-skills'], on: true, off: true }); + expect(consoleErrorSpy).toHaveBeenCalledWith('Error: --on and --off cannot be used together'); + expect(process.exit).toHaveBeenCalledWith(1); + }); + + test('rejects positional args without --on/--off', () => { + const { handleAutoSkills } = require('../src/cli-handlers'); + handleAutoSkills({ _: ['auto-skills', 'review'] }); + expect(consoleErrorSpy).toHaveBeenCalledWith('Error: specify --on or --off with skill names'); + expect(process.exit).toHaveBeenCalledWith(1); + }); + + test('rejects unknown skill names', () => { + const { handleAutoSkills } = require('../src/cli-handlers'); + handleAutoSkills({ _: ['auto-skills', 'nonexistent'], on: true }); + expect(consoleErrorSpy).toHaveBeenCalledWith(expect.stringContaining('Unknown skill(s)')); + expect(process.exit).toHaveBeenCalledWith(1); + }); + }); +}); diff --git a/tests/postinstall.test.js b/tests/postinstall.test.js index db6fef1..3db712d 100644 --- a/tests/postinstall.test.js +++ b/tests/postinstall.test.js @@ -2,6 +2,77 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); +const { mergeHooks } = require('../scripts/postinstall'); + +describe('mergeHooks', () => { + function makeHooksConfig(commands) { + const hooks = {}; + for (const [event, cmd] of Object.entries(commands)) { + hooks[event] = [{ matcher: 'Bash', hooks: [{ type: 'command', command: cmd }] }]; + } + return { hooks }; + } + + test('registers hooks into empty settings', () => { + const settings = {}; + const config = makeHooksConfig({ PreToolUse: '/path/to/pre-bash.sh' }); + const count = mergeHooks(settings, config); + expect(count).toBe(1); + expect(settings.hooks.PreToolUse).toHaveLength(1); + expect(settings.hooks.PreToolUse[0].hooks[0].command).toBe('/path/to/pre-bash.sh'); + }); + + test('skips registration when exact hook already exists', () => { + const settings = { + hooks: { + PreToolUse: [{ matcher: 'Bash', hooks: [{ type: 'command', command: '/path/to/pre-bash.sh' }] }], + }, + }; + const config = makeHooksConfig({ PreToolUse: '/path/to/pre-bash.sh' }); + const count = mergeHooks(settings, config); + expect(count).toBe(0); + expect(settings.hooks.PreToolUse).toHaveLength(1); + }); + + test('replaces old sidecar hook on upgrade (different path, same basename)', () => { + const settings = { + hooks: { + PreToolUse: [{ + matcher: 'Bash', + hooks: [{ type: 'command', command: '/old/claude-sidecar/hooks/pre-bash.sh' }], + }], + }, + }; + const config = makeHooksConfig({ PreToolUse: '/new/claude-sidecar/hooks/pre-bash.sh' }); + const count = mergeHooks(settings, config); + expect(count).toBe(1); + expect(settings.hooks.PreToolUse).toHaveLength(1); + expect(settings.hooks.PreToolUse[0].hooks[0].command).toBe('/new/claude-sidecar/hooks/pre-bash.sh'); + }); + + test('preserves user hooks with same basename but non-sidecar path', () => { + const settings = { + hooks: { + PreToolUse: [{ + matcher: 'Bash', + hooks: [{ type: 'command', command: '/home/user/my-hooks/pre-bash.sh' }], + }], + }, + }; + const config = makeHooksConfig({ PreToolUse: '/lib/claude-sidecar/hooks/pre-bash.sh' }); + const count = mergeHooks(settings, config); + expect(count).toBe(1); + // Both user hook and sidecar hook should be present + expect(settings.hooks.PreToolUse).toHaveLength(2); + }); + + test('returns 0 when hooksConfig has no hooks', () => { + const settings = {}; + const count = mergeHooks(settings, { hooks: {} }); + expect(count).toBe(0); + }); +}); + describe('Postinstall MCP registration', () => { test('addMcpToConfigFile creates config file if it does not exist', () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'postinstall-test-')); diff --git a/tests/start-helpers.test.js b/tests/start-helpers.test.js new file mode 100644 index 0000000..7b6d66f --- /dev/null +++ b/tests/start-helpers.test.js @@ -0,0 +1,88 @@ +/** + * Start Helpers Module Tests + * + * Tests for resolveModelFromArgs and validateFallbackModel. + */ + +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +describe('Start Helpers Module', () => { + let tempDir; + let originalEnv; + let originalExit; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sidecar-start-helpers-test-')); + originalEnv = { ...process.env }; + process.env.SIDECAR_CONFIG_DIR = tempDir; + originalExit = process.exit; + jest.resetModules(); + }); + + afterEach(() => { + process.env = originalEnv; + process.exit = originalExit; + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + function writeConfig(data) { + fs.writeFileSync( + path.join(tempDir, 'config.json'), + JSON.stringify(data, null, 2) + ); + } + + describe('resolveModelFromArgs', () => { + test('resolves a known alias to a model string', () => { + writeConfig({ aliases: { gemini: 'google/gemini-2.0-flash' } }); + const { resolveModelFromArgs } = require('../src/utils/start-helpers'); + const result = resolveModelFromArgs({ model: 'gemini' }); + expect(result.model).toBe('google/gemini-2.0-flash'); + expect(result.alias).toBe('gemini'); + }); + + test('passes through a full model ID', () => { + writeConfig({}); + const { resolveModelFromArgs } = require('../src/utils/start-helpers'); + const result = resolveModelFromArgs({ model: 'openai/gpt-4o' }); + expect(result.model).toBe('openai/gpt-4o'); + }); + + test('uses config default when no model specified', () => { + writeConfig({ default: 'gemini', aliases: { gemini: 'google/gemini-2.0-flash' } }); + const { resolveModelFromArgs } = require('../src/utils/start-helpers'); + const result = resolveModelFromArgs({ model: undefined }); + expect(result.alias).toBe('gemini'); + }); + + test('calls process.exit(1) on resolution error', () => { + writeConfig({ aliases: {} }); + process.exit = jest.fn(); + const { resolveModelFromArgs } = require('../src/utils/start-helpers'); + resolveModelFromArgs({ model: '!!!invalid' }); + expect(process.exit).toHaveBeenCalledWith(1); + }); + }); + + describe('validateFallbackModel', () => { + test('returns model unchanged when --validate-model is not set', async () => { + const { validateFallbackModel } = require('../src/utils/start-helpers'); + const result = await validateFallbackModel( + { model: 'openai/gpt-4o', 'validate-model': false }, + 'gpt4' + ); + expect(result).toBe('openai/gpt-4o'); + }); + + test('returns model unchanged when no alias provided', async () => { + const { validateFallbackModel } = require('../src/utils/start-helpers'); + const result = await validateFallbackModel( + { model: 'openai/gpt-4o', 'validate-model': true }, + undefined + ); + expect(result).toBe('openai/gpt-4o'); + }); + }); +}); From c5231ebd0760560e57be872db401243eda16294d Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 19:00:02 -0500 Subject: [PATCH 18/20] fix: address sidecar review findings from Gemini Pro and Codex Fixes 4 issues found by multi-model sidecar review: - Fix String.replace() $-interpolation in registerHooks by using replacer function (() => safeHooksDir) instead of string argument - Add typeof guard for h.command in mergeHooks dedup filter to prevent TypeError crash on malformed settings.json entries - Return boolean from setAutoSkillsEnabled so CLI can detect and report save failures instead of printing false success (logger.warn -> error) - Compare full matcher object in mergeHooks alreadyExists check so upgraded matchers (e.g. adding MultiEdit) are properly replaced Co-Authored-By: Claude Opus 4.6 --- scripts/postinstall.js | 16 ++++++++----- src/cli-handlers.js | 10 ++++++-- src/utils/auto-skills-config.js | 5 +++- tests/auto-skills-config.test.js | 17 +++++++++++++- tests/postinstall.test.js | 39 ++++++++++++++++++++++++++++++++ 5 files changed, 77 insertions(+), 10 deletions(-) diff --git a/scripts/postinstall.js b/scripts/postinstall.js index 9e77b20..979ed47 100644 --- a/scripts/postinstall.js +++ b/scripts/postinstall.js @@ -166,16 +166,20 @@ function mergeHooks(settings, hooksConfig) { for (const matcher of matchers) { const cmd = (matcher.hooks && matcher.hooks[0] && matcher.hooks[0].command) || ''; - // Check if this exact hook already exists (by full command path) - const alreadyExists = settings.hooks[event].some((existing) => { - return existing.hooks && existing.hooks.some((h) => h.command === cmd); + // Check if this exact hook already exists (same command path AND same matcher) + const exactMatch = settings.hooks[event].some((existing) => { + return existing.matcher === matcher.matcher && + existing.hooks && existing.hooks.some((h) => h.command === cmd); }); - if (alreadyExists) { continue; } - // Remove old sidecar entries (same basename, different path) for upgrades + if (exactMatch) { continue; } + // Remove old sidecar entries (same command or same basename with sidecar path) const basename = path.basename(cmd); settings.hooks[event] = settings.hooks[event].filter((existing) => { if (!existing.hooks) { return true; } return !existing.hooks.some((h) => { + if (typeof h.command !== 'string') { return false; } + // Exact command match (same path, different matcher — upgrade scenario) + if (h.command === cmd) { return true; } const hBase = path.basename(h.command); // Only remove if it looks like a sidecar hook (lives in a node_modules path) return hBase === basename && h.command.includes('claude-sidecar'); @@ -202,7 +206,7 @@ function registerHooks() { const raw = fs.readFileSync(hooksConfigPath, 'utf-8'); // Use JSON.stringify to properly escape all special chars (quotes, backslashes, etc.) const safeHooksDir = JSON.stringify(hooksDir).slice(1, -1); - hooksConfig = JSON.parse(raw.replace(/__HOOKS_DIR__/g, safeHooksDir)); + hooksConfig = JSON.parse(raw.replace(/__HOOKS_DIR__/g, () => safeHooksDir)); } catch (err) { console.error(`[claude-sidecar] Warning: Could not read hooks config: ${err.message}`); return; diff --git a/src/cli-handlers.js b/src/cli-handlers.js index b4b690a..f0c1f4f 100644 --- a/src/cli-handlers.js +++ b/src/cli-handlers.js @@ -166,7 +166,10 @@ function handleAutoSkills(args) { } if (skillArgs.length === 0) { - setAutoSkillsEnabled(enabled); + if (!setAutoSkillsEnabled(enabled)) { + console.error('Error: could not save config'); + process.exit(1); + } console.log(`Auto-skills ${enabled ? 'enabled' : 'disabled'}.`); return; } @@ -179,7 +182,10 @@ function handleAutoSkills(args) { process.exit(1); } - setAutoSkillsEnabled(enabled, valid); + if (!setAutoSkillsEnabled(enabled, valid)) { + console.error('Error: could not save config'); + process.exit(1); + } const labels = valid.map((k) => SKILL_LABELS[k]).join(', '); console.log(`${labels}: ${enabled ? 'enabled' : 'disabled'}.`); } diff --git a/src/utils/auto-skills-config.js b/src/utils/auto-skills-config.js index 36b08e4..416b1f1 100644 --- a/src/utils/auto-skills-config.js +++ b/src/utils/auto-skills-config.js @@ -63,6 +63,7 @@ function isMonitoringEnabled(config) { * Enable or disable auto-skills and save to config. * @param {boolean} enabled - Whether to enable or disable * @param {string[]} [skillNames] - Specific skills, or empty for master switch + * @returns {boolean} true if config was saved successfully */ function setAutoSkillsEnabled(enabled, skillNames) { const config = loadConfig() || {}; @@ -79,9 +80,11 @@ function setAutoSkillsEnabled(enabled, skillNames) { try { saveConfig(config); + return true; } catch (err) { const { logger } = require('./logger'); - logger.warn(`Could not save config: ${err.message}`); + logger.error(`Could not save config: ${err.message}`); + return false; } } diff --git a/tests/auto-skills-config.test.js b/tests/auto-skills-config.test.js index 1fb1285..f247f73 100644 --- a/tests/auto-skills-config.test.js +++ b/tests/auto-skills-config.test.js @@ -135,10 +135,25 @@ describe('Auto-Skills Config Module', () => { test('creates config file if missing', () => { const { setAutoSkillsEnabled } = loadModule(); - setAutoSkillsEnabled(false, ['unblock']); + const result = setAutoSkillsEnabled(false, ['unblock']); + expect(result).toBe(true); const raw = JSON.parse(fs.readFileSync(path.join(tempDir, 'config.json'), 'utf-8')); expect(raw.autoSkills.unblock.enabled).toBe(false); }); + + test('returns false when saveConfig fails', () => { + // Make config dir read-only so writeFileSync fails + const configPath = path.join(tempDir, 'config.json'); + fs.writeFileSync(configPath, '{}'); + fs.chmodSync(configPath, 0o444); + fs.chmodSync(tempDir, 0o555); + const { setAutoSkillsEnabled } = loadModule(); + const result = setAutoSkillsEnabled(true); + expect(result).toBe(false); + // Restore permissions for cleanup + fs.chmodSync(tempDir, 0o755); + fs.chmodSync(configPath, 0o644); + }); }); describe('getAutoSkillsStatus', () => { diff --git a/tests/postinstall.test.js b/tests/postinstall.test.js index 3db712d..fe1adbc 100644 --- a/tests/postinstall.test.js +++ b/tests/postinstall.test.js @@ -71,6 +71,45 @@ describe('mergeHooks', () => { const count = mergeHooks(settings, { hooks: {} }); expect(count).toBe(0); }); + + test('replaces hook when command matches but matcher changed (upgrade)', () => { + const settings = { + hooks: { + PostToolUse: [{ + matcher: 'Edit|Write', + hooks: [{ type: 'command', command: '/path/claude-sidecar/hooks/post-tool-use.sh' }], + }], + }, + }; + const config = { + hooks: { + PostToolUse: [{ + matcher: 'Edit|Write|Bash|MultiEdit', + hooks: [{ type: 'command', command: '/path/claude-sidecar/hooks/post-tool-use.sh' }], + }], + }, + }; + const count = mergeHooks(settings, config); + expect(count).toBe(1); + expect(settings.hooks.PostToolUse).toHaveLength(1); + expect(settings.hooks.PostToolUse[0].matcher).toBe('Edit|Write|Bash|MultiEdit'); + }); + + test('handles malformed hook entries without command property', () => { + const settings = { + hooks: { + PreToolUse: [ + { matcher: 'Bash', hooks: [{ type: 'command' }] }, + ], + }, + }; + const config = makeHooksConfig({ PreToolUse: '/path/claude-sidecar/hooks/pre-bash.sh' }); + // Should not throw + const count = mergeHooks(settings, config); + expect(count).toBe(1); + // Malformed entry preserved, new hook appended + expect(settings.hooks.PreToolUse).toHaveLength(2); + }); }); describe('Postinstall MCP registration', () => { From cb4e059cc3bae37bf5ac333d8a38a5b4839619c4 Mon Sep 17 00:00:00 2001 From: ellisjr Date: Thu, 12 Mar 2026 19:35:35 -0500 Subject: [PATCH 19/20] fix: address CodeRabbit review findings across security, bugs, and tests Security: make remote debugging opt-in (setup-window), fix TOCTOU race conditions in hook event files (post-tool-use, post-failure), add request timeout to API key validation, handle Anthropic 5xx/429 as errors, add curl timeout to publish workflow, fix pre-push hook to block on test failure. Bugs: fix validateOpenRouterKey backwards-compat alias signature, add await to handleAutoSkills, add try-catch and result checking to IPC get-api-keys handler, guard against null m.name in model-validator, default unknown skills to disabled, validate skill names before saving, use smarter thinking level fallback for high/xhigh, handle first release in publish workflow. Tests: fix process.env restoration in config-null-alias, fix nested model id test case, remove silent-pass risk in setup-ui-model test, update setup-window and api-key-validation tests for new behavior. Docs: add lang/title to social card HTML, clarify publishing auth method. Co-Authored-By: Claude Opus 4.6 --- .github/workflows/publish.yml | 8 ++- .husky/pre-push | 2 +- CLAUDE.md | 94 ++++++++++++------------- bin/sidecar.js | 2 +- docs/publishing.md | 4 +- electron/ipc-setup.js | 47 ++++++++----- hooks/post-failure.sh | 9 ++- hooks/post-tool-use.sh | 13 ++-- scripts/generate-docs-helpers.js | 2 +- site/social-card-render.html | 3 +- src/sidecar/setup-window.js | 17 +++-- src/utils/api-key-validation.js | 10 ++- src/utils/auto-skills-config.js | 7 +- src/utils/model-validator.js | 4 +- src/utils/thinking-validators.js | 16 ++++- tests/api-key-store-validation.test.js | 21 +++--- tests/config-null-alias.test.js | 11 ++- tests/model-validator-normalize.test.js | 2 +- tests/setup-ui-model.test.js | 11 +-- tests/sidecar/setup-window.test.js | 4 +- 20 files changed, 177 insertions(+), 110 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 967e280..56c8a04 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -41,6 +41,12 @@ jobs: run: | # Find previous tag PREV_TAG=$(git tag --sort=-version:refname | grep -v "^${TAG_NAME}$" | head -1) + + if [ -z "$PREV_TAG" ]; then + echo "No previous tag found. Skipping Claude-generated release notes." + exit 0 + fi + echo "Generating notes for ${PREV_TAG}..${TAG_NAME}" # Build context: commit log + diff stat + actual diff (truncated) @@ -90,7 +96,7 @@ jobs: role: "user", content: ("Here are the commits from " + $prev_tag + " to " + $tag + ":\n\n" + $commits + "\n\nDiff stat:\n" + $diffstat + "\n\nActual diff (truncated):\n" + $diff + "\n\nChangelog link: https://github.com/" + $repo + "/compare/" + $prev_tag + "..." + $tag + "\n\n" + $prompt) }] - }' | curl -s https://api.anthropic.com/v1/messages \ + }' | curl -s --connect-timeout 10 --max-time 60 https://api.anthropic.com/v1/messages \ -H "content-type: application/json" \ -H "x-api-key: ${ANTHROPIC_API_KEY}" \ -H "anthropic-version: 2023-06-01" \ diff --git a/.husky/pre-push b/.husky/pre-push index 03f55f9..dc06c77 100755 --- a/.husky/pre-push +++ b/.husky/pre-push @@ -15,7 +15,7 @@ if [ -n "$HEAD_SHA" ] && [ "$HEAD_SHA" = "$CACHED_SHA" ]; then echo "Tests already passed for $HEAD_SHA — skipping." else echo "Running full test suite (unit + integration) before push..." - npm run test:all + npm run test:all || exit 1 fi echo "Checking for dependency vulnerabilities..." diff --git a/CLAUDE.md b/CLAUDE.md index 3ad6445..12bb60d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -193,53 +193,53 @@ evals/ | Module | Purpose | Key Exports | |--------|---------|-------------| -| `cli-handlers.js` | CLI Command Handlers | `handleSetup()`, `handleAbort()`, `handleUpdate()`, `handleMcp()`, `handleAutoSkills()` | -| `cli-usage.js` | CLI Usage Text | `getUsage()` | -| `cli.js` | * Default values per spec §4.1 | `parseArgs()`, `validateStartArgs()`, `getUsage()`, `DEFAULTS()` | -| `conflict.js` | File Conflict Detection Module | `detectConflicts()`, `formatConflictWarning()` | -| `context-compression.js` | Context Compression Module | `compressContext()`, `estimateTokenCount()`, `buildPreamble()`, `DEFAULT_TOKEN_LIMIT()` | -| `context.js` | Context Filtering Module | `filterContext()`, `parseDuration()`, `estimateTokens()`, `takeLastNTurns()` | -| `drift.js` | Context Drift Detection Module | `calculateDrift()`, `formatDriftWarning()`, `countTurnsSince()`, `isDriftSignificant()` | -| `environment.js` | Environment Detection Module | `inferClient()`, `getSessionRoot()`, `detectEnvironment()`, `VALID_CLIENTS()` | -| `headless.js` | * Default timeout: 15 minutes per spec §6.2 | `runHeadless()`, `waitForServer()`, `extractSummary()`, `formatFoldOutput()`, `DEFAULT_TIMEOUT()` | -| `index.js` | Claude Sidecar - Main Module | `APIs()`, `startSidecar()`, `listSidecars()`, `resumeSidecar()`, `continueSidecar()` | -| `jsonl-parser.js` | JSONL Parser | `parseJSONLLine()`, `readJSONL()`, `extractTimestamp()`, `formatMessage()`, `formatContext()` | -| `mcp-server.js` | @module mcp-server — Sidecar MCP Server (stdio transport) | `handlers()`, `startMcpServer()`, `getProjectDir()` | -| `mcp-tools.js` | Zod pattern for safe task IDs (alphanumeric, hyphens, underscores only) | `getTools()`, `getGuideText()`, `safeTaskId()`, `safeModel()` | -| `opencode-client.js` | OpenCode SDK Client Wrapper | `parseModelString()`, `createClient()`, `createSession()`, `createChildSession()`, `sendPrompt()` | -| `prompt-builder.js` | System Prompt Builder | `buildSystemPrompt()`, `buildPrompts()`, `buildEnvironmentSection()`, `getSummaryTemplate()`, `SUMMARY_TEMPLATE()` | -| `session-manager.js` | * Session status constants | `createSession()`, `updateSession()`, `getSession()`, `saveConversation()`, `saveSummary()` | -| `session.js` | Session Resolver | `encodeProjectPath()`, `decodeProjectPath()`, `getSessionDirectory()`, `getSessionId()`, `resolveSession()` | -| `prompts/cowork-agent-prompt.js` | Cowork Agent Prompt | `buildCoworkAgentPrompt()` | -| `sidecar/context-builder.js` | Context Builder Module | `buildContext()`, `parseDuration()`, `resolveSessionFile()`, `applyContextFilters()`, `findCoworkSession()` | -| `sidecar/continue.js` | Load previous session data (metadata, summary, conversation) | `loadPreviousSession()`, `buildContinuationContext()`, `createContinueSessionMetadata()`, `continueSidecar()` | -| `sidecar/crash-handler.js` | Crash Handler - Updates metadata to 'error' on uncaught exceptions | `installCrashHandler()` | -| `sidecar/interactive.js` | Check if Electron is available (lazy loading guard) | `getElectronPath()`, `checkElectronAvailable()`, `buildElectronEnv()`, `handleElectronProcess()`, `runInteractive()` | -| `sidecar/progress.js` | Lifecycle stage labels | `readProgress()`, `writeProgress()`, `extractLatest()`, `computeLastActivity()`, `STAGE_LABELS()` | -| `sidecar/read.js` | Sidecar Read Operations Module | `formatAge()`, `listSidecars()`, `readSidecar()` | -| `sidecar/resume.js` | Load session metadata from session directory | `loadSessionMetadata()`, `loadInitialContext()`, `checkFileDrift()`, `buildDriftWarning()`, `buildResumeUserMessage()` | -| `sidecar/session-utils.js` | Standard heartbeat interval in milliseconds | `HEARTBEAT_INTERVAL()`, `SessionPaths()`, `saveInitialContext()`, `finalizeSession()`, `outputSummary()` | -| `sidecar/setup-window.js` | Setup Window Launcher | `launchSetupWindow()` | -| `sidecar/setup.js` | Sidecar Setup Wizard | `addAlias()`, `createDefaultConfig()`, `detectApiKeys()`, `runInteractiveSetup()`, `runReadlineSetup()` | -| `sidecar/start.js` | Generate a unique 8-character hex task ID | `generateTaskId()`, `createSessionMetadata()`, `buildMcpConfig()`, `checkElectronAvailable()`, `runInteractive()` | -| `utils/agent-mapping.js` | * All OpenCode native agent names (lowercase) | `PRIMARY_AGENTS()`, `OPENCODE_AGENTS()`, `HEADLESS_SAFE_AGENTS()`, `mapAgentToOpenCode()`, `isValidAgent()` | -| `utils/alias-resolver.js` | Alias Resolver Utilities | `applyDirectApiFallback()`, `autoRepairAlias()` | -| `utils/api-key-store.js` | Maps provider IDs to environment variable names | `getEnvPath()`, `readApiKeys()`, `readApiKeyHints()`, `readApiKeyValues()`, `saveApiKey()` | -| `utils/api-key-validation.js` | Validation endpoints per provider | `validateApiKey()`, `validateOpenRouterKey()`, `VALIDATION_ENDPOINTS()` | -| `utils/auth-json.js` | Known provider IDs that map to sidecar's PROVIDER_ENV_MAP | `readAuthJsonKeys()`, `importFromAuthJson()`, `checkAuthJson()`, `removeFromAuthJson()`, `AUTH_JSON_PATH()` | -| `utils/auto-skills-config.js` | Valid auto-skill names (keys in autoSkills config) | `VALID_SKILL_NAMES()`, `SKILL_LABELS()`, `getAutoSkillsConfig()`, `isSkillEnabled()`, `isMonitoringEnabled()` | -| `utils/config.js` | Default model alias map — short names to full OpenRouter model identifiers | `getConfigDir()`, `getConfigPath()`, `loadConfig()`, `saveConfig()`, `getDefaultAliases()` | -| `utils/logger.js` | Structured Logger Module | `logger()`, `LOG_LEVELS()` | -| `utils/mcp-discovery.js` | MCP Discovery - Discovers MCP servers from parent LLM configuration | `discoverParentMcps()`, `discoverClaudeCodeMcps()`, `discoverCoworkMcps()`, `normalizeMcpJson()` | -| `utils/mcp-validators.js` | MCP Validators | `validateMcpSpec()`, `validateMcpConfigFile()` | -| `utils/model-fetcher.js` | Hardcoded Anthropic models (no public listing endpoint) | `fetchModelsFromProvider()`, `fetchAllModels()`, `groupModelsByFamily()`, `ANTHROPIC_MODELS()`, `PROVIDER_FAMILY_NAMES()` | -| `utils/model-validator.js` | Alias-to-search-term mapping for filtering provider model lists | `validateDirectModel()`, `filterRelevantModels()`, `normalizeModelId()` | -| `utils/path-setup.js` | Ensures that the project's node_modules/.bin directory is included in the PATH. | `ensureNodeModulesBinInPath()` | -| `utils/server-setup.js` | Server Setup Utilities | `DEFAULT_PORT()`, `isPortInUse()`, `getPortPid()`, `killPortProcess()`, `ensurePortAvailable()` | -| `utils/start-helpers.js` | Start Command Helpers | `resolveModelFromArgs()`, `validateFallbackModel()` | -| `utils/thinking-validators.js` | Thinking Level Validators | `MODEL_THINKING_SUPPORT()`, `getSupportedThinkingLevels()`, `validateThinkingLevel()` | -| `utils/updater.js` | @type {import('update-notifier').UpdateNotifier|null} | `initUpdateCheck()`, `getUpdateInfo()`, `notifyUpdate()`, `performUpdate()` | -| `utils/validators.js` | * Provider to API key mapping | `VALID_AGENT_MODES()`, `PROVIDER_KEY_MAP()`, `MODEL_THINKING_SUPPORT()`, `TASK_ID_PATTERN()`, `validateTaskId()` | +| `cli-handlers.js` | CLI Command Handlers | `handleSetup`, `handleAbort`, `handleUpdate`, `handleMcp`, `handleAutoSkills` | +| `cli-usage.js` | CLI Usage Text | `getUsage` | +| `cli.js` | * Default values per spec §4.1 | `parseArgs`, `validateStartArgs`, `getUsage`, `DEFAULTS` | +| `conflict.js` | File Conflict Detection Module | `detectConflicts`, `formatConflictWarning` | +| `context-compression.js` | Context Compression Module | `compressContext`, `estimateTokenCount`, `buildPreamble`, `DEFAULT_TOKEN_LIMIT` | +| `context.js` | Context Filtering Module | `filterContext`, `parseDuration`, `estimateTokens`, `takeLastNTurns` | +| `drift.js` | Context Drift Detection Module | `calculateDrift`, `formatDriftWarning`, `countTurnsSince`, `isDriftSignificant` | +| `environment.js` | Environment Detection Module | `inferClient`, `getSessionRoot`, `detectEnvironment`, `VALID_CLIENTS` | +| `headless.js` | * Default timeout: 15 minutes per spec §6.2 | `runHeadless`, `waitForServer`, `extractSummary`, `formatFoldOutput`, `DEFAULT_TIMEOUT` | +| `index.js` | Claude Sidecar - Main Module | `APIs`, `startSidecar`, `listSidecars`, `resumeSidecar`, `continueSidecar` | +| `jsonl-parser.js` | JSONL Parser | `parseJSONLLine`, `readJSONL`, `extractTimestamp`, `formatMessage`, `formatContext` | +| `mcp-server.js` | @module mcp-server — Sidecar MCP Server (stdio transport) | `handlers`, `startMcpServer`, `getProjectDir` | +| `mcp-tools.js` | Zod pattern for safe task IDs (alphanumeric, hyphens, underscores only) | `getTools`, `getGuideText`, `safeTaskId`, `safeModel` | +| `opencode-client.js` | OpenCode SDK Client Wrapper | `parseModelString`, `createClient`, `createSession`, `createChildSession`, `sendPrompt` | +| `prompt-builder.js` | System Prompt Builder | `buildSystemPrompt`, `buildPrompts`, `buildEnvironmentSection`, `getSummaryTemplate`, `SUMMARY_TEMPLATE` | +| `session-manager.js` | * Session status constants | `createSession`, `updateSession`, `getSession`, `saveConversation`, `saveSummary` | +| `session.js` | Session Resolver | `encodeProjectPath`, `decodeProjectPath`, `getSessionDirectory`, `getSessionId`, `resolveSession` | +| `prompts/cowork-agent-prompt.js` | Cowork Agent Prompt | `buildCoworkAgentPrompt` | +| `sidecar/context-builder.js` | Context Builder Module | `buildContext`, `parseDuration`, `resolveSessionFile`, `applyContextFilters`, `findCoworkSession` | +| `sidecar/continue.js` | Load previous session data (metadata, summary, conversation) | `loadPreviousSession`, `buildContinuationContext`, `createContinueSessionMetadata`, `continueSidecar` | +| `sidecar/crash-handler.js` | Crash Handler - Updates metadata to 'error' on uncaught exceptions | `installCrashHandler` | +| `sidecar/interactive.js` | Check if Electron is available (lazy loading guard) | `getElectronPath`, `checkElectronAvailable`, `buildElectronEnv`, `handleElectronProcess`, `runInteractive` | +| `sidecar/progress.js` | Lifecycle stage labels | `readProgress`, `writeProgress`, `extractLatest`, `computeLastActivity`, `STAGE_LABELS` | +| `sidecar/read.js` | Sidecar Read Operations Module | `formatAge`, `listSidecars`, `readSidecar` | +| `sidecar/resume.js` | Load session metadata from session directory | `loadSessionMetadata`, `loadInitialContext`, `checkFileDrift`, `buildDriftWarning`, `buildResumeUserMessage` | +| `sidecar/session-utils.js` | Standard heartbeat interval in milliseconds | `HEARTBEAT_INTERVAL`, `SessionPaths`, `saveInitialContext`, `finalizeSession`, `outputSummary` | +| `sidecar/setup-window.js` | Setup Window Launcher | `launchSetupWindow` | +| `sidecar/setup.js` | Sidecar Setup Wizard | `addAlias`, `createDefaultConfig`, `detectApiKeys`, `runInteractiveSetup`, `runReadlineSetup` | +| `sidecar/start.js` | Generate a unique 8-character hex task ID | `generateTaskId`, `createSessionMetadata`, `buildMcpConfig`, `checkElectronAvailable`, `runInteractive` | +| `utils/agent-mapping.js` | * All OpenCode native agent names (lowercase) | `PRIMARY_AGENTS`, `OPENCODE_AGENTS`, `HEADLESS_SAFE_AGENTS`, `mapAgentToOpenCode`, `isValidAgent` | +| `utils/alias-resolver.js` | Alias Resolver Utilities | `applyDirectApiFallback`, `autoRepairAlias` | +| `utils/api-key-store.js` | Maps provider IDs to environment variable names | `getEnvPath`, `readApiKeys`, `readApiKeyHints`, `readApiKeyValues`, `saveApiKey` | +| `utils/api-key-validation.js` | Validation endpoints per provider | `validateApiKey`, `validateOpenRouterKey`, `VALIDATION_ENDPOINTS` | +| `utils/auth-json.js` | Known provider IDs that map to sidecar's PROVIDER_ENV_MAP | `readAuthJsonKeys`, `importFromAuthJson`, `checkAuthJson`, `removeFromAuthJson`, `AUTH_JSON_PATH` | +| `utils/auto-skills-config.js` | Valid auto-skill names (keys in autoSkills config) | `VALID_SKILL_NAMES`, `SKILL_LABELS`, `getAutoSkillsConfig`, `isSkillEnabled`, `isMonitoringEnabled` | +| `utils/config.js` | Default model alias map — short names to full OpenRouter model identifiers | `getConfigDir`, `getConfigPath`, `loadConfig`, `saveConfig`, `getDefaultAliases` | +| `utils/logger.js` | Structured Logger Module | `logger`, `LOG_LEVELS` | +| `utils/mcp-discovery.js` | MCP Discovery - Discovers MCP servers from parent LLM configuration | `discoverParentMcps`, `discoverClaudeCodeMcps`, `discoverCoworkMcps`, `normalizeMcpJson` | +| `utils/mcp-validators.js` | MCP Validators | `validateMcpSpec`, `validateMcpConfigFile` | +| `utils/model-fetcher.js` | Hardcoded Anthropic models (no public listing endpoint) | `fetchModelsFromProvider`, `fetchAllModels`, `groupModelsByFamily`, `ANTHROPIC_MODELS`, `PROVIDER_FAMILY_NAMES` | +| `utils/model-validator.js` | Alias-to-search-term mapping for filtering provider model lists | `validateDirectModel`, `filterRelevantModels`, `normalizeModelId` | +| `utils/path-setup.js` | Ensures that the project's node_modules/.bin directory is included in the PATH. | `ensureNodeModulesBinInPath` | +| `utils/server-setup.js` | Server Setup Utilities | `DEFAULT_PORT`, `isPortInUse`, `getPortPid`, `killPortProcess`, `ensurePortAvailable` | +| `utils/start-helpers.js` | Start Command Helpers | `resolveModelFromArgs`, `validateFallbackModel` | +| `utils/thinking-validators.js` | Thinking Level Validators | `MODEL_THINKING_SUPPORT`, `getSupportedThinkingLevels`, `validateThinkingLevel` | +| `utils/updater.js` | @type {import('update-notifier').UpdateNotifier|null} | `initUpdateCheck`, `getUpdateInfo`, `notifyUpdate`, `performUpdate` | +| `utils/validators.js` | * Provider to API key mapping | `VALID_AGENT_MODES`, `PROVIDER_KEY_MAP`, `MODEL_THINKING_SUPPORT`, `TASK_ID_PATTERN`, `validateTaskId` | --- diff --git a/bin/sidecar.js b/bin/sidecar.js index 01f6791..122b48d 100755 --- a/bin/sidecar.js +++ b/bin/sidecar.js @@ -104,7 +104,7 @@ async function main() { await handleUpdate(); break; case 'auto-skills': - handleAutoSkills(args); + await handleAutoSkills(args); break; default: console.error(`Unknown command: ${command}`); diff --git a/docs/publishing.md b/docs/publishing.md index 49dc71c..b4ad8ae 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -14,7 +14,7 @@ The `.github/workflows/publish.yml` workflow triggers on `v*` tags and publishes ## Publishing Setup -- **Trusted Publisher**: Configured on npm for `jrenaldi79/sidecar` + `publish.yml` (OIDC-based, no manual token management) -- **NPM_TOKEN**: Granular access token stored as GitHub secret (bypass 2FA enabled, scoped to `claude-sidecar`) +- **Trusted Publisher**: Configured on npm for `jrenaldi79/sidecar` + `publish.yml` (OIDC-based provenance attestation) +- **NPM_TOKEN**: Granular automation token stored as GitHub secret (scoped to `claude-sidecar`, required for `npm publish`) - **OIDC provenance**: `--provenance` flag adds Sigstore attestation (requires `id-token: write` permission) - **Trusted publisher config**: https://www.npmjs.com/package/claude-sidecar/access (Settings tab) diff --git a/electron/ipc-setup.js b/electron/ipc-setup.js index a59d315..f077e58 100644 --- a/electron/ipc-setup.js +++ b/electron/ipc-setup.js @@ -40,10 +40,14 @@ function registerSetupHandlers(ipcMain, getMainWindow) { const { removeApiKey } = require('../src/utils/api-key-store'); const { removeFromAuthJson } = require('../src/utils/auth-json'); const result = removeApiKey(provider); - // Always clean auth.json too — prevents auto-import from re-adding the key + // Clean auth.json when present — prevents auto-import from re-adding the key if (result.alsoInAuthJson) { - removeFromAuthJson(provider); - result.alsoInAuthJson = false; + try { + removeFromAuthJson(provider); + result.alsoInAuthJson = false; + } catch (authErr) { + logger.warn('Failed to remove from auth.json', { provider, error: authErr.message }); + } } return result; } catch (err) { @@ -99,21 +103,32 @@ function registerSetupHandlers(ipcMain, getMainWindow) { }); ipcMain.handle('sidecar:get-api-keys', () => { - const { readApiKeys, readApiKeyHints, saveApiKey } = require('../src/utils/api-key-store'); - const { importFromAuthJson } = require('../src/utils/auth-json'); - const status = readApiKeys(); - const hints = readApiKeyHints(); + try { + const { readApiKeys, readApiKeyHints, saveApiKey } = require('../src/utils/api-key-store'); + const { importFromAuthJson } = require('../src/utils/auth-json'); + const status = readApiKeys(); + const hints = readApiKeyHints(); - // Auto-import keys from auth.json that sidecar doesn't have yet - const { imported } = importFromAuthJson(status); - for (const entry of imported) { - saveApiKey(entry.provider, entry.key); - status[entry.provider] = true; - const visible = entry.key.slice(0, 8); - hints[entry.provider] = visible + '\u2022'.repeat(Math.max(0, Math.min(entry.key.length - 8, 12))); - } + // Auto-import keys from auth.json that sidecar doesn't have yet + const { imported } = importFromAuthJson(status); + const successfullyImported = []; + for (const entry of imported) { + const result = saveApiKey(entry.provider, entry.key); + if (result.success) { + status[entry.provider] = true; + const visible = entry.key.slice(0, 8); + hints[entry.provider] = visible + '\u2022'.repeat(Math.max(0, Math.min(entry.key.length - 8, 12))); + successfullyImported.push(entry.provider); + } else { + logger.warn('Failed to import key from auth.json', { provider: entry.provider, error: result.error }); + } + } - return { status, hints, imported: imported.map(e => e.provider) }; + return { status, hints, imported: successfullyImported }; + } catch (err) { + logger.error('get-api-keys handler error', { error: err.message }); + return { status: {}, hints: {}, imported: [], error: err.message }; + } }); ipcMain.handle('sidecar:fetch-models', async () => { diff --git a/hooks/post-failure.sh b/hooks/post-failure.sh index 4a14ede..29645fc 100755 --- a/hooks/post-failure.sh +++ b/hooks/post-failure.sh @@ -55,8 +55,11 @@ EVENT=$(jq -rc --arg ts "$(date -u +%Y-%m-%dT%H:%M:%SZ)" ' }' "$TMP_JSON" 2>/dev/null || echo "") if [ -n "$EVENT" ]; then - if [ ! -f "$EVENT_FILE" ]; then - (umask 177 && : > "$EVENT_FILE") + # Atomically create file with restrictive permissions (noclobber prevents TOCTOU) + (umask 177 && set -o noclobber && : > "$EVENT_FILE" 2>/dev/null) || true + # Verify ownership before appending + FILE_OWNER=$(stat -f '%u' "$EVENT_FILE" 2>/dev/null || stat -c '%u' "$EVENT_FILE" 2>/dev/null || echo "") + if [ "$FILE_OWNER" = "$(id -u)" ]; then + echo "$EVENT" >> "$EVENT_FILE" fi - echo "$EVENT" >> "$EVENT_FILE" fi diff --git a/hooks/post-tool-use.sh b/hooks/post-tool-use.sh index 2519926..eef56c6 100755 --- a/hooks/post-tool-use.sh +++ b/hooks/post-tool-use.sh @@ -64,16 +64,19 @@ if [ -n "$SAFE_SID" ]; then file: (if .tool_name == "Bash" then "" elif .tool_name == "MultiEdit" then (.tool_input.edits[0].file_path // "") else (.tool_input.file_path // "") end), - success: (if .tool_name == "Bash" then ((.tool_response.exit_code // 0) == 0) else true end), + success: (if .tool_name == "Bash" then ((.tool_response.exit_code // 0) == 0) + else true end), # non-Bash tools: hook only fires on success command: (if .tool_name == "Bash" then (.tool_input.command // "") else "" end) }' "$TMP_JSON" 2>/dev/null || echo "") if [ -n "$EVENT" ]; then - # Create with restrictive permissions atomically (umask prevents TOCTOU window) - if [ ! -f "$EVENT_FILE" ]; then - (umask 177 && : > "$EVENT_FILE") + # Atomically create file with restrictive permissions (noclobber prevents TOCTOU) + (umask 177 && set -o noclobber && : > "$EVENT_FILE" 2>/dev/null) || true + # Verify ownership before appending + FILE_OWNER=$(stat -f '%u' "$EVENT_FILE" 2>/dev/null || stat -c '%u' "$EVENT_FILE" 2>/dev/null || echo "") + if [ "$FILE_OWNER" = "$(id -u)" ]; then + echo "$EVENT" >> "$EVENT_FILE" fi - echo "$EVENT" >> "$EVENT_FILE" fi fi fi diff --git a/scripts/generate-docs-helpers.js b/scripts/generate-docs-helpers.js index b5bda0e..f726bc4 100644 --- a/scripts/generate-docs-helpers.js +++ b/scripts/generate-docs-helpers.js @@ -204,7 +204,7 @@ function collectModules(dirPath, rows, relPrefix = '') { rows.push({ module: modulePath, purpose: desc || '', - exports: exps.map(e => `\`${e}()\``).join(', '), + exports: exps.map(e => `\`${e}\``).join(', '), }); } diff --git a/site/social-card-render.html b/site/social-card-render.html index b081ccc..e2d6f86 100644 --- a/site/social-card-render.html +++ b/site/social-card-render.html @@ -1,7 +1,8 @@ - + +Claude Sidecar - Social Card