Require an exit code before exec reports success - #70
Open
MiguelsPizza wants to merge 1 commit into
Open
MiguelsPizza wants to merge 1 commit into
MiguelsPizza wants to merge 1 commit into
Conversation
Non-interactive exec returned 0 when the WebSocket closed without an exit code message, and the exit code could lose a select race to the reader's done channel. Treat a missing or malformed exit code as an error.
MiguelsPizza
force-pushed
the
exec-exit-status
branch
from
October 3, 2026 19:28
e1a99a7 to
26e80bb
Compare
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.
Non-interactive
hypeman execreturns 0 when the WebSocket closes without an{"exitCode":N}message (dropped connection, normal close, malformed or null code), and a received nonzero code can also lose the finalselectto the reader's done channel. This makes the exit code message required: anything else returns an error instead of success. The interactive TTY path is unchanged.Tests:
go test -count=1 ./pkg/cmd/ -run TestRunExecNonInteractive. On main all fourTestRunExecNonInteractiveRequiresExitCodecases fail with "An error is expected but got nil"; they pass with the fix.Note
Medium Risk
Changes CLI exit semantics for non-interactive exec (scripts/CI may now fail on connection issues), but tightens correctness and does not alter the interactive path.
Overview
Non-interactive
hypeman execno longer reports success unless the server sends a valid{"exitCode":N}text message.runExecNonInteractivedrops thedoneChfallback that returned exit 0 when the WebSocket closed cleanly, and treats any read error (including normal close or a dropped connection) as failure with "websocket closed before receiving an exit code". Exit codes are parsed with a nullable*intso malformed JSON,null, or missing values do not count as success. Interactive TTY exec behavior is unchanged.Adds WebSocket harness tests for propagating a nonzero exit code and for the cases that must now error instead of exiting 0.
Reviewed by Cursor Bugbot for commit 26e80bb. Bugbot is set up for automated code reviews on this repo. Configure here.