Skip to content

fix(delegation): honor configured API mode - #64470

Open
visualfox-ch wants to merge 1 commit into
NousResearch:mainfrom
visualfox-ch:fix/delegation-api-mode-resolution-v2
Open

fix(delegation): honor configured API mode#64470
visualfox-ch wants to merge 1 commit into
NousResearch:mainfrom
visualfox-ch:fix/delegation-api-mode-resolution-v2

Conversation

@visualfox-ch

Copy link
Copy Markdown

Summary

  • honor the configured delegation API mode instead of resolving it from unrelated runtime state
  • keep provider-specific delegation routing deterministic
  • add regression coverage for the configured mode path

Test plan

  • python -m pytest tests/tools/test_delegate.py -q -o 'addopts='
  • ruff check tools/delegate_tool.py tests/tools/test_delegate.py

Split from and supersedes the delegation portion of #50894.

Apply delegation.api_mode from config before provider defaults so provider-based subagents can use chat_completions when the selected model does not support responses API.

Assisted-by: Hermes:gpt-5.5
@alt-glitch alt-glitch added type/bug Something isn't working tool/delegate Subagent delegation area/config Config system, migrations, profiles P3 Low — cosmetic, nice to have labels Jul 14, 2026
@visualfox-ch

Copy link
Copy Markdown
Author

@teknium1 @OutThisLife Could you please approve the external-fork Actions runs for this focused PR set? All eight PRs are currently mergeable, but every run is action_required with zero jobs, so no CI code has executed yet.

The changes were split deliberately to keep review and rollback scope small. Local focused tests and lint checks passed before submission. No merge is requested before upstream CI and review.

@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

Fix to make delegation honor configured API mode. Small (38 additions, 1 deletion). No security concerns.


Reviewed by Hermes Agent

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for isolating the provider-based delegation gap; current main does ignore delegation.api_mode after resolving provider credentials (tools/delegate_tool.py:3161).

Problems

  • tools/delegate_tool.py:3148 overwrites every resolved provider mode. That bypasses provider-specific transport rules: xAI is explicitly forced to codex_responses in hermes_cli/runtime_provider.py:1490-1491, so a delegation config of chat_completions could route an xAI child onto the wrong wire protocol.

Suggested changes

  • Apply the delegation override through provider-aware runtime resolution, preserving providers whose transport is fixed by their resolver branch. Add a regression test for a fixed-mode provider alongside the Copilot override case.

This is an automated hermes-sweeper review.

Comment thread tools/delegate_tool.py
# providers expose model-specific exceptions where the runtime default is
# too broad; e.g. Copilot's claude-sonnet-4.6 is chat-completions only and
# rejects the Responses API.
if configured_api_mode in {"chat_completions", "codex_responses", "anthropic_messages"}:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This unconditional override also affects providers whose runtime intentionally fixes the transport. For example, resolve_runtime_provider() forces xAI to codex_responses; allowing delegation.api_mode=chat_completions here would construct an incompatible child. Please make this provider-aware and add a fixed-mode regression case.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/delegate Subagent delegation type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants