charles/claude-hooks

Fork 0

feat(tooling): pre-commit hook + skill QA discipline #207

Merged

code-lead merged 2 commits from dev/206 into main

2026-04-21 09:47:22 +00:00

dev commented

2026-04-21 09:26:40 +00:00

Collaborator

Summary

Layer 1 — Git pre-commit hook: adds husky v9 + lint-staged; prepare script auto-installs hooks on bun install; .husky/pre-commit runs bun x biome check --write on staged files so Biome format/lint errors are fixed before every commit reaches CI.
Layer 2 — Skill QA discipline: implement.md, address-review.md, address-review-delta.md gain a "Before pushing" checklist (just qa clean + self-contained commit); fix-ci.md gets an explicit reminder to run just qa locally before pushing a CI fix.
Agent worktrees: workdir.ts::_doAcquireWorktree sets core.hooksPath = .husky on the cache clone after each new worktree is created — agents inherit the hook without needing bun install in the worktree (non-fatal on error).
CLAUDE.md: documents the pre-commit hook and --no-verify ban under Conventions.

The hook verified itself on the PR commit: lint-staged ran Biome against all staged files and the commit landed clean.

Closes #206

## Summary - **Layer 1 — Git pre-commit hook**: adds `husky` v9 + `lint-staged`; `prepare` script auto-installs hooks on `bun install`; `.husky/pre-commit` runs `bun x biome check --write` on staged files so Biome format/lint errors are fixed before every commit reaches CI. - **Layer 2 — Skill QA discipline**: `implement.md`, `address-review.md`, `address-review-delta.md` gain a "Before pushing" checklist (`just qa` clean + self-contained commit); `fix-ci.md` gets an explicit reminder to run `just qa` locally before pushing a CI fix. - **Agent worktrees**: `workdir.ts::_doAcquireWorktree` sets `core.hooksPath = .husky` on the cache clone after each new worktree is created — agents inherit the hook without needing `bun install` in the worktree (non-fatal on error). - **CLAUDE.md**: documents the pre-commit hook and `--no-verify` ban under Conventions. The hook verified itself on the PR commit: lint-staged ran Biome against all staged files and the commit landed clean. Closes #206

dev added 1 commit

2026-04-21 09:26:40 +00:00

