Skip to content

Python: Add tool name prefixes to FileAccessProvider - #9654

Merged
Jose Alvarez (jpalvarezl) merged 5 commits into
mainfrom
jpalvarezl/fix/issue_9646_tool_dups_multiple_file_stores
Oct 26, 2026
Merged

Jose Alvarez (jpalvarezl) merged 5 commits into
mainfrom
jpalvarezl/fix/issue_9646_tool_dups_multiple_file_stores

Conversation

@jpalvarezl

Copy link
Copy Markdown
Member

Motivation & Context

An agent configured with multiple FileAccessProvider instances fails while merging tools because every provider exposes the same file_access_* names, even when the providers have distinct source IDs and independently rooted stores.

Allow callers to distinguish each provider's tools so an agent can access multiple file stores without tool-name collisions.

Description & Review Guide

  • What are the major changes?
    • Add an optional, keyword-only tool_name_prefix constructor parameter to FileAccessProvider and apply it to all eight registered tool names.
    • Keep the existing names when the prefix is omitted or empty. A prefix such as spec produces names such as spec_file_access_read.
    • Add regression coverage for the issue reproduction, distinct prefixes, prefixed/default provider coexistence, all eight tool names, and reads/writes routed to the correct store.
  • What is the impact of these changes?
    • Providers with distinct tool names can coexist on one agent without changing the default tool names or shared class constants.
    • Distinct source IDs alone still do not prevent collisions; callers must configure distinct prefixes when combining providers.
  • What do you want reviewers to focus on?
    • Prefix semantics, backward compatibility, and correct per-store routing.
    • This is a draft: default instructions and the static auto-approval rules still refer to the original, unprefixed names. Those surfaces need to be aligned with prefixed names before the change is ready for review. The auto-approval mismatch fails closed and leaves approval required.

Related Issue

Fixes #9646

Maintainer agreement with the direction was explicitly confirmed by the submitter during preparation of this draft. No competing open PR was found for the issue.

AI Assistance

  • No material AI assistance was used.
  • This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: GitHub Copilot assisted with code analysis and review, regression tests, validation, and this PR description. The implementation was authored by the contributor.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

The focused file-access unit test module passes: 131 passed, 7 skipped, with three existing warnings. Ruff lint/format checks and Pyright pass for both changed files. The full core unit suite and a package build have not been run, so their checklist items remain unchecked.

@github-code-quality

github-code-quality Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Code coverage overview

Languages: Python

Python / code-coverage/python

main bdf5256 jpalvarezl/fix/issue... f1e886c +/-
92% 92% 0%

ℹ️ Calculations represent line coverage. Results may change as calculations are updated.


Updated October 26, 2026 12:30 UTC

Copilot AI 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.

🟡 Changes recommended

Model-facing guidance and auto-approval rules still use unprefixed names and need to support the configured prefixes.

2 open issues
What changed in this PR

Adds optional tool-name prefixes to Python’s FileAccessProvider, addressing #9646 by allowing independently rooted stores to coexist on one agent without tool-name collisions.

Changes:

  • Adds keyword-only tool_name_prefix and applies it to all eight tools.
  • Preserves existing tool names when the prefix is omitted or empty.
  • Adds regression coverage for collisions, mixed provider configurations, and per-store read/write routing.
File Description
python/​packages/​core/​tests/​core/​test_harness_file_access.py Tests prefix naming, provider coexistence, and isolated store routing.
python/​packages/​core/​agent_framework/​_harness/​_file_access.py Adds prefix configuration and applies it during tool registration.

Review effort: Balanced

Comment thread python/packages/core/agent_framework/_harness/_file_access.py
Comment thread python/packages/core/agent_framework/_harness/_file_access.py

@github-actions github-actions Bot 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.

MAF Automated Review — Iteration 1

Result: Findings reported
Scope: full PR (1 commit(s)): 5e1d6a75ec8b
Model: gpt-5.6-sol

Overview

The PR consistently applies the configured prefix to all eight tools, preserves default names for falsey prefixes, rejects remaining duplicate names centrally, and tests per-store routing. However, renaming leaves the public auto-approval rules authoritative over the old names, which can bypass approval for an unrelated local tool, while the default model instructions still advertise those old names. Prefixes are also accepted verbatim even when they produce names rejected by model providers.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
3 verified findings remained after source verification (1 high, 2 medium) across 1 file. Details are attached to the affected lines below.

Affected areas: python/packages/core/agent_framework/_harness/_file_access.py

Comment thread python/packages/core/agent_framework/_harness/_file_access.py
Comment thread python/packages/core/agent_framework/_harness/_file_access.py
Comment thread python/packages/core/agent_framework/_harness/_file_access.py
@jpalvarezl
Jose Alvarez (jpalvarezl) added this pull request to the merge queue Oct 26, 2026
Merged via the queue into main with commit 89c7018 Oct 26, 2026
52 checks passed
@jpalvarezl
Jose Alvarez (jpalvarezl) deleted the jpalvarezl/fix/issue_9646_tool_dups_multiple_file_stores branch October 26, 2026 13:32

This branch was previously deployed

1 inactive deployment
github-app-auth — f1e886c8 Deployed Oct 26, 2026 by jpalvarezl via add_label #26160
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: Multiple FileAccessProvider context providers fail with duplicate tool names

3 participants