Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions .changeset/loop-command.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@bradygaster/squad-cli": minor
---

feat(cli): Add `squad loop` command — prompt-driven continuous work loop

New `squad loop` command reads a `loop.md` file and runs it as a continuous work loop.
No GitHub issues required — the prompt is the work driver. Includes `--init` to scaffold
a boilerplate loop file, frontmatter validation, and composable capability flags.
26 changes: 26 additions & 0 deletions .squad/agents/eecom/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,32 @@

## Learnings

### Loop command: second-round review fixes (#767) (2025-07-26)

**Context:** Three Copilot review comments on PR #767: (1) `teamRoot` was set to `workTreeRoot` but `.squad/` may live in the main checkout when running inside a git worktree — should derive from `detectSquadDir().path`, (2) `generateLoopFile()` hardcoded the full loop.md scaffold inline, duplicating `templates/loop.md`, (3) docs said `gh` was optional but code hard-requires `gh copilot` unless `--agent-cmd` is passed.

**Fixes:**
1. **teamRoot:** Changed `const teamRoot = workTreeRoot` to `path.dirname(squadDirInfo.path)` — `.squad/`-relative operations now always use the directory where `.squad/` was actually found.
2. **Template dedup:** Replaced 48-line hardcoded template with `readFileSync` reading from `templates/loop.md`. Used `import.meta.url` + `fileURLToPath` to resolve the path from the compiled file (3 levels up to package root). Created `packages/squad-cli/templates/loop.md` since it was missing from the CLI package's templates dir.
3. **Docs:** Updated prerequisites to state `gh` + `gh copilot` are required by default, with `--agent-cmd` as the escape hatch.

**Test impact:** Tests mock `node:fs` globally, so `readFileSync` in `generateLoopFile()` needed mock setup. Added `beforeAll` using `vi.importActual('node:fs')` to read the REAL template file, then `beforeEach` to set the mock return value. This keeps tests validating the actual template content.

**Pattern:** When reading template files in code that has mocked `node:fs` tests, use `vi.importActual<typeof import('node:fs')>('node:fs')` in `beforeAll` to get real filesystem access for loading test fixtures.

### Loop command: streaming output, worktree CWD, docs alignment (#767) (2025-07-25)

**Context:** Copilot code review on PR #767 flagged three issues in the loop command: (1) `execFile` buffered stdout/stderr but never printed it — users saw no Copilot output during loop rounds, (2) `loop.md` was resolved relative to `dest` but execution used `teamRoot` (derived from `.squad/` parent), creating a CWD mismatch in worktree scenarios, (3) docs said `description` defaults to `""` but code uses `'Squad Loop'`.

**Fixes:**
1. **Streaming:** Added `.on('data')` listeners to `currentChild.stdout` and `currentChild.stderr` after `execFile` spawn. Since output streams in real-time, the callback no longer re-writes buffered stdout/stderr on error (would duplicate).
2. **Worktree CWD:** Introduced `workTreeRoot = path.resolve(dest)` and set `teamRoot = workTreeRoot`. Both file resolution and execution CWD now use the same root.
3. **Docs:** Updated `docs/src/content/docs/features/loop.md` description default from `""` to `"Squad Loop"`.

**Key file:** `packages/squad-cli/src/cli/commands/loop.ts` — `runLoop()` entry point (~line 302), `executeRound()` inner function (~line 427).

**Pattern:** When using Node's `execFile` for interactive/long-running child processes, always attach stream listeners for real-time output. The callback's `stdout`/`stderr` args are the same buffered content — writing both duplicates output.

### archiveDecisions() count-based fallback (#626) (2025-07-24)

**Context:** `archiveDecisions()` in `packages/squad-cli/src/cli/core/nap.ts` silently returned `null` when all `###` entries were <30 days old (`old.length === 0`), even if the file was well over 20KB. Active projects generating many decisions per session could hit 145KB+ — 35K tokens burned per agent spawn.
Expand Down
9,484 changes: 6,523 additions & 2,961 deletions .squad/decisions-archive.md

Large diffs are not rendered by default.

