Repository navigation
pilotctl: send-message --wait matches the reply by message ID - #498
Merged
Merged
Conversation
8 of 9 tasks
TeoSlayer
force-pushed
the
fix/send-message-reply-id
branch
2 times, most recently
from
October 7, 2026 13:57
82baba6 to
f903919
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
TeoSlayer
force-pushed
the
fix/send-message-reply-id
branch
from
October 7, 2026 14:31
f903919 to
77ef51b
Compare
--wait took the oldest new inbox message from the peer. Two concurrent requests to one peer, or anything else the peer sent in the window, could hand a caller another request's answer. send-message now sends every message through dataexchange's tagged path (Client.Send) with a new message ID, and the inbox watch uses it: - a message from the peer whose reply_to is one of our IDs is the reply; - a message whose reply_to names another request is never taken; - a message without reply_to is matched by sender and arrival time, as before, so peers that do not echo the ID (every responder today) keep working unchanged. The first-contact re-send gets an ID of its own, because the receiver drops a repeat of the same ID and bytes as a duplicate; the watch accepts a reply to either. --trace is covered: the TRACE wrapper is built here and carries the ID outside it. New flag --reply-to <id> sends a message as the answer to a received one. The JSON result gains message_id, tagged and (with --reply-to) reply_to. Receivers that predate message IDs are reached through the library's fallback: the frame is re-sent in the old format on the same connection, and the result says "tagged": false. An "ERR ..." answer is still reported as the ack, and a send whose ack could not be read still succeeds, as before. No dependency change: dataexchange v0.2.3, already pinned, has the IDs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…inbox shows IDs send-message --wait took the first untagged message from the peer at once, so a reply naming our request that arrived a poll later lost to another client's untagged answer. When our request reached the receiver with its ID, an untagged message is now held for 0.75 s (bounded by the wait) in case our tagged reply follows, and once the peer has been seen naming another request in reply_to, only a reply naming ours is taken. --reply-to refuses a bare flag (parsed as "true") and maps an inbox file id to the message_id stored in that record, refusing it when the record has none. pilotctl inbox shows message_id and reply_to in the listing, --json, and inbox read; the help says which field to use. A lost ack on the tagged first attempt is retried once on a new connection with the same frame and ID: a current receiver keeps one copy, one through v1.13.9 gets the untagged copy. --no-resend opts out. tagged is reported only when the receiver's ack was read. CHANGELOG no longer claims the concurrency bug is fixed for responders that do not echo reply_to. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No service responder echoes reply_to yet, so holding every untagged reply for 0.75 s whenever the receiver's daemon knows message IDs would slow nearly every send-message --wait once daemons upgrade, for nothing. An untagged reply is now taken at once, as before, unless the peer is known to echo IDs: one of its newest messages already in the inbox carries a reply_to. newInboxWatch checks this once, from the snapshot it already reads, looking at no more than the newest 1000 records and 200 from the peer. For such a peer the 0.75 s hold stays, and a peer seen naming another request during the wait still gets only a reply naming ours taken. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A --trace message is not sent again after a lost ack: receivers do not suppress repeated trace frames (dedupeEligible), so even a current receiver would store it twice. - The lost-ack retry still runs inside sendOne, before #501 judges the send: a retry that is acknowledged is delivered, and a send whose retry is not acknowledged fails with the retry's ack error. - The echo-history scan runs in a goroutine started with the inbox snapshot and is waited for only when an untagged reply has to be judged, so it no longer delays the request. It checks for the sender and reply_to bytes before parsing, skips records over 1 MiB and stops after 16 MiB as well as after 1000 files or 200 records from the peer. - The hold runs until 0.75 s after the message arrived (its received_at), and awaitReply polls when it ends, so it is no longer up to a poll interval longer than stated. - --reply-to accepts an inbox file name with .json. - The help and CHANGELOG say a receiver through v1.13.9 can store a retried message twice when the ack of its untagged copy is lost; from v1.13.10 the repeat is suppressed. - inbox read lines up its labels; a misleading poll comment is fixed. - Tests no longer call t.Fatal from goroutines or receiver callbacks, wait with a timeout, and allow 400 ms for a reply taken at once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review fix. The hold counted from the message's received_at even when that was before our request was acknowledged, so after a slow first-contact dial, a large payload or a lost-ack retry, an untagged message from a peer known to echo IDs was taken at the first poll, ahead of the tagged reply. A message that arrived before the ack is the least likely to be the reply; its hold now starts at the ack. The echo-history scan also skips anything that is not a regular file, so a FIFO in the inbox cannot block it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TeoSlayer
force-pushed
the
fix/send-message-reply-id
branch
from
October 7, 2026 14:44
77ef51b to
5904075
Compare
…tion explicitly Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Pull Request
Summary
send-message --waittook the oldest new inbox message from the target after the send. Two concurrent requests to one peer, or any unsolicited message from it, were indistinguishable, so a caller could be handed another request's reply. dataexchange already carries request/reply IDs (MessageID/ReplyTo, in the pinned v0.2.3); nothing in pilotctl used them.Changes
"tagged": false.reply_tois one of ours first, never takes one whosereply_tois someone else's, and otherwise keeps today's sender-and-time rule.--reply-to <id>so a peer can echo the ID.message_id,tagged, andreply_towhen given.--traceis covered.No dependency bump.
Limits
message_idback asreply_tobefore this helps there.--reply-towith no value parses as the IDtrue.Test Plan
go build ./...,go vet ./..., unit suite withGOWORK=offACK UNKNOWN(10)andERR UNKNOWN(10))message_idandreply_toChecklist
go.mod/go.sumunchanged🤖 Generated with Claude Code