Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 145 additions & 0 deletions .copilot/skills/bleed-check/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
# Bleed-Check: Cross-Branch Audit

## Confidence
High

## Domain
Periodic cross-branch bleed audits, stowaway file detection

## Problem Statement
Features developed on forks can accidentally include files intended for \.squad/\, docs, or unrelated purposes. These "stowaway" files pollute the upstream repository if not caught before PR merge. A periodic audit across all open PRs by a contributor identifies and flags these stragglers before they reach main.

## High-Risk Shared Files

**These files are the #1 bleed vectors** — they appear in almost every docs PR and are where cross-branch contamination happens most:

- **`navigation.ts`** — contains site structure and nav entries for all features
- **`test/docs-build.test.ts`** — build verification tests that reference multiple PRs' output paths
- **`docs/` directories** — shared documentation structure

**When checking these files, verify entries are ONLY for this PR's content — not entries from other concurrent PRs or stale previous runs. Flag full-file rewrites of these shared files — surgical additions only.**

## Trigger

- **Scheduled**: Twice per session day (morning and afternoon sweeps)
- **Proactive**: If >4 hours since last check, coordinator offers proactive bleed audit reminder

## Scope

All open PRs by **diberry** targeting **bradygaster/squad**

Query:
\\\ash
gh pr list --author diberry --repo bradygaster/squad --base dev --state open
\\\

## Process

### 1. List PRs
Fetch all open PRs from diberry targeting bradygaster/squad.

### 2. For Each PR: Check File List
Retrieve the file list:
\\\ash
gh pr view {pr-number} --repo bradygaster/squad --json files
\\\
Comment on lines +31 to +45
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These \\\bash / \\\ blocks aren’t valid Markdown fences and include a stray control character before ash, so the example commands won’t render correctly. Use standard triple-backtick code fences (bash ... ), and remove the backslashes/control character throughout this skill file.

Copilot uses AI. Check for mistakes.

### 3. Flag Stowaways
Check each file against the PR's stated purpose (from title/description). Red flags:

