Python: Improve python sample validation workflow

[TaoChenOSU]

Motivation & Context

Addresses #7348

Description & Review Guide

• What are the major changes?
  • Temporarily disables automatic validation on a subset of samples due to GHCP instability.
  • Breaks jobs into multiple ones so that they are easier to be managed and become more stable. For example, this PR breaks the hosting sample job into Foundry hosting agent samples and other hosting samples.
  • Adds playbook capability where previously validated samples that remain unchanged (gated by the sample hashes) such that the sample can be replayed deterministically. GHCP will only get invoked if a sample is changed or replay fails.
  • Fixes a few samples that are broken.
• What is the impact of these changes?
  • A more stable and efficient sample validation workflow.
• What do you want reviewers to focus on?
  • The fixes on the broken samples and format of the playbook.

Run result:

Related Issue

Fixes #7348

We plan to reenable the remaining jobs in the future. Item tracking this effort: #7349

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.

[github-actions]

Flagged issue

python/scripts/sample_validation/playbook.py:107 only includes *.py in the cache key, but the new hosted-agent contract defines the sample as the entire folder (SKILL.md:97-106) and compute_sample_hash itself says "any change to the sample invalidates the cached playbook" (playbook.py:114-117). Changes to agent.yaml, agent.manifest.yaml, or requirements.txt will therefore reuse a stale playbook and can silently skip revalidation.

---

Source: automated DevFlow PR review

[github-actions]

Flagged issue

python/scripts/sample_validation/playbook.py:238 rebuilds replay targets as python_root / sample_path, even though SampleInfo.relative_path is stored relative to samples_dir (models.py:36-40) and samples_dir is python_root / "samples" (__main__.py:124-126). That means replay snapshots/restores python/04-... instead of python/samples/04-..., so any playbook that edits sample files can leave the repo dirty despite the replay contract at playbook.py:232-235.

---

Source: automated DevFlow PR review

[github-actions]

Automated Code Review

Reviewers: 5 | Confidence: 93%

✗ Correctness

The PR introduces playbook caching, sample discovery improvements, and several sample fixes. One high-severity bug exists: delete_hosted_agent_session passes session.session_id (a locally generated UUID) instead of session.service_session_id (the actual service-managed ID) to the delete API, which will fail to delete the hosted session server-side. The previous resolved review comment flaged a related issue but the code still uses the wrong ID. The magentic output handling changes are correct. The new playbook caching system and replay executor are well-structured. The main correctness issue is in replay_playbook where the on-disk sample path is reconstructed incorrectly: playbook.sample stores a path relative to samples_dir (python/samples/) but is joined with python_root (python/), producing a non-existent path like python/04-hosting/... instead of python/samples/04-hosting/.... This makes the snapshot/restore mechanism silently no-op, so a replay script that edits sample files will leave the repository dirty. This was previously flagged and marked resolved, but the diff still shows the unfixed code.

✗ Security Reliability

The delete_hosted_agent_session function in using_deployed_agent.py passes session.session_id (a locally-generated UUID) to the server-side delete_session API, instead of session.service_session_id (the actual hosted-agent session identifier). This was introduced when refactoring the old code which correctly used session.service_session_id. The playbook replay system is well-structured overall with good process isolation and cleanup. However, the path reconstruction bug at playbook.py:238 (previously flaged and resolved but still present in this diff) means the snapshot/restore mechanism operates on incorrect paths, breaking the guarantee that replays never leave the repository dirty. No new security issues beyond the inherent design of executing cached agent-authored scripts.

✓ Test Coverage

This PR introduces substantial new infrastructure code for the sample validation system — playbook caching/replay (~350 lines in playbook.py), a replay executor (replay_executor.py), directory-based sample discovery (discovery.py changes), and snapshot/restore logic (create_dynamic_workflow_executor.py) — with zero unit test coverage. Grep across all test files confirms no file references discover_samples, DiscoverSamplesExecutor, or sample_validation. While this is CI tooling rather than production code, several modules contain non-trivial logic (subprocess process-group management, file-system snapshot/restore, content-hash-based cache invalidation) that is straightforward to unit test and prone to subtle bugs. The sample_validation module has historically had no tests, but the new code contains pure, easily-testable functions (hash computation, serialization round-trips, store load/save, slug generation) and async logic with meaningful edge cases (timeout handling, process cleanup, snapshot/restore). At minimum the core playbook.py functions warrant unit tests to prevent regressions in the caching logic that now gates whether the LM agent is invoked at all.

✗ Failure Modes

The PR is a large workflow infrastructure improvement with playbook caching, job splitting, and sample fixes. Most changes are sound. One concrete failure-mode bug exists: the new delete_hosted_agent_session helper passes session.session_id (an auto-generated local UUID) to the server-side delete call instead of session.service_session_id (the real hosted-agent session identifier). This silently fails to clean up the remote session, leaking server-side resources. This was flagged in a prior review round as resolved, but the code still contains the bug. The path construction in replay_playbook (playbook.py:238) reconstructs the sample's on-disk path incorrectly: playbook.sample stores a path relative to samples_dir (i.e., python/samples/), but line 238 joins it to python_root (python/), omitting the samples/ segment. This causes the snapshot/restore safety mechanism to silently target a non-existent path, so the snapshot is empty and the repo can be left dirty after a replay that edits sample files. This issue was previously raised in an earlier review round and marked resolved, but the code in the current diff remains unchanged.

✗ Design Approach

I found two design-level problems in the current chunk. One is a correctness bug in the hosted-agent cleanup sample: it now deletes sessions by the local AgentSession.session_id instead of the service-managed session id returned by Foundry, which can leak hosted sessions. The other is in the new directory-sample discovery rule: treating any folder with main.py/app.py as a terminal sample and stopping descent silently drops real child samples such as the multiple DevUI samples documented under samples/02-agents/devui. The replay cache path reconstruction is still inconsistent with how samples are discovered and stored, so replay can miss the real sample tree when it snapshots/restores edits. Separately, the new hosted-agent validation script skips the native-runtime phase for every invocations sample even though the documented contract and repo sample matrix treat invocations samples as first-class hosted-agent samples that should pass all three validation phases.

Flagged Issues

• playbook.py:238 — SampleInfo(path=python_root / sample_path, ...) constructs the wrong absolute path because sample_path (from playbook.sample) is relative to samples_dir (python/samples/), not python_root (python/). This causes the snapshot/restore mechanism to silently target a non-existent directory, so replays that edit sample files leave the repository dirty. Previously flaged and marked resolved, but the current diff still contains the unfixed code.
• using_deployed_agent.py:65 — session.session_id is a locally generated UUID, not the service-managed session ID stored in session.service_session_id. The server-side hosted-agent session will never be deleted, leaking resources. Previously flagged and marked resolved, but the current diff still uses the wrong identifier.

---

Automated review by TaoChenOSU's agents

[github-actions]

Flagged issue

playbook.py:238 — SampleInfo(path=python_root / sample_path, ...) constructs the wrong absolute path because sample_path (from playbook.sample) is relative to samples_dir (python/samples/), not python_root (python/). This causes the snapshot/restore mechanism to silently target a non-existent directory, so replays that edit sample files leave the repository dirty. Previously flaged and marked resolved, but the current diff still contains the unfixed code.

---

Source: automated DevFlow PR review

[github-actions]

Flagged issue

using_deployed_agent.py:65 — session.session_id is a locally generated UUID, not the service-managed session ID stored in session.service_session_id. The server-side hosted-agent session will never be deleted, leaking resources. Previously flagged and marked resolved, but the current diff still uses the wrong identifier.

---

Source: automated DevFlow PR review

[eavanvalkenburg]

honestly, I'm not sure how I feel about using agents to do deterministic things, we should be able to validate the vast majority of samples using code, which will be faster and cheaper. Can we scope this to only samples that are more difficult to validate? And I also think we can think about validation more intelligently, for instance by first running static code checks to validate the code, then picking and choosing ones that are in paths that often break, like the most complex OAI Responses sample, run that, if good, no need to run all OAI Responses samples...

[TaoChenOSU]

honestly, I'm not sure how I feel about using agents to do deterministic things, we should be able to validate the vast majority of samples using code, which will be faster and cheaper. Can we scope this to only samples that are more difficult to validate? And I also think we can think about validation more intelligently, for instance by first running static code checks to validate the code, then picking and choosing ones that are in paths that often break, like the most complex OAI Responses sample, run that, if good, no need to run all OAI Responses samples...

I agree. This PR already excludes a bunch of samples from the workflow. The ask was to validate all the FHA samples, which by themselves are hard to validate using code (including running the samples locally and in Foundry). Let's see how well this works and how the requirements evolve. If this doesn't work well, we can scope this to only the FHA samples.