fix(a2a): removing a peer is one write, and a headerless error is not a mismatch

[Jacksondr5]

Problem

Three robustness gaps in peering poll mode, from Bryant's review and CodeRabbit (Closes #485, part of #399):

• 1a: A peer response with no protocol header counted as version 1 before its status was read. After the next protocol bump, a proxy's 502 would become a final "Update J5" error instead of a retry.
• 1e: Removing a peer ran in separate steps, so traffic that landed in between was missed and could survive re-peering. That traffic could be an ask arriving by poll, a local send, or a delivery.
• 1f: A direct message that was already attempted, but cancelled by a removal, told its sender "was not delivered" even though the peer may already hold it.

What changed

1a: a headerless error is not a mismatch. responseProtocolMismatch in peerProtocol.ts reads a missing header as version 1 only on a 2xx. An error answer without a header goes through normal status handling. A real mismatch is still refused with the header, so it is still caught. Every response site uses it: direct send, roster read, hello, and both poller reads.

1e: removal is one write.

• PeerRemovalService.remove does everything in one SQLite write transaction:
  • drops the open Exchanges;
  • cancels the waiting messages and writes their notices;
  • deletes the peer record.
• The ledger appends join the transaction through appendEventsInTransaction. They are published only after the commit, and nothing is published if the transaction rolls back.
• Recording an inbound message checks the peer record inside its own write, on both the deliver route and poll. Once a removal has committed, a delivery is refused; the deliver route answers 403 peer_not_registered.
• The issue assumed local sends to a removed peer were already refused. That held only for unknown receivers: one this server had already exchanged with kept a recorded route. The send's write now checks the peer record too, and refuses with "not currently reachable".
• A poll's hand-out rechecks and stamps its rows in one write: it stamps only rows still waiting, for a peer still recorded. So it never hands out a message that a removal cancelled after the poll chose it.
• The delivery worker's peer-cancel path and its drain-wait test hook are removed; the transaction replaces them.

1f: attempted direct rows get "may not have been delivered". That covers:

• a row with attempts > 0 and no refusal back;
• a first direct send still on the wire when the removal commits. Before it sends, the attempt registers itself under the ledger writer's permit, which the removal also holds;
• a handed-out poll row, as before.

1b and 1d stay unbuilt, as the issue records.

Checklist

• One concern: the description has no "also". It does cover the three robustness items a2a peering: poll-mode robustness follow-ups (atomic removal, headerless errors, attempted-direct wording) #485 groups together.
• Tests cover the changed behavior. Each new test fails without its fix:
  • peerProtocol.test: responses with no header;
  • PeerPoller.test: a proxy's 502, 503 or 500 with no header is retried. Main's feat(a2a): the peer wire says project #489 bumped the protocol to 2, so without the fix these read as mismatches;
  • PeerPollRoundTrip:
    • traffic during a removal is either wiped or refused, and a second, independent connection sees nothing committed until the removal is;
    • a failed removal rolls back and publishes nothing;
  • PeerRemovalService.test:
    • the wording for an attempted direct row, and for a first send still on the wire;
    • a poll that chose a message just before a removal doesn't hand it out.
• UI changes: none
• Upstream-owned files: none touched
• Upstream product: unchanged
• Surfaces:
  • Server only. Every path is covered: the deliver route, poll, local send, the removal route and the CLI.
  • Link modes. A live smoke ran with pr-env.sh --peer and --peer --poll, using GPT-6-Luna Low agents.
    • Push: removing a peer while asks flowed dropped the Exchange with its notice, and later sends were refused.
    • Poll: removing on the poller dropped the Exchange, and a send to a known remote agent was refused.
  • Limit. The exact overlap with a removal is proven by the deterministic tests, not live.
• Docs: cross-device.md, a2a/agent-tools.md and the peering runbook now describe the one-write removal and the wider "may not have been delivered" wording.

Claude Opus 5.5 (1M context), in J5 Code through the Claude Code harness.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Peer removal now processes related exchanges and queued deliveries together, preventing messages from being handed out after removal completes.
  • Senders receive a notice when a message is canceled, including when delivery may have occurred but confirmation was lost.
  • Messages to or from peers that are no longer registered are refused.
  • Failed responses without a protocol header are handled as HTTP errors rather than protocol mismatches.
• Documentation
  • Clarified peer-removal behavior and delivery notices for messages that may not have arrived.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
j5-code	Ready	Preview	Oct 8, 2026 7:35am UTC

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: Jacksondr5/j5code/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 548fed74-6db8-4976-a9d0-930d95d28e62

📥 Commits

Reviewing files that changed from the base of the PR and between 937c6b1 and 7f4b6ed.

📒 Files selected for processing (21)

• apps/server/src/j5/a2a/DeliveryTransport.ts
• apps/server/src/j5/a2a/DeliveryWorker.ts
• apps/server/src/j5/a2a/PeerDirectory.ts
• apps/server/src/j5/a2a/PeerHttp.ts
• apps/server/src/j5/a2a/PeerInboundService.test.ts
• apps/server/src/j5/a2a/PeerInboundService.ts
• apps/server/src/j5/a2a/PeerOutbound.test.ts
• apps/server/src/j5/a2a/PeerPollRoundTrip.test.ts
• apps/server/src/j5/a2a/PeerPoller.test.ts
• apps/server/src/j5/a2a/PeerPoller.ts
• apps/server/src/j5/a2a/PeerRegistryService.ts
• apps/server/src/j5/a2a/PeerRemovalService.test.ts
• apps/server/src/j5/a2a/PeerRemovalService.ts
• apps/server/src/j5/a2a/PeerRoundTrip.test.ts
• apps/server/src/j5/a2a/SendService.ts
• apps/server/src/j5/a2a/deliveryNotices.ts
• apps/server/src/j5/a2a/peerProtocol.test.ts
• apps/server/src/j5/a2a/peerProtocol.ts
• docs/j5/product/a2a/agent-tools.md
• docs/j5/product/cross-device.md
• docs/j5/runbooks/peering.md

Included review availability: This review used your included allowance. 1 included review remains after this review. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour.

---

📝 Walkthrough

Walkthrough

The pull request changes peer response protocol checks and peer removal. Removal now uses a serialized transaction for exchange drops, delivery cancellation, and peer-record deletion. Inbound and outbound operations check peer records, and delivery notices distinguish possible delivery from confirmed non-delivery.

Changes

Peer response protocol checks

Layer / File(s)	Summary
Response version checks apps/server/src/j5/a2a/peerProtocol.ts, apps/server/src/j5/a2a/peerProtocol.test.ts, apps/server/src/j5/a2a/DeliveryTransport.ts, apps/server/src/j5/a2a/PeerDirectory.ts, apps/server/src/j5/a2a/PeerPoller.ts, apps/server/src/j5/a2a/PeerPoller.test.ts, apps/server/src/j5/a2a/PeerRegistryService.ts	Protocol checks now receive the full HTTP response. A missing protocol header on a failed response does not produce a mismatch; tests cover headerless and stated versions.

Atomic peer removal and delivery handling

Layer / File(s)	Summary
Peer-record checks for inbound and outbound traffic apps/server/src/j5/a2a/PeerInboundService.ts, apps/server/src/j5/a2a/PeerHttp.ts, apps/server/src/j5/a2a/SendService.ts, apps/server/src/j5/a2a/PeerInboundService.test.ts, apps/server/src/j5/a2a/PeerOutbound.test.ts, apps/server/src/j5/a2a/PeerRoundTrip.test.ts	Inbound message recording and remote sends check that the peer remains recorded. Inbound requests from an unrecorded peer map to HTTP 403. Test setup records peer connections.
Delivery attempt and hand-out state apps/server/src/j5/a2a/DeliveryWorker.ts, apps/server/src/j5/a2a/deliveryNotices.ts	The worker tracks direct attempts and rechecks delivery state before sending or stamping polling rows. Notices distinguish messages that may have arrived from messages that were not delivered.
Transactional removal and cancellation apps/server/src/j5/a2a/PeerRemovalService.ts, apps/server/src/j5/a2a/PeerRemovalService.test.ts, apps/server/src/j5/a2a/PeerPollRoundTrip.test.ts, docs/j5/product/cross-device.md, docs/j5/product/a2a/agent-tools.md	Peer removal drops exchanges, cancels eligible deliveries, records applicable notices, and removes the peer record in one transaction. Tests cover concurrent traffic, transaction failure, and delivery races. The product documentation describes removal and uncertain direct delivery.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant PeerRemovalService
  participant A2ALedgerTransactionWriter
  participant SQLite
  participant A2ADeliveryWorker
  PeerRemovalService->>A2ALedgerTransactionWriter: acquire writer permit
  PeerRemovalService->>SQLite: begin transaction and read peer exchanges and waiting deliveries
  PeerRemovalService->>A2ADeliveryWorker: query peerAttemptInFlight for direct delivery
  PeerRemovalService->>SQLite: append removal events and delete peer record
  PeerRemovalService->>SQLite: commit transaction
  PeerRemovalService->>A2ALedgerTransactionWriter: publish committed events

Loading

Merge Risk: ⚪ Minimal · up to 7f4b6

This change makes peer removal a single atomic step and treats proxy error pages as ordinary HTTP failures rather than protocol mismatches. It also gives senders accurate "may not have been delivered" notices. No outstanding defects were found, and the change appears ready to merge.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR satisfies the active coding requirements in #485. For 1a, responseProtocolMismatch treats a missing header as version 1 only for 2xx responses and preserves mismatch detection when a header i…
Out of Scope Changes check	✅ Passed	The changes remain within #485. Production changes modify peer protocol handling, peer removal transactions, inbound and outbound peer registration checks, poll hand-out coordination, and delivery not…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Title check	✅ Passed	The title clearly identifies the two primary changes: atomic peer removal and correct handling of headerless error responses. It is concise and related to the main changeset.
Description check	✅ Passed	The description includes the required problem, changes, checklist, issue reference, test coverage, upstream impact, surface review, documentation updates, and model/harness attribution. UI and upstrea…

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}