Take managoat_sandbox 0.2.0: a close before the exit frame is an error - #1
Merged
Conversation
… does
`managoat_sandbox` 0.2.0 changes what a command stream that closes without
an exit frame looks like: `{:error, %{ref: ref}, :closed_before_exit}`
rather than a synthesised `{:exit, %{ref: ref}, 0}`
(managoat/managoat_sandbox#4).
`Connection` needed no change, and that is worth saying out loud rather
than leaving to be rediscovered: this protocol has no way for a session
to end without an exit code. The daemon watches the process and reports
what it exits with, so the only route to a missing exit is the connection
going away — which already broadcasts
`{:error, %{ref: ref}, :runner_disconnected}` to every subscriber. The
adapter has never fabricated a zero.
`FakeDaemon`'s `drop` instruction did, though. It emitted an exit frame
with code 0, which passed the old conformance rule by faking exactly the
thing the rule was wrong about. It now stops the socket, so the
subscriber gets the real disconnect through `Connection.terminate/2`, and
the conformance suite's closes-without-an-exit-frame test passes because
the adapter does the right thing rather than because the fake pretends
to. The socket stops `:normal`, not `:shutdown`: it is linked to whoever
started the daemon, and a non-normal exit would take a test process with
it. A script that uses `drop` is the last thing that daemon does, which
the moduledoc now says.
Verified by reverting: with `drop` emitting the old exit frame, the
conformance suite's drop test fails.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016SwfnLHB6NXqB2t6ZCbBAd
jhgaylor
added a commit
to BinaryBourbon/fountain
that referenced
this pull request
Sep 3, 2026
…e is an error (#1363) (#1470) `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. Claude-Session: https://claude.ai/code/session_016SwfnLHB6NXqB2t6ZCbBAd Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Takes
managoat_sandbox ~> 0.2.0, where a command stream that closes without an exit frame is{:error, %{ref: ref}, :closed_before_exit}rather than a synthesised{:exit, %{ref: ref}, 0}(managoat/managoat_sandbox#4, from BinaryBourbon/fountain#1363).Connectionneeded no change, and that is the interesting partThis protocol has no way for a session to end without an exit code. The daemon watches the process and reports what it exits with, so the only route to a missing exit is the connection going away — and that already broadcasts
{:error, %{ref: ref}, :runner_disconnected}to every subscriber. The adapter has never fabricated a zero, which is why the contract change costs it nothing.FakeDaemon'sdropdid fabricate oneIt emitted an exit frame with code 0, so it passed the old conformance rule by faking precisely the thing the rule was wrong about. It now stops the socket, and the subscriber gets the real disconnect through
Connection.terminate/2— the conformance suite's closes-without-an-exit-frame test passes because the adapter is right, not because the fake pretends.Two details worth reviewing:
:normal, not:shutdown. It is linked to whoever started the daemon (a test process in every current caller), and a non-normal exit would take that process down.dropends the whole connection, so a script that uses it is the last thing that daemon does. The moduledoc says so, next to why the protocol leaves no smaller option.Verification
mix test --cover— 56 tests, 0 failures, 91.29% (gate 85).mix format --check-formatted,mix compile --warnings-as-errors,mix credo --strict,MIX_ENV=dev mix dialyzer(0 errors),MIX_ENV=dev mix hex.build.dropemitting the old exit frame, the conformance suite's drop test fails.Released as 0.2.0 (minor: it requires a new major-in-practice of its one library dependency, and the
FakeDaemonvocabulary behaves differently). A Fountain pin bump follows, together withmanagoat_runtimes.🤖 Generated with Claude Code
https://claude.ai/code/session_016SwfnLHB6NXqB2t6ZCbBAd