Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions tests/tools/test_delegate.py
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,46 @@ def test_missing_config_keys_inherit_parent(self):
self.assertIsNone(creds["model"])
self.assertIsNone(creds["provider"])

@patch("hermes_cli.runtime_provider.resolve_runtime_provider")
def test_copilot_delegation_api_mode_uses_delegation_model(self, mock_resolve):
"""When copilot resolves codex_responses (main model GPT-5+), delegation model
GPT-4.x should override to chat_completions.

Regression test for: delegation api_mode was derived from main model
(gpt-5.4 → codex_responses) instead of delegation model (gpt-4.1 →
chat_completions), causing HTTP 400 on subagent creation.
"""
# Simulate: resolve_runtime_provider returns codex_responses because
# the main model is gpt-5.4 (GPT-5+ triggers Responses API)
mock_resolve.return_value = {
"provider": "copilot",
"base_url": "https://api.githubcopilot.com",
"api_key": "ghu_test_key_123",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

"api_mode": "codex_responses", # Wrong for gpt-4.1
}
parent = _make_mock_parent(depth=0)
cfg = {"model": "gpt-4.1", "provider": "copilot"}
creds = _resolve_delegation_credentials(cfg, parent)
# The fix should override api_mode to chat_completions for gpt-4.1
self.assertEqual(creds["api_mode"], "chat_completions")
self.assertEqual(creds["model"], "gpt-4.1")
self.assertEqual(creds["provider"], "copilot")

@patch("hermes_cli.runtime_provider.resolve_runtime_provider")
def test_copilot_delegation_api_mode_preserves_responses_for_gpt5(self, mock_resolve):
"""When delegation model is also GPT-5+, codex_responses should be preserved."""
mock_resolve.return_value = {
"provider": "copilot",
"base_url": "https://api.githubcopilot.com",
"api_key": "ghu_test_key_123",
"api_mode": "codex_responses",
}
parent = _make_mock_parent(depth=0)
cfg = {"model": "gpt-5.4", "provider": "copilot"}
creds = _resolve_delegation_credentials(cfg, parent)
# GPT-5.4 should keep codex_responses
self.assertEqual(creds["api_mode"], "codex_responses")


class TestDelegationProviderIntegration(unittest.TestCase):
"""Integration tests: delegation config → _run_single_child → AIAgent construction."""
Expand Down
17 changes: 16 additions & 1 deletion tools/delegate_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -801,12 +801,27 @@ def _resolve_delegation_credentials(cfg: dict, parent_agent) -> dict:
f"Set the appropriate environment variable or run 'hermes auth'."
)

api_mode = runtime.get("api_mode")

# Fix: when delegation uses copilot with a different model than the main
# agent, the api_mode resolved by resolve_runtime_provider is based on the
# main model (model.default in config). We must re-derive it for the
# 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",):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

try:
from hermes_cli.models import copilot_model_api_mode
api_mode = copilot_model_api_mode(configured_model, api_key=api_key)
except Exception:
api_mode = "chat_completions"

return {
"model": configured_model,
"provider": runtime.get("provider"),
"base_url": runtime.get("base_url"),
"api_key": api_key,
"api_mode": runtime.get("api_mode"),
"api_mode": api_mode,
"command": runtime.get("command"),
"args": list(runtime.get("args") or []),
}
Expand Down