fix(moa): restore virtual runtime after fallback - #53802
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying the restore path; the current-main premise is valid: restore_primary_runtime() restores MoA's empty client_kwargs and then calls _create_openai_client() at agent/agent_runtime_helpers.py:1193-1198.
Problems
- The proposed
MoAClient(...)reconstruction drops thereference_callbackinstalled during MoA initialization (agent/agent_init.py:846-889). That callback is the only route formoa.reference/moa.aggregatingdisplay events (agent/moa_loop.py:791-798,937-958), so a restored MoA session would lose progress output. - The same virtual-runtime reconstruction remains in
try_recover_primary_transport()atagent/agent_runtime_helpers.py:1001-1026; the conversation loop calls it before fallback (agent/conversation_loop.py:3925-3946). A MoA primary hitting a transient transport recovery would still attempt the empty-kwargs OpenAI rebuild.
Suggested changes
- Reuse a shared MoA-facade factory or preserve the initialized callback when recreating the facade, and add an event-delivery regression test after restore.
- Cover the primary transport-recovery sibling path with the same no-OpenAI-client invariant.
Automated hermes-sweeper review.
| # OpenAI client kwargs; restoring it after a fallback must recreate | ||
| # the facade, not call OpenAI() with an empty api_key. | ||
| from agent.moa_loop import MoAClient | ||
|
|
There was a problem hiding this comment.
MoA initialization passes reference_callback=_moa_reference_relay (agent/agent_init.py:846-889), but this reconstruction omits it. Please preserve that callback or use a shared facade factory; otherwise moa.reference and moa.aggregating progress events stop after a fallback restore.
| @@ -1018,7 +1018,15 @@ def restore_primary_runtime(agent) -> bool: | |||
| ) | |||
There was a problem hiding this comment.
Please apply the same virtual-MoA reconstruction to try_recover_primary_transport (agent/agent_runtime_helpers.py:1001-1026). It restores the same empty MoA snapshot and still calls _create_openai_client; the conversation loop invokes it before fallback for transient failures.
#53802) Follow-up to the salvaged core of #53802: a naive MoAClient(preset) rebuild restores a working facade but silently drops the reference_callback relay wired in agent_init, so moa.reference / moa.aggregating display events stop reaching every frontend for the rest of the session. Introduce agent.moa_loop.build_moa_facade(agent, preset) as the single construction point for the MoA facade and use it at: - initial client construction (agent_init.py) - turn-start fallback restore (restore_primary_runtime) - transient transport recovery (try_recover_primary_transport — previously fell through to _create_openai_client with MoA's empty client_kwargs and died with 'api_key client option must be set') - mid-session model switches (switch_model) The relay reads agent.tool_progress_callback at emit time, so callbacks attached after construction are picked up automatically. Adds test_moa_restored_facade_still_emits_reference_events covering event delivery through a restored facade.
#53802) Follow-up to the salvaged core of #53802: a naive MoAClient(preset) rebuild restores a working facade but silently drops the reference_callback relay wired in agent_init, so moa.reference / moa.aggregating display events stop reaching every frontend for the rest of the session. Introduce agent.moa_loop.build_moa_facade(agent, preset) as the single construction point for the MoA facade and use it at: - initial client construction (agent_init.py) - turn-start fallback restore (restore_primary_runtime) - transient transport recovery (try_recover_primary_transport — previously fell through to _create_openai_client with MoA's empty client_kwargs and died with 'api_key client option must be set') - mid-session model switches (switch_model) The relay reads agent.tool_progress_callback at emit time, so callbacks attached after construction are picked up automatically. Adds test_moa_restored_facade_still_emits_reference_events covering event delivery through a restored facade.
|
Merged via cluster PR #70280 (commit 74a56b7) — your core commit cherry-picked with authorship preserved, plus a follow-up that turns the fix into a shared build_moa_facade() factory used at init/restore/recover and preserves the reference_callback (a bare MoAClient() reconstruction would have silenced advisor display events after restore). Thanks for the restore-path diagnosis and regression test! |
- model_setup_flows: _model_flow_moa (always show presets), _model_flow_bedrock_api_key (mantle region endpoint) - codex migration: _query_codex_plugins (app-server RPC) - slack parser: build_slack_parser (manifest + write) - delegate_tool: _resolve_child_credential_pool (custom endpoint identity NousResearch#7833) - relay: _platform_is_fronted (back-compat alias) - secrets_cli: cmd_token (verify-then-persist, 0. prefix warn) - skills_hub: _github_publish (fork → branch → tree → PR) - main: _tui_need_npm_install (content compare, prebuilt bundle), note_turn_start (interleave tripwire), _build_gateway_vbs_script (wscript no-console NousResearch#45599), _write_full_zip_backup (sqlite safe-copy), _login_openai_codex (device code + reuse), _frame_renderable (rich Group), systemd_install (legacy removal + --force pre-sync), build_moa_facade (reference relay NousResearch#53802), build_plugins_parser, _run_anthropic_oauth_flow (claude-code link), _render_distribution_plan (non-distribution warning), _detect_venv_python_processes (refuse-don't-kill)
…_recover_primary_transport) Whole-function owner tests for transient transport recovery: skip guards (fallback-active / non-transient / OpenRouter / Nous-Portal-OpenAI-wire) + the Portal-native-anthropic-wire exception, wire-aware rebuild from _primary_runtime (openai / anthropic-client-nulled / moa-facade NousResearch#53802), FD-safe retire (NousResearch#70773, skipped when no client, exception swallowed), transport-cache clear, and rebuild-exception -> False. Function (1248-1349) 100%.
…ry_runtime) Whole-function owner tests for turn-scoped primary restore: NousResearch#20465 index reset, rate-limit + reset-aware skip gates (future/already-logged/exception-fallthrough), wire-aware rebuild (moa-facade NousResearch#53802 / anthropic-nulled / openai), _cache_disabled survival (NousResearch#33555), transport-cache clear, pool rebind (mismatch-reload / prefetched-reuse / reload-exception-cleared), credential re-select NousResearch#25205 (match / mismatch / no-key / select-None) incl. custom:<name> disambiguation NousResearch#56885 (match/exception both blocks), reasoning restore, fallback reset, and rebuild fail-safe. Function (1449-1732) 100%. NOTE: an earlier grep-artifact hid the custom-provider blocks as false-covered; caught via re-roast, added the missing cases, switched to a python range filter.
NousResearch#53802) Follow-up to the salvaged core of NousResearch#53802: a naive MoAClient(preset) rebuild restores a working facade but silently drops the reference_callback relay wired in agent_init, so moa.reference / moa.aggregating display events stop reaching every frontend for the rest of the session. Introduce agent.moa_loop.build_moa_facade(agent, preset) as the single construction point for the MoA facade and use it at: - initial client construction (agent_init.py) - turn-start fallback restore (restore_primary_runtime) - transient transport recovery (try_recover_primary_transport — previously fell through to _create_openai_client with MoA's empty client_kwargs and died with 'api_key client option must be set') - mid-session model switches (switch_model) The relay reads agent.tool_progress_callback at emit time, so callbacks attached after construction are picked up automatically. Adds test_moa_restored_facade_still_emits_reference_events covering event delivery through a restored facade.
What does this PR do?
Fixes MoA primary-runtime restoration after provider fallback.
MoA is a virtual provider: it uses a
MoAClientfacade and intentionally does not have real OpenAI client kwargs. If a long-lived MoA session falls back to a concrete provider, then the next turn restoresprovider=modelback to MoA. Before this fix, restoration could try to rebuild a real OpenAI client from MoA's empty/virtual client kwargs, causing restore failure instead of recreating the MoA facade.The restore path now special-cases
provider == "moa"and recreatesMoAClient(agent.model or "default")instead of calling the OpenAI client builder.Related Issue
No linked issue. Found while testing MoA sessions with fallback/restore behavior.
Type of Change
Changes Made
agent/agent_runtime_helpers.pyprovider="moa".tests/run_agent/test_moa_loop_mode.py_create_openai_client.How to Test
python -m py_compile agent/agent_runtime_helpers.pypytest -q tests/run_agent/test_moa_loop_mode.py tests/run_agent/test_primary_runtime_restore.pyResults locally:
py_compile: passed40 passed32 passedChecklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
The regression test guards the original failure mode by making
_create_openai_clientraise if the MoA restore path tries to build a real OpenAI client.