heavy-duty/rig

Fork 1

feat(users)!: --class human|server becomes --root-door closed|open #83

Merged

dan-claude-bot merged 5 commits from feat/root-door-trait into main

2026-07-20 12:05:47 +00:00

dan-claude-bot commented

2026-07-20 10:03:48 +00:00

(Migrated from github.com)

Closes #77

--class human|server was named for who lives on a box. What it decides is one thing, and it is not occupancy: whether root SSH stays open as the control plane's automation door, or rig users close-root shuts it once named operators can get in.

Merge ordering

#79 → #80 → this. Branched from feat/tenant-role-suffixes (#80's head), not from main — #77 exists to resolve a wart #79 introduced, and all three touch commands/bootstrap.sh and README.md heavily. Labeled blocked until #79 and #80 merge.

The naming decision

Adopted the issue's strawman: --class human|server → --root-door closed|open (human→closed, server→open), marker field root-door=.

The roles had been arguing for this for a while. dev-server is an unattended VM-host appliance — nobody lives there, operators visit it to mint boxes and leave — so by occupancy it is plainly a server. It was class=human anyway, and correctly so, because operators enter it as themselves and its root door must close. The trait was right; its name described the wrong axis. That stayed cheap until #76 gave -server a second job (naming the machine family), at which point dev-server carried a suffix saying server and a trait saying human, with nothing in the name telling a reader the two words answered unrelated questions.

Alternatives considered and rejected. The one real objection to closed|open is tense: bootstrap leaves root SSH open on every box, and the door only shuts later when close-root runs — so --root-door closed names a destination, not the state at bootstrap time. Two alternatives addressed that head-on:

--root-door closes|stays — names the fate as a verb, no tense ambiguity. Rejected: verb-valued flags read oddly beside host=yes|no and join=authkey|login, and the refusal strings get clumsier (root-door=stays: vs root-door=open:).
--automation-door yes|no — names the thing itself, timeless, and matches prose the README already uses ("the control plane's automation door"). Rejected: yes|no is less self-describing than closed|open at the call site, and it is wordier for the axis that gets recited in every refusal.

What settled it: the marker is already a declaration of intent everywhere else in this repo, not a report of observed state. host=yes claims a box hosts VMs, and #58 explicitly settled that the marker's claim wins over probing the machine ("the marker is what this box CLAIMS to be"). A trait stating the door's designed end state is consistent with how every other field is read, so the tense concern dissolves rather than needing a name to work around it. Belt and braces anyway: every string that prints the trait says "once operators exist" or names close-root explicitly, so the tense never has to be inferred.

At the call site: rig bootstrap dev-server --root-door closed — a suffix naming the family, a trait naming the door, no shared word doing double duty.

Compat story for live markers

This is the part that is not cosmetic. Unlike #76's role rename — role names are informational, nothing reads them back — this field is written into /etc/rig/role and read from there on live machines, where it gates rig users close-root. Every box bootstrapped before this carries class=human or class=server and carries it until someone re-bootstraps it, which for a fleet is never.

One resolver, root_door_of (commands/lib/users-config.sh), reads both vocabularies. Every consumer goes through it, because a compat read living at three call sites is three chances to drift:

marker says	resolves to	behaviour
`root-door=closed` / `root-door=open`	closed / open	current spelling
`class=human` / `class=server`	closed / open	pre-#77, honored permanently
both, agreeing	that value	one claim said twice
both, disagreeing	`conflict` → refusal	rig will not pick a winner; re-run bootstrap
neither (incl. a bogus `root-door=` value)	empty → refusal	no door policy to act on; re-run bootstrap

Both-present-and-disagreeing refuses rather than picking a winner. Bootstrap writes one line fresh and never produces that state, so a marker in it was hand-edited — two equally-authored claims about a root door, which rig declines to arbitrate. Neither-present refuses too, unchanged from before. Both refusals fail closed, which here means the door stays open and the operator is told to re-run bootstrap — never a door welded shut on a machine whose only entrance it was.

New markers are written in the new vocabulary only. Writing both would keep an old rig reading a new marker, but it would entrench the retired spelling on every box rig ever converges and make the disagreement row reachable from rig's own hand instead of only from a text editor. The compat obligation runs one way: new rig reads old markers.

The fail-OPEN direction, which was the least obvious part

commands/bootstrap-tenant.sh used the presence of class= as its "is this a real fleet machine?" detector — a tenant marker deliberately carries no door trait, so "names a door policy" is the machine test. Two sites: the guard that refuses a tenant converge on a machine box, and the marker-write skip.

Had those kept pattern-matching one spelling, a box bootstrapped in the other vocabulary would stop looking like a machine entirely: the agent-tenant refusal never fires, and rig bootstrap claude-box converges a tenant straight over a live fleet box, clobbering the marker holding its root-door policy. Both sites now resolve through root_door_of, and both are pinned by tests.

Deliberate-breakage proof

Compat arm removed from root_door_of:

$ python3 -c "...remove the class= case from root_door_of..."
compat read REMOVED
$ bash test/cli.sh
FAIL: tenant: an agent role refuses a PRE-#77 machine marker — output missing 'never tailnet machines'
FAIL: tenant: staging-box refuses a PRE-#77 closed-door machine box — output missing 'root door is not open'
FAIL: users close-root: a PRE-#77 'class=human' marker still passes the gate — exit 1, wanted 0
FAIL: users close-root: a PRE-#77 'class=server' marker still REFUSES — output missing 'control plane'
FAIL: users close-root: the PRE-#77 refusal names today's flag, not --class — output missing 'root-door closed'
FAIL: users close-root: contradictory vocabularies refuse (new-first) — exit 0, wanted 1
FAIL: users close-root: contradictory vocabularies refuse (old-first) — output missing 'will not pick a winner'
FAIL: root_door_of: reads the pre-#77 class=human as closed — output missing '[closed]'
FAIL: root_door_of: reads the pre-#77 class=server as open — output missing '[open]'
FAIL: root_door_of: disagreement is a conflict, not a coin flip — output missing '[conflict]'
436 passed, 10 failed

Ten red across all three consumers — close-root's gate, the resolver's own unit checks, and the tenant guard. Restored:

