fix(tui_gateway): let completed Desktop sessions run refine

[fangliquanflq]

What does this PR do?

Desktop and TUI sessions can now run /refine against a completed conversation even when the isolated slash-command worker has no cached agent. The command stays with the session owner, reads the canonical persisted transcript, and preserves the review focus instead of incorrectly reporting that there is nothing to refine.

Symptom

A Desktop session with completed, persisted turns can return Nothing to refine yet - send a message first. when the user invokes /refine after the agent is idle.

Impact

Affected Desktop and TUI users cannot refine an existing conversation even though its transcript remains visible and durable. The command ignores usable session history and the requested background review never starts.

Bug Cause

Trigger: tui_gateway/methods_tools.py / slash.exec dispatch for /refine

Causal chain:

1. Desktop sends /refine through the TUI gateway's slash.exec path.
2. Because refine was not a live-session command, dispatch selected an isolated HermesCLI slash worker that did not own the active session agent or its persisted transcript.
3. The isolated handler saw an empty cache and returned the same response used for a genuinely empty conversation.

Why it is wrong: Session ownership and durable conversation state live in the gateway process or its compute host, not in the isolated slash worker. Cache absence in that worker does not mean the conversation is empty.

Working sibling / contrast: Other live session commands already execute against the gateway-owned session. The messaging gateway's /refine path has its own valid agent cache and is not routed through this isolated Desktop/TUI seam.

Ruled out: An actually empty conversation was ruled out because the regression test supplies persisted user and assistant turns and verifies that they are selected over stale in-memory history.

Fix

Route /refine as an idle-gated live-session command. The gateway now snapshots the persisted transcript including ancestor messages, falls back to locked live history when needed, and starts the review on the owning agent. Turn-isolated sessions forward the command to the compute host so the process that owns the live agent performs the same operation.

Related Issue

Closes #83455

Type of Change

• Bug fix

Changes Made

• tui_gateway/server.py - handle /refine on the live session, load durable history, and forward compute-host-owned sessions.
• tui_gateway/compute_host.py - execute forwarded refine requests on the host-owned agent.
• tui_gateway/host_supervisor.py - register refine as an idle-gated mutator route.
• tests/test_tui_gateway_server.py - cover persisted-history recovery and compute-host forwarding without isolated worker creation.
• tests/tui_gateway/test_compute_host_phase1.py - cover host-owned review startup and route classification.

How to Test

Automated tests already run locally on Windows:

scripts/run_tests.sh tests/test_tui_gateway_server.py
scripts/run_tests.sh tests/agent/test_refine_focus.py
scripts/run_tests.sh tests/tui_gateway/test_compute_host_phase1.py

Results: 534 gateway server tests passed, 3 refine focus tests passed, and 8 compute-host tests passed. One unrelated pre-existing Windows O_APPEND test in the compute-host file still fails; the focused refine and route tests passed.

Checklist

• 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 repo's test entry on relevant tests
• I've added regression tests for this bug fix
• I've tested on Windows

[fangliquanflq]

Note for reviewers / @thatssoheil: the complementary messaging-gateway cold-cache case you called out is tracked separately in #83871 and fixed in #83872, so this PR stays focused on the Desktop/TUI tui_gateway path.

[Enough1122]

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

fix(tui_gateway): let completed Desktop sessions run refine

1. The recursion boundary between the two hops is implicit. server._live_slash_command_output sends a slash.refine control to the compute host, and compute_host._handle_control (tui_gateway/compute_host.py:738-744) then calls server._live_slash_command_output again for the same session. This terminates only because _session_uses_compute_host() evaluates to False on the host side; if a host-side session ever reports compute-host active, the two sides ping-pong forever. Consider an explicit host/owner flag on the session instead of relying on asymmetric _session_uses_compute_host() results.

2. The non-host path calls the private agent._spawn_background_review(...) directly (tui_gateway/server.py refine branch). Any drift in that private method's signature fails at runtime into a generic "/refine failed to start" message. A thin public wrapper (or a more specific except path) would make failures diagnosable.

3. The session.get("running") busy-check runs before the compute-host branch, but for a compute-host session the authoritative running state lives on the host. After a host-side restart the server's in-memory running flag can be stale, producing a spurious "Agent is running — wait" for an actually-idle session. Consider letting the host answer the busy check for slash.refine.

[fangliquanflq]

