Skip to content

fix(gateway): guard against None request_overrides in _build_api_kwargs - #7305

Closed
konsisumer wants to merge 1 commit into
NousResearch:mainfrom
konsisumer:fix/request-overrides-none-guard
Closed

fix(gateway): guard against None request_overrides in _build_api_kwargs#7305
konsisumer wants to merge 1 commit into
NousResearch:mainfrom
konsisumer:fix/request-overrides-none-guard

Conversation

@konsisumer

Copy link
Copy Markdown
Contributor

Summary

  • request_overrides can be None in the gateway code path, causing AttributeError: 'NoneType' object has no attribute 'get' on every gateway message after commit 970192f (fast mode support)
  • Three assignment sites in gateway/run.py could leave request_overrides as None: the no-service-tier early return, the exception fallback, and the direct agent attribute assignment
  • Normalize all three to {} and add a defensive or {} at the access site in run_agent.py

Closes #7215, closes #7259, closes #7277

Test plan

  • New test test_no_crash_when_request_overrides_is_none in TestBuildApiKwargsChatCompletionsServiceTier — sets request_overrides = None and verifies _build_api_kwargs succeeds
  • Updated test_turn_route_skips_priority_processing_for_unsupported_models to assert {} instead of None
  • All existing service tier and fast command tests pass

@konsisumer

Copy link
Copy Markdown
Contributor Author

Cherry-picked 4 commits to fix pre-existing test failures on main (none introduced by this PR):

  1. 3747caae — defensive getattr for _session_model_overrides and request_overrides (fixes ~20 test failures from tests using object.__new__())
  2. 040df35e — repair 13 pre-existing test failures: AudioRecorder.is_recording property, env var cleanup in auth gate tests, _attached_images in voice CLI helper, camofox config version bump
  3. 5d24e60c — add missing .request() / .get_updates_request() mock chain in telegram conflict tests (4 failures)
  4. c9d65f50 — use 'built-in' source for providers in PROVIDER_TO_MODELS_DEV fallback (1 failure)

Total: 38 of 39 pre-existing test failures addressed. The remaining 1 (test_memory_user_id.py::test_multiple_providers_all_receive_user_id) is a flaky skipif guard — importlib.util.find_spec() intermittently returns truthy for a non-existent module. This same test passes on other PRs with identical code.

@konsisumer
konsisumer force-pushed the fix/request-overrides-none-guard branch 4 times, most recently from 639b017 to ce586f3 Compare April 11, 2026 02:04
@konsisumer

Copy link
Copy Markdown
Contributor Author

Rebased onto latest origin/main and fixed the failing test_email_in_session_discovery test.

Root cause: Commit baddb6f7 (fix(gateway): derive channel directory platforms from enum instead of hardcoded list) refactored build_channel_directory() to iterate over the Platform enum dynamically instead of hardcoding platform names. The test used inspect.getsource() to check for a literal "email" string in the function body, which no longer exists.

Fix: Updated the test to verify that Platform contains "email" and that it's not in the _SKIP_SESSION_DISCOVERY frozenset.

Note: The nix (ubuntu-latest) / nix (macos-latest) failures are upstream Nix infrastructure issues (atomicwrites missing setuptools build dependency) — not related to this PR.

@konsisumer

Copy link
Copy Markdown
Contributor Author

Cherry-picked the atomicwrites Nix fix from #7513 — the nix (ubuntu-latest) failure was a pre-existing issue on main where atomicwrites-1.4.1 (sdist-only, legacy setup.py) fails to build without setuptools in the Nix build environment. This was not caused by this PR.

@konsisumer
konsisumer force-pushed the fix/request-overrides-none-guard branch 3 times, most recently from 1ae7736 to 9461b83 Compare April 11, 2026 19:27
@konsisumer

Copy link
Copy Markdown
Contributor Author

Rebased onto latest origin/main. Two intermediate commits were dropped during rebase: the getattr guard for run_agent.py (its access site is already guarded by or {} in main's refactored _build_api_kwargs), and the Dockerfile git dependency addition (already present in main). The core gateway fix (a245d0b7) applies cleanly. Local tests: 81 tests in the PR-specific files (test_fast_command.py, test_provider_parity.py) all pass. Four failures in the full suite (test_discord_allowed_mentions, test_minimax_provider, test_platform_commands, test_matrix) are pre-existing failures in files this PR does not modify.

@konsisumer
konsisumer force-pushed the fix/request-overrides-none-guard branch from f20d30f to 51c366a Compare April 22, 2026 05:42
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/gateway Gateway runner, session dispatch, delivery comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels Apr 22, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Multiple PRs fix the same None request_overrides crash: #7460, #8176, #7210, #7243 — maintainers should pick one and close the rest.

@konsisumer

Copy link
Copy Markdown
Contributor Author

Thanks for flagging the duplicates — happy to let maintainers pick whichever they prefer. For reference, this PR (#7305) normalizes all three None-assignment sites in gateway/run.py (the no-service-tier early return, the exception fallback, and the direct agent.request_overrides = assignment) rather than patching only the access site. The CI failures above are pre-existing failures in files this PR doesn't touch; the PR-specific tests (test_fast_command.py, test_provider_parity.py) pass cleanly.

@konsisumer
konsisumer force-pushed the fix/request-overrides-none-guard branch from 51c366a to a0486c0 Compare April 22, 2026 13:09
@konsisumer

Copy link
Copy Markdown
Contributor Author

Rebased onto latest origin/main. All 81 tests in the PR-specific files (test_fast_command.py, test_provider_parity.py) pass locally. The 9 CI failures in the prior run are all in files this PR does not modify (test_minimax_provider.py, test_agent_cache.py, test_tips.py, test_concurrent_interrupt.py, test_accretion_caps.py, test_browser_camofox.py, test_write_deny.py, test_zombie_process_cleanup.py) and are pre-existing upstream issues. Recent origin/main commits aa5bd092 and fd5df5fe address at least 3 of those 9 failures, so the next CI run on the rebased branch should see fewer failures.

@konsisumer

Copy link
Copy Markdown
Contributor Author

Rebased cleanly onto origin/main (1006 upstream commits); no conflicts. All 95 tests across tests/gateway/test_fast_command.py and tests/run_agent/test_provider_parity.py pass locally. The 27 failures in the prior CI run are all in files this PR does not modify (test_minimax_provider.py, test_agent_cache.py, test_tips.py, test_run_agent_codex_responses.py, test_streaming.py, test_ctx_halving_fix.py, test_accretion_caps.py, test_browser_camofox.py, test_modal_sandbox_fixes.py, test_write_deny.py, test_zombie_process_cleanup.py) — pre-existing upstream issues.

@konsisumer
konsisumer force-pushed the fix/request-overrides-none-guard branch from a0486c0 to e906994 Compare April 28, 2026 07:33
@konsisumer

Copy link
Copy Markdown
Contributor Author

Rebased onto latest origin/main (clean, no conflicts). All 95 tests across tests/gateway/test_fast_command.py and tests/run_agent/test_provider_parity.py pass locally. The 23 failures in the prior CI run are all in files this PR does not modify — pre-existing upstream issues.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via #17032. Your commit was cherry-picked onto current main with your authorship preserved in git log. Clean cherry-pick, no modifications needed — the fix was correct as-is. Thanks for the thorough bug report and the fix!

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

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/gateway Gateway runner, session dispatch, delivery P1 High — major feature broken, no workaround type/bug Something isn't working

Projects

None yet

3 participants