Skip to content

fix(runtime): guarantee terminal turn outcomes - #270

Merged
OmarB97 merged 3 commits into
mainfrom
work/hermes-terminal-turn-outcome-guarantee-20260712
Jul 13, 2026
Merged

fix(runtime): guarantee terminal turn outcomes#270
OmarB97 merged 3 commits into
mainfrom
work/hermes-terminal-turn-outcome-guarantee-20260712

Conversation

@OmarB97

@OmarB97 OmarB97 commented Jul 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

What changed

  • adds a prompt-safe turn_outcomes SQLite record that stays separate from model conversation history
  • gives each live turn a stable ID and funnels success, fallback, timeout, cancellation, initialization failure, context refusal, and unexpected exceptions through one idempotent terminal outcome
  • emits and persists exactly one redacted turn.outcome event while allowing failed persistence to retry without duplicate live delivery
  • hydrates terminal outcomes and the active in-flight turn ID across REST, desktop, and Ink resume/activate paths
  • seeds persisted outcome IDs during Ink hydration so replayed live events do not duplicate transcript rows
  • preserves late accepted steering as the next turn ahead of later queued input
  • composes with the explicit fallback policy from fix: make provider fallback policy explicit and visible #269

Why

A provider timeout, HTTP 502, initialization error, or exception could terminate a turn without a durable explanation. Different clients also synthesized their own interruption markers, which made cancellation behavior inconsistent and could duplicate terminal state.

The root cause was that terminal state existed only in transient control flow and client-local rendering. It had no prompt-safe persistence model and no single idempotent delivery seam shared by normal, exceptional, interrupt, and reconnect paths.

Exact-head review fixes

  • centralizes ownership for queued, goal, poller, and post-turn automatic dispatches; a synchronous start failure emits and persists one failed outcome, releases running/inflight state, and restores the claimed work
  • preserves the full unconsumed notification tail both when the first dispatch fails synchronously and when the first dispatch starts successfully
  • freezes Stop-versus-completion classification under the session history lock before message.complete
  • binds the completion payload and every downstream goal/title/TTS side effect to that frozen classification
  • carries turn_id through in-flight snapshots and hydrates desktop/Ink active-turn trackers
  • seeds desktop/Ink replay dedupe from hydrated persisted outcomes

Impact

Every started gateway turn now has a durable, redacted terminal record without adding synthetic messages to the model prompt or invalidating prompt caching. A delayed outcome cannot settle a newer resumed turn, a Stop-first turn cannot speak/title/continue after cancellation, and automatic work is recoverable after a synchronous dispatch failure.

Validation

  • scripts/run_tests.sh tests/tui_gateway/test_turn_outcomes.py -q — 23 passed
  • scripts/run_tests.sh tests/test_hermes_state.py -q -k 'TestTurnOutcomes' — 3 passed
  • scripts/run_tests.sh tests/hermes_cli/test_web_server.py -q -k 'get_session_messages_follows_compression_tip' — 1 passed
  • desktop focused Vitest — 51 passed across 3 files
  • Ink focused Vitest — 86 passed across 2 files
  • desktop and Ink TypeScript typechecks passed
  • targeted desktop and Ink ESLint — 0 errors; 7 pre-existing padding warnings on untouched lines
  • Python compile and git diff --check passed

Safety boundary

tests/test_tui_gateway_server.py was deliberately excluded and never run. No Hermes, Chrome, TTS, or user app was restarted, installed, or launched. The branch is based on fork commits 28f371dc8b and e1754474b6, whose stable patch IDs match upstream Chrome/TTS containment fixes 398ed14ef and 8f58ce2b55 respectively.

