fix(control): a failed watchdog re-render no longer reports as applied (#395)

[VijitSingh97]

Fixes #395.

What was wrong

install_watchdog ended on systemctl enable ... || true, so it returned 0 whatever had happened
above it — and both apply paths called it under || true, which additionally suppresses set -e for
the whole dynamic extent of the call. A unit write that could not land therefore aborted nothing and
surfaced nowhere. A control /apply changing watchdog_interval_min recorded applied while the rig
kept its old cadence, with nothing anywhere to detect the divergence.

The issue's literal suggestion — deleting the || true at the call sites — would not have fixed
this, and would have made it worse. Confirmed with a repro, not by reading: with || true in place,
a function whose first command fails and which then runs a bare false still returns 0. Remove the
mask and the first failing sudo tee instead ERR-traps control_apply out before it writes any
terminal status, leaving the receiver serving the previous outcome forever — the exact failure #276
already fixed for an unreadable rollback backup. The honest return value has to come from the
function itself.

What changed — four seams

• install_watchdog records the steps that can genuinely fail (the two unit writes,
  daemon-reload, and the removal in the disabled branch) and returns that status. The || true on
  enable/disable of a possibly-absent timer stays deliberate.
• _control_do_apply_fast propagates it and bails before stamping provenance — a change that
  did not take effect must not be recorded as the config in force. That re-render is the fast path's
  entire effect. The caller then routes to the same full-pipeline rollback that path's own comment
  already promises for a fast-path failure.
• apply captures it, still runs the remaining install_* steps (abandoning the reconcile would
  strand more units than the one that failed), warns with an actionable message, and returns non-zero.
• _control_do_apply gates on apply's exit status, which it discarded outright.

The recorded reason now names the watchdog instead of a hashrate that never dropped, and when the
rollback's own re-apply hits the same failure it says so rather than asserting a liveness check that
never ran. Every pre-existing reason string is byte-identical when no such cause is known.

Second caller — please read this before merging

apply gaining a non-zero return affects two callers, not one. I swept every call site:

• _control_do_apply (rigforge.sh:4199) — the intended one.
• the CLI dispatch (rigforge.sh:5960), which calls apply "$@" bare under set -Eeuo pipefail
  with trap on_err ERR armed. So sudo rigforge apply with unwritable watchdog units now prints
  the specific warning, then aborts through on_err and exits non-zero, where it previously exited 0.

I believe that is the correct behaviour — an apply that did not apply should not exit 0, which is the
whole point of the issue — but it is a user-visible change beyond the control path, so it is called
out here and documented in the apply row of docs/operations.md. Say the word if you want the CLI
path left on its old exit code.

Scope

One step beyond the three seams the issue names: the disabled branch's sudo rm -f is guarded too. A
timer that could not be removed is the same lie in the other direction. Two-line revert if unwanted.

The issue's alternative — a doctor check comparing the rendered timer's OnUnitActiveSec against
config.json — is deliberately not implemented here. It is a detector for drift from any cause,
which is a separate piece of work from making this path honest.

What was RUN

Graded by who ran it, because the two are not worth the same.

Re-derived on this branch, just now:

• bash tests/run.sh → 1853 passed, 0 failed, exit 0 (baseline 1826; 27 new assertions, no
  existing test changed by apply gaining a non-zero return).
• shfmt -i 4 -d clean, bash -n clean on both changed files.
• Two mutants, both killed, by disjoint assertion sets — the two seams the whole fix rests on:
  • reverting install_watchdog's return "$rc" to a bare return 0 → suite exit 1, killed by
    four assertions, all of them the Control /apply reports applied even when the watchdog re-render failed #395 unit-write ones. The .service and .timer writes fail
    separately, which is the point: an earlier version of these tests broke both writes with one
    unwritable dir, so reverting either guard left the suite green. The per-file tee stub
    (WDF_TEE_FAIL) is what discriminates them. The discriminator has to be the fixture, not the
    assertion.
  • reverting _control_do_apply's || return 1 back to discarding apply's status → suite exit 1,
    killed by three different assertions (_control_do_apply fails when apply fails, a failed apply short-circuits the liveness wait, the reason says the re-apply hit the same failure).
  • No overlap between the two kill sets, so neither guard is alibiing the other.

Inherited from the session that wrote the tests, and NOT re-derived here: the full mutation
sweep was 10 mutants, all reported killed. Its harness lived in that session's scratchpad and is
gone, so I re-ran only the two above rather than restate a number I cannot reproduce. The remaining
eight are unverified by me.

What was NOT run locally

• shellcheck — its historical peak on a file this size has OOM-killed whole sessions on the
  box this was built on, taking the tmux scope with it. Deferred to CI deliberately.
• The Docker e2e — deferred to CI.

Both have since reported on this PR: Lint (shellcheck + shfmt) pass, End-to-end (Docker)
pass. All nine checks are green and mergeable / mergeStateStatus re-queried separately as
MERGEABLE / CLEAN — a tick count is not a mergeability claim in this project.

🤖 Generated with Claude Code

https://claude.ai/code/session_01E5tRnh79JUNv6q2q4F5qRh