diff --git a/.github/workflows/gate.yml b/.github/workflows/gate.yml new file mode 100644 index 0000000..26b7b8f --- /dev/null +++ b/.github/workflows/gate.yml @@ -0,0 +1,40 @@ +name: gate + +on: + push: + branches: [main] + pull_request: + +# Nothing here writes to the repo; keep the token read-only even for same-repo branches. +permissions: + contents: read + +jobs: + gate: + name: Local quality gate (CLAUDE.md §19.3) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 + + - name: Install shellcheck and jq + run: sudo apt-get update -q && sudo apt-get install -y -q shellcheck jq + + - name: Bash hooks and tests lint clean + run: shellcheck hooks/*.sh tests/hooks/*.sh + + - name: Settings parse as valid JSON + run: jq empty settings.json settings2.json + + - name: Every agent declares a name + run: test -z "$(grep -L '^name:' agents*/*.md)" + + - name: Scaffolder templates match the canonical project template + run: | + diff -q templates/CLAUDE.project.md skills/new-repo/templates/web/CLAUDE.md.tmpl + diff -q templates/CLAUDE.project.md skills/new-repo/templates/android/CLAUDE.md.tmpl + + - name: Commit-guard hook behavior + run: bash tests/hooks/test-guard-commit.sh + + - name: Format hook behavior + run: bash tests/hooks/test-format.sh diff --git a/CLAUDE.md b/CLAUDE.md index a0f73c2..4f91d23 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -58,7 +58,7 @@ Every non-trivial change goes through four phases, in order — wear all four ha 1. **Architect** (read-only): minimal change? existing patterns to match? failure modes (auth, network, concurrency, data loss, partial failure, retry storms)? scope boundaries? → a 3–8 line plan, user-approved. Run this in **plan mode** so the read-only constraint is enforced by the harness, not just intended. 2. **Engineer:** implement the plan, nothing more. One _purpose_ per commit. Run the code; if it doesn't start, fix it before moving on. -3. **Code Reviewer** (read-only, adversarial): walk the review checklist against the diff as if you didn't write it. Zero-open-comments rule — every issue fixed or explicitly justified before proceeding. +3. **Code Reviewer** (read-only, adversarial): walk the review checklist against the diff, and the diff against the Phase 1 plan, as if you didn't write it. Zero-open-comments rule — every issue fixed or explicitly justified before proceeding. 4. **QA:** run the local quality gate (platform rule §7), the happy path, at least one realistic error case, and a regression check. If a test fails, diagnose the failure class (§4.7) before reacting. Don't push broken code. **§4.5 Skip rules.** Typo / config-only: skip Phase 1 & 4. `type:chore|docs|qa` that's text-/config-/test-config-only: skip Phases 1, 3, 4 (use judgement — keep all four if there's real surface area). Everything else: all four, every time. @@ -74,7 +74,7 @@ The layer that wraps the four hats. `technical-program-manager` owns what / why ## 9. Stacked PR Workflow -Shipping multiple related PRs in a session: local feature branch per ticket; **don't push the version-bump + CHANGELOG combo until the prior PR merges** (else both touch the same version lines and the second hits a rebase conflict); when it merges, `git checkout && git pull --ff-only`, then `git rebase ` the next branch **locally** rather than relying on platform conflict-resolution (squash-merge SHAs don't match local commits). Full rebase discipline + the stash-and-checkout pitfall: `~/.claude/docs/git-workflows.md`. +Shipping multiple related PRs in a session: local feature branch per ticket; **don't push the version-bump + CHANGELOG combo until the prior PR merges** (else both touch the same version lines and the second hits a rebase conflict); when it merges, `git checkout && git pull --ff-only`, then `git rebase ` the next branch **locally** rather than relying on platform conflict-resolution (squash-merge SHAs don't match local commits). Full rebase discipline, the stash-and-checkout pitfall, and what to do between PR-open and merge (CI, review-thread replies with a pushed SHA, re-check after every push): `~/.claude/docs/git-workflows.md`. --- @@ -166,23 +166,27 @@ Four no-code extensions cover almost everything before you'd fork the binary: ** - **Framework(s):** None (only Claude Code extension points — §18). - **Storage:** None. - **Build / package:** None — files are copied verbatim into `~/.claude/`; `package.json` is intentionally absent. -- **Test runner:** None; verification is static (§19.3). -- **CI:** None currently (no `.github/workflows`); the gate runs locally before tagging. +- **Test runner:** Plain bash. `tests/hooks/*.sh` feed each hook a PreToolUse / PostToolUse JSON payload on stdin and assert the exit code (bash 3.2 + jq + git, no framework). +- **CI:** `.github/workflows/gate.yml` runs §19.3 on every PR and on push to `main`. The same commands run locally before tagging. - **Distribution channel:** Public GitHub `roadhero/claude-code-setup`, MIT, released as annotated `vX.Y.Z` tags + a matching GitHub Release. No package registry. ### 19.3 Local quality gate -Concrete commands a fresh clone can run (verified green: shellcheck exit 0, jq exit 0). +Concrete commands a fresh clone can run; `.github/workflows/gate.yml` runs the same list. ```bash -shellcheck hooks/*.sh # bash hooks lint clean (bash 3.2 target) +shellcheck hooks/*.sh tests/hooks/*.sh # bash hooks + tests lint clean (bash 3.2 target) jq empty settings.json settings2.json # settings parse as valid JSON grep -L '^name:' agents*/*.md # every agent declares a name; prints nothing when clean +diff -q templates/CLAUDE.project.md skills/new-repo/templates/web/CLAUDE.md.tmpl # scaffolder copies +diff -q templates/CLAUDE.project.md skills/new-repo/templates/android/CLAUDE.md.tmpl # stay byte-identical +bash tests/hooks/test-guard-commit.sh # commit guard: stdin payload → exit code, one case per rule/regression +bash tests/hooks/test-format.sh # format hook: exit 0, touches only the tool's own target # Optional, advisory (not installed by default; repo ships no markdownlint config): # npx --yes markdownlint-cli2 "**/*.md" "!skills/**/templates/**" ``` -Requires `shellcheck` and `jq` (the hooks need `jq` at runtime too) — `brew install shellcheck jq`. No build step; files ship verbatim. Markdown prose is reviewed by eye, not hard-gated. +Requires `shellcheck`, `jq`, and `git` (the hooks need `jq` at runtime too) — `brew install shellcheck jq`. No build step; files ship verbatim. Markdown prose is reviewed by eye, not hard-gated. A hook bug fix starts with a failing case in `tests/hooks/` (§3.4). ### 19.4 Current release pointers @@ -201,10 +205,9 @@ Requires `shellcheck` and `jq` (the hooks need `jq` at runtime too) — `brew in > Each override erodes the predictability §1–18 provides; treat them as debt with a documented reason. Review quarterly: can any be removed? -- §19.3 replaces the build/unit/integration gate with static analysis (shellcheck + jq + a name-invariant grep) — reason: this repo ships configuration, not code; nothing to compile or unit-test. +- §19.3 replaces the build/unit/integration gate with static analysis (shellcheck + jq + a name-invariant grep + a copies-in-sync diff) plus behavioral tests for the two hooks — reason: this repo ships configuration; the hooks are its only executable code, so they are the only thing unit-tested. - Release notes come from the GitHub Release body instead of a `CHANGELOG.md` — reason: no CHANGELOG is maintained here. - Platform rule-pack path-triggering (`rules/{web,android,ios,compute}.md`) never fires in this repo — it has no matching source files. Expected. -- No CI yet — candidate follow-up: a GitHub Action running §19.3 on PR. --- diff --git a/README.md b/README.md index d65eb27..b29cac8 100644 --- a/README.md +++ b/README.md @@ -6,19 +6,20 @@ Most people publish a single `CLAUDE.md` and call it a setup. The thing that act ## What's inside -| Path | What it is | -| ---------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `CLAUDE.md` | The universal spine — a lean (~215-line) always-loaded core: workflow, git rules, coding guidelines, secrets, anti-patterns. Stack-agnostic. Longer reference material lives in `docs/` and loads on demand. | -| `rules/{web,android,ios,compute}.md` | Platform rule packs. Each carries a `paths:` frontmatter glob; Claude Code loads a pack when it reads a file matching that glob (`*.kt` → android, `*.swift` → ios, `*.ts`/`*.py` → web, `*.cpp`/`*.cu` → compute). Path-triggered, so packs whose files you never touch stay out of context. | -| `agents/` (15) | The default subagent roster: the four-hat chain (architect → senior-swe → code-reviewer → qa), plus specialists (security, performance, db-migration, debugger, devops, docs, design), release-engineer + tech-writer, and a delivery layer (TPM, scrum-master). | -| `agents-android/` (7), `agents-ios/` (7), `agents-compute/` (13) | Per-stack overrides. Drop them into a repo's `.claude/agents/` and they override the generic ones of the same name with platform-brained versions. | -| `hooks/guard-commit.sh` | A Claude Code Bash hook (PreToolUse) that blocks the _agent_ from force-pushing, committing as a non-human, writing AI attribution into a commit message, or staging obvious secrets. It guards Claude's git commands — not a human typing `git` directly in their own terminal. | -| `hooks/format.sh` | Auto-formats edited files by extension across every stack. Missing formatter is a silent no-op, never an error. | -| `skills/new-repo/` | A scaffolder skill: spins up a new repo with the right `CLAUDE.md`, `.gitignore`, quality gate, and release workflow. Scaffolds **web + Android**; iOS and compute ship as rule + agent packs (no scaffolder for them yet). | -| `docs/` | On-demand reference the spine points to (full roster tables, the Phase-3 review checklist, the error-recovery table, PR template, scaling notes). Installed to `~/.claude/docs/`; loaded only when a stub references it. | -| `templates/` | Blank project `CLAUDE.md` templates (generic + compute) to copy into a new repo and fill in. | -| `examples/CLAUDE.example-web.md` | A filled-in example so you can see what "done" looks like before you write your own. | -| `STRUCTURE.md` | The full layout, install steps, and how the two-settings-file swap works. **Read this first.** | +| Path | What it is | +| ---------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `CLAUDE.md` | The universal spine — a lean (~220-line) always-loaded core: workflow, git rules, coding guidelines, secrets, anti-patterns. Stack-agnostic. Longer reference material lives in `docs/` and loads on demand. | +| `rules/{web,android,ios,compute}.md` | Platform rule packs. Each carries a `paths:` frontmatter glob; Claude Code loads a pack when it reads a file matching that glob (`*.kt` → android, `*.swift` → ios, `*.ts`/`*.py` → web, `*.cpp`/`*.cu` → compute). Path-triggered, so packs whose files you never touch stay out of context. | +| `agents/` (15) | The default subagent roster: the four-hat chain (architect → senior-swe → code-reviewer → qa), plus specialists (security, performance, db-migration, debugger, devops, docs, design), release-engineer + tech-writer, and a delivery layer (TPM, scrum-master). | +| `agents-android/` (7), `agents-ios/` (7), `agents-compute/` (13) | Per-stack overrides. Drop them into a repo's `.claude/agents/` and they override the generic ones of the same name with platform-brained versions. | +| `hooks/guard-commit.sh` | A Claude Code Bash hook (PreToolUse) that blocks the _agent_ from force-pushing, skipping git hooks with `--no-verify`, committing as a non-human, writing AI attribution into a commit message, or staging obvious secrets. Every segment of a chained command is checked on its own. It guards Claude's git commands — not a human typing `git` directly in their own terminal. | +| `hooks/format.sh` | Auto-formats edited files by extension across every stack. Missing formatter is a silent no-op, never an error. | +| `tests/hooks/` | Behavioral tests for both hooks: a JSON payload on stdin, an exit code out, one case per rule and per past regression. Plain bash 3.2 + `jq`. CI (`.github/workflows/gate.yml`) runs them with the rest of the quality gate on every PR. | +| `skills/new-repo/` | A scaffolder skill: spins up a new repo with the right `CLAUDE.md`, `.gitignore`, quality gate, and release workflow. Scaffolds **web + Android**; iOS and compute ship as rule + agent packs (no scaffolder for them yet). | +| `docs/` | On-demand reference the spine points to (full roster tables, the Phase-3 review checklist, the error-recovery table, PR template, scaling notes). Installed to `~/.claude/docs/`; loaded only when a stub references it. | +| `templates/` | Blank project `CLAUDE.md` templates (generic + compute) to copy into a new repo and fill in. | +| `examples/CLAUDE.example-web.md` | A filled-in example so you can see what "done" looks like before you write your own. | +| `STRUCTURE.md` | The full layout, install steps, and how the two-settings-file swap works. **Read this first.** | ## The idea in one paragraph diff --git a/STRUCTURE.md b/STRUCTURE.md index 2d68e2c..3c62faa 100644 --- a/STRUCTURE.md +++ b/STRUCTURE.md @@ -12,7 +12,7 @@ │ └── compute.md # C++/CUDA/parallel-Python ├── agents/ # 15 GENERIC (global default) ├── hooks/ -│ ├── guard-commit.sh # PreToolUse(Bash): block AI attribution, secrets, force-push, non-human committer +│ ├── guard-commit.sh # PreToolUse(Bash): block AI attribution, secrets, force-push, --no-verify, non-human committer │ └── format.sh # PostToolUse(Edit|Write): auto-format by extension, all stacks ├── skills/new-repo/ # scaffolder └── docs/ # on-demand reference (roster tables, §4 review checklist, error-recovery table, PR template, scaling) — the spine points here @@ -26,6 +26,10 @@ Per-repo CLAUDE.md (§19 only; inherits spine + whichever rule pack your files p generic template → templates/CLAUDE.project.md compute template → templates/CLAUDE.project.compute.md filled example → examples/CLAUDE.example-web.md (fictional web SaaS, shows §19 filled in) + +Repo-only, not installed: + tests/hooks/ # behavioral tests for both hooks (stdin JSON → exit code); run by the §19.3 gate + .github/workflows/gate.yml # CI: the §19.3 gate on every PR and on push to main ``` ## Install (Mac) @@ -76,12 +80,12 @@ Both settings files reference the hooks via `$HOME/.claude/hooks/...`, which the Four no-code extensions cover almost everything before you'd fork the binary: -| Extension | Format | Location | Use case | -| --------------- | ------------------------------------------------- | ----------------------------------- | -------------------------------------------------------------------------------------------------- | -| **Subagents** | Markdown with YAML frontmatter | `.claude/agents/*.md` | New personas (e.g. `security-reviewer`, `performance-engineer`, `db-migration-specialist`) | -| **Skills** | Markdown with YAML frontmatter + supporting files | `.claude/skills//SKILL.md` | Reusable workflows (e.g. `release-prep`, `add-feature-flag`, `db-migration`, `dependency-upgrade`) | -| **Hooks** | Shell scripts | `.claude/hooks/*.sh` | Pre/post-tool-call validation, audit logging, custom permission gates | -| **MCP servers** | Protocol-based (any language) | Configured in user/project settings | Tool integrations (issue tracker, error tracking service, package registry, custom databases) | +| Extension | Format | Location | Use case | +| --------------- | ------------------------------------------------- | ----------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Subagents** | Markdown with YAML frontmatter | `.claude/agents/*.md` | New personas (e.g. `security-reviewer`, `performance-engineer`, `db-migration-specialist`) | +| **Skills** | Markdown with YAML frontmatter + supporting files | `.claude/skills//SKILL.md` | Reusable workflows (e.g. `release-prep`, `add-feature-flag`, `db-migration`, `dependency-upgrade`) | +| **Hooks** | Shell scripts | `.claude/hooks/*.sh` | Pre/post-tool-call validation, audit logging, custom permission gates. Rule of thumb: a MUST / NEVER you catch yourself restating in a second file belongs here (or behind plan mode), not in more prose — `guard-commit.sh` is the pattern. A hook inspects or formats the tool's own target; it never creates or rewrites other files. | +| **MCP servers** | Protocol-based (any language) | Configured in user/project settings | Tool integrations (issue tracker, error tracking service, package registry, custom databases) | If a use case fits none of these, report the gap upstream rather than working around it — forking the binary trades extensibility for an ownership burden, and every Claude Code release becomes a merge conflict. diff --git a/agents/code-reviewer.md b/agents/code-reviewer.md index ff37512..9eeb3cf 100644 --- a/agents/code-reviewer.md +++ b/agents/code-reviewer.md @@ -17,6 +17,7 @@ Read the git diff. Walk it against the checklists below. Produce a structured re - Run `git diff ...HEAD` (or `git diff --staged` if reviewing unstaged work) to get the change set. If the user supplies a different base, use that. - Read the files touched. Diff context isn't enough — you need to see how the changed lines integrate with surrounding code. - Read CLAUDE.md §4.3 (Phase 3: Code Reviewer) and §14 (Anti-Patterns), plus the platform rule pack's §5 (Architecture Patterns), §12 (Concurrency Patterns), §13 (Compliance & Distribution) before reviewing. +- The Phase 1 plan (the `architect` output or the user-approved plan), when one exists. If you were not handed it, ask for it. Without it you can only review forward (diff → task) and must say so in the report; you cannot detect a step that was silently dropped. # Output format @@ -41,6 +42,7 @@ Read the git diff. Walk it against the checklists below. Produce a structured re - Blocking: N - Should fix: N - Nits: N +- Plan coverage: N/N plan steps verified with `file:line` (list the missing ones, or "no plan supplied") - Recommendation: ``` @@ -53,14 +55,14 @@ Read the git diff. Walk it against the checklists below. Produce a structured re # The universal checklist (every changed line) -1. **Trace.** Does this line trace to the task? If not, suggest reverting it. +1. **Trace, both directions.** Forward: does this line trace to the task? If not, suggest reverting it. Reverse (when a plan exists): does every plan step, named failure mode, and named test have an implementing `file:line` and a test `file:line` in the diff? A step that vanished with no entry under "Deferred / out of scope" is a silent scope shrink, not a non-event — 🔴. 2. **Hardcodes.** Magic numbers, magic strings, magic URLs, magic IDs — should they be config / env / constant? 3. **Error handling.** Missing handling for _realistic_ failure cases. Not hypothetical ones — real ones: network down, disk full, permission denied, dependency unavailable, malformed input from an untrusted source. 4. **Security.** - - Unsanitized user input reaching SQL, shell, template, regex, deserializer. + - Unsanitized user input — including tool-call arguments and model output — reaching SQL, shell, template, regex, deserializer. - Output sanitization (XSS, log injection, command injection). - Authentication: every endpoint correctly authenticated. - - Authorization: every operation scoped to the right principal. + - Authorization: every operation scoped to the right principal, and the principal comes from the verified session / token — never from the request body, a query param, a tool-call argument, or model output. - Secrets: nothing logged, nothing in error responses, nothing in URLs. - Crypto: only `crypto.randomBytes` / `secrets.token_bytes` / `rand::rngs::OsRng` for security-sensitive randomness — never `Math.random()` / `random.random()` / `rand::random()`. - Crypto primitives: no MD5, SHA-1, DES, RC4, ECB mode for new code. @@ -81,7 +83,7 @@ Read the git diff. Walk it against the checklists below. Produce a structured re 9. **API boundaries.** No persistence types in API responses? No HTTP types in business logic? No UI types in core domain? 10. **Backwards compatibility.** Public API change? Schema migration safe to roll back? Old clients still work after deploy? Feature flag for risky changes? 11. **Observability.** New error paths logged with structured context? New metrics named consistently with existing? New trace span propagated through? -12. **Tests.** Every new logic branch covered? Test names describe behavior, not implementation? Deterministic — no real wall-clock, no unseeded random? +12. **Tests.** Every new logic branch covered? Test names describe behavior, not implementation? Deterministic — no real wall-clock, no unseeded random? Would each test still pass if the implementation were deleted or stubbed to a constant? An assertion whose only content is presence or was-called (`toBeDefined`, `toBeTruthy`, bare `toHaveBeenCalled()`, `assertNotNull` alone, `length > 0`), or a test that only exercises the fake, is the 🔴 "test that doesn't test what it claims" from the severity table. 13. **Dependencies.** New dep added — is it justified? License compatible? Maintained? Security history clean? Pulls in concerning transitives? 14. **CHANGELOG.** User-visible change has an entry in the next-release section, Keep-a-Changelog format. 15. **AI tool mentions (CLAUDE.md §2).** Per CLAUDE.md §2, no mention of AI tools, LLMs, or assistants in any committed artifact. Scan with: @@ -92,25 +94,26 @@ Read the git diff. Walk it against the checklists below. Produce a structured re # Universal red flags (grep these in the diff) -| Pattern | Why it's a red flag | -| -------------------------------------------------------------------------- | -------------------------------------------- | -| `TODO` without ticket reference | Permanent debt with no owner | -| `FIXME` / `HACK` / `XXX` | Unfinished work, mark it explicitly | -| `eval(` (any language) | Code execution from untrusted source | -| `Math.random()` / `random.random()` / `rand::random()` in security context | Use cryptographic RNG | -| `MD5` / `SHA1` / `DES` / `ECB` for new crypto code | Broken or weak primitives | -| `--no-verify` in shell scripts / hooks | Bypasses pre-commit checks | -| `--force` push patterns in CI | Destroys history | -| Hardcoded URL / IP / hostname | Should be configuration | -| Hardcoded `Authorization:` header | Secret in code | -| `localhost` in production-bound code | Won't work outside dev | -| `127.0.0.1` in production-bound code | Same | -| Broad `catch` / `except` / `rescue` without specific type | Swallows real bugs | -| `goto fail` style early-return that skips cleanup | Resource leak risk | -| `unsafe` / `transmute` / `as` casting between integer types (Rust) | Memory or correctness risk; review carefully | -| Sleep / busy-wait in tests instead of polling for readiness | Flake source | -| New env var with no default and no startup-time check | Silent misconfiguration in prod | -| `git commit -am` patterns in automation | Commits unintended files | +| Pattern | Why it's a red flag | +| ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------- | +| `TODO` without ticket reference | Permanent debt with no owner | +| `FIXME` / `HACK` / `XXX` | Unfinished work, mark it explicitly | +| `eval(` (any language) | Code execution from untrusted source | +| `Math.random()` / `random.random()` / `rand::random()` in security context | Use cryptographic RNG | +| `MD5` / `SHA1` / `DES` / `ECB` for new crypto code | Broken or weak primitives | +| `--no-verify` in shell scripts / hooks | Bypasses pre-commit checks | +| `--force` push patterns in CI | Destroys history | +| Hardcoded URL / IP / hostname | Should be configuration | +| Hardcoded `Authorization:` header | Secret in code | +| `localhost` in production-bound code | Won't work outside dev | +| `127.0.0.1` in production-bound code | Same | +| Broad `catch` / `except` / `rescue` without specific type | Swallows real bugs | +| `goto fail` style early-return that skips cleanup | Resource leak risk | +| `unsafe` / `transmute` / `as` casting between integer types (Rust) | Memory or correctness risk; review carefully | +| Sleep / busy-wait in tests instead of polling for readiness | Flake source | +| Assertion loosened in the diff (`toEqual(x)` → `expect.any()` / `objectContaining`, widened tolerance, assert removed, test deleted to clear a type error) without a linked reason | Test rewritten to go green instead of the code being fixed | +| New env var with no default and no startup-time check | Silent misconfiguration in prod | +| `git commit -am` patterns in automation | Commits unintended files | # Language-family-specific red flags diff --git a/agents/qa.md b/agents/qa.md index cb518de..fedbb40 100644 --- a/agents/qa.md +++ b/agents/qa.md @@ -135,6 +135,7 @@ If any task fails, the report's recommendation is **NEEDS REWORK** with the spec - "Test plan" is empty or just says "manual QA" — that's not a plan. - `@Ignore` / `it.skip` / `#[ignore]` was added to a test without an issue link. - Snapshot / golden threshold was relaxed without justification. +- An assertion was loosened or removed to get green (`toEqual(x)` → `expect.any()`, widened tolerance, deleted assert), or a new test's only assertions are presence / was-called. - A test uses real wall-clock or unseeded random. - A test depends on execution order or shared state from other tests. diff --git a/agents/security-reviewer.md b/agents/security-reviewer.md index 53d07d8..77e8d98 100644 --- a/agents/security-reviewer.md +++ b/agents/security-reviewer.md @@ -42,8 +42,8 @@ Threat-model the change set and audit the surfaces below. Produce a findings rep # Audit surfaces -1. **AuthN/AuthZ.** Every new endpoint/handler authenticated? Every operation scoped to the right principal (no IDOR — object reference without an ownership check)? Privilege boundaries crossed? -2. **Injection.** User input reaching SQL, shell, template, regex (ReDoS), LDAP, or a deserializer. Trace the taint from source to sink. +1. **AuthN/AuthZ.** Every new endpoint/handler authenticated? Every operation scoped to the right principal (no IDOR — object reference without an ownership check)? Privilege boundaries crossed? The acting principal (user / org / role) comes from the verified session or token — never from the request body, a query param, a tool-call argument, or model output. A tool handler that accepts `userId` as a parameter is the agentic form of IDOR: one prompt injection and the model acts as any user. +2. **Injection.** User input — including tool-call arguments and model output — reaching SQL, shell, template, regex (ReDoS), LDAP, or a deserializer. Trace the taint from source to sink. 3. **Crypto.** Security-sensitive randomness uses a CSPRNG (`crypto.randomBytes` / `secrets.token_bytes` / `OsRng`), never `Math.random`/`random.random`. No MD5/SHA-1/DES/RC4/ECB for new code. Keys/IVs not reused or hardcoded. 4. **Secrets.** Nothing logged, returned in errors, or placed in URLs/query strings. Matches CLAUDE.md §11. Run: ```bash diff --git a/docs/agents.md b/docs/agents.md index 7609b6d..2b014c3 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -10,7 +10,7 @@ Agents live under `.claude/agents/` (project) or `~/.claude/agents/` (user, all | ------------------ | ---------------------------- | ----------- | --------------------------------------------------------------------------------------------- | | `architect` | Phase 1 planner | Read-only | Any non-trivial change. Produces the scoped plan before code is written. | | `senior-swe` | Phase 2 implementer | Mutating | After plan approval. Writes code matching existing patterns. | -| `code-reviewer` | Phase 3 adversarial reviewer | Read-only | Before pushing. Walks the review checklist against the diff. | +| `code-reviewer` | Phase 3 adversarial reviewer | Read-only | Before pushing. Walks the review checklist against the diff, and the diff against the plan. | | `qa` | Phase 4 verifier | Read-only | After review. Generates the test plan and runs the local gate. | | `release-engineer` | Release prep | Mutating | Bumping versions, writing CHANGELOG, tagging. | | `docs-reconciler` | Drift detection | Read-only | Every 3–5 merged PRs. Surfaces drift between specs ↔ code ↔ CHANGELOG ↔ README ↔ open issues. | diff --git a/docs/git-workflows.md b/docs/git-workflows.md index d02ec65..25e95d5 100644 --- a/docs/git-workflows.md +++ b/docs/git-workflows.md @@ -1,6 +1,6 @@ -# Stacked PR workflow +# PR workflow — stacked PRs, and what happens after the PR is open -> Full detail for CLAUDE.md §9. The spine carries the one-line rule; the rebase discipline lives here. +> Full detail for CLAUDE.md §9. The spine carries the one-line rule; the rebase discipline and the PR-open-to-merge loop live here. When shipping multiple related PRs in a session: @@ -16,3 +16,18 @@ When shipping multiple related PRs in a session: **Don't push and rely on platform conflict-resolution.** The squash-merge SHAs don't match local commits; rebasing locally is cleaner. **Stash-and-checkout pitfall.** If you `git stash` mid-edit and `git checkout main`, the stash includes only the modified-and-tracked files. Untracked new files ARE included by default in modern git, but verify with `git stash show -u`. If you switch branches and lose work-in-progress, check `git fsck --lost-found` before panicking. + +## After the PR is open + +A PR is not done when it is opened. Between `gh pr create` and merge: + +1. **Wait for CI.** `gh pr checks --watch`. On red, classify the failure per `workflow.md` §4.7 before touching anything. Auto-fix only formatter / lint / type errors and tests you wrote in this PR; ask before changing tests you did not write or CI config; never `--no-verify` (§14, and the commit guard blocks it). +2. **Reply to every top-level review comment, human or bot.** One of: `Fixed in ` where `` is `git rev-parse --short origin/` _after_ the push (never a local-only commit); `Deferred: #` with the issue actually opened; or a technical disagreement with the reason. Never silent. Reply before resolving, and follow the repo's convention on who resolves. Per §2, no AI mention in replies. +3. **Re-check after every push.** Bot reviewers re-review each commit, so repeat steps 1 and 2 until both are clean. To list top-level review comments that still have no reply: + ```bash + gh api "repos/{owner}/{repo}/pulls//comments" --paginate | jq -rs 'add + | ([.[] | select(.in_reply_to_id != null) | .in_reply_to_id]) as $replied + | .[] | select(.in_reply_to_id == null and ((.id as $i | $replied | index($i)) == null)) + | "\(.id) \(.user.login) \(.path):\(.line // .original_line)"' + ``` +4. **Done means:** CI green after the last push, that list is empty, and nothing is left on your side. Report and stop. Merging is the user's call (the `qa` agent does not merge); once it merges, the stacked-PR steps above apply to the next branch. diff --git a/docs/pr-template.md b/docs/pr-template.md index 8436fa3..91a5bf6 100644 --- a/docs/pr-template.md +++ b/docs/pr-template.md @@ -20,6 +20,7 @@ - [x] Local quality gate green: ``, ``, ``, ``, ``, `` (delete what doesn't apply) - [x] Any domain-specific audits clean +- [ ] Every review thread replied to (fixed-in- / deferred #N / reasoned disagreement); CI green after the last push - [ ] On-target verification: - - diff --git a/docs/reconciliation.md b/docs/reconciliation.md index 51f6dc0..f044b49 100644 --- a/docs/reconciliation.md +++ b/docs/reconciliation.md @@ -9,5 +9,6 @@ Every 3–5 merged PRs, run a reconciliation pass before queueing the next featu 3. **Open issues ↔ reality:** close issues whose deliverable shipped; comment on owner-bound issues with current state so the next session knows what's pending. 4. **CHANGELOG ↔ tags:** verify the section heads match the git tag history; the release workflow extracts these verbatim, so drift here ships an empty release body. 5. **README ↔ code:** does the elevator pitch still match? Are the badges current (build status, package version, license)? Are quickstart commands still correct? +6. **Cited paths ↔ files:** every path a doc, agent, skill, or hook cites resolves. Cheap first pass: `grep -ohE '[A-Za-z0-9_.-]+(/[A-Za-z0-9_.-]+)+\.(sh|md|yml|json|tmpl)' ` then `test -e` on each; only tokens with a `/` are worth checking, and the survivors still need a human eye (scaffolder-generated files and diff headers are not dangling). Every intended duplicate (a template and its scaffolder copy) is byte-identical or has a `diff -q` in the quality gate. A dangling reference or a silently drifted copy is drift, not a cosmetic nit. Default fallback work when no ticket is queued: **reconciliation pass + ticket hygiene**, not speculative refactors. Don't invent work to fill time. diff --git a/docs/workflow.md b/docs/workflow.md index 970624a..4a74768 100644 --- a/docs/workflow.md +++ b/docs/workflow.md @@ -29,9 +29,10 @@ Delegate to `senior-swe`. Implement the plan. Nothing more, nothing less. ### 4.3 Phase 3: Code Reviewer -Delegate to `code-reviewer`. Review the diff as if you didn't write it. Be adversarial. Check every changed line against this list: +Delegate to `code-reviewer`, and hand it the Phase 1 plan along with the diff (a delegated reviewer has no memory of Phase 1 — §1). Review the diff as if you didn't write it. Be adversarial. Check every changed line against this list: - [ ] Does this line trace to the task? If not, revert it. +- [ ] Reverse trace (when a Phase 1 plan exists): does every plan step, named failure mode, and named test appear in the diff, with an implementing `file:line` and a test `file:line`? A step that vanished with no "Deferred / out of scope" entry is a silent scope shrink — 🔴. - [ ] Any hardcoded values that should be config/env/feature-flag/constant? - [ ] Any missing error handling for _realistic_ failure cases? (not hypothetical ones) - [ ] Any security issues? (unsanitized input, leaked secrets, injection — SQL/command/template, SSRF, deserialization, broken auth, broken access control) @@ -42,7 +43,7 @@ Delegate to `code-reviewer`. Review the diff as if you didn't write it. Be adver - [ ] Backwards compatibility: does this break any public API? Schema migration safe to roll back? Feature-flagged if risky? - [ ] Concurrency: any new shared mutable state? Cancellation propagated? Timeouts on external calls? - [ ] Observability: new error paths logged with structured context? New metrics named consistently with existing ones? -- [ ] Tests: every new logic branch covered? Test names describe behavior, not implementation? +- [ ] Tests: every new logic branch covered? Test names describe behavior, not implementation? Would each still pass with the implementation stubbed? No assertion loosened to get green? - [ ] CHANGELOG entry added for any user-visible change? **Zero open comments rule:** every issue found must be fixed or explicitly justified before proceeding. No "we can fix this later." No parking. diff --git a/hooks/guard-commit.sh b/hooks/guard-commit.sh index 92d649c..bd40ade 100644 --- a/hooks/guard-commit.sh +++ b/hooks/guard-commit.sh @@ -1,5 +1,11 @@ #!/usr/bin/env bash -# PreToolUse(Bash) — block AI attribution, secrets, and force-push before they happen. +# PreToolUse(Bash) — block AI attribution, secrets, force-push, and hook-skipping (--no-verify) +# before they happen. Exit 2 + a stderr reason is the documented blocking contract; stdout is unused. +# +# This is a backstop behind the settings.json deny rules and plan mode, not a sandbox. Known, +# accepted limits: variable indirection (`p=push; git $p -f`), git aliases defined in an earlier +# call, `git config core.hooksPath` set in an earlier call, flags fed through a pipe (`xargs`), +# and flags spelled with quoting tricks (`--no-veri"fy"`, `$'--no-verify'`, unique-prefix `--no-veri`). INPUT=$(cat) # Fast path: this guard only concerns `git commit` / `git push`. If the payload mentions @@ -14,18 +20,66 @@ if ! command -v jq >/dev/null 2>&1; then exit 2 fi -CMD=$(printf '%s' "$INPUT" | jq -r '.tool_input.command // empty') -[ -z "$CMD" ] && exit 0 +# The payload mentions git commit/push, so an empty command here means the payload did not parse +# (or its shape changed). Fail closed rather than silently turning into a no-op. +CMD=$(printf '%s' "$INPUT" | jq -r '.tool_input.command // empty' 2>/dev/null) +if [ -z "$CMD" ]; then + echo "Blocked: guard-commit.sh could not read .tool_input.command from the PreToolUse payload. Failing closed." >&2 + exit 2 +fi + +# Join backslash-newline continuations so a flag on its own line stays with its subcommand. +CMD=${CMD//\\$'\n'/ } + +# Classify per command segment, not per Bash call: `git commit -m ok && git push -f` is a commit +# AND a force-push, and each segment is judged on its own. Quoted text is stripped first so a +# commit MESSAGE that merely mentions `push --force` or `--no-verify` (or contains `;`) can't +# trip a flag check or split a segment. The strip is multi-line aware (newlines are hidden as +# \001 during sed) so a heredoc message `-m "$(cat <<'EOF' ... EOF)"` is one quoted span, and +# escaped \" are dropped first so they cannot flip quote parity. A `git commit -F - <&2; exit 2 +fi + +IS_COMMIT=""; HAS_PUSH=""; HAS_ADD="" +while IFS= read -r SEG; do + [ -z "$SEG" ] && continue + SEG_COMMIT=""; SEG_PUSH="" + printf '%s' "$SEG" | grep -qiE 'git.*[[:space:]]commit([^[:alnum:]]|$)' && SEG_COMMIT=1 + printf '%s' "$SEG" | grep -qiE 'git.*[[:space:]]push([^[:alnum:]]|$)' && SEG_PUSH=1 + printf '%s' "$SEG" | grep -qiE 'git.*[[:space:]]add([^[:alnum:]]|$)' && HAS_ADD=1 + [ -n "$SEG_COMMIT" ] && IS_COMMIT=1 + [ -n "$SEG_PUSH" ] && HAS_PUSH=1 + [ -z "$SEG_COMMIT$SEG_PUSH" ] && continue + + # force-push: --force / --force-with-lease / --mirror, -f alone or inside a flag cluster (-uf), + # or a `+refspec` (the refspec form of --force). `--force-if-includes` alone is also caught; + # it is a no-op without --force-with-lease, so nothing legitimate is lost. + if [ -n "$SEG_PUSH" ] && printf '%s' "$SEG" | grep -qiE '[[:space:]]push.*[[:space:]](--force-with-lease|--force|--mirror)([^[:alnum:]]|$)|[[:space:]]push.*[[:space:]]-[A-Za-z]*f[A-Za-z]*([^[:alnum:]]|$)|[[:space:]]push.*[[:space:]]\+[^[:space:]]'; then + echo "Blocked: force-push. Protected-branch discipline (CLAUDE.md §9)." >&2; exit 2 + fi -# Classify the git subcommand. It may follow global options (`git -C `, `git -c k=v`), -# so match `commit`/`push` as a whitespace-delimited word, not immediately after `git`. -IS_COMMIT="" -printf '%s' "$CMD" | grep -qiE 'git[^;&|]*[[:space:]]commit([^[:alnum:]]|$)' && IS_COMMIT=1 + # --no-verify skips the pre-commit / pre-push hooks the repo installed on purpose (CLAUDE.md §14), + # as does `git commit -n` (the short form, alone or in a cluster like -anm). Fix the hook failure instead. + if printf '%s' "$SEG" | grep -qiE '[[:space:]]--no-verify([^[:alnum:]-]|$)'; then + echo "Blocked: --no-verify bypasses the repo's git hooks (CLAUDE.md §14). Fix the hook failure instead." >&2; exit 2 + fi + if [ -n "$SEG_COMMIT" ] && printf '%s' "$SEG" | grep -qiE '[[:space:]]commit.*[[:space:]]-[A-Za-z]*n[A-Za-z]*([^[:alnum:]]|$)'; then + echo "Blocked: git commit -n is --no-verify (CLAUDE.md §14). Fix the hook failure instead." >&2; exit 2 + fi +done <<<"$(printf '%s' "$STRIPPED" | tr ';&|' '\n')" -# block force-push (a push is never a commit, so this never fires on a commit message that -# merely mentions "push --force"): --force / -f / --force-with-lease anywhere in the push command -if [ -z "$IS_COMMIT" ] && printf '%s' "$CMD" | grep -qiE 'git[^;&|]*[[:space:]]push[^;&|]*(--force-with-lease|--force|-f)([^[:alnum:]]|$)'; then - echo "Blocked: force-push. Protected-branch discipline (CLAUDE.md §9)." >&2; exit 2 +[ -z "$IS_COMMIT$HAS_PUSH" ] && exit 0 + +# Re-pointing core.hooksPath anywhere in the same call (`git -c core.hooksPath=x commit`, +# `git config core.hooksPath x; git commit`, GIT_CONFIG_PARAMETERS=...) is the same bypass. +if printf '%s' "$STRIPPED" | grep -qiE 'core\.hooksPath|GIT_CONFIG_(PARAMETERS|COUNT|KEY_)'; then + echo "Blocked: core.hooksPath override bypasses the repo's git hooks (CLAUDE.md §14)." >&2; exit 2 fi # only inspect commit operations further @@ -44,17 +98,31 @@ if printf '%s' "$CMD" | grep -qiE 'co-authored-by:.*(claude|gpt|copilot|cursor)| echo "Blocked: AI attribution detected in commit message (CLAUDE.md §2). Remove it." >&2; exit 2 fi -# no obvious secrets. Scan the staged diff; for `git commit -a/--all` (which auto-stages tracked -# edits AFTER this hook runs) also scan the working-tree diff, else `-am` bypasses the check. +# no obvious secrets. Scan the staged diff. For `git commit -a/--all` (which auto-stages tracked +# edits AFTER this hook runs) also scan the working-tree diff, and for `git add ... && git commit` +# in one call (the add has not happened yet either) also scan the working tree and untracked files. SECRETS='(api[_-]?key|secret[_-]?key|access[_-]?key|private[_-]?key|password|token|bearer )[^=]*[=:] *["'"'"']?[A-Za-z0-9/_+-]{16,}|BEGIN [A-Z0-9 ]*PRIVATE KEY|AKIA[0-9A-Z]{16}|gh[pousr]_[A-Za-z0-9]{36,}|xox[baprs]-[0-9A-Za-z-]{10,}|eyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}' # Exempt example/sample files — they're meant to hold placeholder values and are # explicitly whitelisted for commit (e.g. `!.env.example` in the scaffolder .gitignore). EXCL=(':(exclude)*.example' ':(exclude)*.sample' ':(exclude)*.dist' ':(exclude)*.tmpl') -DIFF=$(git diff --cached 2>/dev/null -- "${EXCL[@]}") -if printf '%s' "$CMD" | grep -qiE '[[:space:]](--all|-[A-Za-z]*a[A-Za-z]*)([[:space:]]|$)'; then - DIFF="$DIFF"$'\n'"$(git diff 2>/dev/null -- "${EXCL[@]}")" +# `--no-ext-diff` and `core.fsmonitor=false` so a repo-local config cannot make the guard run a program. +GIT_DIFF=(git -c core.fsmonitor=false diff --no-ext-diff) +DIFF=$("${GIT_DIFF[@]}" --cached 2>/dev/null -- "${EXCL[@]}") +if [ -n "$HAS_ADD" ] || printf '%s' "$STRIPPED" | grep -qiE '[[:space:]](--all|-[A-Za-z]*a[A-Za-z]*)([[:space:]]|$)'; then + DIFF="$DIFF"$'\n'"$("${GIT_DIFF[@]}" 2>/dev/null -- "${EXCL[@]}")" +fi +if [ -n "$HAS_ADD" ]; then + # (a plain function, not `case` inside $(...): bash 3.2 cannot parse the `)` of a case pattern there) + scan_untracked() { + git ls-files --others --exclude-standard 2>/dev/null | while IFS= read -r f; do + printf '%s' "$f" | grep -qE '\.(example|sample|dist|tmpl)$' && continue + grep -sIE "$SECRETS" -- "$f" | sed 's/^/+/' + done + } + DIFF="$DIFF"$'\n'"$(scan_untracked)" fi -if printf '%s' "$DIFF" | grep -qiE "$SECRETS"; then +# Only ADDED lines count: removing a leaked secret from a file must stay committable (§11: rotate, then scrub). +if printf '%s' "$DIFF" | grep '^+' | grep -v '^+++' | grep -qiE "$SECRETS"; then echo "Blocked: a secret appears to be staged (CLAUDE.md §11). Unstage it." >&2; exit 2 fi exit 0 diff --git a/skills/new-repo/SKILL.md b/skills/new-repo/SKILL.md index fc83e63..44bfbe2 100644 --- a/skills/new-repo/SKILL.md +++ b/skills/new-repo/SKILL.md @@ -31,10 +31,12 @@ Starting or initializing a new project, or a repo that's missing the standard sc **common/** (both stacks): `.gitignore` (secret patterns from §11), `CHANGELOG.md` (Keep-a-Changelog header), `docs/REQUIREMENTS.md`, `docs/ROADMAP.md`, and the seed ADR `docs/decisions/0001-foundation.md`. -**android/**: `version.properties` (versionCode/versionName source of truth), `.github/workflows/gate.yml` (Spotless/detekt/lint/unit/Roborazzi/compileRelease), `.github/workflows/release.yml` (tag↔versionName parity → signed build → GitHub Release), `CLAUDE.md` (§19, Android-flavored). +**android/**: `version.properties` (versionCode/versionName source of truth), `.github/workflows/gate.yml` (Spotless/detekt/lint/unit/Roborazzi/compileRelease), `.github/workflows/release.yml` (tag↔versionName parity → signed build → GitHub Release), `CLAUDE.md` (§19; the same generic template as web — the Android specifics go into §19 when you fill it in). **web/**: `.github/workflows/gate.yml` (format/lint/typecheck/test/build), `.github/workflows/release.yml` (tag↔version parity → publish), `CLAUDE.md` (§19, generic). +Both `CLAUDE.md.tmpl` files are byte-identical copies of `templates/CLAUDE.project.md`; the repo's quality gate (`CLAUDE.md` §19.3) diffs them so they cannot drift. + **`/gitignore-extra.tmpl`** (both stacks): appended to `.gitignore` in step 3 — web adds `node_modules/`, `dist/`, `.next/`, `coverage/`, `*.tsbuildinfo`; android adds `.gradle/`, `build/`, `local.properties`, `*.apk`/`*.aab`, etc. ## Secret patterns the .gitignore enforces (§11) diff --git a/skills/new-repo/templates/android/CLAUDE.md.tmpl b/skills/new-repo/templates/android/CLAUDE.md.tmpl index 72138d4..eff45a2 100644 --- a/skills/new-repo/templates/android/CLAUDE.md.tmpl +++ b/skills/new-repo/templates/android/CLAUDE.md.tmpl @@ -8,7 +8,7 @@ ### 19.1 What is this project? -- **One-paragraph description:** +- **One-paragraph description:** - TODO: one-paragraph description (the product, the user, the value). ### 19.2 Stack @@ -16,26 +16,28 @@ State versions specifically. "Latest" rots; pinned versions document reality. - **Language(s):** TODO - - *Example:* `TypeScript 5.6, with a small Rust binary (1.81) for the diff engine` + - _Example:_ `TypeScript 5.6, with a small Rust binary (1.81) for the diff engine` - **Runtime / platform:** TODO - - *Example:* `Node.js 22 LTS on Alpine 3.20 containers` + - _Example:_ `Node.js 22 LTS on Alpine 3.20 containers` - **Framework(s):** TODO - - *Example:* `Fastify 5 for HTTP, BullMQ for jobs, Drizzle ORM` + - _Example:_ `Fastify 5 for HTTP, BullMQ for jobs, Drizzle ORM` - **Storage:** TODO - - *Example:* `PostgreSQL 16 (primary), Redis 7 (queue + cache), S3-compatible object store for diffs` + - _Example:_ `PostgreSQL 16 (primary), Redis 7 (queue + cache), S3-compatible object store for diffs` - **Build / package:** TODO - - *Example:* `pnpm 9, with Turborepo for the monorepo` + - _Example:_ `pnpm 9, with Turborepo for the monorepo` - **Test runner:** TODO - - *Example:* `vitest for unit + integration, Playwright for end-to-end` + - _Example:_ `vitest for unit + integration, Playwright for end-to-end` - **CI:** TODO - - *Example:* `GitHub Actions, self-hosted runners for the integration tests, GitHub-hosted for everything else` + - _Example:_ `GitHub Actions, self-hosted runners for the integration tests, GitHub-hosted for everything else` - **Distribution channel:** TODO - - *Example:* `Container image pushed to GHCR, deployed via ArgoCD to internal Kubernetes` + - _Example:_ `Container image pushed to GHCR, deployed via ArgoCD to internal Kubernetes` ### 19.3 Local quality gate Concrete commands a fresh clone can run. Order matches §7.1 (fail fast on cheap steps). +bash + ```bash # Example — replace with your project's actual commands. pnpm format:check # prettier --check . @@ -51,30 +53,29 @@ Principle: every command must be runnable from a fresh clone without further set ### 19.4 Current release pointers - **Live version:** TODO - - *Example:* `v2.14.3 (released 2026-04-22)` + - _Example:_ `v2.14.3 (released 2026-04-22)` - **In flight:** TODO - - *Example:* `v2.15.0 — theme: "webhook retry hardening", ETA 2026-05-15` + - _Example:_ `v2.15.0 — theme: "webhook retry hardening", ETA 2026-05-15` - **CHANGELOG:** ./CHANGELOG.md -- **Spec / PRD:** ./docs/REQUIREMENTS.md - - *If specs live elsewhere (Notion, Linear, Confluence), put the canonical link here. Don't maintain two sources of truth.* +- **Spec / PRD:** ./docs/SPEC.md + - _If specs live elsewhere (Notion, Linear, Confluence), put the canonical link here. Don't maintain two sources of truth._ - **Roadmap:** ./docs/ROADMAP.md - **WIP file:** ./WIP.md - - *Only exists when a session ended mid-task; deleted on next merge. See §17.1.* + - _Only exists when a session ended mid-task; deleted on next merge. See §17.1._ ### 19.5 Compliance scope - TODO - - *Example (B2B SaaS in EU + US):* `GDPR (EU users), CCPA (CA users), SOC 2 Type II (in audit, target 2026-Q4). No HIPAA. No PCI-DSS — payment data is tokenized by Stripe; we never see PAN.` - - *Example (internal tool):* `None — internal-only, no external users, no regulated data.` - - *Principle:* be specific about what applies AND what explicitly doesn't. "SOC 2 doesn't apply because X" is more useful than silence. + - _Example (B2B SaaS in EU + US):_ `GDPR (EU users), CCPA (CA users), SOC 2 Type II (in audit, target 2026-Q4). No HIPAA. No PCI-DSS — payment data is tokenized by Stripe; we never see PAN.` + - _Example (internal tool):_ `None — internal-only, no external users, no regulated data.` + - _Principle:_ be specific about what applies AND what explicitly doesn't. "SOC 2 doesn't apply because X" is more useful than silence. ### 19.6 Project-specific overrides > Each override erodes the predictability §1–18 provides; treat them as debt with a documented reason. Review quarterly: can any be removed? -- *(none — defaults apply)* -- *Example override:* `§7.1 quality gate skips integration tests on PR (runs nightly instead) — reason: integration suite takes 25 min, blocks PR throughput. Tracked in #1247 for a fix.` -- *Example override:* `§2 squash-merge replaced with rebase-merge — reason: we use a release-train workflow that depends on commit-level traceability.` - ------- +- _(none — defaults apply)_ +- _Example override:_ `§7.1 quality gate skips integration tests on PR (runs nightly instead) — reason: integration suite takes 25 min, blocks PR throughput. Tracked in #1247 for a fix.` +- _Example override:_ `§2 squash-merge replaced with rebase-merge — reason: we use a release-train workflow that depends on commit-level traceability.` +--- diff --git a/skills/new-repo/templates/web/CLAUDE.md.tmpl b/skills/new-repo/templates/web/CLAUDE.md.tmpl index 72138d4..eff45a2 100644 --- a/skills/new-repo/templates/web/CLAUDE.md.tmpl +++ b/skills/new-repo/templates/web/CLAUDE.md.tmpl @@ -8,7 +8,7 @@ ### 19.1 What is this project? -- **One-paragraph description:** +- **One-paragraph description:** - TODO: one-paragraph description (the product, the user, the value). ### 19.2 Stack @@ -16,26 +16,28 @@ State versions specifically. "Latest" rots; pinned versions document reality. - **Language(s):** TODO - - *Example:* `TypeScript 5.6, with a small Rust binary (1.81) for the diff engine` + - _Example:_ `TypeScript 5.6, with a small Rust binary (1.81) for the diff engine` - **Runtime / platform:** TODO - - *Example:* `Node.js 22 LTS on Alpine 3.20 containers` + - _Example:_ `Node.js 22 LTS on Alpine 3.20 containers` - **Framework(s):** TODO - - *Example:* `Fastify 5 for HTTP, BullMQ for jobs, Drizzle ORM` + - _Example:_ `Fastify 5 for HTTP, BullMQ for jobs, Drizzle ORM` - **Storage:** TODO - - *Example:* `PostgreSQL 16 (primary), Redis 7 (queue + cache), S3-compatible object store for diffs` + - _Example:_ `PostgreSQL 16 (primary), Redis 7 (queue + cache), S3-compatible object store for diffs` - **Build / package:** TODO - - *Example:* `pnpm 9, with Turborepo for the monorepo` + - _Example:_ `pnpm 9, with Turborepo for the monorepo` - **Test runner:** TODO - - *Example:* `vitest for unit + integration, Playwright for end-to-end` + - _Example:_ `vitest for unit + integration, Playwright for end-to-end` - **CI:** TODO - - *Example:* `GitHub Actions, self-hosted runners for the integration tests, GitHub-hosted for everything else` + - _Example:_ `GitHub Actions, self-hosted runners for the integration tests, GitHub-hosted for everything else` - **Distribution channel:** TODO - - *Example:* `Container image pushed to GHCR, deployed via ArgoCD to internal Kubernetes` + - _Example:_ `Container image pushed to GHCR, deployed via ArgoCD to internal Kubernetes` ### 19.3 Local quality gate Concrete commands a fresh clone can run. Order matches §7.1 (fail fast on cheap steps). +bash + ```bash # Example — replace with your project's actual commands. pnpm format:check # prettier --check . @@ -51,30 +53,29 @@ Principle: every command must be runnable from a fresh clone without further set ### 19.4 Current release pointers - **Live version:** TODO - - *Example:* `v2.14.3 (released 2026-04-22)` + - _Example:_ `v2.14.3 (released 2026-04-22)` - **In flight:** TODO - - *Example:* `v2.15.0 — theme: "webhook retry hardening", ETA 2026-05-15` + - _Example:_ `v2.15.0 — theme: "webhook retry hardening", ETA 2026-05-15` - **CHANGELOG:** ./CHANGELOG.md -- **Spec / PRD:** ./docs/REQUIREMENTS.md - - *If specs live elsewhere (Notion, Linear, Confluence), put the canonical link here. Don't maintain two sources of truth.* +- **Spec / PRD:** ./docs/SPEC.md + - _If specs live elsewhere (Notion, Linear, Confluence), put the canonical link here. Don't maintain two sources of truth._ - **Roadmap:** ./docs/ROADMAP.md - **WIP file:** ./WIP.md - - *Only exists when a session ended mid-task; deleted on next merge. See §17.1.* + - _Only exists when a session ended mid-task; deleted on next merge. See §17.1._ ### 19.5 Compliance scope - TODO - - *Example (B2B SaaS in EU + US):* `GDPR (EU users), CCPA (CA users), SOC 2 Type II (in audit, target 2026-Q4). No HIPAA. No PCI-DSS — payment data is tokenized by Stripe; we never see PAN.` - - *Example (internal tool):* `None — internal-only, no external users, no regulated data.` - - *Principle:* be specific about what applies AND what explicitly doesn't. "SOC 2 doesn't apply because X" is more useful than silence. + - _Example (B2B SaaS in EU + US):_ `GDPR (EU users), CCPA (CA users), SOC 2 Type II (in audit, target 2026-Q4). No HIPAA. No PCI-DSS — payment data is tokenized by Stripe; we never see PAN.` + - _Example (internal tool):_ `None — internal-only, no external users, no regulated data.` + - _Principle:_ be specific about what applies AND what explicitly doesn't. "SOC 2 doesn't apply because X" is more useful than silence. ### 19.6 Project-specific overrides > Each override erodes the predictability §1–18 provides; treat them as debt with a documented reason. Review quarterly: can any be removed? -- *(none — defaults apply)* -- *Example override:* `§7.1 quality gate skips integration tests on PR (runs nightly instead) — reason: integration suite takes 25 min, blocks PR throughput. Tracked in #1247 for a fix.` -- *Example override:* `§2 squash-merge replaced with rebase-merge — reason: we use a release-train workflow that depends on commit-level traceability.` - ------- +- _(none — defaults apply)_ +- _Example override:_ `§7.1 quality gate skips integration tests on PR (runs nightly instead) — reason: integration suite takes 25 min, blocks PR throughput. Tracked in #1247 for a fix.` +- _Example override:_ `§2 squash-merge replaced with rebase-merge — reason: we use a release-train workflow that depends on commit-level traceability.` +--- diff --git a/tests/hooks/test-format.sh b/tests/hooks/test-format.sh new file mode 100755 index 0000000..cb6b747 --- /dev/null +++ b/tests/hooks/test-format.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +# Smoke tests for hooks/format.sh: a PostToolUse(Edit|Write) payload on stdin, always exit 0, +# only the tool's own target file is ever touched. Usage: bash tests/hooks/test-format.sh +set -u + +HERE=$(cd "$(dirname "$0")" && pwd) +HOOK="$HERE/../../hooks/format.sh" +PASS=0 +FAIL=0 +TMP=$(mktemp -d) +trap 'rm -rf "$TMP"' EXIT + +# check +check() { + if [ "$2" -eq 0 ]; then PASS=$((PASS + 1)); else FAIL=$((FAIL + 1)); echo "FAIL: $1"; fi +} + +# run_hook : feed a PostToolUse payload naming that path +run_hook() { + jq -n --arg p "$1" '{tool_input:{file_path:$p}}' | bash "$HOOK" >/dev/null 2>&1 +} + +# 1. Empty payload: exit 0, nothing happens. +printf '' | bash "$HOOK" >/dev/null 2>&1 +check "empty payload exits 0" $? + +# 2. Nonexistent path: exit 0, file is not created. +run_hook "$TMP/missing.md"; rc=$? +ok=1; [ "$rc" -eq 0 ] && [ ! -e "$TMP/missing.md" ] && ok=0 +check "nonexistent path exits 0 and creates nothing" $ok + +# 3. Unknown extension: exit 0, file untouched. +printf 'keep me\n' >"$TMP/note.xyz" +before=$(cat "$TMP/note.xyz") +run_hook "$TMP/note.xyz"; rc=$? +ok=1; [ "$rc" -eq 0 ] && [ "$(cat "$TMP/note.xyz")" = "$before" ] && ok=0 +check "unknown extension is a no-op" $ok + +# 4. Idempotent: a second run on an already-formatted file changes nothing, and no other file appears. +# (With no formatter installed, e.g. on CI, both runs are no-ops; the "touches nothing else" half still holds.) +printf '# Title\n\nSome text.\n' >"$TMP/doc.md" +run_hook "$TMP/doc.md" +first=$(cat "$TMP/doc.md") +run_hook "$TMP/doc.md"; rc=$? +count=0; for f in "$TMP"/*; do [ -e "$f" ] && count=$((count + 1)); done +ok=1; [ "$rc" -eq 0 ] && [ "$(cat "$TMP/doc.md")" = "$first" ] && [ "$count" -eq 2 ] && ok=0 +check "second run is a no-op and touches nothing else" $ok + +echo "format: $PASS passed, $FAIL failed" +[ "$FAIL" -eq 0 ] diff --git a/tests/hooks/test-guard-commit.sh b/tests/hooks/test-guard-commit.sh new file mode 100755 index 0000000..1215297 --- /dev/null +++ b/tests/hooks/test-guard-commit.sh @@ -0,0 +1,225 @@ +#!/usr/bin/env bash +# Behavioral tests for hooks/guard-commit.sh: a PreToolUse(Bash) payload on stdin, an exit code out. +# 0 = allow, 2 = block. One case per rule the hook enforces and per regression it has had. +# Runs on macOS system bash 3.2 with jq + git only. Usage: bash tests/hooks/test-guard-commit.sh +# shellcheck disable=SC2016 # several cases deliberately pass literal "$(cat <<'EOF' ...)" commit messages +set -u + +HERE=$(cd "$(dirname "$0")" && pwd) +SELF="$HERE/$(basename "$0")" +HOOK="$HERE/../../hooks/guard-commit.sh" +PASS=0 +FAIL=0 +TMP=$(mktemp -d) +trap 'rm -rf "$TMP"' EXIT + +# Isolate git from the machine's real identity/config so the committer checks are deterministic. +export GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 +export GIT_AUTHOR_NAME="Test Human" GIT_COMMITTER_NAME="Test Human" +export GIT_AUTHOR_EMAIL=test@example.com GIT_COMMITTER_EMAIL=test@example.com + +# record +record() { + if [ "$3" -eq "$2" ]; then + PASS=$((PASS + 1)) + else + FAIL=$((FAIL + 1)) + echo "FAIL: $1 (want exit $2, got $3): $4" + sed 's/^/ stderr: /' "$TMP/err" + fi +} + +# run +# Never call this inside a subshell: PASS/FAIL must be counted in this shell. +run() { + printf '%s' "$3" | jq -Rs '{tool_input:{command:.}}' | bash "$HOOK" >/dev/null 2>"$TMP/err" + record "$1" "$2" $? "$3" +} + +# run_raw +run_raw() { + printf '%s' "$3" | bash "$HOOK" >/dev/null 2>"$TMP/err" + record "$1" "$2" $? "$3" +} + +# A throwaway repo with the given committer, one tracked file, and a clean tree. +mkrepo() { + d=$(mktemp -d "$TMP/repo.XXXXXX") + git -C "$d" init -q + git -C "$d" config user.name "$1" + git -C "$d" config user.email test@example.com + echo "hello" >"$d/tracked.txt" + git -C "$d" add tracked.txt + git -C "$d" commit -qm "init" + echo "$d" +} + +# Fake credentials are assembled at runtime so this file never contains a secret-shaped literal +# (the hook would otherwise refuse to commit its own test suite). +AWS_KEY=$(printf 'AKIA%s' "$(printf 'A%.0s' 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16)") +PEM_HEADER=$(printf -- '-----BEGIN RSA %s KEY-----' "PRIVATE") + +REPO=$(mkrepo "Test Human") +cd "$REPO" || exit 1 + +# --- fast path: commands that are not git commit/push are never inspected --------------------- +run "unrelated command" 0 'ls -la' +run "git without commit/push" 0 'git status && git log --oneline -3' +run "empty command" 0 '' +run_raw "empty stdin" 0 '' +run_raw "non-JSON payload mentioning commit is fail-closed" 2 'git commit -m x' +run_raw "payload with renamed key is fail-closed" 2 '{"tool_input":{"cmd":"git push --force"}}' + +# --- force-push (7663993) ----------------------------------------------------------------------- +run "push --force" 2 'git push --force origin feat/x' +run "push -f" 2 'git push -f origin feat/x' +run "push --force-with-lease" 2 'git push --force-with-lease origin feat/x' +run "push --mirror" 2 'git push --mirror origin' +run "push -f after git -C" 2 'git -C /tmp/x push -f' +run "push -uf flag cluster" 2 'git push -uf origin feat/x' +run "push -fu flag cluster" 2 'git push -fu origin feat/x' +run "push +refspec (force by refspec)" 2 'git push origin +main' +run "push +HEAD:branch refspec" 2 'git push origin +HEAD:feat/x' +run "push with env assignment prefix" 2 'FOO=1 git push -f origin x' +run "push --force on a continuation line" 2 'git push \ + --force-with-lease \ + origin feat/x' +run "plain push" 0 'git push origin feat/x' +run "push -u" 0 'git push -u origin feat/x' +run "push --set-upstream" 0 'git push --set-upstream origin feat/x' +run "push of a branch named -f-ish" 0 'git push origin feature-flags' +run "push -n is dry-run, not no-verify" 0 'git push -n origin feat/x' + +# --- commit message that merely mentions a forbidden flag (07dd319) ---------------------------- +run "message mentions push --force" 0 'git commit -m "docs: explain why push --force is blocked"' +run "message mentions --no-verify" 0 'git commit -m "docs: never use --no-verify"' +run "message mentions -n" 0 'git commit -m "fix: handle -n flag"' +run "message with a separator inside" 0 'git commit -m "fix: a; b && git push -f"' +run "message with an escaped quote" 0 'git commit -m "fix: 5\" screen && git push -f"' +run "heredoc message mentioning push -f" 0 'git commit -m "$(cat <<'"'"'EOF'"'"' +feat: x + +A chained git commit && git push -f was allowed before. +EOF +)"' +run "heredoc message mentioning --no-verify" 0 'git commit -m "$(cat <<'"'"'EOF'"'"' +fix: guard + +The guard blocks git commit --no-verify and git push --force. +Also git commit -n is blocked. +EOF +)"' +run "multi-line -m message" 0 'git commit -m "docs: x + +why git push -f is blocked"' + +# --- chained commands: every segment is classified on its own --------------------------------- +run "commit then force-push" 2 'git commit -m "ok" && git push -f origin feat/x' +run "commit; force-push" 2 'git commit -m "ok"; git push --force origin feat/x' +run "commit || force-push" 2 'git commit -m "ok" || git push -f' +run "commit newline force-push" 2 'git commit -m "ok" +git push -f origin feat/x' +run "commit then plain push" 0 'git commit -m "ok" && git push -u origin feat/x' + +# --- --no-verify skips the repo's own git hooks (CLAUDE.md §14) -------------------------------- +run "commit --no-verify" 2 'git commit --no-verify -m "x"' +run "commit --no-verify on continuation" 2 'git commit \ + --no-verify \ + -m "x"' +run "commit -n short flag" 2 'git commit -n -m "x"' +run "commit -anm combined short flags" 2 'git commit -anm "x"' +run "commit -n glued to a redirect" 2 'git commit -m x -n>/dev/null' +run "push --no-verify" 2 'git push --no-verify origin feat/x' +run "commit then push --no-verify" 2 'git commit -m "x" && git push --no-verify' +run "hooksPath -c override on commit" 2 'git -c core.hooksPath=/dev/null commit -m "x"' +run "hooksPath config then commit" 2 'git config core.hooksPath /dev/null; git commit -m "x"' +run "hooksPath via GIT_CONFIG_PARAMETERS" 2 'GIT_CONFIG_PARAMETERS="core.hooksPath=/dev/null" git commit -m "x"' +run "git log --no-verify is not a commit" 0 'git log --no-verify' +run "commit --amend --no-edit" 0 'git commit --amend --no-edit' +run "commit --signoff" 0 'git commit --signoff -m "x"' + +# --- shell wrappers hide the real command: refuse rather than guess ---------------------------- +run "bash -c wrapper" 2 'bash -c "git push -f origin main"' +run "sh -c wrapper" 2 'sh -c '"'"'git commit --no-verify -m x'"'"'' +run "eval wrapper" 2 'eval "git push --force origin main"' +run "script named *.sh before a commit" 0 'bash tests/hooks/test-format.sh && git commit -m "x"' + +# --- AI attribution in the commit message (CLAUDE.md §2) --------------------------------------- +run "co-authored-by trailer" 2 'git commit -m "feat: x" -m "Co-authored-by: Claude "' +run "generated-with line" 2 'git commit -m "feat: x" -m "Generated with Claude Code"' +run "robot emoji" 2 'git commit -m "🤖 feat: x"' +run "bare tool mention is fine" 0 'git commit -m "docs: describe the Claude Code hook"' + +# --- committer must be a human (CLAUDE.md §2) -------------------------------------------------- +BOT=$(mkrepo "Claude") +cd "$BOT" || exit 1 +run "non-human committer" 2 'git commit -m "x"' +BOT2=$(mkrepo "github-actions[bot]") +cd "$BOT2" || exit 1 +run "github-actions committer" 2 'git commit -m "x"' +cd "$REPO" || exit 1 + +# --- secrets in the staged diff (CLAUDE.md §11), incl. the -a auto-stage path (229e95d) -------- +printf 'aws_key=%s\n' "$AWS_KEY" >>tracked.txt +run "dirty tracked secret, plain commit" 0 'git commit -m "x"' +run "dirty tracked secret, commit -am" 2 'git commit -am "x"' +run "dirty tracked secret, commit --all" 2 'git commit --all -m "x"' +run "dirty tracked secret, add && commit" 2 'git add tracked.txt && git commit -m "x"' +git checkout -q -- tracked.txt + +printf 'aws_key=%s\n' "$AWS_KEY" >config.txt +git add config.txt +run "staged secret" 2 'git commit -m "x"' +git rm -q --cached config.txt && rm config.txt + +printf 'aws_key=%s\n' "$AWS_KEY" >config.txt +run "untracked secret, add && commit" 2 'git add config.txt && git commit -m "x"' +run "untracked secret, add -A && commit" 2 'git add -A && git commit -m "x"' +run "untracked secret, plain commit" 0 'git commit -m "x"' +rm config.txt + +printf 'aws_key=%s\n' "$AWS_KEY" >.env.example +git add .env.example +run "staged secret in .env.example (72d9ea4)" 0 'git commit -m "x"' +git rm -q --cached .env.example +run "untracked .env.example, add && commit" 0 'git add .env.example && git commit -m "x"' +rm .env.example + +printf '%s\n' "$PEM_HEADER" >key.txt +git add key.txt +run "staged private key header" 2 'git commit -m "x"' +git rm -q --cached key.txt && rm key.txt + +# Scrubbing a leaked secret (a removed line) must stay committable; git is called directly here, +# the hook is not involved in seeding the leak. +printf 'aws_key=%s\n' "$AWS_KEY" >leaked.txt +git add leaked.txt && git commit -qm "seed leak" +printf 'aws_key=REDACTED\n' >leaked.txt +git add leaked.txt +run "removing a leaked secret is allowed" 0 'git commit -m "chore: scrub leaked key"' +run "removing a leaked secret via add && commit" 0 'git add leaked.txt && git commit -m "chore: scrub"' +git reset -q --hard HEAD~1 + +# --- missing jq: fail closed for git commit/push, stay out of the way otherwise ---------------- +NOJQ="$TMP/nojq" +mkdir -p "$NOJQ" +for t in bash sh grep git sed tr printf cat; do + p=$(command -v "$t") && ln -s "$p" "$NOJQ/$t" +done +run_nojq() { + printf '%s' "$3" | jq -Rs '{tool_input:{command:.}}' >"$TMP/payload" + PATH="$NOJQ" bash "$HOOK" <"$TMP/payload" >/dev/null 2>"$TMP/err" + record "$1" "$2" $? "$3" +} +run_nojq "no jq: commit fails closed" 2 'git commit -m "x"' +run_nojq "no jq: unrelated command allowed" 0 'ls' + +# Every case in this file must have been counted: a case that ran in a subshell would be lost. +EXPECTED=$(grep -cE '^(run|run_raw|run_nojq) ' "$SELF") +if [ "$((PASS + FAIL))" -ne "$EXPECTED" ]; then + echo "FAIL: $EXPECTED cases defined, $((PASS + FAIL)) counted" + FAIL=$((FAIL + 1)) +fi + +echo "guard-commit: $PASS passed, $FAIL failed" +[ "$FAIL" -eq 0 ]