@OmarB97
OmarB97 marked this pull request as ready for review July 12, 2026 22:37
@OmarB97
OmarB97 marked this pull request as draft July 12, 2026 23:14
@OmarB97
OmarB97 marked this pull request as ready for review July 13, 2026 00:05
@OmarB97
OmarB97 merged commit 4b68972 into main Jul 13, 2026
26 of 36 checks passed
OmarB97 added a commit that referenced this pull request Jul 20, 2026
@OmarB97 Omar Baradei
fix(runtime): guarantee terminal turn outcomes (#270)
91a2e20
@OmarB97 OmarB97 mentioned this pull request Aug 2, 2026
Merged
13 tasks
OmarB97 added a commit that referenced this pull request Aug 2, 2026
@OmarB97 Omar Baradei @claude
test(gateway): empty turns are classified failed since #270 (#294)
d752353 
#270 made the turn-outcome classifier the owner of message.complete's
status. _derive_turn_outcome() treats a turn with no visible response as
terminal status "failed" ("turn ended without a visible response"), and
_freeze_turn_outcome() overrides the locally computed status with it, so
the payload ships status="error".

That is the deliberate contract, not a regression: #270's own
tests/tui_gateway/test_turn_outcomes.py parametrizes
{"completed": True, "final_response": ""} -> "failed" /
"without a visible response". #270 added that file but never updated this
one, so the pre-#270 expectation stayed red.

The half of this test that still guards real behavior is the payload text:
classifying the turn must not fabricate an "Error:" string. That assertion
is unchanged and still passes.

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 2, 2026
Merged
Merged
OmarB97 added a commit that referenced this pull request Aug 2, 2026
@OmarB97 Omar Baradei @claude
test(gateway): update turn-isolation parity and activate doubles for #…
fbbf44c 
…270 (#296)

Two test doubles in this file drifted behind the code they stand in for.

_session_info doubles (3 sites)
  The real signature is _session_info(agent, session=None), and the gateway
  turn path calls it with both. Three stubs still took a single argument, so
  the worker thread died with "lambda takes 1 positional argument but 2 were
  given" before the fake model ever streamed — surfacing as the misleading
  "fake model did not stream before activation". Most stubs in this file were
  already updated; these three were not. The two that were not yet failing
  reach the same call path on any future change.

session.activate in-flight payload
  #270 stamps the in-flight turn id so a client can correlate a partial with
  the turn.outcome that closes it. Pin its shape and compare the stable
  fields exactly, since it is a fresh uuid per turn.

golden transcript parity
  #270 added a leading session.info, a turn_id on message.start /
  message.complete, and a terminal turn.outcome to the in-process path. The
  _FakeSupervisor standing in for the compute host still emitted the old
  four-event sequence. The compute-host child proxies prompt.submit back into
  server.handle_request(), so it emits exactly what the in-process path
  emits; the double is what was stale, not the product.

  The transcript now also carries a fresh uuid and wall-clock timestamps, so
  the two runs can never be byte-equal again. Normalize those four volatile
  fields before comparing and keep asserting the sequence and every stable
  field.

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@OmarB97 OmarB97 mentioned this pull request Aug 2, 2026
Merged
13 tasks
OmarB97 added a commit that referenced this pull request Aug 2, 2026
@OmarB97 Omar Baradei @claude
test(gateway): pin who owns the terminal outcome under turn isolation (…
6da9e88 
…#309)

`tests/tui_gateway/test_turn_outcomes.py` covers #270's "every started
gateway turn has a durable terminal record" guarantee in 23 cases, all of
them on the in-process path. None of them touch turn isolation, so nothing
records which process owns the outcome when the turn body runs in the
`python -m tui_gateway.compute_host` child.

That gap invites a specific wrong fix. The parent's completion handler,
`_on_compute_host_turn_done`, looks like the natural place to finalize: it
runs on turn end, it clears the in-flight turn, and the parent did mint a
turn id in `_begin_prompt_dispatch_locked` before routing to the child. It
is the wrong place. `ComputeHost._run_real_turn` re-enters this same module
-- `server._start_inflight_turn()` then `server._run_prompt_submit()` -- so
the child already emits and persists the outcome on the ordinary in-process
path, and reports `session_info_emitted: True` so the parent skips its
`session.info` too.

Finalizing in the parent therefore publishes a *second* terminal record
under a different turn id, and the dedupe in `_finalize_turn_outcome()`
cannot suppress it: that dedupe is per-session-dict in-process state, and
the child holds its own session dict in another process.

Four cases, no production change:

  * the child emits exactly one `turn.outcome` and persists exactly one row
  * its `turn.end` frame sets `session_info_emitted`
  * a raising agent still yields exactly one outcome, status `failed`
  * `_on_compute_host_turn_done` emits and persists nothing, while still
    releasing `inflight_turn`/`running`

The tests drive the real `_run_real_turn` rather than asserting against a
hand-written double, and reuse the file's existing `turn_harness`, `_Agent`
and `_session` helpers.

The fourth case is the regression guard, so it was mutation-tested rather
than trusted: reintroducing a `_finalize_turn_outcome()` call into
`_on_compute_host_turn_done` makes it fail, and removing it makes it pass
again.

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@OmarB97