diff --git a/AGENTS.md b/AGENTS.md index 1b14b4c..1149ddd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,7 +12,7 @@ You are an AI agent (Claude Code, Cursor, Continue, or other) operating against - `operating-model/` — operator/agent contract, gates, escalation, model and tool selection, MCP and `llms.txt`, memory conventions. Load relevant pages before any non-trivial work. - `invariants/` — the non-negotiables. Engineering invariants, deep links into Martin Maurer's `.invariants` framework, agent-era invariants (no silent edits, no fabricated APIs, no committed secrets). -- `workflows/` — PR authoring (delegates to the `chainsafe-research-plan-implement` skill), code review (operator-reviewing-agent and agent-reviewing-PR modes), repo & CI setup, testing & QA, infrastructure & DevOps (deep links into `ChainSafe/infrastructure-general`), incident response, release & deploy. +- `workflows/` — work decomposition (epic → milestones → bite-sized issues), PR authoring (delegates to the `chainsafe-research-plan-implement` skill), code review (operator-reviewing-agent and agent-reviewing-PR modes), repo & CI setup, testing & QA, infrastructure & DevOps (deep links into `ChainSafe/infrastructure-general`), incident response, release & deploy. - `languages//` — for each of Go, Rust, TypeScript, Solidity, Daml, Python, Zig: `architect.md`, `developer.md`, `reviewer.md`, plus shared `idioms.md` and `gotchas.md`. Architect pages deep-link into `.invariants`. - `references/` — attribution, source pointers, contributors. - `skills/` — packaged Anthropic Skills authored via `skill-creator`. Discoverable via `handbook.chainsafe.io/llms.txt` and via direct paths in this repo. @@ -37,6 +37,7 @@ The full policy lives at `operating-model/gates-and-escalation.md`. In summary: - Stop before any action touching production, secrets, irreversible writes, public communication, or `git push` / merge. - Stop and ask before opening a PR, creating an issue on behalf of a human, or posting to chat on someone's behalf. +- Stop before opening a PR that cannot be reviewed in one pass, or that closes more than one issue. Propose a split; an oversized PR ships only with explicit operator approval recorded in its description. - If you encounter a section this repo claims to have but doesn't, escalate. Do not fabricate. - If you are asked to bypass a HARD FAIL from a language reviewer skill (Solidity reentrancy, Daml ledger invariants, etc.), refuse and escalate. The operator can override; you cannot. diff --git a/README.md b/README.md index 696a6eb..46f2e41 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ The structure is: - **`operating-model/`** — the operator/agent contract. How a human and an AI collaborator share responsibility for an output. Read this first. - **`invariants/`** — the non-negotiables. Engineering invariants, the `.invariants` framework (deep-linked, not duplicated), and agent-era invariants. -- **`workflows/`** — PR authoring, code review, repo & CI setup, testing & QA, infrastructure & DevOps (deep-linked into `ChainSafe/infrastructure-general`), incident response, release & deploy. +- **`workflows/`** — work decomposition (epic → milestones → bite-sized issues), PR authoring, code review, repo & CI setup, testing & QA, infrastructure & DevOps (deep-linked into `ChainSafe/infrastructure-general`), incident response, release & deploy. - **`languages/`** — opinionated guidance per language ecosystem (Go, Rust, TypeScript, Solidity, Daml, Python, Zig) split into three roles: architect, developer, reviewer. - **`references/`** — attribution, source pointers, contributors. - **`skills/`** — packaged Anthropic Skills authored via `skill-creator`, distributable to any agent runtime that supports them. diff --git a/invariants/agent-era-invariants.md b/invariants/agent-era-invariants.md index 54c6b86..03e2e40 100644 --- a/invariants/agent-era-invariants.md +++ b/invariants/agent-era-invariants.md @@ -14,6 +14,8 @@ These extend the [Engineering Invariants](./engineering-invariants.md) — they **How it's checked.** Agent surfaces all file paths in the planned diff before acting (the `chainsafe-research-plan-implement` skill makes this part of the plan). Review-time: PR descriptions name what was in scope; diffs that touch out-of-scope files without an explicit "added scope because Y" line in the PR description are a SOFT WARNING from the reviewer skill. +**Scope is necessary but not sufficient.** A diff can stay entirely inside the named scope and still be unreviewable, by collapsing several planned issues into one PR. Review tractability therefore also depends on decomposition ([`../workflows/work-decomposition.md`](../workflows/work-decomposition.md)) and on the size gate ([gate §10](../operating-model/gates-and-escalation.md#10-oversized-or-multi-concern-changes)), which requires explicit operator approval for a change that cannot be reviewed in one pass. + **Override.** Operator may extend scope mid-task by saying so. The extension is recorded in the PR description. ## 2. No fabricated APIs, functions, files, or references diff --git a/llms.txt b/llms.txt index 71fe7a8..6e45d3d 100644 --- a/llms.txt +++ b/llms.txt @@ -17,15 +17,15 @@ URLs below point at the `main` branch and resolve to raw markdown. ## Invariants - [Engineering Invariants](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/invariants/engineering-invariants.md): The seven non-negotiables (open-source by default, standards enforced, quality defined before build, composable by design, decisions in the repo, ship value not effort, auditable context). -- [Agent-Era Invariants](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/invariants/agent-era-invariants.md): Eight invariants specific to agent-assisted work (no silent edits, no fabrication, no secret commits, no protected-branch push, scope boundary, no external comms without per-message auth, no HARD FAIL bypass, audit trail). +- [Agent-Era Invariants](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/invariants/agent-era-invariants.md): Nine invariants specific to agent-assisted work (no silent edits, no fabrication, no secret commits, no protected-branch push, scope boundary, no external comms without per-message auth, no HARD FAIL bypass, audit trail, operational-contract surfacing). - [.invariants Framework (pointer)](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/invariants/invariants-framework.md): Deep-link map into [@boorich/.invariants-starter-kit](https://github.com/boorich/.invariants-starter-kit) and [landing page](https://boorich.github.io/.invariants-starter-kit/). Maintained by [@boorich](https://github.com/boorich). ## Workflows - [OneFlow](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/workflows/oneflow.md): ChainSafe's branching and release model. Trunk-based, single `main`, tag-driven deployments (`v.*.*` for production, `stage-*` for staging). Hot fixes via `hotfix/*` branches off broken production tags. Adapted from Adam Ruka's OneFlow concept. - [Work Decomposition](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/workflows/work-decomposition.md): Epic → milestones → bite-sized issues. What makes an issue bite-sized (one PR, one reviewable pass, written acceptance criteria, no open design decision — no line-count threshold). How `plan.md` phases and tasks map onto the tracker. Tracker-agnostic; GitHub conventions given. -- [PR Authoring](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/workflows/pr-authoring.md): Agent-era author guide. Small/focused/self-contained PRs, AI-generated declaration, scope-drift flags. Delegates non-trivial workflow to the `chainsafe-research-plan-implement` skill. -- [Code Review](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/workflows/code-review.md): Two modes (operator-reviewing-agent, agent-reviewing-PR). Universal principles, what-to-look-for, speed of review, when-reviews-go-wrong. +- [PR Authoring](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/workflows/pr-authoring.md): Agent-era author guide. One issue / one PR, small-focused-self-contained, oversized PRs require explicit recorded operator approval (gate §10), AI-generated declaration, scope-drift flags. Delegates non-trivial workflow to the `chainsafe-research-plan-implement` skill. +- [Code Review](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/workflows/code-review.md): Two modes (operator-reviewing-agent, agent-reviewing-PR). Universal principles, what-to-look-for, size check and how to handle an oversized PR, speed of review, when-reviews-go-wrong. - [Repo and CI Setup](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/workflows/repo-and-ci-setup.md): Ten-step per-repo hygiene checklist as an agent-runnable runbook. Branch protection, CODEOWNERS, CI baseline, security baseline. - [Testing and QA](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/workflows/testing-and-qa.md): What agents generate vs what operators own. Carries the Forest / Ghostty AI-policy pattern. Test type guidance. - [Infrastructure and DevOps (pointer)](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/workflows/infrastructure-and-devops.md): Deep-link map into `ChainSafe/infrastructure-general`. Maintained by [@joshdougall](https://github.com/joshdougall). @@ -92,7 +92,7 @@ Packaged Anthropic Skills authored via `skill-creator`. Each skill is self-conta ### Workflow - [chainsafe-research-plan-implement](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/skills/chainsafe-research-plan-implement/SKILL.md): Research-first coding workflow: research → plan → annotate → decompose → implement. Gates ALL non-trivial code changes behind a human-approved plan, and decomposes that plan into epic / milestones / bite-sized issues before any code is written. Trigger: "implement", "refactor", "fix this bug", "add a feature", "migrate from X to Y", "rewrite", "wire up", "extend", "break this into issues". Adapted from Boris Tane's "How I Use Claude Code." -- [chainsafe-pr-author](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/skills/chainsafe-pr-author/SKILL.md): Language-agnostic PR authoring. Trigger: "open a pull request", "draft a PR description", "split this PR", "respond to reviewer", "AI declaration", "scope drift". +- [chainsafe-pr-author](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/skills/chainsafe-pr-author/SKILL.md): Language-agnostic PR authoring. One issue / one PR; oversized PRs need explicit recorded operator approval. Trigger: "open a pull request", "draft a PR description", "split this PR", "PR is too big", "respond to reviewer", "AI declaration", "scope drift". - [chainsafe-code-review](https://raw.githubusercontent.com/ChainSafe/engineering-handbook/main/skills/chainsafe-code-review/SKILL.md): Universal code-review framework. Trigger: "review this PR", "check this diff", "what should I look for", "review disagreement". Pairs with the language-specific reviewer skills. ### Language × role (5 languages × 3 roles) diff --git a/skills/chainsafe-code-review/SKILL.md b/skills/chainsafe-code-review/SKILL.md index 04aa9aa..655b21b 100644 --- a/skills/chainsafe-code-review/SKILL.md +++ b/skills/chainsafe-code-review/SKILL.md @@ -28,6 +28,18 @@ Universal review framework at ChainSafe. Two reviewer modes; language-specific s - **Comments.** Explain *why*, not *what*. - **Every line.** Look at every assigned line. - **Context.** Sometimes pull the branch. +- **Size.** Can you review this in one focused pass? A PR should close one bite-sized issue. + +## Reviewing an oversized PR + +Check size first — it determines whether the rest of the review is real. + +- **Send it back before reviewing it.** "This closes three issues; please split" is a complete first review. +- **Unless it carries a recorded approval** (`Oversized PR approved by @operator: `). Then review on the merits — and say so if the reason doesn't hold up. +- **Check the exemptions are clean.** Renames, deletions, generated code, lockfile bumps may be large only if that's *all* they are. A logic change hiding in a 4,000-line regeneration is what this catches. +- **Never approve a large PR you skimmed.** Say you don't have time instead. Blind approval is worse than a slow review. + +[Gate §10](../../operating-model/gates-and-escalation.md#10-oversized-or-multi-concern-changes); author's side in [`workflows/pr-authoring.md`](../../workflows/pr-authoring.md#when-a-pr-has-to-be-bigger). ## Speed of review @@ -42,6 +54,7 @@ Most ChainSafe PRs in the v2 era have an agent in the author chair. What to veri - **The plan matches the diff.** Drift between linked `plan.md` and the diff is the most common quiet failure. - **Fabrication.** Imports that don't exist, wrong-signature function calls, broken handbook page references. Verify. - **Silent scope creep.** Files in the diff not in the original plan — PR description should name each with a reason. +- **Collapsed decomposition.** The plan named several issues; the agent shipped one PR. Every file authorized, but the slicing wasn't — the most common way an agent produces an unreviewable diff while staying in scope. Send it back to the plan. - **Over-eager refactor.** Each unrelated improvement is its own PR. - **Generic comments.** Cut comments that restate what the code does. - **Test theatre.** Tests asserting "function was called" rather than its effect. Read test bodies. @@ -50,7 +63,7 @@ Most ChainSafe PRs in the v2 era have an agent in the author chair. What to veri ### When to demand re-plan vs accept vs reject -- **Re-plan:** diff drifted from plan; agent made unapproved design choices; unrelated changes included; fabrication present. Go back to the plan; don't patch. +- **Re-plan:** diff drifted from plan; agent made unapproved design choices; unrelated changes included; fabrication present; several planned issues collapsed into one PR. Go back to the plan; don't patch. - **Accept with notes:** minor issues that don't change the shape. - **Reject entirely:** wrong direction. Close PR; reopen with fresh plan. @@ -64,6 +77,8 @@ Most ChainSafe PRs in the v2 era have an agent in the author chair. What to veri ### Checklist +- Check size and scope first: one issue, one reviewable pass? Flag before spending effort on the contents. +- Confirm an oversized PR carries a recorded operator approval; flag its absence as a gate §10 finding. - Run lint/type-check/test against the branch; report failures. - Diff against the linked plan or spec; flag deviations. - Apply the relevant language reviewer skill (chainsafe--reviewer). @@ -102,6 +117,7 @@ Escalate rather than reviewing if: ## Related - Full reference: [`workflows/code-review.md`](../../workflows/code-review.md) +- Decomposition: [`workflows/work-decomposition.md`](../../workflows/work-decomposition.md) — what a reviewable PR is cut from - Language reviewer skills: `chainsafe-go-reviewer`, `chainsafe-rust-reviewer`, `chainsafe-typescript-reviewer`, `chainsafe-solidity-reviewer` - Counterpart skill: `chainsafe-pr-author` - Invariants: [`invariants/agent-era-invariants.md`](../../invariants/agent-era-invariants.md) diff --git a/skills/chainsafe-pr-author/SKILL.md b/skills/chainsafe-pr-author/SKILL.md index 64ee340..52d239f 100644 --- a/skills/chainsafe-pr-author/SKILL.md +++ b/skills/chainsafe-pr-author/SKILL.md @@ -21,15 +21,27 @@ For any substantive code change, the canonical workflow is the `chainsafe-resear When the PR author is an agent, the skill is loaded and followed. When human, the same shape applies — research artifact when context is non-obvious, plan when work touches more than one file, accept annotation, then commit code. +Work large enough to need a plan is decomposed into epic → milestones → bite-sized issues before implementation starts (see [`workflows/work-decomposition.md`](../../workflows/work-decomposition.md)). A PR can only be as small as the issue behind it. + ## Small, focused, self-contained +- **One issue, one PR.** The PR closes exactly one bite-sized issue and links it. Closing two means it should have been two PRs. - **One PR, one self-contained change.** Reviewable in ~10 minutes. - **Separate refactors from features/fixes.** A refactor is its own PR (exception: tiny refactor genuinely entangled with the feature, <~50 lines). -- **Renames, deletions, generated-code PRs can be large** — they trade scope-width for shallow review depth. +- **Renames, deletions, generated-code PRs can be large** — they trade scope-width for shallow review depth. Scope-width only: a large rename is fine, a rename *plus* a logic change is two PRs. - **Stack PRs** for sequential work rather than one big PR. The right question: *is this change related to the PR's stated goal, or can it live on its own?* +The standard is reviewability, not a line count — can one reviewer hold the whole change in their head in one sitting? + +### When a PR has to be bigger + +- **Stop before opening it.** An oversized PR that is already open has already spent the reviewer's attention. +- **Propose the split first.** Name the issue-sized PRs the work could become. +- **If it truly cannot split, get explicit operator approval and record it** in the PR description: `Oversized PR approved by @operator: .` Verbal approval still gets written into the PR. +- **Agents never self-approve.** This is [gate §10](../../operating-model/gates-and-escalation.md#10-oversized-or-multi-concern-changes). A diff that outgrows its approved plan slice stops and asks — even when every file touched was in scope. + ## PR description required fields - **What changed** — 2-3 sentences, plain language. @@ -67,6 +79,8 @@ The right question: *is this change related to the PR's stated goal, or can it l ## Anti-patterns - The mega-PR. "It's all related" — usually it isn't. +- The unapproved mega-PR. Big *and* nobody agreed it had to be. +- The retroactive issue. Filing issues once the branch is already thousands of lines deep. - The drive-by refactor. - The silent re-scope. - The agent ghost-author (no AI declaration). @@ -75,6 +89,7 @@ The right question: *is this change related to the PR's stated goal, or can it l ## Related - Full reference: [`workflows/pr-authoring.md`](../../workflows/pr-authoring.md) +- Decomposition: [`workflows/work-decomposition.md`](../../workflows/work-decomposition.md) — epic / milestone / bite-sized issue breakdown - The workflow itself: `chainsafe-research-plan-implement` - Counterpart skill: `chainsafe-code-review` - Invariants: [`invariants/agent-era-invariants.md`](../../invariants/agent-era-invariants.md) (especially §1, §8) diff --git a/workflows/code-review.md b/workflows/code-review.md index 6a03032..065ab9b 100644 --- a/workflows/code-review.md +++ b/workflows/code-review.md @@ -23,6 +23,18 @@ What to look for, on either mode: - **Comments.** Explain *why*, not *what*. Exceptions: regex, hard-to-understand algorithms — *what* is desirable there. - **Every line.** Look at every human-written line you've been assigned. Skim is not review. - **Context.** Sometimes pulling the branch and reading it in place beats scrolling the GitHub diff. +- **Size.** Can you review this in one focused pass? A PR should close one bite-sized issue ([`work-decomposition.md`](./work-decomposition.md)). If it doesn't, that is a finding — see below. + +### Reviewing an oversized PR + +Size is the first thing to check, because it determines whether the rest of the review is real. A PR you cannot hold in your head produces either a rubber stamp or a defensive block, and both are failures. + +- **Send it back before reviewing it.** "This closes three issues; please split into three PRs" is a complete and legitimate first review. It costs the author less than a review you couldn't do properly. +- **Unless it carries a recorded approval.** An `Oversized PR approved by @operator: ` line in the description means the call was already made. Review it on the merits — and if the reason doesn't hold up, say so. +- **Check the exemptions are clean.** Renames, deletions, generated code, and lockfile bumps may be large, but only if that's *all* they are. A logic change hiding inside a 4,000-line regeneration is the thing this rule exists to catch. +- **Never approve a large PR you skimmed.** If you don't have the time, say you don't have the time. Blind approval is worse than a slow review. + +This is [gate §10](../operating-model/gates-and-escalation.md#10-oversized-or-multi-concern-changes); the author's side of it is in [`pr-authoring.md`](./pr-authoring.md#when-a-pr-has-to-be-bigger). ## Speed of review @@ -45,6 +57,7 @@ Most ChainSafe PRs in the v2 era have an agent in the author chair. The operator - **The plan matches the diff.** If the PR links a `plan.md`, the diff should be that plan and nothing more. Drift between plan and code is the most common quiet failure. - **Fabrication.** Imports that don't exist, functions called with wrong signatures, references to handbook pages that aren't there. Agents fabricate plausibly; verify the references. - **Silent scope creep.** Files in the diff that weren't in the original plan. The PR description should name each one with a reason. Missing reason → ask. +- **Collapsed decomposition.** The plan named several issues; the agent shipped them as one PR. This is the most common way an agent produces an unreviewable diff while staying technically in scope — every file was authorized, but the slicing wasn't. Send it back to the plan. - **Over-eager refactor.** Agents like to "improve while passing through." Each unrelated improvement is its own PR. - **Generic comments.** Comments that restate what the code does instead of why. Agents add these by default; cut them. - **Test theatre.** Tests that exercise the code without actually checking behavior — e.g., asserting that a function was called rather than asserting on its effect. Read the test bodies, not just the test names. @@ -53,7 +66,7 @@ Most ChainSafe PRs in the v2 era have an agent in the author chair. The operator ### When to demand a re-plan vs. accept -- **Re-plan.** If the diff drifted materially from the linked plan; if the agent made design choices the operator hadn't approved; if the diff includes unrelated changes; if fabrication is present. Don't patch — go back to the plan. +- **Re-plan.** If the diff drifted materially from the linked plan; if the agent made design choices the operator hadn't approved; if the diff includes unrelated changes; if fabrication is present; if the diff collapsed several planned issues into one. Don't patch — go back to the plan. - **Accept with notes.** Minor issues that don't change the shape of the change. The author updates and you approve. - **Reject entirely.** The change is in the wrong direction. Close the PR; reopen with a fresh plan once the direction is right. @@ -69,6 +82,8 @@ Agents can be effective code reviewers when configured well. The shape: ### Checklist +- Check size and scope first: does the PR close exactly one issue, and can it be reviewed in one pass? Flag before spending review effort on the contents. +- Confirm an oversized PR carries a recorded operator approval; flag its absence as a [gate §10](../operating-model/gates-and-escalation.md#10-oversized-or-multi-concern-changes) finding. - Run lint/type-check/test against the branch; report failures. - Diff against the linked plan or spec; flag deviations. - Apply the relevant language reviewer skill (when one exists in [`../languages//reviewer.md`](../languages/)). @@ -109,6 +124,7 @@ In each case the agent surfaces the reason and asks the operator to either expan ## Related - [`pr-authoring.md`](./pr-authoring.md) — the author's counterpart to this page. +- [`work-decomposition.md`](./work-decomposition.md) — epic / milestone / bite-sized issue breakdown; what a reviewable PR is cut from. - [`../invariants/agent-era-invariants.md`](../invariants/agent-era-invariants.md) — §1 (scope), §2 (no fabrication), §7 (HARD FAIL), §8 (audit trail) all show up in review. - [`../operating-model/gates-and-escalation.md`](../operating-model/gates-and-escalation.md#8-reviewer-skill-hard-fail) — HARD FAIL handling. - [`../languages/`](../languages/) — language-specific reviewer skills extend this with language-aware checks. diff --git a/workflows/pr-authoring.md b/workflows/pr-authoring.md index b030589..0b3dd0c 100644 --- a/workflows/pr-authoring.md +++ b/workflows/pr-authoring.md @@ -14,17 +14,33 @@ For any non-trivial code change, the canonical workflow is the [`chainsafe-resea This page does not re-derive the workflow. When the PR author is an agent, the skill is loaded and followed. When the PR author is a human, the same shape applies — write a research artifact when context is non-obvious, write a plan when the work touches more than one file, accept annotation as you go, and only then commit code. +The plan's phased todo list is also where PR size gets decided. Work large enough to need a plan is decomposed into epic → milestones → bite-sized issues *before* implementation starts; see [`work-decomposition.md`](./work-decomposition.md). A PR can only be as small as the issue behind it. + ## Small, focused, self-contained These practices draw on Google's engineering review best practices and ChainSafe's established conventions: +- **One issue, one PR.** The PR closes exactly one bite-sized issue and links it. If it closes two, it should have been two PRs — or the decomposition was wrong and belongs back in `plan.md`. - **One PR, one self-contained change.** The PR addresses one thing plus its tests. It can be reviewed in a single ~10-minute pass. - **Separate refactors from features and fixes.** A refactor is its own PR. The exception is a small refactor (<~50 lines) genuinely entangled with the feature. -- **Renames, deletions, and generated-code PRs may be large** because they trade scope-width for shallow review depth. +- **Renames, deletions, and generated-code PRs may be large** because they trade scope-width for shallow review depth. That exemption covers scope-width only — a large rename is fine; a rename *plus* a logic change is two PRs. - **Stack PRs when work is sequential.** When a feature naturally splits into ordered pieces, ship them as a stack of small PRs rather than one big PR. The right question: *is this change related to the PR's stated goal, or can it live on its own?* If the latter, separate PR. +The standard is deliberately reviewability, not a line count. Line thresholds are easy to satisfy and easy to game; the honest test is whether one reviewer can hold the whole change in their head in one sitting. If you'd have to ask the reviewer to block out time for it, it is too big. + +### When a PR has to be bigger + +Sometimes a change genuinely will not split. That is allowed — but it is the operator's call, not the author's, and never the agent's. + +- **Stop before opening the PR.** An oversized PR that is already open has already spent the reviewer's attention. +- **Propose the split first.** Name the issue-sized PRs the work could become. Most "unsplittable" changes are splittable once the seams are written down. +- **If it truly cannot split, get explicit approval and record it.** A line in the PR description: `Oversized PR approved by @operator: .` Approval given verbally in chat still gets written into the PR — the [decision lives in the repo](../invariants/engineering-invariants.md#5-decisions-live-in-the-repo-not-in-chat), not in Slack. +- **Agents do not self-approve.** This is [gate §10](../operating-model/gates-and-escalation.md#10-oversized-or-multi-concern-changes). An agent whose diff outgrows the approved plan slice stops, surfaces it, and waits — even when every file it touched was in scope. + +Reviewers hold this line too; an oversized PR without a recorded approval is a valid reason to send it back unreviewed (see [`code-review.md`](./code-review.md)). + ## What goes in the PR description Every non-trivial PR opens with a description that lets a reviewer decide whether to engage in 30 seconds and finish the review in 10 minutes. @@ -76,6 +92,8 @@ The threshold: if a reviewer's first comment is going to be "can we hop on a cal ## Anti-patterns - **The mega-PR.** "It's all related" — usually it isn't. +- **The unapproved mega-PR.** Worse: big *and* nobody agreed it had to be. Gate §10 exists so this is a conversation before the PR, not an apology after it. +- **The retroactive issue.** Filing the issues once the branch is already thousands of lines deep. Decomposition is a planning artifact, not paperwork. - **The drive-by refactor.** Renaming variables across the codebase in the same PR as a bugfix. Split it. - **The silent re-scope.** Adding files to the diff without naming them in the PR description. - **The agent ghost-author.** A PR drafted by an agent without an AI-generated declaration. The convention exists to make review better, not as a confession. @@ -84,6 +102,7 @@ The threshold: if a reviewer's first comment is going to be "can we hop on a cal ## Related - [`code-review.md`](./code-review.md) — the reviewer's counterpart to this page. +- [`work-decomposition.md`](./work-decomposition.md) — epic / milestone / bite-sized issue breakdown; where a small PR's size is actually decided. - [`oneflow.md`](./oneflow.md) — ChainSafe's branching and release model; defines branch naming, the trunk-based flow, and the tag patterns that drive deployments. - [`../skills/chainsafe-research-plan-implement/SKILL.md`](../skills/chainsafe-research-plan-implement/SKILL.md) — the workflow this page assumes for non-trivial PRs. - [`../invariants/agent-era-invariants.md`](../invariants/agent-era-invariants.md) — scope discipline (§1), no-fabrication (§2), audit-trail (§8). diff --git a/workflows/repo-and-ci-setup.md b/workflows/repo-and-ci-setup.md index d2af735..d21eb55 100644 --- a/workflows/repo-and-ci-setup.md +++ b/workflows/repo-and-ci-setup.md @@ -102,10 +102,21 @@ A consistent label taxonomy makes triage and reporting tractable. ChainSafe base - **Status:** `triage`, `accepted`, `in-progress`, `blocked`, `wontfix`. - **Type:** `bug`, `feature`, `chore`, `docs`, `security`, `refactor`. - **Priority:** `p0` (critical), `p1` (high), `p2` (medium), `p3` (low). +- **Structure:** `epic` — marks the issue that holds an outcome and its milestones (see [`work-decomposition.md`](./work-decomposition.md)). - **Special:** `good-first-issue`, `help-wanted`, `breaking-change`. Apply via the `labels` API or a labels-config tool (e.g., `github-label-sync`) so the set is reproducible across repos. +### Work tracking + +The label set above supports the three-level breakdown in [`work-decomposition.md`](./work-decomposition.md) — epic, milestone, bite-sized issue. Per repo: + +- **Enable milestones** (`Issues → Milestones`) and use them as the middle level: one milestone per demonstrable slice, burning down to zero. +- **The `epic` label** marks the umbrella issue, which links `plan.md` and lists its milestones as a task list. +- **Ordinary issues** carry the standard labels, belong to a milestone, and are closed by exactly one PR via `Closes: #N`. + +Projects tracking work in Linear or Jira map the same three levels onto their own primitives; see [§10](#10-notifications-and-integrations). + ## 9. CLA bot For public repos accepting external contributions, install the [CLA Assistant](https://github.com/cla-assistant/cla-assistant) (or equivalent). External contributors sign the ChainSafe CLA before their PR can merge. @@ -116,7 +127,7 @@ Internal-only repos can skip this step. - **Slack notifications.** Wire the relevant team channel for PR opens, reviews, and merges. Volume: opt for "all events" on small repos, "review requested + merged" on large ones. - **Scheduled reminders.** A weekly bot ping for stale PRs (>3 business days without review). -- **Linear / Jira integration.** If the project tracks work outside GitHub, wire the integration so PRs link issues and vice versa. +- **Linear / Jira integration.** If the project tracks work outside GitHub, wire the integration so PRs link issues and vice versa. The three-level breakdown still applies — map epic / milestone / issue onto whatever the tracker calls them (Linear: project / cycle-or-milestone / issue), and keep one PR per issue. ## What this page does NOT cover @@ -145,6 +156,7 @@ If any of those don't happen, fix the misconfiguration before considering the re ## Related +- [`work-decomposition.md`](./work-decomposition.md) — the epic / milestone / issue model the labels and milestones here support. - [`pr-authoring.md`](./pr-authoring.md) — how PRs against this repo get authored. - [`code-review.md`](./code-review.md) — how PRs get reviewed. - [`testing-and-qa.md`](./testing-and-qa.md) — the testing baseline CI enforces.