fix: derive delegation api_mode from delegation model, not main model - #6647
fix: derive delegation api_mode from delegation model, not main model#6647AlsayedHoota wants to merge 2 commits into
Conversation
When delegation.provider is 'copilot' and the delegation model differs from the main model, the api_mode was incorrectly derived from the main model's name. For example, a main model of gpt-5.4 triggers the Responses API (codex_responses), but when the delegation model is gpt-4.1, this caused HTTP 400 because gpt-4.1 is not supported on the Responses API. The fix re-derives the api_mode for the delegation model specifically, using copilot_model_api_mode() with the delegation model name instead of relying on the runtime resolution which uses the main model config. This affects any setup where: - model.default is a GPT-5+ model (uses Responses API) - delegation.model is a GPT-4.x or non-GPT model (needs Chat Completions) - delegation.provider is copilot
Two new tests in TestDelegationCredentialResolution: 1. test_copilot_delegation_api_mode_uses_delegation_model: Verifies that when copilot resolves codex_responses (main model is GPT-5+), a GPT-4.x delegation model correctly gets chat_completions instead. 2. test_copilot_delegation_api_mode_preserves_responses_for_gpt5: Verifies that when delegation model is also GPT-5+, the codex_responses mode is correctly preserved. Both tests pass.
`resolve_runtime_provider()` derives the Copilot `api_mode` from the default model in `config.yaml`, but a cron job may override the model. For example, with `model.default: qwen3.5-auto` and a cron job set to `copilot/gpt-5.4-mini`, the job inherits `api_mode: chat_completions` when it should be `codex_responses` for the GPT-5 family. The job then hits `https://api.githubcopilot.com/chat/completions` with a model that only supports the Responses API and gets HTTP 400. After resolving the runtime provider, re-derive `api_mode` via `copilot_model_api_mode()` using the job's actual model when the provider is `copilot`. Same fix pattern as NousResearch#6647 (delegation path). Adds tests covering the corrected mode for a copilot job and a no-op for non-copilot providers.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying a real delegation routing failure. Current main still has the underlying Copilot resolver defect, but this branch needs a focused salvage onto the shared resolver.
Problems
tools/delegate_tool.py:3095already forwardstarget_model, buthermes_cli/runtime_provider.py:446and:2003call_copilot_runtime_api_mode()which readsmodel_cfg['default']at:320. Recomputing only in this delegation caller leaves the same resolver bug for its other Copilot users.- The new tests pass fake keys at
tests/tools/test_delegate.py:720and:737; the added call attools/delegate_tool.py:815invokescopilot_model_api_mode, which fetches the Copilot catalog when an API key is supplied (hermes_cli/models.py:3312-3315,:2857-2867). The tests should mock that fetch.
Suggested changes
- Thread
target_modelinto_copilot_runtime_api_mode()and its Copilot resolver branches, falling back tomodel.defaultonly when absent. - Make the regression tests catalog-stubbed and network-free.
Automated hermes-sweeper review.
| mock_resolve.return_value = { | ||
| "provider": "copilot", | ||
| "base_url": "https://api.githubcopilot.com", | ||
| "api_key": "ghu_test_key_123", |
There was a problem hiding this comment.
This fake key is passed to copilot_model_api_mode() by the new implementation, which fetches the live Copilot catalog whenever api_key is non-empty (hermes_cli/models.py:3312-3315). Please mock fetch_github_model_catalog (or otherwise supply a static catalog) so this regression test remains hermetic.
| # delegation model. E.g. main model gpt-5.4 → codex_responses, but | ||
| # delegation model gpt-4.1 → chat_completions. Without this, the | ||
| # subagent sends gpt-4.1 to the Responses API which returns HTTP 400. | ||
| if configured_model and configured_provider in ("copilot",): |
There was a problem hiding this comment.
Please move this target-model selection into the shared Copilot runtime resolver instead. Current main already forwards target_model from delegation, but _copilot_runtime_api_mode() still reads model.default; fixing it there covers both Copilot resolver paths rather than adding a delegation-only post-resolution override.
|
Heads up: this looks like it landed on main via #19623 (commit 83bbe9b) — both patch tools/delegate_tool.py::_resolve_delegation_credentials so delegation api_mode is derived from configured_model by passing it as target_model to resolve_runtime_provider. Flagging in case you'd like to close or rebase. |
Problem
When
delegation.provideriscopilotand the delegation model differs from the main model, theapi_modewas incorrectly derived from the main model's name via_copilot_runtime_api_mode(model_cfg, ...).For example:
gpt-5.4→ triggers Responses API (codex_responses)gpt-4.1→ needs Chat Completions (chat_completions)The subagent would send
gpt-4.1to the Responses API endpoint, resulting in:Root Cause
In
_resolve_delegation_credentials(), the copilot credentials are resolved viaresolve_runtime_provider(requested='copilot'), which internally calls_copilot_runtime_api_mode(model_cfg, ...). This function readsmodel_cfg['default'](the main model name, e.g.gpt-5.4) to determine the API mode, not the delegation model.Fix
After resolving copilot credentials, re-derive the
api_modeusingcopilot_model_api_mode()with the delegation model name (configured_model). This correctly maps:gpt-4.1→chat_completionsgpt-5.4→codex_responsesgpt-5-mini→chat_completionschat_completions(oranthropic_messagesvia catalog)Affected Configurations
Any setup where:
model.defaultis a GPT-5+ model (uses Responses API)delegation.modelis a GPT-4.x or non-GPT model (needs Chat Completions)delegation.provideriscopilot