chore(config): audit and improve Claude Code skills, agents, hooks - #675
Merged
Conversation
…nd CLAUDE.md CLAUDE.md: - Fix SonarQube → SonarCloud reference - Remove stale TESTING_APPROACH.md reference - Add release branch exception to Git & PRs section - Add "Automated Guardrails (Hooks)" section documenting all hook protections - Update Playwright CLI reference to include all 4 browser agents Agents: - project-architect: fix Next.js 15 → 16, /grill → /drill (4 occurrences) - code-reviewer: add CodeRabbit/SonarCloud checking steps, fix vitest → npm test Skills: - code: add mandatory /drill step before implementation - next (autopilot): add /drill step, replace broken npm run lint:fix with correct per-package commands, add E2E test step - test: fix HEAD~1 → origin/dev..HEAD for multi-commit branch detection - pr: add branch conventions, release branch exception, E2E step, fix SonarQube ref - issue: add commit scopes - Remove github-workflow skill (content consolidated into pr + issue skills) Hooks: - New check-migration-guard.sh: blocks edits to existing migration files (forward-only) - session-context.sh: add Docker health check at session start - settings.json: register migration guard hook Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughThis PR updates the Claude automation framework across agents, skills, and hooks. It introduces a migration-guard pre-tool hook, adds mandatory ChangesClaude Automation Framework Update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/code-reviewer.md:
- Around line 17-22: The review flow currently skips running tests for the
connection package; update the workflow step that runs tests (the list items
4–6) to conditionally run "cd connection && npm test" when there are changes
under the connection/ directory by adding a check for modified files in
connection/ and executing the test command (so the steps now include running
tests for app, component, and conditionally connection); ensure the check
references the connection/ path and the npm test invocation so PRs with
connection/ changes cannot be approved without running those tests.
In @.claude/skills/next/SKILL.md:
- Around line 38-43: Reorder the workflow so the mandatory "/drill <number>"
step runs before any branch creation commands: move the "Step 3 — Run /drill"
block to precede the branch-creation step and update surrounding headings (e.g.,
the current "Step 4 — Read the issue and relevant docs") so the sequence is
Drill -> Branch creation -> Read/implement; ensure the text explicitly states
"Run /drill <issue-number> before creating a branch" and remove any wording that
implies branch creation can occur first.
- Around line 52-60: Replace the brittle sequence of cd commands with
workspace-scoped invocations and an explicit app-only fix step: run lint from
the repo root (npm run lint) so all packages are checked, run the app-specific
auto-fix with npm -w app run lint -- --fix or cd app && npx next lint --fix to
apply Next.js fixes, build the project from root (npm run build), run all tests
from root (npm test) and explicitly run package-scoped tests for component and
app (npm -w component test and npm -w app test), and run Playwright from the app
workspace (npm -w app run playwright test or cd app && npx playwright test);
also ensure connection/ tests are included in the root or workspace test script
so they run with npm test.
In @.claude/skills/pr/SKILL.md:
- Line 23: The pre-flight instruction `git fetch origin && git rebase
origin/dev` conflicts with the release-target exception; update the phrasing (or
command example) so it rebases onto the actual PR target branch instead of
always `origin/dev` — e.g., show a conditional or placeholder such as `git fetch
origin && git rebase origin/$(TARGET_BRANCH)` or explain to use
`origin/release/X.Y` when the PR target is a release branch; update the `git
fetch origin && git rebase origin/dev` occurrence in SKILL.md accordingly.
In @.claude/skills/test/SKILL.md:
- Line 14: The diff command snippet "Changed files: !`git diff --name-only
origin/dev..HEAD 2>/dev/null || git diff --name-only`" can use a stale
origin/dev; before running that command ensure the base ref is refreshed (e.g.,
run a fetch) so changed-package detection is accurate—update the workflow/script
to perform git fetch origin (or git fetch --all --prune) before evaluating the
diff command or compare against a freshly fetched ref (use FETCH_HEAD or an
explicit fetched branch) so the "Changed files" check reliably detects changes.
In `@CLAUDE.md`:
- Around line 171-175: The docs list lifecycle hooks Stop and PreCompact that
aren't defined in .claude/settings.json while only SessionStart exists; update
either the documentation or the hook config to match: either remove the Stop and
PreCompact bullets from the "Session/Lifecycle" section or add corresponding
lifecycle entries named "Stop" and "PreCompact" to .claude/settings.json with
the intended behavior (e.g., Stop: completion checklist, PreCompact: re-inject
critical rules) so the documented phases align with the actual
SessionStart/Stop/PreCompact hook names.
- Around line 96-97: Combine the default rule and the release exception into one
clear statement so there is no contradiction: replace the separate lines
containing "**Exception**: when a `release/X.Y` branch is active, branch from
and target it instead of `dev`." and "PRs target `dev` (integration) before
merging to `main`." with a single explicit sentence such as "By default, open
PRs targeting `dev` (integration) before merging to `main`; exception: if a
`release/X.Y` branch is active, branch from and target `release/X.Y` instead of
`dev`." Ensure the phrases "`release/X.Y`" and "`dev` (integration)" remain
unchanged so the rule is unambiguous.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d954e3df-07fa-41ea-876e-51e366f5fc72
📒 Files selected for processing (12)
.claude/agents/code-reviewer.md.claude/agents/project-architect.md.claude/hooks/check-migration-guard.sh.claude/hooks/session-context.sh.claude/settings.json.claude/skills/code/SKILL.md.claude/skills/github-workflow/SKILL.md.claude/skills/issue/SKILL.md.claude/skills/next/SKILL.md.claude/skills/pr/SKILL.md.claude/skills/test/SKILL.mdCLAUDE.md
💤 Files with no reviewable changes (1)
- .claude/skills/github-workflow/SKILL.md
Comment on lines
+17
to
+22
| 4. After code review, run `cd app && npm test` and `cd component && npm test` to verify tests pass. | ||
| 5. Check external review feedback: | ||
| - CodeRabbit: `gh pr view --comments | grep -A10 'coderabbitai'` | ||
| - SonarCloud: `gh pr checks` — verify quality gate passes | ||
| - Flag any unaddressed CRITICAL/MAJOR findings | ||
| 6. If any UI files changed (`*.tsx` in pages, components, or settings), recommend running `@feature-reviewer` on the affected feature. |
There was a problem hiding this comment.
Review flow omits connection/ test verification.
Current step can approve PRs with unverified connection/ changes. Add a conditional cd connection && npm test path.
Suggested fix
-4. After code review, run `cd app && npm test` and `cd component && npm test` to verify tests pass.
+4. After code review, run tests for affected packages:
+ - `cd app && npm test` (if `app/` changed)
+ - `cd component && npm test` (if `component/` changed)
+ - `cd connection && npm test` (if `connection/` changed)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/agents/code-reviewer.md around lines 17 - 22, The review flow
currently skips running tests for the connection package; update the workflow
step that runs tests (the list items 4–6) to conditionally run "cd connection &&
npm test" when there are changes under the connection/ directory by adding a
check for modified files in connection/ and executing the test command (so the
steps now include running tests for app, component, and conditionally
connection); ensure the check references the connection/ path and the npm test
invocation so PRs with connection/ changes cannot be approved without running
those tests.
Comment on lines
+38
to
43
| ## Step 3 — Run /drill | ||
|
|
||
| Before implementing, run `/drill <number>` to gather requirements, edge cases, and acceptance criteria. This is mandatory per CLAUDE.md. | ||
|
|
||
| ## Step 4 — Read the issue and relevant docs | ||
|
|
There was a problem hiding this comment.
Move /drill before branch creation to match the mandatory workflow.
This step says /drill is mandatory, but branch creation currently happens earlier. Reorder so /drill <number> runs before any branch command.
Based on learnings: Run /drill <issue-number> before creating a branch or starting implementation. Do NOT skip the drill.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/next/SKILL.md around lines 38 - 43, Reorder the workflow so
the mandatory "/drill <number>" step runs before any branch creation commands:
move the "Step 3 — Run /drill" block to precede the branch-creation step and
update surrounding headings (e.g., the current "Step 4 — Read the issue and
relevant docs") so the sequence is Drill -> Branch creation -> Read/implement;
ensure the text explicitly states "Run /drill <issue-number> before creating a
branch" and remove any wording that implies branch creation can occur first.
Comment on lines
+52
to
+60
| ## Step 6 — Test and lint | ||
|
|
||
| ```bash | ||
| npm run lint:fix | ||
| cd app && npx next lint --fix | ||
| npm run lint | ||
| npm run build | ||
| npm run test | ||
| cd app && npm test | ||
| cd component && npm test | ||
| cd app && npx playwright test |
There was a problem hiding this comment.
Step 6 command sequence is brittle due to persistent cd state.
After cd app, subsequent commands are no longer guaranteed to run from repo root, and cd component can fail from inside app/. Use workspace-scoped commands and include connection/ tests.
Suggested fix
-cd app && npx next lint --fix
-npm run lint
-npm run build
-cd app && npm test
-cd component && npm test
-cd app && npx playwright test
+npm -w app exec next lint -- --fix
+npm run lint
+npm run build
+npm -w app run test
+npm -w component run test
+npm -w connection run test
+npm run test:e2eBased on learnings: Run npm run lint from repo root to lint all packages. Run cd app && npx next lint --fix for auto-fixes in app/.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/next/SKILL.md around lines 52 - 60, Replace the brittle
sequence of cd commands with workspace-scoped invocations and an explicit
app-only fix step: run lint from the repo root (npm run lint) so all packages
are checked, run the app-specific auto-fix with npm -w app run lint -- --fix or
cd app && npx next lint --fix to apply Next.js fixes, build the project from
root (npm run build), run all tests from root (npm test) and explicitly run
package-scoped tests for component and app (npm -w component test and npm -w app
test), and run Playwright from the app workspace (npm -w app run playwright test
or cd app && npx playwright test); also ensure connection/ tests are included in
the root or workspace test script so they run with npm test.
| ## Pre-flight (fix failures before creating PR) | ||
|
|
||
| 1. `git fetch origin && git rebase origin/dev` (PRs always target `dev`) | ||
| 1. `git fetch origin && git rebase origin/dev` (PRs always target `dev`; exception: target `release/X.Y` if active) |
There was a problem hiding this comment.
Pre-flight command conflicts with the release-target exception.
The text allows targeting release/X.Y, but the command still always rebases on origin/dev. That can put release PRs on the wrong base.
Suggested fix
-1. `git fetch origin && git rebase origin/dev` (PRs always target `dev`; exception: target `release/X.Y` if active)
+1. `git fetch origin` then rebase onto the intended PR base:
+ - `git rebase origin/dev` (default)
+ - `git rebase origin/release/X.Y` (when targeting release)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 1. `git fetch origin && git rebase origin/dev` (PRs always target `dev`; exception: target `release/X.Y` if active) | |
| 1. `git fetch origin` then rebase onto the intended PR base: | |
| - `git rebase origin/dev` (default) | |
| - `git rebase origin/release/X.Y` (when targeting release) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/pr/SKILL.md at line 23, The pre-flight instruction `git fetch
origin && git rebase origin/dev` conflicts with the release-target exception;
update the phrasing (or command example) so it rebases onto the actual PR target
branch instead of always `origin/dev` — e.g., show a conditional or placeholder
such as `git fetch origin && git rebase origin/$(TARGET_BRANCH)` or explain to
use `origin/release/X.Y` when the PR target is a release branch; update the `git
fetch origin && git rebase origin/dev` occurrence in SKILL.md accordingly.
|
|
||
| - Branch: !`git branch --show-current` | ||
| - Changed files: !`git diff --name-only HEAD~1 2>/dev/null || git diff --name-only` | ||
| - Changed files: !`git diff --name-only origin/dev..HEAD 2>/dev/null || git diff --name-only` |
There was a problem hiding this comment.
Refresh base ref before diffing changed files.
Using origin/dev..HEAD without fetching can miss or misclassify changed packages when the local remote ref is stale, which can skip required tests.
Suggested fix
-- Changed files: !`git diff --name-only origin/dev..HEAD 2>/dev/null || git diff --name-only`
+- Changed files: !`git fetch origin dev --quiet 2>/dev/null || true; git diff --name-only origin/dev...HEAD 2>/dev/null || git diff --name-only`-CHANGED=$(git diff --name-only origin/dev..HEAD 2>/dev/null || git diff --name-only)
+git fetch origin dev --quiet 2>/dev/null || true
+CHANGED=$(git diff --name-only origin/dev...HEAD 2>/dev/null || git diff --name-only)Also applies to: 24-24
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/test/SKILL.md at line 14, The diff command snippet "Changed
files: !`git diff --name-only origin/dev..HEAD 2>/dev/null || git diff
--name-only`" can use a stale origin/dev; before running that command ensure the
base ref is refreshed (e.g., run a fetch) so changed-package detection is
accurate—update the workflow/script to perform git fetch origin (or git fetch
--all --prune) before evaluating the diff command or compare against a freshly
fetched ref (use FETCH_HEAD or an explicit fetched branch) so the "Changed
files" check reliably detects changes.
Comment on lines
+96
to
97
| - **Exception**: when a `release/X.Y` branch is active, branch from and target it instead of `dev`. | ||
| - PRs target `dev` (integration) before merging to `main`. |
There was a problem hiding this comment.
Clarify PR target rule for release branches to avoid contradictory guidance.
The release exception says target release/X.Y, but the next rule still says PRs target dev. Please make the default+exception explicit in one place.
Suggested wording
-- PRs target `dev` (integration) before merging to `main`.
+- PRs target `dev` (integration) before merging to `main`.
+- If working on an active `release/X.Y` branch, PRs target that `release/X.Y` branch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` around lines 96 - 97, Combine the default rule and the release
exception into one clear statement so there is no contradiction: replace the
separate lines containing "**Exception**: when a `release/X.Y` branch is active,
branch from and target it instead of `dev`." and "PRs target `dev` (integration)
before merging to `main`." with a single explicit sentence such as "By default,
open PRs targeting `dev` (integration) before merging to `main`; exception: if a
`release/X.Y` branch is active, branch from and target `release/X.Y` instead of
`dev`." Ensure the phrases "`release/X.Y`" and "`dev` (integration)" remain
unchanged so the rule is unambiguous.
Comment on lines
+171
to
+175
| **Session/Lifecycle:** | ||
| - SessionStart: branch status, PR info, Docker health check | ||
| - Stop: completion checklist (tests run? lint run? screenshots taken?) | ||
| - PreCompact: re-injects critical rules after context compaction | ||
|
|
There was a problem hiding this comment.
The lifecycle hook docs mention phases not present in current hook config.
Stop and PreCompact are documented here, but the provided .claude/settings.json hook config only defines SessionStart for lifecycle hooks. Either add those hooks or remove these bullets to prevent drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` around lines 171 - 175, The docs list lifecycle hooks Stop and
PreCompact that aren't defined in .claude/settings.json while only SessionStart
exists; update either the documentation or the hook config to match: either
remove the Stop and PreCompact bullets from the "Session/Lifecycle" section or
add corresponding lifecycle entries named "Stop" and "PreCompact" to
.claude/settings.json with the intended behavior (e.g., Stop: completion
checklist, PreCompact: re-inject critical rules) so the documented phases align
with the actual SessionStart/Stop/PreCompact hook names.
alfredo1996
added a commit
that referenced
this pull request
May 10, 2026
…nd CLAUDE.md (#675) CLAUDE.md: - Fix SonarQube → SonarCloud reference - Remove stale TESTING_APPROACH.md reference - Add release branch exception to Git & PRs section - Add "Automated Guardrails (Hooks)" section documenting all hook protections - Update Playwright CLI reference to include all 4 browser agents Agents: - project-architect: fix Next.js 15 → 16, /grill → /drill (4 occurrences) - code-reviewer: add CodeRabbit/SonarCloud checking steps, fix vitest → npm test Skills: - code: add mandatory /drill step before implementation - next (autopilot): add /drill step, replace broken npm run lint:fix with correct per-package commands, add E2E test step - test: fix HEAD~1 → origin/dev..HEAD for multi-commit branch detection - pr: add branch conventions, release branch exception, E2E step, fix SonarQube ref - issue: add commit scopes - Remove github-workflow skill (content consolidated into pr + issue skills) Hooks: - New check-migration-guard.sh: blocks edits to existing migration files (forward-only) - session-context.sh: add Docker health check at session start - settings.json: register migration guard hook Co-authored-by: alfredorubin96 <alfredo.rubin@neotechnology.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 tasks
This was referenced Jul 25, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Full audit and cleanup of
.claude/configuration — skills, agents, hooks, andCLAUDE.md. Fixes stale references, strengthens workflow enforcement, reduces redundancy, and documents the hook system.Changes
Stale reference fixes (P0)
/grill→/drillTESTING_APPROACH.mdreference/nextskill: replaced brokennpm run lint:fixwith correct commands/testskill:HEAD~1→origin/dev..HEADfor multi-commit branchesWorkflow enforcement (P1)
/codeand/nextskills now require/drillbefore implementationcheck-migration-guard.shhook: blocks edits to existing migrations (forward-only)session-context.sh: Docker health check at session startDrift & redundancy reduction (P2)
/github-workflowskill — content consolidated into/prand/issuecode-revieweragent: added CodeRabbit + SonarCloud checking steps/prskill: added branch conventions, E2E step, release branch exceptionDocumentation (P3)
Testing
settings.local.jsoncleaned separately (not tracked in git)🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores