Skip to content

Python: fix(foundry-hosting): root file-based approval storage under durable home directory - #7431

Open
PratikWayase wants to merge 2 commits into
microsoft:mainfrom
PratikWayase:fix/7414-fha-approval-storage-home
Open

Python: fix(foundry-hosting): root file-based approval storage under durable home directory#7431
PratikWayase wants to merge 2 commits into
microsoft:mainfrom
PratikWayase:fix/7414-fha-approval-storage-home

Conversation

@PratikWayase

Copy link
Copy Markdown
Contributor

Motivation & Context

Previously, ResponsesHostServer.__init__ rooted the file-based approval storage at the absolute path /.function_approvals/approval_requests.json (the filesystem root). When a WorkflowAgent session compute was stopped and reached idle, a continuation request would recreate the compute, but the approval requests were not restored. Because the approval storage was written to the ephemeral root path, it was lost, causing function approvals to fail after compute recreation.

This is the same class of issue as #7137, which was fixed for checkpoint storage in #7220 by moving from /.checkpoints to $HOME/.checkpoints.

Description & Review Guide

What are the major changes?

  1. Added _resolve_approval_storage_path(is_hosted) static method: When hosted, this method roots the store at $HOME/.function_approvals/approval_requests.json (reading the HOME environment variable, defaulting to /home/session) instead of /.function_approvals/approval_requests.json.
  2. Added defensive guard: The method validates that HOME is not unset, blank, or the filesystem root (e.g., /). If it is, it falls back to /home/session/.function_approvals/approval_requests.json to guarantee the store never lands under the read-only root.
  3. Updated __init__: The approval storage path assignment now calls _resolve_approval_storage_path(self.config.is_hosted) instead of using the raw FUNCTION_APPROVAL_STORAGE_PATH constant.
  4. Added 4 unit tests: covering local mode, hosted mode with valid HOME, missing HOME, and unusable HOME values (/, empty, whitespace).

What is the impact of these changes?

  • Hosted WorkflowAgent function approval storage now persists to the platform's durable $HOME volume, surviving across requests and compute recreation (idle → active).
  • Non-hosted/local usage is unchanged (continues to use {cwd}/.function_approvals/approval_requests.json).

What do you want reviewers to focus on?

Related Issue

Fixes #7414

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 is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • 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.

Copilot AI review requested due to automatic review settings July 30, 2026 09:58
@agent-framework-automation agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Jul 30, 2026

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.

Pull request overview

This PR aims to make hosted ResponsesHostServer function-approval persistence durable across compute recreation by moving the approval-requests JSON storage from the ephemeral filesystem root (/.function_approvals/...) to a durable home directory ($HOME/.function_approvals/..., with a safe fallback to /home/session/...).

Changes:

  • Adds _resolve_approval_storage_path(is_hosted) to compute a hosted-durable approval storage path (with guards for invalid HOME).
  • Updates ResponsesHostServer.__init__ to compute and store _approval_storage_path.
  • Adds unit tests intended to validate local vs hosted approval storage path behavior under various HOME values.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py Adds approval storage path resolution logic and stores the computed value on the server instance.
python/packages/foundry_hosting/tests/test_responses.py Adds/updates tests intended to validate approval storage path resolution behavior.
Comments suppressed due to low confidence (2)

python/packages/foundry_hosting/tests/test_responses.py:4450

  • Similar to the valid-HOME test, this case should also validate the path actually used by the hosted file-based approval store (_approval_storage._storage_path), not only the computed _approval_storage_path attribute.
        server = ResponsesHostServer(MagicMock(), store=InMemoryResponseProvider())
        expected = "/home/session/.function_approvals/approval_requests.json"
        assert server._approval_storage_path == expected

python/packages/foundry_hosting/tests/test_responses.py:4462

  • This test should validate the fallback path is the one actually used by the hosted approval storage (_approval_storage._storage_path), otherwise a wiring bug (still writing to /.function_approvals) can slip through even if _approval_storage_path is correct.
        server = ResponsesHostServer(MagicMock(), store=InMemoryResponseProvider())
        expected = "/home/session/.function_approvals/approval_requests.json"
        assert server._approval_storage_path == expected
        assert not server._approval_storage_path.startswith("/.function_approvals")

Comment thread python/packages/foundry_hosting/tests/test_responses.py Outdated
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]: Move FHA file-based approval storage to under $HOME

2 participants