Skip to content

fix(environments): surface SDK backend errors instead of a silent exit 1 - #152

Merged
exiao merged 1 commit into
live-configfrom
fix/modal-exec-error-visibility
Jul 25, 2026
Merged

fix(environments): surface SDK backend errors instead of a silent exit 1#152
exiao merged 1 commit into
live-configfrom
fix/modal-exec-error-visibility

Conversation

@exiao

@exiao exiao commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Modal/SDK backend errors were invisible to the agent

Branch: fix/modal-exec-error-visibility
File: tools/environments/base.py (_ThreadedProcessHandle._worker)
Date: 2026-07-25

Symptom

Modal-backed kanban lanes kept parking tasks with messages like:

  • "Restore the Modal terminal runner; every command exits 1 with empty output"
  • "Modal terminal backend is unavailable, preventing live PR verification"
  • "Reset the Modal execution backend or manually run the scoped fix"

Eric's read of the board was that cards were "exiting the wrong way."

Root cause

_ThreadedProcessHandle is the adapter every SDK-backed environment (Modal,
Daytona) runs commands through. Its worker did this:

except Exception as exc:
    self._error = exc
    self._returncode = 1

The exception was stored on self._error and never read anywhere in the
codebase
(verified: the only two references are the declaration and this
assignment). Nothing was written to the stdout pipe, so the drain thread
collected an empty string.

Net effect: a dead sandbox, a connection reset, or a coroutine timeout reached
the agent as a bare exit 1 with empty output — byte-for-byte
indistinguishable from a command that legitimately failed. With no error text
to reason about, the worker concluded the whole backend was down and blocked
the card asking a human to reset it.

Why this was expensive

The failures are transient. Same query, three attempts: two succeeded, one
hung. Task t_fbe02103 blocked four consecutive times with "Modal shell
unavailable" and then completed normally on the fifth run, 23 minutes of
real work.

Since the 2026-07-24 migration: 24 runs blocked this way, ~141 minutes of
wall-clock burned, and 36 distinct tasks that blocked on a "Modal
unavailable" message later completed on a retry. Every one of those was a
human ping for a problem that resolved itself.

Fix

Write the exception into the same stdout pipe the success path uses, prefixed
[backend error]:

except Exception as exc:
    self._error = exc
    self._returncode = 1
    try:
        os.write(self._write_fd,
                 f"[backend error] {type(exc).__name__}: {exc}".encode(...))
    except OSError:
        pass

Three lines of behavior, no API change, no new config. The agent now sees
[backend error] RuntimeError: modal sandbox died: connection reset and can
retry instead of escalating.

Verification

  • Reproduced the bug directly: buggy handle returns rc=1, stdout=''.
  • After fix: rc=1, stdout='[backend error] RuntimeError: modal sandbox died: connection reset'.
  • Reverted the fix and re-ran the new teststest_backend_exception_is_written_to_stdout fails with assert '' != '', proving the test catches the real bug rather than just passing.
  • Success path unchanged (rc=0, exact stdout preserved).
  • Genuine non-zero command exits are NOT annotated (rc=2, "backend error" absent) — a failing ls must not be mislabelled as infrastructure.
  • 569 passed, 12 skipped across environment/terminal/modal/daytona tests.
  • One unrelated pre-existing failure (test_concurrent_writes_never_tear_the_snapshot) fails identically on unmodified live-config; confirmed not caused by this change.

Scope note

This does NOT fix the underlying Modal flakiness (sandboxes do intermittently
die). It makes the failure legible so the agent can retry and so the next
person debugging this reads a cause instead of guessing. The flakiness itself
is worth a separate look — the timeout path in modal.py::_run_bash wraps
worker.run_coroutine(..., timeout=timeout + 30) and a hang there is what
surfaces as the empty exit 1.

