Skip to content

fix(delegate): keep child credential pools provider-scoped - #58337

Open
izumi0uu wants to merge 1 commit into
NousResearch:mainfrom
izumi0uu:fix/delegation-credential-pool-58298
Open

fix(delegate): keep child credential pools provider-scoped#58337
izumi0uu wants to merge 1 commit into
NousResearch:mainfrom
izumi0uu:fix/delegation-credential-pool-58298

Conversation

@izumi0uu

@izumi0uu izumi0uu commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes delegated subagents accidentally reusing a parent credential pool whose concrete provider does not match the delegated child provider. When delegation is configured for a different provider/model, a mismatched pool lease can call _swap_credential() and overwrite the child runtime back to the pool provider.

This keeps same-provider pool sharing intact for cooldown/rotation state, but refuses concrete provider mismatches before pool resolution and before child leasing.

Related Issue

Fixes #58298

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • tools/delegate_tool.py: add a provider-match guard for child credential pools and apply it when resolving parent pool reuse and when leasing in _run_single_child().
  • tests/tools/test_delegate.py: cover rejecting a mismatched parent pool and skipping lease/swap/release when a child has a mismatched pool attached.

How to Test

  1. Reproduce the pre-fix resolver behavior on upstream/main (5daa5a0f2f218d2f5c8391dffbc47fe57f76232f): a deepseek parent with a zai credential pool returns the parent pool for a deepseek child.
  2. Run the after-fix resolver probe: deepseek child + zai parent pool resolves to None and does not return the parent pool.
  3. Run the targeted tests listed below.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15.7.7 (Darwin 24.6.0, arm64)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — provider/pool matching is platform-independent
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

For New Skills

N/A

Screenshots / Logs

python -m compileall -q tools/delegate_tool.py tests/tools/test_delegate.py
scripts/run_tests.sh tests/tools/test_delegate.py -q
=== Summary: 1 files, 153 tests passed, 0 failed (100% complete) in 18.9s (20 workers) ===
scripts/run_tests.sh tests/agent/test_credential_pool_routing.py tests/run_agent/test_fallback_credential_isolation.py -q
=== Summary: 2 files, 18 tests passed, 0 failed (100% complete) in 1.7s (20 workers) ===

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/delegate Subagent delegation P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jul 4, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #39862 (earliest open) — both add a provider-match guard to _resolve_child_credential_pool and skip the lease/swap in _run_single_child when a child credential pool's provider doesn't match the child runtime, for the same root cause (mismatched parent-pool lease overwriting the child provider). Same code site (tools/delegate_tool.py), same mechanism, same test scenarios.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for isolating the post-construction pool attachment and the lease/swap path; current main still has the reported mismatch path at tools/delegate_tool.py:3023 and :1770.

Problems

  • The new helper accepts every custom:* pool when the child provider is custom (tools/delegate_tool.py:3011). That does not preserve the endpoint boundary: a child at endpoint A can still lease a custom:endpoint-b pool and have _swap_credential replace its endpoint/key. Main already has the endpoint-aware contract in agent/credential_pool.py:464-499, with coverage in tests/agent/test_credential_pool_provider_boundary.py:16-26.

Suggested changes

  • Reuse credential_pool_matches_provider, passing effective_base_url in the resolver and child.base_url in _run_single_child.
  • Add resolver and lease-path tests for custom endpoint A versus a custom:endpoint-b pool.

Automated hermes-sweeper review.

Comment thread tools/delegate_tool.py Outdated
provider = provider.strip().lower()
if pool_provider == provider:
return True
return provider == "custom" and pool_provider.startswith("custom:")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This accepts any custom:* pool for a custom child, including a pool for a different endpoint. Please reuse agent.credential_pool.credential_pool_matches_provider and pass the runtime base URL; its custom-provider branch verifies the exact custom:<name> key.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
Delegated agents can intentionally use a provider/model different from the parent, but a reused parent credential pool was able to lease a credential for another concrete provider and swap the child's runtime back to that pool's provider. Keep same-provider sharing intact while refusing concrete pool/provider mismatches before resolution and leasing.\n\nConstraint: Same-provider credential pool sharing must keep cooldown and rotation state synchronized for delegated agents.\nRejected: Disable child credential leasing entirely | would regress valid same-provider rotation and custom-provider pooling.\nConfidence: high\nScope-risk: narrow\nDirective: Do not reintroduce parent pool reuse unless the pool provider matches the child provider or the same custom endpoint identity.\nTested: python -m compileall -q tools/delegate_tool.py tests/tools/test_delegate.py; scripts/run_tests.sh tests/tools/test_delegate.py -q; scripts/run_tests.sh tests/agent/test_credential_pool_routing.py tests/run_agent/test_fallback_credential_isolation.py -q; manual after-fix resolver probe for deepseek child with zai parent pool.\nNot-tested: Full pytest suite.
@izumi0uu
izumi0uu force-pushed the fix/delegation-credential-pool-58298 branch from e542739 to bdbb94d Compare July 19, 2026 12:22
@izumi0uu

Copy link
Copy Markdown
Contributor Author

Addressed in bdbb94d.

I replaced the local matching helper with
agent.credential_pool.credential_pool_matches_provider, passing
effective_base_url from _resolve_child_credential_pool and
child.base_url from _run_single_child.

I also added resolver and lease-path regressions for a child on custom
endpoint A with a custom:endpoint-b pool. Both paths now reject the
mismatched pool before it can be reused, leased, or passed to
_swap_credential.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/delegate Subagent delegation type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Subagent delegation ignores config, always uses credential-pool model (glm-4-flash)

3 participants