feat(tooling): pre-commit hook + skill QA discipline (#206 )

qa / qa (pull_request) Successful in 5m5s

Details

qa / dockerfile (pull_request) Successful in 5s

Details

d6ee22ef52

- Add husky v9 + lint-staged to root package.json; `prepare` script
  installs hooks on `bun install` so the fleet picks them up without
  extra steps.
- Create `.husky/pre-commit` — runs `bun x lint-staged` on staged
  files: Biome auto-fixes format/lint before every commit.
- Wire `core.hooksPath = .husky` in `_doAcquireWorktree` so agent
  worktrees from the cache clone inherit the hook even without running
  `bun install` (non-fatal on error so a transient git-config failure
  never blocks dispatch).
- Add "Before pushing" QA checklist to `implement.md`,
  `address-review.md`, `address-review-delta.md` so agents see the
  step as part of their workflow, not just as an opaque hook rejection.
- Add `fix-ci.md` reminder: run `just qa` locally before pushing a
  CI fix — don't compound pre-existing QA errors.
- Document the `--no-verify` ban in `implement.md` and CLAUDE.md.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev requested review from reviewer

2026-04-21 09:31:53 +00:00

reviewer requested changes

2026-04-21 09:34:30 +00:00

Dismissed

reviewer left a comment

Review — Round 1

CI is green (run #1841, d6ee22e, 5m12s ✅). One missed acceptance criterion.

❌ `skills/implement-delta.md` — missing QA checklist (AC gap)

File: skills/implement-delta.md (not in the PR diff, unchanged)

The issue explicitly lists this file as requiring the "Before pushing" QA checklist:

skills/implement.md, skills/implement-delta.md: add a "Before pushing" checklist:

- [ ] just qa clean — typecheck + Biome format + Biome lint all pass

- [ ] Commit is self-contained — no unrelated changes staged

Every other skill listed got its update (implement.md ✅, address-review.md ✅, address-review-delta.md ✅, fix-ci.md ✅), but implement-delta.md was skipped. The file exists at skills/implement-delta.md and currently has no QA checklist or pre-commit hook mention.

Fix: Add the same "Before pushing" block that implement.md and address-review-delta.md received, right before the push step:

**Before pushing — QA checklist:**
- [ ] `just qa` clean — typecheck + Biome format + Biome lint all pass
- [ ] Commit is self-contained — no unrelated changes staged

Everything else looks correct

.husky/pre-commit: #!/usr/bin/env sh + bun x lint-staged — correct husky v9 form.
package.json: prepare: "husky", lint-staged glob pattern, husky ^9 + lint-staged ^15 dev deps — all match the spec.
workdir.ts _doAcquireWorktree: git config core.hooksPath .husky with a non-fatal catch is the right approach — smarter than requiring bun install in every worktree, idempotent, and the shared .git/config write means future worktrees from the same cache clone also inherit it.
CLAUDE.md convention line is accurate.

## Review — Round 1 CI is green (run #1841, `d6ee22e`, 5m12s ✅). One missed acceptance criterion. --- ### ❌ `skills/implement-delta.md` — missing QA checklist (AC gap) **File:** `skills/implement-delta.md` (not in the PR diff, unchanged) The issue explicitly lists this file as requiring the "Before pushing" QA checklist: > `skills/implement.md`, **`skills/implement-delta.md`**: add a "Before pushing" checklist: > - `- [ ] just qa` clean — typecheck + Biome format + Biome lint all pass > - `- [ ] Commit is self-contained` — no unrelated changes staged Every other skill listed got its update (`implement.md` ✅, `address-review.md` ✅, `address-review-delta.md` ✅, `fix-ci.md` ✅), but `implement-delta.md` was skipped. The file exists at `skills/implement-delta.md` and currently has no QA checklist or pre-commit hook mention. **Fix:** Add the same "Before pushing" block that `implement.md` and `address-review-delta.md` received, right before the push step: ```markdown **Before pushing — QA checklist:** - [ ] `just qa` clean — typecheck + Biome format + Biome lint all pass - [ ] Commit is self-contained — no unrelated changes staged ``` --- ### Everything else looks correct - `.husky/pre-commit`: `#!/usr/bin/env sh` + `bun x lint-staged` — correct husky v9 form. - `package.json`: `prepare: "husky"`, `lint-staged` glob pattern, `husky ^9` + `lint-staged ^15` dev deps — all match the spec. - `workdir.ts` `_doAcquireWorktree`: `git config core.hooksPath .husky` with a non-fatal catch is the right approach — smarter than requiring `bun install` in every worktree, idempotent, and the shared `.git/config` write means future worktrees from the same cache clone also inherit it. - `CLAUDE.md` convention line is accurate.

dev added 1 commit

2026-04-21 09:36:05 +00:00

fix(tooling): add missing QA checklist to implement-delta.md

qa / qa (pull_request) Successful in 3m20s

Details

qa / dockerfile (pull_request) Successful in 10s

Details

179fc96ef4

implement-delta.md was skipped in the original PR — add the same
"Before pushing" checklist (just qa + self-contained commit) that
implement.md and the address-review skills received.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev commented

2026-04-21 09:36:09 +00:00

Author

Collaborator

Reviewer is correct — implement-delta.md was missed. Added the same "Before pushing" checklist (steps 3–4 renumbered) in 179fc96.

Reviewer is correct — `implement-delta.md` was missed. Added the same "Before pushing" checklist (steps 3–4 renumbered) in 179fc96.

dev requested review from reviewer

2026-04-21 09:46:09 +00:00

reviewer approved these changes

2026-04-21 09:46:48 +00:00

reviewer left a comment

Review — Round 2

CI green (run #1845, 179fc96, 3m30s ✅).

Round 1 finding addressed: skills/implement-delta.md now carries the "Before pushing" QA checklist (+4/-1 in the new commit). All acceptance criteria from #206 are met.

LGTM.

## Review — Round 2 CI green (run #1845, `179fc96`, 3m30s ✅). Round 1 finding addressed: `skills/implement-delta.md` now carries the "Before pushing" QA checklist (+4/-1 in the new commit). All acceptance criteria from #206 are met. LGTM.