feat(cli)!: report ban enforcement as three states in debug bans

[sanity]

Follow-up to #540, which closed #472 with a boolean. Review of that PR showed the boolean could not carry the information the issue asked for. This replaces it.

Problem

#540 added enforcing: bool, sourced from BansV1::ban_is_enforcing. Reviewers traced post_apply_cleanup gate by gate and found that in any state a user can actually fetch, the flag collapses to !members.contains(target) and carries zero authority information:

• cleanup step 5 retains only bans satisfying ban_signature_matches_current_key, so that gate is unconditionally true for any surviving ban;
• cleanup step 0 removes every target of an authorized ban, so a target still present implies unauthorized implies false;
• for an absent target, ban.rs:274-278 returns bare true without ever calling is_ban_authorized. The authority call at ban.rs:273 is dead on that path.

So the column told an operator whether the person was in the room, which riverctl member list already tells them.

Worse for the issue's actual scenario: deputy bans alice, owner revokes the grant, alice has not yet returned. That displayed ENFORCING, indefinitely, and only flipped after she walked back in — by which point member list shows her. #472's Problem section is "they may well be back in the room"; the boolean covered the "already back" half and missed the "free to come back" half, which is the half a moderator can still act on.

It was also attacker-inflatable: any member can mint bans naming absent ids, each rendering ENFORCING and inflating the header count. The contract's own source documents this as an abuse vector at common/src/room_state.rs:126-134 (#413, Limitation 2).

Approach

The honest answer has three cases, so report three. A ban can be provably dead, provably live, or contingent on something that has not happened yet, and collapsing the third into either neighbour lies in one direction or the other.

State	Condition	Meaning
inert	target is a current member the ban fails to exclude; or the signature does not verify against the banner's current key; or it names the room owner	Definitive: not keeping anyone out.
enforcing	authority holds against current state	Excluding them today. With the target absent this additionally implies an ABSOLUTE grant, so it is durable across their return.
undetermined	target absent and the banner's grant was position-derived (strict ancestor, deputy of a non-owner ancestor) or absent entirely	Not in the room, but whether the ban applies on return depends on who re-invites them.

The classifier is one presence check plus a single is_ban_authorized call, which also revives the authority call that was dead in #540:

signature mismatch          -> Inert   // settled first, or a forged ban reads as enforcing
target == owner             -> Inert   // never a valid target; permanently dead, not contingent
is_ban_authorized           -> Enforcing
target present (so !auth)   -> Inert   // the actionable #472 case: they are in the room
otherwise (target absent)   -> Undetermined

Three states are buildable, not just more honest. The two absolute grants in is_ban_authorized (banner is the owner; banner is an owner-appointed global moderator) read nothing about the target's position, so they re-derive with the target gone. That is what makes enforcing expressible for an absent target at all, and it is the hole in #540's "a stricter check would report every working ban as inert" reasoning. Only strict-ancestor and non-owner-deputy bans become undeterminable. That over-broad claim sat in a Do NOT switch this warning in the merged source; the warning is kept (a bare is_ban_authorized still must not land) but its reason is corrected.

The room-owner case resolves here too: it lands in inert rather than a permanent confident ENFORCING, because the owner cannot be re-invited into a position that would make the ban apply.

Human output:

Ban List (3 bans: 1 enforcing, 1 not enforcing, 1 undetermined)
=========
  7XSOGJTK banned by PMAQEUP5 at 1700000000
  4KDLM2NQ banned by 9WTZR6HB at 1700000042  [NOT ENFORCING]
  QW3RTY88 banned by PMAQEUP5 at 1700000099  [UNDETERMINED]

