fix(contract): stop doomed DMs winning global-cap slots and breaking cleanup idempotence

[sanity]

Problem

Fixes the measured defect in #671. Read that issue to the end — two of its
earlier analyses are corrected in later comments, and this PR implements the
corrected version.

A DM whose sender or recipient is not a live member was handled inconsistently
depending only on which peer happened to be holding it:

• an incoming such DM is rejected inside DirectMessagesV1::apply_delta
  (the resolve_member_vk-returns-None arms);
• an already-held one was not, because apply_delta never re-validated the
  held set. It survived until post_apply_cleanup step 6 swept it.

Those two removal points sit on opposite sides of trim_to_global_cap,
which cost two invariants:

1. Data loss / broken merge laws. The Official room is permanently saturated
at the 300-DM global cap, so every merge presents more candidates than the cap
keeps. A doomed DM ranked against live ones on order_key and — being among the
newest — won a cap slot, evicting a legitimate DM between two current members,
and was then swept anyway. Measured on live Official-room state: six real
messages destroyed by a merge in one direction and not the other.

2. post_apply_cleanup is not idempotent. Step 1 counted DM participants
from every held DM, so a member whose only claim to retention was a DM with a
banned counterparty was exempted from inactivity-prune on pass 1, had that DM
swept by step 6 of the same pass, and was pruned on pass 2. That contradicts the
IDEMPOTENCE invariant the function's own doc comment declares a MUST, and it
matters because peers run cleanup a variable number of times and a full-state
PUT runs it zero.

Approach

Two predicate alignments, one per symptom. Neither touches the retention horizon
or the delta filter — #671's addendum shows the horizon is lossless by
construction, and the measurements agree.

(a) DirectMessagesV1::sweep_unresolvable_endpoints, called from
apply_deltabeforeenforce_caps_and_sort in both arms, drops held DMs
and purge envelopes whose endpoints no longer resolve to a member. The caps then
rank only DMs that can survive the pass.

(b) participants_of_surviving_dms replaces the unfiltered
active_participants at post_apply_cleanup step 1. It shares one
dm_endpoint_is_live function with sweep_after_membership_change, so
exemption ⟺ retention holds by construction rather than by two copies agreeing.
Same remedy #411 round 4 applied to the banner exemption. Not circular: a
counted participant lands in required_ids, survives step 3, and step 6
therefore keeps its DM.

(c) enforced_ban_set_of derives, at DM-apply time, the enforced-ban set
post_apply_cleanup step 0 will compute. That is exact rather than approximate:
the #[composable] macro applies fields in declaration order, so
configuration, bans, members and member_info are all final by then, and
no later field mutates a sibling. Nothing is re-implemented — it calls
BansV1::enforce_user_ban_cap and MembersV1::banned_member_ids, the same two
functions cleanup calls. The only thing done twice is applying them to a copy,
because apply_delta must not mutate its parent, and the copy is taken only when
the ban set is actually over cap.

(d) BansV1::enforce_user_ban_cap is extracted as THE single definition of
the max_user_bans eviction, replacing the inline copy at step 0-cap.
sort_by_cached_key now appears exactly once in the codebase. The two sites
must agree — a DM swept at apply time against a different surviving ban set
than step 6 uses is data loss — and one function is what makes that hold by
construction rather than by two copies happening to match. Two copies of an
almost-identical predicate is precisely what drifted in #671 and in #411 round 4.

An intermediate version of this PR was wrong, and #675 is why

An earlier revision made the apply-time sweep membership-only, on the stated
belief that the enforced-ban set was "not knowable" at that point. That was
false, and review falsified it by execution — it was not knowable only in the
sense that nobody had computed it, and a reviewer computed it in about thirty
lines.

The gap it left is #675: a deputy-issued ban leaves its target a member
all the way through the DM field's apply, because MembersV1::apply_delta is
handed an empty MemberInfoV1 and can therefore only enforce owner and ancestor
authority — a deputy grant lives in member_info.deputies, invisible to it. The
target's DMs were still cap-ranked and only removed afterwards by step 6: the
#671 data loss on a path the narrow fix did not reach. Owner-appointed
moderators banning spammers is that path, so it was not a corner case.

Verified in both directions rather than taken on trust — the reproduction passes
here, and with the apply-time sweep reverted to membership-only it fails with
legitimate DMs at offsets [0, 1, 2, 3, 4] were evicted by DMs a DEPUTY ban dooms. Five real messages per merge, now zero. sweep_unresolvable_endpoints is
deleted outright: once the apply-time sweep is ban-aware it simply is
sweep_after_membership_change. Three sweep predicates collapse to one.

The falsified claim is recorded as falsified in the source, so it is not
re-derived by whoever reads that function next.

On MessagesV1: no new loss, strictly an improvement. A smaller
required_ids means step 4b deletes a pruned member's room messages one pass
earlier, so the surviving message set stops depending on how many times cleanup
happened to run.