7,847 changes: 0 additions & 7,847 deletions .squad/decisions.md

Large diffs are not rendered by default.

48 changes: 48 additions & 0 deletions .squad/log/2026-03-27T21-42-00Z-decisions-archival.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Archival Log: Decisions

**Timestamp:** 2026-03-27T21:42:00Z
**Agent:** Scribe (Copilot)
**Trigger:** Automated archival gate

## Task
Archive decisions.md entries older than 7 days to free up context. Decision entries from 2026-02-21 through 2026-03-25 moved to append-only archive.

## Outcome
✅ SUCCESS

## Changes
- **decisions.md**: Trimmed from 385KB → 11.9KB
- Foundational directives preserved (Type safety, Hook governance, Node.js runtime, etc.)
- Recent entries (2026-03-26+) preserved
- 71 dated entries (2026-02-21:2026-03-25) archived

- **decisions-archive.md**: Created 365.1KB archive
- 71 entries preserved append-only
- Full history intact for reference
- No data loss

## Verification
- ✅ Archive gate (decisions.md < 20KB): **PASS**
- ✅ All foundational directives present in decisions.md
- ✅ All archived entries present in decisions-archive.md
- ✅ Git commit successful: `b8b4c5ae`

## Measurements
| Metric | Value |
|--------|-------|
| Entries archived | 71 |
| Archive date range | 2026-02-21 : 2026-03-25 |
| decisions.md pre | 385 KB |
| decisions.md post | 11.9 KB |
| decisions-archive.md | 365.1 KB |
| Reduction | 96.9% |
| Context savings | ~373 KB |

## Summary
Successfully archived 71 dated decision entries spanning 33 days of team decisions. The decisions.md file is now below the 20KB archival gate threshold (11.9KB), reducing agent context load while preserving full history in append-only archive. All foundational directives remain in active decisions.md for team reference.

## Impact
- **Agent context efficiency:** All agents now load ~373KB less decision context
- **No functionality loss:** Archived decisions remain accessible via decisions-archive.md
- **Append-only integrity:** Archive file can only grow; merge conflicts impossible
- **Future gate compliance:** Next archival not needed until decisions.md reaches 20KB again
268 changes: 268 additions & 0 deletions .squad/skills/pr-review-response/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,268 @@
---
name: "pr-review-response"
description: "Teaches agents to reply to PR review comment threads after fixing issues, making resolutions traceable"
domain: "pull-requests, code-review, traceability"
confidence: "low"
source: "observed (agents fix review feedback silently — reviewers can't tell which comments were addressed)"
tools:
- name: "github-mcp-server-pull_request_read"
description: "Read PR review threads and comments"
when: "Step 1 — fetching review comments to understand what needs fixing"
- name: "gh api (REST)"
description: "Reply to review comment threads and resolve threads via GraphQL"
when: "Step 3 — posting reply to each comment thread after fixing"
---

## Context

When an agent fixes code in response to PR review comments (from Copilot, a human reviewer, or any GitHub reviewer), the fix alone is not enough. The reviewer needs to see — on the PR thread itself — which comments were addressed and how. Without replies, comments stay visually unresolved, reviewers must re-read the entire diff to verify fixes, and there's no traceable link between feedback and resolution.

Use this skill whenever:
- You are fixing code based on PR review feedback
- You are addressing Copilot review suggestions
- You are responding to reviewer-requested changes on a PR
- A squad member hands you review comments to resolve

## SCOPE

✅ THIS SKILL PRODUCES:
- Reply comments on each review thread explaining the fix
- Optionally resolved threads (via GraphQL when appropriate)
- Commit messages that reference the PR and review context

❌ THIS SKILL DOES NOT PRODUCE:
- The code fixes themselves (that's the agent's domain work)
- New review comments or reviews
- PR descriptions or summaries

## Patterns

### Step 1: Read the review comments

**Using MCP tools (preferred when available):**

```
github-mcp-server-pull_request_read
method: "get_review_comments"
owner: "{owner}"
repo: "{repo}"
pullNumber: {pr_number}
```

This returns review threads with metadata: `isResolved`, `isOutdated`, `isCollapsed`, and their associated comments. Each comment has an `id` you'll need for replies.

**Using gh CLI (fallback):**

```bash
gh api repos/{owner}/{repo}/pulls/{pr_number}/comments --paginate
```

Each comment object contains `id`, `body`, `path`, `line`, and `in_reply_to_id`. Top-level comments have no `in_reply_to_id` — those are the ones you reply to.

### Step 2: Fix the code

Make the actual code changes. This is your normal domain work — the skill doesn't prescribe how to fix, only how to communicate the fix.

**Track what you changed.** For each review comment, note:
- The comment `id` (top-level, not a reply)
- The file and line referenced
- What you actually changed (brief description)
- The commit SHA after pushing (if available)

### Step 3: Reply to each review thread

After fixing and committing, reply to **each** review comment thread individually.

**REST API call (via gh CLI):**

```bash
gh api repos/{owner}/{repo}/pulls/{pr_number}/comments/{comment_id}/replies \
-f body="Fixed in {sha_short} — {brief description of what was changed}"
```

**Important:** `{comment_id}` must be the ID of the **top-level** comment in the thread. You cannot reply to a reply — only to the original review comment.

**Example replies:**

```bash
# Specific and traceable
gh api repos/bradygaster/squad/pulls/42/comments/18234/replies \
-f body="Fixed in a1b2c3d — switched to path.dirname(squadDirInfo.path) for worktree consistency"

# When applying a suggested code change
gh api repos/bradygaster/squad/pulls/42/comments/18235/replies \
-f body="Applied suggestion — updated error message to include the file path for debuggability"

# When pushing back on a suggestion
gh api repos/bradygaster/squad/pulls/42/comments/18236/replies \
-f body="Considered but not applied — this path needs to stay absolute because worktree resolution depends on it. See detectSquadDir() in detect-squad-dir.ts."
```

### Step 4: Resolve threads (optional, GraphQL only)

Thread resolution is only available via the GitHub GraphQL API. Use this when your fix fully addresses the comment and no further discussion is needed.

**First, get the thread IDs** (they're different from comment IDs):

```bash
gh api graphql -f query='
query {
repository(owner: "{owner}", name: "{repo}") {
pullRequest(number: {pr_number}) {
reviewThreads(first: 100) {
nodes {
id
isResolved
comments(first: 1) {
nodes { body databaseId }
}
}
}
}
}
}
'
```

Match thread IDs to comment IDs using `databaseId`, then resolve:

```bash
gh api graphql -f query='
mutation {
resolveReviewThread(input: {threadId: "{thread_node_id}"}) {
thread { id isResolved }
}
}
'
```

**When to resolve vs. leave open:**
- ✅ Resolve: You fixed exactly what was requested, no ambiguity
- ❌ Don't resolve: You pushed back, applied a different fix, or the comment needs further discussion
- ❌ Don't resolve: The reviewer is a human — let them confirm and resolve themselves

**Rule of thumb:** Agent-to-agent threads (e.g., Copilot review → agent fix) can be resolved by the fixer. Human reviewer threads should be left for the human to resolve.

### Step 5: Commit message traceability

Commit messages should reference the PR context:

```
fix: address review feedback on PR #{pr_number}

- Switched to path.dirname() for worktree path resolution (comment #18234)
- Updated error message to include file path (comment #18235)

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

For single-comment fixes, a shorter format works:

```
fix: use path.dirname() for worktree consistency (PR #{pr_number} review)

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

## AGENT WORKFLOW (Summary)

1. **READ** — Fetch review threads using MCP tool or `gh api`
2. **FIX** — Make code changes, tracking comment ID → change mapping
3. **COMMIT** — Push with traceable commit message referencing PR and comments
4. **REPLY** — Post individual reply to each thread via `gh api .../replies`
5. **RESOLVE** — (Optional) Resolve agent-to-agent threads via GraphQL
6. **STOP** — Do not batch-reply, do not skip threads, do not resolve human threads

## Examples

### Example: Copilot flags a potential null dereference

**Review comment (id: 55123):**
> `squadDir` could be undefined here. Consider adding a null check.

**Agent workflow:**
1. Read the comment via `get_review_comments`
2. Add the null check in `src/cli/core/detect-squad-dir.ts`
3. Commit: `fix: add null check for squadDir (PR #99 review)`
4. Reply:
```bash
gh api repos/bradygaster/squad/pulls/99/comments/55123/replies \
-f body="Fixed in f4e5d6c — added early return when squadDir is undefined, matching the pattern in loadConfig()"
```
5. Resolve the thread (Copilot → agent, safe to resolve)

### Example: Multiple review comments on one PR

**Comments:**
- id: 55123 — "Null check needed" on `detect-squad-dir.ts:42`
- id: 55124 — "Consider using path.join()" on `detect-squad-dir.ts:58`
- id: 55125 — "This log message is too verbose" on `output.ts:15`

**Agent handles each individually:**
```bash
# Fix all three, commit
git add packages/squad-cli/src/cli/core/detect-squad-dir.ts packages/squad-cli/src/cli/core/output.ts
git commit -m "fix: address 3 review comments on PR #99

- Added null check for squadDir (comment #55123)
- Switched to path.join() for cross-platform paths (comment #55124)
- Reduced log verbosity to debug level (comment #55125)"

git push

# Reply to each thread individually
gh api repos/bradygaster/squad/pulls/99/comments/55123/replies \
-f body="Fixed — added early return when squadDir is undefined"

gh api repos/bradygaster/squad/pulls/99/comments/55124/replies \
-f body="Fixed — switched to path.join(squadDir, 'config.json') for cross-platform consistency"

gh api repos/bradygaster/squad/pulls/99/comments/55125/replies \
-f body="Fixed — changed from console.log to debug() so it only shows with --verbose flag"
```

### Example: Handling Copilot suggestion blocks

Copilot sometimes provides `suggestion` blocks with exact code to apply:

**Review comment (id: 55130):**
````
Consider using optional chaining:
```suggestion
const name = config?.agent?.name ?? 'default';
```
````

**Reply format when applying:**
```bash
gh api repos/bradygaster/squad/pulls/99/comments/55130/replies \
-f body="Applied suggestion — using optional chaining with nullish coalescing"
```

**Reply format when not applying:**
```bash
gh api repos/bradygaster/squad/pulls/99/comments/55130/replies \
-f body="Not applied — config is guaranteed non-null at this point (validated on line 12). Optional chaining would mask errors."
```

### Example: Pushing back on a review comment

Not every review comment should be accepted. When a suggestion is incorrect or doesn't apply:

```bash
gh api repos/bradygaster/squad/pulls/99/comments/55140/replies \
-f body="Considered but not applied — this file is in the zero-dependency bootstrap set (see copilot-instructions.md § Protected Files). Adding path.join() would require importing from the SDK, which breaks the bootstrap constraint."
```

Do NOT resolve the thread when pushing back. Leave it open for the reviewer to confirm.

## Anti-Patterns

- ❌ **Fixing silently** — Making code changes without replying to the review thread. The reviewer has no way to know which comments were addressed.
- ❌ **Batch-replying "all fixed"** — A single comment saying "Addressed all review feedback" on the PR. Each thread needs its own reply so reviewers can verify individually.
- ❌ **Resolving without explaining** — Marking threads resolved without posting a reply first. The resolution gives no context on what was done.
- ❌ **Resolving human reviewer threads** — Only resolve threads from automated reviewers (Copilot, bots). Let human reviewers confirm and resolve their own threads.
- ❌ **Vague replies** — "Fixed" or "Done" without saying what was changed. The reply should be specific enough that the reviewer doesn't need to re-read the diff.
- ❌ **Replying before pushing** — Reply after your fix is committed and pushed, not before. The reply should reference actual committed code.
- ❌ **Ignoring comments you disagree with** — If you don't apply a suggestion, reply explaining why. Silence looks like you missed it.
- ❌ **Replying to replies** — The REST API only supports replying to top-level review comments. Attempting to reply to a reply will fail with a 404.
Loading
Loading