box/CONTRIBUTING.md

279 lines
16 KiB
Markdown
Raw Normal View History

# Contributing
How change lands in this repo. The short version: PRs are born as drafts,
three reviewer bots take the first rounds, a human takes the last word — and
labels tell you where everything is without opening anything.
## The PR loop
1. **Fork and branch.** Contributors work from forks; upstream branches are
for maintainers. Title the PR conventionally (`feat:`, `fix:`, `docs:`),
and include a `CHANGELOG.md` entry under `## Unreleased` when the change
deserves one.
2. **Open as a draft** while you build. Drafts are invisible to the reviewer
bots on purpose.
3. **When it's ready**: mark ready-for-review and request all three bots —
`claude-bot-andresmgsl`, `codex-bot-andresmgsl`, `grok-bot-andresmgsl`.
They poll roughly every 15 minutes.
4. **Rounds are answered whole.** Wait until all three have reviewed, then
answer the entire round in a **single reply**, push the fixes, and
re-request the bots that didn't approve. Prefer verification over
argument: a test settles what a comment thread can't.
5. **Reviews end in a verdict.** A reviewer — bot or human — either
**approves** or **requests changes**, never a bare comment. A
comment-only review is a non-verdict: it doesn't say whether the round
passed, and the state machine (and anyone scanning the board) has to
guess. The verdict carries *blockingness only*, the body carries the
feedback: non-blocking nits ride an **approval** and the author addresses
them at their discretion; anything blocking — including a question that
gates the verdict — is **request changes**, saying what unblocks it. The
reconciler treats a comment-only review as not-approved, so commenting
without a verdict only stalls the PR. The machine never reads review
bodies: when a comment-only reviewer's line is really an agreement, that
judgment belongs to the **author** — escalate by requesting the
maintainer's review (step 6), and the reconciler flips the label on that
request, because an explicit request is a fact it can trust.
6. **When the round passes, the author hands the PR to the maintainer** in
three acts, in this order: post the tagged round summary, request the
maintainer's review, then set `state:needs-human` yourself — removing the
state label it replaces. The review request is what *earns* the label,
provided the PR carries **no `blocker:*` label**. A blocker means the work
is still yours whatever the round said, so on a conflicted or red PR
neither the request nor your own label write will stick — the sweep takes
it straight back off. With three formal head-current approvals the labels
workflow requests the maintainer automatically; when part of the panel is
comment-only, reading their agreement is the author's judgment, so the
author makes the request.
Writing the label by hand is an **optimistic write, not a transfer of
ownership**. The machine stays the authority — but because the workflow
wakes on `labeled`, the author's own write fires the sweep that validates
it, and a handoff that had not earned the label is corrected seconds later.
Forgetting the write is not a failure either; it only means the label waits
for the cron, which is the lag this replaced.
7. **Checks must be green**: `shellcheck` and `bash test/cli.sh` locally
mirror what CI runs; the multi-user rehearsal runs in CI on a real Incus.
## Changelog entries
Every PR that changes behaviour adds **one line** to `## Unreleased`. One line
is the whole rule — if it wraps more than twice in your editor, cut it down.
- **Say what changed, and stop.** Why it was wrong, how it was found, what it
cost, what it implies — that belongs in the PR body and the commit message,
which is where anyone chasing the reasoning already goes. This file answers
one question: what is different in this version.
- **Any word that can be removed, is removed.**
- **Lead with the surface, not the mechanism.** "`state:needs-human` is set at
handoff" beats "the labels workflow now also wakes on `labeled`".
- **Cite the issue or PR** — `(#141)` — and let the reader follow it for the
rest.
- **Mark a breaking change** with a leading `BREAKING:`.
- Group under `### Added` / `### Changed` / `### Fixed` / `### Removed`.
- No bold run-in headings, no sub-paragraphs, no code blocks, no prose essays.
Good:
```markdown
- `state:needs-human` is set at handoff, not by the cron (#141)
- An unreadable check rollup no longer reads as "nothing is failing" (#136)
- BREAKING: `--class human|server` is now `--root-door closed|open` (#77)
```
Not an entry — that is a PR body:
```markdown
- **`state:needs-human` no longer waits on the cron to become true** (#141) —
the labels workflow now also wakes on `pull_request_target: labeled` and
`unlabeled`, and the author sets it themselves when handing a PR over. A
review landing was never a trigger. There is no `pull_request_review_target`,
and on fork PRs — which is all of them here — ...
```
## Releases
feat: merging a release-labeled PR is the release (#96) The 0.7.0 ceremony exposed the gap: the release PR merged with four approvals and nothing happened, correctly, because publishing hung off a separate, manual, silent-when-forgotten tag push — the worst failure shape, no error and no red X. The ship decision already lives in the release PR, so the merge now IS the release. release.yml grows a second door: pull_request closed on main, gated on merged == true AND the hand-set release label (read from the event payload — no extra permission). Four asserts, in order, each fail-loud and creating nothing: VERSION at the merge commit is non--dev; VERSION changed in this PR (merge vs first parent — the -dev interlock that kills a mislabeled ordinary PR); the version's CHANGELOG.md section extracts non-empty via the existing release-notes.sh; and no tag or release exists yet. Then, in the same job, it creates the tag ref at the merge commit via the API and publishes with gh release create --verify-tag. Same-job on purpose: a GITHUB_TOKEN-created tag triggers no workflows (GitHub's anti-recursion), so the tag door can never fire off it and double-publish, and the no-existing assert covers a manual tag racing the merge. The tag-push path stays step-for-step identical as the documented manual fallback and backfill, gated to the push event so a closed PR never runs it against a branch ref. CONTRIBUTING.md's Releases section now reads "the maintainer's merge IS the release", with the manual tag ritual kept as the fallback. test/release.sh grep-pins the merged+labeled gate, all four asserts, the same-job tag+publish, and that the tag-push trigger survives — in the same daemon-free, fail-closed style. Fixes #96 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-19 15:19:31 +00:00
A release is a PR, and merging it ships it
([#96](https://github.com/heavy-duty/box/issues/96), building on
[#83](https://github.com/heavy-duty/box/issues/83)):
1. **The release PR**`release: X.Y.Z`, labeled `release` — bumps `VERSION`
from `X.Y.Z-dev` and stamps the `## Unreleased` section with version +
date (feature PRs land their changelog entry as part of the PR, so the
section is already written).
**Stamping is two edits, not one — the second is re-arming.** After
rewriting `## Unreleased` into `## X.Y.Z — DATE`, put an **empty
`## Unreleased` back at the top**, immediately above the section you just
stamped:
```markdown
## Unreleased
## 0.7.1 — 2026-07-19
### Fixed
...
```
Not cosmetic, and not deferrable to the next PR that happens to need it.
Between the stamp and the next re-creation of that heading, `main` has no
`## Unreleased`. A PR authored *before* the release wrote its entry under
that heading; with the heading gone, git lands the entry under whatever
now occupies the position — **the section that just shipped** — and it
merges **cleanly**, no conflict, no signal. The changelog then credits a
released version with a change it does not contain, and nothing but a
human reading the file will ever say so
([#108](https://github.com/heavy-duty/box/issues/108); confirmed in the
sibling repo as
[heavy-duty/rig#66](https://github.com/heavy-duty/rig/issues/66)).
CI enforces the arming rule with
[.github/scripts/changelog-armed.sh](.github/scripts/changelog-armed.sh),
keyed on `VERSION`: a `-dev` tree must carry `## Unreleased` on top; a
bare-`VERSION` tree (the ceremony PR, and the merge that publishes it) may
carry either `## Unreleased` or its own stamped section. That is why the
guard cannot simply demand `## Unreleased` unconditionally — the
unconditional form is false on the ceremony PR's own tree and makes the
release unshippable, which is why rig and cast both reverted it. The
practical consequence: forgetting to re-arm does **not** block the release
PR, it turns `main` red on the very next push — the automatic `-dev` bump
the release itself makes. Do it in the ceremony PR and main is never
disarmed at all.
**Release headings are append-only.** When you add your entry under
`## Unreleased`, *insert above* the heading below it — never type over
that line. Replacing `## 0.8.0 — 2026-07-19` with your own `## Unreleased`
block deletes a shipped section: its prose is absorbed into `Unreleased`,
`release-notes.sh` can no longer find the version it extracts by heading,
and the next release republishes the absorbed prose as if it were new. git
merges that edit cleanly and `changelog-armed.sh` stays green on it — the
top section is still the right one — so
[.github/scripts/changelog-monotonic.sh](.github/scripts/changelog-monotonic.sh)
asserts the other half on every PR: the set of `## X.Y.Z` headings on your
branch must be a **superset** of the set at the merge base
([#122](https://github.com/heavy-duty/box/issues/122), caught in review of
[#118](https://github.com/heavy-duty/box/pull/118)). The ceremony's own
stamp passes it by construction — rewriting `## Unreleased` into
`## X.Y.Z — DATE` adds a heading and removes none.
feat: CI refuses a release PR with no drill record CONTRIBUTING.md has said since #96 that the release PR is where the full real-hardware drill hangs, recorded in drill/RUNS.md. No release has ever done it: #95, #114 and #148 all shipped as a VERSION bump plus a CHANGELOG.md stamp, and RUNS.md carries no '## Release drill' section at all. A reviewer bot finally blocked on it — which is the point. The one time it was caught is the one time somebody happened to look, and that is not a gate. So the rule moves out of the document and into CI. .github/scripts/drill-recorded.sh, keyed on VERSION the same way changelog-armed.sh is: a -dev tree passes with nothing to assert (which is what keeps the guard installable — a version that fired on every PR would be switched off inside a day), and a bare VERSION must carry a section headed '## Release drill — <version>', optional ' — <date>' tail, with prose under it. The version is compared as a whole awk field, never as a substring, so 0.9.0 cannot be satisfied by a 0.9.0-rc1 drill or vice versa — release-notes.sh's trap, solved the same way so the two cannot disagree about what "the section for X" means. What it asserts is a RECORD, deliberately, not a passing drill: CI cannot run the drill (real hardware, the better part of an hour — ci.yml says as much about the rehearsal job it runs instead). That also keeps the maintainer waiver honest — a release that must ship undrilled writes that under the same heading, so the skip is a reviewable line in the diff rather than silence. Wired into ci.yml as its own step, NOT pull-request-only, for the reasoning #143 applied to the monotonic guard: the merge that publishes a release is a push to main carrying the same bare VERSION, so a PR-only check would leave the tree that actually ships unasserted. test/release.sh grows 27 cases (134 -> 161). Every fixture carries its own VERSION and its own RUNS.md — reaching for $ROOT/VERSION is the coupling #146 had to fix, and it goes red on the ceremony tree, the one tree where the release suite most needs to be trustworthy. CONTRIBUTING.md now states the flow (draft -> ready -> bot round -> drill -> state:needs-human -> merge), the heading format, that three releases shipped through the gap, and the recorded-waiver escape. It also describes the drill as ONE orchestrated run over the whole stack, because box and rig are mutually recursive and cannot be linearly ordered: rig sits below box as the host-builder ('rig bootstrap --host yes' installs box and runs setup-host) and above it as the guest-converger (a box new seed curls rig's installer and runs 'rig bootstrap <tenant>-box'), the inverted edge bin/box already documents as rig#28. The run is host bootstrap -> box new -> tenant converge -> cast. It drills CANDIDATE REFS, not released artifacts: RIG_REPO/RIG_REF are mint-time environment variables defaulting to heavy-duty/rig@main (bin/box:1116-1117), so a run pins the exact commits under test and no repo must be released before another can be drilled. Drilling the candidate is drilling the release — a release diff is VERSION + CHANGELOG.md, so nothing executable differs. One run, one shared run ID; each repo records its own legs citing that ID and the other two SHAs, and the guard reads only this repo's file. Recorded as a known gap, not fixed here: a released box still defaults RIG_REF to main, so a box minted a week after a drill is not the drilled combination. Pinning RIG_REF to a released rig tag in the templates is the outstanding step from #81 (rig#32 step 5). LABELS.md documents blocker:drill-pending — ceremony correct but unevidenced, maintainer-created because the bot account gets a 403 on label creation, with `blocked` standing in until it exists. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-21 15:20:37 +00:00
**The drill gates the release, and CI enforces it.** The release PR's
flow is: draft → ready → bot round → **drill**`state:needs-human`
maintainer merge (which *is* the release). CI proves the tier's semantics
on every PR; the drill is what proves the VM trust boundary, and it wants
real hardware and the better part of an hour, so it runs on the release
PR's branch and nowhere else.
Record it in [drill/RUNS.md](drill/RUNS.md) under its own heading:
```markdown
## Release drill — X.Y.Z — DATE
```
...with the run under it — what it measured, what it found, what it cost.
[.github/scripts/drill-recorded.sh](.github/scripts/drill-recorded.sh)
asserts exactly that on every tree with a bare `VERSION`, which is every
`release` PR and the merge that publishes it; a `-dev` tree passes with
nothing to assert. It is **no longer a thing a reviewer has to remember**.
It became CI's job because remembering did not work. The sentence this
paragraph replaces described a step **no release had ever performed**
`drill/RUNS.md` carried no `## Release drill` section at all — and #95,
#114 and #148 all shipped through the gap as a `VERSION` bump plus a
`CHANGELOG.md` stamp. The one time it was caught was the one time somebody
happened to look, which is not a gate.
**The drill is ONE orchestrated run over the whole stack**, not three
drills in a queue. box and rig are **mutually recursive**, so there is no
linear order to put them in: rig sits *below* box as the host-builder
(`rig bootstrap --host yes` installs box and runs `setup-host`) and
*above* it as the guest-converger (a `box new` seed's cloud-init curls
rig's installer and runs `rig bootstrap <tenant>-box`). box's own source
says as much — the seeds "invert the rig→box install edge (rig#28: rig
installs box on hosts; now box guests install rig)". The run goes:
1. `rig bootstrap --host yes` on a bare Debian host — installs box, runs
`setup-host`
2. `box new` mints a creds-free seed
3. the seed converges itself via `rig bootstrap <tenant>-box`
4. cast on top
It drills **candidate refs, not released artifacts.** `RIG_REPO` and
`RIG_REF` are mint-time environment variables (defaults
`heavy-duty/rig` and `main`, `bin/box`), so a run pins the exact commits
under test. That dissolves the chicken-and-egg: **no repo has to be
released before another can be drilled.** And drilling the candidate *is*
drilling the release — a release PR's diff is `VERSION` plus
`CHANGELOG.md` and nothing else, so no executable byte differs between
the tree that was drilled and the tree that ships.
One run emits **one shared run ID**. Each repo records its own legs under
its own `## Release drill — X.Y.Z — DATE`, citing that run ID and the
other two repos' commit SHAs, so the three records reconcile into a single
run afterwards. The guard reads only this repo's file — it asserts box's
record exists, never the other two.
**A known gap, and box is where it belongs.** A *released* box still
defaults `RIG_REF` to `main`, so what a user mints a week after a drill is
not the combination that was drilled — the guest converges against
whatever rig's main has become since. Pinning `RIG_REF` to a released rig
tag in the templates is the outstanding step from
[#81](https://github.com/heavy-duty/box/issues/81) (rig#32 step 5), and it
is what would make a drilled combination reproducible for users. This PR
does not fix it; box's source already says the two directions "track main
unpinned today, said honestly ... until a release flow exists".
A maintainer **waiver** is possible, and it is still written down. The
guard requires a *record*, not a passing result, so a release that must
ship without a full drill puts the section under the same heading and says
plainly that the drill was waived and why. Skipping then costs a
deliberate, reviewable line in the diff — which is precisely what the
three silent skips above did not.
feat: merging a release-labeled PR is the release (#96) The 0.7.0 ceremony exposed the gap: the release PR merged with four approvals and nothing happened, correctly, because publishing hung off a separate, manual, silent-when-forgotten tag push — the worst failure shape, no error and no red X. The ship decision already lives in the release PR, so the merge now IS the release. release.yml grows a second door: pull_request closed on main, gated on merged == true AND the hand-set release label (read from the event payload — no extra permission). Four asserts, in order, each fail-loud and creating nothing: VERSION at the merge commit is non--dev; VERSION changed in this PR (merge vs first parent — the -dev interlock that kills a mislabeled ordinary PR); the version's CHANGELOG.md section extracts non-empty via the existing release-notes.sh; and no tag or release exists yet. Then, in the same job, it creates the tag ref at the merge commit via the API and publishes with gh release create --verify-tag. Same-job on purpose: a GITHUB_TOKEN-created tag triggers no workflows (GitHub's anti-recursion), so the tag door can never fire off it and double-publish, and the no-existing assert covers a manual tag racing the merge. The tag-push path stays step-for-step identical as the documented manual fallback and backfill, gated to the push event so a closed PR never runs it against a branch ref. CONTRIBUTING.md's Releases section now reads "the maintainer's merge IS the release", with the manual tag ritual kept as the fallback. test/release.sh grep-pins the merged+labeled gate, all four asserts, the same-job tag+publish, and that the tag-push trigger survives — in the same daemon-free, fail-closed style. Fixes #96 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-19 15:19:31 +00:00
2. **The maintainer's merge IS the release.**
[release.yml](.github/workflows/release.yml) fires on the merged,
`release`-labeled PR and asserts before creating anything: `VERSION` at
the merge commit is non-`-dev` **and changed in this PR** (the `-dev`
interlock — a mislabeled ordinary PR fails loudly and creates nothing),
the version's `CHANGELOG.md` section extracts non-empty, and no tag or
release exists for it yet. Then, in the same job, it tags the merge
commit bare `X.Y.Z` (no `v` prefix, the `0.6.0` precedent) and publishes
the GitHub release with that section as the body. No assets — the source
tarball for the tag is the package, and `install.sh` downloads exactly
that.
*Manual fallback/backfill*: the tag-push path stays. Tagging the merge
commit bare `X.Y.Z` by hand and pushing the tag still publishes the same
way (release.yml asserts the tag names the tree's own `VERSION`) — for
backfills, or the day the merge path is red.
3. **The release re-arms main itself**: the same workflow run bumps
`VERSION` to `X.Y.(Z+1)-dev` and pushes the commit straight to main —
no follow-up PR (it opens one only if branch protection refuses the
direct push, and says so loudly). Not cosmetic — the versioned layout
names install trees after `VERSION`, so a `main` install without the
bump would land in `versions/X.Y.Z` and impersonate the release just
cut. On the *manual* tag path the bump stays yours: open the one-line
PR after publishing.
## Labels — who sets what
The full taxonomy lives in [LABELS.md](LABELS.md). What matters day to day is
who sets each kind — most of it is machinery, and hand-moving a
machine-owned label just gets corrected on the next pass:
| Labels | Set by |
|---|---|
| `state:*` | the labels workflow ([.github/workflows/labels.yml](.github/workflows/labels.yml)) — recomputed from GitHub's own facts on PR events (label changes included) and every 15 minutes. Machine-owned, with one exception: the author sets `state:needs-human` at handoff (step 6) and the workflow reconciles it. Otherwise never by hand. Exactly one per PR: *whose ball is it.* |
| `blocker:*` | the same workflow, from the same facts — *what is in the way.* Any number per PR, or none. Never by hand: applying one does not stop a merge, and removing one does not unblock anything. Fix the thing and the next sweep drops the label. |
| `stale` | the same workflow — 48h without commits, comments, or reviews. `blocked` PRs are exempt: they are quiet legitimately. |
| `scope:*` on PRs | actions/labeler, from the changed paths ([.github/labeler.yml](.github/labeler.yml)). Additive — you may add more, the machine won't remove them. |
| `scope:*` on issues | you, when opening or triaging — issues have no paths to derive from. |
| `blocked`, `release` | you — automation never guesses intent. |
| `merge-next` | you or the agent owning the queue. Which PR lands first is a judgement about how they conflict, so the workflow never sets it — it only **clears** it, the moment the PR stops being something a human could merge. |
| `bug` / `enhancement` / `documentation` | you, on issues only — a PR's type already lives in its title. |
## Issues
Give issues the same care as PR titles: say the surface in the title, apply a
`scope:` label and a type label (`bug` / `enhancement` / `documentation`) when
you open one, and `blocked` when it waits on something — that is what keeps
the board navigable as the issue count grows.