Skip to content

fix(fleet): move Fleet Manager TUI blocking I/O off the Textual event loop - #446

Merged
Jason Robert (jrob5756) merged 1 commit into
mainfrom
fix/437-fleet-tui-blocking-io
Aug 16, 2026
Merged

fix(fleet): move Fleet Manager TUI blocking I/O off the Textual event loop#446
Jason Robert (jrob5756) merged 1 commit into
mainfrom
fix/437-fleet-tui-blocking-io

Conversation

@jrob5756

Copy link
Copy Markdown
Collaborator

Closes#437.

What was wrong

Three sites in the Fleet Manager TUI did blocking filesystem and subprocess
work directly on the Textual event loop, so the whole UI stopped responding
while they ran:

  1. screens/runs.py::refresh_runs — every ~2s, on the event loop: a
    directory scan, a JSON parse per record, a per-record liveness probe, a
    destructive prune, and a derive_run_summary() per row. This scales with
    fleet size, on the screen a user leaves open.
  2. screens/history.pybuild_history_entries() read up to
    keep_last (default 200) event logs in full before History painted.
  3. The dashboard-open path_wsl_open shells out to powershell.exe
    with a 15s timeout; on a host where that is slow, the TUI froze for the
    full 15s with no sign it was alive.

What changed

Each now runs in a worker (@work async def + await asyncio.to_thread(<collector>)), with the render half back on the event
loop after the await — matching the pattern screens/new_run.py,
step_detail.py, registries.py and providers.py already established.

Two sites beyond the three in the issue get the same treatment:

  • run_detail.py's poll, which has the same shape as Runs'.
  • actions.py::kill_runsstop_records's signal-and-poll escalation
    ladder can take seconds, and being an async def does not move a blocking
    call off the loop by itself.

