Take managoat_sandbox 0.2.0: a close before the exit frame is an error, not exit 0 - #1470
Merged
Conversation
…e is an error (#1363) `managoat_sandbox`, `managoat_runner` and `managoat_runtimes` all go to 0.2.0. The contract they carry has changed: a command stream that closes without an exit frame is `{:error, %{ref: ref}, :closed_before_exit}`, where it used to be a synthesised `{:exit, %{ref: ref}, 0}`. The upstream `sprites` client made that change in 0.2.2 and the libraries adopted it, because an unexplained close was never a clean exit — reporting one is the mechanism behind #880, where all 533 exit codes we had ever recorded turned out to be synthetic zeros. Two things here were reading that zero. `ConversationServer`'s `:exit` handler wrote a **completed** turn with exit code 0 for a mid-turn close: a turn that never finished, recorded as a clean one, in the conversation the user is watching. The `{:error, ...}` branch beside it — the #413 handler — already does the right thing with any reason, so the frame simply lands there now and the turn fails. The new test pins that, because the correctness here is which handler the frame reaches, and nothing about the handlers themselves says so. `TurnMachine.drain_exited_command/1` drains a mailbox after a stdin write comes back `:command_exited`, and matched only the exit frame. The error frame is as stranded as the output around it — `current_command_ref` is unset on that path, so leaving it costs the full 50ms deadline and then drops it, which is #608 again. It ends the drain now, with a nil code, because nobody measured one. Nothing else in the tree matched `{:exit, _, 0}` on a dropped connection. Library side, for the record: managoat/managoat_sandbox#4 (the contract, the conformance rule, the E2B adapter, an exact 0.2.2 pin with a dependency-contract test on the close path), managoat/managoat_runner#1 (the protocol cannot lose an exit, so only the FakeDaemon's `drop` was faking one) and managoat/managoat_runtimes#4, where the fabricated zero had been reporting a `codex login` whose transport died as a login that worked. Closes #1363. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016SwfnLHB6NXqB2t6ZCbBAd
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.
Closes #1363.
managoat_sandbox,managoat_runnerandmanagoat_runtimesall go to 0.2.0, and the contract they carry has changed: a command stream that closes without an exit frame is{:error, %{ref: ref}, :closed_before_exit}where it used to be a synthesised{:exit, %{ref: ref}, 0}. The upstreamspritesclient made that change in 0.2.2 and the libraries adopted it, because an unexplained close was never a clean exit — reporting one as a clean exit is the mechanism behind #880, where all 533 exit codes we had ever recorded turned out to be synthetic zeros.What was reading that zero here
ConversationServerwrote a completed turn for a dropped connection. A mid-turn close arrived as{:exit, _, 0}and landed on the:exithandler, which writesstatus: "completed", exit_code: 0— a turn that never finished, recorded as a clean one, in the conversation the user is watching. The{:error, ...}branch beside it (the #413 handler) already does the right thing with any reason, so the frame simply lands there now and the turn fails. The new test pins it: the correctness is which handler the frame reaches, and nothing about the two handlers says so on its own.TurnMachine.drain_exited_command/1matched only the exit frame. It drains a mailbox after a stdin write comes back:command_exited, and the error frame is as stranded as the output around it —current_command_refis unset on that path, so leaving it there costs the full 50 ms deadline and then drops it, which is #608 over again. It ends the drain now, with a nil code, because nobody measured one.Nothing else in the tree matched
{:exit, _, 0}on a dropped connection (conversation_server.exandturn_machine.exare the only two consumers of the frames).The library side
spawn_dropasserts the error frame and refutes an exit behind it),Sprites.Errorsclassifying the reason as{:unavailable, :closed_before_exit}soRetry.transient?/1retries a wake, the E2B adapter (which synthesised a zero in two places), and an exact0.2.2pin with a dependency-contract test that drivesSprites.Command's close path directly:runner_disconnected. Only theFakeDaemon'sdropwas faking a zeroCodex.prepare_sandbox/3matched{:exit, _, 0}as a successfulcodex login --with-api-key, so a transport that died mid-login was reported as a login that worked and provisioning carried on against a sandbox with no~/.codex/auth.jsonVerification
mix precommiton this branch: compile with warnings-as-errors, unused deps, format,credo --strict(5,970 mods/funs, no issues), sobelow, dialyzer (0 errors), and the suite.main, the full suite ran 4,114 tests, 0 failures against the three libraries as local path deps — that run is what checked the library changes end to end from here.Fountain.ActivationTest"first_reply_by_user/0 the earliest replied turn, across conversations". It passes on its own (15 tests, 0 failures), and the cause is in the test —at(4)is evaluated once when the row is written and again in the assertion, so the two differ by a second whenever the test straddles a second boundary, andDateTime.compare/2returns:lt. Filed separately rather than fixed here.test/fountain/conversations/directory plus the sandbox reaper (606 tests), and the runner context and controller (43 tests).🤖 Generated with Claude Code
https://claude.ai/code/session_016SwfnLHB6NXqB2t6ZCbBAd