fix(delegation): carry provider request_overrides through the base_url path (#65035) - #65054
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Looks Good
- Well-scoped fix: carries request_overrides through the base_url delegation path, addressing issue #65035
- Clean additions only (82 lines), no deletions
- Logic is straightforward — preserves the override dict and passes it through provider_request_overrides
Reviewed by Hermes Agent
|
Thanks for the focused delegation fix. The premise remains valid on current main: the direct The proposed best-effort runtime lookup preserves the existing explicit endpoint behavior while forwarding values that the runtime resolver already exposes for custom providers ( This is an automated hermes-sweeper review. |
643bc2f to
eca9b3d
Compare
SummaryOne PR addresses #65035. The diff in #65054 corrects the reported explicit-base_url path by forwarding the configured provider runtime’s request_overrides and max_output_tokens while retaining explicit endpoint, key, and API-mode precedence. Related pull requests
Suggested consolidationKeep #65054 open with a salvage path: preserve its direct credential-path fix and regression coverage, with author action to rebase onto main if needed. It is the only PR in this complex, so there are no duplicates to close. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I65035(["issue #65035 (open)"])
P65054["PR #65054 (open)"]
P65054 -->|best fix| I65035
class I65035 open
class P65054 open
class P65054 best
class P65054 target
click I65035 "https://github.com/NousResearch/hermes-agent/issues/65035"
click P65054 "https://github.com/NousResearch/hermes-agent/pull/65054"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 6 kB of PR diffs, 7 kB of issue/PR text, <1 kB of discussion (1 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Summary
Carry the configured provider's
request_overrides(andmax_output_tokens) through_resolve_delegation_credentials()'sbase_urlshort-circuit, so subagents pinned to an explicit endpoint stop silently losing provider request settings likeextra_body: {thinking: {type: disabled}}(#65035).Changes
tools/delegate_tool.py:_resolve_delegation_credentials()— whendelegation.provideris configured alongsidedelegation.base_url, resolve the provider runtime (best-effort) and include itsrequest_overrides+max_output_tokensin the returned bundle. Explicitly configuredbase_url/api_key/api_modekeep winning; a resolution failure logs at debug and proceeds without overrides instead of failing the dispatch (the explicit endpoint worked before this change even when the provider couldn't resolve)tests/tools/test_delegate.py: 3 tests inTestDelegationCredentialResolution— provider+base_url carries the runtime's overrides while config endpoint/key win; bare base_url returnsNoneoverrides (shape parity with the inherit path); runtime failure keeps the endpoint working. All 3 fail on unfixed code (KeyError: request_overrides)Root cause
_resolve_delegation_credentials()has three return paths. The provider path includes"request_overrides": dict(runtime.get("request_overrides") or {}); the inherit path returns"request_overrides": None; but thebase_urlpath — which wins wheneverdelegation.base_urlis set, even with a provider configured — returns a dict without the key entirely. The caller then doesso both silently resolve to
None, and_build_child_agent()'soverride_provider-gated branch hands the childdict(None or {})— empty overrides. A custom provider whose runtime definition carriesextra_bodybehaves correctly in the parent conversation and reverts to provider defaults in every subagent.Sibling of #65038 (fixed in #65052): same file, same class — a delegation config value accepted but dropped on one resolution path. Adjacent to #56876 (parent-runtime
request_overridesinheritance semantics in_build_child_agent); this change is confined to_resolve_delegation_credentialsand doesn't touch that region, so the two compose rather than conflict.Validation
delegation:configprovider+base_url(provider runtime carriesextra_body)request_overrides: None,max_output_tokens: None— overrides silently droppedrequest_overrides+max_output_tokenscarried; configuredbase_url/api_keystill winbase_urlonly.get→ None)Noneexplicitly (shape parity)provider+base_url, provider unresolvableE2E on the resolution path (mocked runtime with
thinking: {type: disabled}):scripts/run_tests.sh tests/tools/test_delegate.py tests/tools/test_async_delegation.py— 2 files, 191 passed, 0 failedFixes #65035. Refs #65038/#65052, #56876.