Skip to content

fix(gateway): restore MoA one-shot model override on failed turns - #53523

Closed
srojk34 wants to merge 1 commit into
NousResearch:mainfrom
srojk34:fix/moa-one-shot-restore-on-failure
Closed

fix(gateway): restore MoA one-shot model override on failed turns#53523
srojk34 wants to merge 1 commit into
NousResearch:mainfrom
srojk34:fix/moa-one-shot-restore-on-failure

Conversation

@srojk34

@srojk34 srojk34 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • When a MoA one-shot turn (`/moa `) fails with an exception in `_handle_message_with_agent`, the MoA model override is never restored — all subsequent messages silently route through MoA reference fan-out permanently
  • The restore data is stored on the per-turn event object (`event._moa_restore_override`), which is discarded after the turn — unlike CLI/TUI where the restore flag persists on `self`/`session` and fires on the next successful turn

Root cause

The MoA one-shot restore ran inside the `try` block (line ~9032), after `_handle_message_with_agent` returns. When that call raises an exception (agent init failure, interpreter shutdown, OOM), execution jumps to the `finally` block which only calls `_release_running_agent_state` — the restore is skipped.

Since `_moa_restore_override` lives on the event object (not on the runner or session), it is permanently lost when the event goes out of scope. The next message creates a new event with no restore data, so the MoA override persists silently.

Impact

  • Every subsequent gateway message fans out to reference models + aggregator (higher cost, higher latency)
  • No user-visible indication that the model switched permanently
  • Affects all gateway platforms (Telegram, Discord, Slack, etc.)
  • Recovery: manual `/model ` switch

Changes

  • `gateway/run.py`: move the MoA one-shot restore from the `try` block to the `finally` block so it fires on every exit path (success, exception, interrupt)
  • `tests/gateway/test_moa_one_shot_restore.py`: 4 tests covering restore-on-success, restore-on-exception, restore-none-clears-override, and no-restore-for-non-moa-turns

Test plan

  • `test_restore_fires_on_success` — successful turn restores previous model
  • `test_restore_fires_on_exception` — failed turn (RuntimeError) still restores
  • `test_restore_none_clears_override` — user with no prior override gets override removed
  • `test_no_restore_when_not_one_shot` — normal turns don't touch model overrides

The MoA one-shot restore ran inside the try block after
_handle_message_with_agent returned. When that call raised an
exception (agent init failure, interpreter shutdown, OOM), the
restore was skipped and the MoA model override stayed permanently
on _session_model_overrides — silently routing all subsequent
messages through the MoA reference fan-out with no user-visible
indication.

Move the restore to the finally block so it fires on every exit
path (success, exception, interrupt). The restore data lives on
the per-turn event object and would be lost if not consumed here.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Moves the MoA one-shot model restore logic from the success path into the finally block so a failed turn (exception) still reverts the model override. Complements PR #53548 (which makes /moa one-shot only).

Looks Good

  • The restore logic in finally is idempotent (pop-on-absent is harmless)
  • Placed before _release_running_agent_state to ensure cleanup ordering
  • Comprehensive test suite: tests for success, exception, None restore, and non-MoA turns
  • The 4 test cases in test_moa_one_shot_restore.py cover the key scenarios clearly

Reviewed by Hermes Agent

@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-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jun 27, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #53559 — your commit was cherry-picked onto current main with your authorship preserved (2f29e3cfc). We added a small follow-up on top (4e0788783): extracted the restore into a _restore_moa_one_shot() helper so it's unit-testable without re-implementing it, restored the load-bearing #28686 zombie-eviction comment your diff had dropped, and pointed the tests at the real helper. Your root-cause analysis (restore in try instead of finally, lost because it lives on the per-turn event) was exactly right. Thanks!

@teknium1 teknium1 closed this Jun 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery 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.

4 participants