Out of scope, per the issue: DirectMessagesV1::verify's hard-reject of a
non-member DM endpoint (the third instance of the #423 class — belongs to #672),
and #413's ban cap-eviction, measured as not firing here.

Cost: +12.3% on a saturated-room merge, accepted deliberately

The remedy makes MembersV1::banned_member_ids run twice per apply — once
in the DM field via enforced_ban_set_of, once at cleanup step 0 — where it
previously ran once. Measured on Official-room-sized live state (93 members,
200 bans against a cap of 200, 300 DMs):

banned_member_ids alone: 471.8 ms
enforced_ban_set_of: 466.6 ms (identical; the ban set is AT cap, not
over, so no clone and no cap sort runs)
whole merge, post-remedy: 4251.2 ms
whole merge, pre-remedy: 3784.6 ms
increase: 12.3 %

Accepted rather than optimised away, because the alternative is the data loss in
#671 and #675. Two things worth recording so the idea is not re-derived:

It cannot be shared between the two sites. The #[composable] macro calls
post_apply_cleanup(&mut self, parameters) with a fixed signature and no
channel from the field applies, and ComposableState::apply_delta hands the DM
field &mut self on the field, not the parent — so the only places to stash a
computed set are a #[serde(skip)] cache on DirectMessagesV1 (a staleness
vector in precisely the code path whose divergence is this PR's data loss, and
it would need a hand-written PartialEq to preserve byte-equality semantics) or
a change to the external freenet-scaffold trait. Neither is worth 470 ms.

A gate was considered and rejected — but not because it fails to help.
Skipping the ban-aware derivation unless a cap can bind preserves the outcome,
and measured on the corpus it would help, on the arm that matters most. The
condition is held > cap; on the delta == None arm — the one that runs on
every ordinary update — every captured state holds exactly 300 against a cap of
300, so the condition is false in 0 of 600 cases, the gate closes, and the
cost is skipped. On the main arm it opens in 86% of merges and saves nothing.

It is rejected for a better reason: it optimises around a build-profile
artifact rather than around anything real. Once the ed25519 cost below is
addressed the whole overhead it targets is ~0.2%, and buying that with a branch,
an outcome-equivalence argument and a fixture rework on a re-keying head is a bad
trade. The fixture rework would not be optional either: the gate would silently
defeat none_delta_path_sweep_is_ban_aware, which drives the delta == None
arm. If that fixture were not cap-binding the gate would route it to the
membership-only path and the guard would keep passing while testing nothing —
introducing a guard that stops guarding without failing, in the act of optimising
a fix whose whole subject is guards that stop guarding.

Most of this cost is not really the fix. A bare ed25519 verification on this
build measures 2373 µs, roughly 50× a normal one; ban_signature_matches_current_key
is that verify plus 0.6 µs of serialization. The cause is Cargo.toml's
[profile.release.package."*"] opt-level = 'z', which size-optimises every
dependency including curve25519-dalek. Overriding just the two dalek crates to
opt-level = 3 gives 39 µs (61× faster) for +1.90% room-contract and
+2.54% delegate WASM. That would take this fix's overhead from 12.3% to
roughly 0.2% — and would speed up every signature check in the contract, not
just this one. Deliberately NOT done here: it re-keys, it trades against a
deliberate size policy, and it is far larger than this PR. Filed as #681.

Measurements

Every figure below was taken on the final head, after the rebuild. No number
from an earlier revision of this PR is carried forward. Baselines come from a
clean origin/main worktree rather than being quoted.

check	corpus	origin/main	this branch
commutativity	25 states, 300 pairs	133 failing	0 failing
commutativity	6 states, 15 pairs	5 failing	0 failing
non-idempotence	6 states, 30 results	5	0
associativity	6 states, 120 ordered triples	30 failing	0 failing
verify() on captured states	25 states	25 pass	25 pass, 0 fail
verify() on merge results	600 merges	600 pass	600 pass, 0 fail
mean DMs retained	600 merges	298.57	300.00
#675 deputy reproduction	synthetic	fails, 5 DMs lost	passes

Differing-field tallies are empty on this branch; on main they are
{"members+member_info+direct_messages": …} for both commutativity and
associativity — the same signature, which is what showed the two laws share one
root.

DMs the branch keeps that main dropped: 858 slots, 45 distinct messages.
DMs the branch drops that main kept: 0. Mean retained 298.57 → 300.00.

Across 133 of the 600 merges, median 8 per affected merge. Re-derived on this
head rather than inherited — the surviving DM signature set was dumped for all
600 ordered merges on origin/main and on the branch and the two diffed — and
it reproduces the review's independently-written measurement to the digit,
including the median and the affected-merge count. The mean reaching exactly
300.00 corroborates the shape: the fix keeps the right 300 rather than
under-filling.

One residual the sweep found, which is NOT this PR's

Checking the stronger property — is each merge result a post_apply_cleanup
fixpoint — 32 of 600 are not. Reported rather than omitted, because it would
otherwise look like this PR claims a clean sweep it does not have.

