From ac5ef953f978f37fccfc37ffc96b20b1e7886a73 Mon Sep 17 00:00:00 2001 From: L <6723574+louisgv@users.noreply.github.com> Date: Fri, 13 Feb 2026 00:52:13 -0800 Subject: [PATCH] fix: enforce worktrees, clarify team separation of concerns (#839) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: add worktree requirement to security team prompts PR reviewers must check out PRs in sub-worktrees before running bash -n or bun test. Scan mode agents must also work inside a worktree. This prevents concurrent agents from conflicting in the main repo checkout. Co-Authored-By: Claude Opus 4.6 (1M context) * fix: enforce worktrees everywhere, refactor pr-maintainer role - SKILL.md: expand worktree convention to cover all agent work (PR review, testing, audits) not just branch creation - refactor.sh pr-maintainer: strip review/approve/merge responsibilities — that's the security team's job. pr-maintainer now focuses on rebasing conflicting PRs, addressing review comments, and fixing failing checks - Remove stale PR auto-merge from pre-cycle cleanup Co-Authored-By: Claude Opus 4.6 (1M context) * fix: remove self-review from refactor team, clarify separation of concerns Refactor team focuses on research, deep-dives, and solving problems. Security team owns the entire PR review/approve/merge lifecycle. - Replace "No Self-Merge Rule" with "Separation of Concerns" section - Remove all self-review steps from issue and refactor mode workflows - Remove needs-team-review labeling from agent instructions - Simplify monitoring loop (no more review verification) - Simplify lifecycle checks (verify PRs exist, not reviewed) Co-Authored-By: Claude Opus 4.6 (1M context) * chore: remove stale needs-team-review label from security triage reference The refactor team no longer applies this label, so remove it from the available labels documentation in triage mode. Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- .claude/skills/setup-agent-team/SKILL.md | 21 ++- .claude/skills/setup-agent-team/refactor.sh | 138 ++++++++------------ .claude/skills/setup-agent-team/security.sh | 71 ++++++++-- 3 files changed, 137 insertions(+), 93 deletions(-) diff --git a/.claude/skills/setup-agent-team/SKILL.md b/.claude/skills/setup-agent-team/SKILL.md index bc6ac20c..fb190a1c 100644 --- a/.claude/skills/setup-agent-team/SKILL.md +++ b/.claude/skills/setup-agent-team/SKILL.md @@ -336,7 +336,9 @@ git fetch origin main git pull origin main ``` -### 2. Use git worktrees for all branch work +### 2. Use git worktrees for ALL work (mandatory) + +**Every agent MUST work in a git worktree — NEVER operate directly in the main repo checkout.** This applies to all work: creating branches, reviewing PRs, running tests, reading code for audits, etc. When multiple agents work in parallel, they MUST use worktrees instead of `git checkout -b` to avoid clobbering each other's uncommitted changes: @@ -349,19 +351,30 @@ git worktree add /tmp/spawn-worktrees/BRANCH-NAME -b BRANCH-NAME origin/main # Work inside the worktree cd /tmp/spawn-worktrees/BRANCH-NAME -# ... make changes ... +# ... make changes, run tests, etc. ... -# Commit, push, create PR, merge +# Commit, push, create PR git push -u origin BRANCH-NAME gh pr create --title "..." --body "... -- TEAM-NAME/AGENT-NAME" -gh pr merge --squash --delete-branch # Clean up git worktree remove /tmp/spawn-worktrees/BRANCH-NAME ``` +**For PR review/testing** (read-only worktree): +```bash +git worktree add /tmp/spawn-worktrees/pr-NUMBER -b review-pr-NUMBER origin/main +cd /tmp/spawn-worktrees/pr-NUMBER +gh pr checkout NUMBER +# ... run bash -n, bun test, read files ... +cd /path/to/repo +git worktree remove /tmp/spawn-worktrees/pr-NUMBER --force +``` + +**Why:** The main checkout must stay clean so concurrent agents don't conflict. Worktrees provide isolated working directories for each agent. + ### 3. Include Agent markers in commits Every agent commit MUST include an `Agent:` trailer identifying which agent authored it: diff --git a/.claude/skills/setup-agent-team/refactor.sh b/.claude/skills/setup-agent-team/refactor.sh index 4649f2ec..a04fe062 100755 --- a/.claude/skills/setup-agent-team/refactor.sh +++ b/.claude/skills/setup-agent-team/refactor.sh @@ -101,20 +101,17 @@ if [[ "${RUN_MODE}" == "refactor" ]]; then fi done - log "Pre-cycle cleanup: stale open PRs (merge if possible, never close)..." - STALE_PRS=$(gh pr list --repo OpenRouterTeam/spawn --state open --json number,updatedAt --jq '.[] | select(.updatedAt < (now - 7200 | todate)) | .number' 2>/dev/null) || true - for pr_num in $STALE_PRS; do + log "Pre-cycle cleanup: checking stale open PRs for conflicts..." + STALE_PRS=$(gh pr list --repo OpenRouterTeam/spawn --state open --json number,updatedAt,mergeable --jq '.[] | select(.updatedAt < (now - 7200 | todate)) | "\(.number) \(.mergeable)"' 2>/dev/null) || true + while IFS=' ' read -r pr_num pr_mergeable; do if [[ -n "$pr_num" ]]; then - log "Found stale PR #${pr_num}, checking if mergeable..." - PR_MERGEABLE=$(gh pr view "$pr_num" --repo OpenRouterTeam/spawn --json mergeable --jq '.mergeable' 2>/dev/null) || PR_MERGEABLE="UNKNOWN" - if [[ "$PR_MERGEABLE" == "MERGEABLE" ]]; then - log "Merging stale PR #${pr_num}..." - gh pr merge "$pr_num" --repo OpenRouterTeam/spawn --squash --delete-branch 2>&1 | tee -a "${LOG_FILE}" || true - else + if [[ "$pr_mergeable" != "MERGEABLE" ]]; then log "Stale PR #${pr_num} has conflicts — will be handled by pr-maintainer agent" + else + log "Stale PR #${pr_num} is mergeable — will be reviewed by security team" fi fi - done + done <<< "$STALE_PRS" fi # Launch Claude Code with mode-specific prompt @@ -153,7 +150,6 @@ Create these teammates: - Implement the fix in an isolated worktree - Run tests to verify the fix - Create a PR with \`Fixes #${SPAWN_ISSUE}\` in the body - - Self-review the PR and label with \`needs-team-review\` (do NOT merge) 2. **issue-tester** (Haiku) - Review the fix for correctness and edge cases @@ -189,9 +185,6 @@ Track issue lifecycle with labels: "pending-review" → "under-review" → "in-p 9. When fix is ready: - Push: \`git push -u origin fix/issue-${SPAWN_ISSUE}\` - PR: \`gh pr create --title "fix: Description" --body "Fixes #${SPAWN_ISSUE}\n\n-- refactor/issue-fixer"\` - - Self-review: \`gh pr review NUMBER --repo OpenRouterTeam/spawn --comment --body "Self-review by issue-fixer: [summary]\n\n-- refactor/issue-fixer"\` - - Label: \`gh pr edit NUMBER --repo OpenRouterTeam/spawn --add-label "needs-team-review"\` - - Do NOT merge — PR stays open for external review 10. Post update comment on the issue linking to the PR 11. Do NOT close the issue — the PR body contains \`Fixes #${SPAWN_ISSUE}\` which will auto-close the issue when the PR is merged 12. Clean up worktree: \`git worktree remove ${WORKTREE_BASE}\` @@ -235,28 +228,24 @@ Complexity-hunter: pick the top 1-2 worst functions, fix them, PR, done. Do NOT Test-engineer: add ONE focused test file, PR, done. Do NOT aim for 100% coverage. Security-auditor: scan for HIGH/CRITICAL only. Document medium/low, don't fix them. -## No Self-Merge Rule (MANDATORY) +## Separation of Concerns (MANDATORY) -Agents must NEVER merge their own PRs. This applies to ALL agents including the team lead. +The refactor team **creates PRs** — the security team **reviews and merges** them. -### What agents MUST do after creating a PR: -1. **Self-review**: Read the PR diff and add a review comment with findings: - `gh pr diff NUMBER --repo OpenRouterTeam/spawn` - `gh pr review NUMBER --repo OpenRouterTeam/spawn --comment --body "Self-review by AGENT-NAME: [summary of changes, what was tested, any concerns]\n\n-- refactor/AGENT-NAME"` -2. **Label for external review**: Add `needs-team-review` label: - `gh pr edit NUMBER --repo OpenRouterTeam/spawn --add-label "needs-team-review"` -3. **Leave the PR open** — do NOT run `gh pr merge` +### What refactor agents MUST do: +1. **Research deeply**: Use web search, code exploration, and deep-dives to understand the problem before writing code +2. **Create a PR** with clear title and description explaining the change and rationale +3. **Leave the PR open** — the security team handles review, approval, and merge -### What agents must NEVER do: -- `gh pr merge` — NEVER, under any circumstances -- Approve their own PR — self-review is comment-only, not approval +### What refactor agents must NEVER do: +- `gh pr review` — NEVER review PRs (that's the security team's job) +- `gh pr merge` — NEVER merge PRs +- Approve or request changes on any PR ### Why: -- Prevents duplicate or conflicting work from landing unreviewed -- Ensures a second set of eyes (human or another team) validates every change -- Catches planning issues before they hit main - -PRs will be reviewed and merged externally (by maintainers or a separate review cycle). +- Clear ownership: refactor team solves problems, security team gates quality +- Prevents unreviewed code from landing +- Lets each team focus on what they do best ## Team Structure @@ -290,35 +279,40 @@ Create these teammates: - Add integration tests for critical paths 5. **pr-maintainer** (Sonnet) + - **Role: Keep PRs healthy and mergeable. Do NOT review, approve, or merge PRs — that is the security team's responsibility.** - FIRST TASK: List ALL open PRs: `gh pr list --repo OpenRouterTeam/spawn --state open --json number,title,headRefName,updatedAt,mergeable,reviewDecision` - For EACH open PR, evaluate and take the appropriate action: - * **Mergeable + approved (or no reviews required)**: approve and merge it - `gh pr review NUMBER --repo OpenRouterTeam/spawn --approve --body "Approved by pr-maintainer: looks good, tests pass.\n\n-- refactor/pr-maintainer"` - `gh pr merge NUMBER --repo OpenRouterTeam/spawn --squash --delete-branch` - * **Mergeable + needs review**: review the diff for correctness, then approve and merge if safe - `gh pr diff NUMBER --repo OpenRouterTeam/spawn` - - Check for: command injection, credential leaks, broken curl|bash patterns, macOS compat issues - - If safe: approve + merge (squash + delete branch) - - If concerns: post a review comment describing the issue (request changes, do NOT close) * **Has merge conflicts**: rebase the PR branch onto main to resolve ``` git fetch origin - git worktree add /tmp/spawn-worktrees/pr-rebase-NUMBER -b pr-rebase-NUMBER origin/BRANCH + BRANCH=$(gh pr view NUMBER --repo OpenRouterTeam/spawn --json headRefName --jq '.headRefName') + git worktree add /tmp/spawn-worktrees/pr-rebase-NUMBER origin/$BRANCH cd /tmp/spawn-worktrees/pr-rebase-NUMBER git rebase origin/main # If rebase succeeds: force-push the branch - git push --force-with-lease origin BRANCH - # Then re-evaluate: if clean, approve and merge - git worktree remove /tmp/spawn-worktrees/pr-rebase-NUMBER + git push --force-with-lease origin $BRANCH + cd /path/to/repo + git worktree remove /tmp/spawn-worktrees/pr-rebase-NUMBER --force ``` - If rebase has unresolvable conflicts, post a comment asking the author to resolve: - `gh pr comment NUMBER --repo OpenRouterTeam/spawn --body "This PR has merge conflicts that couldn't be auto-resolved. Could you rebase on main and push? Happy to re-review once updated.\n\n-- refactor/branch-manager"` - * **Failing checks**: investigate the failure, fix if trivial, otherwise comment with failure details + If rebase has unresolvable conflicts, post a comment: + `gh pr comment NUMBER --repo OpenRouterTeam/spawn --body "Attempted to rebase onto main but conflicts couldn't be auto-resolved. Manual resolution needed.\n\n-- refactor/pr-maintainer"` + * **Has review comments requesting changes**: read the review comments, address them with code fixes in a worktree + ``` + gh pr view NUMBER --repo OpenRouterTeam/spawn --json reviews --jq '.reviews[] | select(.state == "CHANGES_REQUESTED") | .body' + gh api repos/OpenRouterTeam/spawn/pulls/NUMBER/comments --jq '.[].body' + ``` + - Check out the PR branch in a worktree, make the requested fixes, commit, and push + - Post a comment summarizing what was addressed: + `gh pr comment NUMBER --repo OpenRouterTeam/spawn --body "Addressed review feedback:\n- [list of changes]\n\n-- refactor/pr-maintainer"` + * **Failing checks**: investigate the failure in a worktree, fix if trivial (e.g., `bash -n` errors, test failures), push the fix + - If the failure is non-trivial, comment with failure details for the author + * **Mergeable + no issues**: leave it alone — the security team handles review and merge - ALSO clean up orphan branches (no open PR, stale >4 hours): `git push origin --delete BRANCH` + - **NEVER review, approve, or merge PRs** — that is exclusively the security team's job - **NEVER close a PR** — always try to rebase, fix, or request changes instead - - After processing all PRs, report summary: how many merged, rebased, commented, branches cleaned + - After processing all PRs, report summary: how many rebased, fixed, commented, branches cleaned - Run this check AGAIN at the end of the cycle to catch PRs created during the cycle - - GOAL: Zero stale unreviewed PRs left on the remote after each cycle. + - GOAL: All open PRs are conflict-free, review feedback is addressed, and checks are passing — ready for security review. 6. **community-coordinator** (Sonnet) - FIRST TASK: Run `gh issue list --repo OpenRouterTeam/spawn --state open --json number,title,body,labels,createdAt` @@ -387,22 +381,15 @@ When fixing a bug reported in a GitHub issue: gh pr create --title "Fix: description" --body "Fixes #NUMBER -- refactor/AGENT-NAME" -12. Self-review and label (DO NOT merge — see No Self-Merge Rule): - gh pr diff NUMBER --repo OpenRouterTeam/spawn - gh pr review NUMBER --repo OpenRouterTeam/spawn --comment --body "Self-review by AGENT-NAME: [summary of fix, tests run, confidence level] +12. Clean up: git worktree remove WORKTREE_BASE_PLACEHOLDER/fix/issue-NUMBER +13. Community-coordinator posts update comment with PR link (only if no similar update exists) +14. Do NOT close the issue — the PR body contains \`Fixes #NUMBER\` which will auto-close the issue when merged --- refactor/AGENT-NAME" - gh pr edit NUMBER --repo OpenRouterTeam/spawn --add-label "needs-team-review" -13. Clean up: git worktree remove WORKTREE_BASE_PLACEHOLDER/fix/issue-NUMBER -14. Community-coordinator posts update comment with PR link (only if no similar update exists) -15. Do NOT close the issue — the PR body contains \`Fixes #NUMBER\` which will auto-close the issue when merged - -NEVER leave a PR without a self-review comment and \`needs-team-review\` label. If a PR cannot be created (conflicts, superseded, etc.), close it WITH a comment explaining why. NEVER close a PR silently — every closed PR MUST have a comment. NEVER close an issue manually — let the PR merge auto-close it via \`Fixes #NUMBER\`. -The full cycle is: acknowledge → investigate → worktree → fix → update → PR (references issue with \`Fixes #NUMBER\`) → self-review → label → cleanup worktree. -Note: merging is handled externally — agents do NOT merge. Issues close automatically when the PR merges. +The full cycle is: acknowledge → investigate → worktree → fix → update → PR (references issue with \`Fixes #NUMBER\`) → cleanup worktree. +Note: review and merging is handled by the security team. Issues close automatically when the PR merges. ## Commit Markers (MANDATORY) @@ -465,14 +452,7 @@ gh pr create --title "title" --body "body -- refactor/AGENT-NAME" -# 5. Self-review and label (DO NOT merge — see No Self-Merge Rule) -gh pr diff NUMBER --repo OpenRouterTeam/spawn -gh pr review NUMBER --repo OpenRouterTeam/spawn --comment --body "Self-review by AGENT-NAME: [summary] - --- refactor/AGENT-NAME" -gh pr edit NUMBER --repo OpenRouterTeam/spawn --add-label "needs-team-review" - -# 6. Clean up worktree (PR stays open for external review) +# 5. Clean up worktree (PR stays open for security team review) git worktree remove WORKTREE_BASE_PLACEHOLDER/BRANCH-NAME ``` @@ -495,7 +475,7 @@ git worktree remove WORKTREE_BASE_PLACEHOLDER/BRANCH-NAME 1. Create the team with TeamCreate 2. Set up worktree directory: mkdir -p WORKTREE_BASE_PLACEHOLDER 3. Create tasks using TaskCreate for each area: - - PR maintenance: review, rebase, and merge all open PRs; clean orphan branches + - PR maintenance: rebase conflicting PRs, address review comments, fix failing checks; clean orphan branches - Community coordination: scan all open issues, post acknowledgments, categorize, and delegate - Security scan of all scripts - UX test of main user flows @@ -503,7 +483,7 @@ git worktree remove WORKTREE_BASE_PLACEHOLDER/BRANCH-NAME - Test coverage for recent changes 4. Spawn teammates with Task tool using subagent_type='general-purpose' 5. Assign tasks to teammates using TaskUpdate -6. PR-maintainer reviews all open PRs: merge clean ones, rebase conflicting ones, comment on issues +6. PR-maintainer maintains all open PRs: rebase conflicting ones, address review comments, fix failing checks 7. Community-coordinator engages issues FIRST — posts acknowledgments before other agents start investigating 8. Community-coordinator delegates issue investigations to relevant teammates 9. All agents use worktrees for their branch work (never git checkout in the main repo) @@ -527,20 +507,18 @@ git worktree remove WORKTREE_BASE_PLACEHOLDER/BRANCH-NAME 2. Immediately start a polling loop — do NOT just output text saying "I'll wait": while teammates are still active: a. Run TaskList to check task status - b. Run `gh pr list --repo OpenRouterTeam/spawn --state open --label "needs-team-review"` to verify PRs have been self-reviewed and labeled - c. When you receive a teammate message, acknowledge it and update task tracking - d. If a teammate reports completion, mark their task done — verify their PR has a self-review comment and label - e. If a teammate reports an error, coordinate resolution - f. If the time budget is almost up, send wrap-up messages to all teammates - g. If no messages received yet, run `Bash("sleep 30")` then loop back to (a) - h. REMINDER: Do NOT merge any PRs — they stay open for external review + b. When you receive a teammate message, acknowledge it and update task tracking + c. If a teammate reports completion, mark their task done + d. If a teammate reports an error, coordinate resolution + e. If the time budget is almost up, send wrap-up messages to all teammates + f. If no messages received yet, run `Bash("sleep 30")` then loop back to (a) 3. Only after ALL teammates have finished, proceed to shutdown ``` ### Common mistake (DO NOT DO THIS): ``` BAD: Spawn teammates → "I'll wait for their messages" → session ends (agents orphaned!) -GOOD: Spawn teammates → TaskList → sleep 30 → TaskList → receive message → merge PR → ... → shutdown +GOOD: Spawn teammates → TaskList → sleep 30 → TaskList → receive message → coordinate → ... → shutdown ``` ## Lifecycle Management (MANDATORY — DO NOT EXIT EARLY) @@ -548,7 +526,7 @@ GOOD: Spawn teammates → TaskList → sleep 30 → TaskList → receive message You MUST remain active until ALL of the following are true: 1. **All tasks are completed**: Run TaskList and confirm every task has status "completed" -2. **All PRs are self-reviewed and labeled**: Run `gh pr list --repo OpenRouterTeam/spawn --state open --label "needs-team-review"` and confirm every PR from this cycle has a self-review comment and the `needs-team-review` label. Do NOT merge — PRs stay open for external review. +2. **All PRs are created**: Verify PRs from this cycle exist and have clear descriptions. PRs stay open for the security team to review and merge. 3. **All issues are engaged and labeled**: Run `gh issue list --repo OpenRouterTeam/spawn --state open --json number,labels` and for EACH open issue, verify it has at least one comment AND has a status label ("pending-review", "under-review", or "in-progress"). If any issue is missing a status @@ -560,7 +538,7 @@ You MUST remain active until ALL of the following are true: ### Shutdown Sequence (execute in this exact order): 1. Check TaskList — if any tasks are still in_progress or pending, wait and check again (poll every 30 seconds, up to 10 minutes) -2. Verify all PRs are self-reviewed and labeled: `gh pr list --repo OpenRouterTeam/spawn --state open --label "needs-team-review"` (PRs stay open — do NOT merge) +2. Verify all PRs from this cycle have been created with clear descriptions 3. Verify all issues engaged: `gh issue list --repo OpenRouterTeam/spawn --state open` 4. For each teammate, send a `shutdown_request` via SendMessage 5. Wait for all `shutdown_response` confirmations diff --git a/.claude/skills/setup-agent-team/security.sh b/.claude/skills/setup-agent-team/security.sh index 32c21d82..58df37b4 100644 --- a/.claude/skills/setup-agent-team/security.sh +++ b/.claude/skills/setup-agent-team/security.sh @@ -324,7 +324,7 @@ fi - \`Pending Review\` → \`Under Review\` → \`In Progress\` **PR labels** (used by review_all mode): -- \`security-approved\`, \`security-review-required\`, \`security-notes\`, \`needs-team-review\` +- \`security-approved\`, \`security-review-required\`, \`security-notes\` ## Rules @@ -358,6 +358,26 @@ This cycle MUST complete within 30 minutes. This is a HARD deadline. - At the 29-minute mark, send shutdown_request to all agents - At 30 minutes, force shutdown +## Worktree Requirement + +**All agents MUST work in git worktrees — NEVER operate directly in the main repo checkout.** This prevents conflicts between concurrent agents and protects the main working tree. + +The team lead sets up the base worktree at \`${WORKTREE_BASE}\` from \`origin/main\`. All agents that need to read or test code should work inside their own worktree: + +\`\`\`bash +# Team lead creates base worktree: +git worktree add ${WORKTREE_BASE} origin/main --detach + +# PR reviewers: checkout the PR branch in a sub-worktree +git worktree add ${WORKTREE_BASE}/pr-NUMBER -b review-pr-NUMBER origin/main +cd ${WORKTREE_BASE}/pr-NUMBER +gh pr checkout NUMBER +# ... run bash -n, bun test, etc. here ... +# Clean up when done: +cd ${REPO_ROOT} +git worktree remove ${WORKTREE_BASE}/pr-NUMBER --force +\`\`\` + ## Step 1 — Discover Open PRs Run: @@ -436,7 +456,16 @@ PR #NUMBER was auto-closed due to staleness + merge conflicts, but the change it - The PR is stale but mergeable — still do the security review below (it may be fine to merge). * Is the PR fresh (<48h)? → Proceed to security review normally. -3. Review every changed file for security issues: +3. **Set up a worktree** to test the PR locally: + \`\`\`bash + git worktree add ${WORKTREE_BASE}/pr-NUMBER -b review-pr-NUMBER origin/main + cd ${WORKTREE_BASE}/pr-NUMBER + gh pr checkout NUMBER + \`\`\` + All file reads, \`bash -n\` checks, and \`bun test\` runs MUST happen inside this worktree. + Clean up when done: \`cd ${REPO_ROOT} && git worktree remove ${WORKTREE_BASE}/pr-NUMBER --force\` + +4. Review every changed file for security issues: * **Command injection**: unquoted variables in shell commands, unsafe eval/heredoc, unsanitized input in bash * **Credential leaks**: hardcoded API keys, tokens, passwords; secrets logged to stdout; credentials in committed files * **Path traversal**: unsanitized file paths, directory escape via ../ @@ -445,17 +474,17 @@ PR #NUMBER was auto-closed due to staleness + merge conflicts, but the change it * **curl|bash safety**: broken source/eval fallback patterns, missing integrity checks * **macOS bash 3.x compat**: echo -e, source <(), ((var++)) with set -e, local in subshells, set -u -4. For each changed .sh file: +5. For each changed .sh file (run inside the worktree): * Run \`bash -n FILE\` to check syntax * Verify the local-or-remote source fallback pattern is used * Check for macOS bash 3.x incompatibilities -5. For changed .ts files: +6. For changed .ts files (run inside the worktree): * Run \`bun test\` to verify tests pass -6. Classify each finding as CRITICAL, HIGH, MEDIUM, or LOW +7. Classify each finding as CRITICAL, HIGH, MEDIUM, or LOW -7. Make the review decision and label the PR: +8. Make the review decision and label the PR: **If CRITICAL or HIGH issues found** — request changes + label: \`\`\`bash @@ -478,7 +507,13 @@ PR #NUMBER was auto-closed due to staleness + merge conflicts, but the change it \`\`\` If merge fails (conflicts, branch protection), log the error and move on. -8. Review body format: +9. **Clean up the worktree** after review: + \`\`\`bash + cd ${REPO_ROOT} + git worktree remove ${WORKTREE_BASE}/pr-NUMBER --force 2>/dev/null || true + \`\`\` + +10. Review body format: \`\`\` ## Security Review @@ -497,7 +532,7 @@ PR #NUMBER was auto-closed due to staleness + merge conflicts, but the change it *-- security/pr-reviewer* \`\`\` -9. Report results to the team lead: PR number, verdict (approved+merged / changes-requested / closed-stale), finding count, merge status +11. Report results to the team lead: PR number, verdict (approved+merged / changes-requested / closed-stale), finding count, merge status ## Step 3 — Branch Cleanup @@ -647,6 +682,7 @@ Required pattern: ## Safety Rules +- **ALWAYS use worktrees** for testing PR code — never run \`bash -n\` or \`bun test\` in the main repo checkout - NEVER approve a PR with CRITICAL or HIGH findings - Auto-merge PRs that have no CRITICAL/HIGH findings and all tests pass - MEDIUM/LOW findings are informational — still approve and merge @@ -678,9 +714,25 @@ This cycle MUST complete within 15 minutes. This is a HARD deadline. - At the 14-minute mark, send shutdown_request to all agents - At 15 minutes, force shutdown +## Worktree Requirement + +**All agents MUST work in git worktrees — NEVER operate directly in the main repo checkout.** This prevents conflicts between concurrent agents. + +Set up the base worktree before spawning agents: +\`\`\`bash +git worktree add ${WORKTREE_BASE} origin/main --detach +\`\`\` + +Tell each agent to work inside \`${WORKTREE_BASE}\` (or a sub-worktree if needed). Clean up at the end: +\`\`\`bash +cd ${REPO_ROOT} +git worktree remove ${WORKTREE_BASE} --force 2>/dev/null || true +git worktree prune +\`\`\` + ## Team Structure -Create these teammates: +Create these teammates (all working inside \`${WORKTREE_BASE}\`): 1. **shell-auditor** (Opus) - Scan ALL .sh files in the repo for security issues: @@ -828,6 +880,7 @@ Required pattern: ## Safety Rules +- **ALWAYS use worktrees** — never read or test files in the main repo checkout - Do not modify any code — this is audit only - Always dedup against existing issues before filing - Classify findings conservatively — if unsure, rate it one level higher