ReviewOS

also looking at this

stacks/ts-cloud

feat(fleet): refuse to destroy a server still holding a moved site's rollback

#183
Merged glennmichael123 wants to merge feat/destroy-drained-guard into main
4 files +284 -2

Refs #167. The last real hole in the move.

The problem

site:move deliberately never deletes anything on the source: it stops the units, drops the gateway route, and leaves the whole tree in place. That leftover tree is the rollback — the one thing that makes a bad cutover recoverable, and the reason the operation can promise reversibility "until the source server is destroyed".

Which made destroying that server the moment the promise expires, with nothing anywhere saying so. The box has no running units for the site, so it looks idle; the config has moved on; the operator is tidying up. Terminating it is exactly the right thing to do once the move is verified, and exactly the wrong thing to do before — and from outside those two are identical.

The fix

destroy asks the box whether it is holding anyone's rollback, and refuses:

This server still holds 1 site tree with nothing running for it:
  hq-bughq (1.2G)
A site moved off this box keeps its files here on purpose — they are the rollback,
and they are what makes the move reversible until this server is destroyed.
Destroying it now discards that.
Verify the site is serving from its new home first, then re-run with
--discard-drained-sites to destroy it anyway.

Deliberate details

  • Checked before the existing prompt. An operator answering "yes" to a generic irreversibility warning has not been told that a moved site's only copy is on this disk.
  • --discard-drained-sites is its own flag, not folded into --force. --force exists so a teardown can run unattended; a CI job skipping a prompt must not also silently discard a rollback.
  • A site with something running is not reported. That's an ordinary site on a box being torn down, already covered by the existing confirmation. The interesting case is files with nothing running.
  • Only directories holding releases/ count, so a stray folder under /var/www can't block a teardown the operator genuinely wants.
  • The scan is read-only, and a box that can't be reached produces a note rather than a block — this exists to stop one specific silent loss, not to stand between an operator and an unreachable server they're trying to be rid of.

Verification

11 unit tests (scan construction, slug regex escaping, read-only-ness including redirect targets, parsing, the refusal's wording for one vs several).

Plus 3 end-to-end cases against real systemd — the Docker harness from #180 already leaves a genuinely drained box, so the scan is exercised for free: reported as drained on the source, not on the target that's serving it, and no longer drained once the source is restarted. 18 e2e tests passing.

Full suite: 4268 pass, 0 fail.

Note on local test failures

If you see ~51 local failures on main right now, they're a stale packages/core/dist/ — it predates resolveMailService, so the CLI subprocess tests fail to import it. CI builds fresh, which is why it's green. cd packages/core && bun run build clears it. Nothing wrong with the code; it cost me a detour, so flagging it.

4 changed files on the files tab, with 0 review threads.