SYSTEMCTL_VERBS assertion is order-sensitive (ruled acceptable; revisit or close)

[ssandys]

Summary

test_the_prompt_is_never_suppressed asserts tuple equality, so reordering
the verbs inside LIFECYCLE_VERBS or BOOT_VERBS fails the test even though
nothing meaningful changed. Raised during the boot-toggle review; I ruled it
acceptable at the time. Filed so the ruling is visible and revisitable rather
than lost.

Where

tests/test_action.py:53-55:

        self.assertEqual(action.SYSTEMCTL_VERBS,
                         action.LIFECYCLE_VERBS + action.BOOT_VERBS,
                         "SYSTEMCTL_VERBS must cover every systemctl verb")

What it does and does not catch

Mutation-tested when it was written:

Change to SYSTEMCTL_VERBS	Result
Narrowed to LIFECYCLE_VERBS, dropping the boot verbs	fails — correct
Hand-written literal missing a verb	fails — correct
Hand-written literal, complete and correctly ordered	passes — correct
Verbs reordered, same members	fails — arguably over-strict

Only the last row is the complaint. The assertion is not tautological: it pins
the relationship between the three tuples, and the regression it exists to
catch — someone narrowing SYSTEMCTL_VERBS so new verbs escape the
--no-ask-password guard — is caught.

Why the assertion matters at all

SYSTEMCTL_VERBS exists so the flag guard's contract is every verb that shells
out to systemctl, not the lifecycle verbs. Re-adding --no-ask-password
does not fail loudly: it sets allow_interactive_authorization = false, which
silently converts every authentication dialog into permission denied with
nothing logged. A new verb category placed in its own tuple would ship unguarded
while the per-verb loop kept passing. This assertion is what makes that
impossible without deliberately editing it.

Options

1. Close as wontfix. Order stability is cheap to require, the failure
   message names the invariant, and anyone reordering the verbs will read it and
   understand immediately. This was the original ruling.
2. Compare as sets, keeping a separate length check so duplicates still
   fail:

   self.assertEqual(set(action.SYSTEMCTL_VERBS),
                    set(action.LIFECYCLE_VERBS) | set(action.BOOT_VERBS),
                    "SYSTEMCTL_VERBS must cover every systemctl verb")
   self.assertEqual(len(action.SYSTEMCTL_VERBS),
                    len(action.LIFECYCLE_VERBS) + len(action.BOOT_VERBS),
                    "SYSTEMCTL_VERBS must not drop or duplicate a verb")

   Two assertions instead of one, to buy tolerance for a reordering nobody has
   yet wanted to do.

I lean to option 1. Option 2 trades a clear single assertion for two, and the
scenario it accommodates is hypothetical.

Verify, if option 2 is chosen

Re-run the mutation table above by hand against scripts/colophon_action.py —
the first three rows must behave identically, the fourth must now pass.
Then ./bin/test: 121 Python + 27 JavaScript, 0 skips, unchanged.

Priority

Lowest of the open issues. No defect, no user-visible behaviour.

[ssandys]

Correction — the expected suite counts in this issue are stale. 53dba0d ported a devkit from galley which added tests/test_dev.py; the current baseline is 150 Python + 29 JavaScript, 0 skips, not the 121 + 27 quoted above. The mutation table and the decision itself are unaffected.

[ssandys]

Revisited 2026-08-31, alongside a broader cleanup pass (#8–#17). Recommendation: leave open and unchanged, or close as wontfix. Do not fix it.

The assertion is still exactly as described — tests/test_action.py:53-55:

self.assertEqual(action.SYSTEMCTL_VERBS,
                 action.LIFECYCLE_VERBS + action.BOOT_VERBS,
                 "SYSTEMCTL_VERBS must cover every systemctl verb")

assertEqual on tuples compares order, so reordering the concatenation fails the test without any behavioural change. That is the original complaint and it is accurate.

Why it still is not worth fixing. Two things have changed since the ruling, and both cut the same way:

1. The verb tuples have grown — MODEL_VERBS and PARAM_VERBS now sit beside these two (scripts/colophon_action.py:33-40) — but SYSTEMCTL_VERBS itself is unchanged and nothing has come close to reordering it. The predicted false failure has not occurred in the ~2 weeks since.
2. The order it pins is not arbitrary. LIFECYCLE_VERBS + BOOT_VERBS is the order the panel presents them in and the order the privilege spec discusses them in, so a diff that reorders it is a diff worth a human glance anyway.

Converting to a set comparison would cost a line and lose that. The failure mode if it ever does fire is loud, immediate, and self-explanatory from the message.

Flagging it here rather than silently: this was reviewed as part of the same pass that produced ten new issues, so its remaining open should be read as a decision rather than an oversight.