Skip to content

fix(terminal): stop the silent output blackout when stdout fd >= FD_SETSIZE - #553

Merged
Kyzcreig merged 1 commit into
mainfrom
fix/drain-select-fdsetsize
Aug 10, 2026
Merged

fix(terminal): stop the silent output blackout when stdout fd >= FD_SETSIZE#553
Kyzcreig merged 1 commit into
mainfrom
fix/drain-select-fdsetsize

Conversation

@Kyzcreig

Copy link
Copy Markdown
Collaborator

The bug

The terminal tool intermittently returns {"output": "", "exit_code": 0} for every command — including a bare echo — while execute_code keeps working. In the same window write_file fails its own post-write verification with wrote N chars, read back 0 chars ... The write did not persist.

This has happened at least three times (2026-08-07, 2026-08-09 x2). Each instance ended at "a gateway restart cleared it". It was never root-caused, and the empty-output-with-exit-0 shape is actively dangerous: it is indistinguishable from a command that legitimately printed nothing, so an empty ls reads as "the directory is empty" and a mutating command that never ran reads as success.

Root cause

BaseEnvironment._wait_for_process's drain thread polled the subprocess stdout pipe with select.select():

try:
    ready, _, _ = select.select([fd], [], [], 0.1)
except (ValueError, OSError):
    break  # fd already closed

select(2) cannot represent a file descriptor at or above FD_SETSIZE (1024 on Linux and macOS). Above it, Python raises ValueError: filedescriptor out of range in select(). That bare break swallowed the exception, the collector stayed empty, and the result was an empty capture carrying the command's real exit code.

This accounts for every documented symptom:

