chore: Claude-first setup#2783
Conversation
- Stop forcing reviewers to fabricate findings; nits/notes are advisory - Require explicit APPROVE:/CLEAN: verdict (not just absence of BLOCK:) - Use repo root in lint/test hooks so they work from subdirectories - Add branch guards to rebase agent (skip protected branches) - Pass --allowedTools to review agents for Bash/Read access - Add per-project lessons link to CLAUDE.md - Remove dangling .claude/skills symlink Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> fix: CI monitor needs --allowedTools and effort: medium to actually execute
Adds a PostToolUse hook that fires after PR creation and spawns a changelog-manager agent to classify the diff. The agent determines whether a CHANGELOG.md entry or "no changelog" label is needed, and feeds actionable instructions back to the main agent via exit code 2. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Improve Claude setup based on NTL battle-testing Consolidated pre-push hook: - Merged pre-push-test.sh into pre-push-review.sh (tests run first, then reviewers only if tests pass - saves reviewer tokens on broken code) - Severity-based blocking: nits and notes don't block push, only Critical/Important/Warning findings do - Proper diff base resolution: tries upstream, falls back to default branch via gh, then HEAD~1 - Handles reviewer crashes gracefully (empty/malformed output = block) - Escape hatch: SKIP_PRE_PUSH=1 Input guards on all hooks: - Each hook validates the command matches before acting (defense in depth against settings.json filter regressions) Improved settings.json: - Removed separate pre-push-test.sh entry (now in pre-push-review.sh) - Added proper `if` filter on pre-pr-draft.sh (was firing on all Bash) - Removed post-pr-create-ci-monitor.sh (superseded) Lessons system: - Added .claude/lessons.md for codifying review feedback - Added .claude/commands/codify-lesson.md slash command - Updated CLAUDE.md to reference in-repo lessons file instead of global ~/.claude path Agent improvements: - Code reviewer and security reviewer use gh-based default branch resolution instead of hardcoded branch names Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address review: remove guards, split hooks, drop lessons - Remove input guards from all hooks (boilerplate, unclear value) - Restore pre-push-test.sh as separate hook (keep tests and reviews conceptually separate per review feedback) - Remove lessons.md and codify-lesson.md (use SKILLS.md instead) - Revert CLAUDE.md lessons reference - Keep: severity-based blocking, diff base resolution, crash handling, proper if filters, agent branch resolution via gh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Adding new lines at the end of the file * Removing unecessary code from post-pr-create-changelog.sh * removing code from pre-pr-draft.sh * remove escape hatch from pre-pr-draft.sh * simplifying pre-pr-draft.sh * removing escape hatch and default org repo from pre-push-review.sh --------- Co-authored-by: Claude (Opus) <noreply@anthropic.com>
There was a problem hiding this comment.
Looks good to me, like a very detailed plan of what I'd write in plan mode. Agents are cool, but updating anything related to claude setup is very difficult.
A few things I noticed while using this:
-
I've noticed after using this setup, is that when running it, my machine M4 pro mac runs very hot because I think a lot of time is spent running
cargo build&cargo testrelated commands. Maybe its because I had a couple agents working at once? In my normal workflow pre claude code, I would just runmake testand this would catch most issues. Maybe we need to update the setup so that claude code knows to runmake testas opposed to othercargo Xcommands. -
Its difficult to update the hooks since some of the changes to the hooks are not allowed by the classifier model. Ideally its easy to update the hooks, CLAUDE.md, and agent skills inside of a PR, so that over time the claude setup gets better over time.
Overall I think the setup is good. It one shotted this PR after a deep design / planning session: #2912
Can you elaborate @partylikeits1983 ? I'm not sure I understand |
The difficulty is that say you want to use claude code to update the hooks or agents. Claude code really makes it hard to do because in the rules it says to not update the hooks or agents. |
Desired workflow:
make lintmake test--draftmodeCHANGELOG.mdexists; if yes, proceeds; if not, determines whether it should add an entry or ano changeloglabel - the main agent takes care of this.The goal is to codify our shared knowledge into a set of actionable artifacts.
TODOs: