Fix protocol inconsistency problems - #3
Draft
rmccue wants to merge 11 commits into
Draft
Conversation
The payload length was computed from UTF-16 code units and the bytes written via charCodeAt() truncated mod 256, while the SHA-256 digest was computed over the UTF-8 encoding. Any non-ASCII payload was corrupted on the wire and carried a digest for different bytes than were sent. Encode the payload once with TextEncoder and derive the wire bytes, PayloadLength, and PayloadDigest from that single byte sequence, matching the reference client (messageparser.go SerializeClientMessage). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018jiE4vjDs72pG61zLXP1xC
putLong only wrote the low 4 bytes, so CreatedDate went out as Date.now() mod 2^32 with the top half zeroed; the reference client sends the full 64-bit epoch millis. Write both 32-bit halves instead (exact for values below 2^53). decodeInt built values with 32-bit shifts, which would wrap at 2^31 when decoding 8-byte fields such as SequenceNumber; use multiplication. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018jiE4vjDs72pG61zLXP1xC
The wire format stores the UUID's least-significant 8 bytes first, then the most-significant 8 (see the reference client's putUuid). decodeUuid already un-swapped on read, but encode wrote the UUID bytes in plain RFC order, so the peer decoded a different UUID than the one generated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018jiE4vjDs72pG61zLXP1xC
The outgoing sequence number was reset to the last acknowledged number when publication resumed. An acknowledgement for N means the remote received N, so re-using it labels new input with an already-consumed sequence number, which the receiver discards as a duplicate — silently swallowing the first message sent after every pause/resume cycle. The reference client never rewinds: it ignores pause/start publication for sequencing and keeps the counter monotonic even across reconnects (streaming.go, sessionhandler.go). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018jiE4vjDs72pG61zLXP1xC
Every output_stream_data payload was emitted as terminal output regardless of its payload type, so a session performing the handshake (newer agents on non-shell or encrypted sessions) would print the handshake JSON to the terminal, never respond, and hang. Mirror the reference client's HandleOutputMessage: emit only data payloads (Output/Error/StdErr) as output, respond to HandshakeRequest with a HandshakeResponse (SessionType accepted, anything else reported as Unsupported — we don't support KMS encryption), and surface the HandshakeComplete customer message. Also add the missing StdErr and ExitCode payload types from the protocol. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018jiE4vjDs72pG61zLXP1xC
Inbound stream messages were emitted in arrival order and deduplicated by MessageId in an unbounded map, which leaked every payload for the session's lifetime and relied on the service preserving MessageIds across retransmits. Follow the reference client (HandleOutputMessage): track the expected sequence number, buffer out-of-order messages until the gap is filled (capped at the reference's capacity of 10000), and re-acknowledge but drop duplicates below the expected number. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018jiE4vjDs72pG61zLXP1xC
Sent messages were fire-and-forget: a single lost input message would permanently stall agent-side input, since the agent waits on the missing sequence number while buffering everything after it. Buffer sent messages until their acknowledgement arrives and resend the oldest one when its ack is overdue, mirroring the reference client's ResendStreamDataMessageScheduler (100ms check interval, capacity 10000; a fixed 1s retransmission timeout in place of the reference's RTT-tuned value). Duplicate resends are safe: the receiver re-acknowledges and drops messages below its expected sequence number. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018jiE4vjDs72pG61zLXP1xC
The reference client sends acknowledgements with sequence number 0 and flags 3, and data messages always with flags 0; we sent acks carrying the current outgoing counter and derived a SYN flag from the sequence number. Make Flags an explicit optional encode field defaulting to 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018jiE4vjDs72pG61zLXP1xC
NUL-stripping is an accommodation for the padded fixed-width header fields (the service pads differently than the reference client, which tolerates both NUL and space padding); applying it to the payload silently altered legitimate output. Expose the raw payload bytes from decode() and decode the payload without stripping. Each payload was also decoded with a fresh TextDecoder, so a multi-byte UTF-8 sequence split across two output messages decoded to U+FFFD garbage at the boundary; terminal output now goes through a single streaming decoder. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018jiE4vjDs72pG61zLXP1xC
ping() sent the literal text "ping" as a data frame; the reference client's keepalive is a WebSocket ping control frame with payload "keepalive". Use the underlying socket's ping() when it exists (Node `ws`), keeping the text frame only as a browser fallback since browsers cannot send control frames. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018jiE4vjDs72pG61zLXP1xC
payloadData.Output.length threw an uncaught TypeError when the Output key was absent. The client-side contract includes Output, so this is defensive, but Go zero-values a missing key where JS throws. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018jiE4vjDs72pG61zLXP1xC
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
1 participant
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.
This library was created through reverse-engineering the SSM protocol back before anything was available on it. Now that the session-manager-plugin is open source, we can rectify these issues by porting the behaviour from that instead.
This PR was generated primarily by Claude Fable using session-manager-plugin and amazon-ssm-agent as a reference.
Still needs verification and testing to confirm the changes here.