a2a peering: poll-mode robustness follow-ups (atomic removal, headerless errors, attempted-direct wording) #485

Follow-up to the peering poll-mode stack (#400–#408), from Bryant's review (#401, #404, #405, #407) and CodeRabbit. Part of #399.

To build

1a. A response with no protocol header shouldn't be read as a version mismatch (before the next protocol bump)

A peer response with no x-j5-peer-protocol header counts as protocol 1, and that check runs before the status code (DeliveryTransport.ts, PeerDirectory.ts, PeerRegistryService.ts). Some responses never carry our header, even from an up-to-date server:

• a 502, 503 or 504 from Tailscale, a tunnel or a proxy;
• an unhandled 500;
• a non-J5 server answering at that address.

Once PEER_PROTOCOL_VERSION reaches 2 on both sides, a momentary proxy 502 becomes a final "Update J5 on " error, written to last_error, instead of a retry.

Fix: treat a missing header as protocol 1 only on a 2xx. A non-2xx response with no header goes through normal status handling. A real mismatch still arrives as a 409 with the header, so it's still caught. Must land before any protocol bump.

1e. Removing a peer must be atomic

PeerRemovalService reads open Exchanges and waiting rows first and deletes the peer record last. In between:

• our own poller for that peer keeps running;
• local agents can still send to it;
• the direct-row cancel can wait up to the 15 s peer timeout on drainPermit.

Anything that lands in that window is missed:

• an ask arriving by poll opens an Exchange that's never dropped (the Removing a peer leaves its open Exchanges open #286 symptom);
• a local send is never cancelled or noticed, and alarms later;
• after re-peering, such a message can still be delivered, which breaks "peering again starts empty".

Fix:

• Do the whole removal in one SQLite write transaction: read, cancel, drop, and delete the record. Writers are serialized, so nothing can interleave. The ledger appends need to join that transaction.
• Make recording an inbound message (deliver route and poll) check the peer record inside its own transaction, so anything after the commit is refused.
• Local sends to a removed peer are already refused.

Residual gap: a direct send already on the wire when removal starts can't be helped by a transaction. 1f covers its wording.

Test: traffic during removal, both an inbound ask and a local send, is either fully wiped or refused.

1f. Direct messages that were attempted get "may not have been delivered"

Only handed-out poll rows get the "may not have been delivered" wording (DeliveryWorker.ts, keyed on handed_out_at). A direct attempt that timed out or lost its response may already be recorded on the peer, yet when it's cancelled the sender hears "was not delivered" and may resend, so the recipient gets it twice.

Fix: use "may not have been delivered" for direct rows with attempts > 0 whose last failure wasn't a decoded refusal. Add a test.

Low priority

1b. A direct ask can lose its closing notice

peerNeverHeldIt (LifecycleService.ts) treats any cancelled ask as proof the peer never held it. The sequence:

1. A direct ask is recorded by the peer, but the response is lost; the row is retry_scheduled with attempts = 1.
2. The sender is archived before the retry.
3. The drop notice is skipped, and the peer keeps an open Exchange.

It self-heals when the receiver replies: the reply settles its side, and the refused reply produces a not-delivered notice. So the cost is wasted work by the receiver, not a stuck state. The exception is a receiver that never replies, which holds an open Exchange indefinitely.

Fix if taken: narrow the check to attempts = 0 OR handed_out_at IS NOT NULL, as CodeRabbit suggested.

Known limitation (not building)

1d. A message the poller can never record blocks later deliveries

Every expected rejection is a refusal: it's acked, notified, and the queue moves on. A non-refusal failure from inbound.receive leaves that row and everything after it unacked, retried each minute. Stored rows count no attempts, so no sender is alarmed. The only symptoms are the peer's last_error and a growing waiting count.

It needs one of:

• a bug in our own code, such as an undecodable body at a matching protocol version or a ledger inconsistency;
• persistent local storage failure, in which case resuming once it's fixed is correct anyway.

Recorded here so it's recognisable if it ever shows up. If it does, the fix is to refuse a row after N consecutive failures.

Dropped

• The storing server never records a mismatch from polls. From the storing side, a mismatch is indistinguishable from offline, and the poller's own row already names which server to update.
• A failed name save drops the peer from a roster read (CodeRabbit). It needs a SQLite write failure, which in practice means a broken disk.