Repository navigation
fix(cli): delete the PTY session when the terminal WebSocket never opens - #2523
Merged
Pangjiping merged 1 commit intoNov 1, 2026
Conversation
`osb terminal` creates a PTY session and then connects its WebSocket. The session was torn down only after a clean exit or a reported startup failure, so when the connect itself failed (refused, rejected with an HTTP status, or interrupted) the session stayed on the sandbox and its ID was never shown. The shell starts on the first WebSocket, so a session whose WebSocket never opened has nothing to keep. Track whether the connection opened and delete the session otherwise. A connection lost on a live shell still keeps it, as before.
acodercat
requested review from
Pangjiping,
hittyt,
jwx0925 and
ninan-nn
as code owners
October 31, 2026 08:52
github-actions
Bot
added
the
size/S
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
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.
osb terminalfirst creates a PTY session withPOST /ptyand then opens/pty/{id}/ws. In itsfinally, it deletes the session only whenteardownis set, andteardownis set only afterasyncio.runreturns: on a clean exit or a reported startup failure.When the WebSocket connect itself fails,
asyncio.runraises beforeteardownis set. That happens when the connect is refused, gets an HTTP status (InvalidStatus), or is interrupted. On current main the command exits 1 with "Terminal connection failed", the session created byPOST /ptyis never deleted, and its ID is never printed, so the user can't clean it up either. Percomponents/execd/PTY.md, the shell starts on the first WebSocket, so such a session has nothing worth keeping.Summary
_connect_and_streamtakes an optionalon_connectedcallback, called once the WebSocket is open.terminalrecords whether the connection opened, and also deletes the session when it never did._delete_pty_sessionignores HTTP errors, so a failed delete can't hide the connect error.Testing
Not run (explain why)
Unit tests
Integration tests
e2e / manual verification
New
test_failed_connect_deletes_session, for anOSErrorand anInvalidStatus(403)from the connect: the command must exit 1 and deletesess-1. Both cases fail on current main.The existing
test_closed_without_exit_frame_fails_and_keeps_sessionstill passes, so the session is still kept after a lost connection.The full CLI suite passes (332 passed).
ruffandpyrightare clean.Breaking Changes
Checklist