Skip to content

fix(desktop): keep clarify dialogs alive while the server bridge is still blocked - #86036

Open
ayushnangia wants to merge 1 commit into
NousResearch:mainfrom
ayushnangia:fix/hud-clarify-vanish
Open

ayushnangia wants to merge 1 commit into
NousResearch:mainfrom
ayushnangia:fix/hud-clarify-vanish

Conversation

@ayushnangia

Copy link
Copy Markdown
Contributor

Summary

Fixes #83319 (Desktop HUD mode: clarify choice dialog appears, disappears, turn hangs — family E desktop member in the stall triage #84047).

The dialog-vanish and the hang are the same bug: the desktop stream pipeline's turn-end (message.complete) and error handlers clear the parked clarify dialog unconditionally. The Python side stays blocked on clarify.respond until the user answers or the server's clarify timeout expires. In HUD mode (separate overlay window with its own stream lifecycle) a spurious turn-end/error event arrives while the bridge is still blocked → dialog wiped → user has nothing to answer → turn hangs until timeout or /stop.

Root cause

  • apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts (~L764, ~L1299): clearClarifyRequest(undefined, sessionId) on turn-end/error with no check of whether the server bridge is still waiting.
  • The normal flow needs a clear somewhere (the tool.complete handler only drops the needsInput flag), so the fix is a deadline-aware guard, not a removal.

Changes

  • Server (tui_gateway/server.py, _block): the blocking-request payload now advertises the bridge deadline — timeout_seconds when finite, omitted when the server waits forever (clarify_timeout <= 0). Additive field; older renderers ignore unknown fields (same contract as the multi_select hint).
  • Client store (apps/desktop/src/store/clarify.ts): ClarifyRequest gains receivedAt + timeoutSeconds; new pure guard clarifyStillBlocking(request, now) — true while a finite server deadline hasn't elapsed, true forever for wait-forever requests, false when nothing is parked.
  • Client stream handler: both turn-end and error clear sites now call clearClarifyRequest only when !clarifyStillBlocking(...).

Semantics

case behavior
user answered, turn ends request already cleared by the tool's answer path — no-op, unchanged
server timed out (finite), turn-end arrives late deadline elapsed → dialog dropped (stale)
spurious turn-end while bridge still blocked dialog survives; user can answer; respond succeeds (server still waiting)
wait-forever request (clarify_timeout <= 0) dialog survives turn-end/error; only an explicit answer/skip clears it
/stop interrupt dialog survives; respond fails gracefully; the #84560 interrupt path unblocks the turn

Validation

  • Desktop: npx vitest run src/store/clarify.test.ts src/app/session/hooks/use-message-stream21 + 79 passed, 0 failed (5 new guard tests: no-request false, pre-deadline true, post-deadline false, wait-forever true, parked survival).
  • Python: scripts/run_tests.sh tests/test_tui_gateway_server.py553 passed, 0 failed (2 new payload tests: finite → timeout_seconds emitted; None → omitted). One unrelated flaky (test_compute_host_turn_end_updates_metadata_mirror) passes on re-run and is untouched by this diff.
  • Red proof: the guard tests fail against the pre-change clear semantics (unconditional clear wipes a still-blocked request).

Credit to @cycocyco for the HUD-mode reproduction; composes with #84560 (interrupt unblocking) — that PR lets /stop out of the wedge, this one stops the wedge from forming.

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) 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 labels Aug 14, 2026
@ayushnangia

Copy link
Copy Markdown
Contributor Author

CI note: the desktop check:lint failure was real and mine — three test fixtures constructed ClarifyRequest without the new fields from the store change (prompts.test.ts, clarify-tool.test.tsx x2). Fixed in 2ce8cbd11 / 3da43b91d / 37f311087. The remaining typecheck errors on this branch are pre-existing main drift (respondToApproval vs @assistant-ui, missing bippy module, fromThreadMessageLike/normalizeMathDelimiters exports) — verified by running the typecheck with this branch's diff stashed: identical errors. They're part of the desktop churn teknium1 is tracking in #76627, not this PR.

@ayushnangia
ayushnangia force-pushed the fix/hud-clarify-vanish branch from 37f3110 to 1f8fdc7 Compare August 14, 2026 11:34
@teknium1
teknium1 temporarily deployed to trusted-automation August 14, 2026 12:18 — with GitHub Actions Inactive
@ayushnangia

Copy link
Copy Markdown
Contributor Author

Reopening — this was closed by my account at 11:34Z with no close comment, which looks like an accidental close (the fix and the CI state were both settled by then). Branch is rebased onto current main; the fix stands: server advertises its bridge deadline, client only drops the clarify dialog once the server can no longer be blocked. If a maintainer sees a reason this shouldn't land, happy to hear it — otherwise it's ready for review.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(desktop): keep clarify dialogs alive while the server bridge is still blocked

  1. clarifyStillBlocking decides staleness from the client clock (now - receivedAt < timeoutSeconds * 1000), but the server's own timeout starts at _block() and receivedAt is set on event arrival. Clock skew or scheduling can make the client conclude "stale" a moment before the server actually gives up, dropping the dialog while the bridge is still blocked — the exact failure the guard exists to prevent. A small safety margin (e.g. +1–2s) or comparing against a server-derived expiry would close that edge.
  2. Forward compatibility: with an older backend that does not emit timeout_seconds, every clarify is treated as wait-forever and never dropped on turn-end, so a stale dialog from a backend that already timed out lingers until the user answers/dismisses. Acceptable degradation, but a code comment making the trade-off explicit would help future maintainers.
  3. setClarifyRequest now requires receivedAt/timeoutSeconds and all callers were updated — good, the TS typecheck enforces it. Minor: in gateway-event.ts the guard reads sessionClarifyRequest(sessionId) before clearClarifyRequest(undefined, sessionId) — confirm sessionClarifyRequest(undefined) and the null-session key use the same keying so a global (null-session) parked clarify is protected too.

@ayushnangia
ayushnangia force-pushed the fix/hud-clarify-vanish branch 3 times, most recently from e42691c to 81d9c03 Compare August 18, 2026 08:00
@alt-glitch alt-glitch added P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint and removed type/bug Something isn't working 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 comp/desktop Electron desktop app (apps/desktop/*) labels Aug 20, 2026
@ayushnangia
ayushnangia force-pushed the fix/hud-clarify-vanish branch from 81d9c03 to 19eedb9 Compare August 22, 2026 10:03
@ayushnangia

Copy link
Copy Markdown
Contributor Author

Great catch — and fixed on the current head:

  • Server: session.interrupt's _clear_pending now emits clarify.cancel for each released clarify prompt (scoped to the interrupted session; sudo/approval/secret prompts stay silent as before).
  • Desktop: new handlers for clarify.cancel AND the previously-unhandled server-side clarify.expire — a parked request is cleared immediately when the wait is gone, regardless of any client-side deadline (your wait-forever case is covered: no deadline can ever make it stale, so the explicit signal has to).
  • Tests: interrupt → clarify.cancel emitted (and non-clarify prompts stay silent); desktop clears the parked request on both events.

Your invariant is now the contract: transient turn-end/error preserves the dialog while the request is live; explicit cancellation/completion invalidates it immediately.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state and removed type/feature New feature or request comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have labels Aug 22, 2026
ayushnangia added a commit to ayushnangia/hermes-agent that referenced this pull request Aug 28, 2026
session.interrupt's _clear_pending released the pending clarify with an
empty answer but told the client nothing — a wait-forever dialog parked
on the desktop stayed actionable-but-dead forever (keeltrace's lifecycle
review on NousResearch#86036). _clear_pending now emits clarify.cancel for released
clarify prompts (scoped to the interrupted session; sudo/approval/secret
prompts stay silent as before).
ayushnangia added a commit to ayushnangia/hermes-agent that referenced this pull request Aug 28, 2026
The server now tells the client when a parked clarify's wait is gone
(cancel from /stop, expire from the deadline). The desktop clears the
parked request immediately — regardless of any client-side deadline —
so an actionable-but-dead dialog can never persist (NousResearch#86036 follow-up).
@ayushnangia
ayushnangia force-pushed the fix/hud-clarify-vanish branch from 19eedb9 to 9501eb8 Compare August 28, 2026 12:53
@ayushnangia
ayushnangia force-pushed the fix/hud-clarify-vanish branch from a0f5272 to b3e726b Compare September 11, 2026 13:38
ayushnangia added a commit to ayushnangia/hermes-agent that referenced this pull request Sep 11, 2026
session.interrupt's _clear_pending released the pending clarify with an
empty answer but told the client nothing — a wait-forever dialog parked
on the desktop stayed actionable-but-dead forever (keeltrace's lifecycle
review on NousResearch#86036). _clear_pending now emits clarify.cancel for released
clarify prompts (scoped to the interrupted session; sudo/approval/secret
prompts stay silent as before).
ayushnangia added a commit to ayushnangia/hermes-agent that referenced this pull request Sep 11, 2026
The server now tells the client when a parked clarify's wait is gone
(cancel from /stop, expire from the deadline). The desktop clears the
parked request immediately — regardless of any client-side deadline —
so an actionable-but-dead dialog can never persist (NousResearch#86036 follow-up).
ayushnangia added a commit to ayushnangia/hermes-agent that referenced this pull request Sep 13, 2026
session.interrupt's _clear_pending released the pending clarify with an
empty answer but told the client nothing — a wait-forever dialog parked
on the desktop stayed actionable-but-dead forever (keeltrace's lifecycle
review on NousResearch#86036). _clear_pending now emits clarify.cancel for released
clarify prompts (scoped to the interrupted session; sudo/approval/secret
prompts stay silent as before).
ayushnangia added a commit to ayushnangia/hermes-agent that referenced this pull request Sep 13, 2026
The server now tells the client when a parked clarify's wait is gone
(cancel from /stop, expire from the deadline). The desktop clears the
parked request immediately — regardless of any client-side deadline —
so an actionable-but-dead dialog can never persist (NousResearch#86036 follow-up).
@ayushnangia
ayushnangia force-pushed the fix/hud-clarify-vanish branch from b3e726b to ec24a59 Compare September 13, 2026 17:42
@ayushnangia
ayushnangia force-pushed the fix/hud-clarify-vanish branch from ec24a59 to 437583f Compare September 14, 2026 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) 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

Development

Successfully merging this pull request may close these issues.

[Bug] Desktop HUD mode: clarify choice dialog disappears and hangs the session — turn never resumes