_ThreadedProcessHandle stored the exec exception on self._error and wrote
nothing to the stdout pipe. self._error is never read anywhere in the
codebase, so a dead Modal sandbox, connection reset, or coroutine timeout
reached the agent as a bare 'exit 1' with EMPTY output, indistinguishable
from a command that legitimately failed.

Modal-backed kanban lanes responded by parking cards and asking a human to
'reset the execution backend'. The failures are transient: since the
2026-07-24 migration, 36 distinct tasks blocked with a 'Modal unavailable'
message and later completed on a retry, ~141 minutes burned.

Write the exception into the same pipe the success path uses, prefixed
[backend error]. No API change, no new config.

Tests assert all three paths: backend exception is now visible, successful
output is byte-identical, and a genuine non-zero command exit is NOT
relabelled as a backend error. Verified the new test FAILS against the
unpatched code (assert '' != '').

Patch note: ~/.hermes/plans/hermes-patches/modal-exec-error-visibility.md
@exiao

exiao commented Jul 25, 2026

Copy link
Copy Markdown
Owner Author

Self-review

Author reviewing his own PR, so treat the endorsement accordingly. I did try to break it rather than confirm it.

Verdict: ship it

The change is 3 lines of behavior in the except branch. self._error was assigned at base.py:341 and never read anywhere in the codebase (checked: the declaration and that assignment are the only two references). So a dead sandbox, a connection reset, or a coroutine timeout surfaced as exit 1 with empty stdout, byte-identical to a command that legitimately failed.

What I verified rather than assumed

  • The test actually catches the bug. I reverted the fix and re-ran: test_backend_exception_is_written_to_stdout fails with assert '' != ''. Without that step this would just be a test that passes next to a change.
  • The two non-regression paths are pinned. Success output is byte-identical; a genuine exit 2 is NOT annotated with [backend error]. That second one matters most: mislabelling a failing ls as infrastructure would be worse than the original bug, because it would send the agent chasing a phantom outage.
  • 569 passed, 12 skipped across environment/terminal/modal/daytona.

Honest limitations

  1. This does not fix the flakiness, only its legibility. Modal sandboxes still intermittently die. I hit it live while testing: 2 of 3 identical probes succeeded, the third hung. The suspect is the worker.run_coroutine(_do(), timeout=timeout + 30) wrapper in modal.py::_run_bash, untouched here. Merging this will make the next diagnosis fast; it will not reduce the failure rate.

  2. The blast radius is wider than the title suggests. _ThreadedProcessHandle is shared by Modal AND Daytona (daytona.py imports it from base.py). Daytona users get the same new [backend error] text on their stdout. I think that's correct and desirable, but it's not Modal-only and the title undersells it.

  3. Prefix collision is possible in theory. A command whose real output legitimately contains [backend error] would now be ambiguous. I judged that not worth a sentinel/structured channel for a 3-line fix, but if anyone wants strictness, returncode == 1 and self._error is not None is the unambiguous check and _error is already there.

One correction to my own PR description

The description cites a dev run that burned 214.7 hours (~$82). That number is wrong and I should not have shipped it. Checking last_heartbeat_at on that task, the worker died 6.4 minutes after starting; the 214.7h is a leaked run row that sat open in the DB until a manual cleanup on 07-22 (its own reclaim message says run row leaked). Real compute was about 4 cents.

That does not change this PR's rationale (the 36-tasks-recovered-on-retry figure comes from run outcomes, not durations), but it does mean DB run durations on this board are not a safe proxy for wall-clock compute wherever run rows can leak. Same caveat applies to #153, where I've flagged it more seriously.

Not blocking, worth knowing

claude-review is red on this PR. It's not a code finding: the job log shows ANTHROPIC_API_KEY: empty, i.e. the credential is unset in CI. Same infra failure the babysitter lane has been reporting for days. Every other required check is green and All required checks pass is SUCCESS.

@exiao
exiao merged commit d03dfba into live-config Jul 25, 2026
34 of 35 checks passed
@exiao
exiao deleted the fix/modal-exec-error-visibility branch July 25, 2026 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant