fix(runtime): update shell session wire protocol - #1968
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
|
|
||
| ws.on('message', (data: Buffer) => { | ||
| // Connection is ready immediately after WebSocket opens — no confirmation frame wait. | ||
| ws.on('open', () => { |
There was a problem hiding this comment.
Could we update the close handling so only code 1000 is treated as a clean exit? Now that the connection resolves when the WebSocket opens, an abnormal close such as 1006 can happen before the shell is usable. Treating that as exit 0 reports a failed connection as successful.
There was a problem hiding this comment.
Addressed in the next commit!
| settled = true; | ||
| resolve({ | ||
| ws, | ||
| shellId: shellIdFromHeader ?? shellId ?? '', |
There was a problem hiding this comment.
Could we return an error when a new connection does not include the shell ID header? Returning an empty shell ID means the user cannot reconnect to the session.
There was a problem hiding this comment.
Addressed in the next commit!
64269af to
07c99c7
Compare
|
Claude Security Review: no high-confidence findings. (run) |
Coverage Report
|
|
Claude Security Review: no high-confidence findings. (run) |
Description
Remove the 0x03 metadata frame dependency and 0xFF close frame send from the shell connection flow to align with the K8s wire protocol.
Changes:
encodeClose()method fromShellFramer— CLI already usesws.close()for detach, this was dead codeheader (eliminates up to 10s wait)
reconnected/bytesDroppedfromShellConnection— these were populated by the confirmation frame we no longer wait forExit codes, keepalive/reconnect, and Ctrl+] detach all continue to work unchanged.
Related Issue
Closes #
Documentation PR
N/A — no user-facing documentation changes needed.
Type of Change
Testing
How have you tested the change?
npm run test:unitandnpm run test:integnpm run typechecknpm run lintsrc/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsResults:
Breaking changes:
ShellConnection.reconnectedandShellConnection.bytesDroppedno longer existShellReconnectOptions.onBytesDroppedandonNewSessioncallbacks removedShellFramer.encodeClose()method removedper team decision (no protocol negotiation)
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.