fix(#689): only remove control-runner units the acting checkout owns - #692
Merged
Conversation
The pithead-control.{path,service} names are box-global, but a release
bench holds several checkouts (live stack + e2e harness + bundle-smoke
tmp dirs). Both removal paths — provision_control_runner's disabled
branch and the e2e teardown reap — deleted whatever units were
installed, stranding the live stack's dashboard control requests (the
config editor sat at "Previewing…" until the next apply reinstalled the
units; hit live on the bench 2026-07-19, twice in one afternoon).
Key both removals on the service unit's ExecStart matching $PWD, with
grep -F throughout — versioned dirs carry dots (pithead-v1.9.3), and a
regex dot could claim a sibling's unit as our own. Foreign-owned units
are left in place (the e2e reap counts that as success); a dangling
path unit with no service file is still reaped. PITHEAD_UNIT_DIR env
seam added for the tier-1 tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… entry Review fixes for the ownership check. The literal ExecStart=$PWD match missed that one checkout has two path spellings: production units carry the versioned dir, while an operator's disable apply runs through the `current` symlink — the apply saw its own unit as foreign and never removed it (fails safe, never converges). Both removal paths now extract the ExecStart path from the service unit and compare physical resolutions (deepest existing ancestor via cd/pwd -P, remainder kept verbatim; unparseable ExecStart stays foreign). The e2e teardown mirror in tests/integration/run.sh gets the same treatment, and the tier-1 block gains the versioned-spelling-via-symlink case — it fails on the pre-fix literal comparison, and the foreign-owner cases still hold. Also the missing Keep-a-Changelog entry under [Unreleased]. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ctions Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #689.
What
The
pithead-control.{path,service}unit names are box-global, but a release bench holds several checkouts at once (live stack + e2e harness + bundle-smoke tmp dirs). Two removal paths deleted the units without checking which checkout installed them:provision_control_runner's removal branch inpithead(control disabled → remove), which also runs from bundle-smoke/tmpextractions;_remove_control_unitsintests/integration/run.sh(tier-4 harness: failed baseline restore can orphan the root pithead-control systemd units after run_hardening #477), documented as "unconditional".Hit live on the bench 2026-07-19, twice in one afternoon (15:46 tmp-smoke, 16:07 targeted e2e): the live stack's runner was deleted, every dashboard Save & Preview spooled a request nothing processed, and the config editor sat at "Previewing…".
How
Both removals now key on the service unit's
ExecStartmatching the acting checkout ($PWD), withgrep -Fthroughout — versioned install dirs carry dots (pithead-v1.9.3), and a regex dot could claim a sibling's unit as our own (adversarial-review catch; the pre-existing "already installed" greps got-Ftoo). Foreign-owned units are left in place, and the e2e reap counts that as success. A dangling path unit with no service file is still reaped.PITHEAD_UNIT_DIRenv seam added so the removal branch is testable at tier 1.Known ceiling, out of scope (noted in #689): the full e2e hardening phase installs its own units over the live stack's and its teardown then removes them — the live stack is unitless until its next apply/upgrade re-converges.
Testing
tests/stack/run.sh(foreign owner left alone, own units removed, dangling path unit reaped, regex-dot foreign owner left alone — the last two fail on the pre-fix code): 1504 pass, 0 fail.make lintclean; docs updated (docs/operations.mddocuments the ownership check).🤖 Generated with Claude Code