Conversation
Add an opt-in session_scoped mode (with an optional explicit scope) that confines file-access tool operations to a working folder derived from the session id via the shared _storage_key_segment derivation under the file-access-specific "~access-" prefix, mirroring FileMemoryProvider. The mode fails closed (ValueError) when neither a scope nor a session id is available, creates the session folder in before_run, appends a session-isolation note to the tool instructions, and logs the derived folder at debug level. All eight tools route store calls through a single _session_path helper; ls/grep treat an empty directory as the session root and grep results stay session-relative. Default mode is byte-for-byte unchanged. Adds seven regression tests, including the COLLIDING_IDENTIFIERS attack strings parametrization, fail-closed coverage, scope override, and shared-store default regression. All harness file-access tests pass; the only full-suite failure is the pre-existing test_feature_stage.py::test_feature_id_allows_lowercase_values on main. Addresses microsoft#8539.
Expose the FileAccessProvider session-scoped mode from create_harness_agent via a new file_access_session_scoped flag (default False, preserving shared-store semantics), threaded through _assemble_context_providers and synchronized across the .pyi stub and the package AGENTS.md file-access section. Adds a pass-through regression test. Addresses microsoft#8539.
agent-framework-automation
Bot
added
documentation
github-actions
Bot
changed the title
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Scoped instructions and directory validation are inconsistent with the new API’s documented behavior.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Adds opt-in session-scoped isolation for Python harness file-access tools.
Changes:
- Derives per-session or explicit-scope workspace paths.
- Exposes session scoping through
create_harness_agent. - Adds documentation and isolation tests.
| File | Description |
|---|---|
python/packages/core/agent_framework/_harness/_file_access.py |
Implements scoped file-access routing. |
python/packages/core/agent_framework/_harness/_agent.py |
Wires the new harness option. |
python/packages/core/agent_framework/_harness/_agent.pyi |
Updates the public signature stub. |
python/packages/core/tests/core/test_harness_file_access.py |
Tests isolation and compatibility. |
python/packages/core/tests/core/test_harness_agent.py |
Tests harness option wiring. |
python/packages/core/AGENTS.md |
Documents scoped file access. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+75
to
+78
| _SESSION_SCOPED_INSTRUCTIONS_SUFFIX = ( | ||
| "\n- Your file workspace is isolated to the current session: files written " | ||
| "here are not visible to other sessions or agents." | ||
| ) |
Contributor
Author
There was a problem hiding this comment.
Good catch — addressed in ebbbd6d. The instruction suffix now says the workspace is "isolated to the current session or configured scope: files written here are not visible outside that workspace", which is accurate in both modes. The same workspace-relative wording was also applied to the two docstring locations flagged here (the class docstring now notes the default is shared and scoped mode confines operations to the derived workspace; the session_scoped parameter doc now spells out "per session by default, or shared across sessions when an explicit scope is set").
All harness file-access tests still pass; ruff clean.
…kspace The session-scoped instruction suffix claimed files are never visible to other sessions, which is wrong when an explicit scope intentionally shares a workspace across sessions. Rephrase the suffix, the class docstring, and the session_scoped parameter docs to describe isolation relative to the resolved workspace (session or configured scope). Addresses review feedback on PR microsoft#8542.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
create_harness_agent'sfile_access_storeexpects a caller-suppliedAgentFileStorewhose root is already the desired working directory. UnlikeFileMemoryProvider,FileAccessProviderprovides no session isolation, so hosts that compile the harness agent before a session exists (AG-UI entrypoints, workflow executors restored from checkpoints) cannot give each session an isolated workspace without baking the session id into the path, recreating the agent per session, or mutating private store fields. Sessions therefore share one workspace, or hosts rely on private-API workarounds.Fixes #8539
Solution
Mirror
FileMemoryProvider's scoping model with an opt-insession_scopedmode onFileAccessProvider(plus an optional explicitscope). When enabled,before_runderives a working folder fromself.scope or context.session_idvia the shared_storage_key_segmentderivation under a file-access-specific~access-prefix, fails closed (ValueError) when neither is available, creates the folder, and all eight tools route store calls through a single_session_pathhelper;ls/greptreat an empty directory as the session root and grep results stay session-relative. Default mode is unchanged, preserving shared-store semantics and backward compatibility.create_harness_agentexposes it viafile_access_session_scoped.Changes
FileAccessProvider:session_scoped/scopeparams,_resolve_session_key(fail-closed,~access-prefix), session folder creation inbefore_run, session-isolation instruction suffix, debug log of the derived folder, and prefixed store paths across all eight tools.create_harness_agent:file_access_session_scopedflag, assembly pass-through,.pyistub sync, andpackages/core/AGENTS.mddoc update.COLLIDING_IDENTIFIERSattack-string parametrization, fail-closed coverage, scope override, grep session-relative names, default shared-store regression) and one wiring test.Testing
pytest tests/core/test_harness_file_access.py tests/core/test_harness_file_memory.py tests/core/test_harness_agent.py— all pass; new session-scoped tests were written first (red) and pass with the implementation (green).test_feature_stage.py::test_feature_id_allows_lowercase_values, which also fails on a clean checkout of main (verified via stash).ruff check/ruff formatclean; pyright reports no new errors in touched files.ValueError; default mode keeps files at the shared store root.Notes for Reviewer
~access-rather than the~session-/~scope-used by other components, per the core AGENTS.md convention of one~-prefixed namespace per component.scopeis deliberately not exposed oncreate_harness_agent(mirroring howFileMemoryProvideris wired without scope); callers needing an explicit scope construct the provider directly.file_access_store_factoryalternative, the provider-level plumbing is independent of that choice.