Skip to content

fix(tui-gateway): release session leases at idle boundary#2

Open
darshthakkar wants to merge 1 commit into
lEWFkRAD:fix/tui-idle-session-leasesfrom
darshthakkar:fix/tui-idle-lease-boundaries
Open

fix(tui-gateway): release session leases at idle boundary#2
darshthakkar wants to merge 1 commit into
lEWFkRAD:fix/tui-idle-session-leasesfrom
darshthakkar:fix/tui-idle-lease-boundaries

Conversation

@darshthakkar

@darshthakkar darshthakkar commented Jul 20, 2026

Copy link
Copy Markdown

summary

  • schedule cancellable per-session lease timers when the full turn chain becomes idle, so tui_lease_idle_seconds is honored at the configured boundary instead of waiting for the periodic sweep
  • keep runtimes, agents, and conversation history warm while releasing only the cross-process capacity lease
  • protect running turns, queued prompts, pending approval/clarification requests, and agent construction; cancel stale timers on new turns and teardown
  • release registry ownership while holding the session history lock, preventing a new turn from seeing an empty lease slot before its old registry entry is removed
  • retain the periodic idle sweep as a recovery backstop and schedule release when queued-prompt dispatch fails synchronously

context

Stacked onto NousResearch#57059. That PR introduces lazy active-session leases and an idle release window, but release currently depends on the five-minute session reaper cadence. A short configured idle window can therefore remain occupied for almost five extra minutes.

This follow-up makes the idle boundary event-driven without closing the warm session and closes the release/reacquire race in both the timer and recovery-sweep paths.

verification

  • scripts/run_tests.sh tests/test_tui_gateway_server.py tests/tui_gateway/test_protocol.py -q — 421 passed
  • atomic release/reacquire regression reproduces the at-cap race and now passes
  • python -m py_compile tui_gateway/server.py tests/tui_gateway/test_protocol.py
  • python -m ruff check tui_gateway/server.py tests/tui_gateway/test_protocol.py
  • git diff --check

The integrated current-main branch was also verified with 492 gateway/registry tests, Desktop typecheck, 2,169 Desktop tests under supported Node 22, and a production Desktop build. No live deployment or gateway restart was performed.

Schedule precise per-session idle lease timers after turn completion while keeping warm runtimes and history intact. Cancel stale timers on new turns and teardown, preserve active, queued, pending, and building work, and retain the periodic sweep as a recovery backstop.

Co-Authored-By: Claude <noreply@anthropic.com>
@darshthakkar
darshthakkar force-pushed the fix/tui-idle-lease-boundaries branch from 2ab24d6 to eb7ef3b Compare July 20, 2026 21:41
lEWFkRAD pushed a commit that referenced this pull request Jul 25, 2026
…arch#65919)

* fix(desktop): preserve interim assistant text wiped at message.complete

When the agent emits interim text (commentary alongside tool calls, or the
attempted final answer before a verify-on-stop nudge), all UI surfaces
streamed it live but then wiped it at message.complete — keeping only the
final response. The user saw text appear during inference, then disappear.

This is the complete fix across all three layers: agent core, gateway
transport, and all UI surfaces (desktop + Ink TUI).

The verify-on-stop and pre_verify paths flagged the assistant's attempted
final answer as _verification_stop_synthetic, suppressing it from both
state.db and the UI. The user only saw the terse post-verification reply.

Now the assistant response is real content: it's persisted to state.db and
emitted as an interim message via _emit_interim_assistant_message(force_display=True)
before the verification loop runs. Only the synthetic nudge messages keep
the synthetic flags. The turn finalizer drops nudges from live history and
compares content (not just role) to avoid duplicating a published candidate.
Message sequence repair collapses verification candidates in the
consecutive-assistant merge.

Wire agent.interim_assistant_callback both at construction (_agent_cbs())
and per-turn (defense-in-depth), emitting a new message.interim event with
{text, already_streamed}. Gated on display.interim_assistant_messages
(default true). Cleared in the finally block so a stale closure can't
fire on a later turn.

Add message.interim to the GatewayEventName union (apps/shared) and a
typed payload to the TUI's GatewayEvent discriminated union.

The TUI already had the segment-anchoring machinery (flushStreamingSegment +
finalTail) but had no handler for message.interim. Added recordInterimMessage
+ interimBoundaryIndex to seal segments mid-turn, and updated
recordMessageComplete to only dedupe segments after the interim boundary.

Replaced the fragile sealed-set approach with a proper interimBoundaryPending
state flag on ClientSessionState. finalizeInterimAssistantMessage finalizes
the streaming bubble in place (or creates a standalone one), rotates the
stream ID so next deltas create a new bubble, and sets the flag. When the
final text equals an already-sealed interim, they stay as distinct messages.

Extracted mergeFinalAssistantText() as a pure function in chat-messages.ts,
used by both completeAssistantMessage and finalizeInterimAssistantMessage.
Split the bidirectional dedup predicate: reasoning is a restatement only when
the final FULLY covers it. A short final ("Done.") no longer swallows a
longer reasoning block that merely starts with it.

Honor display.interim_assistant_messages (default true) across all layers:
the tui_gateway gates the callback, the desktop wires it to a nanostores
atom via use-hermes-config. Updated hermes_cli/config.py and
cli-config.yaml.example comments to document the Desktop behavior.

