Skip to content

Python: FoundryToolbox.as_skills_provider() disable_caching is a no-op; toolbox skills re-read every run #7100

Description

@giles17

Summary

FoundryToolbox.as_skills_provider() (in agent-framework-foundry-hosting) accepts a disable_caching parameter, but it currently has no effect, and the toolbox's skill discovery is re-read from skill://index.json on every agent run.

Surfaced during review of #7099: #7099 (comment)

Details

as_skills_provider() passes a caller-supplied SkillsSource (_FoundryToolboxSkillsSource) to SkillsProvider. By design, SkillsProvider does not auto-wrap a caller-supplied source in caching (it only caches the built-in Skill/sequence leaf sources it constructs itself), to avoid leaking skills across agents/tenants for context-aware sources. As a result:

  • disable_caching on as_skills_provider() is a no-op — it is forwarded to SkillsProvider, which ignores it for caller-supplied sources.
  • _FoundryToolboxSkillsSource.get_skills() runs MCPSkillsSource(...).get_skills() on every call, re-reading skill://index.json on every agent run — an avoidable MCP round-trip per request for a hosted agent.
  • The docstring claims "caching after the first discovery", which is inaccurate.

The same non-caching behavior exists in the reference sample samples/02-agents/providers/foundry/foundry_chat_client_with_toolbox_skills.py (SkillsProvider(MCPSkillsSource(client=session))), though it's a one-shot CLI so the cost is invisible there.

Proposed fix

_FoundryToolboxSkillsSource (like MCPSkillsSource) is context-independent — its get_skills ignores SkillsSourceContext and returns the same toolbox skills for every caller. So the leak concern that stops SkillsProvider from auto-caching does not apply. as_skills_provider() can safely compose the caching itself, matching the pattern core/AGENTS.md documents for custom pipelines:

python source: SkillsSource = _FoundryToolboxSkillsSource(self) if not disable_caching: source = DeduplicatingSkillsSource(CachingSkillsSource(source, refresh_interval=cache_refresh_interval)) return SkillsProvider(source, ...) # stop forwarding disable_caching (no-op there)

Also worth considering:

  • Add a cache_refresh_interval: timedelta | None param (forwarded to CachingSkillsSource) so callers can re-query when a toolbox's attached skills change over the process lifetime.
  • Update the docstring to describe the real behavior.
  • Add unit tests asserting the index is read once by default and re-read every run when disable_caching=True.
  • Decide whether the reference sample above should adopt the same pattern.

Acceptance criteria

  • disable_caching on as_skills_provider() has an observable effect.
  • By default, toolbox skill discovery is cached (skill://index.json read once, not per run).
  • Docstring accurately describes caching behavior.
  • Unit tests cover cached vs. non-cached behavior.

Metadata

Metadata

Assignees

Labels

pythonUsage: [Issues, PRs], Target: Python

Type

No type

Fields

No fields configured for issues without a type.

Projects

Status
No status

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions