feat(bootstrap)!: box tenant roles carry a -box suffix
#80
Labels
No labels
attention
blocked
blocker:ci-red
blocker:conflict
blocker:drill-pending
blocker:unrequested
bug
claimed
documentation
enhancement
epic
merge-next
needs-ruling
needs-triage
offsite
post-merge
ready
release
scope:bootstrap
scope:coolify
scope:db
scope:docs
scope:drill
scope:installer
scope:labels
scope:platform
scope:runner
scope:users
stale
state:addressing
state:bots-reviewing
state:building
state:needs-human
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: heavy-duty/rig#80
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/tenant-role-suffixes"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Tenant half of #76. Stacked on #79 (machine roles) — review that first; this branch contains its commits, and the diff GitHub shows will be against
mainuntil #79 merges.What
claude→claude-box,codex→codex-box,grok→grok-box,staging→staging-box. With both halves in, a role name always says which family it belongs to —-serverbuilds a fleet machine,-boxconverges a guest a box minted — and the two can no longer collide on a word the waystagingdid.The role carries the suffix; nothing inside the guest does
Worth being explicit, because it is the one place this could have gone wrong: a tenant user is the account the box seed created (
BOX_USER), and each agent CLI reads its own dotdir.claude-boxstill converges theclaudeuser and still writes~/.claude/CLAUDE.md.Every rename in
bootstrap-tenant.shis a$ROLEcomparison or acasearm — no CLI binary name (claude,codex,grokare all still invoked bare), no dotdir path, and no account moved. The diff is worth skimming for exactly this: the file mentions those three words constantly, and only about twenty of the occurrences are roles.READMEs tenant table now shows role and user in adjacent columns, since that distinction stopped being cosmetic the moment they differed.
Hard cut
No aliases. Old names are refused as unknown at both entrypoints —
rig bootstrap <name>andbootstrap-tenant.shdirectly — and the suite asserts all four at each. Both, becausebootstrap.shkeeps its own dispatch list and a name could survive in one and not the other. An alias left in for a single tenant is the shape that survives review: the taxonomy reads complete while one old name still quietly converges.Cross-repo ordering
A seed carrying
BOX_BOOTSTRAP_ROLE="claude"now fails its own mint-time bootstrap. heavy-duty/box#125 updates the seeds and is labeledblockeduntil this lands. Merge order: #79 → this → box#125.Tests
420 passing (
test/cli.sh), 68 passing (test/release.sh),shellcheck -xclean.Verdict: Approve — I agree with this as-is (stacked on #79).
Why
claude|codex|grok|staging→*-box. Role name carries the family; guest user / agent dotdirs stay unsuffixed (claudeuser,~/.claude/…) — the dangerous half of the rename is correctly left alone.tenant-config/ bootstrap-tenant cases updated.No blockers.
Verdict: I have feedback.
Blocking: the tenant rename updates most workload guidance, but commands/bootstrap-tenant.sh still emits sudo rig bootstrap workload at the end of a staging-box convergence (line 390), and its help text repeats the old command across lines 47–48. Because this stack hard-cuts the bare role, the documented/runtime next step fails. Change these to workload-server and add an assertion covering the emitted staging-box next-step.
Verdict: request changes — the rename itself is correct and complete on the role surface, but the file this PR owns still points operators at a command the stack just hard-cut.
Reviewed as stacked: merge-base of this branch is exactly #79's head (
250daa9), so only this PR's own delta was reviewed (7 files, +199/−151) at head436756b.Blocking
commands/bootstrap-tenant.sh:390— the emitted next-step is a dead command. After astaging-boxconverge, the script logsnext (operator-run, holds a credential): box shell → sudo rig bootstrap workload --hostname <name> …. #79 (contained in this branch) hard-cutworkload, so the runtime guidance now exits 2 as an unknown role. The same stale spelling survives in the usage text (commands/bootstrap-tenant.sh:47-48:`sudo rig bootstrap workload`) and the header comment (commands/bootstrap-tenant.sh:14). This is squarely in this PR's court: #79 never touchedbootstrap-tenant.sh, and this PR updated the other occurrences in the same file toworkload-server(commands/bootstrap-tenant.sh:83,86) while editing the very paragraph line 48 sits in. Fix all three, and add a suite assertion pinning the emitted next-step string — the harness pins refusal messages per name, but nothing pins this log line, which is exactly how it regressed silently. (This confirms codex-bot's finding.)Non-blocking, same push
README.md:1044— the rehearsal sentence was half-renamed: line 1043 now says`rig bootstrap claude-box`but the continuation on 1044 still reads/ `staging`. Should bestaging-box.CHANGELOG.md:31citesheavy-duty/box#123for the seed update while the PR body cites box#125. box#123 is the tracking issue and #125 the PR that closes it, so the reference resolves — but citing the PR (or both) would save the future reader a hop.Verified good
BOX_BOOTSTRAP_ROLEto exactlyclaude-box/codex-box/grok-box/staging-box— same set, same spelling as this PR'scasearms incommands/bootstrap-tenant.sh:60-63and dispatch incommands/bootstrap.sh:110-116.blankcorrectly stays unsuffixed (noBOX_BOOTSTRAP_ROLE).BOX_BOOTSTRAP_ROLE="claude"and fails mint-time bootstrap against rig@main (seeds install rig unpinned). Both PRs state this openly; it is unavoidable without aliases, but box#125 should follow promptly.commands/bootstrap-tenant.sh:113-127) matches onlyhost=yes/class=— an old-name tenant marker (role=claude tenant=yes host=no) passes and gets rewrittenrole=claude-boxon re-converge.users-apply.sh:158,257and both coolify installers likewise gate onclass=/host=/control-plane-server, never a tenant role word. Tenants never join the tailnet, so no tag exposure.commands/lib/tenant-config.shmapsclaude-box→userclaude, dotdirs (~/.claude/CLAUDE.mdetc.) and CLI binary names unchanged; every rename inbootstrap-tenant.shis a$ROLEcomparison orcasearm — I checked each hunk for exactly this.test/cli.sh:661-666loops all four old names through bothbootstrap-tenant.sh(unknown tenant role) andbootstrap.sh(unknown role).test/cli.sh420 passed / 0 failed,test/release.sh68 passed / 0 failed,shellcheck -xclean on the touched scripts.Fix the
workload→workload-serverleftovers (with the pinning assertion) and this is ready.— automated review by
claude-bot-andresmgsl· heavy-duty-review-botRound answered whole — all three findings taken. Rebased onto #79's new head; this branch is now
94d9628.Blocking — the dead next-step (codex-bot, claude-bot)
Confirmed and fixed, but moved down the stack to #79 rather than fixed here, and that is worth explaining since claude-bot explicitly reasoned it was "squarely in this PR's court" on the grounds that #79 never touched
bootstrap-tenant.sh.That is true as a statement about the diffs, and it is the wrong place to fix it. The breakage is introduced by #79 — that is the commit that deletes the
workloadrole. Fixing it here would mean #79 is mergeable while shipping a runtime next-step naming a role it just removed, and this repo merges the stack in order. Sobootstrap-tenant.shnow gets itsworkload-servercorrection in #79, and this PR carries it via rebase. Same fix, one PR earlier, no window where main is wrong.The sweep assertion you both asked for also lives in #79 for the same reason. It is repo-wide rather than pinned to the four known sites — a stale next-step is copy-pasted onto a different box minutes after the run reported success, so the class matters more than the instances. Verified non-vacuous over there.
Non-blocking 1 —
README.md:1044half-renamed (claude-bot)Real miss, thank you — the rehearsal sentence said
`rig bootstrap claude-box` / `staging`. The continuation line was a separate markdown line so my pass over the tenant names never saw it. Nowstaging-box.Non-blocking 2 — box#123 vs box#125 (claude-bot)
Fair. Now cites
heavy-duty/box#125 (closing heavy-duty/box#123), so the reader gets the PR that actually lands the seeds without losing the tracking issue.Also applied
feat(bootstrap)!:on the subject and title, matching what claude-bot asked for on #79 — this entry says BREAKING too, so the marker belongs on both.Verification
Also confirming claude-bot's cross-repo check independently: box#125's seeds set
BOX_BOOTSTRAP_ROLEto exactly the four names this PR'scasearms accept,blankstays unsuffixed, and box#125 additionally now carries theworkload-servercorrection for the join hint it prints (three sites there —cmd_new, the seed comment, the README).Re-requesting @codex-bot-andresmgsl and @claude-bot-andresmgsl; @grok-bot-andresmgsl too, since the push moved head out from under your approval.
Verdict: I agree with everything and have no additional feedback.
Verdict: approve — the blocking point from my previous review is resolved, and the resolution is better-placed than the fix I asked for.
Re-reviewed from scratch at head
94d9628. The prior block wascommands/bootstrap-tenant.sh:390(plus lines 14 and 47-48) still directing operators tosudo rig bootstrap workload, a command this stack hard-cut.The blocking point, resolved
workload-server: the header comment (commands/bootstrap-tenant.sh:14), the usage text (commands/bootstrap-tenant.sh:47-48), and the emitted staging-box next-step (commands/bootstrap-tenant.sh:390).workload, so fixing it there closes the window where the stack's first PR is mergeable while shipping a dead runtime next-step. Same fix, no wrong-on-main interval.test/cli.sh:106-108): grepsbin/rigandcommands/forrig bootstrap (control-plane|workload|runner|dev)as a bare word. That is strictly stronger than pinning line 390 — the next instance of this class will be at a different site. I verified it is non-vacuous: re-injecting the oldworkloadspelling at line 390 makes the sweep grep hit (and the check expects exit 1, so the suite would fail).Full-tree verification at this head
grep -rn "bootstrap workload"over the tree: every shipped-surface hit isworkload-server. The only barebootstrap workloadsurvivors are indocs/plans/2026-07-17-staging-role.mdanddocs/plans/2026-07-11-runner-install.md— dated historical design records, not operator guidance, and deliberately outside the sweep's scope. No shipped file tells an operator to run any bare pre-#76 name, tenant names included (rig bootstrap (claude|codex|grok|staging)without a suffix: zero hits inbin/,commands/,README.md,CHANGELOG.md).!marker is backed:CHANGELOG.md:11-32carries a BREAKING entry for the tenant rename with migration guidance — hard cut, no aliases, old names refused as unknown at both entrypoints, and the cross-repo consequence (BOX_BOOTSTRAP_ROLE="claude"fails mint-time bootstrap; box#125 must land after). The citation is nowheavy-duty/box#125 (closing heavy-duty/box#123)(CHANGELOG.md:30-31), resolving my non-blocking point 3.README.md:1044now readsstaging-box, resolving my non-blocking point 2.BOX_BOOTSTRAP_ROLEto exactlyclaude-box/codex-box/grok-box/staging-box— same set, same spelling as thecasearms this PR ships —blankstays without a role, and box#125 also carries the matchingworkload-servercorrection for the join hint box itself prints (cmd_new, the staging seed comment, its README), with its own assertion that the join is printed, never exec'd.Delta since my last review (
436756b..94d9628)Benign and exactly scoped to the round: the three
workload-servercorrections incommands/bootstrap-tenant.sh, the README half-rename, the CHANGELOG citation plus a new paragraph documenting the operator-string sweep, and test additions — the sweep, the per-name tenant hard-cut loop at both entrypoints (test/cli.sh:483-488), and new pre-#76 marker migration assertions pinning that an oldrole=control-planemarker warns but is never refused (test/cli.sh:651-678), which turns the CHANGELOG's compat promise from documented into tested.Tests
bash test/cli.sh: 424 passed, 0 failed.bash test/release.sh: 68 passed, 0 failed. Matches the PR's claim.Merge order stands: #79 → this → box#125, promptly, since the seed window is open between the last two.
— automated review by
claude-bot-andresmgsl· heavy-duty-review-botVerdict: Approve — I agree with this as-is at
94d9628(stacked on #79).Re-reviewed from the tip after the explicit re-request (prior approve was on
436756b).Round fixes, verified
workload-serverin comment, usage, and staging next-step). Correct stack placement: no window where #79 is mergeable while shipping a dead command.READMErehearsal — continuation now saysstaging-box.heavy-duty/box#125 (closing heavy-duty/box#123).feat(bootstrap)!:matches the BREAKING tenant entry.Design still holds
*-box; guest user / agent CLI / dotdirs stay unsuffixed (claudeuser,~/.claude/…) — the dangerous half of the rename is correctly left alone.tenant-config.shcase arms and bootstrap dispatch fully renamed; CI green.Merge order: #79 → this → box#125 (promptly; seed window between the last two).
Round two passed — three head-current approvals at
94d9628, all CI green. Handing to @danmt.Nothing outstanding. The blocking finding was fixed one layer down in #79 (that being the commit which deletes the
workloadrole), and this branch carries it via a clean rebase —00f6351is its ancestor with one commit on top. Both non-blocking notes taken:README.md:1044's half-renamed/ stagingcontinuation line, and the CHANGELOG now citing heavy-duty/box#125 alongside the box#123 tracking issue.Merge second: rig#79 → this → heavy-duty/box#125.
The cross-repo contract has been checked from both sides — box#125's seeds set
BOX_BOOTSTRAP_ROLEto exactly the four names this PR'scasearms accept, andblankcorrectly stays unsuffixed.One thing worth knowing that no test can cover: none of this has been exercised on real hardware. The suites prove the CLI surface, the refusals and the sweeps; a cold mint of a
claude-boxorstaging-boxguest against a post-rename rig has not run, and cannot until both of these land, since box#125 is blocked behind them. The first realbox new --template claude-boxis the thing to watch.