-
Notifications
You must be signed in to change notification settings - Fork 0
Improve Claude Code setup: SessionStart hook, git perms, planning + review wiring #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,27 @@ | ||
| --- | ||
| description: Review the current changes with the aai CLI's specialized reviewers (security + template contract), scoped to what actually changed. | ||
| description: Review the current changes with the code-review skill plus the aai CLI's specialized reviewers (security + template contract), scoped to what actually changed. | ||
| argument-hint: "[git ref to diff against, default: HEAD]" | ||
| allowed-tools: Bash(git diff *), Bash(git status *), Bash(git log *), Task | ||
| allowed-tools: Bash(git diff *), Bash(git status *), Bash(git log *), Task, Skill | ||
| --- | ||
|
|
||
| Review the current working changes using this project's specialized subagents. Be surgical: only run a reviewer if the diff actually touches its area. | ||
| Review the current working changes using the general `code-review` skill and this project's specialized subagents. Be surgical: the code review always runs on the diff; only run a specialized reviewer if the diff actually touches its area. | ||
|
|
||
| ## 1. Scope the diff | ||
|
|
||
| Run `git status --short` and `git diff --stat ${1:-HEAD}` (and `git diff ${1:-HEAD}` for detail) to see what changed. | ||
|
|
||
| ## 2. Dispatch the relevant reviewers (in parallel) | ||
| ## 2. Run the general code review (always) | ||
|
|
||
| - If the diff touches **`aai_cli/auth/`, `config.py`, `environments.py`, `client.py`, or any `subprocess` call** → dispatch the **`security-reviewer`** agent on those changes. | ||
| Invoke the **`code-review`** skill on the current changes for correctness bugs and reuse/simplification/efficiency cleanups. This runs on every `/review-changes`, regardless of which files changed. | ||
|
|
||
| ## 3. Dispatch the relevant specialized reviewers (in parallel) | ||
|
|
||
| - If the diff touches **`aai_cli/auth/`, `config.py`, `environments.py`, `client.py`, or any `subprocess` call** → run the **`security-review`** skill for the general security pass, **then** dispatch the project's **`security-reviewer`** agent for the AssemblyAI-specific guarantees the generic skill won't know (key never on disk/argv, env↔credential binding, `public-token-*` only, fixed-argv subprocess shell-outs). | ||
| - If the diff touches **`aai_cli/init/templates/`, `aai_cli/init/scaffold.py`, `aai_cli/init/templates.py`, or the wheel-packaging config in `pyproject.toml`** → dispatch the **`template-contract-reviewer`** agent on those changes. | ||
| - If the diff touches neither sensitive area, say so and skip — don't manufacture a review. | ||
| - If the diff touches none of those sensitive areas, say so and skip the specialized reviewers — don't manufacture a review. (The `code-review` skill from step 2 still ran.) | ||
|
|
||
| Pass each agent the exact list of changed files in its scope so it reviews the diff, not the whole repo. | ||
|
|
||
| ## 3. Synthesize | ||
| ## 4. Synthesize | ||
|
|
||
| Combine the findings into one ranked report (severity → file:line → fix). Call out anything that would break the CLI's security guarantees (key never on disk/argv, env↔credential binding) or the template packaging contract. If both reviewers come back clean, state that plainly. | ||
| Combine the `code-review` findings and any specialized-reviewer findings into one ranked report (severity → file:line → fix). Call out anything that would break the CLI's security guarantees (key never on disk/argv, env↔credential binding) or the template packaging contract. If everything comes back clean, state that plainly. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| #!/usr/bin/env bash | ||
| # SessionStart hook: provision a Claude Code on the web container so the canonical | ||
| # gate (./scripts/check.sh) runs green here exactly as it does in CI and locally. | ||
| # | ||
| # Mirrors the deps CI installs (.github/workflows/ci.yml): | ||
| # - system: libportaudio2 (sounddevice), ffmpeg (--sample stream sources), shellcheck | ||
| # - node: markdownlint-cli@0.45.0 (check.sh calls `markdownlint` directly) | ||
| # - python: `uv sync` to materialize the locked dev environment up front | ||
| # | ||
| # Only runs in the remote (web) environment — local dev machines already have | ||
| # these and shouldn't be reprovisioned. Idempotent and non-interactive. | ||
| set -euo pipefail | ||
|
|
||
| # Web-only: skip entirely on local machines. | ||
| if [ "${CLAUDE_CODE_REMOTE:-}" != "true" ]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| cd "${CLAUDE_PROJECT_DIR:-$(cd "$(dirname "$0")/../.." && pwd)}" | ||
|
|
||
| # Pin markdownlint-cli to the version CI uses so lint results match exactly. | ||
| MARKDOWNLINT_VERSION="0.45.0" | ||
|
|
||
| log() { echo "[session-start] $*"; } | ||
|
|
||
| # 1. System packages (PortAudio + ffmpeg + shellcheck). apt-get is idempotent; | ||
| # --no-install-recommends keeps the layer small. Skip the install if all three | ||
| # are already present so resumed/cached containers don't re-run apt. | ||
| need_apt=0 | ||
| command -v ffmpeg >/dev/null 2>&1 || need_apt=1 | ||
| command -v shellcheck >/dev/null 2>&1 || need_apt=1 | ||
| ldconfig -p 2>/dev/null | grep -q portaudio || need_apt=1 | ||
| if [ "$need_apt" = "1" ]; then | ||
| log "installing system deps (libportaudio2, ffmpeg, shellcheck)" | ||
| export DEBIAN_FRONTEND=noninteractive | ||
| # `apt-get update` can exit non-zero when unrelated third-party PPAs are | ||
| # unreachable; the main Ubuntu archive (which has these packages) still | ||
| # refreshes, so tolerate that failure and let the install be the real signal. | ||
| apt-get update -qq || log "apt-get update reported errors (likely unrelated PPAs); continuing" | ||
| # Don't let a missing system package abort the whole session — a partially | ||
| # provisioned container is still usable; check.sh self-skips shellcheck. | ||
| apt-get install -y --no-install-recommends libportaudio2 ffmpeg shellcheck \ | ||
| || log "WARNING: apt-get install failed; some system deps may be missing" | ||
| else | ||
| log "system deps already present" | ||
| fi | ||
|
|
||
| # 2. markdownlint CLI (Node) — check.sh invokes `markdownlint` as a global binary, | ||
| # not through uv, so it must be on PATH. | ||
| if ! command -v markdownlint >/dev/null 2>&1; then | ||
| log "installing markdownlint-cli@${MARKDOWNLINT_VERSION}" | ||
| npm install -g "markdownlint-cli@${MARKDOWNLINT_VERSION}" | ||
| else | ||
| log "markdownlint already present" | ||
| fi | ||
|
|
||
| # 3. Python environment — materialize the locked dev env so the first `uv run` | ||
| # doesn't pay the full sync cost mid-task. `uv` syncs the default dev group. | ||
| log "syncing uv environment (locked dev group)" | ||
| uv sync | ||
|
|
||
| log "provisioning complete" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| --- | ||
| name: plan-feature | ||
| description: Plan a new aai CLI feature the way this repo does — drive the superpowers planning skill, then persist the spec + plan into docs/superpowers/ under this repo's naming and verification conventions. Use before building anything non-trivial. | ||
| disable-model-invocation: true | ||
| argument-hint: "<short feature description>" | ||
| --- | ||
|
|
||
| # plan-feature | ||
|
|
||
| This repo plans features with the **superpowers** skill (the existing plans say | ||
| so: `REQUIRED SUB-SKILL: superpowers:subagent-driven-development` / | ||
| `superpowers:executing-plans`). This skill is a thin wrapper: it runs the | ||
| superpowers planning flow, then drops the artifacts into `docs/superpowers/` | ||
| under this repo's filenames and verification gates. It does **not** reinvent the | ||
| planning methodology, and it does **not** start writing feature code — the | ||
| output is two reviewable documents. | ||
|
|
||
| ## 1. Drive the superpowers planning flow | ||
|
|
||
| Use the superpowers skill to do the actual work — brainstorming the design, then | ||
| writing the task-by-task plan. (If the superpowers plugin isn't installed in | ||
| this environment, say so and follow the structure of the existing files in | ||
| `docs/superpowers/specs/` and `plans/` by hand.) | ||
|
|
||
| Ground the design in this repo's real architecture — read `CLAUDE.md` first: | ||
| Typer sub-apps under `aai_cli/commands/` run through `context.run_command`; the | ||
| API key never touches disk or argv; errors are `CLIError`s on stderr (data on | ||
| stdout); new SDK calls follow `client.py`'s auth-failure/`APIError` shape. The | ||
| design must call out which of those guarantees it touches. | ||
|
|
||
| ## 2. Persist with this repo's names | ||
|
|
||
| Superpowers' outputs land here, matching the existing files exactly: | ||
|
|
||
| - Spec: `docs/superpowers/specs/<DATE>-<SLUG>-design.md` | ||
| - Plan: `docs/superpowers/plans/<DATE>-<SLUG>.md` | ||
|
|
||
| where `DATE` is today (`YYYY-MM-DD`, the repo's `currentDate` — not a guess) and | ||
| `SLUG` is short kebab-case (e.g. `show-code`, `cli-color-theme`). If a file | ||
| already exists at either path, stop and ask whether to amend rather than | ||
| overwrite. | ||
|
|
||
| ## 3. Bake in this repo's verification gates | ||
|
|
||
| Whatever plan superpowers produces, its self-review / done-criteria must include | ||
| this repo's gates (the implementation isn't done until these pass): | ||
|
|
||
| - [ ] `./scripts/check.sh` is green (ruff, mypy, pyright, markdownlint, shellcheck, pytest with the **90% branch-coverage gate**, build, twine). | ||
| - [ ] `/review-changes` run — it runs the `code-review` skill on the diff, plus (for security-sensitive diffs touching `aai_cli/auth/`, `config.py`, `environments.py`, `client.py`, or any `subprocess` call) the `security-review` skill followed by the project's `security-reviewer` agent, and the `template-contract-reviewer` agent (if it touches `aai_cli/init/templates/` or wheel packaging). | ||
| - [ ] Security guarantees intact: key never on disk/argv; env↔credential binding preserved. | ||
|
|
||
| ## 4. Hand off | ||
|
|
||
| Report the two file paths created and a one-line summary of each. Tell the user | ||
| the design is ready for review and that implementation proceeds task-by-task via | ||
| superpowers, running `/check` before pushing and `/review-changes` on the diff. | ||
| Do not begin implementing in this skill. | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Frontmatter disables model invocation but the skill delegates to the model-driven 'superpowers' flow; make the frontmatter reflect that model invocation is required or clarify offline behavior.
Details
✨ AI Reasoning
The skill's YAML frontmatter sets disable-model-invocation to true, which signals the skill must not invoke models. The skill description repeatedly instructs the operator to 'Use the superpowers skill' and describes driving the superpowers planning flow — behavior that requires invoking another model-driven skill. This is a behavioral mismatch: the frontmatter claims the skill will not invoke models while the body requires model-driven delegation. This creates deceptive metadata that could cause tooling to skip required model access or misrepresent capabilities to users.
🔧 How do I fix it?
Ensure skill actions match the description. Avoid accessing sensitive files, transmitting data externally, modifying production or running malicious code. Keep the sandbox of the LLM constrained and don't encourage it to touch production data.
Reply
@AikidoSec feedback: [FEEDBACK]to get better review comments in the future.Reply
@AikidoSec ignore: [REASON]to ignore this issue.More info