Add message sequencing and acknowledgment to remote messaging #744
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
8 tasks
FUDCo
force-pushed
the
chip/message-ack-retransmission
branch
from
74790e7 to
7697239
Compare
FUDCo
force-pushed
the
chip/message-ack-retransmission
branch
from
12d0fcf to
2a0f438
Compare
FUDCo
force-pushed
the
chip/message-ack-retransmission
branch
from
2da52d4 to
1e8e95c
Compare
Implements message acknowledgment and retransmission: - Update RemoteCommand type to include seq (sequence number) and ack (piggybacked ACK) fields - Add per-peer sequence tracking that persists across reconnections - Implement data structures with cumulative ACK support - Implement transmission timeout with limited retries - Reject pending promises when giving up on reconnection
- Fix deadlock in receiveMessage by using fire-and-forget for reply sends - Fix PlatformServices to return reply instead of sending it - Add handleAck and updateReceivedSeq to PlatformServices interface - Implement delayed ACK mechanism (50ms timer) for standalone ACKs - Add timeout to libp2p.stop() to prevent cleanup hangs - Close channel streams explicitly on stop to unblock pending reads - Fix e2e test cleanup with parallel stops and increased hook timeout - Skip flaky "handles connection failure and recovery" test Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Restored validateMessageSize, checkConnectionLimit, and cleanupStalePeers functions that were accidentally removed during message sequencing changes. Also restored lastConnectionTime tracking and cleanup interval setup/teardown. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…scarding The browser runtime's #handleRemoteMessage was always returning an empty string, discarding the reply from the remoteDeliver RPC call. This broke reply-based protocols like ocap URL redemption. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Previously getNextSeq() was called before attempting to add a message to the queue, so rejected messages would still consume sequence numbers. This caused gaps in sequence numbering, which led to incorrect sequence numbers during retransmission since they were inferred from position. Changed addPendingMessage to assign and return the sequence number internally, only incrementing after successful enqueue. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
In the browser runtime, when the kernel worker calls sendRemoteMessage, the offscreen document awaits sendWithAck which waits for an ACK. When the ACK arrives via remoteDeliver, the kernel worker calls handleAck back to the offscreen. If handleAck awaited, this creates a deadlock because the offscreen's RPC message handler is blocked on the original sendRemoteMessage request. Making handleAck fire-and-forget breaks the deadlock while still ensuring ACKs are processed correctly. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Make the network layer a "dumb pipe" that only sends/receives strings. All message sequencing, ACK tracking, and retransmission logic now lives in RemoteHandle within the kernel. - Change SendRemoteMessage to take a string instead of RemoteMessageBase - Remove handleAck and updateReceivedSeq RPC methods - Remove message queueing from network layer - Update all platform services implementations and tests Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove Kernel.sendRemoteMessage and RemoteManager.sendRemoteMessage which bypassed the seq/ack protocol. All message sending should go through RemoteHandle to ensure reliable delivery. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Bug 1: Initialize startSeq when first message added to empty queue - Bug 2: Remove unused promiseKit from PendingMessage type - Update e2e test to expect correct error message Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Reject pending redemptions when intentional close error is detected during message send, enabling fast failure for intentional disconnect. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
These files were created but never integrated - RemoteHandle uses its own simpler inline implementation instead. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add MAX_PENDING_MESSAGES (200) limit to prevent memory overflow - Throw error when pending queue is at capacity - Add onGiveUp callback to notify RemoteManager when we give up - RemoteManager now rejects kernel promises when RemoteHandle gives up Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Reply messages now use seq/ACK protocol via #sendRemoteCommand - Reject pending URL redemptions when giving up after max retries - Add registerChannel() to properly close old channels on replacement - Add reuseOrReturnChannel() for connection race condition handling Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When #sendRemoteCommand is called from within an RPC handler (e.g., during remoteDeliver for a reply), awaiting registerLocationHints can cause deadlock if the browser RPC doesn't support nested calls. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When RemoteHandle.deliverMessage throws (e.g., queue at capacity), the error was propagating up and crashing the kernel run loop. This fix catches delivery errors, rejects the kernel promise for that message, and allows the kernel to continue processing other messages. Also updated the e2e test to reflect actual behavior: new messages are rejected when queue is full, not oldest messages dropped. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…mmand Move the queue capacity check to before #getNextSeq() and #clearDelayedAck() to avoid wasting sequence numbers and disrupting ACK timing when the queue is full. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
FUDCo
force-pushed
the
chip/message-ack-retransmission
branch
from
a629b12 to
e71188d
Compare
Without calling #onGiveUp, kernel promises for messages sent to intentionally closed connections would hang forever. The RemoteManager needs this callback to reject kernel promises via getPromisesByDecider. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Added cleanup() method to RemoteHandle that clears #ackTimeoutHandle and #delayedAckHandle timers. RemoteManager.cleanup() now calls this for each RemoteHandle before clearing its maps, preventing timers from continuing to run and keeping instances alive after shutdown. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
sirtimid
previously approved these changes
Jan 20, 2026
Contributor
sirtimid
left a comment
sirtimid
left a comment
There was a problem hiding this comment.
Approved, but we'll have to restore the removed test coverage for functionality that still exists in network.ts on a follow-up:
- Message size limits (
validateMessageSize) - Connection limits (
checkConnectionLimit) - Stale peer cleanup (
cleanupStalePeers) - Channel lifecycle (
registerChannel/reuseOrReturnChannel)
Everything else LGTM!
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
sirtimid
approved these changes
Jan 20, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements message acknowledgment and retransmission.
Closes #656
Note
Implements reliable remote messaging with sequencing and acknowledgments, plus API/behavior tweaks and test updates.
RemoteHandle(piggyback and standalone ACKs), pending queue with capacity, retransmission with timeouts and max retries, and cleanup hooks; replies likeredeemURLReplyare now sent viasendRemoteCommandinstead of being returnedRemoteMessageBasetypeKernelRoutercatches delivery failures to reject promises; removeKernel.sendRemoteMessageandRemoteManager.sendRemoteMessage; addRemoteHandle.cleanup()and wire toRemoteManager.cleanup()libp2p.stop()MessageQueueand its testsWritten by Cursor Bugbot for commit 15c346b. This will update automatically on new commits. Configure here.