Thanks for the review. I checked each point against the current PR head and the compute-host lifecycle:

1. No change needed (invalid): the second call cannot forward back to the parent. Every compute-host child sets HERMES_COMPUTE_HOST_CHILD=1, and _turn_isolation_enabled() unconditionally returns False in that process before _session_uses_compute_host() inspects session fields. The host-side call therefore always takes the local-agent branch, even if _compute_host_active were present in the session dict.
2. No change needed (informational): _spawn_background_review(...) is the established review-start boundary used by the CLI, messaging gateway, turn finalizer, and Codex runtime with the same keyword contract. This path also preserves the underlying exception text in /refine failed to start: <exception>; adding a new public wrapper only for this call would be a speculative API/refactor outside this focused routing fix.
3. No change needed (invalid): the parent running flag is the turn admission state and is cleared on both normal host completion and host failure. On a host crash/restart, HostSupervisor._wait_for_exit() fails every pending turn, invokes its completion callback, and _on_compute_host_turn_done() clears session["running"] before respawn. An idle host restart has no in-flight turn and therefore no legitimate True state to clear.

Focused verification on 08a3a5352ceccb072761ca8f88fc1e7ebbb20572:

• scripts/run_tests.sh tests/test_tui_gateway_server.py -k "refine_uses_live_agent_and_persisted_history_without_spawning_worker or refine_forwards_to_compute_host_owner_without_spawning_worker" — 2 passed
• scripts/run_tests.sh tests/tui_gateway/test_compute_host_phase1.py -k "compute_host_refine_control_uses_host_owned_agent or mutator_route_table_matches_prd_inventory" — 2 passed

No code changes were made.

[thatssoheil]

Review: PR #83520

Summary: Fixes #83455 correctly on the reported surface (Desktop/TUI via tui_gateway). /refine now loads the persisted transcript first, falls back to locked history, and spawns the review fork — no longer returns the false "Nothing to refine yet" when the in-memory agent cache is cold.

Tests: 3 new regression tests pass locally (scripts/run_tests.sh); full CI 33/33 green.

Standards axis: No hard violations. One undocumented but real Duplicated Code smell — the refine-start chain is now the third near-verbatim copy (cli_commands_mixin.py:2556, gateway/slash_commands.py:2859, tui_gateway/server.py:12879). Per AGENTS.md "Extend, don't duplicate / fix the bug class" — a shared helper would serve all three hosts and prevent a fourth copy. Not a blocker for this fix; worth a follow-up.

Spec axis: Implements the issue's preferred option 1 (persisted transcript fallback). Two minor corner-case divergences from sibling paths:

• In-memory history fallback feeds raw display_kind="hidden" scaffolding into the review prompt (CLI/gateway normalize via _history_to_messages).
• Host-owned refine reads the session dict's history only; after a host crash the parent's session dict survives, so it works via the parent-side snapshot — but the host path never independently consults SQLite. Not the issue's repro.

All three AI-review points rebutted correctly — especially the ping-pong recursion concern (#1), which cannot happen because HERMES_COMPUTE_HOST_CHILD=1 short-circuits turn-isolation in the child process before the forward check.

Verdict: Merge-worthy. Suggested follow-up: extract the review-start logic into a shared utility used by CLI, messaging gateway, and TUI gateway.

[fangliquanflq]

Thanks for the detailed review. I rechecked the current head (08a3a5352c) against the PR diff, the three /refine entry points, the compute-host lifecycle, and current main:

• Shared helper — informational / follow-up: the repeated admission/snapshot/output orchestration is real, but each surface owns different session lookup, locking, persistence, and rendering behavior. They already converge on AIAgent._spawn_background_review(...). Extracting a cross-surface coordinator would be a broader refactor rather than a correction required by [Bug]: /refine rejects completed Desktop sessions when their in-memory agent cache is absent #83455, so I am keeping it out of this focused fix.
• Hidden-history fallback — invalid as a TUI divergence: CLI passes raw conversation_history and the messaging gateway passes raw agent._session_messages; neither normalizes through tui_gateway._history_to_messages. That helper is explicitly a display projection and intentionally removes display_kind="hidden" rows. The new TUI path likewise passes the canonical model-history snapshot to the review fork, so it does not uniquely introduce hidden scaffolding relative to the sibling /refine paths.
• Host-side SQLite lookup — informational / out of scope: turn.start seeds a newly created compute-host session with the parent history, and subsequent controls operate on the host-owned live session. Independent rehydration of an idle control after a compute-host crash is a broader control-plane recovery concern; there is no evidence that it is needed for the reported cold-cache Desktop repro, and the parent-side persisted-history path remains the fix for that repro.

No code changes were made. The focused persisted-history and compute-host routing tests are present on this head, and GitHub currently reports the PR mergeable with all required checks passing.

[gokhanyildirimlar]

Independent confirmation on Desktop / Linux (v0.20.5, upstream 42e39d06, nous provider - so this reproduces across providers, not just the OpenAI Codex setup in the linked issue).

Reproduced today with a session holding 169 persisted messages (sessions.message_count=169 in state.db): /refine returned Nothing to refine yet — send a message first. while the conversation was fully alive - my next regular message a minute later went through normally with history=127+ attached.

One addition to the RCA that may help review: on the Desktop surface the failing command demonstrably runs in a separate slash-worker subprocess, not the backend process that owns the warm agents. From :~/.hermes/logs/agent.log:

15:30:59 INFO hermes_cli.mem_trim: memory trim: reason=tui turn completion ... rss_kib=366328->352172        # regular turn, main backend process
15:36:21 INFO hermes_cli.mem_trim: memory trim: reason=slash worker command completion ... rss_kib=58132->57860   # /refine served here -> "Nothing to refine yet"
15:37:04 INFO tui_gateway.server: tui prompt accepted: ui_session=d21c4eea session_key=202608...aaa2 ...            # next normal message, same session, works fine

The process that served /refine sits at ~58 MB RSS (a bare HermesCLI whose .agent is still None) versus ~350 MB for the backend holding the cached agent + transcript - consistent with _handle_refine_command bailing out at self.agent is None. Worth noting the error text ("send a message first") actively misleads in this state: dozens of messages had been sent all day; what is missing is a resident AIAgent in the worker's process, not conversation history.

Impact scoping: automatic post-turn background reviews keep working (they fire inside the process that owns the agent), so this breaks only the manual /refine path on Desktop. Falling back to the persisted transcript as this PR does covers the gap; happy to test the branch if useful.

[fangliquanflq]

Thank you for the independent Desktop/Linux confirmation. This is informational and requires no additional code change: the separate slash-worker process with no resident AIAgent matches the PR RCA, while the persisted session remains valid and available to the owning TUI gateway process. The current head routes /refine through that live-session path, loads the persisted transcript (with the in-memory history as fallback), and routes compute-host sessions to the host-owned agent.

Revalidated on 08a3a5352ceccb072761ca8f88fc1e7ebbb20572:

• scripts/run_tests.sh tests/test_tui_gateway_server.py -k "refine_uses_live_agent_and_persisted_history_without_spawning_worker or refine_forwards_to_compute_host_owner_without_spawning_worker" — 2 passed
• scripts/run_tests.sh tests/tui_gateway/test_compute_host_phase1.py -k "compute_host_refine_control_uses_host_owned_agent or mutator_route_table_matches_prd_inventory" — 2 passed

No code changes were needed.

[zaze-oO]

Cross-link from another install: we hit the same bug on Linux (Desktop app + TUI gateway, session reattached from disk after a restart) and opened #109934 — a fresh implementation of this same fix on top of current main (476dbfed).

Two reasons for a separate branch rather than a rebase of this one:

• This branch is ~12,600 commits behind origin/main and no longer rebases: git rebase origin/main conflicts in tui_gateway/server.py (the live-session plumbing around it was rewritten upstream).
• GitHub reports it as not mergeable, so the fix is effectively stalled while users keep hitting Desktop/TUI: /refine always says "Nothing to refine yet — send a message first." (routed to slash worker, live agent never consulted) #93918 / [Bug]: /refine rejects completed Desktop sessions when their in-memory agent cache is absent #83455.

Credit for the approach is yours in the #109934 description. Two details we found we needed on top of it, in case they're useful here too:

• slash.refine must be registered in MUTATOR_ROUTE_TABLE as idle-gated — otherwise the compute-host route admits the request while the agent is mid-turn.
• the background review has to be spawned with explicit=True, so an explicit /refine is not swallowed by the unattended / background-review gates.

If you'd rather land it here, take anything you need — no hard feelings about closing #109934 in favour of a rebase of this PR.