Conversation
Related: #34078, #33153, and #28869. #74099 preserves and prioritizes requested |
bf9ca8a to
da15bb5
Compare
|
Thanks for surfacing #34078, #33153, and #28869. The config contract intended by this PR is deliberately backward-compatible:
The distinction from #34078 is that this PR does not collapse the config contract to bare keys only. Hermes runtime routing already accepts named custom routes, normalized provider keys, and display The identity is also preserved through the secondary Current verification after rebasing onto main I’m happy to narrow or rebase this onto whichever config contract maintainers choose, but wanted to make the compatibility intent explicit. |
da15bb5 to
542ce1f
Compare
|
Thanks for the focused compatibility repair. The premise is verified on current The PR's route-first candidate lookup ( No blocking issue found in this read-only review. This is an automated hermes-sweeper review. |
SummarySeven open PRs address or reference this issue complex: #33153, #34078, and #74099 directly repair named-custom-provider timeout lookup, while #33466, #38644, #57194, and #62326 change adjacent timeout or streaming controls without repairing the same identity-loss cause. The direct fixes differ materially: #33153 infers identity from static config, #34078 propagates the runtime-requested provider but uses bare-key resolution, and #74099 propagates that identity across more construction paths with route-first alias and fallback lookup. Related pull requests
Suggested consolidationKeep #74099 open with the concrete salvage path of retaining its route-first alias/fallback lookup, broad identity propagation, and integration coverage while explicitly reconciling the config-key contract with #34078. Preserve #34078 as the recorded best-fix candidate but require author action to rebase onto main and complete the contributor-requested timeout paths and entrypoint tests; #33153 should likewise remain open only for a revised runtime-identity approach, and #33466, #38644, #57194, and #62326 should be triaged separately because their diffs address distinct timeout or streaming features, leaving no evidence-backed duplicate closures in this set. Cross-PR triage: Reviewed 7 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 149 kB of PR diffs, 22 kB of issue/PR text, 16 kB of discussion (13 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
542ce1f to
2c8dfd3
Compare
|
Production validation from a multi-profile Hermes deployment:
This is additional real-world evidence that the requested named-provider identity needs to survive canonicalization to the runtime For clarity, I’m not opening a competing PR here because #74099 already addresses the same root cause more comprehensively; I wanted to add production validation from a current multi-profile deployment. |
2c8dfd3 to
a4fd9cb
Compare
Summary
customtransportproviders.customretained as a fallbackRoot cause
Named custom providers intentionally resolve to
provider="custom"at runtime. Timeout lookup used only that canonical runtime ID, so settings underproviders.<named-route>were skipped and the generic stale watchdog default was used instead. Several secondaryAIAgentconstruction paths also dropped the already-resolvedrequested_provideridentity.Tests
AIAgent(provider="custom", requested_provider="custom:sub2api")**kwargsAIAgentconstruction pathsgit diff --checkpassed