| Red Flag | Example | Why It's Bad |
|---|---|---|
| \.squad/\ files | \.squad/decisions/...\, \.squad/agents/...\ | Should not ship in app PRs |
| Navigation entries | \.squad/routing.md\ changes | Wrong PR can cause nav breakage |
| Test expectations | \.squad/agents/*/expected-output\ | Unmaintainable across PRs |
| Full-file rewrites | Accidental large refactors | Out of scope, causes merge debt |
| Build artifacts | \dist/\, \uild/\, \.next/\ | Should be in \.gitignore\ |
| Root-level strays | Unexpected \.env.local\, \secrets.json\ | Likely accidental commits |

### 3.5: Convention Gate Checks

While auditing files, also check for house-style violations. **These are blockers, not nits — per team directive.**

| Convention | Rule | Blocker? |
|-----------|------|----------|
| Internal link format | Use bare paths like `/features/memory`, not `/docs/features/memory` | ✅ Yes |
| Blank lines | Single blank line between sections (not double) | ✅ Yes |
| Entry duplication | Each nav entry appears exactly once | ✅ Yes |
| Stale TDD comments | Clean up "RED PHASE", "TODO: implement", "WIP" markers before merge | ✅ Yes |

### 3.6: CI Path Debugging Pattern

When CI reports a step as successful but tests fail on a missing file, path mismatches often indicate cross-branch contamination or stale config:

Example: CI says "generator succeeded — output at docs/public/" but the test looks for docs/dist/ and fails.

**Check actual path**:
\\\ash
ls -la docs/public/
ls -la docs/dist/
grep "outDir" build.config.ts
grep "docs/dist" test/docs-build.test.ts
\\\

**Pattern**: Add `ls -la {expected-path}` verification steps when debugging CI file issues. This reveals if the build wrote to the wrong directory (often from stale config or entries from another PR).

### 4. Output Format: Emoji-Based Table

\\\
| PR # | Title | Status | Bleed? | Details |
|---|---|---|---|---|
| #42 | Add auth feature | 🟢 CLEAN | No | All files in scope |
| #43 | Refactor parser | 🔴 BLEED | Yes | \.squad/routing.md\ found (stowaway) |
| #44 | Update docs | 🟢 CLEAN | No | Docs changes as intended |
\\\

Status indicators:
- 🟢 CLEAN: All files align with PR purpose
- 🔴 BLEED: Stowaway files detected, PR needs cleanup

## Coordinator Behavior

The Copilot acting as coordinator:

1. **Track Last Check Time**: Record timestamp of last bleed audit
2. **Proactive Reminders**: After 4+ hours, suggest: "Hey, time for a bleed check? Want me to audit your open PRs?"
3. **Detailed Reports**: Show emoji table with file-by-file breakdown
4. **Actionable Guidance**: If bleed detected, suggest: "Pull request #43 has \.squad/routing.md\ - should this be removed before merging?"

## Session Integration

- Check at start of session for any overnight bleeds
- Offer mid-session reminder if threshold exceeded
- Report findings before upstream PR opens
- Track frequency for team metrics

## Example Session Flow

\\\
✓ Session starts
"Last bleed check was 6 hours ago. Want me to run an audit?"
[User: Yes]

→ Auditing 4 open PRs by diberry...
| #42 | auth feature | 🟢 CLEAN |
| #43 | parser refactor | 🔴 BLEED | .squad/routing.md detected |
| #44 | docs update | 🟢 CLEAN |
| #45 | ui polish | 🟢 CLEAN |

⚠️ Found 1 bleed in PR #43. Recommend removing .squad/routing.md before merging.
\\\

## Files to Inspect

- PR title and description (stated purpose)
- \gh pr view --json files\ output (file manifest)
- Commit diff per file (spot-check suspect files)
- PR body for merge strategy notes

## Non-Bleed Scenarios

NOT considered bleed:
- Legitimate test files for the PR feature
- README or CONTRIBUTING updates (if PR documents the change)
- New src/ files (app code is in scope)
- Configuration files directly related to the feature (tsconfig.json tweaks, etc.)
242 changes: 242 additions & 0 deletions .github/PR_REQUIREMENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,242 @@
# PR Requirements — Squad Repository

> **This file is the canonical source of truth for PR requirements.**
> Issue #106 tracks the PRD; this file is the versioned spec.
> Changes to this file must go through PR review (audit trail).

---

## Definition of "User-Facing Change"

A PR contains a **user-facing change** if it introduces a CRUD operation to the CLI or SDK layer that is exposed in TypeScript or true executable functionality.

"User-facing" is anchored to two concrete package boundaries:
- **SDK layer**: `packages/squad-sdk/src/` exports exposed in `package.json` subpath exports
- **CLI layer**: `packages/squad-cli/src/cli/` commands and subcommands

### CRUD Operations — What Counts as User-Facing

| Operation | SDK Layer | CLI Layer |
|-----------|-----------|-----------|
| **Create** | New export added to `packages/squad-sdk/src/`; new subpath export added to `package.json` | New CLI command or subcommand added to `packages/squad-cli/src/cli/commands/` |
| **Read** | N/A — reading doesn't change the surface | N/A — reading doesn't change the surface |
| **Update** | Change to existing public API signature (parameters, return types, behavior) | Change to CLI command flags, behavior, or subcommand structure |
| **Delete** | Remove a previously public export from `package.json` | Remove a CLI command or subcommand |

### What Is NOT User-Facing

- `.squad/` state files (decisions, history, skills, templates)
- Internal refactors that don't change the public API surface
- Infrastructure (CI, GitHub Actions, workflows)
- Documentation-only changes
- Dependency updates that don't change API surface

Why this matters: The requirement categories below apply conditionally to user-facing changes. Knowing what counts as user-facing prevents disputes about "does this need a CHANGELOG entry?"

### Examples of "new module"

- Adding a new directory under packages/squad-sdk/src/ (e.g., src/storage/, src/casting/)
- Adding a new subpath export to package.json (e.g., ./storage, ./casting)
- Adding a new CLI command file under packages/squad-cli/src/cli/commands/

### NOT a new module

- Adding a new function to an existing module
- Refactoring internals without changing the public API surface
- Adding test files

---

## PR Requirements by Category

Requirements are ordered by review flow. All marked **REQUIRED** must be satisfied before merge. Some are automatically enforced (CI gates); others require human review.

### a) Git Hygiene (REQUIRED — automated)

- [ ] Commits reference the issue number: `Closes #N` or `Part of #N` in commit message
- [ ] No binary files or build artifacts committed (`dist/`, `node_modules/`, `.DS_Store`, etc.)
- [ ] No unrelated file changes (bleed check: only files needed for the stated goal)
- [ ] Clean commit history: logical commits or squashed to a single coherent commit
- [ ] No force-pushes to shared branches (`dev`, `main`)

**Enforced by**: squad-ci.yml bleed check + git hooks

### b) CI / Build (REQUIRED — automated)

- [ ] Build passes: `npm run build` completes successfully
- [ ] All existing tests pass: `npm test` or `npm run test`
- [ ] No new build warnings introduced without justification in PR description
- [ ] Feature flags documented in code and CHANGELOG if gating new behavior

**Enforced by**: GitHub Actions CI pipeline

### c) Code Quality (REQUIRED — manual review for completeness, automated for linting)

- [ ] All new public APIs have JSDoc/TSDoc comments
- [ ] No lint errors: `eslint` passes
- [ ] No type errors: `tsc --noEmit` passes
- [ ] Tests written for all new functionality
- [ ] All tests pass: `vitest` exits 0

**Enforced by**: eslint in CI + manual code review

### d) Documentation (REQUIRED for user-facing changes — manual review)

- [ ] CHANGELOG.md entry under `[Unreleased]` following Keep-a-Changelog format
- Example: `### Added — FeatureName (#N) — short description`
- Required when: `packages/squad-sdk/src/` or `packages/squad-cli/src/` changes
- [ ] README.md section updated if adding a new feature or module to the SDK
- [ ] Docs feature page added under `docs/src/content/docs/features/` if adding a user-facing capability
- [ ] `docs/src/navigation.ts` updated if a new docs page is added
- [ ] Docs build test updated if a new feature page is added (Playwright test in CI)

**Enforced by**: Manual PR review (FIDO, Flight) + future automated checks in #104

### e) Package / Exports (REQUIRED for new modules — manual review)

- [ ] `package.json` subpath exports updated when new modules are added
- Both types (`.d.ts`) and import (`.js`) entries required per export
- Example: the `/storage` export was missing from #640 until the audit caught it
- [ ] No accidental dependency additions: `package-lock.json` changes reviewed for unexpected deps
- [ ] No export regressions: existing exports remain present and functional

**Enforced by**: Manual PR review + future export audit gate in #104

### f) Samples (REQUIRED when API changes — manual review)

- [ ] Existing sample projects updated if the PR changes a public API they use
- [ ] New user-facing features include at least one sample demonstrating usage
- [ ] Sample README.md explains what the sample does and how to run it
- [ ] Samples build and run: `npm run build` in sample directory succeeds

**Enforced by**: Manual PR review + #103 (Samples CI coverage)

---

## Breaking Change Policy

Any PR that changes the public API in a backward-incompatible way (removal, signature change, behavior change) must:

- [ ] Document the breaking change in CHANGELOG.md under a `[BREAKING]` section
- [ ] Add a migration guide as a docs note in the affected feature page
- [ ] Update all sample projects to use the new API
- [ ] Include the migration path in the PR description (`## Breaking Changes` section)

**Examples of breaking changes:**
- Removing an export that was previously public
- Changing function signature (parameter name, order, type)
- Changing CLI command name or subcommand structure
- Changing configuration file format in backward-incompatible way

**Examples NOT breaking:**
- Adding new optional parameters
- Adding new exports
- Adding new subcommands (existing ones unchanged)
- Fixing documented behavior that was incorrect

---

## Exception / Waiver Process

Emergencies (security fixes, critical bugs) require flexibility. Any requirement can be waived if documented and approved.

### How to Request a Waiver

Add to PR description under a `## Waivers` section:

```
## Waivers

- Item waived: (d) Documentation CHANGELOG entry
- Reason: Security fix needs urgent release (CVE-2024-XXXXX)
- Approval by: [Flight/FIDO to be stated in PR review comment]
```

### Waiver Approval Authority

- **Flight (architecture decisions)**: Internal refactors, infrastructure, module reorganization
- **FIDO (quality decisions)**: Bug fixes, test improvements, samples, CI changes
- **Both**: External API changes, breaking changes

### Waiver Approval Examples

| Scenario | Waivable Items | Approver |
|----------|----------------|----------|
| Internal refactor (no API change) | Documentation, Samples | Flight |
| Security fix | Documentation, Samples (with follow-up issue) | FIDO |
| Test infrastructure change | Documentation, Exports, Samples | FIDO |
| CI/GitHub Actions update | Documentation, Exports, Samples | FIDO |
| Documentation-only change | Code Quality (tests), Exports, Samples | FIDO |
| Non-breaking SDK bugfix | Samples | FIDO |

### Self-Waiving Is Not Allowed

- Do NOT use skip labels without explicit reviewer approval in PR comments
- Do NOT remove requirements from checklist without documented waiver
- Do NOT claim "documentation will follow" without a tracked follow-up issue

---

## Edge Case Exemptions

Certain PR types are categorically exempt from specific requirements by nature. These are **NOT** waivers (no approval needed); they are structural exemptions.

| PR Type | Exempt From | Reason |
|---------|-------------|--------|
| Internal refactor (no public API change) | (d) Docs, (f) Samples | No user-visible change |
| Test-only changes | (d) Docs, (e) Exports, (f) Samples | No production code changed |
| CI/GitHub Actions workflow changes | (d) Docs, (e) Exports, (f) Samples | Infrastructure only |
| Documentation-only changes | (c) Code Quality (tests), (e) Exports, (f) Samples | No code changes |
| Dependency bumps (routine maintenance) | (d) Docs feature page, (f) Samples | Routine dependency update |

**Important**: Exemptions apply only when the PR genuinely affects NO code in the exempted categories. A PR touching both infrastructure AND public APIs is NOT exempt.

---

## PR Size Guidelines

| Size | Files Changed | Guidance |
|------|---------------|----------|
| Small | < 10 files | Preferred. Fast to review, easy to revert. |
| Medium | 10–20 files | Acceptable. Justify scope in PR description. |
| Large | > 20 files | Must be split or justified with written explanation. |
| Red flag | > 50 files | STOP — split the PR or get explicit written approval before proceeding. |

For migration PRs (> 20 files): include test output summary in the PR description.

---

## Known Limitations

### 1. Enforceability Gap

9 of 18 requirements are manual-only with no CI gate (README updates, docs page relevance, sample completeness, migration guide quality). Enforcement depends on consistent reviewer judgment.

**Automation dependency**: 9 of 18 requirements are manual-only. Issue #104 (PR completeness gates) is the critical path to automated enforcement. Until #104 ships, this spec depends on reviewer discipline.

**Mitigation**: Phase 4 (Flight/FIDO checklist) codifies consistent judgment. Regular enforcement audits catch drift.

### 2. Theater Risk

Calling something "REQUIRED" without automated enforcement creates appearance of rigor without substance. Phase 3 (#104) prioritizes high-leverage automated gates (CHANGELOG, exports, build). Manual-only items are clearly labeled.

### 3. Silent Drift Risk

This requirements file is the source of truth. CI gate scripts (#104) and reviewer checklists (#100) must be manually updated if requirements change. Quarterly review cycle to resync.

### 4. Ambiguities

- **"Module" definition**: new directory under `packages/squad-sdk/src/` or new export namespace. Refactoring existing module does not trigger export requirement.
- **"User-facing" detection for CHANGELOG**: Currently manual. #104 will implement export-diff detection.

---

## Relationship to Other Issues

| Issue | Role |
|-------|------|
| #100 (Review completeness) | Defines the review *process* — this file defines *what reviewers check against* |
| #103 (Samples CI coverage) | Implements the "samples" requirement category (f) |
| #104 (PR completeness gates) | Implements automated enforcement of REQUIRED items as CI gates |

This file is the source of truth. #100 and #104 derive their checklists from it.
Loading
Loading