Skip to content

fix(tui_gateway): offload remote-media attach handlers to the RPC pool - #41510

Open
vondelomlo wants to merge 1 commit into
NousResearch:mainfrom
vondelomlo:fix/offload-media-attach-rpc
Open

fix(tui_gateway): offload remote-media attach handlers to the RPC pool#41510
vondelomlo wants to merge 1 commit into
NousResearch:mainfrom
vondelomlo:fix/offload-media-attach-rpc

Conversation

@vondelomlo

Copy link
Copy Markdown
Contributor

What does this PR do?

Stops the gateway from freezing while a remote PDF or image upload is processed.

dispatch() only offloads methods listed in _LONG_HANDLERS to the thread
pool; everything else runs inline on the single reader loop, which reads the
next RPC only after the current handler returns. The remote-media-relay
feature (16786f3) added two genuinely long-running handlers that were left
off that list:

  • pdf.attach shells out to pdftoppm with a 120s timeout.
  • image.attach_bytes base64-decodes and writes multi-MB uploads to disk.

While either runs inline, inbound RPCs — notably session.interrupt and
approval.respond — sit unread in the pipe/socket, so the UI appears hung for
the full render/decode duration and the user cannot interrupt the turn or
answer an approval prompt. The old image.attach only resolves a local path,
so it stays correctly inline.

Fix: register both new handlers in _LONG_HANDLERS so dispatch() routes them
to the pool and frees the reader loop immediately — the exact pattern already
used by slash.exec, shell.exec, and session.compress.

Related Issue

N/A

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tui_gateway/server.py: add pdf.attach and image.attach_bytes to the _LONG_HANDLERS frozenset; refresh the explaining comment to cover the media-relay handlers.
  • tests/tui_gateway/test_protocol.py: add a parametrized test proving each media handler is offloaded and a concurrent fast handler completes in <0.5s instead of blocking behind it.
  • scripts/release.py: register the contributor email in AUTHOR_MAP.

How to Test

  1. scripts/run_tests.sh tests/tui_gateway/test_protocol.py — full file green.
  2. Targeted: pytest tests/tui_gateway/test_protocol.py -k "media or dispatch" -q — 16 pass, including test_dispatch_media_attach_does_not_block_fast_handler[pdf.attach] and [image.attach_bytes].
  3. Before the fix the new test blocks ~5s on the fast handler and fails the <0.5s assertion; after the fix it passes.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run the relevant tests and they pass
  • I've added tests for my changes
  • I've tested on my platform: macOS 15.5

Documentation & Housekeeping

  • I've updated relevant documentation (docstrings/comment) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact — N/A (pure dispatch routing, no platform-specific code)
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

## What does this PR do?

Stops the gateway from freezing while a remote PDF or image upload is processed.

`dispatch()` only offloads methods listed in `_LONG_HANDLERS` to the thread
pool; everything else runs inline on the single reader loop, which reads the
next RPC only after the current handler returns. The remote-media-relay
feature (16786f3) added two genuinely long-running handlers that were left
off that list:

- `pdf.attach` shells out to `pdftoppm` with a 120s timeout.
- `image.attach_bytes` base64-decodes and writes multi-MB uploads to disk.

While either runs inline, inbound RPCs — notably `session.interrupt` and
`approval.respond` — sit unread in the pipe/socket, so the UI appears hung for
the full render/decode duration and the user cannot interrupt the turn or
answer an approval prompt. The old `image.attach` only resolves a local path,
so it stays correctly inline.

Fix: register both new handlers in `_LONG_HANDLERS` so `dispatch()` routes them
to the pool and frees the reader loop immediately — the exact pattern already
used by `slash.exec`, `shell.exec`, and `session.compress`.

## Related Issue

N/A

## Type of Change

- [x] 🐛 Bug fix (non-breaking change that fixes an issue)

## Changes Made

- `tui_gateway/server.py`: add `pdf.attach` and `image.attach_bytes` to the `_LONG_HANDLERS` frozenset; refresh the explaining comment to cover the media-relay handlers.
- `tests/tui_gateway/test_protocol.py`: add a parametrized test proving each media handler is offloaded and a concurrent fast handler completes in <0.5s instead of blocking behind it.
- `scripts/release.py`: register the contributor email in `AUTHOR_MAP`.

## How to Test

1. `scripts/run_tests.sh tests/tui_gateway/test_protocol.py` — full file green.
2. Targeted: `pytest tests/tui_gateway/test_protocol.py -k "media or dispatch" -q` — 16 pass, including `test_dispatch_media_attach_does_not_block_fast_handler[pdf.attach]` and `[image.attach_bytes]`.
3. Before the fix the new test blocks ~5s on the fast handler and fails the `<0.5s` assertion; after the fix it passes.

## Checklist

### Code

- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits
- [x] I searched for existing PRs to make sure this isn't a duplicate
- [x] My PR contains only changes related to this fix
- [x] I've run the relevant tests and they pass
- [x] I've added tests for my changes
- [x] I've tested on my platform: macOS 15.5

### Documentation & Housekeeping

- [x] I've updated relevant documentation (docstrings/comment) — or N/A
- [x] I've updated cli-config.yaml.example if I added/changed config keys — N/A
- [x] I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
- [x] I've considered cross-platform impact — N/A (pure dispatch routing, no platform-specific code)
- [x] I've updated tool descriptions/schemas if I changed tool behavior — N/A
@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/) labels Jun 7, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. The premise still holds on current main: tui_gateway/server.py:1271 invokes every method absent from _LONG_HANDLERS inline, while image.attach_bytes performs decoding/disk writes (tui_gateway/server.py:9601-9619) and pdf.attach runs pdftoppm with a 120-second timeout (tui_gateway/server.py:9723-9728). Adding these two names follows the existing pool-routing contract.

The related merged PR #53895 covered the separate complete.path and complete.slash handlers, not these media methods.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants