Skip to content

fix(tui): retain compute-host failures for resume - #110182

Open
JoaoMarcos44 wants to merge 1 commit into
NousResearch:mainfrom
JoaoMarcos44:fix/110106-compute-host-failure-retention
Open

JoaoMarcos44 wants to merge 1 commit into
NousResearch:mainfrom
JoaoMarcos44:fix/110106-compute-host-failure-retention

Conversation

@JoaoMarcos44

@JoaoMarcos44 JoaoMarcos44 commented Sep 13, 2026 •

Copy link
Copy Markdown

Summary

Preserve a failed compute-host turn for reconnect and resume instead of deleting the only in-flight failure snapshot before the terminal error is delivered.

This is a complementary fix for #110106. Complementary to #110113. It addresses the failure-survival half of the issue without changing the WAL lifecycle or duplicating the profile-store anchor in #110113.

Fixes #110106

Reproduction and premise validation

The issue reports that a DeletedWalGenerationError can abort a turn while the client is disconnected or while the compute-host boundary is being restarted. The normal in-process TUI path already retains failed turns, but the compute-host child and its parent mirror had a different lifecycle:

  1. ComputeHost._run_real_turn starts session["inflight_turn"].
  2. An exception escaping the child turn path enters tui_gateway/compute_host.py.
  3. The exception handler set running=False and called _clear_inflight_turn.
  4. The child emitted turn.error only after that snapshot had been discarded.
  5. The parent bridge received turn.error and also called _clear_inflight_turn before emitting its error frame.

A reconnecting client could therefore observe no replayable prompt, partial answer, or failure even though the transport emitted an error.

The regression test was run RED before production changes:

  • scripts/run_tests.sh tests/tui_gateway/test_compute_host_failure_retention.py -q
  • Initial result: 0 passed, 1 failed because the child snapshot was None.
  • After adding the parent-mirror case: 1 passed, 1 failed because the parent snapshot was None.

The failures were the expected missing-behavior failures, not test collection or setup errors.

Root cause

The compute-host boundary treated an exception frame as a transport-only notification. It cleared the in-flight state before the error frame crossed the boundary, unlike the established _fail_inflight_turn contract used by tui_gateway/prompt_turn.py and tui_gateway/session_history.py.

The failure is an ownership/lifecycle bug in the error projection, not a reason to retry SQLite writes or remove the existing deleted-WAL guard. Retrying a lost WAL generation could use a stale handle and create split-brain state.

Fix

  • tui_gateway/compute_host.py

    • Marks the child session as no longer running, then calls _fail_inflight_turn(session, exc) in the exception path.
    • Keeps the original prompt and any streamed assistant text replayable until the next turn or session close.
  • tui_gateway/compute_host_bridge.py

    • Retains the parent mirror with _fail_inflight_turn for turn.error frames.
    • Emits the same terminal error contract with status: "error", error, and recoverable: true.
    • Keeps the existing _clear_inflight_turn behavior for normal turn.end frames.

No WAL, SessionDB, profile path, retry, provider, or credential behavior was changed.

Scope and related work

This PR is intentionally complementary:

The patch does not implement a process singleton or attach/hand-off protocol. A singleton would break supported multiple-client topologies, while the broader control-socket design is tracked separately by #92091.

Behavior contract

For an exception escaping a compute-host turn:

  • session["running"] becomes False.
  • session["inflight_turn"] remains a dict with the original user prompt.
  • The snapshot has status: "error", recoverable: true, and the exact failure message.
  • The client receives one message.complete error projection from the parent bridge.
  • A normal successful turn.end still clears the in-flight snapshot.

Tests

Focused regression and neighboring paths:

  • scripts/run_tests.sh tests/tui_gateway/test_compute_host_failure_retention.py -q
    • 2 passed, 0 failed.
  • scripts/run_tests.sh tests/tui_gateway/test_compute_host_failure_retention.py tests/tui_gateway/test_compute_host_turn_protocol.py tests/tui_gateway/test_tui_gateway_ws.py -q
    • 19 passed, 0 failed.
  • The new regression file was repeated in three independent runner invocations:
    • round 1: 2 passed, 0 failed;
    • round 2: 2 passed, 0 failed;
    • round 3: 2 passed, 0 failed.
  • C:/Users/Nitro/hermes-agent/.venv/Scripts/python.exe -m ruff check tui_gateway/compute_host.py tui_gateway/compute_host_bridge.py tests/tui_gateway/test_compute_host_failure_retention.py
    • All checks passed.
  • python -m py_compile tui_gateway/compute_host.py tui_gateway/compute_host_bridge.py tests/tui_gateway/test_compute_host_failure_retention.py
    • Passed.
  • git diff --check
    • Passed.

Broader compute-host comparison was also run against a clean origin/main worktree. Both base and candidate reproduced the same unrelated Windows failures:

  • tests/tui_gateway/test_compute_host.py::test_compute_host_line_json_hello_and_shutdown — the venv launcher PID differs from Popen.pid on this host.
  • tests/tui_gateway/test_compute_host_phase1.py::test_append_log_record_single_write_lines — concurrent file-write test loses lines on this Windows run.

These failures were not changed by this PR and were reproduced with the same signatures on the clean baseline. The existing compute-host protocol file also has a timing flake where the first attempt can miss the five-second turn.end wait; the canonical runner retries it and reports the historical flake. The final focused command above completed with no failure or flake.

Full tests/tui_gateway/ receipt