Design notes

  • An overrunning poll drops the next tick rather than queueing behind it.
    Each screen's refresh_*() entry point stays a synchronous dispatcher
    (so it is still callable from on_mount, set_interval and action
    handlers), guarded by a plain _refreshing boolean. Deliberately not
    @work(exclusive=True), which would cancel the in-flight worker instead of
    skipping the new tick.
  • The _refreshing flag is set in the synchronous dispatcher, not as the
    first line of the worker — a @work method's body does not start running
    until the event loop schedules it, so setting it inside would let a second
    tick slip through.
  • A dedicated Static (#runs-loading / #detail-loading /
    #history-loading) shows a dim muted("Loading…") line from on_mount,
    hidden once the first result lands, covering the frame between mount and
    the first collector result.
  • Collectors are module-level functions so they are trivially unit-testable
    and safe to hand to asyncio.to_thread.

Test caution worth knowing about

A pilot test that presses a key opening a modal (kill confirm, gate options)
and then needs a second keypress to resolve it must use a plain await pilot.pause() between the two, never tests/test_fleet/conftest.py::settle.
settle awaits app.workers.wait_for_complete(), and the action's own
@work method is suspended on push_screen_wait until that second
keypress — so awaiting it first deadlocks the test. This is documented in
AGENTS.md alongside the rest.

Validation

  • uv run pytest tests/test_fleet tests/test_cli/test_bg_runner.py — 666
    passed, 1 skipped.
  • make check — ruff, ruff format, and ty all clean.

AGENTS.md, CHANGELOG.md and docs/fleet.md updated.

… loop
Three sites in the Fleet Manager TUI did blocking filesystem and
subprocess work directly on the Textual event loop, freezing the whole
UI while they ran (#437):
- `screens/runs.py::refresh_runs` — a directory scan, per-record JSON
parse, liveness probe, destructive prune and per-row
`derive_run_summary()` on every ~2s poll tick, scaling with fleet size
on the screen a user leaves open.
- `screens/history.py` — `build_history_entries()` reading up to
`keep_last` (default 200) event logs in full before the screen paints.
- the dashboard-open path — `_wsl_open` shelling out to `powershell.exe`
with a 15s timeout, with no sign the TUI was still alive.
Each now runs in a worker (`@work async def` + `await
asyncio.to_thread(<collector>)`), with the render half back on the event
loop after the await, matching the pattern `screens/new_run.py` already
established. The same treatment is applied to `run_detail.py`'s poll and
to `actions.py::kill_runs`, whose `stop_records` signal-and-poll
escalation ladder can take seconds even though the caller is already
`async def` — being a coroutine does not move a blocking call off the
loop by itself.
Each polled screen's `refresh_*()` entry point stays a synchronous
dispatcher (still callable from `on_mount`, `set_interval` and action
handlers) guarded by a plain `_refreshing` boolean, so a tick arriving
mid-scan is dropped rather than started alongside the in-flight one —
deliberately not `@work(exclusive=True)`, which would cancel the
in-flight worker instead. An *explicit* refresh (after a kill, or after a
gate is resolved) is coalesced via `_refresh_pending` rather than
dropped: those callers promise the table reflects what they just did, and
the scan they collide with started before it. The guard flags are also
released if worker *dispatch* raises, since they are set before the
worker body exists.
Splitting the poll into a collector and a renderer moved three failure
paths, and each would otherwise land somewhere that looks like a working
app:
- The notifier was pruned against the rows that *rendered* rather than
the records that were *read*, so a run whose summary failed to derive
on one tick was forgotten and re-fired its gate/failure notification on
the next successful one. `_collect_runs` returns a `RunScan` carrying
`seen_run_ids` and `failed` alongside the rows.
- `_render_runs` branched on "no rows" where it used to branch on "no
records", so a fleet whose summaries all failed rendered as the "No
runs — press n to launch one" empty state, with `_displayed_records`
emptied so `k`/`K`/`enter` became silent no-ops.
- A failed directory scan skipped the tick silently: on first load that
left a "Loading…" line that never resolved, and mid-session a table
frozen at its last good contents with no staleness marker.
- History degraded a read failure into "No run history yet." — a positive
claim of absence, permanent because that screen loads once with no poll
— instead of the red-error-line convention `step_detail.py` and
`registries.py` already set for this worker pattern.
Also surfaces the `stop_records` diagnostics the silent console was
discarding (including the `--force` remedy for an unconfirmed kill), adds
the missing `except` to `_open_dashboard_worker`, guards History's render
like the other two screens since `@work` defaults to `exit_on_error`, and
drops four `if not self.is_mounted: return` guards that never fire —
Textual never resets `_is_mounted`, and `Widget._on_unmount` already
cancels the worker.
Every new test was verified against a mutant that reintroduces the bug it
covers, after the first version of the notifier test was found to pass
against the regression. `settle()` is bounded with a timeout, since this
project has no pytest-timeout and the documented `push_screen_wait`
misuse hung the entire run rather than failing one test.
@jrob5756

Copy link
Copy Markdown
CollaboratorAuthor

Review round applied

Ran a seven-agent code review over this PR. Three agents independently found the same regression, and two reproduced it empirically, so the branch has been rebased onto main and force-pushed with the fixes squashed in.

Blocking, all fixed

  1. Duplicate gate/failure notifications. The notifier was pruned against the rows that rendered rather than the records that were read, so a run whose summary failed to derive on one tick was forgotten and re-fired on the next successful one — the once-per-transition contract _refresh_worker's own docstring exists to protect. Reproduced: main fired 1 notification, this branch fired 2. _collect_runs now returns a RunScan carrying seen_run_ids and failed alongside the rows.
  2. A total derivation failure rendered as the "No runs — press n to launch one" empty state, with _displayed_records emptied so k/K/enter became silent no-ops. An operator was affirmatively told nothing was running while their workflows were still burning tokens.
  3. A persistent scan failure left the screen on "Loading…" forever — no table, no empty state, no notification, no timeout.
  4. History rendered a read failure as "No run history yet." — a positive claim of absence, permanent because that screen loads once with no poll to self-heal.

Also addressed

Guard flags are released if worker dispatch raises; an explicit post-kill/post-gate refresh is coalesced via _refresh_pending rather than dropped (those callers promise the table reflects what they just did, and on the slow-fleet case this PR targets a scan is in flight most of the time); History's render is guarded like the other two screens, since @work defaults to exit_on_error; _open_dashboard_worker gets its missing except; the stop_records diagnostics the silent console was discarding are now logged, including the --force remedy for an unconfirmed kill; and the four if not self.is_mounted: return guards are gone — Textual never resets _is_mounted (verified against the source, a pop probe, a 40/40 race probe and coverage), and Widget._on_unmount already cancels the worker.

Nits: the loading line is declared in compose() (loading.display = True was a no-op), reuses the existing unused .notice app class instead of new per-screen CSS, and "Loading…" moved into theme.py — five screens were spelling it three ways.

Verification

Every new test was checked against a mutant that reintroduces the bug it covers. That mattered: the first version of the notifier test passed against the regression, because a total-failure fixture takes the new error branch and never reaches the prune. It now uses a partial failure and genuinely kills the mutant. Eight mutants, eight caught.

settle() is now bounded — this project has no pytest-timeout, so the documented push_screen_wait misuse hung the entire run rather than failing one test.

Rebased onto #448, which landed in the same two files. Its test_the_poll_keeps_running_under_a_modal needed a settle() since the scan is no longer synchronous; I mutation-checked that its contract ("only the render is suppressed while covered, never the poll") is still enforced afterwards.

575 fleet tests pass; make check clean.

The one recommendation I did not take: extracting a PollingScreen base class. The reviewing agent argued against it and I agree — the two polled screens differ in failure contract, and 5 of the 7 threaded-load screens would not use it.

@jrob5756
Jason Robert (jrob5756) merged commit 406cc96 into mainAug 16, 2026
11 checks passed
Sign up for freeto 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.

fleet TUI: blocking I/O on the Textual event loop (2s poll, History mount, WSL dashboard open)

1 participant

@jrob5756