It is pre-existing and unrelated, and now filed as #676. The identical check
on the pre-remedy head gives the identical result (32 / 600, tally
{"recent_messages": 32}), the differing field is never direct_messages,
members or bans, and it is not a merge defect at all: those merges are
no-ops, the #[composable] macro short-circuits the whole apply_delta on
a bare top-level None so post_apply_cleanup never runs, and the captured
states already carry an unpopulated actions_state cache while holding action
messages. The messages Vec is byte-identical across passes; only the computed
cache differs, and it settles after one pass. See #676 for the full trace,
including why this class is structurally invisible to state_idempotence.

Harness provenance. The commutativity figures come from merge_law_replay
and the sweep from a purpose-written checker, both run against the built branch.
merge_law_simfix is an approximation of the fix (it sweeps inputs before
merging rather than inside apply_delta) and no headline number rests on it. The
harnesses live on the merge-law-commutativity branch and are deliberately not
in this diff — they need a corpus of real user state that is not checked in.

Testing

New tests in common/tests/dm_merge_law_test.rs, synthetic — no captured
fixture, the corpus is real user data and far too large to check in.

test	what it pins
cleanup_is_idempotent_when_a_member_is_held_alive_only_by_a_doomed_dm	(b): cleanup(cleanup(S)) == cleanup(S), and that the fixpoint is the correct one (M and Q gone), so "prune everything" would not satisfy it
dm_participants_are_still_exempt_from_inactivity_prune_when_both_are_live	the counterpart that keeps the above honest
global_cap_must_not_evict_legitimate_dms_for_doomed_ones	(a): exact surviving offset set; two premises asserted on the input so it cannot pass vacuously
whole_state_merge_is_commutative_for_the_671_shape	the corpus commutativity result as a synthetic pair
whole_state_merge_is_associative_for_the_671_shape	associativity, which the field-level tests cannot see
none_delta_path_sweeps_before_the_cap	the delta == None arm
cleanup_is_a_fixpoint_after_the_trim_discards_a_members_only_dm	the case that kills the "move the trim after step 6" reordering
deputy_ban_must_not_let_a_doomed_dm_evict_a_legitimate_one	#675 — a deputy-banned endpoint's DM must not take a cap slot either
dm_ban_derivation_must_apply_the_user_ban_cap	the apply-time ban derivation must apply max_user_bans exactly as step 0-cap does
none_delta_path_sweep_is_ban_aware	the delta == None arm's sweep must be ban-aware, not membership-only
field_declaration_order_puts_members_before_direct_messages	Pin A — direct_messages must be declared after members

Plus two debug_asserts, both compiled out of the release WASM:

• Pin B, at step 6: steps 2-5 are removal-only for members, which is what
  makes exemption ⟹ retention a subset argument.
• The equality assert, at the top of post_apply_cleanup: the ban set
  enforced_ban_set_of derived at DM-apply must EQUAL step 0's
  enforced_banned_ids. This is the invariant the whole Deputy-banned DM endpoint still wins a global-cap slot and evicts a legitimate message (residual of #671) #675 remedy rests on —
  if they diverge, the apply-time sweep deletes DMs step 6 would have kept — and
  it was left to prose. The guard above catches the known break at one site; the
  assert catches any break across every test that already runs. Measured: under
  the uncapped_bans mutation it also fires in two PRE-EXISTING tests
  (over_cap_ban_does_not_one_shot_remove and one in deputy_ban_test) that
  nobody wrote for this. And three DM-bearing cases added to
  post_apply_cleanup_is_idempotent_on_adversarial_states (banned counterparty,
  deputy-banned counterparty).

Mutation-verified. Every new test fails under a mutation of the code it
covers, and no test is carried without one:

mutation	fails
revert (b) to the unfiltered active_participants walk	cleanup_is_idempotent_…, and adversarial states C and D independently
move (a) to after trim_to_global_cap	global_cap_must_not_evict_…
remove (a) from the main arm only	global_cap_…, whole_state_merge_is_commutative_…, whole_state_merge_is_associative_…
remove (a) from the delta == None arm only	none_delta_path_sweeps_before_the_cap — and nothing else
move trim_to_global_cap into cleanup after step 6	cleanup_is_a_fixpoint_after_the_trim_discards_… — and nothing else
declare direct_messages before members	Pin A, plus pruned_sender_can_dm_when_bundling_rejoin_delta and global_cap_must_not_evict_…
uncapped_bans — skip the cap replication in enforced_ban_set_of	dm_ban_derivation_must_apply_the_user_ban_cap, plus the equality assert in two pre-existing tests. Killed 0 of 421 before this guard
membonly_none — empty ban set at the delta == None site only	none_delta_path_sweep_is_ban_aware — and nothing else. Killed 0 of 421 before
revert the whole fix	5 of the 7

Three findings worth stating rather than burying:

• The delta == None arm had zero coverage. Removing the sweep from that arm
  alone failed 0 of 415 existing tests. That arm is what converges an
  unnormalised state on the first update after a PUT, which is exactly the path a
  re-keying release depends on. It now has a test that fails on that mutation and
  no other.
• The associativity test as contributed was decoration. It passed with the
  entire fix reverted. It has been reshaped — all three peers saturated at the
  cap with disjoint DM windows, so the intermediate merge publishes a different
  horizon depending on the bracketing — and now fails, losing five legitimate DMs
  in one bracketing and not the other. The dead version and why it was dead are
  recorded in its doc comment.
• The pre-existing post_apply_cleanup_stays_idempotent_under_the_global_cap
  stays green under the (b) revert (as does all 25 of dm_global_cap_test), so
  the new idempotence test is doing work the old one did not.

common/tests/dm_global_cap_test.rs shows in the diff but is two renamed
doc-comment lines, not new coverage — it referenced the removed
active_participants by name.

Also confirmed: nothing outside the #[composable] macro calls the DM field's
apply_delta in production code, which is what the placement argument for (a)
rests on. (Field-level tests call it directly, as intended.)

Full suites green: river-core lib (254) and integration, river-ui --bins
(946), riverctl --lib (353). cargo fmt clean; no new clippy warnings.

Migration — re-keys the room contract AND the chat delegate

One rebuild, one re-sign. common/src/room_state{,/direct_messages}.rs compiles
into both WASMs.

river-core 0.1.20 → 0.1.21 is not cosmetic and had to be in this PR. 0.1.20
is already published, and release-riverctl.yml calls
publish_if_needed river-core, which sees the version on crates.io and skips. It
would then publish riverctl 0.2.15, whose cargo publish strips the path
dependency and resolves river-core from the registry — the unfixed crate,
without this fix and without the V32 registry entry. Green pipeline, broken
artifact; in-workspace builds use the path, so no CI job can see it, and
check-cli-wasm only gates the cli version. A version bump also re-keys both
WASMs on its own, so deferring it would have invalidated this whole migration
payload and forced a second one. 0.1.21 rather than 0.2.0: crates.io lists one
reverse dependency (riverctl, in this workspace), cli/Cargo.toml pins it exactly
so the compatibility range is moot, and the V31 precedent 394c27dd shipped a
genuinely breaking public type change as a patch bump.

• common/legacy_room_contracts.tomlV32 — predecessor room-contract code
  hash e765339b…, recorded before rebuilding, so a room dormant across the
  upgrade is still found by the backward probe (Identity/room import can't recover a room several contract-WASM generations behind (restores stale "old IDs") #292). common/src/migration.rs's
  registry value-pin updated to 32 entries / f6e6f995….
• legacy_delegates.tomlV31 — predecessor chat-delegate code hash
  a44c6401…; the UI's legacy_set_fingerprint pin updated to
  c32c326c1328de9a, verified by recomputing BLAKE3 over the registry rather
  than copied from the sibling PR.
• pointer-records.toml re-signed at version 2 against both new WASMs.
  Signing only; publishing is a separate step from main.
• New hashes: room contract a3e63c8c…, chat delegate c2e60638…. Both
  predecessor hashes are the bytes committed on main — what users' keys
  actually derive from — and are unaffected by anything this branch rebuilt.

What the V32 generation actually bundles — BOTH committed artifacts on main
were already stale. A canonical rebuild of unmodified main (40ca5b0) yields
room contract 9040b9dd… and delegate 8a39ab73…, against committed
e765339b… and a44c6401…. So this generation carries, beyond the #671 fix:
common/src/mention.rs gaining a public render_plaintext_transformed (#633),
the workspace 0.1.19 → 0.1.20 bump that was never rebuilt into the committed
artifacts (and now 0.1.21 on top), and a Cargo.lock move, freenet-migrate
0.5 → 0.6. None of it changes contract behaviour, but the published artifact is
not "main plus #671" and the registry entry should not imply it is. The #671 fix
alone does not move the delegate at all — its hash equals a canonical rebuild
of untouched main — so the delegate re-key is entirely that drift plus the
version bump.

Build provenance. Built only with the canonical co-build
(scripts/sync-wasm.sh, one cargo build --locked -p room-contract -p chat-delegate). Building the contract alone yields a different key, because
chat-delegate unifies river-core's ecies feature — Makefile.toml:46-56.
Verified byte-reproducible from a different absolute path.

Comments are not hash-neutral in this codebase, which is a finding this PR
made the hard way and has now written down. The contract WASM embeds panic
Location records — file and line — for direct_messages.rs,
configuration.rs, member_info.rs and util.rs. Inserting one comment line in
any of them shifts every panicking site below it and changes the contract key.
Measured, same co-build and same path, a single // probe line as the only
variable: 0ab72165… → 237a17ba….

Two consequences worth carrying past this PR, both now in
.claude/rules/delegate-migration.md, which previously implied only code changes
re-key:

• check-wasm-sync cannot catch a source/artifact mismatch — it compares the ui/
  and cli/ committed copies to each other, not to a build of the source. A
  docs-only push without a rebuild would ship an artifact that is not the build
  of its own tree, with every gate green.
• The companion result that registry edits ARE hash-neutral is narrower than it
  reads: it holds because of the #[cfg(feature = "migration")] gate, not
  because non-code changes are free.

The artifacts here were rebuilt once, from this branch's own final source, and
reproduce byte-identically at a different absolute path.

Migration safety, measured rather than assumed

A migration is a full-state PUT: it bypasses post_apply_cleanup and reaches
state through validate_state → verify alone, and DirectMessagesV1::verify
hard-rejects a state carrying a DM with a non-member endpoint. A real captured
state that failed the new verify would mean this re-key strands rooms.

captured states: 25 pass verify, 0 fail
merge results: 600 pass verify, 0 fail; mean DMs held 300

All 25 real captured Official-room states pass, and so do all 600 pairwise merge
results — the merge result being what a peer actually stores, and therefore what
a later migration PUT would carry. mean DMs held 300 also confirms the fix is
not reaching commutativity by quietly under-filling the DM set. This confirms
rather than establishes safety (verify is untouched here), but the hazard was
worth measuring rather than reasoning about.

Coordination with #672

#672 is open and also re-keys the contract, with its own V32 entry recording
the same predecessor hash — its registry digest f6e6f995… matches mine
independently. The two should share one publish and one migration: whichever
merges second must rebase, regenerate the WASMs, re-sign the pointer records, and
re-run review and CI on the rebased head, and the publish then happens once from
main. This branch is cut from origin/main, not stacked on #672.

Nothing has been published from here — no publish-river, no riverctl publish,
no pointer publish.

Closes#671
Refs #672, #675, #413

[AI-assisted - Claude]

https://claude.ai/code/session_016ZwzXP3vsR2CfA14BvjZAp

[sanity]

Associativity: 30 failing → 0

The sweep promised in the description has finished. merge(merge(A,B),C) vs
merge(A,merge(B,C)) over every ordered triple of the same 6-state live-corpus
sample (120 triples), baseline measured on a clean origin/main worktree:

[origin/main] associativity: 30 failing / 120 ordered triples
[origin/main] differing-field tally: {"members+member_info+direct_messages": 30}
[fix-671] associativity: 0 failing / 120 ordered triples

So the full picture on this corpus is:

	commutativity	non-idempotent results	associativity
origin/main	5 / 15 pairs	5 / 30	30 / 120 triples
this branch	0 / 15	0 / 30	0 / 120

This is worth having because #671's fdev verify-merge run reported 5
associativity violations alongside 12 commutativity ones, and the argument that
one change closes both was an argument rather than a measurement. It now is one:
the associativity failures carry the same members+member_info+direct_messages
field signature as the commutativity ones — same root, and the same fix closes
them.

Not a new claim about the fix, just the third law measured rather than assumed.

[AI-assisted - Claude]

[sanity]

Migration sweep finished: 600 / 600 merge results also pass the new verify

The companion check promised in the description:

captured states: 25 pass verify, 0 fail
merge results: 600 pass verify, 0 fail; mean DMs held 300

The second line is the one that was still running: every ordered pair of the
25 captured states merged (600 merges) and the result — which is what a peer
would actually store, and therefore what a later migration PUT would carry —
verified against the new contract. None fails.

mean DMs held 300 is a useful side reading: the merged states sit at exactly
the DEFAULT_MAX_DIRECT_MESSAGES cap, so the fix is not quietly under-filling
the DM set to reach commutativity. It reaches it by keeping the right 300.

This confirms rather than establishes migration safety — verify is untouched
by this PR and the change is removal-only inside apply_delta — but the whole
hazard was that a full-state PUT bypasses post_apply_cleanup and reaches state
through validate_state → verify alone, so it was worth measuring rather than
reasoning about.

[AI-assisted - Claude]

[sanity]

Full-tier review — 7 lenses, at f137daf6

Risk tier: Full, without question — a contract change on the convergence and state-authorization surface, in a deployed contract, that re-keys it and requires a migration for every live room.

Lenses that ran

Lens	Verdict
code-first	1 blocking (P1, release), 2 P2
testing	1 blocking (coverage), 3 mutations re-verified
skeptical	no blocking finding — could not break the sweep by construction or on 600 live merges
big-picture	1 medium (self-retracted), 4 findings
migration / re-key	1 blocking (P1, same as code-first), payload otherwise PASS
convergence-from-first-principles	falsified a published proof; produced the read-then-shrink enumeration
migration re-run at the new head	payload PASS

External model: attempted, unavailable, substituted

codex review --base origin/main was attempted twice and failed both times:

ERROR: You've hit your usage limit. … try again at Sep 7th, 2026 8:14 AM.

Per the standing rule the review was not skipped or delayed. Two additional independent Claude lenses were substituted — big-picture and convergence-from-first-principles — and the substitution is recorded here rather than left implicit. Both substitutes produced findings, including the falsification of a proof I had published on #671, so the pass was not a formality.

Blocking findings — both fixed in f137daf6

1. river-core was not version-bumped. This is the one that would have shipped. Workspace stayed 0.1.20, cli/Cargo.toml pinned =0.1.20, and 0.1.20 is already on crates.io and immutable. A reviewer downloaded the published crate:

river-core-0.1.20/legacy_room_contracts.toml → 31 entries, last = V31
grep -c e765339b… → 0

cargo publish rewrites the path dep to the registry dep, so riverctl 0.2.15 would have bundled the new room contract against a registry that omits the generation live right now — a cargo install riverctl build would derive the new key, probe back to a generation dead since 2026-07-30, and find no existing room, Official room included. Silent by construction: the workflow step is literally "skip if already on crates.io".

Treated as merge-blocking rather than publish-blocking because crates.io is the one irreversible step — a wrong 0.2.15 can be yanked but not replaced.

Fixed: 0.1.21 in all three places. Verified independently against the sparse index, queried exactly as publish_if_needed does: 0.1.21 absent, so it will publish.

2. The delta == None arm had zero coverage. A mutation removing the sweep from only that arm failed 0 of 415 tests. Fixed; the mutation now fails exactly one test and nothing else.

Correctness — the strongest evidence, from the full corpus

Re-measured on all 25 captured states (300 pairs, 600 ordered merges), not the 6-state sample:

	main	branch
commutativity	133 / 300 pairs fail	0
merge output is a post_apply_cleanup fixpoint	127 / 600 fail	0
verify on merge results	600 / 600 pass	600 / 600 pass
DMs the branch drops that main kept	—	0
DMs the branch saves	—	858 slots / 45 distinct real messages
mean DMs retained	298.57	300.00

The last row matters: the fix reaches commutativity by keeping the right 300, not by under-filling below the cap.

Idempotence is structurally closed, not merely closed on the observed defect. A reviewer enumerated all nine read-then-shrink pairs in post_apply_cleanup and showed pair 4/5 now closes in both directions for arbitrary reachable states — forward via required_ids survival, backward because step 6's member set (post-3) ⊆ step 1's (post-0).

The skeptical lens specifically chased and closed the "transiently smaller member set" cases: remove_banned_members on the pre-cap ban set, remove_excess_members on a transiently-over-cap union, and a partially-hydrated UI state. In all three the member is also absent from the final set, so the sweep causes earlier loss, never loss step 6 would not have caused.

Migration payload — PASS, verified against the live network

Not merely against origin/main:

• Canonical co-build in a throwaway clone at a different path reproduces both artifacts byte-identically. (-p room-contract alone yields a different key — chat-delegate unifies river-core's ecies feature. Only the co-build is canonical.)
• On-network pointer states read via fdev execute get: room v1 e765339b…, delegate v1 a44c6401….
• Bytes actually served by the live webapp: identical to both.
• Network, main and registry all agree. Recorded predecessors are what is deployed.
• Pointer records at v2, both signatures verify, single clean step over the v1 the network holds.
• Guard mutations fail correctly: deleting either registry entry fails its check.
• The added debug_assert! cannot reach the contract WASM — [profile.release] does not set debug-assertions and sync-wasm.sh builds --release. Worth stating because panic = 'abort' is set, so a compiled-in assert would have killed the room rather than failing softly.

Disclosed, not fixed

• The deputy-banned-endpoint residual — filed as Deputy-banned DM endpoint still wins a global-cap slot and evicts a legitimate message (residual of #671) #675 with a working reproduction. Pre-existing; origin/main fails identically. Reachable on the Official room's real moderation flow (owner-appointed moderators are grant 3, the deputy path), and measured at 5 legitimate DMs destroyed per occurrence.
• The stated blocker for fixing it is false. The PR argued the enforced-ban set is not knowable at DM-apply. A reviewer implemented it in ~30 lines — deputy repro 5 lost → 0, full suite green. The doc comment asserting impossibility is being corrected before merge; shipping it would have stopped the next person attempting something that demonstrably works.
• This change widens Ban cap-eviction hardening: transient cross-order divergence + un-ban DoS #413's blast radius from {bans, members, direct_messages} to also include {member_info, recent_messages}, because step 1 now reads enforced_banned_ids. Keeping the sweep membership-only is what stops that being worse and is what leaves Deputy-banned DM endpoint still wins a global-cap slot and evicts a legitimate message (residual of #671) #675 open — the same decision, and it should be read as a trade-off rather than as free.
• Both committed artifacts on origin/main were already stale — neither reproduces from main's own source — so this generation bundles fix(cli): escape attacker-controlled nicknames before terminal output #633's public render_plaintext_transformed and the version bump alongside the Room contract: whole-state merge is not commutative, and post_apply_cleanup is not idempotent (live Official-room state) #671 fix. Cause: no CI job rebuilds and compares. Filing separately.
• river#674 — the DM retention horizon goes stale mid-merge. Older than and independent of this PR; the membership sweep is pair-uniform and cannot cause it. This falsified a losslessness proof I had published on Room contract: whole-state merge is not commutative, and post_apply_cleanup is not idempotent (live Official-room state) #671, which I have retracted there.

Outstanding before merge

1. The doc-comment correction above (will move the head; CI must re-run).
2. Test re-verification at the new head, in progress.
3. fix: stop MemberInfo/message verify from permanently forking rooms #672 is also open and also re-keys, and carries the identical river-core defect (flagged there). Whichever merges second must rebase, append rather than replace the registry entry, regenerate via co-build, re-sign pointers at v3, and bump to 0.2.16 — then re-run review and CI on the rebased head. The two should share one publish.

CI: fully green on f137daf6 (build, playwright, delegate-migration, room-contract-migration, pointer-freshness, wasm-sync, CLA).

[AI-assisted - Claude]

[sanity]

Correction to my review above, and a pre-existing CI gate hole

Correction: #633 is NOT in this generation

My consolidated review said this generation "bundles #633's public render_plaintext_transformed and the version bump alongside the #671 fix". The #633 half is wrong.

render_plaintext_transformed lives in common/src/mention.rs, and that whole module is gated:

// common/src/lib.rs:10-11#[cfg(feature = "mentions")]pubmod mention;

mentions is documented in common/Cargo.toml as "deliberately OFF for the room-contract and chat-delegate WASM so their bytes (and keys) stay byte-identical", and neither WASM crate enables it — both take river-core.workspace = true with no feature additions. So it is not in the contract and could not have been.

I verified this myself rather than relaying it. The gating is working exactly as designed; I asserted the opposite without checking the feature flags, which is the same mistake I have flagged others for in this thread.

What remains true and unchanged: origin/main's committed artifacts do not reproduce from origin/main's source, so this generation does carry something beyond the #671 fix — most likely the version bump and the Cargo.lock move. I no longer claim to know precisely what, and the PR body should not claim it either.

Pre-existing gate hole: silently skips two clauses

Not introduced by this PR (git diff --name-only origin/main..f137daf6 -- scripts/ is empty) and it does not invalidate this payload — a reviewer verified monotonicity by hand: base v1 → head v2 for both records, network holding v1, and no record vanished. But it is worth knowing before anyone relies on that gate.

scripts/check-pointer-freshness.sh:114, inside the --ci block, reassigns TOML_PATH from the repo-relative "pointer-records.toml" to an absolute temp path. Two later clauses still feed it to git show:

173: if git show "$BASE_SHA:$TOML_PATH">"$BASE_TOML_ALL"2>/dev/null;then# "no record may VANISH"
269: if git show "$BASE_SHA:$TOML_PATH">"$BASE_TOML"2>/dev/null;then# clause 3, monotonicity

git show <sha>:/abs/path exits 128; 2>/dev/null swallows it; both if bodies are skipped and the script reports success. Always, in every --ci invocation — not data- or timing-dependent. In local mode both clauses are --ci-gated and do not run either, so neither mode performs either check.

Demonstrated rather than argued. Deleting the entire river.chat-delegate pointer record from head:

records remaining: 1
All 1 pointer record(s) are fresh, signed, and name the committed WASM.
EXIT=0

A whole pointer record vanished and CI stayed green — the exact scenario the gate's own header at :160-170 calls "the one way a pointer can go stale with CI green".

The monotonicity clause's consequence is the one that matters for releases: a record re-signed at a stale or equal version passes CI. Its own comment at :266-268 explains why that is bad — "A republish at an already-used version is a no-op SUCCESS on the network — the contract refuses to error on a stale update by design, so nothing downstream would ever tell us the release was ignored."

The tell that exposed it: the --ci run never printed version advanced 1 -> 2, despite base v1, head v2 and differing states — the one input that must produce that line.

Filing separately; not a blocker for this PR.

[AI-assisted - Claude]

[sanity]

Re-measured on the final head (f137daf6)

The head changed after the numbers in the description were taken — the review
round added the version bump, the pins, four tests and the doc corrections — so
per the per-code-content rule the corpus was re-run rather than the earlier
figures being carried forward:

[fix-671 @ f137daf6] commutativity: 0 failing / 15 pairs
non-idempotent: 0 / 30
associativity: 0 failing / 120 ordered triples

Identical to the numbers in the description. That is expected and is worth
stating precisely rather than just asserting: stripping comments and doc
comments, the executable diff between the commit those figures were measured on
and this head is empty for direct_messages.rs and, for room_state.rs, a
single hunk — the Pin B debug_assert!, which the release profile compiles out.
The shipped WASM and the harnesses both build release, so behaviour is provably
unchanged; the re-run confirms it rather than substituting for it.

CI on this head: build and ui-playwright-tests pass, along with
check-delegate-migration, check-room-contract-migration,
check-pointer-freshness, check-wasm-sync and the CLA.

[AI-assisted - Claude]

[sanity]

Independent artifact verification at 8c833270 — PASS

Run by the reviewer coordinating this work, not by the author, in a throwaway worktree at a different absolute path (/home/ian/code/freenet/river/verify-8c833270, since removed). Canonical co-build only — cargo build --locked --release --target wasm32-unknown-unknown -p room-contract -p chat-delegate, as scripts/sync-wasm.sh runs it. Building the contract alone yields a different key, because chat-delegate unifies river-core's ecies feature (Makefile.toml:46-56).

room_contract built a3e63c8c0426255376f9f6f00948512e…
commit a3e63c8c0426255376f9f6f00948512e… MATCH
chat_delegate built c2e6063899624b65715d683fbbba0463…
commit c2e6063899624b65715d683fbbba0463… MATCH

Both committed copies of the room contract (ui/public/ and cli/) are identical to each other. Tracked files unmodified by the build.

Why this was done by hand, and by a second party

No CI job compares a committed WASM to a build of its own source — filed as #678. Every gate is artifact-to-artifact or record-to-artifact: check-wasm-synccmps the two committed copies, the migration gates hash the committed blob across refs, and check-pointer-freshness compares the record to the committed bytes. A committed artifact that is not the build of the source beside it passes all of them.

That is not hypothetical here. Both artifacts on origin/main were already stale across two commits, undetected — and this PR's own commit message establishes that a single comment line re-keys the room contract, because the WASM embeds panic Location records carrying file and line.

It also mattered specifically at this head. The final commit added two tests, a debug_assert! and a docstring fix, and was predicted to be hash-neutral — room_state.rs is not among the files whose Location records reach the WASM, direct_messages.rs changed by one visibility keyword with no line-count change, and tests are not in the contract build. That prediction is correct, but it is exactly the case where a stale artifact would be indistinguishable from a correct one, so it wanted checking rather than reasoning.

And it was checked by someone who did not produce the build. As the author put it when declining to have it skipped on their behalf: a build verified only by whoever produced it has a correlated blind spot.

What this does not establish

Path-independence, not machine-independence. This build ran on the same machine as the author's. Nothing anywhere provides the latter, which is part of what #678 asks for.

Still a required manual step before publish

The recorded predecessor hashes (e765339b… / a44c6401…) were verified against the live network earlier in this review — the on-network pointer records via fdev execute get, and the bundle actually served by raAqMhMG…. Re-confirm that immediately before publishing, since nothing automated does.

[AI-assisted - Claude]

[sanity]

🔴 New blocker: riverctl 0.2.15 is already published — origin/main moved

origin/main advanced from 40ca5b05 to 73a6ef3f (#680, fix(cli): emit author_verifying_key on every message stream JSON event). That PR bumped riverctl to 0.2.15, and it is now on crates.io and immutable:

riverctl published: 0.2.11, 0.2.12, 0.2.13, 0.2.14, 0.2.15
origin/main cli/Cargo.toml: version = "0.2.15"

This PR also sets riverctl 0.2.15, so the merge result carries a version that is already taken.

Consequence — the same defect as the river-core one, from a different direction.publish_if_needed riverctl queries the index, sees 0.2.15, prints "already on crates.io — skipping", exits 0. riverctl is never republished. The CLI on crates.io stays the artifact #680 built, carrying the pre-fix room contract and a registry without the current generation. A cargo install riverctl build then cannot find the Official room. Green pipeline, wrong artifact.

Fix: bump riverctl to 0.2.16. river-core 0.1.21 is still unpublished and remains correct.

Then rebase onto 73a6ef3f and rebuild-and-compare rather than assuming the bump is hash-neutral. It should not reach the contract build — the WASM comes from room-contract + chat-delegate via river-core, not riverctl — but Cargo.lock changes, and this review has already established that a single comment line re-keys the room contract. If either hash moves, re-sign the pointer records.

Both #672 and #673 are affected, and both currently claim 0.2.15. Whichever merges second needs a further bump again, plus the registry append (not replace), regenerated artifacts via the co-build, and pointer records re-signed at the next version.

How this was found, since it is the interesting part

Not by checking this branch. A reviewer noticed origin/main had moved, then built the actual auto-merge result rather than the branch tip — parents 73a6ef3f + 8c833270, merge 008d3021. Git auto-merged Cargo.lock and cli/Cargo.toml, both of which carry release payload state. The merge itself was clean and reproduced both artifacts byte-identically with the =0.1.21 pin intact — but the version collision is only visible once you look at the merge, not the branch.

Verifying a branch tip does not verify what lands on main. Worth adding to the release checklist, alongside the fact that nothing in CI compares a committed artifact to a build of its source (#678) and that two check-pointer-freshness clauses never execute (#677).

[AI-assisted - Claude]