Skip to content

fix(migration): durable interrupted-migration marker (#760) + perf(delegate): byte fields as CBOR byte strings (#757) - #758

Closed
sanity wants to merge 8 commits into
mainfrom
perf/delegate-bytes-757
Closed

sanity wants to merge 8 commits into
mainfrom
perf/delegate-bytes-757

Conversation

@sanity

@sanity sanity commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Part of #757: fix 4 ("PR B"). Also fixes #760, a production data-stranding bug in delegate migration that the rehearsal for this re-key exposed. It ships here because the fix has to be live no later than the first migration into the new generation. PR A (fixes 1-3, UI only) is separate.

1. Delegate message byte fields as CBOR byte strings

ChatDelegateRequestMsg / ChatDelegateResponseMsg byte fields were serialized by ciborium as CBOR integer arrays. Every byte >= 0x18 costs 2 bytes, so delegate round-trips were ~1.9x their payload: a 1.18 MB stored room arrived as a 2.32 MB GetResponse on every page load.

What changes. The byte fields are now encoded through a small hand-rolled cbor_bytes helper in common/src/chat_delegate.rs:

  • requests: StoreRequest.value, CasStoreRequest.value, and all eight Sign* payloads;
  • responses: GetResponse.value, GetVersionedResponse.value, CasStoreResult::Conflict.current_value. These three Option fields also get #[serde(default)], because with drops serde's "absent Option is None".

What stays an integer array.

  • ChatDelegateKey. It is the one field sent to frozen predecessor delegates, and it is only a few dozen bytes.
  • SignResponse.signature (64 bytes).
  • No room-contract state type is touched.

Why not serde_bytes. Adding that dependency to river-core re-keyed the room contract (measured a3e63c8c… -> 48d91e7b…). A river-core version bump does the same (c29522a5…). With the hand-rolled helper and no version bump, room_contract.wasm stays byte-identical, so this is a delegate-only re-key and riverctl does not ship.

Compatibility, both directions, under ciborium 0.2.2. Every delegate generation in git embeds 0.2.2.

  • The new UI decodes legacy integer-array replies, because deserialize_byte_buf accepts an array.
  • A legacy delegate decodes byte strings, because its plain Vec<u8> goes through deserialize_seq, which accepts bytes.
  • Requests sent to predecessors (GetRequest, ListRequest) are byte-identical to before.
  • Data at rest is opaque bytes inside the versioning.rs envelope, so nothing stored changes.
  • riverctl does not use these types.

2. Interrupted migrations no longer strand rooms (#760)

This activates recovery logic that has never run in production. The interrupted-migration recovery (#345 follow-up: decide_per_room_load_action → re-run the legacy fill) keyed off a flag in localStorage. The gateway serves River in an iframe whose sandbox omits allow-same-origin, so localStorage throws SecurityError (confirmed in the rehearsal) and the flag was never written.

Reproduced with b5e08b0, before this fix: I killed the tab right after the first per-room CAS write of the migration. Every later load showed 1 of 4 rooms; the other 3 stayed stranded in the predecessor.

The fix: a marker in the successor delegate's KV store, __river_legacy_migration_in_progress__.

  • Write. It is stored once per session, acknowledged and retried once, before the first per-room write of a legacy re-save or current-blob explosion.
    • An unacknowledged store does not abort the re-save. The merged rooms are already in ROOMS, and other save paths (room sync, the walk's flush) write them anyway, so aborting would only make a complete set less likely.
  • Delete. Only the existing legacy fan-out quiescence seal deletes it, and only when no re-save is in flight and none failed.
    • Several generations re-save concurrently and share the marker, so it is never deleted when one of them finishes.
    • A recovery that finds nothing to add converges at the same seal.
    • The delete re-checks under a lock that no late re-save started.
  • Read. It is read from the current delegate's ListResponse, so there is no extra round trip.
    • A freenet-migrate walk wip marker without its done marker also counts as interrupted, because the walk's flush writes per-room keys too and is the only importer when the sweep's send fails.
  • Serialisation. Marker Store and Delete are serialised: they share one correlation slot in the single-waiter registry.
  • Session state. Session state is the pure, unit-tested MigrationMarkerState (present / in_flight / failed / written). The Import Identity should allow overwriting an existing room identity (warn, don't refuse) #414 identity-import gate reads its in_progress().
  • Forward copy. is_migration_marker_key excludes the marker, so it never travels to a later generation.
  • Cost. One extra Store per migrating session and one Delete at the seal. Nothing on non-migrating loads.

A recovery re-run never clobbers newer data. That covers a lost delete, a crash before the seal, and app use in between. The legacy copy is merged with MergeAuthority::OlderSnapshot (room_data.rs merge_from_source, #590), which is additive only:

  • It may add a room the live set lacks.
  • An identity conflict keeps the live identity, and messages are CRDT-merged.
  • A tombstone from the current delegate outranks a legacy Present, so a room left since stays left.
  • The save is per-room CAS read-merge-write.

a_recovery_rerun_only_adds_rooms_the_live_set_lacks pins this, and I watched it fail with resolve_identity_conflict forced to AdoptIncoming.

Not fixed here: #761. The legacy "done" seal is still dead localStorage, so an account with no rooms re-probes all 29 legacy delegates on every load. Making that seal effective means activating a permanent "never probe again" decision, which needs its own review.

Tests

common/src/chat_delegate.rs, against a legacy mirror of the pre-change types. The mirror has its own key type, so an encoding change to ChatDelegateKey would show up.

Test What it pins
every_byte_field_encodes_as_a_cbor_byte_string all 13 fields, checked on the decoded CBOR value; signature stays an array
byte_fields_decode_legacy_integer_array_responses / byte_fields_legacy_decoder_accepts_byte_strings both directions, for Some, empty and None
predecessor_bound_requests_are_wire_identical_to_legacy requests sent to frozen WASM are unchanged
get_response_golden_vectors, request_and_null_golden_vectors literal wire bytes
absent_optional_byte_field_decodes_as_none all three Option fields
legacy_array_… / byte_string_with_lying_length_header_errors_not_panics lying-length headers fail cleanly

UI tests:

  • MigrationMarkerState tests: overlapping re-saves keep the marker; a failed re-save blocks the seal; a recovery with nothing to add converges; store once and a late re-save cancels the delete; a listing never hides a running re-save; the import gate follows the state.
  • marker_store_outcome table (all 4 branches).
  • a_walk_wip_marker_without_done_counts_as_interrupted.
  • plan_load_from_keys with the marker.
  • a_recovery_rerun_only_adds_rooms_the_live_set_lacks.
  • Source pins: mark before save, no abort, end after save on both re-save paths; only the quiescence seal deletes the marker (needle built from parts so the pin can't match itself); the listing seeds the state before the plan.

Every guard was watched failing under a mutation.

Artifacts (rebuilt from source at the final head)

scripts/sync-wasm.sh's co-build gives chat_delegate.wasm ea5005e9… and room_contract.wasm a3e63c8c…, both identical to the committed files. Review lenses independently reproduced these hashes from git archive copies at b5e08b0 and 485d7a6. Later commits touch only UI code, tests and docs, and the rebuild at the final head is unchanged.

  • V32 entry. code_hash c2e60638… is the delegate inside the webapp currently live on the network: I fetched raAqMh… via 7509 and hashed contracts/chat_delegate.wasm. Its delegate_key is c15cbbb7… = BLAKE3(code_hash).
  • Pointer. The river.chat-delegate pointer record is re-signed at v3 for ea5005e9….
  • Crate versions. freenet-migrate and freenet-migrate-build versions are unchanged, and there is no Cargo.toml or Cargo.lock change.

Measurement (isolated network-mode node, real WASM)

  • Node: freenet network --is-gateway --skip-load-from-network (freenet 0.2.142), with no peers, its own dirs and telemetry off.
  • App frame: the app runs in the gateway's sandboxed iframe, where localStorage is unavailable, as in production.
  • Signing: every UI build was signed with the same throwaway container key (contract FDfXQ61p…). Delegate KV is scoped by the webapp origin, so this is what lets each build see the previous one's data, exactly as successive production publishes do. Delegate keys are the real production ones.
  • Account: 4 rooms (3 created in the UI with signing keys, plus 1 joined room with 70 messages of ~9 KB each, a ~1.2 MB delegate slot), one outbound DM and one inbound DM.

Steady-state WS bytes per page load. I ran two loads of each and they were identical:

up down
main UI (V32 delegate) 5,181,003 6,312,381
this PR (after migration) 5,183,850 3,794,589

Download falls by 2.52 MB per load (-40%), which matches the large room's ~1.2 MB slot being read twice per load (fix 2 in PR A removes the second read). Upload is unchanged; it is the room re-PUTs that PR A removes. The first load after the upgrade moves 6.49 MB up and 6.35 MB down, because legacy replies are still integer arrays.

Rehearsal (currently published delegate -> this PR)

The "old" UI was built from main 7b3ba90 and embeds the live delegate (c2e60638…). Its node dir E1ohZwYq… is base58 of V32's delegate_key. Before each upgrade, the successor dir 3tzJ9m93… was moved aside so it started empty. Each step names the build it ran on; 7cd6991 is the final code.

  1. First load after the upgrade (empty successor).
    • The sweep migrated per-room slots, signing keys and outbound_dms. The legacy 226-byte integer-array reply decoded: "Hydrated 1 outbound-DM entries … from legacy delegate".
    • The freenet-migrate walk reported each generation that holds data as Imported and sealed it. It also visited the absent generations (Unresponsive, not sealed) and did not stop on them.
  2. Populated successor (every later load). The rooms load from the new delegate. The steady-state measurement above comes from these loads.
  3. Rooms split across generations. I built the V31-era UI (8c0ca8c3^, delegate a44c6401), published it under the same container key, and created a room ("Delta") that only V31 holds. The upgrade recovered 5 rooms: Delta from V31 plus 4 from V32. Walk gens 27 and 28 were both Imported. So rooms stranded by an earlier interrupted migration come back on this re-key, because the first load visits every registered generation.
  4. Interrupted at the first per-room write (7cd6991).
    • The next load listed __river_legacy_migration_in_progress__, logged "Prior migration was interrupted — re-running", recovered all 5 rooms, and at quiescence logged "deleted the migration-in-progress marker".
    • The load after that has no marker and 5 rooms.
    • With b5e08b0 (before the fix), the same kill left 1 of 4 rooms permanently.
  5. Interrupted after both generations' re-saves finished but before quiescence (7cd6991). The kill came at 12.4 s, during the walk's import.
    • The next load still saw the marker, because it is no longer deleted per re-save. It recovered, then deleted the marker.
    • Both generations' re-saves coalesce into one save_rooms_to_delegate pass here (both finish at 6.37 s). So the strict "kill inside the second generation's own save" window did not occur on this node, and the pre-fix build did not lose rooms under the same kill. The overlap ordering is pinned by unit tests instead.
  6. Signing keys are in the NEW delegate (7cd6991).
    • On a fresh load, GetPublicKeyResponse … present came back for all 5 rooms, and EnsureRoomSubscriptionResponse ok: true for the 4 owned rooms.
    • Creating an invite issued a real SignMember (byte-string payload) to the new delegate WASM and got SignResponse ok: true, with no "Delegate signing failed" fallback.
  7. DMs (7cd6991). The outbound DM plaintext (stored only in outbound_dms) renders after migration, and the inbound DM decrypts.
  8. The V32 store is never written by migration. It did gain one 42-byte secret from its own room subscription when a room updated, which is pre-existing behaviour for any re-key.

Not exercised end to end: a walk-only import, where the sweep's send to a predecessor fails. I could not force that on a healthy node, so a_walk_wip_marker_without_done_counts_as_interrupted covers it at unit level.

Publish procedure (coordinator, from main after merge, in this order)

  1. UI only: cargo make publish-river, not publish-all. riverctl embeds only room_contract.wasm, which is byte-identical, so there is no riverctl release and no check-cli-wasm bump. Set up the build environment from the river-publish skill's Step 6: export PATH=$HOME/.cargo/bin:$PATH RUSTC_WRAPPER= CARGO_BUILD_RUSTC_WRAPPER=, check that dx --version matches dioxus 0.7.9, and grep the publish log for incompatible.
  2. Counter PR: land the bumped published-contract/contract-version.txt through its own PR (.claude/rules/river-publish.md Step 6) and merge it before step 3. The pointer script refuses a dirty tree and requires being on main with CI green.
  3. Pointer: from main, per river-publish.md Step 7:
    POINTER_NODE_PORT=<a network node of yours, NOT 7509> \
    POINTER_WASM=~/code/freenet/freenet-migrate/contracts/pointer-contract/pointer-v1.wasm \
      cargo make publish-pointer-records
    
    Its pre-flight checks that the record's hash (ea5005e9…) matches the delegate inside the live webapp, so it only passes after step 1.
  4. No river-core version bump. A bump re-keys the room contract. river-core 0.1.21 on crates.io keeps the old encoding until the next bump, which is wire-compatible both ways for ciborium users. FREENET.md now tells other-library integrators to accept both forms.
  5. Verify. On the live deploy, rooms are still listed, a second load shows no "migration-in-progress marker" log line, and the console reports "Attempting to migrate data from N legacy delegate(s)" on a first load, where N is the number of [[entry]] rows in legacy_delegates.toml (29 at this PR).

Also in this PR

  • ui/.gitignore. public/ excluded the directory, so git never evaluated the !public/contracts/*.wasm negations, and the documented git add … ui/public/contracts/ … exited non-zero. It is now public/*, !public/contracts/, public/contracts/*, plus the two WASM negations. Stray files in ui/public/contracts/ (e.g. an upgrade_assistant.wasm) stay ignored, and the two committed WASMs are addable.
  • Rules. .claude/rules/delegate-migration.md and river-publish.md say riverctl ships only for room-contract changes, and now record that a river-core dependency or version change re-keys the room contract. The add-*-migration.sh hints include the pointer re-sign step.

[AI-assisted - Claude]

https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

sanity added 2 commits October 9, 2026 10:38
…ngs (#757)

ChatDelegateRequestMsg / ChatDelegateResponseMsg byte fields (Store/CAS
values, Get/GetVersioned values, CAS conflict values, Sign* payloads)
serialized via ciborium as CBOR integer arrays, roughly doubling every
delegate round-trip: a 1.18 MB stored room arrived as a 2.32 MB
GetResponse on every page load.

They now encode as CBOR byte strings through a small hand-rolled
`cbor_bytes` helper that still decodes the legacy integer-array form,
so the new UI reads legacy delegates during migration, and ciborium's
plain Vec<u8> decoder already accepts byte strings, so legacy
delegates decode the new form too. ChatDelegateKey stays an integer
array: it is the one field sent to frozen predecessor delegates.

Hand-rolled rather than the serde_bytes crate because adding that
dependency to river-core re-keyed the room contract (measured); with
this helper room_contract.wasm is byte-identical (a3e63c8c...).

Delegate re-key: V32 registered in legacy_delegates.toml before the
rebuild, WASM synced, chat-delegate pointer record re-signed (v3),
UI pins updated.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE
…docs

- Test every changed field encodes as a CBOR byte string (not just
  GetResponse.value), add golden wire vectors, a lying byte-string
  header case, and an absent-field case.
- `#[serde(default)]` on the three `cbor_bytes::option` fields: `with`
  dropped serde's "missing Option is None". Response-only, so the
  delegate WASM is unchanged (still ea5005e9...).
- Correct the doc comments (SignResponse.signature and keys stay
  integer arrays; the 4096 cap is payload_bytes', not serde's).
- Rules: riverctl ships only for room-contract WASM changes; record the
  measured fact that a river-core dependency or version bump re-keys the
  room contract.
- ui/.gitignore: `public/` excluded the directory, so the `!public/contracts/`
  negations never applied and the documented `git add ui/public/contracts/`
  failed outright. `public/*` lets them apply.
- FREENET.md: note the byte-string change for non-ciborium integrators.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

@sanity sanity left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comprehensive PR Review: #758 (round 1)

Summary

  • Type: perf (wire format + chat-delegate re-key)
  • CI at reviewed head 69cd88f: all green (build, ui-playwright-tests, check-delegate-migration, check-room-contract-migration, check-pointer-freshness, check-wasm-sync)
  • Linked issue: #757 (fix 4)
  • Review tier: Full (wire format/serialization, delegate WASM re-key)
  • Reviewers run: code-first, testing, skeptical, big-picture, plus two extra Claude lenses: wire-compat/serialization, migration data-loss.
  • External model: Codex (codex review --base origin/main) failed with "You've hit your usage limit". It will be retried once on the re-review. If it is still down, the extra lenses stand in for it and this comment records that.

What the reviewers verified independently

  • The V32 entry is right: code_hash c2e60638… = BLAKE3 of main's committed chat_delegate.wasm, and delegate_key c15cbbb7… = BLAKE3(code_hash).
  • Every historical delegate WASM in git hashes to a registry entry.
  • Rebuilding from the PR tree gives room_contract.wasm a3e63c8c… (unchanged) and chat_delegate.wasm ea5005e9… (matching the commit, the UI pin and pointer record v3).
  • ciborium 0.2.2 is the only version any generation could have used; all 29 delegate blobs in git history embed it. Its deserialize_byte_buf accepts arrays and its deserialize_seq accepts byte strings.
  • The compat lens linked three river-core versions (PR head, main, Jan 2026) side by side and ran 148 checks: old→new and new→old for every changed field, Some/empty/None, tags, indefinite lengths, and a 1.18 MB value (2,249,412 bytes legacy vs 1,180,052 new).
  • Predecessors only ever receive key-only GetRequest/ListRequest, which stay byte-identical. Markers and CAS writes go to the successor only.
  • Data at rest is opaque bytes in the versioning.rs envelope. riverctl does not use these types.

Findings and dispositions

Must Fix

None.

Should Fix (all fixed in b5e08b0)

  1. Only GetResponse.value was pinned as a byte string. Dropping serde(with) from any other field would pass every test. Fixed: every_byte_field_encodes_as_a_cbor_byte_string checks all 13 fields on the decoded ciborium::Value (Bytes/Null). Watched it fail with SignBan.ban_bytes un-annotated. The PR body's "each watched failing (attribute removed)" was overstated for round 1; that is now true.
  2. Doc said "every Vec<u8> field", but SignResponse.signature and keys stay integer arrays. Fixed: the doc now names both exceptions and tells future fields to use the helper and the test.
  3. Rules contradicted a delegate-only re-key. delegate-migration.md and river-publish.md said riverctl must ship on any WASM change. Fixed: now "room-contract WASM", since riverctl embeds only room_contract.wasm.
  4. The dependency/version re-key finding lived only in a code comment. Fixed: new section in delegate-migration.md with measurements. Adding serde_bytes moved the room contract to 48d91e7b…; a river-core 0.1.21→0.1.22 bump moved it to c29522a5… (measured for this review, so the UI comment's claim is now a measurement).
  5. PR body lacked measurement, rehearsal and post-merge steps. Fixed in the PR body.

Consider

  • Absent value field. serde(with) turned a missing field into an error instead of None. Fixed: #[serde(default)] on the three Option fields, pinned by absent_optional_byte_field_decodes_as_none. These are response-only, so the delegate WASM is unchanged.
  • Wrong cap comment. The 4096 cap is payload_bytes', not serde's. Fixed.
  • Private intra-doc link. Fixed: the doc now uses plain code formatting.
  • "ListRequest" in the key note. Fixed: reworded.
  • Stale "27 entries … V30" comment. Fixed: now 29 entries, V1..V32.
  • Golden vectors and lying byte-string header. Added.
  • Missing pointer-records.toml in river-publish.md's git add. Fixed, and add-migration.sh's hint now includes sign-pointer-records.
  • Non-ciborium integrators. Added a note to FREENET.md.
  • river-core 0.1.21 on crates.io now differs from source. Noted in the PR body. This is deliberate: a bump re-keys the room contract (measured), and the change is wire-compatible both ways for ciborium users.
  • debug_assert that predecessors get only key-only requests. Not done. The invariant is already pinned on the wire by predecessor_bound_requests_are_wire_identical_to_legacy, and even if it broke, old delegates decode byte strings (ciborium deserialize_seq), so an assert would guard a failure that cannot lose data.
  • Duplicate helper vs payload_bytes. Kept separate. Sharing it would mean editing room_state/content.rs, and a line shift there re-keys the room contract. The new helper's doc points at payload_bytes.
  • Pre-existing, not this PR: a tab still running the old UI after another tab migrates keeps writing to the old delegate, and the old delegate's own room subscription keeps writing its member-set secret after the re-key. The rehearsal saw that second one: one 42-byte secret appeared in the V32 store when a room updated.

Found while fixing

ui/.gitignore had public/, which excludes the directory, so the !public/contracts/*.wasm negations never applied. The documented git add ui/public/contracts/ … therefore failed and staged nothing. Fixed with public/*.

Verdict

State: Needs Changes — Re-review Required After Fix. More than three Should Fix items were addressed and the diff grew by more than 30 lines, so a full re-review runs on b5e08b0.
HEAD SHA reviewed: 69cd88f

[AI-assisted - Claude]

sanity added 2 commits October 9, 2026 11:47
…t localStorage

The interrupted-migration recovery (#345 follow-up) keyed off
a "migration in progress" flag in localStorage. The gateway serves the app
in an iframe whose sandbox omits allow-same-origin, so localStorage is
unavailable and the flag was never written in production. Reproduced on
the #757 rehearsal node (sandboxed iframe, real WASM): closing the tab
right after the first per-room CAS write left 1 of 4 rooms on every later
load, the other 3 stranded in the predecessor delegate.

The marker now lives in the current (successor) delegate's KV store under
`__river_legacy_migration_in_progress__`:
- written and ACKED before the first per-room write of a legacy re-save
  or current-blob explosion; if the delegate does not ack it, the save is
  skipped (nothing written, so the next load migrates from scratch);
- deleted after a full successful save;
- read from the current delegate's ListResponse at load, which feeds the
  existing decide_per_room_load_action -> recovery path unchanged;
- excluded from the freenet-migrate walk's import via
  is_migration_marker_key, so it never travels to a later generation.

Recovery itself is unchanged: the existing registry-driven sweep plus the
freenet-migrate walk. No migration logic is added.

Also from the #758 round-2 review: absent-field test for
CasStoreResult::Conflict, request and null golden vectors, a pin that
SignResponse.signature stays an integer array; tighter ui/.gitignore so
stray files in ui/public/contracts stay ignored; pointer re-sign step in
add-room-contract-migration.sh's hint; hedged rule text.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE
With the durable in-progress marker, a crash mid-save or a lost marker
delete re-runs the legacy fill on top of rooms the user kept using. Pin
that the OlderSnapshot merge restores a stranded room, keeps a room
joined since, keeps a room left since left, and keeps the live identity.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE
@sanity sanity changed the title perf(delegate): encode delegate message byte fields as CBOR byte strings (#757) perf(delegate): byte fields as CBOR byte strings (#757); durable interrupted-migration marker Oct 9, 2026

@sanity sanity left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comprehensive PR Review: #758 (round 2, at b5e08b0)

  • Reviewers: code-first, testing, skeptical, big-picture, wire-compat and migration-data-loss. These were resumed from round 1, so each re-reviewed the full diff.
  • Codex: retried once and failed again with "You've hit your usage limit". The six Claude lenses stand in for it.
  • CI at b5e08b0: green.

Verified independently this round

  • Three lenses rebuilt the WASMs from git archive copies (69cd88f and b5e08b0) with the sync-wasm co-build. All got chat_delegate ea5005e9… and room_contract a3e63c8c…, identical to the committed files.
  • The compat lens re-ran its 151-check, three-generation probe. The absent-field behaviour now matches the old types, and no other wire bytes moved.

Findings and dispositions

Must Fix: none.

Should Fix:

  1. The migration lens's finding 4 was REAL, and is fixed in b730d0e. The interrupted-migration flag lived in localStorage, which is unavailable in the gateway's sandboxed iframe, so it was never written. On the rehearsal node, a tab killed after the first per-room write left 1 of 4 rooms permanently.
    • The marker now lives in the successor delegate. It is acked before the first write, deleted after a full save, and read from the listing.
    • The rehearsal now recovers all 4 rooms.
    • a_recovery_rerun_only_adds_rooms_the_live_set_lacks pins that a re-run (for example after a lost marker delete) only adds rooms. See the PR body, section 2.
  2. "Signing keys survived" was not proven by a posted message (migration lens). Fixed with real evidence. Against the NEW delegate:
    • GetPublicKeyResponse present for all 4 rooms;
    • EnsureRoomSubscriptionResponse ok for the 3 owned rooms;
    • a real SignMember returned SignResponse ok: true with no fallback.
  3. The rehearsal needed its localStorage state stated (migration lens). Done: the probe in the app frame reports unavailable: SecurityError, as in production.
  4. Conflict.current_value default was untested (testing lens). Added to absent_optional_byte_field_decodes_as_none; I watched it fail with default removed.
  5. .gitignore public/* un-ignored stray WASMs (skeptical lens). Fixed with public/contracts/* plus the two negations, and verified with git check-ignore --no-index.
  6. The PR body's publish steps were incomplete (big-picture and code-first lenses). Fixed:
    • the pointer environment;
    • the merge order (counter PR before the pointer);
    • the .gitignore explanation;
    • the rebuild hashes;
    • the legacy-count check, derived from the TOML.

Consider:

  • Request golden vector, null golden vector, SignResponse.signature pin. Added.
  • Mislabelled "populated successor" case. Relabelled.
  • Same throwaway key across builds. Stated: the scope is origin-based.
  • Hedging. The serde_bytes mechanism and the "byte-identical" generalisation in the rules are now hedged. "40-line" is now "~55-line". The river-core-bump exception is documented.
  • Stale hint. add-room-contract-migration.sh now includes the pointer step.
  • Users stranded by earlier re-keys (from the lead's question). Verified: a room held only by V31 plus four by V32 all came back on this upgrade, with walk gens 27 and 28 both Imported.
  • debug_assert for predecessor-bound request types. Still declined; the round-1 reason stands.

Verdict

State: Needs Changes — Re-review Required After Fix. b730d0e adds migration-critical code, so round 3 runs on the new head.
HEAD SHA reviewed: b5e08b0

[AI-assisted - Claude]

sanity added 2 commits October 9, 2026 12:00
…save

Round-3 review of #758:
- Several legacy generations re-save concurrently and share one marker.
  Deleting it when the first finished left a slower generation's rooms
  unprotected; a crash then would strand them again. The marker is now
  deleted only by the existing quiescence seal (schedule_legacy_seal),
  once no re-save is in flight and none failed.
- A recovery that found nothing to add never reached a delete, and the
  seal itself was gated on is_legacy_migration_in_progress(), which a
  merely-present marker kept true: the marker stuck forever. The seal now
  gates on MigrationMarkerState::may_seal (in-flight/failed only), so a
  recovery converges.
- Session state is a pure MigrationMarkerState (present / in_flight /
  failed) behind one mutex, unit-tested on local instances. This replaces
  a test that toggled the process-wide flag under parallel tests (flaky).
- A marker store that is not acknowledged no longer leaves the rail on
  "Migrating..." when rooms are already in memory.
- marker_store_outcome is a pure, table-tested reply interpretation;
  plan_load_from_keys is tested with the marker; source pins cover
  mark-before-save, abort-on-failure and end-after-save on both re-save
  paths, and that only the seal deletes the marker.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE
Round-3 review of #758 (#760):
- Marker Store and Delete share one bare-key correlation slot in the
  single-waiter registry; they are now serialised behind MARKER_OPS, and
  the store happens once per session (later re-saves reuse it).
- The delete re-checks under that lock that no late re-save started (and
  stored the marker) after the seal decided to delete.
- An unacknowledged marker store no longer aborts the re-save. The merged
  rooms are already in ROOMS and other save paths (room sync, the walk's
  flush) write them regardless, so aborting only made a complete per-room
  set less likely. The store is retried once.
- A freenet-migrate walk wip marker without its done marker now counts as
  an interrupted migration: the walk's flush writes per-room keys too, and
  it is the only importer when the sweep's send to a predecessor failed.
- Tests: store-once and late-resave delete cancel, import gate follows the
  marker state, walk wip-without-done detection. Rule doc updated.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE
@sanity sanity changed the title perf(delegate): byte fields as CBOR byte strings (#757); durable interrupted-migration marker fix(migration): durable interrupted-migration marker (#760) + perf(delegate): byte fields as CBOR byte strings (#757) Oct 9, 2026

@sanity sanity left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comprehensive PR Review: #758 (round 3, at 485d7a6)

The six lenses from round 2 re-ran on the full diff: code-first, testing, skeptical, big-picture, wire-compat and migration-data-loss. Codex was still unavailable ("usage limit"), so the Claude lenses stood in for it.

Findings and dispositions (fixed in d85e386 and 7cd6991)

Must Fix (testing lens). A test toggled the process-wide marker flag while another test asserted it was false, which is a parallel-test race.
Fixed: session state is now a pure MigrationMarkerState, tested only on local instances.

Should Fix and elevated items (all lenses):

  1. A marker shared by concurrent generations was deleted on the first save's completion (migration, testing, skeptical, code-first). Fixed: only the existing quiescence seal deletes it, and only when nothing is in flight and nothing failed. The delete re-checks under a lock.
  2. A stale marker never converged. A recovery with nothing to add never deleted it, and the seal was gated on is_legacy_migration_in_progress(), which a merely-present marker kept true. Fixed: the seal now gates on in-flight/failed only.
  3. A reconnect listing could clear the flag mid-migration (skeptical, migration, code-first). Fixed: a listing can no longer hide a running re-save.
  4. Abort on a failed marker store. The premise was false, because other save paths write the merged rooms anyway, and the abort left "Migrating…" on screen and the import gate shut (skeptical, migration, big-picture, code-first). Fixed: the re-save always runs; the store is retried once.
  5. Store and Delete shared one correlation slot (skeptical). Fixed: they are serialised behind MARKER_OPS, and the store happens once per session.
  6. The walk's flush wrote rooms with no marker (migration). Fixed: a walk wip marker without its done marker counts as interrupted. This uses the crate's own durable markers.
  7. Marker paths were covered only by source scrapes (testing). Fixed:
    • a table test for marker_store_outcome;
    • plan_load_from_keys tests with the marker;
    • state-machine tests;
    • pins on both re-save paths;
    • an "only the seal deletes" pin whose needle is built from parts so it can't match itself.
  8. Docs.
    • Fixed: the stale localStorage comments, the rule doc, the publish env citation, and the PR title and body, which now say this activates recovery that was dead in production and give the extra round-trip cost.
    • Filed: #760 for the stranding bug (fixed here) and #761 for the done seal, which is also dead localStorage and is out of scope; the reason is in the issue.

Consider:

  • Added: the "never rename" note on the marker constant, a kept-room state check in the recovery merge test, and the import-gate test.
  • Done: the rehearsal re-ran on 7cd6991 with interrupt-and-reload, an interruption after both generations' saves, keys and signing, and DMs. The literal "kill inside the second generation's save" window does not occur, because the two generations' saves coalesce into one pass. That is stated in the PR body, and unit tests cover the overlap ordering.

Verdict

State: Needs Changes — Re-review Required After Fix. Round 4 runs on 7cd6991.
HEAD SHA reviewed: 485d7a6

[AI-assisted - Claude]

Only conflict: the chat_delegate import list in response_handler.rs
(main added claim_list_load / request_outbound_dms_once /
seed_saved_slots_from_load and still imported the localStorage-era
clear_legacy_migration_in_progress, which this branch replaced).

Merged rather than rebased so the PR branch never needs a force-push.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE
@sanity

sanity commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Converted to draft. This is blocked on migration completeness in freenet-migrate: freenet/freenet-migrate#37.

Round 4 (6 fresh Claude lenses; Codex was out of quota, so they stood in) found no problem with part 1, the CBOR byte-string wire change: cross-decoding was verified in both directions, and the delegate and room-contract hashes rebuilt from source. Part 2, the durable interrupted-migration marker for #760, still decides "migration complete" in-session. That decision fails in several durable-state cases, detailed in freenet-migrate#37:

  • an undecodable predecessor item leaves the walk's wip marker forever;
  • a timed-out slot fetch lets the seal delete the marker;
  • an absent generation's instant "not found" can trigger the seal before V32 answers;
  • reconnect listings can raise or lower the session state.

The fix is a verifiable expected-set completeness mechanism, and it belongs in freenet-migrate rather than in River. Shipping this re-key before that lands would expose every user to #760, so the branch is kept as-is (head 51366d9, which includes PR A #759 via a merge).

When the crate support lands, this PR needs to:

  • adopt it, replacing the River-side marker in part 2;
  • re-run the rehearsal on that build;
  • go through a fresh Full-tier review.

[AI-assisted - Claude]

sanity added a commit that referenced this pull request Oct 12, 2026
…key after V33 (#757, #592)

Carries PR B of #757 from #758 (without its River-side interrupted-migration
marker, which the freenet-migrate journal replaces, #763):

- ChatDelegateRequestMsg / ChatDelegateResponseMsg byte fields encode as CBOR
  byte strings through the hand-rolled `cbor_bytes` helper, which still decodes
  the legacy integer-array form. ChatDelegateKey stays an integer array.
- chat-delegate key index (#592): an index that is stored but does not decode
  is an error, never an empty index; store / CAS / list / delete read the index
  before writing anything; delete rewrites the index only when it lists the
  key, and before removing the value, so a failed index write can never leave
  a listed key with no value.
- V33 (81b562f8..., the river-core 0.1.22 delegate from #772)
  registered in legacy_delegates.toml BEFORE the rebuild. V32 (c2e60638...)
  came from main. The WASM rebuild, pointer re-sign and delegate pin are in
  the branch's final build commit.
- ui/.gitignore: `public/*` so the contracts negations apply; rules and
  add-*-migration.sh hints from #758's review.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz
sanity added a commit that referenced this pull request Oct 12, 2026
…key after V33 (#757, #592)

Carries PR B of #757 from #758 (without its River-side interrupted-migration
marker, which the freenet-migrate journal replaces, #763):

- ChatDelegateRequestMsg / ChatDelegateResponseMsg byte fields encode as CBOR
  byte strings through the hand-rolled `cbor_bytes` helper, which still decodes
  the legacy integer-array form. ChatDelegateKey stays an integer array.
- chat-delegate key index (#592): an index that is stored but does not decode
  is an error, never an empty index; store / CAS / list / delete read the index
  before writing anything; delete rewrites the index only when it lists the
  key, and before removing the value, so a failed index write can never leave
  a listed key with no value.
- V33 (81b562f8..., the river-core 0.1.22 delegate from #772)
  registered in legacy_delegates.toml BEFORE the rebuild. V32 (c2e60638...)
  came from main. The WASM rebuild, pointer re-sign and delegate pin are in
  the branch's final build commit.
- ui/.gitignore: `public/*` so the contracts negations apply; rules and
  add-*-migration.sh hints from #758's review.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz
sanity added a commit that referenced this pull request Oct 12, 2026
#757 byte-string re-key (#768)

* perf(delegate): byte fields as CBOR byte strings, loud key index; re-key after V33 (#757, #592)

Carries PR B of #757 from #758 (without its River-side interrupted-migration
marker, which the freenet-migrate journal replaces, #763):

- ChatDelegateRequestMsg / ChatDelegateResponseMsg byte fields encode as CBOR
  byte strings through the hand-rolled `cbor_bytes` helper, which still decodes
  the legacy integer-array form. ChatDelegateKey stays an integer array.
- chat-delegate key index (#592): an index that is stored but does not decode
  is an error, never an empty index; store / CAS / list / delete read the index
  before writing anything; delete rewrites the index only when it lists the
  key, and before removing the value, so a failed index write can never leave
  a listed key with no value.
- V33 (81b562f8..., the river-core 0.1.22 delegate from #772)
  registered in legacy_delegates.toml BEFORE the rebuild. V32 (c2e60638...)
  came from main. The WASM rebuild, pointer re-sign and delegate pin are in
  the branch's final build commit.
- ui/.gitignore: `public/*` so the contracts negations apply; rules and
  add-*-migration.sh hints from #758's review.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* feat(ui): freenet-migrate 0.7 (git pin), port the contract probe to 0.6 semantics

Moves the UI from freenet-migrate 0.5 to 0.7.0 as merged on
freenet/freenet-migrate main (rev 4a5c4f10a7). 0.7.0 is unpublished; its
publish is gated on River's real-node rehearsal of this branch
(freenet/freenet-migrate#43). Switch to crates.io "0.7" before merging.

0.6 (between) changed the contract half: `on_timeout` is deprecated and
forwards to `on_unknown`, and an unknown now HALTS `NewestFirstWins`. River's
backward probe sees every miss as a watchdog timeout, so it would have stopped
at the first generation and never recovered a dormant room. Ported the way
riverctl did: one `new_probe_driver` with `continue_past_unknown`, the
watchdog reports `on_unknown`, and `Outcome::Indeterminate` seeds the local
snapshot exactly as the pre-0.6 `SeedLocal` did for the same answers (a pure
`probe_finish`, unit-tested). An unknown `Outcome` variant does nothing.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* feat(ui): adopt freenet-migrate's completeness journal (#763)

An interrupted delegate migration could strand rooms for good (#760): the
in-progress flag meant to catch it lived in localStorage, which the
gateway's sandboxed iframe does not have. The crate's durable journal,
stored in the current delegate, replaces every in-session guess.

- RiverSuccessorIo implements JournaledSuccessorIo: the journal under
  MIGRATION_JOURNAL_KEY, load Err on silence, store awaits the ack,
  import_revision 1 (raised with the byte-string decoder).
- Marker contract: a data in-progress marker beside an empty done marker
  reads as in progress (rule 1); done/wip are separate keys (rule 2); the
  wip marker is written by compare-and-swap so neither this session nor a
  concurrent one downgrades data to empty (rule 3).
- fetch_secrets never returns a short list (#592): raw index read twice,
  refused if undecodable, if the reads or the listing disagree, if absent
  beside a stored fixed key, or if a listed key answers "no value".
- probe_verdict: a TYPED missing delegate is NotInstalled (counts toward
  the absent streak); silence and the untyped wording stay NoReply.
- migration_journal.rs drives it: begin on every load from the current
  listing; the journaled walk while incomplete, whatever the load plan,
  rate-limited (#761) only while every pending generation is typed-absent;
  every sweep merge gated by note_app_import_started (Flagged -> merge,
  AlreadyImported -> merge unflagged through the ranked merge, Err or
  unknown -> skip), with an item count for the per-room sweep; all journal
  calls serialised; the walk re-run after a sweep flag.
- Identity-import gate composed from the journal (walk in flight, status
  not read, begin failed, status None, interrupted_import, rejected items),
  with a user override that ignores, never resolves, a blocking answer.
- Removed the localStorage "done" seal and in-progress flag entirely, so
  the journal is the only authority.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* docs(rules): the migration journal is the only migration authority (#763)

river-publish.md: per-room recovery is decided from the journal, the
localStorage seal and in-progress flag are gone, every sweep merge is
journal-gated, and a new section describes the journaled walk (every load
while incomplete, the #761 cadence, what lives in the current delegate's
KV, the marker contract, the loud fetch, the identity-import gate,
RIVER_IMPORT_REVISION). The #253 trade-off paragraph is replaced by why the
every-load walk cannot clobber newer state.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(ui): never walk or re-run the sweep over a room set that failed to load

The journaled walk now runs on every load while the journal is incomplete,
not only behind an empty current delegate. Merging older copies into a room
set this load could not fully read, then saving, lets the save keep the
in-memory identity of a room whose stored slot failed to load and overwrite
it (#595's chain). So the walk driver, and the per-room
recovery re-run, skip a load attempt that saw a room fail to load; the
journal keeps everything pending and the next attempt retries.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* test(ui): the #768 testing review's probes (driver env, session transitions, flake fix)

From the testing lens of the river#768 review (r768-probes-ui-common.patch):
- all_absent_status takes a fresh key pair per call: NOT_INSTALLED is
  process-global and two tests sharing one pair raced (67 of 300 paired runs);
- the walk driver runs against a WalkEnv, so its branches (fetch-failure gate,
  the bounded wait, the cap, a mid-walk poke) are natively testable;
- JournalSession transitions are pure methods; sweep_write_allows_merge;
- probes: fresh-generation CAS retry, cadence counter, marker read faults,
  permanent vs retryable item failures, refused/misaddressed store acks,
  the gate across a page load, room-scale byte fields, first-attempt claim.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* chore(deps): re-pin freenet-migrate to ff457290 (freenet-migrate#49)

The rehearsal and the 0.7.0 publish use this exact SHA.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(delegate): refuse an absent index read beside a stored key (#592 residual)

freenet-core reports a failed secret read as "not found" (#5857), so an
index read that comes back absent was still taken as an empty index: store
and CAS rewrote a one-key index over the real one, and list answered "no
keys" (#595 route 2). The index now reads as absent only when none of the
keys every River delegate stores early (rooms_meta, rooms_data,
outbound_dms) is present; otherwise store, CAS, delete and list refuse.

Takes the #768 testing review's SecretStore seam and its 7 handler tests
(fake store: write order, refusal over an undecodable index, delete
semantics), plus a test for the absent-index refusal on every path.

Value-then-index order is kept on store/CAS: index-first would turn a failed
value write into a listed key with no value, which the loud migration fetch
and the per-room load treat as a fault on every later load; the current order
leaves an unlisted value that the next save of that key re-indexes.

WASM is rebuilt in a later commit of this PR.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(ui): walk only over a current room set this load attempt read in full (#768 review A1)

- A listed slot answering no value, and any rooms_meta failure (decode, no
  value, transport), now mark the load's fetch failure: both are read faults,
  so the room set is incomplete.
- Per-attempt 'room set complete' latch (mark_room_set_complete), set by a
  clean per-room load, an empty listing, or a whole blob read.
- The walk driver captures the attempt before its wait and runs only if the
  load settled (a load still pending at the 60 s cap aborts), the attempt did
  not change (a reconnect/Retry zeroes the counters it waited on), and the set
  is complete; otherwise it re-arms the load and does not spend the walk
  budget.
- LiveImportSeam re-checks room_set_complete(attempt) before every merge and
  its flush, so a failure or reconnect DURING a walk refuses retryably:
  nothing merged, saved or sealed over an incomplete set.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(ui): retry the sweep's journal gate, mark a refused merge, skip walk replies (#768 review A2, A3)

A2: a journal refusal is retried with bounded backoff (1 s, 3 s) before the
sweep skips. A final skip marks the load's fetch failure in all three sweep
arms (UI shows Retry, not a false 'no rooms'; the walk does not run over the
set), and holds back every OLDER generation for the page load, so the sweep
never saves an older copy past a newer generation whose data it held but
could not merge (the cross-load shadowing of freenet-migrate#42).

A3: the legacy rooms_data and outbound_dms arms skip a reply that completed
a walk waiter, as the ListResponse arm already did: it is the walk's import.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(ui): loud fetch always probes rooms_meta, refuses a stored unlisted key (#768 review A4)

- rooms_meta is always probed (last): every per-room-era save writes it, so
  an absent index beside it is a read fault, not an empty delegate (probe P2).
- A probed key that is stored but missing from an index that exists proves
  the index short: a frozen generation's corrupt index became empty and its
  next store rewrote a one-key index, so listing and index agree and are both
  short (probe P1, #592). Refused, never certified.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(ui): persist each migrated identity's legacy source so a newer generation still wins (#768 review B3)

Under Union a silent newest generation lets an older one's identity be
imported and saved first. On the next load that room reloaded at the CURRENT
delegate's rank, so when the newer generation (V32) answered, its newer
self_sk lost the identity conflict for good.

rooms_meta now carries identity_sources: for each room whose identity a
LEGACY generation supplied, that generation's delegate key (not its registry
index). The save fills it from the rank registry; the per-room load restores
the rank after hydrate's merge, only lowering a rank the load itself just
recorded (never an explicit choice). Omitted when empty, so a meta without
legacy identities serializes byte-identically; older builds ignore it.

Tests: the cross-load scenario through the real walk (newer key wins; the
control without restore keeps the older key), persist/restore unit rules,
meta backward compatibility, and a wiring pin. Module docs corrected: the
'nothing is lost' claim was wrong; a newer generation's tombstone across a
load remains a documented residual.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* feat(ui): bounded post-resolve top-up from the newest predecessor (#768 review B1)

During a rollout an old tab still on the previous build keeps writing to what
is now the newest predecessor (V32), and once the journal resolves V32 the
walk never reads it again: a room created there was never imported.

For a window after the newest predecessor resolves (14 days or 30 loads,
whichever ends first; its state persisted in the current delegate under
__river_migration_topup_window__, a bookkeeping key), each load re-lists it
and merges every room the successor does not know at all (neither present nor
tombstoned), plus its outbound-DM cache, through the ranked, tombstone-safe
merge, unflagged. It runs inside the walk's run, so it shares the walk's gate
(a complete room set for the attempt) and the seam's re-checks. Additive and
idempotent: an interrupted top-up is retried on the next load in the window.

Tests: an old tab's room is imported and a room deleted in the new build is
not resurrected (and a tombstone outranks it even when offered); the window
runs then ends by loads and by time; the newest-resolved check.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(ui): anchor and nudge the import modal's signal reads; stale docs; Import-anyway test (#768 review A7, A8, A6)

A7: the identity-import modal reads the signal-guard anchor first and nudges
on every degraded read (ROOMS, JOURNAL_GATE_VERSION, ROOMS_LOAD_STATE), per
dioxus-signal-safety.md, so a contended read cannot leave it unsubscribed
with Import disabled. Pinned in source.

A8: module docs (29 generations; the adapter needs nothing new from the
delegate, the re-key is for #757/#592; markers vs the dead localStorage
seal; the top-up exception), error.rs DelegateMissing, the
on_current_delegate_missing, MarkDone and fire_legacy_migration_request docs,
and the per-room load's case list no longer describe a seal.

A6: a behavioural test for the Import-anyway branch: a blocked gate opens for
the page load, nothing is resolved, and a running walk still waits.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* docs(rules): the room-set gate, sweep retries, post-resolve top-up, identity sources (#768 review)

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(delegate): crash between any two key-value writes can wedge nothing (#768 review 7)

A crash (node restart) between the value write and the index write of a
store could leave a witness key stored with no index, and the absent-index
refusal then locked the user out for good.

- The (empty) index is created before any value is ever written, so a
  stored key can never exist without one; an absent index beside a stored
  key stays a refused read fault (the A5 guarantee).
- A store/CAS of a NEW key and a delete of a listed key record an intent
  under <origin>:::kv_intent first, then write, then clear it. Every
  key-value operation first recovers a recorded intent (open_kv): an
  interrupted add indexes the key if its value landed, an interrupted delete
  finishes. Recovery is idempotent; writes to a listed key are one write.
- Host write failures are checked on every path through the SecretStore
  seam (natively the context answers true, as it silently did before).

Tests: crash after each individual write of store (new witness key and
listed key), CAS, delete, and the first load (list + first witness store);
after each, list/store/CAS/delete succeed, every listed key is readable,
every stored value is listed, and acknowledged keys survive. Plus the A5
refusal still holds and recovery never rewrites a short index.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* test(ui): the key-index registration pin follows the crash-safe write path

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(ui): the walk's gate reads only the current delegate's load; latch after hydration (#768 round 2 items 1, 2, 5)

1. room_set_complete no longer reads the attempt-wide SAW_FETCH_FAILURE,
   which legacy probe errors and a refused sweep merge also set: a legacy
   skip marked the failure, the failure gated the walk the skipped data was
   left to, and Retry hit the same page-scoped hold-back again. The latch is
   now the only input, and it is set only by a clean read of the CURRENT
   delegate. The sweep's hold-back of older generations is removed (it
   blocked the walk; B3's persisted identity sources cover the identity
   half); the gate retry with backoff stays, and a refused merge still marks
   the failure for the Retry UI.
2. The latch is set from a defer queued after hydrate's (deferred) merge and
   the identity-rank restore, on the per-room and the blob paths, so it never
   vouches for a set still being hydrated.
5. Each deferred room and DM mutation in LiveImportSeam re-checks
   room_set_complete(attempt) before touching a signal and answers Err
   through the oneshot for a superseded attempt.

Also: the delegate's ::kv_intent record is migration bookkeeping.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(ui): keep another tab's identity provenance through the rooms_meta save (#768 round 2 item 3)

reconcile_meta now keeps stored identity_sources for every room this tab
does not represent (present or tombstoned in its own Rooms), so a concurrent
tab's provenance for a room it just migrated survives this tab's save. For a
room this tab holds, its own view wins.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(ui): carry an imported rooms_meta's identity provenance into the rank registry (#768 round 2 item 4)

When this delegate itself becomes a predecessor, its rooms_meta records
which OLDER generation supplied each room's identity. The walk (via the new
RiverImportSeam::lower_identity_ranks, after the predecessor's rooms merged)
and the sweep's legacy per-room path now lower those rooms' ranks to that
provenance, instead of re-stamping every room at the predecessor's rank, so a
generation newer than the provenance still wins.

Test: a predecessor carrying gen0's identity with provenance, beside gen1's
newer identity: gen1's wins.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(ui): the post-resolve top-up merges DM entries, never hidden threads (#768 round 2 item 6)

Re-merging the newest predecessor's whole outbound_dms on every load of the
window re-archived threads the user had un-archived in the new build. The
walk imported the predecessor's hidden threads once; the top-up now carries
DM entries only.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(ui): the top-up window counts only runs, restarts when the predecessor changes, and never forces a walk (#768 round 2 item 8)

- The window counted a load at decision time, even when the top-up never
  ran (a silent predecessor, an error, a gated attempt re-armed), so a
  flaky slot could burn all 30 loads with no top-up. A run is now counted
  only after the top-up succeeded, and the decision is made under the
  journal lock inside the driver run, after the room-set gate passed.
- Each load reads the newest predecessor's raw key index (one GetRequest)
  and records its BLAKE3 digest; a changed index (an old tab added or
  removed a key) restarts the window, even after it ended.
- A silent predecessor's ListRequest is an error, not a run of zero.
- The top-up is its own driver request (poke_topup): it no longer forces a
  full walk on every load of the window (bypassing the #761 cadence), does
  not spend the walk budget, and does not hold the import gate Pending.
  A gated attempt clears both requests; the next attempt decides afresh.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* test(ui): a top-up whose listing goes unanswered is not counted (#768 round 2 item 8)

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(ui): refuse a short index that lists rooms_meta when a room it names is stored unlisted (#768 round 2 item 9)

The common #592 shape: a frozen generation's corrupt index became empty and
its next rooms_meta store rewrote it as a one-key index that lists
rooms_meta, so the fixed-key check passed while room A (and its self_sk)
was stored but unlisted, and the journal sealed it away. The loud fetch now
GETs room:<vk> for every room the predecessor's rooms_meta names (order,
notification modes, cursor, identity provenance) that the index does not
list, and refuses the export when one is stored. A named room whose slot is
genuinely gone is not a fault.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* fix(ui): the identity import waits, overridably, while a predecessor that may hold rooms has not answered (#768 round 2 item 10)

P4 (probe R4): with the newest generation silent and the current delegate
empty, the gate opened, so a backup import or an accepted invite took
authoritative rank and that generation's newer key lost for good when it
answered. The gate now reads a new ImportGate::Unresolved while ANY pending
predecessor has not been found definitively not installed (absent_streak 0:
silent, or answered with a refused export). It waits like Pending (the
import stays disabled through rooms_recovery_in_progress), but the user may
override it with "Import anyway", since a generation this node cannot run
may never answer. Generations found not installed do not gate, so most
nodes are unaffected after their first walk. Blocking inputs still read
Blocked.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* test(ui): pin that a newer generation's tombstone wins across a page load; correct the B3 doc (#768 round 2, Consider)

The honest-bounds note said a newer generation's tombstone answering on a
later load does not remove a room an older generation supplied. With the
restored identity rank it does; the test pins it and the doc now says so,
and records the remaining bound (the losing identity is discarded, not
archived).

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* docs: the journal section matches round 2 (#768 round 2 item 11)

Documents the end state of a predecessor refused for a short index (pending
for good, its unlisted rooms do not migrate, the user is told only in the
identity-import modal), the current-delegate-only room-set latch and the
deliberate permanent gate on an unreadable slot, the dropped sweep hold-back,
the top-up window's run counting, index-digest restart and DM-entries-only
merge, the Unresolved import gate, and the delegate's crash-safe writes.

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* docs(ui): rooms_recovery_in_progress names the Unresolved gate

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* style(ui): a doc line that clippy read as a list item

Claude-Session: https://claude.ai/code/session_01MzSwWofxEmdfdRJvbHxeuE

* build(delegate): rebuild chat_delegate.wasm (aca9f6b5...) on river-core 0.1.22 and re-sign the pointer at v4

Rebased onto main after #772, which re-keyed the delegate to
81b562f8... (registered here as V33, BLAKE3, verified by rebuilding
origin/main 6303d89 to that hash). This branch's build is the only new
generation: aca9f6b5... The river.chat-delegate pointer is re-signed from
main's v3 to v4. room_contract.wasm is unchanged from main (b98f49cf...).
Docs and test labels that named V32 as the newest predecessor now name V33.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* fix(ui): rate-limit a persistently silent predecessor, pin the load plan, reset the cadence only when the walk runs (#768 round 3 CF1, CF2, CF6, S5)

A predecessor that is installed but cannot execute answers every request with
an untyped error (freenet-core 0.2.144 flattens it to OperationError), and a
node before 0.2.137 answers a missing one with an empty success. Both read as
silence, so the journal never settles and the walk ran on every load. After
SILENT_MIN_ATTEMPTS (6) walks leave every pending predecessor unresolved or
absent, the walk now runs on one load in SILENT_WALK_EVERY_N_LOADS (4).

start_journal_load's post-begin decision is the pure load_plan, tested, and
pinned in source at the wiring. RunResetCadence no longer writes the counter
at decision time: run_work_locked writes it once the room-set gate let the
walk through.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* fix(delegate,ui): a listing writes nothing, a refused listing shows Retry, a delete finishes its removal (#768 round 3 S1, S4, CF5)

Delegate:
- ListRequest on a fresh delegate answers an empty list and writes nothing;
  the first store creates the index (S4). Crash recovery still runs on a
  listing.
- The migration journal key is an index witness (S4).
- A refused ListRequest carries LIST_REFUSED_MARKER (river-core), because
  the node hands the client the delegate's error text in an untyped error
  with no delegate key (S1).
- Delete and delete recovery read the value back after removing it. A
  removal that did not take keeps the intent: a listing goes ahead, a write
  is refused rather than replace the intent and strand the value unlisted.
- Tests: crash during crash recovery for add and delete, recovered by a list
  or a store (CF5); fresh listing writes nothing; journal witness; marker;
  stuck removal. Each watched failing under its mutation.

UI: while this attempt's room listing is outstanding, an error carrying the
marker fails the load now and shows Retry, instead of "no rooms" after the
60 s backstop (S1).

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* build(delegate): rebuild chat_delegate.wasm (cea989c8...) and re-sign the pointer at v4

The round-3 delegate fixes (a listing writes nothing, the list-refusal
marker, the journal witness, delete removal read-back) move the delegate.
Still one new generation: V33 (81b562f8...) stays the outgoing one, and
aca9f6b5... was never published. room_contract.wasm is unchanged
(b98f49cf...).

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* fix(ui): prove a short index from rooms_meta first, recover its proven rooms, refuse it cheaply after (#768 round 3 S2, S3)

S2: the loud fetch reads rooms_meta first and checks the rooms it names
before GETting the listed keys, so proving a short index no longer downloads
every listed room. Once a refused predecessor's proven rooms are merged and
flushed, its digest (raw index + rooms_meta) is recorded under
__river_migration_short_index__; while the digest is unchanged the walk
refuses it after the two index reads and the rooms_meta read, downloading no
room slot. With the silent-predecessor cadence, the refused generation is
also walked less often.

S3: room slots that rooms_meta names and that are stored but unlisted are
merged after the walk, at the generation's rank, unflagged through the
ranked merge (as the post-resolve top-up does), while the generation stays
pending. A salvage that did not flush records nothing and is retried.

Tests watched failing under: no short-circuit, no salvage, recording a
salvage whose flush failed.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* test(ui): pin identity provenance at its live call sites; Import-anyway test clears the override on failure; top-up window cost (#768 round 3 CF3, nits)

The cross-load identity pin now finds the provenance save inside
do_save_rooms_to_delegate, binds the restore to this load's rooms_meta, checks
that its only guard is 'there is provenance', and that it is queued before
the room-set latch. Watched failing with an added condition on the guard.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* chore(deps): freenet-migrate 0.7 from crates.io

0.7.0 was published from freenet-migrate main ff457290, the commit the git pin
named; the published src/ is identical to that checkout. Only the
freenet-migrate lock entry changes (source and checksum). The delegate WASM
is unchanged (cea989c8...): freenet-migrate is a UI dependency.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* fix(ui): a migration never re-archives a DM thread a newer generation un-archived (#776); top-up merges only new DM entries (#768 round 3 N2, N3)

#776: un-archiving deletes a hidden_threads entry with no tombstone, and every
predecessor's entries were unioned into the successor, so an older
generation's copy came back (V31's entry in 81b562f8 on a live account).

- Ranked import: a predecessor's archive set goes through
  apply_legacy_hidden_import at its generation rank. The newest predecessor
  whose set applied is authoritative: an older set is ignored, and a newer
  set removes entries an older one supplied that it no longer holds. The
  successor's own entries and the user's in-session actions carry no legacy
  provenance and are never removed by it. The walk (merge_outbound_dms gets
  the generation), the top-up and the sweep (handle_outbound_dms_get_response
  gets the rank) all use it.
- Repair: an archive entry is dropped when the outbound-DM cache holds a
  newer DM to that peer in that room (a reply un-archives, #265), on every
  hidden-thread and outbound-DM hydration, and the healed store is saved.
  This also heals stores the #772 migration already wrote one into.

N2: the top-up merges only DM entries the cache does not already hold, so an
unchanged predecessor costs no merge and no save. N3 is documented: only the
newest predecessor is topped up.

Tests, each watched failing under its mutation: the newer generation's
deletion wins in either arrival order; owned and shared entries; the repair
rule; the walk never resurrects an entry the newest predecessor deleted;
wiring pins for the seam, the sweep and the hydration paths; the top-up with
nothing new merges and saves nothing. The delegate WASM is unchanged.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* fix(ui): drop the #776 "newer outbound DM" repair rule; it un-archives threads the user replied to and then archived

An archive's hidden_at_ts is the thread's newest INBOUND timestamp
(dm_rail_section::archive_cutoff), not the time of the archive. A DM the user
sent before archiving is therefore also newer than it, so the rule dropped the
archive of every thread the user had replied to and then archived. The #768
rehearsal hit exactly this: its fixture archived after sending, and the rule
fired on load. The ranked import alone stops the #776 resurrection; healing
stores #772 already wrote a resurrected entry into needs a different signal
and is left to a follow-up decision.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* fix(ui): heal DM archives #772's union migration resurrected (#776)

An entry the newest predecessor seen holds, the next-newer answering
predecessor lacks, and an older generation holds at the SAME hidden_at_ts was
copied in by a union migration after the user un-archived it (#772 did this to
81b562f8). apply_legacy_hidden_import records every predecessor's archive set
for the session and drops such entries (union_resurrections), whatever the
arrival order. Only predecessor-supplied entries are considered.

The one genuine archive with that signature: re-archived with no inbound DM
since, after being un-archived one generation earlier, while an older
generation still held it. Dropping it only shows the thread again; no message
is lost.

Tests, red on revert: every arrival order heals it; an entry the next-newer
generation also holds, one with a different cutoff, one with no next-newer
answer, and the successor's own entry are kept; the real walk heals it.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* fix(ui): the post-resolve top-up never touches DM archive state (#768 round 4 M1)

The top-up handed merge_outbound_dms an empty hidden_threads set at V33's
rank. The ranked import (#776) counted it: V33 claimed authority and every
archive a lower generation supplied was removed. merge_outbound_dms now takes
the archive rank as an Option; the top-up passes None and only DM entries are
merged. Test through the seam's archive composition (fails when the top-up
passes the rank) and a pin that the live seam imports archives only under
Some(rank).

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* fix(ui): the #776 archive ranks hold across page loads (#768 round 4 S1)

LEGACY_HIDDEN_RANKS was per page load, so a predecessor answering in a later
load than a newer one resurrected an un-archived thread again, in either
order. The ranks (authority, per-entry provenance and the seen sets the heal
compares) are now persisted in the current delegate under
__river_migration_hidden_ranks__: read in the begin that every import waits
for, merged into the session's state, and written after every import and
every provenance change, never before this session has read it. The current
delegate's own hydration no longer forgets provenance, which is what lets a
newer generation answering later remove an entry an older one supplied.

Test: two page loads, newer-first and older-first, each a fresh session whose
ranks come only from the record stored and read through the production
functions; red when the record is not merged. Wiring pins for the begin
load, the import persist and the current hydration.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* fix(ui): a recorded short-index refusal counts toward the walk cadence (#768 round 4 S-C)

A short-index predecessor the sweep flagged stays mid-import for good, and
walk_cadence returned None for any mid-import predecessor, so it walked on
every load. walk_cadence now takes the predecessors refused for a short index
whose rooms were salvaged (SHORT_INDEX_KEY, read once per load while the
journal is incomplete); such a predecessor counts toward the silent cadence
like any refusal. Any other mid-import predecessor still walks every load.

Test through real walks: unrecorded or before SILENT_MIN_ATTEMPTS it walks;
recorded after it, one load in SILENT_WALK_EVERY_N_LOADS. Red when the
recorded refusal is not counted.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* test(ui): brace-scoped load-plan pins; an unreadable short-index record is not overwritten; list-refusal attribution note (#768 round 4 S3, nits)

The load-plan and cadence-reset pins strip comments and match braces
(skipping string literals): poke_walk and the reset store must sit inside the
plan.walk block, poke_topup inside the plan.topup block, and the driver's
reset at its own level after the no-walk return. Watched failing with
poke_walk moved out of the block and with the reset nested.

A short-index salvage no longer writes the record back when the stored one
could not be read. Test with a silent read and a prior record, red on revert.

is_current_list_refusal documents that the refusal carries no delegate key,
so a future predecessor's refusal inside the next build's listing window
would show Retry.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* Revert "fix(ui): the #776 archive ranks hold across page loads (#768 round 4 S1)"

This reverts commit 1e314d7.

* Revert "fix(ui): heal DM archives #772's union migration resurrected (#776)"

This reverts commit d1fbd00.

* fix(ui): #776 by a journal-derived rule: only the newest data-holding predecessor's DM archives merge

Replaces the ranked archive import (reverted with its persisted record and
the heal) with a rule that needs no new state. A predecessor's hidden_threads
are merged only when every NEWER predecessor is known to hold nothing: it
exported nothing earlier in this walk (the walk runs newest-first; the reader
and writer share this walk's answers), or the journal resolved it as not
data-bearing, or it is typed "not installed" (absent streak, or this walk's
pre-warm). Otherwise the generation's archives are skipped. Nothing is
removed; the successor's own entries and in-session actions are untouched.
The sweep and the top-up never merge archives: the walk imports every
predecessor's DM store while the journal is incomplete.

Tests, each red on revert: within a load (a newer data holder keeps the older
archive out; a newer generation holding nothing, or not installed, lets it
in); across loads with the newer generation silent first and with the older
one silent first; the sweep never imports archives (pin); the existing walk
test. The not-installed case uses keys no other test shares, because
NOT_INSTALLED is process-global (shared keys failed 5 in 12 runs; 15 in 15
pass now).

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* fix(ui): #776 archive data means a DM store; pin the S-C cadence wiring

For newest_archive_holder, a newer generation holds archive data only if it
exported a DM store (outbound_dms), not merely rooms. Archiving and
un-archiving both save the DM store, so a generation with rooms and no DM
store recorded no archive decision, and an older generation's archives merge
past it. A generation the journal resolved data-bearing in an earlier load
still counts as holding archives (the journal does not record which items).
Test: a newer generation with a room and no DM store lets the older archive
in; red when any data counts.

start_journal_load's S-C wiring is pinned: the refusals come from the
short-index record and reach both needs_cadence_counter and walk_decision.
Red when either receives an empty list.

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz

* test(ui): table test of newest_archive_holder; docs: a #776 skip is permanent (#768)

A direct table test of the pure rule over journals from real walks: this
walk's answer (with and without a DM store), the journal's resolution
(data-bearing and not), the absent streak, the pre-warm's not-installed,
silence, and no journal. Each disjunct watched failing when removed.

river-publish.md: a skipped generation's archives are sealed with it in the
same walk and never retried; the cost on nodes before 0.2.137 is the silent
case (a missing delegate answers with silence there).

Claude-Session: https://claude.ai/code/session_01TDtucfREtGHJPZaH6zgjrz
@sanity

sanity commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

Heads-up from #797, which re-keys the room contract and chat delegate (V35, river-core 0.2.1, riverctl 0.2.22) and is about to merge and publish.

If this PR lands after it, its delegate migration entry needs to chain from #797's published chat delegate (eca06ace…) as the next version (V36, or V37 if #793 lands first). It also needs the pointer records re-signed from the new main.

[AI-assisted - Claude]

@sanity

sanity commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded. Both parts landed on main by another route:

The delegate has since re-keyed again (#797, #793), so this branch also conflicts with main.

[AI-assisted - Claude]

@sanity sanity closed this Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Interrupted delegate migration strands rooms in the previous generation (in-progress flag lived in localStorage)

1 participant