feat(post-rebuild-check): let a machine declare which units it means to leave disabled

Check 2 treats "enabled" as a proxy for "will actually run", and the proxy is wrong for a unit nobody intends to enable on that box. velox has four: geoclue-agent is redundant because hyprland's exec-once starts the binary itself, emacs is started on demand by emacsclient, obs-record-watchdog only matters while recording, and obsbot-wb-guard needs an OBSBOT the laptop doesn't have. All four reported at every run.

Four permanent lines in front of every real finding teach me to skim the output, which is the argument check 4 already makes about CLAUDE.md. On velox this takes the run from 8 findings to 4.

The list is machine-local, read from $XDG_CONFIG_HOME/post-rebuild-check/units-expected-disabled. It can't live in the unit file, because obsbot-wb-guard is correctly enabled on ratio. One unit, a different right answer per machine.

An entry whose unit is enabled after all is reported too. The main loop can't catch that, since it skips any state that isn't disabled or linked, so the list gets its own pass. Without it a stale exemption sits there suppressing nothing, and the list becomes somewhere real findings go to die.

The exemption is tested last, so it never hides the dangling-symlink finding decided on the filesystem above it.

2 files changed, 173 insertions, 1 deletions

diff --git a/scripts/post-rebuild-check b/scripts/post-rebuild-check
index 14f2415..ee77a19 100755
--- a/scripts/post-rebuild-check
+++ b/scripts/post-rebuild-check
@@ -72,6 +72,15 @@
 #                         running, the special value MISSING = not installed
 #   PRC_REPO_REMOTES      newline list of "path origin-url"; an empty URL
 #                         means origin could not be read
+#   PRC_UNITS_EXPECTED_DISABLED
+#                         newline list of units whose not-enabled state is
+#                         deliberate here, replacing the file below
+#   PRC_UNITS_EXPECTED_DISABLED_FILE
+#                         path to that list (default:
+#                         $XDG_CONFIG_HOME/post-rebuild-check/units-expected-disabled).
+#                         One unit per line, # starts a comment. Machine-local
+#                         on purpose: the same unit is correctly enabled on one
+#                         box and not another
 #   PRC_SYSTEMCTL         path to the systemctl binary (a fake, under test)
 #   PRC_SYSTEMCTL_TIMEOUT seconds to allow each systemctl call (default 5)
 #
@@ -209,6 +218,39 @@ done < "$STAGE"
 report "check 1/8: failed units"
 
 # --- 2. user unit files present but not enabled ---------------------------
+#
+# Units nothing intends to enable here are read from a machine-local list.
+# "Enabled" is this check's proxy for "will actually run", and the proxy is
+# wrong for a unit nobody means to enable on this box. velox carries four, for
+# four different reasons: geoclue-agent is redundant because hyprland's
+# exec-once starts the binary directly, emacs is started on demand by
+# emacsclient, obs-record-watchdog only matters while recording, and
+# obsbot-wb-guard needs an OBSBOT the machine does not have. Left unexempted
+# they report at every run, and four permanent lines in front of every real one
+# teach you to skim the output -- the same argument check 4 makes about
+# CLAUDE.md.
+#
+# Machine-local rather than a marker in the shared unit file, because
+# obsbot-wb-guard is correctly ENABLED on ratio. One unit, a different right
+# answer per machine, so the shared file cannot hold the answer.
+#
+# An entry that turns out to be enabled after all is still a finding. Without
+# that the list rots into somewhere real findings go to die, which is worse
+# than the noise it removes.
+
+EXPECT_DISABLED_FILE="${PRC_UNITS_EXPECTED_DISABLED_FILE:-${XDG_CONFIG_HOME:-$HOME/.config}/post-rebuild-check/units-expected-disabled}"
+if [ -n "${PRC_UNITS_EXPECTED_DISABLED+set}" ]; then
+    expect_disabled=$PRC_UNITS_EXPECTED_DISABLED
+elif [ -f "$EXPECT_DISABLED_FILE" ]; then
+    expect_disabled=$(cat "$EXPECT_DISABLED_FILE" 2>/dev/null)
+else
+    expect_disabled=""
+fi
+# Strip comments and blanks once, here, so the membership test below is a
+# plain word match. The reason a unit is exempt is the most useful thing about
+# the entry, so the format has to carry one.
+expect_disabled=$(printf '%s\n' "$expect_disabled" \
+    | sed 's/#.*//' | awk 'NF {print $1}')
 
 if [ -n "${PRC_UNIT_STATES+set}" ]; then
     states=$PRC_UNIT_STATES
@@ -294,8 +336,34 @@ while read -r name state; do
             esac
             ;;
     esac
+    # Deliberately not enabled on this machine. Checked last, so it suppresses
+    # only this finding and never the dangling-link one decided above on the
+    # filesystem.
+    case "
+$expect_disabled
+" in
+        *"
+$name
+"*) continue ;;
+    esac
     finding "unit file present but not enabled: $name ($state)"
 done < "$STAGE"
+# The exemption list, checked in the other direction. An entry whose unit is
+# enabled after all suppresses nothing, and leaving it there is how the list
+# turns into a place real findings go to die. The loop above cannot catch this:
+# it skips any state that is not disabled or linked, so an enabled unit never
+# reaches it.
+printf '%s\n' "$expect_disabled" > "$WORK/expect" 2>/dev/null || {
+    echo "post-rebuild-check: cannot write $WORK/expect" >&2
+    echo "  nothing was checked; this is not a pass" >&2; exit 1; }
+while IFS= read -r name; do
+    [ -n "$name" ] || continue
+    estate=$(awk -v u="$name" '$1 == u {print $2; exit}' "$WORK/states")
+    case "$estate" in
+        enabled|enabled-runtime)
+            finding "$name is listed as expected-disabled but is $estate — drop the stale exemption" ;;
+    esac
+done < "$WORK/expect"
 report "check 2/8: unit files"
 
 # --- 3. *.example files whose real sibling is missing ---------------------
diff --git a/tests/post-rebuild-check/test_post_rebuild_check.py b/tests/post-rebuild-check/test_post_rebuild_check.py
index 9902299..a034f87 100644
--- a/tests/post-rebuild-check/test_post_rebuild_check.py
+++ b/tests/post-rebuild-check/test_post_rebuild_check.py
@@ -31,6 +31,9 @@ probe"):
                         running; "MISSING" = not installed on this machine)
   PRC_REPO_REMOTES      newline list of "path<space>origin-url" for the
                         push-capability check ("" = no repos to check)
+  PRC_UNITS_EXPECTED_DISABLED
+                        newline list of units whose not-enabled state is
+                        deliberate on this machine ("" = no exemptions)
 
 Run from repo root:
     python3 -m unittest tests.post-rebuild-check.test_post_rebuild_check
