Drill records move from sections inside drill/RUNS.md to one file per
version: drills/<version>.md. drill/RUNS.md is untouched — it stays the
harness's own run log, traps table and lore. drills/ is release evidence
only, and the docs now say which is which.
The old guard parsed headings: em-dash field matching, an optional
' — DATE' tail, whole-version comparison so 0.9.0-rc1 could not satisfy
0.9.0, avoiding \x escapes because CI runs mawk not gawk, and a non-blank
body rule. Every one of those existed only because records shared one
file, and two separate defects were found in review because of that
complexity — the sed '/./,$!d' whitespace bypass, and heading-grammar
drift from the sibling repos.
One file per version makes almost all of it unrepresentable: 0.9.0.md and
0.9.0-rc1.md are simply different files, so whole-version matching is
free rather than a trap. The guard keeps only the rule that was never
about headings — a record of pure whitespace is not a record — and keeps
the failure message that names the version, the path, the unblock, and
the recorded-waiver escape hatch.
Plain drills/, not .drills/ — a dot-directory is invisible to globs
without dotglob, which is what caused #116 and #118.
Also corrects the release-drill framing: the three repos' drills are
INDEPENDENT and run in any order. What dissolves the box<->rig recursion
is that every drill pins the same fixed candidate refs, not sequencing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
Both reviewers landed on the same line independently, and they are right.
cmd_new's hook-failure path offered `box restore $name pristine`
unconditionally, but three mints reach that line with no pristine mark, all
by this stack's own design: a `dir` pool (skipped, no CoW),
BOX_SNAPSHOT_PRISTINE=0, and a create incus refused (warned, mint continued).
On a `dir` host that is EVERY hook failure — so the operator standing at the
one moment the message exists for copy-pastes a restore that errors.
It could not simply be read off the return status. The never-fatal contract
means snapshot_mark returns 0 on all four paths on purpose — taken, skipped,
knob-disabled, refused — so `if snapshot_mark ...` cannot distinguish a mark
that exists from one that does not, and making it distinguish would put the
mint's success at the mercy of a checkpoint, which is what that contract
exists to prevent.
So the fact is recorded rather than inferred: `marks` accumulates the labels
snapshot_mark actually created, and `mark_taken <label>` answers the only
question a rollback offer may ask. Per-label, so one mark never answers for
another.
This is the same refusal the message one line earlier already makes when it
declines to invent a `bootstrapped` it never watched — a promise the disk
cannot deliver is the failure mode #130 was built to refuse. It just was not
applied to the restore offer sitting under it.
Pinned per path rather than once, because the three no-mark paths fail
differently and a single case would let the other two regress silently: five
drives over snapshot_mark + mark_taken (created / dir-skip / knob / refused /
no bleed between labels) and a static check that the call site is gated.
Verified by mutation — dropping the recording, and un-gating the offer, each
fail the suite rather than passing quietly.
converged anything. This takes the checkpoint one step later: after the rig
bootstrap hook has run and box has WATCHED IT SUCCEED, the box is converged
and untouched. 'box restore <box> bootstrapped' keeps the tenant role and
undoes only what happened after it, which is the undo an operator reaches
for far more often and which otherwise costs a ~10-minute re-mint.
The policy is #128's, shared rather than copied: snapshot_pristine and the
new snapshot_bootstrapped are thin wrappers over one snapshot_mark, so the
never-fatal contract and the 'dir' skip exist in exactly one place. What
does not generalise is the prose — the two marks name different moments —
so each wrapper owns its own narration.
Deliberately conditional where 'pristine' is unconditional. 'pristine' marks
a MOMENT every fresh mint has; 'bootstrapped' marks an EVENT, and a blank
box has no such event, so it gets no mark rather than a byte-identical
duplicate of 'pristine' at twice the disk cost. A FAILED hook gets none
either: the failure message already sends the operator to 'box shell', which
is a run box does not watch, so box hands over 'box snapshot <box>
bootstrapped' instead of labelling a convergence it never saw.
The label is therefore documented as one-directional on every surface: its
presence means the hook converged untouched, its absence means nothing at
all. Same durability caveats restated everywhere it appears — it dies with
the box on 'box rm', and no filesystem rollback reaches off-box state
(heavy-duty/rig#62).
Closes#130
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
All three reviewers landed on the same line. Reproduced on this head under
`set -euo pipefail`:
prev_n=08
[ -n "$prev_n" ] && [ "$prev_n" -eq "$prev_n" ] 2>/dev/null || prev_n=0
# -> guard PASSES: test parses 08 as decimal
n=$((prev_n + 1))
# -> bash: 08: value too great for base (error token is "08")
Two holes that had to close together. `test -eq` reads decimal, arithmetic
reads a leading zero as octal, so a value can pass the guard and still abort
the arithmetic it was guarding. The abort lands after the physical
`incus import` and before the stamp, the placement correction and the start —
the exact window the degrade-never-die contract exists to protect, and the
same side box_provenance()'s schema check falls on.
case "$prev_n" in ''|*[!0-9]*) prev_n=0 ;; esac
n=$((10#$prev_n + 1))
Digits-only closes sign and garbage; 10# forces base ten. Checked across
08 -> 9, 007 -> 8, 5 -> 6, and '' / not-a-number / -3 / 3x / 00 -> 1.
The existing not-a-number fixture could never have caught this: that value
fails the guard and degrades, so it exercises the path that already worked.
Added a count=08 fixture beside it, asserting both halves — the import does
not fail, AND the count advances to 9 rather than degrading to 1, because 08
is a real previous total and reading it as anything else would be its own
small lie. A zero-padded count is not exotic; it is what any external tool
that formats numbers writes.
Verified by mutation: with the old guard restored both new assertions fail.
Also drops user.box.mode.asked from the IMPCLONE fixture (grok's nit). Since
#129 the clone path clears that key, so a fixture built from the mint shape
that kept it described a box the clone path cannot produce. Nothing asserts
it today — which is precisely why it would have rotted unnoticed.
An imported box kept the artifact's mint stamp verbatim (#103) — correct, the
mint time, box version, image and origin belong to the originating host and
should survive the trip. But nothing recorded the import, so an imported box
was indistinguishable from one minted here at the artifact's mint time.
Not origin=import. 'origin' answers how the instance came into BEING — mint or
clone — and overwriting it would make an exported clone come back claiming to
be an import, with nothing left saying it was ever a clone and an origin.from
naming a lineage no key explains. The import is a third fact, orthogonal to
the first two, so it takes its own keys and leaves every other one alone.
Birth pair plus latest pair, the shape heavy-duty/rig#61 settled on for the
same repeated-event question: imported/imported.by pinned once and never
rewritten, imported.last/.last.by refreshed on every arrival, imported.count
for the trips in between. Last-wins alone would erase the evidence of the
earlier trips, which is the same mistake origin=import makes one level up.
box info prints IMPORTED directly under MINTED, because that adjacency is what
stops the artifact's mint time being misread as this host's. It states only
the ordering and never claims another host: box has no record of which host
minted a box, and a re-import onto the same host is the documented upgrade
flow. user.box.schema does not move — adding a key is not breaking — and is
not written by the import at all, so a legacy artifact still reads as
MINTED (not recorded) rather than acquiring a shape it does not have.
Closes#131.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both from claude-bot's review of #128.
storage_driver's two driver probes had no '|| true' while the pool probe one
line up did. They were safe only via a bash subtlety: command substitution
strips errexit, so a failing 'incus storage show' fell through to the fallback
instead of aborting. Add 'shopt -s inherit_errexit' to bin/box — the class of
robustness tweak #107 documents sailing through review — and under pipefail a
restricted tier's probe refusal becomes a fatal abort mid-mint, inside the
function whose contract is NEVER fatal. All three probes now read alike and
depend on nothing. Driven test: refuse both probes with inherit_errexit on,
assert the caller survives and the driver reads empty (the unreadable-pool
case, which already takes the mark anyway).
The clone's inheritance narration piped a multi-line 'incus snapshot list'
straight into 'grep -q' — #124's racing-reader class in a 'set -euo pipefail'
file. A 141 there reads as "no pristine" and narrates the WRONG inheritance
shape on a clone that does carry one. Capture first, then read. Pinned by
shape (grep|head|sed|awk|read), not by instance spelling.
bin/box has ~10 more sites of that class and no sweep covers the file; one of
them unpins an exposure's static address on a fail-open read. Filed as #134
rather than widened into this PR.
Refs #104, #107, #124, #134.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cmd_new's fresh-mint branch runs launch → wait_agent → cloud-init
status --wait → the rig bootstrap hook. In the gap between the last two
the guest IS pristine Debian plus box's thin seed — the state
heavy-duty/rig#62 calls "back to pristine Debian". It existed for a few
seconds on every mint box has ever taken and nothing captured it.
Take it there, unconditionally within a fresh mint and never fatally, so
'box restore <box> pristine' is a complete undo for every creds-free
tenant role. A --from clone takes none: it has no pristine moment, so it
inherits its source's snapshots or starts with none, and box will not
label a worked-in state as pristine. On a 'dir' pool (no CoW) the mark
would be a full multi-GB copy per mint, so the mint skips it loudly.
Closes#104
The sweep added in #107 matched `ufw status[^|]*\| *grep` — every historical
instance, and none of the equivalent spellings. `head -n1`, `sed -n '1p;q'`,
`awk '/x/ {print; exit}'` and `read` all close the pipe early, SIGPIPE the
writer, and yield the same 141 under pipefail. The pin guarded the instance
spelling of the very thing it existed to generalise.
Both halves of the matcher are alternations now.
Readers are deliberately not narrowed to the early-exit spellings: telling
`grep -q` from `grep -c` by regex is precision that rots, and all six
`ufw status` sites in the tree already capture first, so banning the pipe
outright costs nothing real.
Writers gain `incus config trust list`. host/revoke-user.sh:206 piped it into
`grep -q` as the --purge leftover assert, under `set -euo pipefail` — so
unlike drill/wipe.sh nothing but the writer's size was holding it, and left of
`&&` a 141 is set -e-exempt too. It would have read as "no leftover cert" on a
host that still trusts the revoked user's certificate and called the purge
complete: fail-open, on the path whose job is to prove access is gone. Now
captured into `trust_csv` and matched with `[[ ]]`, with a leading newline so
the first CSV row anchors like the `^` it replaces.
Writers are enumerated rather than generalised — ~150 legitimate `| grep`
sites exist under host/ and drill/, nearly all re-reading a captured string —
so the sweep claims only what it can check. The `id -nG | tr | grep -qx`
shapes in grant/revoke/setup are left alone: single tiny writes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The rebase onto main was textually clean and semantically wrong. #123
renamed the tenant templates (claude -> claude-box, following rig#76) while
this branch was open; the mint-stamp drive was written against the old name
and git had no reason to say so — the two changes never touch the same line.
`box new --template claude` now dies at the template lookup, so all 18 mint
assertions failed on a shimmed mint that never reached a launch line. Renamed
at the two mint invocations and the two stamp assertions that read the new
name back (user.box.template, user.box.role).
Deliberately NOT renamed:
- `user.box.user=claude` — the seed user did not move. #123 split the two
namespaces on purpose: the template and the rig role are `claude-box`,
the user the role converges stays `claude`. The assertion pins that split.
- the canned `$STAMPED`/`$PRERENAME` info fixtures — they stand for boxes
minted BEFORE the rename, which is exactly the case box promises to keep
reading (cmd_new matches both spellings of user.box.template for the login
hint). Renaming them would delete the coverage.
The clone's re-stamp split provenance into two columns: event facts
re-stamped (schema/version/created/origin/origin.from), lineage facts left
alone (template/user/image/role/rig/mode). Review found one key that sits in
neither: user.box.mode.asked.
It is a mint-EVENT fact by the split's own criterion — only the mint knew
whether a container was asked for or fallen back into for want of /dev/kvm —
but the asker was the SOURCE's operator. A clone refuses --vm/--container
outright, so nobody was asked anything about this instance. Riding through
'incus copy' untouched, it made 'box info' on a clone print
MODE vm (asked: auto)
describing a demand never made of it, with nothing marking it ancestral.
There is no true value to re-stamp it with, so it is CLEARED — cleared and
not set-to-empty, because an empty value is still a key a reader would find.
The unset tolerates failure: a source that predates the stamp never carried
the key, and a clone must not die over a key that was already absent. It
lands with the re-stamp and before the start, the same rule and the same
reason as the rest: a clone is never observable wearing an 'asked' its
operator never gave.
The read side needs no special case. The MODE line was already gated on
'asked', so absence renders as silence — the same absence-is-silence rule the
whole provenance block uses — while TYPE still reports VM or CT off the
instance type 'incus copy' preserves.
+7 checks (529 -> 536), each proven to bite.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cmd_new knew a great deal at the moment it launched and wrote three user.*
keys, dropping the rest: the box version that minted it, the base image (an
unpinned alias on a moving remote), the rig role, which rig repo and ref
converged it, the mint time, and whether a container was chosen or fallen
back into for want of /dev/kvm. There is no host-side per-box store — the
Incus instance config IS the database — so every one of those facts was gone
the moment the mint returned.
The same single write point now carries them as user.box.*, plus
user.box.schema=1 naming the stamp's shape. The alias's resolved fingerprint
is pinned in a second call after the launch, read from volatile.base_image,
best-effort by construction: a box that exists and boots must never be failed
over a provenance field.
A clone re-stamps rather than inheriting. 'incus copy' carries every user.*
key forward (audit B2), so an inherited stamp would not go stale, it would go
false. --from now re-stamps schema/version/created/origin/origin.from on the
copied instance before it starts, and leaves the lineage keys (template,
user, image, role, rig pin) alone — the clone's disk genuinely came from
them. origin.from records one hop.
cmd_info grows a provenance block, tolerating absence everywhere: boxes
minted before this stamp existed render as a box with blanks, and a schema
this box does not recognise is treated as newer than it, not as broken.
Closes#103.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
wipe.sh piped `ufw status` straight into `grep -q "Status: active"`. That is
closes the pipe, ufw takes SIGPIPE, and the pipeline yields 141.
It was correct today, and only by accident — the file is `set -u` with no
`pipefail`, so the 141 was discarded and grep's 0 carried the branch. It was
one line from wrong: adding `set -o pipefail` for unrelated robustness would
have silently skipped every UFW removal on a host the operator was told is
wiped, with no error and no red X. Measured on a shim: 5/5 runs took the
wrong branch under pipefail, 3/3 the right one without.
Transplant #106's pattern verbatim from host/teardown-host.sh: capture
ufw_status once and match with `[[ ]]`; rewrite the numbered-delete loop —
whose condition was itself an early-exit reader, plus an un-captured re-read
to get the number — as a `while :` that reads one capture per iteration and
breaks on absence. The re-scan stays per-delete, since numbers shift after
each removal; it just no longer races. Removals keep the file's
`cmd && say "did X"` idiom.
Generalize the test/cli.sh pin from the one site to the class: sweep every
host/*.sh and drill/*.sh for the racing shape and name the offenders, so a
new script in either directory inherits the pin instead of being one more
site to remember. Comment lines are stripped before matching — each fix's
own commentary quotes the racing shape to explain it, and a prose-blind pin
would fail on the comment documenting why it exists. The positive pins (the
capture, the break-on-absence) now run per file over both, so the sweep
cannot be satisfied by deleting a block instead of fixing it.
drill/doctor.sh was checked and needs nothing: it already reads into
`ufw_out` and is safe by construction, not by absent pipefail.
Refs #107
The monotonic step is the LAST step of the `check` job, so stopping only at
the next `- name:` ran the block into `rehearsal` and swallowed its job-level
`if:`. Unanchored `grep -q 'if:'` then fired on it — reintroducing the bug the
scoping was meant to fix, moved from "any step in the file" to "this step plus
the head of the next job".
Terminates on a new step OR a new job now, and the key is anchored so an `if:`
inside a `run:` line is not mistaken for a step condition.
Mutation-tested four ways: baseline green; an unrelated job gated stays green
(was the failure); the monotonic step gated fails; the step renamed trips the
companion found-the-block guard.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The inner `bash -c` takes its paths as positionals, so the single quotes are
deliberate — same class as the release-notes fixtures above it, and reusing
their exact disable wording.
Mine to catch: I linted .github/scripts, bin/box and install.sh by hand, while
CI lints `bin/* **/*.sh` under globstar+dotglob, which includes test/. The
narrower local invocation is what let this through; reproduced CI's exact
command before pushing this time.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Dropping the pull_request gate made merge_base == HEAD a routine path rather
than a degradation, and the success line did not follow. On every push to main
the step printed "all N release heading(s) at the merge base are still present"
— a containment claim on the one event where deletion is undetectable, since
the comparison is the file against itself.
That is the dishonesty this PR fixed in the skip messages, surviving in the
success message. The line now has two forms: containment vacuous, naming
uniqueness as the half that ran, or the existing containment wording when a
real base exists. Both pinned.
Also scopes the ci.yml negative pin to the monotonic step's own block. As a
file-wide grep it forbade any FUTURE step from being pull_request-gated and
would have failed citing #143 when one legitimately was.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Uniqueness is a property of HEAD alone — no base ref, no merge base, no base
blob. It sat downstream of all three, so every degradation path returned
success on a tree carrying a duplicate.
The base-blob path was the worst: a branch that introduces CHANGELOG.md hit a
bare `exit 0` on a message that was true about deletion and silent about the
duplicate in front of it. STRICT could not reach it — STRICT guards the two
skip() calls, and that is not one of them.
That inverted the two halves. Deletion needs a diff to see; duplication is the
one release-notes.sh actually mis-renders, re-arming its grab on the second
heading (#118). The half with the live extraction bug behind it had the most
ways to silently not run.
Moved, not rewritten. The skip messages now say containment skipped and that
uniqueness already passed. The CI step is no longer pull_request-only, with a
`github.ref_name` fallback because base_ref is empty on a push and a bare
`origin/` under STRICT would redden every push to main.
Found by claude-bot-andresmgsl reviewing heavy-duty/rig#99 and
heavy-duty/cast#134, which inherited the ordering from here.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The "Landed in all three repos together" line pointed at the sibling
tracking issues rather than the sibling PRs. The entry already opens with
its own issue ref, so a reader following "landed together" was sent to more
issues and never reached the actual sibling changes.
All three PRs carried it identically because the three entries came from one
generator that took sibling references from its issue-number map, and
expanded them into the sentence without re-wrapping — which is also why the
line ran to 108 columns in a file that wraps at 83.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A review landing was never a trigger for the labels workflow, so the exact
moment `state:needs-human` became true — the third bot approving — fired
nothing, and the label waited on the `*/15` cron. That cron does not run at
its declared rate: measured across box, rig and cast over a two-hour window
on 2026-07-20, one scheduled run each against the eight `*/15` implies.
The obvious fix does not work. There is no `pull_request_review_target`, and
on fork PRs — all of them here — `pull_request_review` runs with a read-only
token and cannot label anything.
So the handoff wakes the sweep itself:
- `pull_request_target` also fires on `labeled`/`unlabeled`
- the author sets `state:needs-human` at handoff, as the third act after the
round summary and the review request
The author's own label write fires the sweep that validates it — an
optimistic write, not a transfer of ownership. The reconciler confirms or
corrects it seconds later, and the cron falls back to a last resort. It
cannot loop: the reconciler writes with GITHUB_TOKEN, which does not create
workflow runs; agent writes use a PAT, which does.
`labels-reconcile.sh` is unchanged — it already recomputes every open PR
from scratch on every run, which is what makes the optimistic write safe.
The `scope` job is skipped on label events, where no path can have changed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The who-sets-what table is the day-to-day answer to "can I move this by
hand", and it never mentioned blocker:* -- a whole machine-owned family
added when state:needs-rebase was retired. merge-next was missing too, and
that is the one label whose ownership actually needs saying, because it is
the only one in the machine's vocabulary the machine deliberately does not
set.
Step 6 also read as though requesting the maintainer is sufficient to flip
state:needs-human. It is not: needs-human requires zero blockers, so the
request does nothing on a conflicted or red PR.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Round 2 review (claude-bot, codex-bot, grok-bot -- all three, independently).
The label pre-flight added in round 1 returned out of reconcile_pr entirely.
Everything below that point is independent of the state:* taxonomy: clearing a
stale merge-next, and the stale sweep. Stranding them meant a merge-next claim
reading "merge this one next" survived on a PR the board had moved to the
agent -- the same false invitation as #136, one scope smaller -- and the
staleness detector went silent. On a cold-start repo, where no state:* label
exists yet, that was EVERY PR.
It was also a regression against main rather than a missed improvement: the old
code failed the edit, logged, and fell through to both blocks. The round-1 fix
turned a per-edit failure into a per-PR abort.
Now skip_edit=true, and control reaches the rest of the function.
Also taken, both from claude-bot and grok-bot: the dead "$desired" term in the
filter loop, and `[ -n "$missing" ] && log` becoming a proper elif rather than
an &&-as-statement under set -e.
Four fixtures now drive reconcile_pr itself, stubbing run/gh -- the first in
this suite to reach past the pure functions, which is exactly why a per-PR
return was invisible to the fixtures that existed. Fixtures 68 -> 72.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Round 1 review (claude-bot, codex-bot on #138 and the sibling PRs).
gh issue edit rejects the WHOLE call on one unknown label name, and this
change made state and blockers share a single edit for anti-flicker. On a repo
whose taxonomy predates the blocker:* labels -- every repo, at merge -- the
first sweep would therefore converge NOTHING on exactly the PRs the change
exists to fix, surfacing only as a warning in a cron log. Adds are now
filtered against the repo's real label set, read once per sweep. Removals need
no filter: they are built from has_label, so they provably exist. An
unreadable label set does not filter, because a failed read must not silently
strip the board.
blocker:unrequested also missed the stale round. MISSING and STALE are both
verdicts this head does not have -- nobody reviewed it, or everybody reviewed
something else -- and the agent owes an ask either way. The stale one is the
worse of the two: it has approvals on the page that no longer describe the
tree, which is the case LABELS.md already called the most dangerous.
Fixtures 66 -> 68.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Its own Fixed entry rather than a line in the refactor above it: the two-axis
split is a design change, this is a correctness bug in a different function,
and a reader bisecting a false "merge me" wants the second one named.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gh returning nothing left the rollup KEY absent, which collapsed into the same
NONE as a PR that genuinely has no checks -- so an API hiccup presented as
mergeable-by-a-human. That is the unknown-certified-as-green shape this machine
exists to stop, surviving in the one place the #136 fix did not look.
checks_state now reports UNREADABLE for an absent key, and the sweep leaves
that PR exactly as it is rather than recomputing on facts it did not read.
Blocking on it instead would flap the whole board on one bad call.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Retires state:needs-rebase. Mergeability, check status and the review round
are independent facts; forcing them through one totally-ordered label meant
one always won and the rest vanished, which is where every precedence bug in
this machine has lived. needs-rebase was the clearest casualty -- it fired on
both a conflict and a failing check, which need opposite work.
Blockers are a set, so there is no ordering between them to get wrong. The
ordered axis is now purely about reviews. bots-reviewing also tightens to mean
a request is actually live; a PR nobody was asked to review is the agent's.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Third and final correction to the supersede rule, and the second one
that silently restored #136.
Round 3 replaced "date by completion" with "date by the newest stamp
the run carries". That is still not an ordering on runs: `max` over
[startedAt, createdAt, completedAt] resolves to completedAt for a
FINISHED run and startedAt for a LIVE one — different quantities, so
the comparison was never between like and like.
The consequence is the ordinary concurrency-group path, not an edge
case. A run cancelled by a concurrency group does not stop instantly;
it drains AFTER its replacement has already started, so
predecessor.completedAt > successor.startedAt is the normal shape. On
this PR's own aa5a6ba the window was 13 seconds. Inside it the dead
predecessor out-dated the live run replacing it, and a green
predecessor reported SUCCESS while a re-run was still in flight:
SUCCESS completing 15:30:13, replacement started 15:30:00
max -> SUCCESS (#136: needs-human over a disabled merge button)
first -> PENDING
Fixed with `max` -> `first`. The list is already in preference order,
so `first` IS "date it by when it began" — and a replacement always
begins after the run it replaces, whatever order they finish in. The
sentinel filtering and the undateable-sorts-last tiebreak are
unchanged; this narrows the rule to a quantity that actually orders.
Prescribed independently by claude-bot-andresmgsl and
codex-bot-andresmgsl.
Two fixtures pin the drain window in both colours. 49 -> 51.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The supersede fixtures pinned "an in-flight re-run is the newer word".
Nothing pinned the converse: a run that FINISHED after an earlier
in-flight entry is also the newer word, so the context is settled and
reads SUCCESS rather than staying PENDING.
That is the assertion that catches an over-correction — widening
"an undateable entry sorts last" into "an in-flight entry always wins".
Verified it fails under exactly that mutation and that nothing else in
the suite does.
Also corrects the comment above the undateable fixture: that case is
guarded by the sort tiebreak, not by the dating expression, so the two
changes are now separately pinned. Reverting only the dating expression
fails 2 fixtures (the two zero-sentinel ones), not 3.
48 -> 49. Reconciler unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
