Click-to-read deletes message from inbox list (cross-node, kept_for rebuild miss)

[iduartgomez]

Repro

FREENET_LIVE_E2E_SEND=1 cargo make test-e2e-real-node

Test 3 (live-node.spec.ts:343 — multi-round + read + archive across nodes) fails at line 478:

Error: round one stays visible after click-to-read
Locator: locator('iframe#app').contentFrame().getByText(/round one/i)
Expected: visible
Timeout: 10000ms
Error: element(s) not found

Flow that fails

1. alice@gw → bob@peer: 'round one' (delivered, asserted visible)
2. bob clicks the row → opens detail (asserted visible via fm-detail-time)
3. bob navigates back to inbox list
4. Expected: row still visible, read=true
5. Actual: row gone

Suspected root cause

ui/src/app.rs:1191-1214 rebuilds the inbox list by merging contract messages with local_state::kept_for(&alias) (rows the contract evicted but the user has read). On click-to-read:

• local_mark_read (local_state.rs:445) bumps the local-state delegate.
• The contract's copy of the message gets evicted from the live emails list (read-window scrubbing in inbox contract).
• kept_for should resurface it as a kept row (read=true).

Either:

• local_mark_read isn't producing a KeptMessage entry in SNAPSHOT before the rebuild fires, OR
• The rebuild reads SNAPSHOT before the delegate's echo lands, OR
• Cross-node specific: contract eviction races the local-state delegate write so the row falls out of both sets.

Single-node manual flow (offline example-data,no-sync) doesn't exercise this — Playwright email-app.spec.ts doesn't click-to-read.

Why it matters for #81

Issue #81 wants FREENET_LIVE_E2E_SEND gate flipped off so cross-node delivery is a release blocker. The alice → bob test passes (modulo a separate too-greedy log regex — see #81 followup). Multi-round test 3 fails on this real product bug. Gate cannot flip until this is fixed, otherwise tag releases break.

Proposal

1. Trace the kept_for rebuild race: log when local_mark_read writes the kept entry vs when the rebuild reads it.
2. Likely fix: make mark_read synchronously add to SNAPSHOT (optimistic write) before awaiting the delegate echo, the way drafts are handled.
3. Add a unit test on the offline path (mock contract evicting a read message) that pins kept_for resurrection.

Out of scope

alice → bob log-grep regex over-matches webapp re-publish noise. Fix in #81.

[iduartgomez]

Update — unit test pinpoints the race

Added 5 unit tests in `ui/src/local_state.rs` (commit pending). 4 pass, 1 ignored documents the bug:

• `local_mark_read_round_trips_via_kept_for` ✓
• `kept_for_is_alias_scoped` ✓
• `local_mark_read_is_idempotent` ✓
• `replace_snapshot_preserves_kept_when_delegate_includes_it` ✓
• `replace_snapshot_does_not_clobber_optimistic_kept` (#[ignore]'d, fails today)

The ignored test is the smoking gun:

```text
thread '...' panicked at ui/src/local_state.rs:655:
assertion left == right failed: optimistic kept entry must
survive a stale echo (#113)
left: 0
right: 1
```

`replace_snapshot` (called every time the local-state delegate echoes a `GetAll` reply) clobbers `SNAPSHOT` wholesale. If the optimistic `local_mark_read` write hasn't yet round-tripped through the delegate, an in-flight echo from BEFORE the click overwrites the kept entry and `kept_for` returns empty. The contract has already evicted the row (per `mark_as_read` at `ui/src/app.rs:387`), so the row vanishes from the inbox list.

This matches the test 3 failure pattern in #81: click-to-read → goBack → row gone.

Proposed fix

Mirror the `DELETED_DRAFTS` pattern (`ui/src/local_state.rs:31`): keep a per-(alias, msg_id) tombstone of optimistic kept writes and re-merge them on `replace_snapshot` until an echo includes them. Same shape as the existing draft-deletion fix from #107.

`bump()` was also moved to a runtime-aware no-op when called outside Dioxus, so unit tests can exercise SNAPSHOT mutations without mounting an app.

[iduartgomez]

Trace artifacts captured

Re-ran iso harness with the new tracing wired in (`multi-round-{alice,bob}-trace.zip` under `ui/tests/test-results/`). Inspect with `npx playwright show-trace ui/tests/test-results/multi-round-bob-trace.zip`.

Key console sequence on bob:

```
GetResponse: StoredInbox decoded, 1 messages ← round one delivered
GetResponse: InboxModel built, 1 decrypted messages
... click round one ...
GetResponse: StoredInbox decoded, 0 messages ← contract eviction reflected back
GetResponse: InboxModel built, 0 decrypted messages
... expect row visible ... TIMEOUT
```

No `pageerror` or wasm panic in the bob context. Release wasm strips `debug!` so the local-state delegate path is silent — but the unit-test race (#113) is sufficient evidence of the cause.

Next step is the actual fix (DELETED_DRAFTS-style merge for `kept`) — happy to land that on this branch when ready.

[iduartgomez]

Update — fix lands but doesn't solve test 3

Tombstone fix in commit `fix(ui): tombstone optimistic kept entries to survive stale GetAll echo` lands and the four new unit tests pass. However, test 3 in the iso harness still fails the same way after the fix.

Looking at bob's console mid-test:

```
GetResponse: matched inbox key=AsH9sX... alias=bob ← inbox A, 1 msg
GetResponse: matched inbox key=5Vocqbk3... alias=bob ← inbox B, 0 msg (different contract!)
```

Bob has two inbox contracts both matched to alias "bob". And the gateway/peer logs confirm:

```
$ grep "create alias new bob" ~/freenet-mail-iso/peer/logs/freenet.*
... create alias new bob for 67QZ... contract=Cbc3...
... create alias new bob for 67QZ... contract=Cbc3... (3s later)

$ grep "create alias new alice" ~/freenet-mail-iso/gw/logs/freenet.*
3 entries
```

The `createIdentity` test helper hammers `set_alias` multiple times per call (form-stage submit, confirm-stage submit, and possibly the reload fallback at `live-node.spec.ts:578`). Each set_alias call appears to spawn a fresh identity → a fresh inbox contract → both get registered against `alias=bob`.

When the test clicks "round one", the OpenMessage handler runs `mark_as_read` against `inbox_data[active_id]` (`ui/src/app.rs:382`). With two inboxes both registered to bob's `UserId`, the indexing picks one — possibly the empty one — and the click fires a no-op `RemoveMessages` against the wrong contract. The row that was visible in the inbox list (sourced from the OTHER inbox in MessageList's rebuild loop) stays in current_model.messages but the rebuild logic then can't find it because of the same indexing collision, and it falls out.

This is a separate bug from the `kept_for` race — needs its own fix. Filing as a follow-up.

The kept_for race fix in this PR is still independently correct (proven by the four unit tests, including the previously-`#[ignore]`'d `replace_snapshot_does_not_clobber_optimistic_kept`). Plan: keep #113 open for the click-to-read end-to-end story, land this fix as a partial step, file a sibling issue for duplicate-inbox-per-alias.