The complete TUI gateway directory was also run with:

  • HERMES_PYTHON=/c/Users/Nitro/hermes-agent/.venv/Scripts/python.exe scripts/run_tests.sh tests/tui_gateway/ -q
  • Result: 1,874 passed, 13 failed, 9 skipped across 151 files.
  • The new test_compute_host_failure_retention.py passed 2/2 in this run.

This is not reported as a green full-suite result. The failing files/tests were:

  • test_compute_host.py::test_compute_host_line_json_hello_and_shutdown — Windows venv launcher PID differs from Popen.pid; the same failure was reproduced in the clean baseline.
  • test_entry_import_off_main_thread.py::test_entry_imports_cleanly_from_worker_thread — Windows signal.SIGPIPE is unavailable in the subprocess probe.
  • test_bot_relay_methods.py::test_deliver_write_failure_still_removes_tempfile — temporary relay file remained after the simulated write failure.
  • test_compute_host_turn_protocol.py::test_turn_start_streams_deltas_then_turn_end_with_history_identity — timing timeout; the same file showed the same first-attempt timeout in the clean baseline and passed on the focused retry.
  • test_compute_host_turn_protocol.py::test_interrupt_frame_acks_and_marks_turn_interrupted — timing timeout in the directory-wide run; it passed in the final focused command.
  • test_hosted_room_two_gateway_scoped.py::test_in_process_scoped_transport_contract_finishes_headlessly — peer reply was not published before the test deadline.
  • test_profile_target_unavailable.py::test_explicit_profile_target_never_falls_back — Windows symlink privilege error (WinError 1314).
  • test_kanban_notify_poller.py::TestNotificationPollerLoopKanbanWiring::test_idle_session_gets_status_update_and_agent_turn — agent turn was not dispatched before the deadline.
  • test_isolated_orphan_activity.py::test_real_child_detached_turn_activity[fresh] — activity freshness assertion.
  • test_isolated_orphan_activity.py::test_real_child_detached_turn_activity[stale] — activity freshness assertion.
  • test_isolated_orphan_activity.py::test_real_child_detached_turn_activity[missing] — activity freshness assertion.
  • test_isolated_orphan_activity.py::test_real_child_detached_turn_activity[previous] — activity freshness assertion.
  • test_tui_gateway_server.py::test_model_options_preserves_canonical_custom_row_after_agent_init — existing custom-provider expectation failure; it also fails on the clean baseline.
  • test_serve_exit_flush.py — no tests ran because collection/import timed out.

These failures are retained as limitations and were not suppressed, deleted, or used as evidence against the two-line production behavior changed here. The final focused command for this PR completed with 19 passed and 0 failed.

Graphify and verification state

  • graphify update . --no-cluster completed with exit code 0 after scanning 3,338 uncached files.
  • The update reported four pre-existing syntax warnings in unrelated corpus files; none is in this diff.
  • graphify explain '_on_compute_host_turn_done()' confirms both _fail_inflight_turn and the normal _clear_inflight_turn branches.
  • graphify path was executed for the child/bridge symbols; the graph returned a four-hop relationship through the compute-host protocol test and bridge module.

Complexity and operational impact

The new branch is O(1) per terminal error and adds no work to the healthy streaming path beyond the existing frame-type check. It retains the existing bounded in-memory snapshot, at most one failed turn per session, until the next turn or session close. No additional database I/O, retry, lock, dependency, environment variable, or credential is introduced.

The change is fail-closed for persistence: it does not continue using a stale SQLite handle and does not attempt to mint a replacement WAL. It only preserves the already-created in-memory error snapshot and exposes the existing terminal error contract.

Delivery receipt

  • Base: main at b9271bcb34e1a8b8fe0eeaef0ef4a6e1f93ba543.
  • Head commit: 1efe46577f98d4b666ed83bdf8d6d91cbb80084b.
  • Branch: JoaoMarcos44:fix/110106-compute-host-failure-retention.
  • Changed files: tui_gateway/compute_host.py, tui_gateway/compute_host_bridge.py, tests/tui_gateway/test_compute_host_failure_retention.py.
  • No secrets, credentials, or unrelated working-tree files were included.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Sep 13, 2026

@ehz0ah ehz0ah left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two blocking lifecycle defects remain in the failure-retention path. Both were reproduced at this exact head with production-path probes. The focused regression and neighboring suites passed, but the added tests do not cover these paths.

if turn_error:
_fail_inflight_turn(session, error_message)
else:
_clear_inflight_turn(session)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Bug] (blocking) The normal production failure path still ends with turn.end, so this branch clears the parent snapshot. _run_prompt_submit() catches DeletedWalGenerationError, emits the error message.complete, retains the child snapshot, and returns normally. ComputeHost._run_real_turn() then emits turn.end, which makes _on_compute_host_turn_done() call _clear_inflight_turn(). An exact-head probe retained the child error but left the parent inflight_turn as None, so reconnect and resume still lose the failed turn. The new test replaces _run_prompt_submit() with a raising stub, which only exercises the outer turn.error path. The terminal host frame must carry the real worker outcome so the parent retains failures from the production path.

if turn_error:
_fail_inflight_turn(session, error_message)
else:
_clear_inflight_turn(session)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Bug] (blocking) A queued turn does not replace the retained parent snapshot before it is dispatched. If prompt A fails, this branch keeps A and then _drain_queued_prompt() starts prompt B without calling _start_inflight_turn() for the parent mirror. If B also fails, _fail_inflight_turn() writes B's error into A's snapshot. A direct probe produced user="prompt A" with error="failure B", so resume returns the wrong prompt and error after consecutive failures. Start a fresh parent snapshot whenever a queued compute-host prompt is claimed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

3 participants