with a note per state, emitted only when that state is present so neither trains the reader to ignore it. Neither note claims where the target is: a full-state PUT bypasses cleanup via verify, so an uncleaned state can hold an inert ban whose target is already gone (#540's note asserted "those users are in the room" unconditionally).

One deviation from the review, flagged deliberately

The review asked for the remediation hint to become deputized-by <room> <banned_user_id> for the target-deputized-the-banner cause. I checked the two commands' semantics in cli/src/commands/member.rs and did not make that change:

• member deputies <room> <ID> lists who ID has deputized;
• member deputized-by <room> <ID> lists who has deputized ID.

The step-4 guardrail is deputies_of(target).contains(banner) — the target deputized the banner. deputized-by <banned_user_id> asks who deputized the banned user, which is unrelated. The hint stays deputized-by <room> <banned_by_id>, which answers both inert causes in one command: an empty result means the banner holds no grant, and the banned user appearing in the result is the guardrail. The note now says to look for both. Happy to change it if I have misread the intent.

A NOT ENFORCING ban is dormant, not dead

Added in review, and probably the most operationally useful thing in the PR.

Bans are add-only: nothing removes one once stored, and MembersV1::banned_member_ids re-evaluates every stored ban against current state on every cleanup. A revoked deputy who remains a member keeps passing the step-5 signature sweep, so their bans persist indefinitely in an inert state.

Which means re-granting that deputy retroactively re-arms every ban they ever issued — ejecting those targets and their whole invite subtrees at the next cleanup. An operator restoring a moderator's authority going forward would silently re-eject everyone that moderator had ever banned, including bans they may have considered long since undone.

debug bans is the only place that can warn before the re-grant, so the human output now does, under the inert note:

WARNING: a NOT ENFORCING ban is dormant, not deleted. Bans are never removed once
stored, and every one of them is re-checked each time room state is cleaned up.
Restoring a moderator's authority therefore re-arms every ban they ever issued,
ejecting those users AND everyone they invited at the next cleanup. Read this list
before re-granting anyone.

This is also why the inert rustdoc no longer says such bans "can never apply at all".

The signature gate is load-bearing, not defensive

Stating this plainly because it is the piece most likely to be deleted later as redundant.

ban_signature_matches_current_key runs in front of the match. Cleanup step 5 retains only bans that satisfy it, so on a cleaned state it is unconditionally true and looks like dead weight. That reasoning holds only on the cleaned path. A full-state PUT reaches a client without cleanup having run — verify accepts it, and verify deliberately skips a ban's signature when the banner was absent at bans-apply time, since bans apply before members. So a state you can actually fetch may carry a ban attributed to a current, fully-authorized member but signed by somebody else.

It also has to come first, before the authority question. The authority check knows nothing about who signed: for a forgery attributed to an owner-appointed global moderator, is_ban_authorized answers yes, and the classifier would report a forged ban as enforcing. That is the #472 failure mode with an attacker holding the pen.

Worth noting how this was found: it was not in the first version of this PR. My own mutation run showed that deleting the gate killed zero tests, because no fixture produced a mis-signed ban. ban_signed_by_someone_other_than_its_attributed_banner_is_inert exists because of that, forging with sign_struct + with_signature and asserting the contract does not exclude the target either. The reasoning above is now also recorded at the call site so the next reader does not re-derive the "redundant post-cleanup" conclusion and act on it.

Where the boundary is drawn, and the calls that were not obvious

The reviewer asked specifically whether the three-state line is in the right place, so here are the judgement calls rather than a smooth surface. Two feel settled to me and two are genuinely arguable.

Not actually a judgement call: a lapsed positional grant and no grant at all share undetermined. I first presented this as a deliberate design decision. It is stronger than that — no alternative is expressible. classify_ban receives (ban, members_by_id, member_info, owner_id, owner_vk), and with the target absent, "the banner was their inviter", "the banner was an owner-appointed moderator whose grant was revoked" and "the banner never had authority" are the same input tuple: the invited_by edge died with the target's AuthorizedMember, and the superseded member_info record was collapsed by dedup_to_canonical. The real decision is "do not add grant-history tracking", which River does not keep and this command should not introduce.

Consequence, stated so the mutation table is not read as claiming more than it does: ban_by_a_member_with_no_grant_is_undetermined_once_the_target_is_absent cannot kill a mutation that the ancestor test does not already kill. Both reduce to "signature ok + target absent + no absolute grant → Undetermined", differing only in fixture history the classifier never reads. It is kept because pinning the fourth corner of the matrix is worth having explicitly, not because it is an independent pin.

Settled: a ban naming the room owner is inert, not undetermined. is_ban_authorized denies target == owner_id outright before any grant, and no re-invite can change that, so it is permanently dead rather than contingent. It needs an explicit guard because the owner is not in the members list and would otherwise fall through the absent-target branch and read UNDETERMINED forever.

Was arguable, now resolved against a fourth state: mis-signature folds into inert. I raised this as possibly wanting a separate "suspicious" state. Review showed that argument was about the minority sub-case. ban_signature_matches_current_key returns false for two reasons, and the common one is benign: the banner is not a current member, so their key is simply unavailable to check against. A fourth state would therefore fire mostly on pruned-moderator rows, not forgeries. It would also be wrong about permanence — that case reverses if the banner rejoins with the same key. Both reasons belong in inert, and the docs now say so rather than framing the gate as forgery defence.

Arguable: enforcing covers two things that differ in durability. With the target absent it implies an absolute grant and is durable across their return. With the target still present (only reachable in uncleaned state, since cleanup removes authorized targets) the grant may be positional and would lapse if that ancestor left. Both are truthfully "excluding them today", and the variant docs separate the two claims, but a reader who sees only the word could over-trust the present-target case. Splitting it would mean four states for a distinction that is invisible on the cleaned path, which seemed the worse trade.

Corrected after review: the split is target present vs absent, not "definitive vs contingent". The enum docs originally claimed the cleaner epistemics. They do not hold: an inert present-target ban is contingent too, since re-granting the banner's authority re-arms it, which is arguably likelier than the target being re-invited under some particular member. The boundary stays where it is (present vs absent is the informative split and the notes are present-tense and true), but the docs no longer overclaim, because a reader reasoning from "inert means definitively dead" would get the re-arming hazard wrong.

Testing

21 tests in cli/src/commands/debug.rs, all mutation-verified rather than assumed. Every mutation below was applied to the committed content and confirmed to fail:

Mutation	Killed by
classify_ban always Enforcing / Inert / Undetermined	6 / 8 / 10 tests
bare is_ban_authorized, false collapsed to Inert (the #540 shape)	legitimate_ancestor_ban_becomes_undetermined_once_its_target_is_removed, each_ban_is_classified_independently
absent target => Undetermined without checking authority (the lazy three-state)	absolute_grants_stay_enforcing_with_the_target_absent
owner-as-target guard removed	ban_naming_the_room_owner_is_inert_not_undetermined
signature gate removed	ban_signed_by_someone_other_than_its_attributed_banner_is_inert
[UNDETERMINED] marker collapsed into [NOT ENFORCING]	human_output_marks_each_state_distinctly
Bans arm builds BanInfo inline with a hardcoded verdict	bans_command_delegates_to_the_shared_helpers
human branch stops calling ban_list_lines	same
JSON branch serializes a different value	same

Addressing the specific gaps the testing lens found:

• Call site is now pinned. All three call-site mutations previously compiled and left every test green, including one restoring the original bug with a confident flag on top. bans_command_delegates_to_the_shared_helpers is an include_str! source pin in the idiom already used by storage.rs and identity.rs, cutting the body at mod tests so the assertions cannot satisfy themselves. This is not hypothetical: feat(cli): show whether each ban still enforces in debug bans #540's salvaged first draft added the helper and left the arm building BanInfo inline.
• Multi-ban coverage. each_ban_is_classified_independently builds a room holding one ban of each state and asserts the verdicts line up with the bans in stored order, so a classifier hoisted out of the closure, or one verdict stamped on every entry, fails.
• JSON branch pinned, plus every_enforcement_state_has_a_distinct_json_spelling for the wire spelling of all three variants.
• Owner-as-target has its own test.
• The signature-gate test exists because my own mutation run found that removing the gate killed nothing. It was the one real hole in the suite.

a_present_target_agrees_with_the_contracts_excluded_set ties the verdict to MembersV1::banned_member_ids for present targets, where the equivalence genuinely holds, and asserts Undetermined never occurs there.

Full riverctl lib suite: 300 passed, 0 failed. cargo fmt --check clean, no new clippy warnings.

JSON field change (nothing published can break)

enforcing: bool is replaced by enforcement, a string enum of inert / enforcing / undetermined. banned_user_id, banned_by_id and banned_at_secs are unchanged.

Labelled a breaking change in the commit trailer for accuracy, but it cannot break a consumer: enforcing shipped only in #540, and git tag --contains d36faf3e returns zero tags. The latest release is riverctl-v0.2.5 while cli/Cargo.toml is at 0.2.7, so the bool has never been in a published riverctl. The key is renamed rather than changed in place so that anyone tracking main sees a missing field instead of a bool that silently became a string.

[sanity]

Independent review — boundary lens

Reviewed at b7b829cd by a reviewer that did not write the code, reading the source before the PR description. Verdict: the three-state boundary is in the right place. Merge after fixing finding 2.

cargo test -p riverctl --lib debug:: run independently: 21 passed.

Confirmed by independent tracing

• The absolute-grant set is exactly banner == owner_id (common/src/room_state/member.rs:336) and banner_is_member && deputies_of(owner).contains(banner) (:378). With the target absent, strict_ancestors collapses to {owner_id} because the walk seeds from members_by_id.get(&target) == None, so no third absolute grant exists.
• The owner-target guard (debug.rs:153) is correct and reachable: ban.rs:127-129 returns early with the ban valid when the target is not in the member map, so such a ban can sit in fetched state and would otherwise render Undetermined forever.
• The signature gate is genuinely load-bearing and order-dependent: in the forged fixture the banner is in deputies_of(owner), so is_ban_authorized returns true at :378; deleting or reordering the gate reports a forgery as Enforcing.

Finding 1 — the contested merge is forced by the inputs, not a judgement call

classify_ban receives (ban, members_by_id, member_info, owner_id, owner_vk). With the target absent, former inviter, revoked owner-deputy, and never had authority are the same input tuple — the invited_by edge died with the target's AuthorizedMember, and dedup_to_canonical (room_state.rs:282) collapsed the superseded record. No alternative classifier is expressible, so the decision being defended is really "don't add grant-history tracking".

Consequence: ban_by_a_member_with_no_grant_is_undetermined_once_the_target_is_absent cannot kill any mutation the ancestor test doesn't already kill — both reduce to "sig ok + target absent + no absolute grant". Worth keeping as a pinned corner, but it is a documentation test, not an independent pin.

Finding 2 — MEDIUM, fix before merge

cli/src/commands/debug.rs:269-273 tells the operator the banner's authority "came from a position in the invite tree relative to that target". Per finding 1 the function has no input that could establish this, and it is false for at least two sub-cases that land in Undetermined: a revoked owner-appointed global moderator, and a banner who never held authority.

The revoked-moderator case is this issue's headline scenario. With the target absent it renders Undetermined, and this note sends the moderator to inspect the invite tree when the actionable fact is that the grant was revoked — while the command that surfaces it (member deputized-by <room> <banned_by_id>) is advertised only in the inert note.

Fix is a string. The existing pin only requires "depends on who re-invites them", so no test moves.

Finding 3 — highest-value addition

A NOT ENFORCING row is not a dead row. Bans are add-only tombstones and banned_member_ids re-evaluates every stored ban on every cleanup (member.rs:229-291), so a revoked deputy who remains a member keeps passing the step-5 signature sweep. Re-granting that deputy retroactively re-arms every ban they ever issued, ejecting those targets and their subtrees at the next cleanup. An operator restoring a moderator's authority going forward would silently re-eject everyone that moderator ever banned, and this command is the only place that could warn them.

Findings 4-5 — comment/string polish

• Inert's rustdoc (:57-61) says such a ban "can never apply at all". ban_signature_matches_current_key returns false for two reasons (ban.rs:225-231): genuine mis-signature, and banner not a current member — the more common case, exercised by ban_is_inert_once_its_banner_is_no_longer_a_member, and reversible if that banner rejoins with the same key. The verdict Inert is right; "never" is wrong. The new comment at :140-155 frames the gate purely as forgery defence and repeats the omission.
• The inert note (:256-262) lists three causes and omits "it names the room owner", which :153 deliberately routes there.

On what is arbitrary

The docs frame the Inert/Undetermined split as "definitive vs contingent" (:50-53, :129-132); the code implements target present vs absent. Inert-by-unauthorized is also contingent — that is finding 3's mechanism. The boundary should not move (present-vs-absent is the informative split and the notes are present-tense and true), but the stated rationale is not the one the code implements, and a reader reasoning from "Inert means definitively dead" gets finding 3 wrong.

PR body corrections

• Says 20 tests; there are 21.
• The enforcing: bool JSON change is labelled BREAKING, but git tag --contains d36faf3e returns zero tags (latest riverctl-v0.2.5, cli/Cargo.toml at 0.2.7) — it was never released, so nothing published can break.

[AI-assisted - Claude]