@@ -51,7 +54,8 @@ CHECK = os.path.join(REPO_ROOT, "scripts", "post-rebuild-check")
 def run_check(failed_units="", unit_states="", local_roots="",
               project_roots="", signal_accounts="+15045551234",
               ntp_sources="162.159.200.1\npool.ntp.org",
-              idle_daemon="4242", repo_remotes=""):
+              idle_daemon="4242", repo_remotes="",
+              units_expected_disabled=""):
     """Run the script with every probe stubbed; defaults are all-clean.
 
     Roots are newline-separated. Empty means "the seam is set and names no
@@ -67,6 +71,7 @@ def run_check(failed_units="", unit_states="", local_roots="",
     env["PRC_NTP_SOURCES"] = ntp_sources
     env["PRC_IDLE_DAEMON"] = idle_daemon
     env["PRC_REPO_REMOTES"] = repo_remotes
+    env["PRC_UNITS_EXPECTED_DISABLED"] = units_expected_disabled
     return subprocess.run(
         ["sh", CHECK], capture_output=True, text=True, timeout=30, env=env,
     )
@@ -165,6 +170,7 @@ class NtpBootstrap(unittest.TestCase):
                     "PRC_SIGNAL_ACCOUNTS": "+15045551234",
                     "PRC_IDLE_DAEMON": "4242",
                     "PRC_REPO_REMOTES": "",
+                    "PRC_UNITS_EXPECTED_DISABLED": "",
                     "PRC_CHRONY_CONF": chrony_conf})
         env.pop("PRC_NTP_SOURCES", None)
         return subprocess.run(["sh", CHECK], capture_output=True, text=True,
@@ -281,6 +287,100 @@ class IdleDaemon(unittest.TestCase):
                               run_check(idle_daemon=pids).stdout.lower())
 
 
+class UnitsExpectedDisabled(unittest.TestCase):
+    """Check 2 — units nothing intends to enable on this machine.
+
+    "Enabled" is the check's proxy for "will actually run", and the proxy is
+    wrong for a unit nobody means to enable here. velox carries four such
+    units, for four different reasons: geoclue-agent is redundant because
+    hyprland's exec-once starts the binary directly, emacs is started on demand
+    by emacsclient, obs-record-watchdog only matters while recording, and
+    obsbot-wb-guard needs an OBSBOT the machine doesn't have.
+
+    Left unexempted they report at every run, which is the standing-findings
+    problem check 4's own comment already argues against: four permanent lines
+    in front of every real one teach you to skim the output.
+
+    The exemption is machine-local rather than a marker in the shared unit
+    file, because obsbot-wb-guard is correctly ENABLED on ratio. Same unit,
+    different right answer per machine.
+    """
+
+    # --- Normal cases ---------------------------------------------------
+
+    def test_an_exempt_unit_is_not_flagged(self):
+        r = run_check(unit_states="emacs.service linked",
+                      units_expected_disabled="emacs.service")
+        self.assertEqual(r.returncode, 0, r.stdout)
+        self.assertNotIn("emacs.service", r.stdout)
+
+    def test_a_non_exempt_unit_still_flags(self):
+        r = run_check(unit_states="roam-sync.timer linked",
+                      units_expected_disabled="emacs.service")
+        self.assertEqual(r.returncode, 1)
+        self.assertIn("roam-sync.timer", r.stdout)
+
+    def test_several_exemptions_all_apply(self):
+        r = run_check(
+            unit_states=("emacs.service linked\n"
+                         "geoclue-agent.service linked\n"
+                         "obsbot-wb-guard.service linked"),
+            units_expected_disabled=("emacs.service\n"
+                                     "geoclue-agent.service\n"
+                                     "obsbot-wb-guard.service"))
+        self.assertEqual(r.returncode, 0, r.stdout)
+
+    # --- Boundary cases -------------------------------------------------
+
+    def test_an_exemption_that_is_actually_enabled_is_a_finding(self):
+        # A stale exemption must surface rather than sit there suppressing
+        # nothing. Otherwise the list rots into a place real findings go to
+        # die, which is worse than the noise it was added to remove.
+        r = run_check(unit_states="obsbot-wb-guard.service enabled",
+                      units_expected_disabled="obsbot-wb-guard.service")
+        self.assertEqual(r.returncode, 1)
+        self.assertIn("obsbot-wb-guard.service", r.stdout)
+
+    def test_comments_and_blank_lines_are_ignored(self):
+        # The reason a unit is exempt is the most useful thing about the
+        # entry, so the format has to hold a comment next to it.
+        r = run_check(unit_states="emacs.service linked",
+                      units_expected_disabled=("# started on demand\n"
+                                               "\n"
+                                               "emacs.service   # not by systemd\n"))
+        self.assertEqual(r.returncode, 0, r.stdout)
+
+    def test_no_exemptions_flags_everything_as_before(self):
+        r = run_check(unit_states="emacs.service linked",
+                      units_expected_disabled="")
+        self.assertEqual(r.returncode, 1)
+        self.assertIn("emacs.service", r.stdout)
+
+    def test_an_exemption_does_not_suppress_a_dangling_link(self):
+        # A stowed unit pointing at a missing target is a different finding,
+        # decided on the filesystem. Exempting the name must not hide that.
+        d = tempfile.mkdtemp(prefix="prc-units-")
+        self.addCleanup(shutil.rmtree, d, True)
+        unit_dir = os.path.join(d, "systemd", "user")
+        os.makedirs(unit_dir)
+        link = os.path.join(unit_dir, "emacs.service")
+        os.symlink(os.path.join(d, "gone.service"), link)
+        env = dict(os.environ)
+        env.update({"PRC_FAILED_UNITS": "", "PRC_LOCAL_SCAN_ROOTS": "",
+                    "PRC_PROJECT_ROOTS": "",
+                    "PRC_SIGNAL_ACCOUNTS": "+15045551234",
+                    "PRC_NTP_SOURCES": "162.159.200.1",
+                    "PRC_IDLE_DAEMON": "4242",
+                    "PRC_REPO_REMOTES": "",
+                    "PRC_UNITS_EXPECTED_DISABLED": "emacs.service",
+                    "XDG_CONFIG_HOME": d})
+        env.pop("PRC_UNIT_STATES", None)
+        r = subprocess.run(["sh", CHECK], capture_output=True, text=True,
+                           timeout=30, env=env)
+        self.assertIn("points at a missing target", r.stdout)
+        self.assertEqual(r.returncode, 1)
+
+
 class RepoPushCapability(unittest.TestCase):
     """Check 8 — a working repo cloned from the read-only endpoint.
 
@@ -797,6 +897,7 @@ class SignalAccount(unittest.TestCase):
                     "PRC_SIGNAL_ACCOUNTS": "+15045551234",
                     "PRC_IDLE_DAEMON": "4242",
                     "PRC_REPO_REMOTES": "",
+                    "PRC_UNITS_EXPECTED_DISABLED": "",
                     "signal_missing": "1"})
         r = subprocess.run(["sh", CHECK], capture_output=True, text=True,
                            timeout=30, env=env)
@@ -867,6 +968,7 @@ class ProbeFailure(unittest.TestCase):
                     "PRC_SIGNAL_ACCOUNTS": "+15045551234",
                     "PRC_IDLE_DAEMON": "4242",
                     "PRC_REPO_REMOTES": "",
+                    "PRC_UNITS_EXPECTED_DISABLED": "",
                     "TMPDIR": "/nonexistent-tmp-dir"})
         r = subprocess.run(["sh", CHECK], capture_output=True, text=True,
                            timeout=30, env=env)
@@ -890,6 +992,7 @@ class RealUnitDirEnumeration(unittest.TestCase):
                     "PRC_SIGNAL_ACCOUNTS": "+15045551234",
                     "PRC_IDLE_DAEMON": "4242",
                     "PRC_REPO_REMOTES": "",
+                    "PRC_UNITS_EXPECTED_DISABLED": "",
                     "XDG_CONFIG_HOME": config_home})
         env.pop("PRC_UNIT_STATES", None)
         return subprocess.run(["sh", CHECK], capture_output=True, text=True,
@@ -945,6 +1048,7 @@ class WedgedSystemctl(unittest.TestCase):
                         "PRC_SIGNAL_ACCOUNTS": "+15045551234",
                         "PRC_IDLE_DAEMON": "4242",
                         "PRC_REPO_REMOTES": "",
+                        "PRC_UNITS_EXPECTED_DISABLED": "",
                         "PRC_SYSTEMCTL": fake,
                         "PRC_SYSTEMCTL_TIMEOUT": timeout_s,
                         "XDG_CONFIG_HOME": d})