Fix reflect skill: remove $() substitution that fails permission checks
The !`command` blocks in SKILL.md go through Claude Code's Bash permission checker, which rejects $() command substitution. Simplified the git log command, replaced dynamic verify script path with a Read tool instruction, and narrowed allowed-tools to specific command patterns. Also documented the constraints in CLAUDE.md for future skill development. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
5ff98763cd
commit
85dfa2ef2e
10
CLAUDE.md
10
CLAUDE.md
@ -32,3 +32,13 @@ custom-claude-skills/
|
|||||||
- **Restrict tools** with `allowed-tools` to the minimum needed — this reduces permission prompts
|
- **Restrict tools** with `allowed-tools` to the minimum needed — this reduces permission prompts
|
||||||
- **Use $ARGUMENTS** for user input, `$0`, `$1` etc. for positional args
|
- **Use $ARGUMENTS** for user input, `$0`, `$1` etc. for positional args
|
||||||
- Dynamic commands in `!`command`` run at skill load time, not during Claude's execution
|
- Dynamic commands in `!`command`` run at skill load time, not during Claude's execution
|
||||||
|
|
||||||
|
## `!`command`` Gotchas
|
||||||
|
|
||||||
|
The `!`command`` syntax in SKILL.md runs shell commands at skill load time to pre-gather context. These commands go through Claude Code's Bash permission checker against the `allowed-tools` patterns. Key constraints:
|
||||||
|
|
||||||
|
- **No `$()` command substitution** — the permission checker rejects commands containing `$()` even if the outer command matches an allowed pattern. This means you cannot nest subshells (e.g., `git log --since="$(git log ...)"` will fail).
|
||||||
|
- **No complex shell pipelines relying on subshells** — keep `!`command`` commands simple and self-contained. If you need complex logic, have the skill instructions tell Claude to run it via tool calls instead.
|
||||||
|
- **`allowed-tools` patterns must match the command binary** — `Bash(git *)` allows any `git` subcommand, but `Bash(cat *)`, `Bash(sed *)`, `Bash(ls *)` must be listed separately if those commands appear in `!`command`` blocks.
|
||||||
|
- **Prefer specific tool patterns over broad ones** — `Bash(git log *)`, `Bash(git diff *)` is safer than `Bash(git *)` to avoid accidentally allowing destructive git commands (e.g., `git reset`, `git push --force`).
|
||||||
|
- **Fallback to tool instructions for dynamic paths** — if a command needs `$ARGUMENTS` to compute a file path (e.g., `cat scripts/verify-m${N}.sh`), replace the `!`command`` with a plain-text instruction telling Claude to use the Read tool. This avoids `$()` substitution and is more reliable.
|
||||||
|
|||||||
@ -4,7 +4,7 @@ description: >
|
|||||||
Run a milestone reflection after completing a milestone. Reviews the full conversation history,
|
Run a milestone reflection after completing a milestone. Reviews the full conversation history,
|
||||||
git log, and current project docs to produce structured reflection artifacts. Use when a milestone
|
git log, and current project docs to produce structured reflection artifacts. Use when a milestone
|
||||||
is complete and verified. Invoke with the milestone number, e.g. /reflect M8
|
is complete and verified. Invoke with the milestone number, e.g. /reflect M8
|
||||||
allowed-tools: Read, Edit, Write, Grep, Glob, Bash(git *)
|
allowed-tools: Read, Edit, Write, Grep, Glob, Bash(git log *), Bash(git diff *), Bash(git show *), Bash(cat *), Bash(sed *), Bash(ls *)
|
||||||
---
|
---
|
||||||
|
|
||||||
# Milestone Reflection Skill
|
# Milestone Reflection Skill
|
||||||
@ -19,8 +19,8 @@ detour, every wrong assumption, every backtrack — and distill it into structur
|
|||||||
|
|
||||||
## Pre-gathered context
|
## Pre-gathered context
|
||||||
|
|
||||||
### Git log for this milestone
|
### Git log (last 50 commits)
|
||||||
!`git log --oneline --since="$(git log --all --oneline --grep="[Mm]$(echo '$ARGUMENTS' | tr -d 'Mm' | awk '{print $1-1}').*[Vv]erif\|[Mm]$(echo '$ARGUMENTS' | tr -d 'Mm' | awk '{print $1-1}').*[Rr]eflect\|[Mm]$(echo '$ARGUMENTS' | tr -d 'Mm' | awk '{print $1-1}').*[Cc]omplete" --format='%aI' | head -1 || echo '3 months ago')" 2>/dev/null || git log --oneline -30`
|
!`git log --oneline -50`
|
||||||
|
|
||||||
### Current MEMORY.md
|
### Current MEMORY.md
|
||||||
!`cat MEMORY.md 2>/dev/null || echo "No MEMORY.md found"`
|
!`cat MEMORY.md 2>/dev/null || echo "No MEMORY.md found"`
|
||||||
@ -35,7 +35,7 @@ detour, every wrong assumption, every backtrack — and distill it into structur
|
|||||||
!`ls -1 scripts/ 2>/dev/null || echo "No scripts directory"`
|
!`ls -1 scripts/ 2>/dev/null || echo "No scripts directory"`
|
||||||
|
|
||||||
### Verify script for this milestone
|
### Verify script for this milestone
|
||||||
!`cat scripts/verify-m$(echo '$ARGUMENTS' | tr -d 'Mm' | awk '{print $1}').sh 2>/dev/null || echo "No verify script found for this milestone"`
|
(Read the verify script manually using the Read tool: `scripts/verify-m<N>.sh` where N is the milestone number from $ARGUMENTS)
|
||||||
|
|
||||||
## Instructions
|
## Instructions
|
||||||
|
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user