-
Notifications
You must be signed in to change notification settings - Fork 0
chore(config): audit and improve Claude Code skills, agents, hooks #675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| #!/bin/bash | ||
| # Hook: Prevent editing existing migration files (forward-only migrations) | ||
| # Rule: "Forward-only. Idempotent." — CLAUDE.md | ||
| INPUT=$(cat) | ||
| FILE_PATH=$(echo "$INPUT" | jq -r '.tool_input.file_path // .tool_input.filePath // empty') | ||
| [ -z "$FILE_PATH" ] && exit 0 | ||
|
|
||
| # Only check migration files | ||
| case "$FILE_PATH" in | ||
| *migrations/*.sql|*migrations/*.ts) | ||
| # Allow creating NEW migration files (Write tool with no existing file) | ||
| TOOL_NAME=$(echo "$INPUT" | jq -r '.tool_name // empty') | ||
| if [ "$TOOL_NAME" = "Write" ] && [ ! -f "$FILE_PATH" ]; then | ||
| exit 0 | ||
| fi | ||
| # Block editing existing migration files | ||
| if [ -f "$FILE_PATH" ]; then | ||
| echo "BLOCKED: Cannot edit existing migration file: $(basename "$FILE_PATH")" >&2 | ||
| echo "Rule: Migrations are forward-only. Create a new migration instead." >&2 | ||
| echo "Use: npm run db:generate" >&2 | ||
| exit 2 | ||
| fi | ||
| ;; | ||
| esac | ||
|
|
||
| exit 0 |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,32 +35,39 @@ git checkout -b <type>/<short-description> | |
|
|
||
| Branch prefix from labels: bug → fix/, enhancement → feat/, security → security/, docs → docs/. | ||
|
|
||
| ## Step 3 — Read the issue and relevant docs | ||
| ## 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 | ||
|
|
||
|
Comment on lines
+38
to
43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move This step says Based on learnings: Run 🤖 Prompt for AI Agents |
||
| Read the full issue body. Check `claude_code_docs/` for relevant context. | ||
| Identify which package(s) are affected: app/, component/, connection/. | ||
|
|
||
| ## Step 4 — Implement | ||
| ## Step 5 — Implement | ||
|
|
||
| Follow all CLAUDE.md rules. Respect package boundaries. | ||
| If building UI, check existing components first (`find component/src -name '*.tsx'`). | ||
|
|
||
| ## Step 5 — Test and lint | ||
| ## 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 | ||
|
Comment on lines
+52
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Step 6 command sequence is brittle due to persistent After 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 🤖 Prompt for AI Agents |
||
| ``` | ||
|
|
||
| Fix any failures. Do not skip. | ||
|
|
||
| ## Step 6 — Commit | ||
| ## Step 7 — Commit | ||
|
|
||
| Use Conventional Commits: `type(scope): description` | ||
| Reference the issue: `Closes #<number>` | ||
|
|
||
| ## Step 7 — Push and create PR | ||
| ## Step 8 — Push and create PR | ||
|
|
||
| ```bash | ||
| git push -u origin HEAD | ||
|
|
@@ -71,7 +78,7 @@ gh pr create \ | |
| --label '<labels from the issue>' | ||
| ``` | ||
|
|
||
| ## Step 8 — Report | ||
| ## Step 9 — Report | ||
|
|
||
| Output: | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,13 +12,20 @@ allowed-tools: Bash(gh *), Bash(git *), Bash(npm *) | |||||||||
| - Commits: !`git log origin/dev..HEAD --oneline 2>/dev/null || echo 'No upstream'` | ||||||||||
| - Changed: !`git diff origin/dev --name-only 2>/dev/null || git diff --name-only` | ||||||||||
|
|
||||||||||
| ## Conventions | ||||||||||
|
|
||||||||||
| - Branch prefixes: `feat/`, `fix/`, `chore/`, `docs/`, `refactor/`, `security/` | ||||||||||
| - Commits: `type(scope): description` | ||||||||||
| - Scopes: app, component, connection, auth, encryption, migration, api, widget, chart | ||||||||||
|
|
||||||||||
| ## 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pre-flight command conflicts with the release-target exception. The text allows targeting 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| 2. `npm run lint` | ||||||||||
| 3. `npm run build` | ||||||||||
| 4. Run tests for affected packages (`cd app && npm test`, `cd component && npm test`) | ||||||||||
| 5. If updating existing PR: `gh pr view <number> --comments` — address CodeRabbit/SonarQube feedback | ||||||||||
| 5. Run E2E if UI changed: `cd app && npx playwright test` | ||||||||||
| 6. If updating existing PR: `gh pr view <number> --comments` — address CodeRabbit/SonarCloud feedback | ||||||||||
|
|
||||||||||
| ## Labels (required: type + package) | ||||||||||
|
|
||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ allowed-tools: Bash(npm *), Bash(npx *), Bash(git *), Bash(cd *) | |
| ## State | ||
|
|
||
| - 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refresh base ref before diffing changed files. Using 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 |
||
|
|
||
| ## Instructions | ||
|
|
||
|
|
@@ -21,7 +21,7 @@ Detect which packages have changes and run the appropriate test suites. | |
|
|
||
| ```bash | ||
| # Check which packages have changes | ||
| CHANGED=$(git diff --name-only HEAD~1 2>/dev/null || git diff --name-only) | ||
| CHANGED=$(git diff --name-only origin/dev..HEAD 2>/dev/null || git diff --name-only) | ||
| RUN_APP=false | ||
| RUN_COMPONENT=false | ||
| RUN_CONNECTION=false | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,7 +47,6 @@ Rules: | |
| - Run the relevant test suite before and after every change to confirm Red → Green. | ||
| - Every new behavior, bug fix, and edge case gets a test. | ||
| - Tests live in `__tests__/` next to the file under test, same package. | ||
| - See `claude_code_docs/TESTING_APPROACH.md` for suite structure, commands, and patterns. | ||
|
|
||
| ## Testing Boundaries (app/ package) | ||
|
|
||
|
|
@@ -94,6 +93,7 @@ Playwright E2E with **server-side coverage collection** (`collectServer: true` i | |
|
|
||
| - Conventional Commits: `type(scope): description`. | ||
| - Branch from `dev`: `feat/issue-<N>-<slug>`, `fix/issue-<N>-<slug>`, `chore/`, etc. | ||
| - **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`. | ||
|
Comment on lines
+96
to
97
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clarify PR target rule for release branches to avoid contradictory guidance. The release exception says target 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 |
||
| - Do not push if tests are failing. | ||
| - PRs need labels: type + package + area. See `/github` skill. | ||
|
|
@@ -103,7 +103,7 @@ Playwright E2E with **server-side coverage collection** (`collectServer: true` i | |
|
|
||
| - Read `gh pr view <number> --comments` when resuming work on an existing PR. | ||
| - Address all CodeRabbit suggestions or dismiss with justification. | ||
| - SonarQube quality gate must pass (coverage, duplications, code smells). | ||
| - SonarCloud quality gate must pass (coverage, duplications, code smells). | ||
|
|
||
| ## Query Safety — DO NOT VIOLATE | ||
|
|
||
|
|
@@ -146,6 +146,33 @@ Includes: SSO, Custom Roles, Connector Labels, Bulk Import, Connector CRUD API, | |
| Forward-only. Idempotent. Advisory lock prevents concurrent runs. | ||
| Test version-skip paths. `--skip-migrations` flag exists for emergency debugging. | ||
|
|
||
| ## Automated Guardrails (Hooks) | ||
|
|
||
| The `.claude/settings.json` hooks enforce critical rules automatically: | ||
|
|
||
| **PreToolUse (Edit/Write):** | ||
| - Package boundary enforcement — blocks cross-package imports | ||
| - Query interpolation guard — blocks `${...}` near SQL/Cypher keywords | ||
| - Credential logging guard — blocks `console.log` of sensitive variables | ||
| - Migration file guard — blocks edits to existing migration files (forward-only) | ||
| - ECharts import guard — blocks `import * from 'echarts'` | ||
| - SSR guard — blocks chart components without `ssr: false` | ||
| - Main branch guard — blocks edits on `main` | ||
|
|
||
| **PreToolUse (Bash):** | ||
| - Dependency install guard — blocks `npm install/uninstall` without approval | ||
| - E2E enforcement — blocks `git commit` if UI files edited but Playwright not run | ||
|
|
||
| **PostToolUse:** | ||
| - Auto-format + lint on every TypeScript file edit | ||
| - E2E marker tracking (marks UI files as needing E2E, clears after playwright runs) | ||
| - Coverage threshold warning after test runs | ||
|
|
||
| **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 | ||
|
|
||
|
Comment on lines
+171
to
+175
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The lifecycle hook docs mention phases not present in current hook config.
🤖 Prompt for AI Agents |
||
| ## Design Review | ||
|
|
||
| Before touching any UI code, read `.claude/skills/design-review/skill.md` — tokens, spacing, typography, color, chart patterns. | ||
|
|
@@ -174,4 +201,4 @@ Agents work together in a pipeline. Each stage gates the next: | |
|
|
||
| ### Playwright CLI (for browser agents) | ||
|
|
||
| `feature-reviewer` and `ux-crawler` use `npx @playwright/cli` to interact with the running app at `http://localhost:3000`. Ensure Docker is running before invoking them. | ||
| `feature-reviewer`, `ux-crawler`, `user-sim-admin`, and `user-sim-creator` use `npx @playwright/cli` to interact with the running app at `http://localhost:3000`. Ensure Docker is running before invoking them. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review flow omits
connection/test verification.Current step can approve PRs with unverified
connection/changes. Add a conditionalcd connection && npm testpath.Suggested fix
🤖 Prompt for AI Agents