Skip to content

fix(gateway): call post-turn goal continuation on streaming path to prevent dead /goal loop (#62202) - #62235

Closed
webtecnica wants to merge 1 commit into
NousResearch:mainfrom
webtecnica:fix/62202-goal-continuation-streaming
Closed

webtecnica wants to merge 1 commit into
NousResearch:mainfrom
webtecnica:fix/62202-goal-continuation-streaming

Conversation

@webtecnica

Copy link
Copy Markdown
Contributor

Summary

The gateway's /goal loop was effectively dead because _post_turn_goal_continuation() was never called after streaming responses. The goal judge never fired, turns_used stayed at 0, and users reported the goal system did nothing despite correct configuration.

Root Cause

_handle_message_with_agent() returns None when streaming already delivered the response (already_sent=True). The caller at _handle_message level only calls _post_turn_goal_continuation() after a non-None response, so the streaming path entirely skipped goal evaluation.

Change

Added a goal continuation call inside _handle_message_with_agent() before the streaming return None path, mirroring the non-streaming path's logic. The streaming path now evaluates the goal judge after every turn.

Verification

9 goal-related tests pass, 1 skipped.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages duplicate This issue or pull request already exists labels Jul 10, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #54222, which fixes the same bug (goal judge never fires on the streaming path in gateway/run.py because _handle_message_with_agent returns None) via the same mechanism. #54222 is the earlier open PR and additionally recovers the final assistant text from the persisted transcript when final_response is empty. Both fix #62202 — consolidating on #54222.

@falkoro

falkoro commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Traced this on current main and the diagnosis is correct: the streaming/already_sent branch returns None from _handle_message_with_agent (gateway/run.py:12122), so the goal hook in _handle_message (10302) computes an empty _final_text and skips the judge — standing goals never continue on streaming platforms.

Three things before this is mergeable, all mirroring the existing hook at 10302:

  1. self.session_store.get_or_create_session(source) is the sync store — it takes threading locks, can wait on another flight owner's Event, and creates SQLite rows, all on the event loop. AsyncSessionStore exists exactly for this boundary; the outer hook does await self.async_session_store.get_or_create_session(source).
  2. The judge call isn't wrapped. The outer hook wraps the whole block in try/except with the comment "a broken judge never breaks normal message handling" — here, a judge exception lands in _handle_message_with_agent's generic error handler after the user already received their streamed reply.
  3. No test — tests/gateway/test_goal_verdict_send.py has the harness pattern for asserting the judge fires on this path.

Happy to re-check once updated; the underlying fix is worth landing.

@webtecnica

Copy link
Copy Markdown
Contributor Author

The duplicate label was applied because another contributor (@Kewe63) submitted PR #62273 fixing the same issue around the same time. That PR has since been closed by its author, leaving this as the sole PR for #62202. Could a maintainer remove the duplicate label? 🙏

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for tracing the streamed-response gap; current main still has it: gateway/run.py:12054-12076 returns None after an already-sent response, while the outer hook only evaluates non-empty returned text at gateway/run.py:10267-10286.

Problems

  • The new await needs the outer hook's best-effort error boundary. _post_turn_goal_continuation() calls GoalManager.evaluate_after_turn() at gateway/run.py:12707 without a local catch; an exception from the added streaming-path call would reach _handle_message_with_agent()'s generic error handler at gateway/run.py:12080 after delivery.
  • No regression test covers this branch. tests/gateway/test_goal_verdict_send.py:111-220 tests the helper directly, not the already_sent=True handler return path.

Suggested changes

  • Catch and debug-log continuation/judge failures around the new streaming-path call, matching gateway/run.py:10267-10288.
  • Add a handler-level streamed-response test that verifies the judge hook is invoked and that a hook failure does not turn into an agent error.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 11, 2026
@alt-glitch alt-glitch added sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users and removed sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 11, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Re: the duplicate label — it isn't stale. It points to #54222, not to the closed #62273. #54222 is the earliest open PR fixing this same bug (#62202) at the same code site (the streaming already_sent return-None branch in gateway/run.py) via the same mechanism, so #62235 is a duplicate of #54222 and the label is correct regardless of #62273's closure.

@teknium1's keep_open salvageability=medium verdict is about merge-worthiness — it flags the missing best-effort error boundary around the added judge call and the absent regression test — which is orthogonal to the dedup relationship. Consolidating the goal-continuation fix on #54222; keeping this open for the salvageable work.

@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Re-anchoring the duplicate link: the canonical fix is #54222 (created 2026-06-28, still open) which patches the same streaming already_sent return-None branch in gateway/run.py so _post_turn_goal_continuation fires on streamed turns. #62273 (the other same-day PR) is now closed by its author, so this stays a duplicate of the earliest-open #54222. Issue: #62202.

@webtecnica

Copy link
Copy Markdown
Contributor Author

Closing as duplicate — another PR already addressed this. Appreciate the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/streaming Streaming responses: gateway delivery, provider wire comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants