mirror of
https://github.com/OpenRouterTeam/spawn.git
synced 2026-05-09 11:10:10 +00:00
fix: enforce worktrees, clarify team separation of concerns (#839)
* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c602443711
commit
ac5ef953f9
3 changed files with 137 additions and 93 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue