fix(delegation): honor delegation.fallback_providers for child agents (#65038)

[ayushnangia]

Summary

Honor a delegation.fallback_providers chain for delegated children, so a worker explicitly routed via delegation.* no longer silently escalates onto fallback models intended only for the head agent (#65038).

Changes

• tools/delegate_tool.py: _build_child_agent() — resolve the child chain as delegation.fallback_providers > parent inheritance. Key absent/null → inherit the parent's resolved _fallback_chain exactly as before (Feature request: per-delegation fallback provider chain #7481 behavior preserved); [] → explicitly disable child fallback; malformed value → logger.warning + inherit (mirrors the delegation.reasoning_effort fallback pattern a few lines up). Normalization reuses hermes_cli.fallback_config.get_fallback_chain(), the same chokepoint the top-level chain goes through — no parallel parser
• hermes_cli/config.py: delegation.fallback_providers: None added to DEFAULT_CONFIG (null sentinel = inherit, so the deep-merged default cannot flip existing users off inheritance; no _config_version bump needed for an added key)
• cli-config.yaml.example: commented example in the delegation: block
• tests/tools/test_delegate.py: TestDelegationFallbackChain — 5 tests: override beats inheritance, absent key inherits, null (the shipped default) inherits, [] disables, malformed inherits. The override and [] tests fail on unfixed code
• tests/tools/test_async_delegation.py: follow-up — deterministic drain of the two blocker completions in test_dispatch_rejected_at_capacity. This is a latent race, not fallout of this change: the blockers enqueue their completion events asynchronously after ev.set(), and any event landing after the fixture's teardown drain leaks into the next test's _drain_one(), which then reads a stale completed event instead of its own error event. Fails 8/8 consecutive bare-pytest runs on clean main; masked under scripts/run_tests.sh by hermetic-env timing until any config-load change (like the DEFAULT_CONFIG key above) re-times it

Root cause

_build_child_agent() reads the parent's resolved chain and passes it unconditionally:

parent_fallback = getattr(parent_agent, "_fallback_chain", None) or None
...
child = AIAgent(..., fallback_model=parent_fallback, ...)

The delegation config section is consulted on this path for provider, model, base_url, api_mode, and reasoning_effort — but not for a fallback chain; delegation.fallback_providers was never a key (git log -S shows the inheritance landed for #7481's "workers should inherit the parent chain", closed implemented_on_main). This issue is the complementary control: with a worker pinned to a distinct provider/model, a failure walks the head agent's chain — wrong models, wrong provider, wrong spend, silently.

The same leak was already recognized and fixed for OpenRouter provider filters in this exact function ("parent-level filters would silently force the child back onto the parent's provider" — cleared when delegation.provider is set). This change applies the identical reasoning to the fallback chain, but opt-in via config rather than implicit, since unconditional inheritance is deliberate, relied-upon behavior (#7481, and #49477's pinned-base_url coverage asserts it — the key-unset path here keeps those semantics byte-identical).

_build_child_agent() is the single AIAgent() construction site for delegation (sync and async paths both flow through it), so the chain resolution covers all delegated children.

Validation

delegation.fallback_providers	Before	After
unset (all existing configs)	inherit parent chain	inherit parent chain (unchanged)
null (shipped default)	—	inherit parent chain
[{provider: d, model: worker-fb}]	ignored — head chain used	worker chain used
[]	—	child fallback disabled
malformed entries	—	warn + inherit parent chain

• Runtime repro before/after: child fallback_model was [{provider-b, head-fallback-model}] with the worker chain configured; now receives the configured chain
• scripts/run_tests.sh tests/tools/test_delegate.py tests/tools/test_async_delegation.py tests/hermes_cli/test_config.py — 3 files, 339 passed, 0 failed
• scripts/run_tests.sh tests/hermes_cli/test_fallback_cmd.py — 32 passed
• tests/tools/test_async_delegation.py — 3× consecutive bare-pytest runs green (was 8/8 red on main)

Fixes #65038. Refs #7481, #49477, #65035 (sibling defect in the same file: the base_url credential path drops request_overrides — separate fix).

[tonydwb]

Code Review Summary

Verdict: Approved

Looks Good

• Fix honors delegation.fallback_providers for child agents (#65038)
• 71 additions, 2 deletions — minimal and targeted
• No issues detected

---

Reviewed by Hermes Agent

[teknium1]

Thanks for the focused fallback-routing fix. The premise holds on current main: _build_child_agent() loads delegation config at tools/delegate_tool.py:1100, but unconditionally derives the child fallback from parent_agent._fallback_chain at tools/delegate_tool.py:1280 and passes it at line 1332. Reusing hermes_cli/fallback_config.py:51-72 is the correct normalization path, and agent/agent_init.py:1166-1178 confirms that an explicit empty list disables the child chain.

Problems

• The new public setting needs user-documentation updates. website/docs/user-guide/features/fallback-providers.md:384 describes inherited child fallback, while line 431 still says delegation has no automatic fallback; website/docs/user-guide/configuration.md:1990-2011 omits the new key and its precedence semantics.
• The added tests mock both _load_config and AIAgent, so they do not cover the real profile-aware config/default merge used on this path.

Suggested changes

• Document absent/null inheritance, configured-chain override, [] disablement, and invalid-only fallback behavior in the two user guides.
• Add one temporary-HERMES_HOME config-load regression alongside the focused constructor tests.

This is an automated hermes-sweeper review.

[teknium1]

This introduces a user-facing config key, but the user guides still omit it and the fallback routing table currently says delegation has no automatic fallback. Please update website/docs/user-guide/features/fallback-providers.md and website/docs/user-guide/configuration.md with the override, inherit, and [] semantics.

[ayushnangia]

Done in aecc4c3 — added a "Delegation Fallback Chain" section to fallback-providers.md (semantics table: unset/null = inherit, list = own chain, [] = disabled, invalid-entries = warn + inherit), corrected the "Where Fallback Works" row and the routing table's "no automatic fallback" line, and documented the key in configuration.md's delegation block (yaml example + prose, cross-linked).

[teknium1]

These cases patch both _load_config and AIAgent, so they do not exercise the profile-aware config/default merge that _build_child_agent() uses. Please add one temporary-HERMES_HOME regression that loads delegation.fallback_providers through the real config loader.

[ayushnangia]

Done in aecc4c3 — test_fallback_providers_flow_through_real_config_loader writes delegation.fallback_providers into the hermetic $HERMES_HOME/config.yaml and drives _build_child_agent() with only run_agent.AIAgent mocked, so _load_config() \u2192 load_config_readonly() runs the real DEFAULT_CONFIG deep-merge (where the shipped default is None) against the user file. 162/162 in the file under scripts/run_tests.sh.

[mlahatte]

Reporter confirmation — still reproduces on current main (c2ee5039e), 12 days after this PR was opened.

I re-verified against upstream origin/main rather than my local checkout (my local copy was 44 lines behind on this file, so I checked git show origin/main:tools/delegate_tool.py directly):

1324:    parent_fallback = getattr(parent_agent, "_fallback_chain", None) or None
1379:            fallback_model=parent_fallback,

grep -c fallback_providers on origin/main:tools/delegate_tool.py → 0. The key is never consulted, matching @teknium1's line-level analysis above.

Repo-wide check — it is not read anywhere else either:

grep -rn "delegation.*fallback_providers\|fallback_providers.*delegation" --include=*.py . | grep -v ./tests/
# (no hits)

Meanwhile hermes_cli/config.py:936 ships "fallback_providers": [] inside the delegation config block, so the setting is advertised in the schema but inert at runtime — which is the part that makes this quietly costly: it fails silently. Nothing warns that the configured child chain is being discarded in favour of the parent's, so a user who sets a cheap flat-rate fallback for subagents can have children silently fall back onto the parent's (potentially metered) chain instead. That was my original motivation for filing #65038.

Minimal E2E repro against a temp HERMES_HOME with real imports, asserting the contract rather than a snapshot:

# delegation.fallback_providers: [{provider: anthropic, model: CHILD-FALLBACK-MODEL}]
# fallback_providers:            [{provider: openai-codex, model: PARENT-FALLBACK-MODEL}]
# EXPECT child chain <- CHILD-FALLBACK-MODEL
# ACTUAL child chain <- parent_agent._fallback_chain  (delegation key never read)

Not asking for special treatment on my own issue — just recording that the premise has not gone stale, since the sweeper marked this salvageability=high and @tonydwb approved it. Happy to test a merged build against my setup (delegation seat on openai-codex OAuth, parent on anthropic OAuth) if that would help move it along.

[ayushnangia]

Thanks for the re-verification @mlahatte.

Rebased onto current main (d71033a40) — the branch had gone stale after main wrapped the child AIAgent(...) construction in delegated_child_context() and extended the call site. The conflict was confined to that constructor call; the resolution is the same one-line fallback_model=child_fallback. Chain resolution, the None default, docs, and tests are unchanged from the reviewed head.

Validation on the rebased head (180746323):

• scripts/run_tests.sh tests/tools/test_delegate.py tests/hermes_cli/test_config.py — 353 passed, 0 failed
• scripts/run_tests.sh tests/tools/test_async_delegation.py — 40 passed, 0 failed

One clarification on the schema point: hermes_cli/config.py:936 on main is the top-level fallback_providers: []. The delegation block (config.py:2403 on d71033a40) has no fallback_providers key at all — repo-wide grep agrees with yours. So the key isn't advertised-but-inert; it doesn't exist yet, which is what this PR adds. The upside of that: since main ships no delegation-level default, merging with the None sentinel cannot flip any existing config from inherit to disabled.

And yes — I'd gladly take you up on validating a merged build against your split-credential setup (delegation on openai-codex OAuth, parent on anthropic OAuth); that's exactly the wrong-provider/wrong-spend path this guards.

[ayushnangia]

@mlahatte taking you up on your offer, ahead of the merge: the rebased branch is directly testable against your split-credential setup (delegation seat on openai-codex OAuth, parent on anthropic OAuth):

git fetch https://github.com/ayushnangia/hermes-agent fix/delegation-fallback-providers && git checkout FETCH_HEAD
# your config: delegation.fallback_providers with the worker chain

Expected: a delegated child's failure walks your configured chain instead of the parent's metered one; [] disables child fallback entirely. A reporter-verified run on the real credential split would be the strongest evidence this thread can carry.

[GottZ]

This was generated by AI during triage.

Summary

One PR addresses #65038. #65052 changes delegated-child fallback resolution so a configured delegation.fallback_providers chain overrides the parent chain, while unset/null preserves inheritance and [] disables child fallback.

Related pull requests

• fix(delegation): honor delegation.fallback_providers for child agents (#65038) #65052 best fix — (+162/-7) — n/a: The diff resolves delegation.fallback_providers through the existing fallback parser, passes the result to the child agent, documents the precedence semantics, and tests override, inheritance, disablement, malformed input, and the real config-loader path. The contributor keep_open review requested documentation and real profile-aware config coverage; both are present in the current diff.

Suggested consolidation

Keep #65052 open with a salvage path: retain the focused child-fallback resolution, public documentation, and regression coverage while the PR remains the sole implementation addressing #65038. This follows the contributor keep_open review, whose requested documentation and real-loader test are addressed by the visible diff; there are no competing PRs to close as duplicates.

Complex graph

flowchart 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
    I65038(["issue #65038 (open)"])
    P65052["PR #65052 (open)"]
    P65052 -->|best fix| I65038
    class I65038 open
    class P65052 open
    class P65052 best
    class P65052 target
    click I65038 "https://github.com/NousResearch/hermes-agent/issues/65038"
    click P65052 "https://github.com/NousResearch/hermes-agent/pull/65052"

Loading

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: 16 kB of PR diffs, 9 kB of issue/PR text, 6 kB of discussion (5 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[mlahatte]

Reporter-verified run, as offered above.

Environment. Isolated clone of this branch (head 3b916dc176) against upstream main a991dfc25; Python 3.11.14. No live model calls — AIAgent is mocked at the constructor boundary, so this observes the resolved fallback_model kwarg rather than spending credits.

1. Existing suites — 159/159 passed

scripts/run_tests.sh tests/tools/test_delegate.py \
                     tests/hermes_cli/test_config.py \
                     tests/tools/test_async_delegation.py

=== Summary: 3 files, 159 tests passed, 0 failed (100% complete) in 6.8s ===

2. Split-credential E2E through the real config loader

Wrote a config.yaml into a temp HERMES_HOME and called _build_child_agent with no _load_config patch, so the profile-aware DEFAULT_CONFIG deep-merge is actually exercised. Shape is the split you described: parent on anthropic OAuth, delegation seat on openai-codex OAuth, and a parent fallback chain whose entries are not the delegation seat.

case	expected	result
delegation.fallback_providers set	child gets the worker chain, not the parent's	pass
key absent	child inherits parent chain	pass
explicit []	child fallback disabled	pass

3/3. That first row is exactly the wrong-provider / wrong-spend path from #65038, and this diff closes it.

3. Operator note on [] and malformed input

Disclosure so the evidence is read correctly: we have been running an equivalent local patch in production since mid-July — not this diff, a narrower version of our own. So this is operator experience, not a first look.

Ours gates on a non-empty list:

if isinstance(cfg, list) and cfg:
    child = get_fallback_chain({"fallback_providers": cfg}) or None
else:
    child = parent_chain or None

Executing both against the real get_fallback_chain surfaces two divergences, and yours is better on both:

• [] — ours falls through to parent inheritance; yours disables. Yours is right. An operator needs some way to pin a worker to exactly one seat, and with inheritance-on-empty there is no way to express that.
• malformed entries (e.g. an entry with the model key missing) — get_fallback_chain returns [], and ours collapses that to None, which silently disables worker fallback. Yours warns and inherits.

That second one is a live bug in our patch that I found by testing yours. We are adopting your semantics locally.

No behavioral objections. The precedence table in fallback-providers.md matches what the code actually does in all four cases exercised here.

[ayushnangia]

Thank you — this is a more rigorous verification than I could have asked for: the real-config-loader E2E on the exact split-credential shape from #65038, plus the production-operator comparison.

Your malformed-entries finding is worth highlighting for whoever merges: your local patch's isinstance(cfg, list) and cfg collapse turns a typo'd entry (missing model) into silently disabled worker fallback — the same silent-failure class this whole issue is about, one level down. That's exactly why the warn+inherit semantics here mirror the delegation.reasoning_effort fallback pattern a few lines up rather than inventing a stricter one: a config mistake should degrade loudly to the safe default (inheritance), never silently to nothing. Glad the comparison surfaced it before it bit you in production.

For the record the thread now carries: premise confirmed by the sweeper review, reporter re-verification on c2ee5039e, and a reporter-run E2E matrix (3/3) on head 3b916dc17 against a991dfc25 — with an operator adopting these semantics over their own production patch.

[andrexibiza]

FILE-LIST / credit

#80421 reimplements this design against current main (6e9cae6ac4b) with DCO and a green local suite, and interlocks #65038 both ways.

Your approach (honor delegation.fallback_providers via get_fallback_chain, null/absent inherit, [] disables) is preserved and credited (Co-authored-by + PR body). Closing or superseding this PR is maintainer call — no conflict intended; happy to fold any unique bits you still want carried.

[ayushnangia]

Appreciate the clean interlock and the credit — the reimplementation against current main with both-ways #65038 linkage is exactly the right outcome for the design. No unique bits left here that #80421/#80479 don't carry; maintainer's call on close order, no objection from me.

[ayushnangia]

Cross-link for reviewers navigating the delegation-fallback family, since triage just closed #80438 into it: the precedence is #65052 (config plumbing — delegation.fallback_providers honored at all, docs + defaults) → #80479 (runtime resolution — _resolve_child_fallback_chain, full pin × config decision table, composes #80465 + the #80438/#80421 semantics with the matrix). #80474 is the executable map tracking the whole class. They're layered, not competing; reviewing #80479 first and #65052 second gives the full picture, and #80474 flips green as they land.

[ayushnangia]

Cross-ref: #85290 (@motochan) arrived today with the same delegation.fallback_providers feature for #65038 — fourth implementation of the class, same files, same issue. Coordination offer posted there; whichever way it resolves, this PR remains the earliest and broadest coverage.