pr-lifecycle

Context

This skill is the canonical Copilot-agent lifecycle for the Squad repository. It covers the full path from picking up a GitHub issue to merging a PR. Where older docs (templates, copilot-instructions, CONTRIBUTING.md) conflict with this skill, this skill takes precedence for Copilot agents.

For advanced scenarios (worktrees, multi-repo coordination), see .copilot/skills/git-workflow/SKILL.md.

Scope

✅ THIS SKILL COVERS:

• Issue pickup and branch creation
• Implementation, build, test, lint workflow
• Pre-push safety verification
• PR creation with correct target, title, body, and labels
• All 11 PR readiness checks (what they check, how to pass, how to fix)
• Post-creation maintenance (review feedback, rebasing, CI)
• Merge preconditions and cleanup

❌ THIS SKILL DOES NOT COVER:

• Worktree-based parallel work (see git-workflow skill)
• Multi-repo coordinated PRs (see git-workflow skill)
• Release process / publishing (see .squad/skills/release-process)
• Reviewer lockout protocol (see .copilot/skills/reviewer-protocol)
• Architectural or security review checklists

Lifecycle Procedure

Phase 1 — Issue Pickup

1. Read the issue. Understand the acceptance criteria before writing code.
2. Confirm capability fit. Check your capability profile in .squad/team.md. 🟢 = proceed. 🟡 = proceed but flag in PR. 🔴 = comment on issue and stop.
3. Branch from dev:

   git fetch origin dev
   git checkout dev
   git rebase origin/dev
   git checkout -b squad/{issue-number}-{slug}

   • Branch name format: squad/{issue-number}-{kebab-case-slug}
   • Example: squad/42-fix-login-validation
   • Never branch from main
4. Mark in-progress (optional):

   gh issue edit {number} --add-label "status:in-progress"

Phase 2 — Implementation

1. Make your changes. Follow the codebase conventions:
   • TypeScript strict mode, no @ts-ignore
   • ESM-only, async/await
   • JSDoc on new public APIs
2. Build:

   npm run build

3. Test:

   npm test

4. Type check:

   npm run lint

5. Stage specific files only:

   git add path/to/file1.ts path/to/file2.ts

   • ❌ NEVER git add ., git add -A, or git commit -a
   • ✅ ALWAYS name each file explicitly
6. Pre-push safety check:

   # Verify file count matches intent (expect ≤10 files for most fixes)
   git diff --cached --stat
   
   # Verify NO unintended deletions
   git diff --cached --diff-filter=D --name-only

   If you see unexpected files or deletions, unstage them: git reset HEAD <file>
7. Single commit with issue reference:

   git commit -m "Brief description of change
   
   Closes #{issue-number}
   
   Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>"

   • If you already have multiple commits, squash: git rebase -i origin/dev and squash all into one
   • Never use git reset --soft to squash — it picks up delta from dev and contaminates the commit

8. Add changeset (if required): A changeset is required when your PR modifies files under packages/squad-sdk/src/ or packages/squad-cli/src/.

   npx changeset add

   This prompts for: which packages changed, bump type (patch/minor/major), summary.

   Or create manually at .changeset/{descriptive-name}.md:

   ---
   '@bradygaster/squad-cli': patch
   ---
   
   Brief description of the change

   If the changeset creates a second commit, squash it into your main commit.

Phase 3 — PR Creation

1. Push:

   git push -u origin squad/{issue-number}-{slug}

2. Create PR targeting dev:

   gh pr create --repo bradygaster/squad --base dev \
     --title "fix: brief description (#issue-number)" \
     --body "Closes #{issue-number}
   
   ## Summary
   What this PR does and why.
   
   ## Changes
   - File-level description of changes
   
   Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>"

3. Title conventions:
   • Bug fix: fix: description (#N)
   • Feature: feat: description (#N)
   • Docs: docs: description (#N)
   • Chore/infra: chore: description (#N)
4. Labels:
   • fix, feat, docs, or repo-health for type
   • squad:{agent-name} if working as a squad member
   • skip-changelog only with reviewer approval (escape hatch)
5. Scope rules by label:
   • repo-health: Only modify .github/, scripts/, root config, tests, docs. Never modify packages/*/src/
   • fix or feat: May modify product source. Must include changeset when touching packages/*/src/

Phase 4 — PR Readiness

An automated readiness check runs on every push and posts a checklist comment on the PR. All 11 checks must pass before review (check 11 is informational-only).

Check 1: Single Commit

Check 2: Not in Draft

Check 3: Branch Up to Date

Check 4: Copilot Review

Check 5: Changeset Present

Check 6: No Merge Conflicts

Check 7: Scope Clean

Check 8: Copilot Threads Resolved

Check 9: CI Passing

Check 10: Issue Linked

Check 11: Protected Files (informational)

Phase 5 — Post-Creation Maintenance

1. Handling Copilot review feedback:
   • Read each comment carefully
   • Fix valid issues in your code
   • Resolve each thread after addressing it
   • Amend your single commit: git commit --amend --no-edit && git push --force-with-lease
2. Rebasing when behind dev:

   git fetch origin dev
   git rebase origin/dev
   # Resolve any conflicts
   git push --force-with-lease

3. Re-running CI:
   • CI re-runs automatically on every push
   • To re-run without changes: gh run rerun {run-id} --failed
4. If readiness check is stale:
   • The readiness check re-runs on push and after CI completes
   • If it's stale, push a trivial fix or empty commit to re-trigger

Phase 6 — Merge & Cleanup

1. Merge preconditions:
   • All 11 readiness checks pass (✅ across the board)
   • Human reviewer has approved (or maintainer merges directly)
   • No open blocking conversations
2. Who merges:
   • Agents do not merge PRs themselves
   • Maintainers (bradygaster or designated reviewers) merge via GitHub UI
   • The repo uses squash merge, so commit history is clean regardless
3. Post-merge cleanup:

   git checkout dev
   git pull origin dev
   git branch -d squad/{issue-number}-{slug}
   git push origin --delete squad/{issue-number}-{slug}

4. Verify issue auto-close:
   • If PR body contains Closes #{N}, the issue closes automatically on merge
   • If not, manually close: gh issue close {number}

Examples

Example: Bug fix PR (no source changes)

# Branch
git fetch origin dev && git checkout dev && git rebase origin/dev
git checkout -b squad/610-fix-broken-link

# Fix
# ... edit docs/some-file.md ...

# Validate
npm run build && npm test

# Commit (no changeset needed — docs only)
git add docs/some-file.md
git diff --cached --stat        # verify: 1 file
git commit -m "docs: fix broken link in contributing guide

Closes #610

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>"

# Push and PR
git push -u origin squad/610-fix-broken-link
gh pr create --repo bradygaster/squad --base dev \
  --title "docs: fix broken link (#610)" \
  --body "Closes #610"

Example: SDK feature PR (source changes)

# Branch
git fetch origin dev && git checkout dev && git rebase origin/dev
git checkout -b squad/42-add-profile-api

# Implement
# ... edit packages/squad-sdk/src/profile/index.ts ...
# ... edit packages/squad-sdk/src/index.ts (re-export) ...

# Validate
npm run build && npm test && npm run lint

# Changeset (required — touches packages/squad-sdk/src/)
npx changeset add
# Select: @bradygaster/squad-sdk, minor, "Add profile API"

# Stage and commit
git add packages/squad-sdk/src/profile/index.ts packages/squad-sdk/src/index.ts .changeset/
git diff --cached --stat                    # verify file count
git diff --cached --diff-filter=D --name-only  # verify no deletions
git commit -m "feat: add profile API

Closes #42

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>"

# Push and PR
git push -u origin squad/42-add-profile-api
gh pr create --repo bradygaster/squad --base dev \
  --title "feat: add profile API (#42)" \
  --body "Closes #42

## Summary
Adds profile resolution API to the SDK.

## Changes
- New module: packages/squad-sdk/src/profile/
- Re-exported from barrel file"

Anti-Patterns

• ❌ Branching from main (always branch from dev)
• ❌ Targeting main with PRs (always target dev)
• ❌ Using git add . or git add -A (stage specific files only)
• ❌ Using git commit -a (same risk as broad staging)
• ❌ Using git reset --soft to squash (contaminates commit with dev delta)
• ❌ Force-pushing to dev or main (only force-push your own feature branch)
• ❌ Merging your own PR (maintainers merge)
• ❌ Skipping the changeset when source files changed (CI will fail)
• ❌ Self-resolving Copilot threads without addressing the feedback
• ❌ Pushing >1 commit without squashing (readiness check will flag it)
• ❌ Including .squad/ files in product PRs without intention (scope check warns)
• ❌ Mixing product and infrastructure changes in one PR (create separate PRs)

Readiness Check Gaps & Recommendations

After analyzing .github/workflows/squad-ci.yml, three gaps were identified. Gaps 1 and 3 are now implemented (checks 10 and 11). Gap 2 is deferred.

Gap 1: Issue Linkage Check (Check 10) — IMPLEMENTED

Problem: Nothing verifies that the PR body or commit message references an issue (Closes #N or Part of #N). Orphan PRs are hard to trace.

Recommendation: Add checkIssueLinkage(prBody, commitMessages) to pr-readiness.mjs.

/**
 * Check: Issue linkage.
 * @param {string} prBody — PR description text
 * @param {Array<{ commit: { message: string } }>} commits
 * @returns {{ pass: boolean, detail: string }}
 */
export function checkIssueLinkage(prBody, commits) {
  const issuePattern = /(closes|fixes|resolves|part of)\s+#\d+/i;
  const bodyHasRef = issuePattern.test(prBody || '');
  const commitHasRef = (commits || []).some(
    (c) => issuePattern.test(c.commit?.message || '')
  );
  if (bodyHasRef || commitHasRef) {
    return { pass: true, detail: 'Issue reference found' };
  }
  return {
    pass: false,
    detail: 'No issue reference — add `Closes #N` to PR body or commit message',
  };
}

Integration point: After check 2 (draft status), before check 3 (branch freshness). Data is already available from the commits and PR body fetched in the orchestrator.

Gap 2: No Required Checks Presence Verification

Problem: The CI status check (check 9) verifies that existing checks are green, but doesn't verify that the expected set of checks actually ran. If a workflow is misconfigured or skipped, the PR could pass with zero CI checks.

Recommendation: Add checkRequiredChecksPresent(checkRuns, files) to pr-readiness.mjs.

/** Minimum required check names that must appear for source PRs. */
export const REQUIRED_CHECKS = ['Squad CI / test'];

/**
 * Check: Required CI checks are present.
 * @param {Array<{ name: string }>} checkRuns
 * @param {Array<{ filename: string }>} files
 * @returns {{ pass: boolean, detail: string }}
 */
export function checkRequiredChecksPresent(checkRuns, files) {
  const touchesSource = (files || []).some(
    (f) => SOURCE_PATTERN.test(f.filename)
  );
  if (!touchesSource) {
    return { pass: true, detail: 'No source changes — required checks not enforced' };
  }
  const checkNames = new Set((checkRuns || []).map((cr) => cr.name));
  const missing = REQUIRED_CHECKS.filter((name) => !checkNames.has(name));
  if (missing.length > 0) {
    return {
      pass: false,
      detail: `Required check(s) not found: ${missing.join(', ')}`,
    };
  }
  return { pass: true, detail: 'All required checks present' };
}

Integration point: After check 9 (CI status). Uses the same checkRuns and files data already fetched.

Gap 3: Protected File Change Detection (Check 11) — IMPLEMENTED

Problem: The repo has zero-dependency bootstrap files that must never import external packages (documented in copilot-instructions.md). The squad-repo-health.yml workflow runs a bootstrap protection check, but the PR readiness comment doesn't surface it — contributors don't see the warning until they check the separate workflow.

Recommendation: Add checkProtectedFiles(files) to pr-readiness.mjs as an informational check (always passes, like scope clean).

/** Bootstrap files that must remain zero-dependency. */
export const PROTECTED_FILES = [
  'packages/squad-cli/src/cli/core/detect-squad-dir.ts',
  'packages/squad-cli/src/cli/core/errors.ts',
  'packages/squad-cli/src/cli/core/gh-cli.ts',
  'packages/squad-cli/src/cli/core/output.ts',
  'packages/squad-cli/src/cli/core/history-split.ts',
];

/**
 * Check: Protected file changes (informational).
 * @param {Array<{ filename: string }>} files
 * @returns {{ pass: boolean, detail: string }}
 */
export function checkProtectedFiles(files) {
  const touched = (files || []).filter(
    (f) => PROTECTED_FILES.includes(f.filename)
  );
  if (touched.length === 0) {
    return { pass: true, detail: 'No protected bootstrap files changed' };
  }
  return {
    pass: true,
    detail: `⚠️ ${touched.length} protected bootstrap file(s) changed: ${touched.map((f) => f.filename.split('/').pop()).join(', ')} — verify zero-dependency constraint`,
  };
}

Integration point: After check 7 (scope clean). Uses the same files data. Informational only — warns but doesn't block.

Summary of Recommended Changes

All three use data that pr-readiness.mjs already fetches — no new API calls needed. Total addition: ~60 lines of check functions + ~10 lines of orchestration wiring.