From 50ce85d013302da1ec8b886cd9bf3fa06fda1f17 Mon Sep 17 00:00:00 2001 From: Frank Steiler Date: Wed, 8 Jul 2026 01:45:51 +0200 Subject: [PATCH] docs(skills): fix broken board commands, exit propagation, worktree policy, and skill contradictions Fixes #1819 Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) --- .claude/agent-memory/dev-team-lead/MEMORY.md | 1 + .../meta-skill-reconciliation.md | 60 ++++++++++++ .claude/skills/batch-develop/SKILL.md | 42 +++++--- .claude/skills/dependabot/SKILL.md | 10 +- .claude/skills/develop/SKILL.md | 47 +++++---- .claude/skills/epic-close/SKILL.md | 9 +- .claude/skills/epic-run/SKILL.md | 4 +- .claude/skills/epic-start/SKILL.md | 4 +- .claude/skills/fix-e2e/SKILL.md | 8 +- .claude/skills/release/SKILL.md | 96 ++++++++++++++++--- .claude/skills/review-pr/SKILL.md | 23 ++--- CLAUDE.md | 30 +++--- 12 files changed, 246 insertions(+), 88 deletions(-) create mode 100644 .claude/agent-memory/dev-team-lead/meta-skill-reconciliation.md diff --git a/.claude/agent-memory/dev-team-lead/MEMORY.md b/.claude/agent-memory/dev-team-lead/MEMORY.md index 96e822afa..a93ecdba0 100644 --- a/.claude/agent-memory/dev-team-lead/MEMORY.md +++ b/.claude/agent-memory/dev-team-lead/MEMORY.md @@ -19,6 +19,7 @@ You operate in three modes: `[MODE: spec]`, `[MODE: review]`, `[MODE: commit]`. - [Sandbox & worktree environment quirks](sandbox-environment.md) — node_modules corruption fixes, shared-package build order, prettier CWD, git index corruption recovery, gh CLI `--json` gap on `pr checks`, wiki submodule git-identity setup - [Testing patterns (Jest/TS/React)](testing-patterns.md) — ThemeProvider over mocking ThemeContext, JSX.Element typing workaround, dynamic-import timing, matcher misuse, overly-broad absence regexes - [Code patterns confirmed during review](code-patterns.md) — drizzle-orm `sql.join` availability, intentional CSS cross-imports (AutosaveIndicator) +- [Meta-skill reconciliation (issue #1819)](meta-skill-reconciliation.md) — gh project item-add pattern, CLAUDE.md drifts fast, orchestrator has no trailer, worktree cleanup sequence, count-every-occurrence self-check gap ## See Also diff --git a/.claude/agent-memory/dev-team-lead/meta-skill-reconciliation.md b/.claude/agent-memory/dev-team-lead/meta-skill-reconciliation.md new file mode 100644 index 000000000..e8f32d928 --- /dev/null +++ b/.claude/agent-memory/dev-team-lead/meta-skill-reconciliation.md @@ -0,0 +1,60 @@ +--- +name: meta-skill-reconciliation +description: Patterns for auditing/fixing .claude/skills/*.md and CLAUDE.md itself (issue #1819) — gh CLI verification, orchestrator trailer convention, worktree cleanup sequence +metadata: + type: project +--- + +## `gh project` board-status command + +`gh project item-list --owner --query "..."` **does not exist** — `item-list` only takes +`--format/--jq/--limit/--owner/--template`, no `--query`. The correct idempotent pattern (proven +in ~16 real invocations during the 2026-07-06/07 batch-develop session, now the canonical form in +`develop`/`epic-start`/`release` SKILL.md files): + +```bash +ITEM_ID=$(gh project item-add 4 --owner steilerDev --url https://github.com/steilerDev/cornerstone/issues/ --format json --jq '.id') +gh project item-edit --id "$ITEM_ID" --project-id PVT_kwHOAGtLQM4BOlve --field-id PVTSSF_lAHOAGtLQM4BOlvezg9P0yo --single-select-option-id +``` + +`item-add` is idempotent — if the issue is already on the board it just returns the existing +item's ID, no duplicate is created. Always verify `gh --help` directly rather than +trusting a skill file's existing command — this bug shipped in 3 skills for a while. + +## CLAUDE.md drifts fast — verify current state before trusting an issue body's claims + +An issue describing a "defect" can predate a fix that already landed. On 2026-07-07 I found the +worktree-removal policy had **already flipped** (PR #1825, merged same day) and a Canonical Agent +Trailers table had **already been added** (PR #1818) — both assumed-stale by the issue I was +fixing. Always `grep`/`Read` the actual current file before writing the fix, don't just diff +against the issue's quoted snippets. + +## Orchestrator-authored commits/PRs have no `Co-Authored-By` trailer + +CLAUDE.md's Canonical Agent Trailers table lists the 11 named sub-agents only — no `orchestrator` +row. Per the Agent Attribution section ("use that agent's name in the trailer" — implying no +trailer when no agent contributed), orchestrator-only commits (e.g. a `.claude/checklists/` +lessons-learned update) should carry **no** Co-Authored-By line at all. For orchestrator-authored +**PR bodies** specifically (not git trailers), the established precedent — already used correctly +in `fix-e2e` and `dependabot` before this reconciliation — is a `🤖 Generated with [Claude +Code](https://claude.com/claude-code)` footer, not a fabricated `Co-Authored-By: Claude Opus 4.6` +line. Apply this precedent instead of inventing a new convention when you find a stale/fake +trailer. + +## Worktree cleanup sequence (post-#1825 policy) + +CLAUDE.md's Session Isolation section: remove the worktree **before** deleting the branch, both +from the *base* repository, and force-delete (`-D` not `-d`) because this repo squash-merges to +`beta` — squash history fails `-d`'s ancestry check even for genuinely-merged branches. Capture +`CURRENT_BRANCH`/`WORKTREE_PATH` *before* `cd`-ing to the base repo (`git worktree list +--porcelain | awk '/^worktree/{print $2; exit}'` finds the base repo path from any worktree). This +must be the session's last action — the cwd it started from no longer exists afterward. Any skill +that loops `/develop` as a sub-routine (`batch-develop`, `epic-run`, `release` step 4e) must +explicitly exclude this cleanup mid-loop, or the first loop iteration terminates the whole batch. + +## Self-check gap: count *every* occurrence a fix pattern gets applied + +When a spec says "apply fix X to every skill that does Y," explicitly enumerate the file list +before writing the verification grep's expected count — I undercounted (said 3, actual was 4) +because I forgot my own spec item added a 4th occurrence (batch-develop's new Session Cleanup +step) beyond the 3 I was tracking from the original bug report's file list. diff --git a/.claude/skills/batch-develop/SKILL.md b/.claude/skills/batch-develop/SKILL.md index 5b30994ab..aa7ede6b5 100644 --- a/.claude/skills/batch-develop/SKILL.md +++ b/.claude/skills/batch-develop/SKILL.md @@ -1,13 +1,13 @@ --- name: batch-develop -description: 'Reads /tmp/notes.md and runs /develop for each line sequentially. Each line gets its own branch, PR, and full development cycle. Skips /develop step 1 (Rebase) since it handles its own baseline reset.' +description: 'Reads /tmp/batch-queue.md and runs /develop for each line sequentially. Each line gets its own branch, PR, and full development cycle. Skips /develop step 1 (Rebase) since it handles its own baseline reset.' --- # Batch Develop — Sequential Development from Notes File -You are the orchestrator running a sequential development pipeline. This skill accepts a list of GitHub issue numbers (as arguments) or reads `/tmp/notes.md`, and invokes the `/develop` workflow for each item, one at a time. Each item gets its own branch, PR, and full development cycle. +You are the orchestrator running a sequential development pipeline. This skill accepts a list of GitHub issue numbers (as arguments) or reads `/tmp/batch-queue.md`, and invokes the `/develop` workflow for each item, one at a time. Each item gets its own branch, PR, and full development cycle. -**When to use:** Processing a backlog of issues or bug descriptions listed in `/tmp/notes.md`, where each item should be developed independently. +**When to use:** Processing a backlog of issues or bug descriptions listed in `/tmp/batch-queue.md`, where each item should be developed independently. **When NOT to use:** When items should be batched into a single PR (use `/develop @/tmp/notes.md` instead). When running a full epic lifecycle (use `/epic-run`). ## Input @@ -15,11 +15,11 @@ You are the orchestrator running a sequential development pipeline. This skill a `$ARGUMENTS` determines the input source: - **Issue list**: `$ARGUMENTS` contains issue numbers (e.g., `#741 #742 #743` or `741, 742, 743` or `#741 #742`). Any combination of `#N` or bare `N` separated by spaces, commas, or semicolons. -- **File mode** (no arguments): When `$ARGUMENTS` is empty, falls back to reading `/tmp/notes.md`. If the file does not exist or is empty, ask the user to provide issue numbers or create the file before proceeding. +- **File mode** (no arguments): When `$ARGUMENTS` is empty, falls back to reading `/tmp/batch-queue.md`. If the file does not exist or is empty, ask the user to provide issue numbers or create the file before proceeding. -### File Format (when using `/tmp/notes.md`) +### File Format (when using `/tmp/batch-queue.md`) -`/tmp/notes.md` contains one item per line: +`/tmp/batch-queue.md` contains one item per line: - **Issue number**: `#42` or `42` - **Bug/feature description**: Free-text description @@ -32,7 +32,7 @@ Use tasks to track progress across the batch. Tasks survive context compression **Create these tasks upfront** (using `TaskCreate`): -1. **Read and parse queue** — Parse input arguments or /tmp/notes.md into items queue +1. **Read and parse queue** — Parse input arguments or /tmp/batch-queue.md into items queue 2. **Session setup** — Fetch latest beta, sync wiki **After parsing** — create one task per item in the queue: @@ -53,7 +53,7 @@ Use tasks to track progress across the batch. Tasks survive context compression **If `$ARGUMENTS` contains issue numbers:** Extract all issue numbers from the arguments (strip `#` prefixes, commas, semicolons). Each number becomes an item in the queue. -**If `$ARGUMENTS` is empty:** Read `/tmp/notes.md`. Parse each non-empty, non-comment line into an ordered **items queue**. +**If `$ARGUMENTS` is empty:** Read `/tmp/batch-queue.md`. Parse each non-empty, non-comment line into an ordered **items queue**. Print the queue: @@ -98,14 +98,14 @@ git checkout -B batch-dev-temp origin/beta Invoke the `/develop` skill with the current item as `$ARGUMENTS`. The `/develop` skill handles the full cycle (steps 1–11): resolve issue → visual spec → branch → move to in progress → implement + test → verify PR → review → fix loop → merge → close. -**Note:** Skip `/develop` step 1 (Rebase) — the baseline reset is already handled by step 3a above. Execute `/develop` steps 2–11 for each item. +**Note:** Skip `/develop` step 1 (Rebase) — the baseline reset is already handled by step 3a above. Execute `/develop` steps 2–11 for each item, **except step 11's worktree/branch cleanup item** — the batch session must stay in the same worktree to process the next item. Do not let `/develop` run its worktree removal mid-batch; that step only runs once, after the entire queue is processed (see step 5, Session Cleanup, below). Per-item local branches are left in place during the loop rather than individually deleted — this avoids the checkout-conflict fragility of deleting a branch that a subsequent iteration's `git checkout -B` may still be touching, at the cost of leaving a handful of merged local branches for the final cleanup (or a manual `git branch --merged beta` sweep) to catch. #### 3c. Track Result After `/develop` completes for the item: -- **Success** (PR merged, issue closed): Add to completed list. If in file mode, remove the line from `/tmp/notes.md`. -- **Failure** (retry budget exhausted or error): Add to failed list, continue to next item. If in file mode, keep the line in `/tmp/notes.md`. +- **Success** (PR merged, issue closed): Add to completed list. If in file mode, remove the line from `/tmp/batch-queue.md`. +- **Failure** (retry budget exhausted or error): Add to failed list, continue to next item. If in file mode, keep the line in `/tmp/batch-queue.md`. #### 3d. Progress Update @@ -125,7 +125,23 @@ Batch Develop Complete Total items: N Completed: N — #42, #55, #61 Failed: N — #88 (reason) -Remaining in /tmp/notes.md: N lines +Remaining in /tmp/batch-queue.md: N lines ``` -In file mode: if all items succeeded, `/tmp/notes.md` will be empty (or contain only comments). If any failed, the file retains those lines for a future run. +In file mode: if all items succeeded, `/tmp/batch-queue.md` will be empty (or contain only comments). If any failed, the file retains those lines for a future run. + +### 5. Session Cleanup + +After the batch completes (all items processed, summary printed), clean up the worktree per CLAUDE.md's Session Isolation policy — the batch-deferred equivalent of `/develop` step 11's cleanup item: + +```bash +git status --porcelain # must be empty before proceeding +CURRENT_BRANCH=$(git branch --show-current) +WORKTREE_PATH=$(pwd) +BASE_REPO=$(git worktree list --porcelain | awk '/^worktree/{print $2; exit}') +cd "$BASE_REPO" +git worktree remove "$WORKTREE_PATH" +git branch -D "$CURRENT_BRANCH" +``` + +**Skip this step** if any items in the batch failed and remain unresolved in the queue file, or if the worktree has uncommitted changes — leave it in place for manual follow-up in that case. diff --git a/.claude/skills/dependabot/SKILL.md b/.claude/skills/dependabot/SKILL.md index 74b3ad369..7d49c2de0 100644 --- a/.claude/skills/dependabot/SKILL.md +++ b/.claude/skills/dependabot/SKILL.md @@ -194,17 +194,13 @@ Each agent receives: - PR number and URL - The full failed-check excerpt - The `BREAKING` findings from step 3 (if any) -- Explicit instruction to commit on the current branch (the Dependabot branch) and NOT to push — the orchestrator pushes after committing. +- Explicit instruction to only edit files — per CLAUDE.md's flat delegation model, implementation agents never run `git commit` or `git push` themselves. Staging, committing, and pushing are handled exclusively by `dev-team-lead [MODE: commit]` in step 5d. #### 5d. Commit & push -Use `dev-team-lead [MODE: commit]` to stage, write the conventional commit message with all contributing agent trailers, and push. Per CLAUDE.md trailer-verification rules, every production-file change must carry the correct `Co-Authored-By` trailer. +Launch `dev-team-lead [MODE: commit]` to stage the changes, write the conventional commit message with all contributing agent trailers, and push — this is the sole commit/push action for this PR; do not run a separate `git push`. Per CLAUDE.md trailer-verification rules, every production-file change must carry the correct `Co-Authored-By` trailer. -```bash -git push -``` - -(No `-u` — the branch already tracks the Dependabot remote ref.) +This PR already exists (it _is_ the Dependabot PR) — explicitly instruct dev-team-lead to **skip PR creation** in its `[MODE: commit]` protocol (there is no new PR to open) and to push without `-u` (the branch already tracks the Dependabot remote ref). #### 5e. Wait for CI diff --git a/.claude/skills/develop/SKILL.md b/.claude/skills/develop/SKILL.md index 402049140..98b0b5ca5 100644 --- a/.claude/skills/develop/SKILL.md +++ b/.claude/skills/develop/SKILL.md @@ -157,7 +157,7 @@ Move the issue(s) to **In Progress** on the Projects board. #### Single-item mode ```bash -ITEM_ID=$(gh project item-list 4 --owner steilerDev --format json --limit 1 --query "is:issue #" --jq '.items[0].id') +ITEM_ID=$(gh project item-add 4 --owner steilerDev --url https://github.com/steilerDev/cornerstone/issues/ --format json --jq '.id') gh project item-edit --id "$ITEM_ID" --project-id PVT_kwHOAGtLQM4BOlve --field-id PVTSSF_lAHOAGtLQM4BOlvezg9P0yo --single-select-option-id 296eeabe ``` @@ -274,10 +274,11 @@ git log origin/beta..HEAD --format="%b" If production files were changed (`git diff --name-only origin/beta..HEAD | grep -E '^(server|client|shared)/'`), verify the commit body contains the appropriate Co-Authored-By trailers: -- Files under `server/` or `shared/` → must have `Co-Authored-By: Claude backend-developer (Haiku` -- Files under `client/` (except `client/src/i18n/de/` and `client/src/i18n/glossary.json`) → must have `Co-Authored-By: Claude frontend-developer (Haiku` +- Files under `server/` or `shared/`, excluding `*.test.ts`/`*.test.tsx` → must have `Co-Authored-By: Claude backend-developer (Haiku` +- Files under `client/` (except `client/src/i18n/de/`, `client/src/i18n/glossary.json`, and `*.test.ts`/`*.test.tsx`) → must have `Co-Authored-By: Claude frontend-developer (Haiku` - Files under `client/src/i18n/de/` or `client/src/i18n/glossary.json` → must have `Co-Authored-By: Claude translator (Sonnet` - Files under `e2e/` → must have `Co-Authored-By: Claude e2e-test-engineer (Sonnet` +- Files matching `*.test.ts` or `*.test.tsx` outside `e2e/` → must have `Co-Authored-By: Claude qa-integration-tester (Sonnet` If trailers are missing, the dev-team-lead missed an agent in the contributing list. Re-launch `[MODE: commit]` with the corrected list. @@ -299,7 +300,7 @@ Fixes # - [ ] Integration tests pass - [ ] Pre-commit hook quality gates pass -Co-Authored-By: Claude Opus 4.6 +🤖 Generated with [Claude Code](https://claude.com/claude-code) EOF )" ``` @@ -334,7 +335,7 @@ Fixes #61 - [ ] Integration tests pass - [ ] Pre-commit hook quality gates pass -Co-Authored-By: Claude Opus 4.6 +🤖 Generated with [Claude Code](https://claude.com/claude-code) EOF )" ``` @@ -430,20 +431,23 @@ After merge: ``` 2. Move the issue to **Done** on the Projects board: ```bash - ITEM_ID=$(gh project item-list 4 --owner steilerDev --format json --limit 1 --query "is:issue #" --jq '.items[0].id') + ITEM_ID=$(gh project item-add 4 --owner steilerDev --url https://github.com/steilerDev/cornerstone/issues/ --format json --jq '.id') gh project item-edit --id "$ITEM_ID" --project-id PVT_kwHOAGtLQM4BOlve --field-id PVTSSF_lAHOAGtLQM4BOlvezg9P0yo --single-select-option-id c558f50d ``` 3. **Remove resolved line from source file** (only when input was a `@`-prefixed file path): - Remove the line from the source file that produced the resolved item (matched by original text). - Preserve comments (`#`-prefixed lines) and empty lines. -4. Clean up the branch: - ``` - git checkout beta && git pull && git branch -d - ``` -5. Exit the session and remove the worktree: - ``` - /exit +4. Clean up the worktree and branch, per CLAUDE.md's Session Isolation policy (only once the PR is merged and the worktree has no uncommitted changes): + ```bash + git status --porcelain # must be empty before proceeding + CURRENT_BRANCH=$(git branch --show-current) + WORKTREE_PATH=$(pwd) + BASE_REPO=$(git worktree list --porcelain | awk '/^worktree/{print $2; exit}') + cd "$BASE_REPO" + git worktree remove "$WORKTREE_PATH" + git branch -D "$CURRENT_BRANCH" ``` + Use `-D` (force), not `-d` — this repo squash-merges to `beta`, so `-d`'s "fully merged" check will refuse even for genuinely merged branches. This must be the last action of the session (the working directory it started from no longer exists afterward). #### Multi-item mode @@ -456,11 +460,14 @@ After merge: - For each closed issue, remove the line from the source file that produced it (matched by original text — the issue number or description as it appeared in the file). - Preserve comments (`#`-prefixed lines) and empty lines that were not part of the resolved items. - If all non-comment, non-empty lines have been removed, leave the file with only its comments (or empty). -4. Clean up the branch: - ``` - git checkout beta && git pull && git branch -d - ``` -5. Exit the session and remove the worktree: - ``` - /exit +4. Clean up the worktree and branch, per CLAUDE.md's Session Isolation policy (only once the PR is merged and the worktree has no uncommitted changes): + ```bash + git status --porcelain # must be empty before proceeding + CURRENT_BRANCH=$(git branch --show-current) + WORKTREE_PATH=$(pwd) + BASE_REPO=$(git worktree list --porcelain | awk '/^worktree/{print $2; exit}') + cd "$BASE_REPO" + git worktree remove "$WORKTREE_PATH" + git branch -D "$CURRENT_BRANCH" ``` + Use `-D` (force), not `-d` — this repo squash-merges to `beta`, so `-d`'s "fully merged" check will refuse even for genuinely merged branches. This must be the last action of the session (the working directory it started from no longer exists afterward). diff --git a/.claude/skills/epic-close/SKILL.md b/.claude/skills/epic-close/SKILL.md index ec5f49bda..feef7899a 100644 --- a/.claude/skills/epic-close/SKILL.md +++ b/.claude/skills/epic-close/SKILL.md @@ -5,7 +5,7 @@ description: 'Close an epic: refinement, E2E validation, UAT, documentation, and # Epic Close — Refinement, UAT & Promotion Workflow -You are the orchestrator running the closing phase for a completed epic. Follow these 13 steps in order. **Do NOT skip steps.** The orchestrator delegates all work — never write production code, tests, or architectural artifacts directly. +You are the orchestrator running the closing phase for a completed epic. Follow these 8 steps in order. **Do NOT skip steps.** The orchestrator delegates all work — never write production code, tests, or architectural artifacts directly. **When to use:** After all user stories in an epic have been merged to `beta` and are closed. This skill handles refinement, E2E validation, UAT, documentation, and promotion to `main`. **When NOT to use:** Planning a new epic (use `/epic-start`). Implementing a single story or bug fix (use `/develop`). @@ -60,13 +60,10 @@ If any story is still open, stop and inform the user. All stories must be comple ### 2a. Lint Health Check -Check the most recent auto-fix workflow run for unfixable lint issues: +There is no CI job that runs lint (`ci.yml`'s Quality Gates covers typecheck + test + build + audit only; lint cleanliness is enforced per-PR by implementing agents and dev-team-lead's review per CLAUDE.md's Local Validation Policy). Run a full-repo lint pass directly to catch any cumulative drift across the epic's merged PRs: ```bash -# Get the latest auto-fix run ID -RUN_ID=$(gh run list --workflow=auto-fix.yml --limit=1 --json databaseId --jq '.[0].databaseId') -# Extract lint errors and warnings -gh run view "$RUN_ID" --log 2>/dev/null | grep -E '##\[(warning|error)\]' | grep -v 'Process completed' +npm run lint ``` If there are unfixable lint errors or warnings, include them in the refinement items (step 3). These should be addressed in the refinement PR alongside any review observations. diff --git a/.claude/skills/epic-run/SKILL.md b/.claude/skills/epic-run/SKILL.md index 07402ad08..41358119e 100644 --- a/.claude/skills/epic-run/SKILL.md +++ b/.claude/skills/epic-run/SKILL.md @@ -111,7 +111,7 @@ Specifically: 2. **Execute `/epic-start` step 4** (Plan: Product Architect) — launches the architect agent to design schema/API/ADRs and update wiki pages. -3. **Execute `/epic-start` step 5** (Present to User) — post the plan as a comment on the epic issue and present it to the user. Skip the step 6 handoff instructions (the user is not invoking `/develop` manually — Phase 2 handles it). +3. **Execute `/epic-start` step 5** (Present to User) — post the plan as a comment on the epic issue and present it to the user. **Do not wait for explicit approval here** — unlike standalone `/epic-start`, `/epic-run` is autonomous end-to-end and only pauses at the `beta` → `main` promotion gate (see `/release` step 4). Proceed immediately to Build Story Queue. Skip the step 6 handoff instructions (the user is not invoking `/develop` manually — Phase 2 handles it). ### 1.4 Build Story Queue @@ -169,7 +169,7 @@ Execute `/develop` steps 2 through 11 for the current story, using **single-item - **Step 8** (Review) — launch 4 reviewer agents in parallel - **Step 9** (Fix Loop) — fix loop if reviewers flag blocking issues - **Step 10** (Merge) — wait for CI, present summary, squash merge -- **Step 11** (Close Issues & Clean Up) — close issue, move to Done on board. **Skip the branch cleanup and `/exit`** — the session continues with the next story. +- **Step 11** (Close Issues & Clean Up) — close issue, move to Done on board. **Skip step 11's worktree/branch cleanup item** — the session continues with the next story. After step 11 completes, add the story to `completedStories`. diff --git a/.claude/skills/epic-start/SKILL.md b/.claude/skills/epic-start/SKILL.md index 0b380dbef..e3e61524e 100644 --- a/.claude/skills/epic-start/SKILL.md +++ b/.claude/skills/epic-start/SKILL.md @@ -61,7 +61,7 @@ Launch the **product-owner** agent to: - Set board statuses: **Backlog** for future-sprint stories, **Todo** for first-sprint stories: ```bash - ITEM_ID=$(gh project item-list 4 --owner steilerDev --format json --limit 1 --query "is:issue #" --jq '.items[0].id') + ITEM_ID=$(gh project item-add 4 --owner steilerDev --url https://github.com/steilerDev/cornerstone/issues/ --format json --jq '.id') # Move to Todo (dc74a3b0) or Backlog (7404f88c) gh project item-edit --id "$ITEM_ID" --project-id PVT_kwHOAGtLQM4BOlve --field-id PVTSSF_lAHOAGtLQM4BOlvezg9P0yo --single-select-option-id dc74a3b0 @@ -90,7 +90,7 @@ Present the complete epic plan to the user: - **Architecture**: Summary of schema changes, new API endpoints, ADRs created - **Sprint plan**: Which stories are in the first sprint (Todo) vs backlog -Post the plan as a comment on the epic GitHub Issue and proceed immediately to step 6. +Post the plan as a comment on the epic GitHub Issue. **Wait for the user to review and explicitly approve the plan before proceeding to step 6** — do not advance automatically. ### 6. Handoff diff --git a/.claude/skills/fix-e2e/SKILL.md b/.claude/skills/fix-e2e/SKILL.md index bb52b7be6..5f56128a6 100644 --- a/.claude/skills/fix-e2e/SKILL.md +++ b/.claude/skills/fix-e2e/SKILL.md @@ -116,12 +116,14 @@ Provide each agent with: #### 4a. Commit -Ensure the branch name follows conventions. If on a worktree branch, rename it: +Ensure the branch name follows conventions. This skill's input is a CI run URL/ID, not an issue — there is no issue number to slot into the standard `/-` pattern. Use the run ID in its place (mirroring the precedent in `/dependabot` step 6d, which substitutes a GHSA ID for the same reason): ```bash -git branch -m fix/-e2e-fixes +git branch -m fix/e2e-- ``` +Where `` is a 2–4 word kebab-case summary of the dominant failure category identified in step 1 (e.g., `fix/e2e-48213021-gantt-selector`). + Commit with appropriate trailers based on which agents contributed: ```bash @@ -145,7 +147,7 @@ git push -u origin If no PR exists, create one targeting `beta`: ```bash -gh pr create --title "fix(e2e): resolve failing E2E tests" --body "$(cat <<'EOF' +gh pr create --base beta --title "fix(e2e): resolve failing E2E tests" --body "$(cat <<'EOF' ## Summary - diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index ea2759f54..68f51107e 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -77,7 +77,63 @@ Use the epic-enriched body provided by the calling skill — it includes stories ```bash gh pr create --base main --head beta --title "release: promote epic # to main" --body "$(cat <<'EOF' - +## Release Summary + + + +## Epic + +Closes # — + +## Stories Completed + +- **#** — +- ... + +## Changes + +### Features +- + +### Fixes +- + +### Chores / Refactoring +- + +## Change Inventory + +### Backend (`server/`, `shared/`) + + +### Frontend (`client/`) + + +### E2E Tests (`e2e/`) + + +### Docs / Config + + +## Refinement Summary + + +## E2E Validation Summary + + +## Security Findings Summary + + +## UAT Validation Checklist + + +- [ ] +- [ ] +- ... + +## Testing +- **DockerHub beta image**: `docker pull steilerdev/cornerstone:beta` +- **PR-specific image**: `docker pull steilerdev/cornerstone:pr-` EOF )" ``` @@ -198,7 +254,7 @@ Launch the **product-owner** agent to: For each group of issues from 4d: 1. Create a fresh branch from `origin/beta`: `git checkout -B fix/- origin/beta` -2. Execute `/develop` steps 2-11 (skipping step 1 Rebase and step 4 Branch — branch is already created) +2. Execute `/develop` steps 2-11 (skipping step 1 Rebase and step 4 Branch — branch is already created), **and skip step 11's worktree/branch cleanup item** — the release session continues to process further fix groups (or step 4f) in the same worktree. The worktree is cleaned up once, at the very end of the whole `/release` flow, in step 7. 3. **Fix batches MUST go through the standard review pipeline.** Launch at minimum: - `product-architect` review - `product-owner` review (if any items are user-story-adjacent or touch acceptance criteria) @@ -263,17 +319,26 @@ Update the implementation checklist with patterns learned: - `ux-designer/MEMORY.md` — recurring token/pattern violations - `product-architect/MEMORY.md` — recurring architecture deviations 2. Identify any new recurring patterns that are NOT yet in `.claude/checklists/implementation-checklist.md` -3. If new patterns found, add them to the checklist and commit: +3. If new patterns found, commit them via a dedicated PR — **never push directly to `beta`** (per CLAUDE.md's Branching Strategy): ```bash + git fetch origin beta + git checkout -b chore/lessons-learned- origin/beta git add .claude/checklists/implementation-checklist.md - git commit -m "chore: update implementation checklist with lessons learned - - Co-Authored-By: Claude Opus 4.6 " - git push + git commit -m "chore: update implementation checklist with lessons learned" + git push -u origin chore/lessons-learned- + gh pr create --base beta --title "chore: update implementation checklist with lessons learned" --body "$(cat <<'EOF' + ## Summary + - Add recurring patterns surfaced during this release's reviews to the implementation checklist + + 🤖 Generated with [Claude Code](https://claude.com/claude-code) + EOF + )" ``` -4. If no new patterns, skip the commit + Wait for the beta CI gate (**CI Gate Polling** pattern from CLAUDE.md, beta variant), then squash-merge: `gh pr merge --squash `. Return to the original release branch/worktree afterward. + +4. If no new patterns, skip this step entirely. ### 7. Merge & Post-Merge @@ -290,10 +355,17 @@ After user approval: 3. **If epic context is provided**, close the epic issue and move to Done on the Projects board: ```bash gh issue close - ITEM_ID=$(gh project item-list 4 --owner steilerDev --format json --limit 1 --query "is:issue #" --jq '.items[0].id') + ITEM_ID=$(gh project item-add 4 --owner steilerDev --url https://github.com/steilerDev/cornerstone/issues/ --format json --jq '.id') gh project item-edit --id "$ITEM_ID" --project-id PVT_kwHOAGtLQM4BOlve --field-id PVTSSF_lAHOAGtLQM4BOlvezg9P0yo --single-select-option-id c558f50d ``` -4. Exit the session and remove the worktree: - ``` - /exit +4. Clean up the worktree and branch, per CLAUDE.md's Session Isolation policy (only once the promotion PR is merged and the worktree has no uncommitted changes): + ```bash + git status --porcelain # must be empty before proceeding + CURRENT_BRANCH=$(git branch --show-current) + WORKTREE_PATH=$(pwd) + BASE_REPO=$(git worktree list --porcelain | awk '/^worktree/{print $2; exit}') + cd "$BASE_REPO" + git worktree remove "$WORKTREE_PATH" + git branch -D "$CURRENT_BRANCH" ``` + Use `-D`, not `-d` — squash/merge-commit history on this branch may not satisfy `-d`'s ancestry check. This must be the final action of the session. diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md index e6270c312..0eb8ee5e2 100644 --- a/.claude/skills/review-pr/SKILL.md +++ b/.claude/skills/review-pr/SKILL.md @@ -5,7 +5,7 @@ description: 'Comprehensive PR review using the full agent team. Reviews for dup # Review PR — External & Internal PR Review Workflow -You are the orchestrator running a comprehensive review of a pull request using the full agent team. Follow these 7 steps in order. **Do NOT skip steps.** The orchestrator delegates all reviews — never post review comments directly. The orchestrator reads the diff independently to produce a behavior change summary. +You are the orchestrator running a comprehensive review of a pull request using the full agent team. Follow these 6 steps in order. **Do NOT skip steps.** The orchestrator delegates all reviews — never post review comments directly. The orchestrator reads the diff independently to produce a behavior change summary. **When to use:** Reviewing any PR — external contributions, Dependabot PRs, or re-reviewing after contributor pushes fixes. **When NOT to use:** PRs created by the `/develop` skill (those are reviewed inline during development). @@ -66,7 +66,7 @@ Launch all applicable review agents simultaneously. Each agent posts its own `gh - **product-architect** — architecture compliance, API contract adherence, schema conventions, naming conventions, dependency policy - **security-engineer** — **conditional**: only launch if the PR touches security-relevant files. Launch if changed files match ANY of: `server/src/routes/**`, `server/src/plugins/auth*`, `server/src/plugins/session*`, `Dockerfile`, `docker-compose.yml`, `**/package.json`, `**/package-lock.json`, or any path containing `sql`, `crypto`, `cookie`, `session`, `token`, `auth`, or `secret`. Skip for frontend-only, test-only, docs-only, or CSS-only PRs. -- **dev-team-lead** `[MODE: review]` — code quality, TypeScript strictness, ESM conventions, consistent-type-imports, test co-location +- **dev-team-lead** `[MODE: review]` — code quality, TypeScript strictness, ESM conventions, consistent-type-imports, test co-location. **Does not post its own `gh pr review`** — per its defined interface it has no GitHub write access in this mode and returns `VERDICT: APPROVED` or `VERDICT: CHANGES_REQUIRED` as plain text to the orchestrator (handled in Step 4 below). **Conditional launches (based on affected areas from Step 1):** @@ -79,34 +79,35 @@ Each agent receives: - The PR number and URL - The list of changed files relevant to their domain -- Instruction to post a `gh pr review` with their findings + +Each agent **except dev-team-lead** receives instruction to post its own `gh pr review` with its findings. dev-team-lead instead returns its `VERDICT` text directly to the orchestrator — it never calls `gh pr review`. ### 4. Aggregate Reviews -After all agents complete, fetch all reviews: +After all agents complete, fetch all posted reviews: ```bash gh api repos/steilerDev/cornerstone/pulls//reviews ``` -Parse each review to determine the agent's verdict (`approve`, `request-changes`, or `comment`) and summarize their findings. +Parse each posted review to determine the agent's verdict (`approve`, `request-changes`, or `comment`) and summarize their findings. Separately, take the `VERDICT` text `dev-team-lead` returned directly (it does not post to GitHub) and map it into the same vocabulary: `VERDICT: APPROVED` → `approve`, `VERDICT: CHANGES_REQUIRED` → `request-changes`. -**Determine overall verdict:** +**Determine overall verdict** across both the posted reviews and dev-team-lead's mapped verdict: -- **BLOCK**: any agent posted `request-changes` -- **APPROVE**: all agents approved or commented +- **BLOCK**: any agent (including dev-team-lead's mapped verdict) is effectively `request-changes` +- **APPROVE**: all posted reviews are `approve`/`comment` AND dev-team-lead returned `VERDICT: APPROVED` ### 5. Verdict Action #### If BLOCK -Post a consolidated `gh pr review --request-changes` comment on the PR listing all blocking findings grouped by agent. Include specific file references and remediation guidance from each agent's review. +Post a consolidated `gh pr review --request-changes` comment on the PR listing all blocking findings grouped by agent. Include specific file references and remediation guidance from each agent's review. Include dev-team-lead's findings (from its returned VERDICT text in Step 4) in this consolidated review, grouped alongside the other agents' findings. Present the blocking findings to the user. **Do NOT wait for CI.** #### If APPROVE -Post a consolidated `gh pr review --approve` comment on the PR summarizing the review outcome. +Post a consolidated `gh pr review --approve` comment on the PR summarizing the review outcome. Include dev-team-lead's findings (from its returned VERDICT text in Step 4) in this consolidated review, grouped alongside the other agents' findings. **Wait 5 seconds**, then check mergeability: `gh pr view --repo steilerDev/cornerstone --json mergeable -q '.mergeable'`. **Only continue if `MERGEABLE`.** If `CONFLICTING`, report the conflict to the user — do not attempt to resolve. Once mergeability is confirmed, wait for CI using the **CI Gate Polling** pattern from `CLAUDE.md` (use the beta or main variant based on the PR's target branch). @@ -118,7 +119,7 @@ Present to the user: 1. **Agent Review Summary** — the table from Step 4 2. **Independent Behavior Change Summary** — read the diff stored from Step 1 and describe what actually changed in user-visible terms, independent of the PR description or changelog. Flag any discrepancies between what the PR claims to do and what the code actually does. -3. **CI Status** — pass/fail for each required check (`Quality Gates`, `Docker`, `Merge E2E Reports`), or "skipped" if review was blocked +3. **CI Status** — pass/fail for the required gate check(s) per CLAUDE.md: `Quality Gates` (required on both `beta`- and `main`-targeted PRs), plus `E2E Gates` (required only when the PR targets `main`). `Docker`, `Docker PR Release`, and `Merge E2E Reports` run but are not required gates — report them only if directly relevant, not as blocking checks. "Skipped" if review was blocked. 4. **Overall Verdict** — `APPROVED` or `BLOCKED` with specific next steps: - If approved: user can merge at their discretion (`gh pr merge --squash `) - If blocked: list what the contributor needs to fix, suggest re-running `/review-pr ` after fixes are pushed diff --git a/CLAUDE.md b/CLAUDE.md index 6082b6424..3b3b7cda5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -78,19 +78,24 @@ The GitHub Projects board uses 5 statuses: Backlog, Todo, In Progress, Done, Won The orchestrator uses the following skills to drive work. Each skill contains the full operational checklist with exact commands and agent coordination. -| Skill | Purpose | Input | -| ------------- | -------------------------------------------------------------------------- | --------------------------------------------------------------- | -| `/epic-start` | Planning: PO creates stories, architect designs schema/API/ADRs | Epic description or issue number | -| `/develop` | Full dev cycle for one or more stories/bug fixes, bundled into a single PR | Issue number, description, semicolon-separated list, or `@file` | -| `/epic-close` | Refinement, E2E validation, UAT, then delegates to `/release` | Epic issue number | -| `/release` | Promote `beta` to `main`: sync, PR, CI, approval loop, docs, merge | Optional epic issue number (standalone if omitted) | -| `/epic-run` | Autonomous end-to-end epic: plan, develop all stories, close | Epic description or issue number | - -See the skill files (`.claude/skills/`) for the full operational checklists. The typical lifecycle is: `/epic-start` (once per epic) → `/develop` (once per story, or batched for multiple small items) → `/epic-close` (once per epic after all stories merged). Alternatively, `/epic-run` chains all three phases in a single session (only pauses for promotion approval). Use `/release` standalone to promote `beta` to `main` without a prior epic definition. +| Skill | Purpose | Input | +| ---------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------- | +| `/epic-start` | Planning: PO creates stories, architect designs schema/API/ADRs | Epic description or issue number | +| `/develop` | Full dev cycle for one or more stories/bug fixes, bundled into a single PR | Issue number, description, semicolon-separated list, or `@file` | +| `/epic-close` | Refinement, E2E validation, UAT, then delegates to `/release` | Epic issue number | +| `/release` | Promote `beta` to `main`: sync, PR, CI, approval loop, docs, merge | Optional epic issue number (standalone if omitted) | +| `/epic-run` | Autonomous end-to-end epic: plan, develop all stories, close | Epic description or issue number | +| `/batch-develop` | Sequential development from a list/file: **each item gets its own branch and PR** (not bundled — contrast with `/develop`'s multi-item mode, which bundles into one PR) | Issue number list, or falls back to `/tmp/batch-queue.md` | +| `/mini-epic` | Analyze a spec, decompose into 2–6 work items, challenge assumptions with the user, hand off to `/batch-develop` | Inline spec, `@file`, or issue number | +| `/dependabot` | Process every open Dependabot PR and security alert: changelog review, merge, fix, remediate orphans, file adoption follow-ups | None (always processes the full queue) | +| `/fix-e2e` | Iteratively analyze and fix failing E2E tests from a CI run until all shards pass | GitHub Actions run URL or ID | +| `/review-pr` | Comprehensive full-team review of a PR not created by `/develop` (external contributions, Dependabot, re-reviews) | PR number | + +See the skill files (`.claude/skills/`) for the full operational checklists. The typical lifecycle is: `/epic-start` (once per epic) → `/develop` (once per story, or batched for multiple small items) → `/epic-close` (once per epic after all stories merged). Alternatively, `/epic-run` chains all three phases in a single session (only pauses for promotion approval). Use `/release` standalone to promote `beta` to `main` without a prior epic definition. `/mini-epic` and `/batch-develop` are the mid-size workflow for cohesive multi-item work that doesn't warrant full epic planning. `/dependabot`, `/fix-e2e`, and `/review-pr` are maintenance/support workflows invoked on demand. ### Acceptance & Validation -Every epic has two phases: **Development** (`/develop`) where QA and E2E write and run tests for each story and the applicable PR reviewers approve per the **PR Review Gate** below; and **Epic validation** (`/epic-close`) where E2E coverage is confirmed and UAT runs before promotion. The only human gate is `beta` → `main` — the user approves after reviewing the change inventory. Feedback goes to `/tmp/notes.md`; fixes loop autonomously until approved. +Every epic has two phases: **Development** (`/develop`) where QA and E2E write and run tests for each story and the applicable PR reviewers approve per the **PR Review Gate** below; and **Epic validation** (`/epic-close`) where E2E coverage is confirmed and UAT runs before promotion. The only human gate is `beta` → `main` — the user approves after reviewing the change inventory. Feedback goes to `/tmp/notes.md`; fixes loop autonomously until approved. This is the release/promotion feedback channel specifically — the `/batch-develop` work queue uses a separate file, `/tmp/batch-queue.md`, to avoid the two protocols colliding on one path. ### Key Rules @@ -120,10 +125,11 @@ The orchestrator launches all implementation agents directly using specs produce The orchestrator runs a **trailer verification** after every commit: 1. Commit trailers must include appropriate co-authors for production file changes -2. Files under `server/` or `shared/` → must have `backend-developer` trailer -3. Files under `client/` (except `client/src/i18n/de/` and `client/src/i18n/glossary.json`) → must have `frontend-developer` trailer +2. Files under `server/` or `shared/`, excluding `*.test.ts`/`*.test.tsx` → must have `backend-developer` trailer +3. Files under `client/` (except `client/src/i18n/de/`, `client/src/i18n/glossary.json`, and `*.test.ts`/`*.test.tsx`) → must have `frontend-developer` trailer 4. Files under `client/src/i18n/de/` or `client/src/i18n/glossary.json` → must have `translator` trailer 5. Files under `e2e/` → must have `e2e-test-engineer` trailer +6. Files matching `*.test.ts` or `*.test.tsx` outside `e2e/` (co-located unit/integration tests) → must have `qa-integration-tester` trailer Commits that change production files without the appropriate co-author trailers (see the Canonical Agent Trailers table below) are rejected and re-committed with corrected trailers.