Closes the final phase of docs/chat-hardening-plan.md. Two problems: a
locally echoed message always looked sent even when the core had no active
session or the gossip broadcast failed; and a fast burst could broadcast
'successfully' yet be silently dropped by every receiver's per-author rate
bucket (8 burst, then 1/s) with no sender feedback.
Send status: CoreCommand::SendChat/SendChatFile carry a local-only id (never
on the wire); the core replies with UiEvent::ChatSendResult after the gossip
broadcast succeeds or fails, and a no-active-session is now an explicit
failure rather than a silent no-op. gossip send_chat, which previously
returned Ok on a missing sender/topic or an encode failure, now returns Err.
ChatEntry gains local_send: Option<LocalSend>; failed sends render a red
'Not sent — {reason} [Retry]' line, Broadcast/Pending render nothing
(there are no delivery receipts, so silence is the honest success state).
Sender-side pacing (new src/app/sendqueue.rs): sends past the burst queue
locally as 'queued…' and trickle out at the receivers' sustained rate, so
nothing is lost and typing is never blocked (user chose queue-and-trickle
over input throttling). The pacer reuses the gossip gate's own TokenBucket +
per-author constants (now pub(crate)) so the two sides of the policy can't
drift. A 250ms drain subscription runs only while the queue is non-empty.
Retry re-dispatches the retained payload; re-serving the same attachment id
replaces the ServeStore entry rather than double-counting bytes. The pacer
and monotonic send-id counter survive a room reset (receivers' buckets
persist; ids never alias a late result); queue and retry payloads are cleared.
582 lib tests (+11: 4 pacer/queue seam, 7 app-level transition/retry/reset);
all-targets green, clippy -D warnings clean, fmt clean, smoke launch OK. No
wire change (GOSSIP_PROTO stays 5). Tests-green-only — the two owed
two-machine field-test items are logged in the plan.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
+86
-26
@@ -1,18 +1,23 @@
|
||||
# Chat hardening — ephemeral implementation plan
|
||||
|
||||
**Status (2026-07-18):** Phases 1–4 COMPLETE. Phase 1 = shared text policy in
|
||||
`src/sanitize.rs`, ceilings enforced at UI input, sign point, and gossip ingress.
|
||||
Phase 2 = roster-bound authorship (`src/core/chatroster.rs`), replay dedup + rate
|
||||
limits (`ChatIngressGate` in `src/network/gossip.rs`). Phase 3 = attachment
|
||||
cache/serve-store budgets, downscaled previews, auto-fetch byte/request budgets
|
||||
(`src/core/fetchbudget.rs`), exact transfers, bounded local reads. Phase 4 =
|
||||
parsed-URL link policy (`is_safe_web_url`/`link_ranges` in `src/sanitize.rs`,
|
||||
`url` crate), 8-link cap, cached link ranges in `ChatEntry`, 512 KiB history
|
||||
text budget, chat-body bidi-override strip (closes S14). All gates green each
|
||||
phase. Phase 5 not started. This is a temporary scope contract for hardening
|
||||
the existing room chat. Update the checkboxes and decision log as work lands,
|
||||
then delete this file when the work is complete. Do not add link previews as
|
||||
part of this effort.
|
||||
**Status (2026-07-18):** Phases 1–5 COMPLETE (all plan phases done). Phase 1 =
|
||||
shared text policy in `src/sanitize.rs`, ceilings enforced at UI input, sign
|
||||
point, and gossip ingress. Phase 2 = roster-bound authorship
|
||||
(`src/core/chatroster.rs`), replay dedup + rate limits (`ChatIngressGate` in
|
||||
`src/network/gossip.rs`). Phase 3 = attachment cache/serve-store budgets,
|
||||
downscaled previews, auto-fetch byte/request budgets (`src/core/fetchbudget.rs`),
|
||||
exact transfers, bounded local reads. Phase 4 = parsed-URL link policy
|
||||
(`is_safe_web_url`/`link_ranges` in `src/sanitize.rs`, `url` crate), 8-link cap,
|
||||
cached link ranges in `ChatEntry`, 512 KiB history text budget, chat-body
|
||||
bidi-override strip (closes S14). Phase 5 = honest local send status
|
||||
(`CoreCommand::SendChat`/`SendChatFile` carry a local id, `UiEvent::ChatSendResult`,
|
||||
`SendStatus` on own echoes) PLUS sender-side pacing (`src/app/sendqueue.rs`
|
||||
mirrors the receivers' per-author budget so fast bursts trickle instead of being
|
||||
silently dropped downstream). All gates green each phase. This is a temporary
|
||||
scope contract for hardening the existing room chat; with every phase complete
|
||||
and the two-machine field test done, delete this file (see the completion note
|
||||
at the end). The two-machine field-test section below is still owed before that
|
||||
deletion. Do not add link previews as part of this effort.
|
||||
|
||||
## Goal
|
||||
|
||||
@@ -294,24 +299,37 @@ small spans an unnecessary UI/launcher surface.
|
||||
**Target:** never present a locally echoed message as successfully broadcast when
|
||||
the core rejected it or gossip broadcast failed.
|
||||
|
||||
- [ ] Add a local-only message id and `Pending`/`Broadcast`/`Failed` state to local
|
||||
chat entries. Do not put this id or state on the wire.
|
||||
- [ ] Carry the local id through `CoreCommand::SendChat`/`SendChatFile` and return a
|
||||
- [x] Add a local-only message id and `Pending`/`Broadcast`/`Failed` state to local
|
||||
chat entries. Do not put this id or state on the wire. (`ChatEntry.local_send:
|
||||
Option<LocalSend>`; `SendStatus` also has `Queued` for the paced-but-not-yet-sent
|
||||
state — see the pacing decision-log entry.)
|
||||
- [x] Carry the local id through `CoreCommand::SendChat`/`SendChatFile` and return a
|
||||
`UiEvent` result after the local gossip broadcast call succeeds or fails.
|
||||
- [ ] If the core is not in an active session, return failure instead of silently
|
||||
doing nothing.
|
||||
- [ ] Show failure compactly with a retry action. A successful local broadcast must
|
||||
(`SendChat`/`SendChatFile` gained `local_id`; new `UiEvent::ChatSendResult { local_id,
|
||||
error }`.)
|
||||
- [x] If the core is not in an active session, return failure instead of silently
|
||||
doing nothing. (`send_chat` now `Err`s on missing sender/topic and on encode
|
||||
failure; the core arm maps no-session to a `ChatSendResult` error.)
|
||||
- [x] Show failure compactly with a retry action. A successful local broadcast must
|
||||
not be labeled “delivered” or “read”; PeerSpeak has no peer acknowledgements.
|
||||
- [ ] Retry creates one new signed broadcast while retaining replay correctness and
|
||||
attachment serving state.
|
||||
(Failed → red "⚠ Not sent — {reason} [Retry]" line; Broadcast/Pending render
|
||||
nothing — silence is the honest success state.)
|
||||
- [x] Retry creates one new signed broadcast while retaining replay correctness and
|
||||
attachment serving state. (`RetryChatSend(id)` re-dispatches the retained
|
||||
`PendingSend`; re-serving the same attachment id REPLACES the `ServeStore`
|
||||
entry, never double-counts — see `serve_store_replacement_accounting_and_remove_clear`.)
|
||||
|
||||
### Phase 5 tests
|
||||
|
||||
- [ ] Local echo starts pending, becomes broadcast on success, and becomes failed
|
||||
on no-session/channel/gossip error.
|
||||
- [ ] Results update only the matching local entry, including after history
|
||||
eviction or room reset.
|
||||
- [ ] Retry does not duplicate served bytes or mutate an unrelated entry.
|
||||
- [x] Local echo starts pending, becomes broadcast on success, and becomes failed
|
||||
on no-session/channel/gossip error. (`send_status_pending_then_broadcast_on_success`,
|
||||
`send_status_failed_keeps_payload_for_retry`.)
|
||||
- [x] Results update only the matching local entry, including after history
|
||||
eviction or room reset. (`send_result_updates_only_the_matching_entry`,
|
||||
`send_result_after_eviction_drops_orphan_payload`, `send_result_after_room_reset_is_a_noop`.)
|
||||
- [x] Retry does not duplicate served bytes or mutate an unrelated entry.
|
||||
(`retry_redispatches_only_the_targeted_send`; served-byte dedup =
|
||||
`serve_store_replacement_accounting_and_remove_clear` in `files.rs`.)
|
||||
|
||||
## Compatibility and versioning
|
||||
|
||||
@@ -359,6 +377,11 @@ it; do not make ordinary unit tests depend on external network access.
|
||||
rest as selectable plain text, with nothing dropped.
|
||||
- [ ] A message attempting bidi-override display spoofing renders in send order
|
||||
(the override characters are stripped, emoji/joining-script text intact).
|
||||
- [ ] Send a fast burst (>8 messages in a second): all arrive at the peer in
|
||||
order, none silently lost; the sender sees "queued…" on the overflow that
|
||||
then clears as each goes out.
|
||||
- [ ] Send with no active session (or a failing broadcast): the message shows
|
||||
"⚠ Not sent" with a Retry, and Retry resends it once when connectivity is back.
|
||||
|
||||
## Completion criteria
|
||||
|
||||
@@ -522,3 +545,40 @@ The plan is complete when:
|
||||
an open Save/Play on an evicted line keeps its bytes-in-hand (the save
|
||||
dialog falls back to the generic "download" name). The just-pushed entry is
|
||||
never evicted; a single message's 8 KiB ceiling cannot exceed the budget.
|
||||
- **2026-07-18 (Phase 5):** Sender-side PACING was added to Phase 5's scope
|
||||
(originally receiver-status only). The Phase 2 decision log deferred the
|
||||
"apply the same local submit policy to accidental rapid Enter" item to pair
|
||||
with Phase 5, and honest status alone would still let a fast burst broadcast
|
||||
successfully yet be silently dropped by every receiver's per-author bucket
|
||||
(8 burst, then 1/s) with no sender feedback. The user chose "queue and
|
||||
trickle" over "throttle input": sends past the burst queue locally as
|
||||
`SendStatus::Queued` ("queued…") and release at the receivers' sustained
|
||||
rate, so nothing is lost and typing is never blocked.
|
||||
- **2026-07-18 (Phase 5):** The pacer (`src/app/sendqueue.rs`) reuses the
|
||||
gossip gate's OWN `TokenBucket` + `CHAT_AUTHOR_BURST`/`CHAT_AUTHOR_REFILL_PER_MS`
|
||||
(made `pub(crate)`), so the two sides of the rate policy are one definition
|
||||
and cannot drift. It mirrors only the PER-AUTHOR budget, not the room-wide
|
||||
one — we cannot know other members' send rates, and the per-author bucket is
|
||||
the one guaranteed to apply to us at every receiver.
|
||||
- **2026-07-18 (Phase 5):** Send status renders as a line UNDER the message
|
||||
(user pick over an inline suffix glyph); `Broadcast` and the transient
|
||||
`Pending` show nothing because PeerSpeak has no delivery/read receipts, so an
|
||||
unadorned message IS the honest "handed to the swarm" state. Only `Queued`
|
||||
and `Failed` (with Retry) are surfaced.
|
||||
- **2026-07-18 (Phase 5):** The pacer and the monotonic send-id counter
|
||||
deliberately SURVIVE a room reset while the queue and retry payloads are
|
||||
cleared: receivers' per-author buckets persist across our rejoin (so the
|
||||
pacer should not refill to full), and never-reused ids keep a late
|
||||
`ChatSendResult` from a pre-reset send from aliasing a new entry — verified by
|
||||
`send_result_after_room_reset_is_a_noop`.
|
||||
- **2026-07-18 (Phase 5):** The pacer clock is `Instant`-based
|
||||
(`AppState.send_clock`), not wall-clock, so a system time jump can neither
|
||||
rewind nor fast-forward the send budget.
|
||||
|
||||
## Completion
|
||||
|
||||
All five phases are implemented and every gate is green. Per the scope-contract
|
||||
note at the top, this file should be DELETED once the owed two-machine field
|
||||
test (the checklist below) has been run — that deletion is a separate,
|
||||
user-gated step, not part of the Phase 5 commit. Until then the plan stays as
|
||||
the record of what shipped and what remains to verify on real hardware.
|
||||
|
||||
Reference in New Issue
Block a user