Observation Explanation
Intermittent; worsens with gateway uptime fds only cross 1024 once enough accumulate. This tree already carries two fixed fd-leak regressions in long-running gateways (NousResearch#69567 cron ledger; gateway/delivery_ledger), and gateways run with RLIMIT_NOFILE=4096 — well above FD_SETSIZE.
write_file degrades at the same time file_operations._execenv.execute() → this same drain. Its cat read-back returned "", so the verification honestly reported a mismatch. The write itself was fine.
execute_code unaffected Separate runtime; does not use this drain.
A gateway restart cures it The new process starts with low fds.
It can also self-clear with no restart fds get released; a later spawn lands back below 1024.
env -i /bin/bash --noprofile --norc also blank The shell was always healthy. The fault is downstream of it, in the capture path.

The fix

1. poll() instead of select() in both drain loops (tools/environments/base.py, tools/process_registry.py). poll(2) takes an array of pollfd structs and has no FD_SETSIZE ceiling, so the failure mode cannot recur. EINTR is now retried rather than treated as end-of-stream. Windows keeps the existing blocking-read path (it supports neither call on pipes).

2. Fail loud instead of silent. The drain records any abnormal abort reason; _finalize_wait_result prepends an OUTPUT CAPTURE FAILED marker, sets a drain_error key on the result, and logs at error level. Even if some future capture failure slips through, it can never again masquerade as "the command printed nothing".

Part 2 is deliberately independent of part 1 — it is the general guard against this whole class.

Verification

RED-proof — reverting only tools/environments/base.py (keeping the new tests):

FAILED tests/tools/test_base_environment_high_fd_drain.py::TestDrainFailsLoud::test_partial_output_is_kept_below_the_marker
FAILED tests/tools/test_base_environment_high_fd_drain.py::TestDrainFailsLoud::test_clean_drain_is_untouched
FAILED tests/tools/test_base_environment_high_fd_drain.py::TestDrainFailsLoud::test_drain_error_prepends_marker_and_sets_key
FAILED tests/tools/test_base_environment_high_fd_drain.py::test_high_fd_stdout_is_still_captured
4 failed, 2 passed in 0.51s

E       AssertionError: high-fd stdout was silently dropped — this is the blackout bug
E       assert 'HIGH_FD_MARKER_OK' in ''

That assert ... in '' is the production signature, reproduced on demand.

End-to-end through the real tool paths (LocalEnvironment.execute and ShellFileOperations.write_file, fds burned past 1024). Unpatched tree:

burned 552 pipe pairs; top fd = 1106 (FD_SETSIZE=1024)
[1] LocalEnvironment.execute('echo ...')
    returncode = 0
    output     = ''
[2] ShellFileOperations.write_file()
    error        = Post-write verification failed ... (wrote 29 chars, read back 0 chars
                   after normalizing line endings). The write did not persist ...
=== VERDICT ===
FAIL: terminal execute() returned an empty/short capture
RC=1

Both production symptoms reproduced verbatim, including the exact read back 0 chars string. Patched tree, same harness:

[1] output = 'E2E_TERMINAL_MARKER_OK\n'   drain_error = None
[2] error = None   bytes_written = 29   verified = True
=== VERDICT ===
PASS: both real tool paths captured output correctly at fd > 1024
RC=0

Regression suites572 passed, 5 skipped, 0 failed across tests/tools/ -k "terminal or environment or process_registry or file_op or file_tool". ruff check clean on all three changed files.

Notes for review

  • The new test's fixture asserts the pipe actually landed above FD_SETSIZE before running, so it cannot pass vacuously if fd pressure fails to materialise; it skips instead.
  • process_registry.py got the same treatment as the sibling call path — it had the identical select() + bare-break shape and would silently drop a background process's entire output under the same conditions.
  • The drain_error key is additive; existing consumers reading output/returncode are unaffected.

…ETSIZE

The terminal tool intermittently returned `{"output": "", "exit_code": 0}`
for every command — including a bare `echo` — while `execute_code` kept
working. In the same window `write_file` failed its own post-write
verification with "wrote N chars, read back 0 chars ... The write did not
persist". Three occurrences (2026-08-07, 2026-08-09 x2); each ended at
"a gateway restart cleared it", never root-caused.

Root cause: `BaseEnvironment._wait_for_process`'s drain thread polled the
subprocess stdout pipe with `select.select()`. select(2) cannot represent a
file descriptor at or above FD_SETSIZE (1024) and raises
`ValueError: filedescriptor out of range in select()`. The drain loop
swallowed that with a bare `break`, so the collector stayed empty and the
result was an empty capture with the command's real exit code — identical
in shape to a command that legitimately printed nothing.

This explains every observed symptom:

* Intermittent, and worsens with gateway uptime — fds only cross 1024 after
  enough accumulate. The tree already carries two fixed fd-leak regressions
  in long-running gateways (NousResearch#69567, gateway/delivery_ledger), and the
  gateways run with RLIMIT_NOFILE=4096, well above FD_SETSIZE.
* `write_file` degrading simultaneously — `file_operations._exec` runs
  through `env.execute()` -> this same drain. Its `cat` read-back returned
  "", so the verification honestly reported a mismatch.
* `execute_code` unaffected — separate runtime, not this drain.
* A gateway restart cures it — the new process starts with low fds.
* It can also self-clear — fds get released, and a later spawn lands back
  below 1024.
* `env -i /bin/bash --noprofile --norc` also blank — the shell was always
  healthy; the fault is downstream of it.

Fix, in two parts:

1. Replace `select()` with `poll()` in both drain loops
   (`environments/base.py`, `process_registry.py`). poll(2) takes an array
   of pollfd structs and has no FD_SETSIZE ceiling, so the failure mode
   cannot recur. EINTR is retried rather than treated as end-of-stream.
   Windows keeps the existing blocking-read path (it has neither call for
   pipes).

2. Fail LOUD instead of silent. The drain records any abnormal abort reason
   and `_finalize_wait_result` prepends an `OUTPUT CAPTURE FAILED` marker,
   sets a `drain_error` key, and logs at error level. A capture we failed to
   read must never again be indistinguishable from a command that printed
   nothing.

Verification:

* RED-proof: reverting only `tools/environments/base.py` makes 4 of the 6
  new tests fail, with the exact production signature
  `assert 'HIGH_FD_MARKER_OK' in ''`.
* End-to-end against the real tool paths (`LocalEnvironment.execute` and
  `ShellFileOperations.write_file`) with fds burned past 1024: the
  unpatched tree exits 1 and reproduces BOTH production symptoms verbatim,
  including "wrote 29 chars, read back 0 chars ... The write did not
  persist"; the patched tree exits 0 with output and content intact.
* 572 passed / 0 failed across tests/tools -k
  "terminal or environment or process_registry or file_op or file_tool".
* ruff clean on all three changed files.
@Kyzcreig
Kyzcreig added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit efd03b4 Aug 10, 2026
44 checks passed
@Kyzcreig
Kyzcreig deleted the fix/drain-select-fdsetsize branch August 10, 2026 07:00
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