git:20261005.317bf30 to git:20261006.f33ce3a

---

name: loom-builder-pr

description: "This document covers PR creation, test output handling, and quality requirements for the Builder role. For the core builder workflow, see `builder.md`."

---

<!-- loom-managed-skill -->

<!-- GENERATED FILE — DO NOT EDIT DIRECTLY.

Produced by `loom-daemon generate-agent-skills` from

defaults/.claude/commands/loom/builder-pr.md (the same source Claude Code

reads as /loom:builder-pr via .claude/commands/loom/). This is the

cross-vendor skill-discovery surface (.agents/skills/<name>/SKILL.md)

read natively by Codex, Kimi Code, Mistral Vibe, and Grok — see

runtime-adapters.md §5. To change this file, edit the source above

and re-run the generator; CI (`loom-daemon generate-agent-skills

--check`) fails if this file is stale. -->

# Builder: PR Creation and Quality

This document covers PR creation, test output handling, and quality requirements for the Builder role. For the core builder workflow, see `builder.md`.

<!-- toc:begin generated by scripts/check-doc-tocs.sh --fix; do not edit by hand -->

**Contents**

- [Pre-Implementation Review: Check Recent Main Changes](#pre-implementation-review-check-recent-main-changes)

- [Test Output: Truncate for Token Efficiency](#test-output-truncate-for-token-efficiency)

- [Acceptance Criteria Verification: REQUIRED Before PR Creation](#acceptance-criteria-verification-required-before-pr-creation)

- [Test-First Discipline (TDD line, required in PR body)](#test-first-discipline-tdd-line-required-in-pr-body)

- [PR Titles: Conventional Commit Style Required](#pr-titles-conventional-commit-style-required)

- [Commit Messages: Same Rules as PR Titles](#commit-messages-same-rules-as-pr-titles)

- [Creating Pull Requests: Label and Auto-Close Requirements](#creating-pull-requests-label-and-auto-close-requirements)

- [Handling Pre-existing Lint/Build Failures](#handling-pre-existing-lintbuild-failures)

- [Raising Concerns](#raising-concerns)

<!-- toc:end -->

## Pre-Implementation Review: Check Recent Main Changes

**CRITICAL:** Before implementing, review recent changes to main to avoid conflicts with recent architectural decisions.

### Why This Matters

The codebase evolves while you work. Recent PRs may have:

- Introduced new utilities or helper functions you should use

- Changed authentication/authorization patterns

- Updated API conventions or response formats

- Added shared components or abstractions

- Modified configuration or environment handling

**Without this review**: You may implement using outdated patterns, leading to merge conflicts, inconsistent code, or duplicated functionality.

### Required Commands

**Step 1: Review recent commits to main**

```bash

# Show last 20 commits to main

git fetch origin main

git log --oneline -20 origin/main

# Show files changed in recent commits

git diff HEAD~20..origin/main --stat

```

**Step 2: Check changes in your feature area**

```bash

# Check recent changes in directories related to your feature

git log --oneline -10 origin/main -- "src/relevant/path"

git log --oneline -10 origin/main -- "*.ts" # or relevant file types

```

**Step 3: Look for these architectural changes**

| Change Type | What to Look For | Why It Matters |

|-------------|------------------|----------------|

| **Authentication** | New auth middleware, session handling, token patterns | Use the new auth approach, not old patterns |

| **API Patterns** | Response formats, error handling, validation | Match existing conventions |

| **Utilities** | New helper functions, shared modules | Reuse instead of reimplementing |

| **Shared Components** | Common UI elements, base classes | Extend rather than duplicate |

| **Configuration** | New env vars, config patterns | Follow established patterns |

### Example Workflow

```bash

# 1. Fetch latest main

git fetch origin main

# 2. See what changed recently

git log --oneline -20 origin/main

# 5b55cb7 Add dependency unblocking to Guide role (#997)

# cc41f95 Add guidance for handling pre-existing lint/build failures (#982)

# 6b55a3e Add rebase step before PR creation (#980)

# ...

# 3. If you see relevant changes, investigate

git show 5b55cb7 --stat # See what files changed

git show 5b55cb7 # See the actual changes

# 4. Check changes in your feature area

git log --oneline -10 origin/main -- "src/lib/auth"

# -> If you see auth changes, read them before implementing!

# 5. Adapt your implementation plan based on findings

```

### When to Skip This Step

- **Trivial fixes**: Typos, documentation, obvious bugs

- **Isolated changes**: Changes that don't interact with other code

- **Fresh main**: You just pulled main and no time has passed

### Integration with Worktree Workflow

This review happens BEFORE creating your worktree:

1. Read issue (with comments)

2. **Review recent main changes** (YOU ARE HERE)

3. Check dependencies

4. Create worktree

5. Implement (using patterns learned from review)

## Test Output: Truncate for Token Efficiency

**IMPORTANT**: When running tests, truncate verbose output to conserve tokens in long-running sessions.

### Why Truncate?

Test output can easily exceed 10,000+ lines, consuming significant context:

- Full test suites dump every passing test

- Stack traces repeat for related failures

- Coverage reports add thousands of lines

- This wastes tokens and pollutes context for subsequent work

### Truncation Strategies

**Option 1: Failures + Summary Only (Recommended)**

```bash

# Run tests, capture only failures and summary

pnpm test 2>&1 | grep -E "(FAIL|PASS|Error|Summary|Tests:)" | head -100

# Or use test runner's built-in options

pnpm test --reporter=dot # Minimal output (dots for pass/fail)

pnpm test --silent # Suppress console.log from tests

pnpm test --onlyFailures # Re-run only failed tests

```

**Option 2: Tail for Summary**

```bash

# Get just the final summary

pnpm test 2>&1 | tail -30

```

**Option 3: Head + Tail**

```bash

# First 20 lines (test start) + last 30 lines (summary)

pnpm test 2>&1 | (head -20; echo "... [truncated] ..."; tail -30)

```

**Option 4: Grep for Failures**

```bash

# Show only failing tests and their immediate context

pnpm test 2>&1 | grep -A 5 -B 2 "FAIL\|Error"

```

### When Full Output Is Needed

Sometimes you need full output for debugging:

- First run after major changes (to see all failures)

- Investigating intermittent failures

- Understanding test coverage gaps

In these cases, run full output but don't include it all in your response. Instead:

1. Run the full test suite

2. Analyze the output

3. Report only relevant failures in your response

4. Include actionable summary, not raw dumps

### Example: Good Test Reporting

**Instead of dumping 500 lines of output:**

```

Test Results: 3 failures

1. `src/lib/state.test.ts` - "should update terminal config"

- Expected: { name: "Builder" }

- Received: undefined

- Likely cause: Missing null check in updateTerminal()

2. `src/lib/worktree.test.ts` - "should create worktree"

- Error: ENOENT: no such file or directory

- Likely cause: Test cleanup not running

3. `src/main.test.ts` - "should initialize app"

- Timeout after 5000ms

- Likely cause: Async setup not awaited

Summary: 47 passed, 3 failed, 50 total

```

This gives you all the information needed to fix issues without wasting tokens on verbose output.

## Acceptance Criteria Verification: REQUIRED Before PR Creation

**CRITICAL:** Before creating a PR, you MUST explicitly verify that ALL acceptance criteria from the issue are met. This prevents incomplete PRs that require manual intervention during review.

### Why This Matters

During orchestration, incomplete PRs cause:

- CI failures when criteria are missed

- Manual intervention mid-workflow

- Wasted review cycles

- Sweep/Judge time spent on fixable issues

**Example failure**: Issue #1441 listed 4 shellcheck warnings to fix. Builder fixed 3/4, missed `cli/loom-start.sh:47`, requiring manual fixes after CI failed.

### Step 1: Extract Acceptance Criteria

Before starting implementation, extract ALL acceptance criteria from the issue:

**Look for these patterns in issue body and comments:**

| Pattern | Example |

|---------|---------|

| Checkbox items | `- [ ] Fix shellcheck warning in file.sh` |

| Numbered requirements | `1. Add validation to input` |

| "must"/"should"/"required" statements | `Must handle edge case X` |

| Explicit test conditions | `Verify that Y works when Z` |

| File-specific changes | `Update config.json to include...` |

**Create a working checklist:**

```markdown

## My Acceptance Criteria Checklist

From issue #123:

- [ ] Fix shellcheck warning in `scripts/build.sh:42`

- [ ] Fix shellcheck warning in `scripts/test.sh:15`

- [ ] Fix shellcheck warning in `scripts/deploy.sh:88`

- [ ] All `find ... -exec shellcheck` returns 0 warnings

```

### Step 2: Verify Each Criterion During Implementation

As you complete each item, explicitly verify it works:

```bash

# Example: Issue says "Fix all shellcheck warnings in scripts/"

# DON'T just fix what you find - verify the COMPLETE list from the issue

# If issue lists specific files:

shellcheck scripts/build.sh # Check file 1

shellcheck scripts/test.sh # Check file 2

shellcheck scripts/deploy.sh # Check file 3

# etc. - check EVERY file mentioned

# If issue says "all shellcheck warnings":

find scripts -name "*.sh" -exec shellcheck {} \; 2>&1 | grep -c "error\|warning"

# Must return 0

```

### Step 3: Pre-PR Verification Checklist

**BEFORE opening the PR (`./.loom/scripts/create-pr.sh`, see "Creating the PR"), complete this checklist:**

```markdown

## Pre-PR Verification

Issue #123 acceptance criteria:

1. [ ] Criterion A - Verified by: [describe how you checked]

2. [ ] Criterion B - Verified by: [describe how you checked]

3. [ ] Criterion C - Verified by: [describe how you checked]

Root cause verification (for process/behavior issues):

- [ ] Changes address root cause, not just surface symptom

- [ ] Fix is structural (enforcement, validation, inlining) not just documentation

- [ ] If documentation-only: justified why docs will change behavior this time

Local verification:

- [ ] The project's check command passes (see `buildGate.command` in `.loom/config.json`, or the repo's documented CI command, e.g. `pnpm check:ci`)

- [ ] Formatter + linter run on changed files (see "Format and Lint Changed Files" below) — a format-only CI failure is a guaranteed Judge rejection

- [ ] Commits are signed off if required (`commit.signoff: true` in `.loom/config.json`, or a DCO/`sign-off` requirement — `git commit --signoff`; see "DCO sign-off" above)

- [ ] Relevant tests pass

- [ ] Each criterion has explicit verification (not "I think it works")

- [ ] Ran the "defaults/ Version-Bearing-Files Gate" command block below (not just read it) — exited 0. It fails if this PR hand-edits any version-bearing file's value (`package.json`, `mcp-loom/package.json`, `Cargo.toml`, `VERSION`) — those are now bumped automatically at merge time (#7743), never by hand in a feature PR.

```

**Run the defaults/ Version-Bearing-Files Gate locally — an actual command, not a checklist bullet to read (#6675, recurring Judge rejection: #6598, #6599, #6610, #6611, #6630, #6668 all hit this in CI because it was never run pre-PR). A single automated workflow (`.github/workflows/version-bump-on-merge.yml`, #7743) now owns bumping `VERSION` and the other version-bearing files, once, right after any merge that touched `defaults/` — a feature PR must not carry its own edit to any of them. `./.loom/scripts/create-pr.sh` also runs a version-bearing-file consistency check (`version-check-gate.sh`, #6730) before creating the PR, so running the block below by hand is defense-in-depth, not the only line of defense; still run it locally to catch a hand-edit before pushing rather than at CI time:**

```bash

# Run from your worktree, AFTER your last commit, BEFORE

# ./.loom/scripts/create-pr.sh. Mirrors the CI job "PRs Must Not Hand-Edit

# Version-Bearing Files" (.github/workflows/ci.yml) — no PR exists yet at

# Builder time, so use the merge-base with origin/main as --base instead of

# a PR base sha. (`--forbid-bump` also narrows to merge-base(base, head)

# internally since #7823, so passing one here is idempotent, not redundant

# belt-and-braces you could drop: it keeps this block correct on an older

# installed copy of the script too.)

MERGE_BASE="$(git merge-base origin/main HEAD)"

# Exit 0 = no version-bearing file's VALUE changed anywhere in your diff;

# exit 1 = your PR hand-edits one of package.json/mcp-loom/package.json/

# Cargo.toml/VERSION. Do NOT "fix" a failure here by running

# ./scripts/version.sh bump patch -- that command is now exclusively the

# post-merge workflow's job; a Builder should never run it. Revert the

# edit(s) to the flagged file(s) instead.

if ! bash defaults/scripts/check-defaults-version-bump.sh --forbid-bump --base "$MERGE_BASE" --head HEAD; then

echo "BLOCKER: this PR hand-edits a version-bearing file's value (see the diff above)." >&2

echo "Fix: revert the change(s) to that file -- version bumps happen automatically at merge (#7743), never in a feature PR." >&2

exit 1

fi

echo "OK: no version-bearing file's value was hand-edited."

```

**Treat a non-zero exit above as a hard local blocker** — revert your edit to the flagged file(s) and re-run the block until it prints the final `OK:` line before calling `create-pr.sh`. Do not proceed on the strength of having merely read the checklist bullet.

### Step 4: Document Verification in PR Description

Include criterion verification in your PR description:

```markdown

## Summary

Fix shellcheck warnings in deployment scripts.

## Acceptance Criteria Verification

| Criterion | Status | Verification |

|-----------|--------|--------------|

| Fix `scripts/build.sh:42` | ✅ | `shellcheck scripts/build.sh` returns no warnings |

| Fix `scripts/test.sh:15` | ✅ | `shellcheck scripts/test.sh` returns no warnings |

| Fix `scripts/deploy.sh:88` | ✅ | `shellcheck scripts/deploy.sh` returns no warnings |

| All warnings resolved | ✅ | `find scripts -name "*.sh" -exec shellcheck {} \;` returns 0 |

Closes #123

```

### Common Verification Commands

| Criterion Type | Verification Command |

|----------------|---------------------|

| Shellcheck fixes | `shellcheck <file>` or `find ... -exec shellcheck {} \;` |

| TypeScript errors | `pnpm tsc --noEmit` |

| Lint issues | `pnpm lint` or scoped `biome check <file>` |

| Rust compilation | `cargo check` (see Language-Specific section below) |

| Rust linting | `cargo clippy` (see Language-Specific section below) |

| Rust formatting | `cargo fmt --all -- --check` (see Language-Specific section below) |

| Test passes | `pnpm test -- <pattern>` |

| File exists/content | `cat <file>` or `grep <pattern> <file>` |

| Config changes | Read file and verify expected content |

### Format and Lint Changed Files: MANDATORY Before Committing

**Run the project's formatter and linter on your changed files before every

commit, not just before opening the PR.** A format-only CI failure (e.g. `cargo

fmt --check` / `ruff format --check` / `biome format --check` catching an

unformatted file with otherwise-correct code) is a **guaranteed Judge

rejection** — Judge never approves with a failing required check (see "PR

Creation Checklist" below), so a one-command mechanical fix costs a full

Doctor -> Judge cycle (an extra dispatch, re-review, and CI wait). This

happened repeatedly in production (kicad-tools PRs #4532/#4533/#4535, #4882):

otherwise approve-ready PRs rejected solely on `ruff format --check`.

**Discover the commands from repo convention** — don't skip this because the

language or toolchain is unfamiliar:

1. Check `buildGate.command` in `.loom/config.json` (may already run

format+lint as part of the project check command).

2. Check `CONTRIBUTING.md`, `package.json` scripts, a `Makefile`, or the CI

workflow (`.github/workflows/*.yml`) for the documented format/lint

commands.

3. Fall back to the language's standard tool:

| Language | Format | Lint |

|----------|--------|------|

| Rust | `cargo fmt` | `cargo clippy` |

| Python | `ruff format <files>` (or `black <files>`) | `ruff check <files>` |

| TypeScript/JavaScript | `biome format --write <files>` / `prettier --write <files>` | `biome check <files>` / `eslint <files>` |

| Go | `gofmt -w <files>` | `go vet ./...` |

| Shell | — | `shellcheck <file>` |

**Scope to your changed files** (same pattern as "Handling Pre-existing

Lint/Build Failures" below) so you don't pull unrelated files into your PR:

```bash

# Example: Python project

git diff --name-only origin/main -- '*.py' | xargs -r uv run ruff format

git diff --name-only origin/main -- '*.py' | xargs -r uv run ruff check

```

Add to your pre-PR checklist:

```markdown

Local verification:

- [ ] Formatter run on changed files (no remaining diff / `--check` passes)

- [ ] Linter run on changed files (0 errors)

```

### Language-Specific Verification

**Rust Code Changes**

If you modified any `.rs` files, run these checks **before committing**:

```bash

# Compile check - catches type errors, borrow issues, async Send violations

cargo check

# Lint - catches common mistakes, anti-patterns, correctness issues

cargo clippy

# Format all Rust files (applies formatting)

cargo fmt

# Verify formatting (check only, no changes - returns non-zero if unformatted)

cargo fmt --all -- --check

```

**Add to your pre-PR checklist when modifying Rust:**

```markdown

Local verification:

- [ ] The project's check command passes (`buildGate.command` in `.loom/config.json`, e.g. `pnpm check:ci`)

- [ ] `cargo check` returns 0 (Rust files only)

- [ ] `cargo clippy` returns 0 (Rust files only)

- [ ] `cargo fmt --all -- --check` returns 0 (Rust files only)

```

### Red Flags: Don't Create PR Yet

**STOP and verify if:**

- You haven't explicitly checked each criterion from the issue

- You're unsure if a criterion is met ("it should work")

- The issue mentions files you haven't touched

- CI might fail on something you didn't test locally

**Instead:**

1. Go back to Step 1 and re-extract criteria

2. Verify each one explicitly

3. Only then create the PR

---

## Test-First Discipline (TDD line, required in PR body)

**Why**: Loom's cross-session `Builder → Judge` cycle is the *between-turn* half

of an evaluator-optimizer loop (`CLAUDE.md` § "Sweep Lifecycle"). What's

missing is a checkable trace of the *in-Builder* half — did the test exist

before the fix, not just after it. This section is the concrete signal ADR-0015

(`docs/adr/0015-builder-test-first-checkpoint.md`) decided on, adapted from

[damusix/atomic-claude](https://github.com/damusix/atomic-claude)'s

maker/checker split (issue #5849, evaluation `docs/research/atomic-claude-evaluation.md`).

**When your diff touches executing code** (anything Judge would run tests

against — not a docs-only, ADR-only, or pure-config change), your PR body's

`## Test Plan` section MUST include one `TDD:` line:

```

TDD: yes — <test path> written first; failed for the right reason before the fix, passes after

TDD: no — <reason, e.g. "pure refactor, existing coverage in tests/foo_test.rs already exercises this path">

```

**How to earn a `TDD: yes` line — practice this while implementing, not

retroactively while writing the PR body:**

- **New behavior**: write the test first, run it, confirm it fails for the

right reason (not a typo/syntax error) — then implement until it passes.

- **Bug fix**: write a test that reproduces the bug (fails on the pre-fix

code) — then fix — then confirm it passes.

- If you did this, the `TDD:` line's `<test path>` must be a real path in your

diff — Judge checks it against the changed-files list (see

`judge.md` § "Test-First (TDD) Claim Verification"). Do not write `yes` for

a test you added *after* confirming the fix already worked; that is exactly

the unverified self-report this checkpoint exists to catch. If you didn't

actually write the test first, write `TDD: no` and say why (e.g. "test

added after implementation for coverage, not written first") — an honest

`no` is advisory-only; a false `yes` is a blocking Judge finding.

- **When there is nothing to test-first** (design/investigation issues like

this one, docs, ADRs, `.github/labels.yml`, config-only changes), use

`TDD: no — <reason>` or omit the line entirely — both are advisory, never

blocking. Do not invent a placeholder test to satisfy this checkpoint.

**This is advisory on absence, blocking only on contradiction** — a missing

line or a plausible `TDD: no` never blocks approval; a `TDD: yes` claim the

diff does not corroborate does. Full decision and rationale (including why

this isn't a `buildGate` check): ADR-0015.

---

## PR Titles: Conventional Commit Style Required

**CRITICAL:** PR titles MUST describe the actual change. Never use generic or issue-referencing titles.

### Format

```

<type>: <concise summary of what the code change does>

```

### Allowed Prefixes

| Prefix | When to Use |

|--------|------------|

| `fix:` | Bug fixes |

| `feat:` | New features or capabilities |

| `refactor:` | Code restructuring without behavior change |

| `docs:` | Documentation-only changes |

| `test:` | Adding or updating tests |

| `chore:` | Build, config, or tooling changes |

| `perf:` | Performance improvements |

### How to Derive the Title

1. **Look at your diff**, not the issue title — what files changed and what do the changes accomplish?

2. **Pick the correct prefix** based on the nature of the change (bug fix → `fix:`, new capability → `feat:`, etc.)

3. **Summarize the change itself** in a few words — a reader should understand the change without opening the PR

4. **Keep it under 70 characters** total (prefix + description)

**Ask yourself:** "If someone reads only this title in `git log --oneline`, will they understand what changed?" If the answer is no, rewrite it.

### Examples

**WRONG (generic body that just references the issue):**

```

feat: implement changes for issue #2584

fix: address issue #2557

feat: implement feature from issue #123

```

**WRONG (bare issue number):**

```

Issue #2557

```

**WRONG (raw issue title copied as PR title):**

```

Builder should generate descriptive PR titles instead of generic 'Issue #N'

```

**WRONG (double prefix — issue title's own prefix copied and another prefix prepended):**

```

feat: bug: MCP status bar noise misclassifies builder failures as MCP failures

fix: feat: add workspace snapshot caching for daemon state

```

**CORRECT (describes what the code change actually does):**

```

fix: standardize timestamp format to ISO 8601 UTC across log scripts

feat: add workspace snapshot caching for daemon state

refactor: rename instant-exit to low-output terminology

docs: update troubleshooting guide for worktree cleanup

fix: prevent duplicate label transitions in sweep phase validation

```

### Issue Title Prefix Mapping

Issue titles sometimes use non-standard prefixes (like `bug:`) that are **not** valid conventional commit types. If you're tempted to use an issue title as inspiration for your PR title, be aware that you must strip and remap the issue prefix — never copy it verbatim.

| Issue Title Prefix | Correct PR Prefix | Notes |

|-------------------|-------------------|-------|

| `bug:` | `fix:` | `bug:` is not a conventional commit type |

| `feature:` | `feat:` | Abbreviate to `feat:` |

| `feat:` | `feat:` | Already valid — but still rewrite the description |

| `fix:` | `fix:` | Already valid — but still rewrite the description |

| `docs:` | `docs:` | Already valid — but still rewrite the description |

| `chore:` | `chore:` | Already valid — but still rewrite the description |

**CRITICAL**: Never prepend a new prefix to a title that already has one. `feat: bug: MCP status bar...` is malformed. Instead:

1. Strip the issue title prefix entirely

2. Map to the correct conventional commit type using the table above

3. Rewrite the summary to describe what your **code change does**, not what the issue says

Remember: even if an issue title has a valid prefix, the PR title should describe your diff — not copy the issue title.

### Why This Matters

- `gh pr list`, `git log --oneline`, and release notes become readable at a glance

- Generic titles like "implement changes for issue #N" provide zero information

- Aligns with the existing commit style in this repository

- Enables automated changelog generation

### PR Title Checklist

Before creating a PR, verify your title:

- [ ] Starts with a conventional commit prefix (`fix:`, `feat:`, etc.)

- [ ] Describes what the PR **does**, not what issue it addresses

- [ ] Does NOT contain generic phrases like "implement changes for", "address issue", or "implement feature from"

- [ ] Does NOT reference an issue number in the title (issue references go in the PR body)

- [ ] Does NOT contain double prefixes (e.g., `feat: bug:`, `fix: feat:`) — strip any prefix from the issue title before composing your own

- [ ] Is under 70 characters

- [ ] A reader can understand the change from the title alone without looking at the diff

---

## Commit Messages: Same Rules as PR Titles

Commit messages follow the **exact same rules** as PR titles above. This repo merges with merge commits (#9105), so each commit message survives onto main verbatim — write each one as durable history.

### How to Write Commit Messages

```bash

# Step 1: Review your diff BEFORE writing the commit message

git diff --stat

git diff

# Step 2: Describe what the code change does

git commit -m "fix: validate PR title format in sweep phase validation"

# NOT: git commit -m "feat: implement changes for issue #2678"

# NOT: git commit -m "fix: address issue #2557"

```

### Commit Message Anti-Patterns

These patterns are **WRONG** — sweep phase validation will reject PRs with titles matching them:

| Anti-Pattern | Why It's Wrong |

|-------------|---------------|

| `feat: implement changes for issue #N` | Describes the issue, not the code change |

| `fix: address issue #N` | Says nothing about what was fixed |

| `feat: implement feature from issue #N` | Generic — could be any feature |

| `<copy of issue title>` | Issue titles describe problems; commits describe solutions |

### DCO sign-off

If `commit.signoff` is `true` in `.loom/config.json`, pass `--signoff` on **every**

commit you author (including `git commit --amend`) so each carries a

`Signed-off-by:` trailer — DCO-requiring repos gate PRs on it:

```bash

git commit --signoff -m "fix: validate PR title format in sweep phase validation"

```

If the knob is unset, do a one-time check before your first commit: if

`CONTRIBUTING.md`/a `DCO` file mentions `Signed-off-by`/DCO **or** a required status

check name matches `dco`/`sign-?off`, use `--signoff` too and note it in the PR

body. `--signoff` is harmless when not required. See

`defaults/docs/commit-signoff.md`.

---

## Creating Pull Requests: Label and Auto-Close Requirements

> **CRITICAL**: PRs MUST include an issue reference in the body — either a closing keyword

> (`Closes #N` / `Fixes #N` / `Resolves #N`) for a full implementation, or a non-closing

> reference (`Part of #N` / `Contributes to #N`) for a declared **partial increment** of a

> family/epic issue (see "Multi-PR landings" below).

> A closing keyword is required for:

> 1. GitHub to auto-close the issue when the PR merges

> 2. **Sweep orchestration to detect your PR during phase validation**

>

> A partial-increment PR intentionally omits the closing keyword so the family/epic issue

> stays open — it still references the issue with `Part of #N` so the PR is discoverable.

### PR Label Rules

**When creating a NEW PR:**

- Add `loom:review-requested` during creation, plus each priority label the issue

carries (`loom:operator-priority`, `loom:operator-high-priority`,

`loom:high-priority-inherited`; #9244/#10307: the one set a role copies, never invents;

level list: keep in sync with operator_levels.rs LEVELS until #10311)

- This is the ONLY time you add labels to a PR

**After PR creation:**

- NEVER remove `loom:review-requested` (Judge does this)

- NEVER remove `loom:pr` (Judge adds this, Champion uses it)

- NEVER add `loom:pr` yourself (only Judge can approve)

- NEVER modify any labels on PRs you didn't create

**Why?** PR labels are review-pipeline signals (`loom:review-requested` -> Judge

-> `loom:pr` -> Champion merges). Touching them breaks the pipeline.

### GitHub Auto-Close Requirements

**IMPORTANT**: When creating PRs, you MUST use GitHub's magic keywords to ensure issues auto-close when PRs merge.

### The Problem

If you write "Issue #123" or "Fixes issue #123", GitHub will NOT auto-close the issue. This leads to:

- Orphaned open issues that appear incomplete

- Manual cleanup work for maintainers

- Confusion about what's actually done

### The Solution: Use Magic Keywords

**ALWAYS use one of these exact formats in your PR description:**

```markdown

Closes #123

Fixes #123

Resolves #123

```

### Examples

**WRONG - Issue stays open after merge:**

```markdown

## Summary

This PR implements the feature requested in issue #123.

## Changes

- Added new functionality

- Updated tests

```

**CORRECT - Issue auto-closes on merge:**

```markdown

## Summary

Implement new feature to improve user experience.

## Changes

- Added new functionality

- Updated tests

Closes #123

```

### Why This Matters

GitHub's auto-close feature only works with specific keywords **immediately followed** by the issue reference:

- `Closes #X`

- `Fixes #X`

- `Resolves #X`

- `Closing #X`

- `Fixed #X`

- `Resolved #X`

**Any other phrasing will NOT trigger auto-close.**

> **The keyword does NOT have to start a line — GitHub scans the ENTIRE body.**

> `close`/`closes`/`closed`/`fix`/`fixes`/`fixed`/`resolve`/`resolves`/`resolved` creates a

> real closing reference **wherever** it appears — mid-sentence, inside a numbered list, in a

> "follow-up" checklist. What breaks the link is a word between the keyword and the reference

> (`close issue #X`, `Fixes issue #X`), not its position in the body. This cuts both ways: it

> is why `Fixes issue #123` silently fails to close, and why a stray `then close #123` buried

> in prose silently *does* close — see "Multi-PR landings" below (#4569).

### Multi-PR landings: every PR declares its issue (#9465)

The auto-close guidance above assumes one issue → one PR → close. **Two shapes break that, and

the reference requirement applies to both** — not only to declared families:

- a **family/epic** issue (`loom:epic` / `loom:epic-phase`, or a body scoping a family such as

"~346 conformance failures across N files") landed deliberately in slices;

- an **ordinary** issue that happens to land through more than one merged PR — Doctor rework that

opened a fresh PR, a Judge-rejected-then-refixed cycle, a first slice shipped on a reused

`feature/issue-N` branch. This is the majority case and had no documented requirement at all:

19 of 329 sweep-landing PRs (2026-08-15..09-29) were invisible to GitHub's issue↔PR link, and

one issue landed through 47 PRs.

For a PR that does NOT complete the issue, use a **non-closing** reference *and* the

machine-readable trailer:

```markdown

Part of #123

Loom-Issue: rjwalters/loom#123

```

- `Part of #N` / `Contributes to #N` keeps the PR discoverable without triggering auto-close, so

the issue survives the merge, and is the form merge-pr.sh parses to fire the `loom:building` →

`loom:issue` reset (#3667). **Do not substitute `Refs #N`** — nothing parses it, so the reset

silently no-ops.

- `Loom-Issue: owner/repo#N` is the machine-readable link (`loom-daemon merge-pr-refs

loom-issue-trailer-refs`), greppable without GitHub's keyword parser or prose matching. The

`owner/repo` slug is **required** (a bare `Loom-Issue: #123` does not parse). **Required on

every PR whose body does not close its issue**; optional on a `Closes #N` PR, whose closing

reference already carries the link. Only the **final** PR uses `Closes #N`.

**Write both as PLAIN TEXT — never in backticks (#8796).** A trailer inside an inline code span

(or a fenced block) is **silently ignored** — merge-pr.sh blanks code spans before matching,

deliberately (#5234), so the PR looks correct to a reviewer and to Judge while the #3667 reset

never fires and the issue is stranded at `loom:building` with nothing logged (the

rjwalters/kicad-tools PR #5686 / #5240 incident).

```markdown

Part of #123 <- declaration: parsed, the label reset fires on merge

`Part of #123` <- code span: NOT a declaration, reset silently skipped

```

**Both the PR body and the commit messages must carry the same reference.** GitHub harvests closing keywords from the PR body and the merge commit's subject (this repo merges with merge commits, #9105) — individual commit bodies are not parsed, but keep the reference consistent in them anyway. When in doubt, prefer `Part of #N`.

#### A stray closing keyword ANYWHERE in the body defeats `Part of #N` (#4569)

`Part of #N` / `Contributes to #N` is not a shield. GitHub does not weigh the two references

against each other — **one** closing keyword adjacent to `#N` anywhere in the body is enough

to close the issue on merge, no matter how explicitly the rest of the body says otherwise. This

is observed, not hypothetical (a `Contributes to #2` PR whose operator-handoff step read "then

close #2" closed #2 on merge, and it had to be reopened by hand).

**Rules for a non-closing PR body:**

- **Never** write a closing keyword immediately before the tracked issue's number. Write

`then close the issue`, or `then close issue #2` (the intervening word breaks the link).

- The tracked issue number should appear exactly once with a keyword: the `Part of #N` /

`Contributes to #N` reference. Scan the whole body — including checklists, test plans, and

operator-handoff sections — and every commit message on the branch (#4595), before pushing:

```bash

# Must print nothing for the tracked issue N:

grep -inE '\b(close[sd]?|fix(e[sd])?|resolve[sd]?)[[:space:]]+#N\b' <<<"$PR_BODY"

git log --format=%B origin/main..HEAD \

| grep -inE '\b(close[sd]?|fix(e[sd])?|resolve[sd]?)[[:space:]]+#N\b'

```

merge-pr.sh backstops some of this — it warns on a whole-line backticked `Part of` trailer,

warns naming which source carries a contradicting closing keyword, and reopens the issue if

GitHub closed it anyway — but every warning reaches only whoever runs the merge, a reopen leaves

a close/reopen flicker plus notification churn, and **nothing checks the `Loom-Issue:` trailer at

merge time**. Get it right in the body. Full rationale,

incidents, and the trailer's parser contract: `.loom/docs/issue-pr-linking.md`.

### PR Creation Checklist

When creating a PR, verify:

1. **PR title uses conventional commit format** - e.g., `fix: descriptive summary` (see "PR Titles" above)

2. **Acceptance criteria verified** - Each criterion from issue explicitly checked (see "Acceptance Criteria Verification" above)

3. PR description references the issue: `Closes #X` for a full implementation, or `Part of #X` for a declared partial increment (not "Issue #X" or "Addresses #X") — same reference in the commit message

4. Issue number is correct

5. PR has `loom:review-requested` label

6. All CI checks pass locally (the project's check command — `buildGate.command` in `.loom/config.json`, e.g. `pnpm check:ci`) — **including the formatter/linter on changed files** (see "Format and Lint Changed Files" above); a format-only failure is a guaranteed Judge rejection, not a warning

7. PR description includes verification table for each criterion

8. Tests added/updated as needed

9. Commits carry a `Signed-off-by:` trailer if required (`commit.signoff: true` in `.loom/config.json`, or a DCO/`sign-off` check — see "DCO sign-off")

10. `## Test Plan` includes a `TDD:` line for any diff touching executing code (see "Test-First Discipline" above) — omit only for docs/config/ADR-only changes

11. **Rebased onto latest `origin/main` immediately before push** — see "Pre-Push Rebase: Sync with `origin/main`" below

### Pre-Push Rebase: Sync with `origin/main` (#7668)

**Immediately before `git push` + opening the PR** — after the checklist above

passes and right before the Lease Fencing check below — sync your branch onto

the current tip of `main`:

```bash

git fetch origin main

git rebase origin/main

```

Do this **unconditionally**, not only when your diff happens to touch a file

you know is contested. A branch cut even a few hours ago against a busy repo

can be stale by the time you are ready to push, and the cost of skipping this

is not hypothetical: three consecutive PRs (example-org/tool-repo#164, #165,

#166) each landed `DIRTY` with CI never run, purely because every branch was

cut before the previous PR in the sequence merged and each touched the same

narrow, frequently-updated surface (a README status paragraph). Each one cost

a full Judge-rejection + Doctor-rebase round-trip that this step is meant to

absorb up front, before a reviewer ever sees the PR.

**If the rebase conflicts, resolve it now — this is the moment, not later

after a review round-trip:**

1. Resolve each conflicted file, `git add` it, `git rebase --continue`

(repeat until the rebase completes).

2. Re-run the project's check command (`buildGate.command`, e.g. `pnpm check:ci`)

— a rebase can silently change behavior even when it resolves cleanly.

3. **Version-bearing-file sync gate (#7168; moot after #7743)**: a clean rebase

lands `origin/main`'s exact version values, so it never fires unless your

branch carries its own version-bearing edit — revert it, never `version.sh bump`:

```bash

if [ -x ./.loom/scripts/version-check-gate.sh ] && ! ./.loom/scripts/version-check-gate.sh --fix-hint "before push."; then

echo "Aborting: version-bearing files out of sync after rebase (see BLOCKER: above) -- revert, never bump." >&2

exit 1

fi

```

4. If you cannot resolve the conflict confidently (e.g. it touches code you

don't understand well enough to merge correctly), stop and report rather

than force a resolution — this is the same bar as any other blocked-work

outcome in `builder.md`.

This is a pure timing/ordering change — it does not weaken any Judge check,

label invariant, or lifecycle gate; it only moves a rebase that would

otherwise happen reactively (via Doctor, after a `loom:changes-requested`

round-trip) to before the PR is ever opened.

### Lease Fencing: Confirm You Still Own the Claim (Epic #6165 Phase 3, #6309)

**Immediately before `git push` + opening the PR** — the one irreversible,

externally-visible action of this whole Builder run — run the sweep-side

fencing check:

```bash

./.loom/scripts/sweep-lease-fence.sh check "$N" --branch "$(git rev-parse --abbrev-ref HEAD)"

FENCE_RC=$?

if [[ "$FENCE_RC" -eq 3 ]]; then

echo "Lease fence: EXPIRED — MY OWN claim's lease record is stale on the forge's own clock (my renewal loop died). Aborting before push/PR-open; NOT pushing, NOT opening a PR." >&2

# Stop here for issue $N. Do not push, do not create a PR, do not touch

# the loom:building label or contest any peer's claim — report this issue

# as not-contributed-this-run. (#6783: an expired lease of a DIFFERENT,

# abandoned host is not an abort; it folds into FENCE_RC == 0 below.)

elif [[ "$FENCE_RC" -eq 4 ]]; then

echo "Lease fence: SUPERSEDED — a different host's lease is now the freshest for issue $N. Aborting before push/PR-open; NOT pushing, NOT opening a PR." >&2

# Same stop-here handling as the EXPIRED branch above.

elif [[ "$FENCE_RC" -ne 0 ]]; then

# 5: yours -> adopt via create-pr.sh, else stand down; NEVER a suffix

# branch. 6: loom-daemon too old/missing (#10027).

echo "Lease fence: exit $FENCE_RC (5 BRANCH_COLLISION / 6 BRANCH_PROBE_UNAVAILABLE) — NOT pushing." >&2

else

# FENCE_RC == 0 (fresh & own host, OR no lease evidence to fence against —

# fail-open, see the script's own header doc — OR an EXPIRED lease owned

# by a DIFFERENT, abandoned host, Issue #6783) -> proceed exactly as

# before.

git push -u origin "$(git rev-parse --abbrev-ref HEAD)"

# ... then open the PR (see "Creating the PR" below) ...

fi

```

This is **fencing, not a lock** — it bounds the cost of a race to one wasted

build, it does not prevent the race (Phase 1, `defaults/docs/lease-renewal.md`,

keeps the lease alive for the whole run; Phase 2,

`loom-daemon/src/claim_reconciliation.rs`, is the *daemon's* symmetric check

before reclaiming a peer's claim). Reads the freshest

`<!-- loom:lease host=… sweep=… -->` comment on issue `$N` and confirms BOTH:

the comment is still fresh (`now - updated_at <= LEASE_TTL_MINUTES`, default

15, override with `--ttl-minutes` or `LOOM_LEASE_TTL_MINUTES`) and its

`host=` still names **this** host (`--host`, defaulting to this host's own

identity — same `LOOM_HOST_ID` > `$HOSTNAME` > `hostname` precedence

`sweep_registry::host_identity()` uses). It aborts (exit `3` = expired-and-

own-host, `4` = superseded, `5` = branch collision)

**before doing anything externally-visible**: no push, no PR. It

never touches a peer's claim — the `loom:building` label is

left as-is. Since #6320 an in-session run

(manual `/loom:sweep`, GH Actions cron, `--no-daemon`) publishes its own

lease at pre-flight (`sweep-lease-publish.sh`, sweep.md Step 1b), so this

check is now meaningful on both dispatch paths rather than only the

daemon-dispatched one. When there genuinely is no lease comment — a run

predating those writers, or one whose lease write failed — the check fails

open (exit `0`), same as every other lease-record reader in this repo treats

"no lease" as "no evidence", never as "not fresh".

**Issue #6783: an EXPIRED lease owned by a DIFFERENT host now PASSes, not

aborts.** Exit `3` (EXPIRED) fires only when the freshest lease is stale AND

belongs to *this sweep's own* host — a sweep whose own renewal loop died

should not trust its claim. An expired lease belonging to a *different* host

is an abandoned record, not a live peer, and is now treated as no fencing

evidence (fail-open, exit `0`) rather than a permanent abort — a dead sweep's

never-renewed, never-yielded lease would otherwise fence out every successor

dispatch forever (observed on issue #6694 / PR #6773). Exit `4` (SUPERSEDED

— a *fresh* lease from a live peer) is unaffected: that case still means a

live peer genuinely holds the claim.

### Creating the PR

**Open the PR with `./.loom/scripts/create-pr.sh`, never a bare `gh pr create` (#6074).**

The flags are a subset of `gh pr create`'s, so the call below reads the same — but four

things a bare `gh pr create` cannot do are load-bearing here:

- **It adopts an already-open PR for your branch** (prints that PR's URL, exits 0, creates

nothing). So if a previous attempt on this issue already pushed and opened a PR, you

converge on it instead of failing or duplicating.

- **It re-verifies the target issue's freshness immediately before opening the PR

(#6277).** When the body carries a closing keyword (`Closes`/`Fixes`/`Resolves #N`), the

script re-checks whether `#N` was already closed by a *different*, already-merged PR — the

two-workers-race scenario that otherwise isn't caught until Judge review. On a detected

supersede it refuses to open a duplicate PR (names the superseding PR, exits non-zero,

does not push further, does not delete the branch). `Part of #N` / `Contributes to #N`

partial-increment references are exempt by construction — see "Multi-PR landings" below.

- **It enforces a 1:1 issue-to-PR review-gate (#9453 phase 5)** — the race adopt-first

above can't catch: two builders on *different* branches for the *same* issue. Re-runs

`forge check-open-pr` on your body's referenced issue (closing OR `Part of`); an open PR

on a different head branch refuses (exit `6`, names it). **If refused: stand down** —

close your branch, or escalate to Judge/a human if yours genuinely supersedes it.

- **It survives the GitHub App permission window.** A cached App installation token can

hold `Contents:write` while `Pull-requests:write` has not propagated into it yet, so

your `git push` succeeds and the very next `gh pr create` returns `403 Resource not

accessible by integration`. The script force-mints a fresh installation token (bypassing

the ~1h cache) and then falls back to a personal token, instead of dying. Before this,

that 403 killed the sweep with no PR and the next dispatch **rebuilt the identical

work**, orphaning a `feature/issue-N` branch each pass.

**If it still fails, do NOT rebuild and do NOT delete the branch** — your commits are

pushed. Re-run the script (idempotent), or report the branch name for a hand-opened PR.

On an install predating this script (no `.loom/scripts/create-pr.sh` — use plain

`gh pr create`), a `403 … not accessible by integration` after a successful push is

still a transient credential window, never a signal to redo the work.

```bash

# CORRECT way to create PR

# Title MUST use conventional commit format: "fix:", "feat:", "refactor:", etc.

./.loom/scripts/create-pr.sh --title "fix: descriptive summary of the change" --label "loom:review-requested" --body "$(cat <<'EOF'

## Summary

Brief description of what this PR does and why.

## Changes

- Change 1

- Change 2

- Change 3

## Acceptance Criteria Verification

| Criterion | Status | Verification |

|-----------|--------|--------------|

| Criterion 1 from issue | ✅ | How you verified it |

| Criterion 2 from issue | ✅ | How you verified it |

## Test Plan

How you verified the changes work.

TDD: yes — tests/foo_test.rs written first, failed for the right reason, now passes

(or: TDD: no — <reason>, e.g. docs-only / config-only / refactor with existing coverage)

Closes #123

EOF

)"

```

**Remember**:

- Put "Closes #123" (or "Part of #123" for a partial increment) on its own line in the PR description, and use the same reference in the commit message

- Include the acceptance criteria verification table showing each criterion was checked

## Handling Pre-existing Lint/Build Failures

**IMPORTANT**: When the target codebase has pre-existing issues, don't let them block your focused work.

### The Problem

Target codebases may have pre-existing failures that are unrelated to your issue:

- Deprecated linter configurations

- CSS classes not defined in newer framework versions

- A11y warnings in unrelated files

- Type errors in untouched code

**These are NOT your responsibility to fix when implementing a specific feature.**

### Strategy: Focus on Your Changes

**Step 1: Identify what you changed**

```bash

# Get list of files you modified

git diff --name-only origin/main

```

**Step 2: Run scoped checks on your changes only**

```bash

# Lint only changed files (Biome)

git diff --name-only origin/main -- '*.ts' '*.tsx' '*.js' '*.jsx' | xargs npx biome check

# Lint only changed files (ESLint)

git diff --name-only origin/main -- '*.ts' '*.tsx' '*.js' '*.jsx' | xargs npx eslint

# Type-check affected files (TypeScript will check dependencies automatically)

npx tsc --noEmit

```

**Step 3: If full checks fail on pre-existing issues**

1. **Document in PR description** what pre-existing issues exist

2. **Don't fix unrelated issues** - this expands scope

3. **Optionally create a follow-up issue** for the tech debt

### PR Documentation Template

When pre-existing issues exist, add this to your PR:

```markdown

## Pre-existing Issues (Not Addressed)

The following issues exist in the codebase but are outside the scope of this PR:

- [ ] `biome.json` uses deprecated v1 schema (needs migration to v2)

- [ ] `DashboardPage.tsx` has a11y warnings (unrelated to this feature)

- [ ] Tailwind 4 CSS class `border-border` not defined

These should be addressed in separate PRs to maintain focused scope.

```

### Decision Tree

```

Lint/Build fails

|

Is the failure in YOUR changed files?

|-- YES -> Fix it (your responsibility)

+-- NO -> Pre-existing issue

|-- Document in PR description

|-- Continue with your implementation

+-- Optionally create follow-up issue

```

### Creating Follow-up Issues (Optional)

If you want to track pre-existing issues for future cleanup:

> **File issues with `./.loom/scripts/create-issue.sh`, never a bare `gh issue create` (#5047).**

> `gh issue create` fails outright when GraphQL quota is exhausted, while the independent REST

> pool sits ~99% unused. The script takes the same flags (`--title`, `--body`/`--body-file`,

> repeatable `--label`, `--repo`) and prints the same issue URL, but falls back to a single REST

> POST that applies labels **atomically with creation**. Recipe and rationale:

> `.loom/docs/gh-issue-create-rest-fallback.md` (or `forge_gh_create_issue_rl_safe` in

> `lib/forge-helpers.sh` if scripting). `loom-daemon forge issue create` is a byte-identical `gh`

> passthrough — NOT a fallback.

```bash

./.loom/scripts/create-issue.sh --title "Tech debt: Migrate biome.json to v2 schema" --body "$(cat <<'EOF'

## Problem

`biome.json` uses deprecated v1 schema which causes warnings on every lint run.

## Discovery

Found while working on #969. Not fixed there to maintain focused scope.

## Solution

Run `npx @biomejs/biome migrate` to update configuration.

## Impact

- Removes deprecation warnings

- Enables new linter rules

- Estimated: 30 minutes

EOF

)"

```

### What NOT to Do

**Don't block your PR on unrelated failures**

```bash

# WRONG: Spending hours fixing biome config for an unrelated feature

```

**Don't include unrelated fixes in your PR**

```bash

# WRONG: PR titled "Add login button" that also migrates linter config

```

**Don't ignore failures in YOUR code**

```bash

# WRONG: Introducing new lint errors in the code you wrote

```

### Why This Matters

**Scope creep kills productivity:**

- Issue #921 spent 2+ hours on biome migration (unrelated to feature)

- Issue #922 fixed a11y warnings in files not touched by the feature

- Each detour adds risk and delays the actual goal

**Focused PRs are better:**

- Easier to review (one concern per PR)

- Faster to merge (no surprises)

- Clearer git history (each commit has one purpose)

- Lower risk (smaller blast radius)

## Raising Concerns

After completing your assigned work, you can suggest improvements by creating unlabeled issues. The Architect will triage them and the user decides priority.

**Example of post-work suggestion:**

```bash

./.loom/scripts/create-issue.sh --title "Refactor terminal state management to use reducer pattern" --body "$(cat <<'EOF'

## Problem

While implementing #42, I noticed that terminal state updates are scattered across multiple files with inconsistent patterns.

## Current Code

- State mutations in: `src/lib/state.ts`, `src/main.ts`, `src/lib/terminal-manager.ts`

- No single source of truth for state transitions

- Hard to debug state-related issues

## Proposed Refactor

- Single `terminalReducer` function handling all state transitions

- Action types for each state change

- Easier to test and debug

## Impact

- **Files**: ~5 files affected

- **Complexity**: Medium (2-3 days)

- **Risk**: Low if we add comprehensive tests first

Discovered while working on #42

EOF

)"

```

**Note:** For out-of-scope work discovered during implementation, use the **Scope Management** section in `builder-complexity.md` - pause immediately and create an issue before continuing.