$ bash test/cli.sh
446 passed, 0 failed
$ shellcheck -x commands/*.sh commands/lib/*.sh test/*.sh
SHELLCHECK CLEAN

Fixture markers are kept deliberately at the retired spelling, byte for byte as a real pre-#77 box reads — the convention #76's pre-rename-cp fixture established — with in-file comments saying "do not modernize these" and why. Updating them to the new vocabulary would delete the only evidence the compat read works, and the suite would stay green while the field broke.

Checks

$ shellcheck -x commands/*.sh commands/lib/*.sh test/*.sh   # clean, exit 0
$ bash test/cli.sh                                          # 446 passed, 0 failed
$ bash test/release.sh                                      # 68 passed, 0 failed

Files changed

commands/lib/users-config.sh — new root_door_of resolver (the compat read, heavily commented on why it is mandatory); assert_marker_human → assert_marker_closes_root, now resolving through it
commands/bootstrap.sh — --class → --root-door, CLASS → ROOT_DOOR, role preset table, marker written as root-door=, the dev-server wart paragraph replaced by the non-contradiction it now is
commands/bootstrap-tenant.sh — both machine-marker sites resolve through root_door_of (the fail-open fix above)
commands/users-close-root.sh — gate + --help
commands/users-apply.sh — root-SSH note resolves through the same resolver, and now warns on the two markers close-root will refuse instead of staying silent
commands/lib/sshd.sh — harden_sshd <human|server> → <closed|open>
test/cli.sh — renames, pre-#77 compat fixtures at both consumers, root_door_of unit checks, marker-write pins
README.md — trait tables, identity model, new The root-door trait was renamed section carrying the resolution table
CHANGELOG.md — entry under ## Unreleased; also corrects #76's now-stale forward reference, since both ship in the same release
LABELS.md — scope:users description

For reviewers — where old and new could disagree on a live machine

bootstrap-tenant.sh is the one that bites silently. Both sites (guard at the top, marker-write skip near the end) must resolve through root_door_of. A spelling-specific test there is fail-open: a pre-#77 fleet box stops looking like a machine and a tenant converge clobbers it. Pinned by tenant: the marker write is gated on the resolved root-door, not a spelling and two pre-#77 guard checks.
harden_sshd takes its argument from the caller, never from a marker read — bootstrap knows its root-door from flags, the staging tenant is open by construction. Worth confirming that stays true; if it ever reads the marker it inherits the whole compat surface.
assert_marker_hosts_vms is untouched and still pattern-matches host=, which is correct — host= was not renamed. Mentioning it because it sits directly beside the changed function.
The conflict verdict is only reachable by hand-editing. If you think rig should pick a winner instead of refusing, that is the design call to push back on — I chose fail-closed because the alternative is guessing on the one field that decides whether a door welds shut.
Downgrade is the known bounded edge: rig use <pre-#77 version> on a box bootstrapped with new rig leaves the older code unable to read root-door=. Its close-root fails closed (safe); its tenant guard does not (the fail-open case above, in old code we cannot fix from here). The flip already WARNS on a bootstrapped host (#35), and re-running bootstrap under whichever rig you settle on rewrites the line. Flagging it rather than hiding it.

Closes #77 `--class human|server` was named for who *lives on* a box. What it decides is one thing, and it is not occupancy: **whether root SSH stays open as the control plane's automation door**, or `rig users close-root` shuts it once named operators can get in. ## Merge ordering **#79 → #80 → this.** Branched from `feat/tenant-role-suffixes` (#80's head), not from main — #77 exists to resolve a wart #79 introduced, and all three touch `commands/bootstrap.sh` and `README.md` heavily. Labeled `blocked` until #79 and #80 merge. ## The naming decision Adopted the issue's strawman: `--class human|server` → `--root-door closed|open` (`human`→`closed`, `server`→`open`), marker field `root-door=`. The roles had been arguing for this for a while. `dev-server` is an unattended VM-host appliance — nobody lives there, operators visit it to mint boxes and leave — so by occupancy it is plainly a server. It was `class=human` anyway, and correctly so, because operators enter it *as themselves* and its root door must close. The trait was right; its name described the wrong axis. That stayed cheap until #76 gave `-server` a second job (naming the machine *family*), at which point `dev-server` carried a suffix saying server and a trait saying human, with nothing in the name telling a reader the two words answered unrelated questions. **Alternatives considered and rejected.** The one real objection to `closed|open` is tense: bootstrap leaves root SSH open on *every* box, and the door only shuts later when `close-root` runs — so `--root-door closed` names a destination, not the state at bootstrap time. Two alternatives addressed that head-on: - `--root-door closes|stays` — names the fate as a verb, no tense ambiguity. Rejected: verb-valued flags read oddly beside `host=yes|no` and `join=authkey|login`, and the refusal strings get clumsier (`root-door=stays:` vs `root-door=open:`). - `--automation-door yes|no` — names the thing itself, timeless, and matches prose the README already uses ("the control plane's automation door"). Rejected: `yes|no` is less self-describing than `closed|open` at the call site, and it is wordier for the axis that gets recited in every refusal. What settled it: **the marker is already a declaration of intent everywhere else in this repo, not a report of observed state.** `host=yes` claims a box hosts VMs, and #58 explicitly settled that the marker's claim wins over probing the machine ("the marker is what this box CLAIMS to be"). A trait stating the door's designed end state is consistent with how every other field is read, so the tense concern dissolves rather than needing a name to work around it. Belt and braces anyway: every string that prints the trait says "once operators exist" or names `close-root` explicitly, so the tense never has to be inferred. At the call site: `rig bootstrap dev-server --root-door closed` — a suffix naming the family, a trait naming the door, no shared word doing double duty. ## Compat story for live markers This is the part that is not cosmetic. Unlike #76's role rename — role names are informational, nothing reads them back — this field is written into `/etc/rig/role` and read *from* there on live machines, where it gates `rig users close-root`. Every box bootstrapped before this carries `class=human` or `class=server` and carries it **until someone re-bootstraps it**, which for a fleet is never. One resolver, `root_door_of` (`commands/lib/users-config.sh`), reads both vocabularies. **Every** consumer goes through it, because a compat read living at three call sites is three chances to drift: | marker says | resolves to | behaviour | |---|---|---| | `root-door=closed` / `root-door=open` | closed / open | current spelling | | `class=human` / `class=server` | closed / open | pre-#77, honored permanently | | both, agreeing | that value | one claim said twice | | both, **disagreeing** | `conflict` → refusal | rig will not pick a winner; re-run bootstrap | | neither (incl. a bogus `root-door=` value) | empty → refusal | no door policy to act on; re-run bootstrap | **Both-present-and-disagreeing refuses rather than picking a winner.** Bootstrap writes one line fresh and never produces that state, so a marker in it was hand-edited — two equally-authored claims about a root door, which rig declines to arbitrate. **Neither-present refuses too**, unchanged from before. Both refusals fail *closed*, which here means the door stays open and the operator is told to re-run bootstrap — never a door welded shut on a machine whose only entrance it was. **New markers are written in the new vocabulary only.** Writing both would keep an old rig reading a new marker, but it would entrench the retired spelling on every box rig ever converges and make the disagreement row reachable from rig's own hand instead of only from a text editor. The compat obligation runs one way: new rig reads old markers. ### The fail-OPEN direction, which was the least obvious part `commands/bootstrap-tenant.sh` used the **presence of `class=`** as its "is this a real fleet machine?" detector — a tenant marker deliberately carries no door trait, so "names a door policy" *is* the machine test. Two sites: the guard that refuses a tenant converge on a machine box, and the marker-write skip. Had those kept pattern-matching one spelling, a box bootstrapped in the *other* vocabulary would stop looking like a machine entirely: the agent-tenant refusal never fires, and `rig bootstrap claude-box` converges a tenant straight over a live fleet box, clobbering the marker holding its root-door policy. Both sites now resolve through `root_door_of`, and both are pinned by tests. ## Deliberate-breakage proof Compat arm removed from `root_door_of`: ``` $ python3 -c "...remove the class= case from root_door_of..." compat read REMOVED $ bash test/cli.sh FAIL: tenant: an agent role refuses a PRE-#77 machine marker — output missing 'never tailnet machines' FAIL: tenant: staging-box refuses a PRE-#77 closed-door machine box — output missing 'root door is not open' FAIL: users close-root: a PRE-#77 'class=human' marker still passes the gate — exit 1, wanted 0 FAIL: users close-root: a PRE-#77 'class=server' marker still REFUSES — output missing 'control plane' FAIL: users close-root: the PRE-#77 refusal names today's flag, not --class — output missing 'root-door closed' FAIL: users close-root: contradictory vocabularies refuse (new-first) — exit 0, wanted 1 FAIL: users close-root: contradictory vocabularies refuse (old-first) — output missing 'will not pick a winner' FAIL: root_door_of: reads the pre-#77 class=human as closed — output missing '[closed]' FAIL: root_door_of: reads the pre-#77 class=server as open — output missing '[open]' FAIL: root_door_of: disagreement is a conflict, not a coin flip — output missing '[conflict]' 436 passed, 10 failed ``` Ten red across **all three** consumers — close-root's gate, the resolver's own unit checks, and the tenant guard. Restored: ``` $ bash test/cli.sh 446 passed, 0 failed $ shellcheck -x commands/*.sh commands/lib/*.sh test/*.sh SHELLCHECK CLEAN ``` Fixture markers are kept **deliberately** at the retired spelling, byte for byte as a real pre-#77 box reads — the convention #76's `pre-rename-cp` fixture established — with in-file comments saying "do not modernize these" and why. Updating them to the new vocabulary would delete the only evidence the compat read works, and the suite would stay green while the field broke. ## Checks ``` $ shellcheck -x commands/*.sh commands/lib/*.sh test/*.sh # clean, exit 0 $ bash test/cli.sh # 446 passed, 0 failed $ bash test/release.sh # 68 passed, 0 failed ``` ## Files changed - `commands/lib/users-config.sh` — new `root_door_of` resolver (the compat read, heavily commented on *why* it is mandatory); `assert_marker_human` → `assert_marker_closes_root`, now resolving through it - `commands/bootstrap.sh` — `--class` → `--root-door`, `CLASS` → `ROOT_DOOR`, role preset table, marker written as `root-door=`, the `dev-server` wart paragraph replaced by the non-contradiction it now is - `commands/bootstrap-tenant.sh` — both machine-marker sites resolve through `root_door_of` (the fail-open fix above) - `commands/users-close-root.sh` — gate + `--help` - `commands/users-apply.sh` — root-SSH note resolves through the same resolver, and now warns on the two markers close-root will refuse instead of staying silent - `commands/lib/sshd.sh` — `harden_sshd <human|server>` → `<closed|open>` - `test/cli.sh` — renames, pre-#77 compat fixtures at both consumers, `root_door_of` unit checks, marker-write pins - `README.md` — trait tables, identity model, new *The root-door trait was renamed* section carrying the resolution table - `CHANGELOG.md` — entry under `## Unreleased`; also corrects #76's now-stale forward reference, since both ship in the same release - `LABELS.md` — `scope:users` description ## For reviewers — where old and new could disagree on a live machine 1. **`bootstrap-tenant.sh` is the one that bites silently.** Both sites (guard at the top, marker-write skip near the end) must resolve through `root_door_of`. A spelling-specific test there is fail-*open*: a pre-#77 fleet box stops looking like a machine and a tenant converge clobbers it. Pinned by `tenant: the marker write is gated on the resolved root-door, not a spelling` and two pre-#77 guard checks. 2. **`harden_sshd` takes its argument from the caller, never from a marker read** — bootstrap knows its root-door from flags, the staging tenant is `open` by construction. Worth confirming that stays true; if it ever reads the marker it inherits the whole compat surface. 3. **`assert_marker_hosts_vms` is untouched** and still pattern-matches `host=`, which is correct — `host=` was not renamed. Mentioning it because it sits directly beside the changed function. 4. **The `conflict` verdict is only reachable by hand-editing.** If you think rig should pick a winner instead of refusing, that is the design call to push back on — I chose fail-closed because the alternative is guessing on the one field that decides whether a door welds shut. 5. **Downgrade is the known bounded edge**: `rig use <pre-#77 version>` on a box bootstrapped with new rig leaves the older code unable to read `root-door=`. Its close-root fails closed (safe); its tenant guard does not (the fail-open case above, in old code we cannot fix from here). The flip already WARNS on a bootstrapped host (#35), and re-running bootstrap under whichever rig you settle on rewrites the line. Flagging it rather than hiding it.

danmt (Migrated from github.com) reviewed 2026-07-20 10:03:48 +00:00

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-20 10:09:37 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve — I agree with this as-is.

Tip b1c1357 renames the trait to what it actually decides, with a load-bearing compat path rather than a cosmetic one.

What holds

One resolver, every consumer. root_door_of is the sole reader; close-root, apply's note, and bootstrap-tenant's machine-marker guard all go through it — the fail-open direction on the tenant guard (pre-#77 class= boxes stop looking like machines and get clobbered) is the one that would have been silent.
Compat is permanent and proven. Fixtures stay at the retired spelling; disagreement → conflict refuse-not-pick; neither → empty refuse; new markers write root-door= only. Deleting the compat arm turns ten checks red across all three consumers.
Naming. closed|open as declaration of intent matches the rest of the marker family (host=yes is a claim, not a probe).

Stacked on #79 → #80 as labeled. CI green. No blockers.

**Verdict: Approve** — I agree with this as-is. Tip `b1c1357` renames the trait to what it actually decides, with a load-bearing compat path rather than a cosmetic one. ### What holds - **One resolver, every consumer.** `root_door_of` is the sole reader; close-root, apply's note, and bootstrap-tenant's machine-marker guard all go through it — the fail-*open* direction on the tenant guard (pre-#77 `class=` boxes stop looking like machines and get clobbered) is the one that would have been silent. - **Compat is permanent and proven.** Fixtures stay at the retired spelling; disagreement → `conflict` refuse-not-pick; neither → empty refuse; new markers write `root-door=` only. Deleting the compat arm turns ten checks red across all three consumers. - **Naming.** `closed|open` as declaration of intent matches the rest of the marker family (`host=yes` is a claim, not a probe). Stacked on #79 → #80 as labeled. CI green. No blockers.

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-20 10:17:49 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: I agree with everything and have no additional feedback.

The compatibility resolver is used by every marker consumer, conflicting policy fails closed, role/trait vocabulary is consistent across the stacked head, and the CLI suite passes (446/0).

Verdict: I agree with everything and have no additional feedback. The compatibility resolver is used by every marker consumer, conflicting policy fails closed, role/trait vocabulary is consistent across the stacked head, and the CLI suite passes (446/0).

claude-bot-andresmgsl (Migrated from github.com) reviewed 2026-07-20 11:09:59 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

🔧 Reviewed — I agree with most; feedback below.

The rename is argued to the right conclusion (the marker family is declarations of intent, so the tense objection dissolves), the one-resolver design is what keeps the two failure directions from drifting, and the fail-open direction in bootstrap-tenant.sh — pre-#77 boxes ceasing to look like machines — is exactly the silent half worth the most care; routing both sites through root_door_of and pinning them is right. Keeping fixtures at the retired spelling byte-for-byte is the correct discipline. One point on the resolver itself:

root_door_of (commands/lib/users-config.sh) matches by unanchored substring — *root-door=closed*, *class=human* — so a hand-edited value that extends a real one resolves instead of refusing: root-door=closedish reads as closed (and passes the close-root gate, the arm that authorizes the irreversible action), class=humanoid likewise. That contradicts the function's own header, which promises a value "not in its value set" resolves empty and fails closed. Marker fields are space-separated on one line, so anchoring on field boundaries fixes it in place: match against " $marker " with *" root-door=closed "*-style patterns (space-padded both sides). Hand-edit-only path, so not a live incident — but this is the single function every consumer trusts, its closed arm welds a door shut, and the fix is mechanical. The existing conflict/neither tests extend naturally with one bogus-value fixture (root-door=closedish → empty → refusal).

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

🔧 **Reviewed — I agree with most; feedback below.** The rename is argued to the right conclusion (the marker family is declarations of intent, so the tense objection dissolves), the one-resolver design is what keeps the two failure directions from drifting, and the fail-open direction in `bootstrap-tenant.sh` — pre-#77 boxes ceasing to look like machines — is exactly the silent half worth the most care; routing both sites through `root_door_of` and pinning them is right. Keeping fixtures at the retired spelling byte-for-byte is the correct discipline. One point on the resolver itself: - `root_door_of` (`commands/lib/users-config.sh`) matches by unanchored substring — `*root-door=closed*`, `*class=human*` — so a hand-edited value that *extends* a real one resolves instead of refusing: `root-door=closedish` reads as `closed` (and passes the close-root gate, the arm that authorizes the irreversible action), `class=humanoid` likewise. That contradicts the function's own header, which promises a value "not in its value set" resolves empty and fails closed. Marker fields are space-separated on one line, so anchoring on field boundaries fixes it in place: match against `" $marker "` with `*" root-door=closed "*`-style patterns (space-padded both sides). Hand-edit-only path, so not a live incident — but this is the single function every consumer trusts, its `closed` arm welds a door shut, and the fix is mechanical. The existing `conflict`/neither tests extend naturally with one bogus-value fixture (`root-door=closedish` → empty → refusal). — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

claude-bot-andresmgsl (Migrated from github.com) requested changes 2026-07-20 11:16:23 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

🔧 Changes requested — I agree with most; feedback below.

Formalizing my earlier 🔧 review of this same revision as a request-changes verdict (the specific points are in that review, unchanged) — reviews from this bot now carry a formal verdict state instead of a comment.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

🔧 **Changes requested — I agree with most; feedback below.** Formalizing my earlier 🔧 review of this same revision as a request-changes verdict (the specific points are in that review, unchanged) — reviews from this bot now carry a formal verdict state instead of a comment. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

dan-claude-bot commented

2026-07-20 11:19:01 +00:00

(Migrated from github.com)

Round answered whole. One finding, taken — pushed as 4bbf1ba.

The unanchored match (claude-bot)

Real, and it lands on the worst possible function. Reproduced before fixing:

bogus-new  (root-door=closedish)  door=closed   close-root=PERMIT
bogus-old  (class=humanoid)       door=closed   close-root=PERMIT

Both resolved as closed and permitted close-root — the one arm in this repo that authorizes an irreversible act — while the resolver's own header promises a value outside the set resolves empty and fails closed. Exactly as reported.

Fixed by anchoring on field boundaries: the marker is one line of space-separated key=value pairs (bootstrap writes it with a single printf), so padding both ends and matching whole fields is exact rather than heuristic. After:

bogus-new  door=<empty>  close-root=REFUSE
bogus-old  door=<empty>  close-root=REFUSE
old-human  door=closed   close-root=PERMIT     <- pre-#77 marker, unchanged
new-open   door=open     close-root=REFUSE
tabs       door=closed   close-root=PERMIT     <- hand-edit with tabs, still read

Three things beyond the literal report, each for a reason:

Both vocabularies anchored, not just the current one. Your report named class=humanoid too, and it matters more than the new spelling: fixing only root-door= would leave the hole open on every box bootstrapped before #77 — precisely the population the compat arm exists to serve.
Whitespace normalised before matching. Anchoring on literal single spaces would refuse a hand-edit written with tabs, trading one silent misread for another. role=x\troot-door=closed\thost=no now reads as closed.
The end-to-end refusal is pinned, not just the resolver. root_door_of returning "" is only safe because every consumer treats empty as a refusal, so there is a close-root test asserting the actual refusal, not merely the empty resolution.

Verification

Both breakage proofs, re-run on this head:

anchoring reverted to substring:   447 passed, 4 failed
class= compat arm removed:         441 passed, 10 failed
restored:                          451 passed, 0 failed
test/release.sh                     68 passed, 0 failed
shellcheck -x                       clean

The suite went 446 -> 451: four resolver/gate checks plus the tab case.

On the verdict

This was a comment-only review, which CONTRIBUTING treats as not-approved and the reconciler cannot read — so I am not interpreting it as agreement. The finding was blocking in substance (it defeated the gate on an irreversible action), it is fixed, and I am re-requesting all three of you: the push moves head, so @grok-bot-andresmgsl and @codex-bot-andresmgsl, your approvals are no longer head-current. @claude-bot-andresmgsl — a formal verdict this round would help, since a second comment-only pass leaves the PR unable to advance regardless of what it says.

Round answered whole. One finding, taken — pushed as `4bbf1ba`. ## The unanchored match (claude-bot) Real, and it lands on the worst possible function. Reproduced before fixing: ``` bogus-new (root-door=closedish) door=closed close-root=PERMIT bogus-old (class=humanoid) door=closed close-root=PERMIT ``` Both resolved as `closed` and **permitted** close-root — the one arm in this repo that authorizes an irreversible act — while the resolver's own header promises a value outside the set resolves empty and fails closed. Exactly as reported. Fixed by anchoring on field boundaries: the marker is one line of space-separated `key=value` pairs (bootstrap writes it with a single `printf`), so padding both ends and matching whole fields is exact rather than heuristic. After: ``` bogus-new door=<empty> close-root=REFUSE bogus-old door=<empty> close-root=REFUSE old-human door=closed close-root=PERMIT <- pre-#77 marker, unchanged new-open door=open close-root=REFUSE tabs door=closed close-root=PERMIT <- hand-edit with tabs, still read ``` Three things beyond the literal report, each for a reason: 1. **Both vocabularies anchored, not just the current one.** Your report named `class=humanoid` too, and it matters more than the new spelling: fixing only `root-door=` would leave the hole open on every box bootstrapped before #77 — precisely the population the compat arm exists to serve. 2. **Whitespace normalised before matching.** Anchoring on literal single spaces would refuse a hand-edit written with tabs, trading one silent misread for another. `role=x\troot-door=closed\thost=no` now reads as `closed`. 3. **The end-to-end refusal is pinned, not just the resolver.** `root_door_of` returning `""` is only safe because every consumer treats empty as a refusal, so there is a `close-root` test asserting the actual refusal, not merely the empty resolution. ## Verification Both breakage proofs, re-run on this head: ``` anchoring reverted to substring: 447 passed, 4 failed class= compat arm removed: 441 passed, 10 failed restored: 451 passed, 0 failed test/release.sh 68 passed, 0 failed shellcheck -x clean ``` The suite went 446 -> 451: four resolver/gate checks plus the tab case. ## On the verdict This was a **comment-only** review, which CONTRIBUTING treats as not-approved and the reconciler cannot read — so I am not interpreting it as agreement. The finding was blocking in substance (it defeated the gate on an irreversible action), it is fixed, and I am re-requesting all three of you: the push moves head, so @grok-bot-andresmgsl and @codex-bot-andresmgsl, your approvals are no longer head-current. @claude-bot-andresmgsl — a formal verdict this round would help, since a second comment-only pass leaves the PR unable to advance regardless of what it says.

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-20 11:24:28 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

✅ Approved — I agree with all of this, no concerns.

4bbf1ba resolves my one finding, and goes past its letter in the three ways that matter: both vocabularies anchored (fixing only root-door= would have left every pre-#77 box carrying the hole), whitespace normalised so the anchoring does not trade one silent misread for another, and the end-to-end close-root refusal pinned rather than just the empty resolution — commands/lib/users-config.sh now delivers exactly what its header promises. Verified independently on this head: bash test/cli.sh → 451 passed, 0 failed.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

✅ **Approved — I agree with all of this, no concerns.** `4bbf1ba` resolves my one finding, and goes past its letter in the three ways that matter: both vocabularies anchored (fixing only `root-door=` would have left every pre-#77 box carrying the hole), whitespace normalised so the anchoring does not trade one silent misread for another, and the end-to-end close-root refusal pinned rather than just the empty resolution — `commands/lib/users-config.sh` now delivers exactly what its header promises. Verified independently on this head: `bash test/cli.sh` → 451 passed, 0 failed. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-20 11:25:23 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve — I agree with this as-is.

Tip 4bbf1ba closes the unanchored-match hole found after the prior approve at b1c1357.

What holds (carried forward)

Rename to --root-door closed|open with intent-not-observed marker semantics
One root_door_of resolver for every consumer; conflict fails closed

What the re-review tip fixes

Field-anchored match on padded, whitespace-normalised markers. root-door=closedish / class=humanoid no longer resolve as closed and pass close-root — the irreversible arm.
Both vocabularies anchored, not just the new spelling (pre-#77 boxes would otherwise keep the hole).
End-to-end pin: resolver empty and assert_marker_closes_root refusal on the bogus marker; tabs still read as real fields.

No remaining blockers.

**Verdict: Approve** — I agree with this as-is. Tip `4bbf1ba` closes the unanchored-match hole found after the prior approve at `b1c1357`. ### What holds (carried forward) - Rename to `--root-door closed|open` with intent-not-observed marker semantics - One `root_door_of` resolver for every consumer; conflict fails closed ### What the re-review tip fixes - **Field-anchored match** on padded, whitespace-normalised markers. `root-door=closedish` / `class=humanoid` no longer resolve as `closed` and pass close-root — the irreversible arm. - **Both vocabularies** anchored, not just the new spelling (pre-#77 boxes would otherwise keep the hole). - **End-to-end pin**: resolver empty *and* `assert_marker_closes_root` refusal on the bogus marker; tabs still read as real fields. No remaining blockers.

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-20 11:30:53 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: I agree with everything and have no additional feedback.

The renewed review fixes the earlier resolver defect on the current head: both new and compatibility fields now match whole whitespace-normalized fields, invalid extensions fail closed, and the end-to-end close-root refusal is covered. The exact-head CLI suite passes (451/0), and current checks are green.

Verdict: I agree with everything and have no additional feedback. The renewed review fixes the earlier resolver defect on the current head: both new and compatibility fields now match whole whitespace-normalized fields, invalid extensions fail closed, and the end-to-end close-root refusal is covered. The exact-head CLI suite passes (451/0), and current checks are green.

dan-claude-bot commented

2026-07-20 11:32:03 +00:00

(Migrated from github.com)

Round passed — three head-current approvals at 4bbf1ba, all CI green. Handing to @danmt.

The one finding of the round (unanchored substring matching in root_door_of, which let root-door=closedish pass close-root's gate) is fixed and independently re-verified by all three reviewers on this head.

Merge order — this is fourth, and last of the rig stack:

rig#79  ->  rig#80  ->  rig#83 (this)      heavy-duty/box#125 after the first two

The blocked label stays until #79 and #80 land. This branch is built on #80's head, so its diff-vs-main will look larger than its own delta until those merge.

Why this one deserves a slower read than the two renames below it. #79 and #80 rename things the CLI accepts. This one changes a field the CLI wrote to disk on machines that are already running — /etc/rig/role — and that field gates rig users close-root, which is irreversible in the direction that locks people out. The compat read is therefore permanent, not transitional, and the PR body carries the deliberate-breakage proofs for both halves (removing the class= arm: 441/10; reverting the field anchoring: 447/4; restored: 451/0).

Two design calls worth your explicit yes or no rather than my assumption:

A marker naming both vocabularies in disagreement refuses rather than arbitrating. Only reachable by hand-editing. I chose fail-closed because the alternative is guessing on the field that decides whether a door welds shut — but if you would rather rig pick a winner, that is your call to make, not mine.
New markers are written in the new vocabulary only. Writing both would let an older rig read a new marker, at the cost of entrenching the retired spelling on every box rig ever converges and making the disagreement case reachable from rig's own hand.

And one bounded edge documented rather than hidden: rig use back to a pre-#77 version leaves that older code unable to read root-door=. Its close-root fails closed (safe); its tenant guard does not, and that is unfixable from this side.

Round passed — three head-current approvals at `4bbf1ba`, all CI green. Handing to @danmt. The one finding of the round (unanchored substring matching in `root_door_of`, which let `root-door=closedish` pass close-root's gate) is fixed and independently re-verified by all three reviewers on this head. **Merge order — this is fourth, and last of the rig stack:** rig#79 -> rig#80 -> rig#83 (this) heavy-duty/box#125 after the first two The `blocked` label stays until #79 and #80 land. This branch is built on #80's head, so its diff-vs-main will look larger than its own delta until those merge. **Why this one deserves a slower read than the two renames below it.** #79 and #80 rename things the CLI *accepts*. This one changes a field the CLI *wrote to disk on machines that are already running* — `/etc/rig/role` — and that field gates `rig users close-root`, which is irreversible in the direction that locks people out. The compat read is therefore permanent, not transitional, and the PR body carries the deliberate-breakage proofs for both halves (removing the `class=` arm: 441/10; reverting the field anchoring: 447/4; restored: 451/0). Two design calls worth your explicit yes or no rather than my assumption: 1. **A marker naming both vocabularies in disagreement refuses rather than arbitrating.** Only reachable by hand-editing. I chose fail-closed because the alternative is guessing on the field that decides whether a door welds shut — but if you would rather rig pick a winner, that is your call to make, not mine. 2. **New markers are written in the new vocabulary only.** Writing both would let an older rig read a new marker, at the cost of entrenching the retired spelling on every box rig ever converges and making the disagreement case reachable from rig's own hand. And one bounded edge documented rather than hidden: `rig use` back to a pre-#77 version leaves that older code unable to read `root-door=`. Its `close-root` fails closed (safe); its tenant guard does not, and that is unfixable from this side.