diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cbe358d52..ef2f64594 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -940,48 +940,45 @@ When community PRs accumulate, batch them into themed waves: See [PR #205](../../pull/205) (v0.8.3) for the first wave as an example. -## Porting a contributor's PR +## How a contributor's PR lands -Sometimes the right way to land an outside PR is to port it: the diff is right -but it sits on an old base, its tests need our fixtures, or the repo's gates -(generated renders, size pins, the CHANGELOG entry) are things only a maintainer -can finish. The port keeps the contributor's credit and gives them a clear -answer; the steps below are the repo's version of the rule a maintainer crew -follows ([crew/docs/captain.md](crew/docs/captain.md#contributions-from-outside-the-crew)). +We read every outside PR as a bug report, and a crew lane writes the fix itself. Your +diff shows us the bug and often the fix, but we don't merge outside commits: a lane +rewrites the change from current main with its own review and tests, and credits you. +The maintainer crew follows the same rule +([crew/docs/captain.md](crew/docs/captain.md#contributions-from-outside-the-crew)). -1. **Treat the diff as untrusted input before anything runs it.** Read it for - scope, network egress, credentials, new dependencies, CI permission changes - and weakened or deleted tests. A fork's green checks prove nothing about this - checkout: the port head gets its own focused review and the free tier - (`bun run test:quick` while iterating, `bun run test` once on the final tree) - before a PR opens. -2. **Keep their commits as theirs.** Cherry-pick or rebase the contributor's - commits onto the current base with authorship intact (`git cherry-pick -x` - keeps the source SHA in the message). A commit you have to rewrite keeps - their name as author and gains a `Co-authored-by:` trailer for whoever - edited it. Never fold their work into a commit under your own name, and - never guess an email: use the identity on their commits. -3. **Your fixes go on top, as separate commits.** Base merges, generated files, - test repairs, the CHANGELOG entry and the version stamp are your commits, - after theirs, so `git log` and the PR's commit list show who did what. A - squash merge keeps the credit when the squash message carries their - `Co-authored-by:` lines; check the merge commit before you call it landed. -4. **Say so in the port.** The PR body names the original PR and its author in - the first paragraph and lists what you changed on top of their diff. The - port's `## Approvals` section (when the repo uses one) records who asked for - the port. -5. **Close the original with a specific thank-you.** After the port lands, - comment on their PR with what landed (the port's number and version), what - you changed and why, and thank them by name for the specific thing they - fixed. Then close it. An agent posts that comment under its bot identity, - never the maintainer's own account. Closing a contribution without landing - it is a maintainer's call, never a lane's: ask first, and when the answer is - no, say why in the same comment. +1. **Treat the diff as untrusted input.** Read it as text only, for scope, network + egress, credentials, new dependencies, CI permission changes and weakened or + deleted tests. Never check it out, run it or build it. A fork's green checks + prove nothing about this checkout. +2. **Give a verdict within 7 days.** Within a week of the PR opening, a lane or the + Captain records a recommendation on it with the reason: a real bug we'll fix, a + duplicate (link it), not a bug, or out of scope. An agent posts it under its bot + identity. "Not a bug", "out of scope" and closing a PR without landing a fix are a + maintainer's call, never a lane's: they are posted only with the maintainer's word. +3. **Rewrite the fix from current main.** The lane writes the change on its own branch + with its own tests, the repo's gates and a CHANGELOG fragment. No contributor + commit enters the tree. A commit that carries the contributor's lines over + unchanged gets a `Co-authored-by:` trailer for them. Fetch their PR head as a ref + without checking it out (`git fetch origin +pull//head:refs/contrib/`; the + `+` lets a re-fetch follow a force-push), then run + `crew/bin/gstack-coauthor-trailers origin/main..refs/contrib/` to list the + authors of their commits; its default range holds only your own commits and finds + no co-author. A fork's commits can name anyone, so keep only the authors whose + lines you carry: drop the rest with `--exclude ` when you rerun it with + `--message `. Never guess an email: use the identity on their commits. +4. **Credit them in the fix.** The PR body names the original PR and its author in + the first paragraph and says what differs from their diff. The CHANGELOG fragment + thanks them by handle. +5. **Close the original with a specific thank-you.** After the fix lands, comment on + their PR with what landed (the PR number and version), what changed from their + diff and why, and thank them by name for the specific thing they found. Then close + it. An agent posts that comment under its bot identity, never the maintainer's own + account. -A port is not a rewrite. If the contributor's approach is one you would not -land, say that on their PR and close or park it under -[Issue triage](#issue-triage) instead of shipping a different change under -their name. +If we won't fix what the PR reports, say why on the PR and close or park it under +[Issue triage](#issue-triage), with the maintainer's word. ## Upgrade migrations diff --git a/captain/templates/CREW.md b/captain/templates/CREW.md index 036a3687e..47c523752 100644 --- a/captain/templates/CREW.md +++ b/captain/templates/CREW.md @@ -108,8 +108,9 @@ Homes are paths in the gstack checkout the lane checked at session start. 17. **New files follow `REPO_CONVENTIONS`.** Home: the repository's AGENTS.md. -18. **Porting a contributor's PR.** Review and test the port head first; keep their - commits, yours on top. Home: `crew/docs/captain.md` (Contributions from outside the crew). +18. **Outside PRs are bug reports.** A verdict within 7 days; a real bug is rewritten on + main with credit, and no contributor commit enters the tree. Home: `crew/docs/captain.md` + (Contributions from outside the crew). 19. **CHANGELOG-only conflicts** are the train's, not yours (folded into 14). Home: `docs/ship-policy.md` (/ship before READY: the stamp keeps the pin's receipt). diff --git a/captain/templates/brief.md b/captain/templates/brief.md index 7a7064993..602a74169 100644 --- a/captain/templates/brief.md +++ b/captain/templates/brief.md @@ -48,8 +48,8 @@ Context: after the install check, read the gstack skill's lane-card.md (the chec audit your decision trail, write Attention and send READY; after READY, handle PR events as usual (crew/docs/captain.md, the lane's /ship sequence). -Done when: the READY block is sent at a frozen head, reports/.md is written, -and its Approvals table carries the plan approval row (the owner's words, the date) or the +Done when: the READY block is sent at a frozen head, reports/.md is written +(its Open work line names a live owner for every open PR and issue it leaves), and its Approvals table carries the plan approval row (the owner's words, the date) or the Plan: exempt reason. ``` diff --git a/captain/templates/report.md b/captain/templates/report.md index 775dbc1cd..bac7b35c8 100644 --- a/captain/templates/report.md +++ b/captain/templates/report.md @@ -16,6 +16,9 @@ Unverified: · SHIP_RECEIPT: pr= head= · screen: +Open work: + ## Needs <"nothing", or the owner's decisions in the ask format (gstack crew/docs/captain.md): one lettered line each, "A1 to take recommendation"> diff --git a/changelog.d/factory-lessons-from-gbrain-queue-gst-2077.md b/changelog.d/factory-lessons-from-gbrain-queue-gst-2077.md new file mode 100644 index 000000000..449e090c2 --- /dev/null +++ b/changelog.d/factory-lessons-from-gbrain-queue-gst-2077.md @@ -0,0 +1 @@ +- **Crew docs now say how a factory keeps work from being orphaned, and how outside PRs land.** These are lessons from gbrain's merge queue, which reached 220 open PRs in November 2026. crew/docs/captain.md has a new "No orphaned work" section. A Captain archives or rotates a thread only once a live successor owns its open PRs and issues. Successor triggers sit on an issue, never only in notes. Sweeps re-own open PRs whose owning thread is archived. A crew PR closes only as landed, superseded or dropped, and anything else is salvaged onto current main. Before archiving a thread, the Captain re-assigns its issues instead of closing them or moving them to Backlog. The wave guidance adds one open queue PR per lane, follow-ups riding that PR or the next wave, waves cut after a day or once full, and when to move from direct landing to waves. Outside PRs are now bug reports. Each gets a verdict within 7 days, and real bugs are rewritten from main with credit, so no contributor commit enters the tree. CONTRIBUTING.md, captain/templates/CREW.md rule 18 and docs/review-policy.md say the same, including how to pass a contributor's commits to gstack-coauthor-trailers for credit. The report and brief templates carry an "Open work" line. Docs only; no tool behavior changed (GST-2077). diff --git a/crew/README.md b/crew/README.md index 16378c59b..7fb0ee28c 100644 --- a/crew/README.md +++ b/crew/README.md @@ -42,7 +42,7 @@ CI runs `crew/test/` with the rest of the free suite. | Path | What it holds | |---|---| -| `bin/` | `gstack-release-gate` (the paid gate on main every 8 hours) and `gstack-coauthor-trailers` (credit for ported contributor work) | +| `bin/` | `gstack-release-gate` (the paid gate on main every 8 hours) and `gstack-coauthor-trailers` (credit for contributor lines a rewrite carries over) | | `lib/` | merge trains and waves (`ship-queue-train*`, `ship-queue-wave`, the land lock, screens, rechecks), the release gate, version-gate trains and lane ownership | | `scripts/` | the release cut and its daily check (`release-cut`, `release-cadence`), and the paid-eval CI steps only our crews run (train gate and backstop, gate cost, gate reuse, receipt recovery, census arms, judge calibration configs) | | `test/` | their tests | diff --git a/crew/docs/captain.md b/crew/docs/captain.md index 14c85070f..fcadc665a 100644 --- a/crew/docs/captain.md +++ b/crew/docs/captain.md @@ -36,7 +36,8 @@ by a daily pass; nothing is retried into green. slots, relays asks to the owner in the three shapes, runs the flaky pass, and decides everything that needs no access, credentials, spend over cap or a cited approval. It archives only its own crew threads, each when its plan for that thread says the work is - done; it knows each thread's next step, so nobody else archives them. A thread outside + done and never while it still owns open work ([No orphaned work](#no-orphaned-work)); it + knows each thread's next step, so nobody else archives them. A thread outside every crew belongs to the owner and stays as it is: no Captain archives it, flags it or lists it for archiving unless the owner asks, even when the Captain's own automation created it. @@ -116,7 +117,7 @@ Where each rule lives: | 15 | CHANGELOG entry under `## [Unreleased]` before READY; the stamp touches only `STAMP_FILES` | [ship-policy.md](../../docs/ship-policy.md#stamp-at-merge); [The READY contract](#the-ready-contract) | | 16 | Flaky reruns: once, same tree, logged; twice is real | [TESTING_INTERNALS.md](../../docs/TESTING_INTERNALS.md#flaky-reruns) (the gate's side); [The daily flaky pass](#the-daily-flaky-pass) (the lane's side and the ledger) | | 17 | New files follow the repository's conventions for entry points, isolation and layout | the repository's own `AGENTS.md` (the `REPO_CONVENTIONS` parameter names the section) | -| 18 | Porting a contributor's PR: review and test the port head first; keep their commits | [Contributions from outside the crew](#contributions-from-outside-the-crew) | +| 18 | Outside PRs are bug reports: a verdict within 7 days; rewrite real bugs on main with credit, no contributor commit in the tree | [Contributions from outside the crew](#contributions-from-outside-the-crew) | | 19 | CHANGELOG-only conflicts are the train's to resolve, not the lane's | folded into 14: [ship-policy.md](../../docs/ship-policy.md#ship-before-ready) ("the stamp keeps the pin's receipt") | | 20 | A READY from before a contract change needs a fresh receipt | [The READY contract](#the-ready-contract) | | 21 | Approvals, plan asks and merge asks in the three shapes | [Approvals](#approvals-three-shapes); [capy/skill/gstack/captain.md](../../capy/skill/gstack/captain.md) | @@ -212,18 +213,48 @@ the work's issue. Why: a crew thread once created a thread in another project directly. That thread ran outside every crew, so no Captain briefed it, tracked it or could archive it. +## No orphaned work + +A PR or issue whose owning thread is archived has nobody to wake. CI, reviews and merges +wake only the thread that opened the PR, and nothing wakes anyone when a note's "start the +successor when X happens" comes true. In November 2026 gbrain's queue reached 220 open PRs +partly this way: eight trains and a finished feature sat for 1 to 8 days with no owner, +went stale and conflicting, and started to look like junk. + +- **Archive only with a live owner.** Before the Captain archives or rotates a thread that + still has an open PR, a handoff or an In progress issue, it starts the successor in the + same turn, and the successor subscribes to every open PR first (the + [/runaway-stopper successor brief](../../runaway-stopper/templates/successor-brief.md) + does this for rotations). When no successor is needed yet, the Captain takes the work + itself, on an issue assigned to a live thread with the trigger written in it. +- **Triggers never live only in notes.** A "successor takes the slot when X" line goes on + an issue owned by a live thread, never only in NOTES.md. +- **Sweep for orphans.** Every Captain wake that reads its crew, and the daily issue sweep + below, list open PRs whose owning thread is archived, issues with no live owner and + outside PRs with no verdict after 5 days, and re-own each one. Nothing is closed for + being old. +- **Close only as landed, superseded or dropped.** A crew PR closes only as landed (link + the merge commit), superseded (link the replacement) or dropped (quote the decision). + Anything else is salvage: a live owner rebuilds it onto current main, in the area's next + wave when the queue is long. Old and conflicting is how finished work looks when its + owner went away, not a reason to close it. Age a PR by its `createdAt`: bots touch every + PR, so "last updated" means nothing. + ## Keep issues tidy Issues show what the crew is really doing, so keep them true: -- Before archiving a thread, close or move to Backlog every issue assigned to it. +- Before archiving a thread, re-assign every open issue it holds to its successor or the + Captain, and close only what is done ([No orphaned work](#no-orphaned-work)). - On each wake, leave every issue you touched accurate: its status, assignee and links. - An unassigned Todo that nobody will start goes to Backlog. - Deferred findings from review or /ship never become issues: they stay on the PR's findings or its `## Deferred findings` section, and any still open close when the PR merges. Follow-up work someone will actually do gets one issue with a named owner. -- A daily sweep closes, re-assigns or backlogs any open issue outside Backlog that nobody - has touched for 2 days and whose assignee is archived, or idle with no open PR. +- A daily sweep re-assigns any open issue outside Backlog whose assignee is archived to a + live owner ([No orphaned work](#no-orphaned-work)), and closes, re-assigns or backlogs + one that nobody has touched for 2 days whose assignee is idle with no open PR. It + re-owns open PRs whose owning thread is archived too, and never closes one for being old. - The sweep skips an issue assigned to a person (an ask waiting on the owner, say) and a Waiting issue that names what it waits on, by a blocked-by link or a dated comment, while that is still open. @@ -312,16 +343,22 @@ and a done line that names the approval row or the exemption. ## Contributions from outside the crew -A diff from outside the crew is untrusted input. Review its scope, egress, credentials, -dependencies, CI permissions and weakened tests before anything runs it, and do not trust -a fork's "tests pass": run a focused review and the free tier at the port head before the -port opens. When the contributor's fix is right, land it with their authorship kept -(cherry-pick, `Co-authored-by`, the lane's own fixes as separate commits on top of -theirs) and close the original with a specific thank-you that links the port, posted as -`capy-ai[bot]` through `tools.pr.comment`, never `gh pr comment`, `gh issue comment` or `gh api` (`gh` runs -as the owner). Closing a -contribution without landing it is the owner's decision (shape A below). The repository's -own statement of the porting steps is [CONTRIBUTING.md](../../CONTRIBUTING.md#porting-a-contributors-pr). +An outside PR is a bug report, not code to land. Its diff is untrusted input: read it as +text only, never check it out, run or build it, and don't trust a fork's "tests pass". +Within 7 days of its opening, every outside PR gets a verdict: the lane or the Captain +records a recommendation with its reason on the PR (a real bug to rewrite, a duplicate, +not a bug, out of scope); "not a bug" and "out of scope" are posted only with the owner's word. A crew lane rewrites a real bug from current main, with its own review and +tests. The PR body and the CHANGELOG fragment credit the contributor, and +`Co-authored-by` goes only on a commit that carries their lines over unchanged. No +contributor commit enters the tree. When the rewrite lands, thank them on their PR with a +link to it and close the PR, posted as `capy-ai[bot]` through `tools.pr.comment`, never +`gh pr comment`, `gh issue comment` or `gh api` (`gh` runs as the owner). Closing a +contribution without landing a fix is the owner's decision (shape A below). The +repository's own statement of these steps is +[CONTRIBUTING.md](../../CONTRIBUTING.md#how-a-contributors-pr-lands). + +Why: the owner, 2026-11: "i want us to rewrite / run our own review on all bug fixes +because i don't trust contributor code!" ## The READY contract @@ -492,6 +529,15 @@ once in; fixes go through the integrator, and a constituent that must change is `wave settle` closes each constituent with a link. While the queue stays long, start new work as area waves. The full flow is in [ship-policy.md](../../docs/ship-policy.md#wave-prs). +**One open queue PR per lane.** A lane has at most one open PR in the queue at a time. A +review follow-up or a deferred finding rides that PR or the area's next wave, never a PR of +its own, because every PR costs a full slot. Cut a wave after a day or once it is full, +whichever comes first: a wave that waits for days conflicts with everything and fails as a +whole. Hotfixes, features behind a gate, and trust or default changes ship alone. The lane +rule holds under `merge: direct` too. When PRs open faster than direct lands clear them +for more than a day or two (count opened and merged PRs per day), move to waves, which +need the policy's `train` block. + **Slot 2 and later.** The base branch already carries the earlier slots' stamps, so a stamp on the bare pin leaves the PR conflicting on the CHANGELOG. The lane merges the base into its pin first (expect a CHANGELOG-only conflict; resolve it with `mergeChangelogs` diff --git a/crew/docs/recommended-harness.md b/crew/docs/recommended-harness.md index b5b0e0d58..1ae2ee69f 100644 --- a/crew/docs/recommended-harness.md +++ b/crew/docs/recommended-harness.md @@ -353,7 +353,7 @@ on a few points. Ship the one that fits the repository; both are supported. | Small fixes | one PR each, batched by the train | batched into wave PRs, since serial slots cost about 30 minutes each | | After the merge | the main backstop gates any push a train did not cover, and freezes the queue on red | the controller watches main's test, e2e and release runs until the release publishes; a red main freezes the queue until root-caused | | Severe triage | the Captain watches issues and PRs and starts a thread per unowned severe item | a scheduled severe sweep (every 4 to 8 hours) with a fixed severity bar, dedupe against owned lanes, one hotfix thread per batch | -| Contributor PRs | ported onto a crew branch with the author's commits kept and `Co-authored-by`; the fork PR is closed with a thank-you that links the port, posted as `capy-ai[bot]` through `tools.pr.comment`, never `gh` | landed through a wave PR with "Contributed by @handle", `Co-Authored-By` and `Supersedes` lines | +| Contributor PRs | read as bug reports and rewritten from main on a crew branch, so no contributor commit enters the tree; `Co-authored-by` only on lines carried over unchanged; the fork PR is closed with a thank-you that links the fix, posted as `capy-ai[bot]` through `tools.pr.comment`, never `gh` | the rewritten fixes land through a wave PR with "Contributed by @handle" and `Supersedes` lines, `Co-authored-by` only on lines carried over unchanged | | Privacy | the pregate's sealed-output and secret scans before every push | plus a per-user denylist grep (names, hostnames, identifying counts from private sources) before any public artifact | ## Known Capy limits diff --git a/docs/review-policy.md b/docs/review-policy.md index e4a7511e9..b74564120 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -849,10 +849,17 @@ and a later reader agree on what was flagged: under test. -## Porting a contributor's work +## Crediting a contributor's lines -A ported branch keeps the contributor's commits as theirs and adds the -porter's fixes on top, each crediting the original authors. +Outside PRs are rewritten from main rather than ported +([CONTRIBUTING.md](../CONTRIBUTING.md#how-a-contributors-pr-lands)). When a rewrite +carries a contributor's lines over unchanged, the commit that carries them credits them. +Pass the contributor's PR commits as the range, fetched as a ref and never checked out +(`git fetch origin +pull//head:refs/contrib/`, then `origin/..refs/contrib/`; +the `+` lets a re-fetch follow a force-push): the default range holds only the rewrite's +own commits and finds no co-author. A fork's commits can name any author, so list the +range first and `--exclude ` every author whose lines the commit does not carry +before adding `--message`. `gstack-coauthor-trailers [ | --base ] [--message ] [--exclude ]...` lists every distinct author of the range's commits (default `origin/..HEAD`) other than the checkout's own committer @@ -1034,7 +1041,7 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: - `lib/review-collect.ts`: `collectCommit` (git objects only, from any checkout) and `collectLive` (the working tree), per-path lines and the base commit's registry (`test/review-collect.test.ts`). - `lib/review-scope.ts`, `bin/gstack-review-scope`: `classify`, `defer`, `render`, `summary` (with `fix_checks`, `dispatches`, `est_input_tokens`, `unrecorded`, `ledgers_read`); refusals are `REVIEW_SCOPE_INPUT`, `REVIEW_SCOPE_RANGE` and `REVIEW_SCOPE_LEDGER` ([troubleshooting.md](troubleshooting.md#review-scope-input)). - `lib/ship-policy.ts`: the `review_scope` key and its `ship: review_scope …` consequence line. -- `review/checklist.md`: `prereg-with-results` under Gate Integrity and `test-keyboard-count-loop` under Pass 2; `crew/bin/gstack-coauthor-trailers` for the contributor-credit trailers of a port. +- `review/checklist.md`: `prereg-with-results` under Gate Integrity and `test-keyboard-count-loop` under Pass 2; `crew/bin/gstack-coauthor-trailers` for the contributor-credit trailers of a rewrite. - `lib/ship-receipt-prose.ts`, `gstack-ship-receipt read --check-prose`: the body's quoted counters against the receipt (`RECEIPT_PROSE_MISMATCH`, [ship-policy.md](ship-policy.md#ship-receipt)). - `lib/ship-receipt.ts`, `lib/headless-artifacts.ts`: the `review_loop` receipt object, its schema and the summary line. - `lib/no-stop.ts`, `bin/gstack-no-stop`, `lib/autoplan-carry.ts`: the [no-stop defaults](#no-stop), their ledger and the autoplan re-gate (`test/no-stop.test.ts`, `test/autoplan-carry.test.ts`).