_split_segment_tokens now accepts posix=False and _find_ad_hoc_match tries
both posix modes so ad-hoc verification scripts with Windows backslash
paths are matched correctly. (response_previewed forwarding from NousResearch#53553
is not included — our emit-interim + persist approach makes it unnecessary
since the attempted answer is now surfaced before the verification loop.)

- tsc: clean (desktop + TUI + shared)
- vitest desktop: 73/73 pass (7 interim-sealing + 5 mergeFinalAssistantText + 4 config atom)
- vitest TUI: 83/83 pass (4 new message.interim tests)
- python: 390 tests pass (340 tui_gateway + 33 verification/finalizer + 6 config gating + 3 evidence + 8 continuation budget)

Co-authored-by: Liam Zhang <yingliang-zhang@users.noreply.github.com>
Co-authored-by: Lucas D'Alessandro <lucasfdale@users.noreply.github.com>
Co-authored-by: Eric Manganaro <superposition@users.noreply.github.com>
Co-authored-by: sweetcornna <sweetcornna@users.noreply.github.com>
Co-authored-by: DECK6 <DECK6@users.noreply.github.com>
Co-authored-by: matantsevs <matantsevs@users.noreply.github.com>
Co-authored-by: gitcommit90 <gitcommit90@users.noreply.github.com>

* fix: prefix-match interim streamed content to avoid benign duplicate bubbles

_interim_content_was_streamed used exact equality (streamed == visible_content),
so a final response that was the streamed text plus a trailing delta — or a
partial stream before the verify nudge fired — failed the match and left
_response_was_previewed false. The turn then showed two bubbles (interim +
identical final) instead of settling the interim in place.

Relax to a prefix check (visible_content.startswith(streamed)) in both the
core match and the desktop's settle-in-place gate. The TUI already used
prefix matching via finalTail. The reverse direction (streamed longer than
final) is intentionally not matched — that could suppress a needed resend
in the gateway path where already_streamed=True calls on_segment_break().

* test(desktop): add partial-stream-then-nudge dedup edge case

Third edge case for the interim-sealing dedup: model streams part of its
answer via message.delta, verify nudge fires, interim seals the streamed
prefix, then the final response is the same text plus a trailing delta.
Asserts one bubble (not two) containing the full final text.

Acceptance protocol #2 — covers all three dedup edges:
  1. interim == final (existing)
  2. interim = strict prefix of final (existing)
  3. partial-stream-then-nudge (this commit)

---------

Co-authored-by: Liam Zhang <yingliang-zhang@users.noreply.github.com>
Co-authored-by: Lucas D'Alessandro <lucasfdale@users.noreply.github.com>
Co-authored-by: Eric Manganaro <superposition@users.noreply.github.com>
Co-authored-by: sweetcornna <sweetcornna@users.noreply.github.com>
Co-authored-by: DECK6 <DECK6@users.noreply.github.com>
Co-authored-by: matantsevs <matantsevs@users.noreply.github.com>
Co-authored-by: gitcommit90 <gitcommit90@users.noreply.github.com>
lEWFkRAD pushed a commit that referenced this pull request Jul 25, 2026
Blocking #1 — gateway-connecting-overlay.tsx reduced-motion regression:
the top `if (reduce) setPhase('gone')` fired unconditionally on mount
whenever reduce-motion was on, so every OS reduced-motion user lost the
CONNECTING overlay during cold boot entirely (jumped to 'gone' before the
gateway was even open). The intent was to skip the exit *choreography*,
not to skip showing the overlay. Removed the unconditional top block and
the redundant nested preview block; kept only the third branch
(`gatewayState === 'open' && shownRef.current` → `reduce ? 'gone' :
'text-out'`) which correctly gates the short-circuit on connect. Also
fixed `if(reduce)` missing-space, 6-space misindent, and the same 3-line
comment pasted three times.

Nit #1 — tsconfig excludes e2e, so specs were never typechecked in CI.
Added tsconfig.e2e.json (extends base, includes e2e/ + playwright.config.ts,
adds @playwright/test types) and wired it into the typecheck script. This
surfaced three latent type errors that are fixed in the same commit:
  - fix-electron-tracing.ts: `app._context` and `electron._playwright` are
    private APIs — added `as any` on the access before the existing cast.
  - playwright.config.ts: `reducedMotion: 'reduce'` directly under `use:`
    is not a valid UseOptions property in playwright 1.58; it's a
    BrowserContextOption accessed via `contextOptions: { reducedMotion:
    'reduce' }`. The old form was silently ignored at runtime, so
    reduced-motion emulation wasn't actually active — screenshots could
    catch overlays mid-fade (exactly what the comment warned about).

Nit #2 — fix-electron-tracing.ts reaches into Playwright internals
(_playwright, _allContexts, _context) with no public contract. Added a
header comment calling out the `@playwright/test` exact pin (=1.58.2) so a
future bump knows to re-verify the private symbols still exist.

Nit NousResearch#3 — main.ts TEST_WORKER_INDEX block had stray 6-space indentation.

Verified: tsc -p . && tsconfig.electron && tsconfig.e2e → 0 errors;
vitest boot-failure-overlay (3/3) + boot-failure-reauth (21/21) pass;
npm run build clean; playwright e2e/boot-failure.spec.ts 2/2 pass.
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