Skip to content

fix(sdk/python): skip schema output dir for schema-free harness runs - #1072

Merged
santoshkumarradha merged 1 commit into
Agent-Field:mainfrom
remote-controlled-man:codex/agentfield-harness-readonly-20260924
Sep 25, 2026
Merged

santoshkumarradha merged 1 commit into
Agent-Field:mainfrom
remote-controlled-man:codex/agentfield-harness-readonly-20260924

Conversation

@remote-controlled-man

Copy link
Copy Markdown
Contributor

Summary

HarnessRunner.run() allocates the per-run .agentfield-out-* directory before dispatching the provider, even when schema is None. That directory only ever holds .agentfield_output.json (#684, #891), and every consumer of it is already inside an if schema is not None: guard — so a text-only run pays a filesystem write it never reads, and aborts during setup when the project root cannot accept one. A plain permission_mode="plan" call in a read-only checkout (immutable CI workspace, read-only mount) therefore fails before the coding agent is invoked at all. This allocates the directory only for schema-bearing runs, leaving their per-run isolation and cleanup exactly as they were.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Docs only
  • Tests only
  • CI / tooling
  • Breaking change

Test plan

  • cd sdk/python && .venv/Scripts/python -B -m pytest tests/test_harness_runner.py -q -o addopts='' — 21 passed, including the new regression and the existing per-run isolation, concurrency, and cleanup-on-error tests.
  • cd sdk/python && PYTHONUTF8=1 .venv/Scripts/python -B -m pytest tests -q -o addopts='' -k 'harness and not functional' -m 'not integration and not harness_live' — 308 passed, 2170 deselected, 0 failed, with no test files excluded.
  • cd sdk/python && .venv/Scripts/python -B -m mypy --config-file mypy.ini agentfield/ — Success: no issues found in 81 source files (the same command CI runs).
  • cd sdk/python && .venv/Scripts/python -B -m ruff check . — All checks passed, at the CI-pinned ruff==0.15.22.
  • cd sdk/python && .venv/Scripts/python -B -m ruff format --diff agentfield/harness/_runner.py tests/test_harness_runner.py — 2 files already formatted.
  • cd sdk/python && PYTHONUTF8=1 .venv/Scripts/python -B -m pytest --ignore=tests/test_agent_mesh.py — 2391 passed, 7 failed, 7 skipped, 38 deselected in 190s, using the project's default addopts.

The new test fails on the unmodified runner at _runner.py:302 in os.makedirs(base_dir, exist_ok=True), before the provider is dispatched, and passes after the fix. It uses a real filesystem boundary rather than a mock: project_dir points below a regular file, so creating it raises OSError on every platform.

Two things in that full run are pre-existing on this Windows machine, not caused by this PR, and both were confirmed by re-running them in a pristine main worktree: the 7 failures are all in tests/test_log_writer.py (event-loop-blocking assertions from the #1066 area — identical 7 failed / 14 passed on unmodified main), and tests/test_agent_mesh.py stalls after three tests when run as part of the suite (identical stall on unmodified main; its fourth test passes in 0.55s in isolation). Neither file references the harness runner. Linux CI is the authority for both.

Not run locally: the repo-wide coverage gate (./scripts/coverage-summary.sh) aggregates Go, TypeScript, and web-ui surfaces that need toolchains this machine does not have, and ./scripts/test-all.sh / full CI were not run. Tests were executed on Windows with Python 3.13.12 in an isolated uv sync --frozen --extra dev venv; PYTHONUTF8=1 is process-local and only affects how two packaging tests decode pyproject.toml under a non-UTF-8 host locale.

Test coverage

  • I ran tests for the surface(s) I changed locally.
  • New code paths are covered by tests in this PR (no bare additions).
  • The coverage gate check is green in CI before requesting review.

The change adds a test and removes no code, and agentfield.harness is not among the default --cov targets in pyproject.toml, so no coverage-baseline.json update should be needed.

Checklist

The commit message is conventional. The signing box stays unchecked until the published head is actually signed. No issue is linked: this is a small self-discovered bug fix, and the contributing guide asks for an issue before large changes. Say the word if you would rather have one filed first.

Related issues / PRs

#684 established that the schema output file must live inside the agent root, and #891 gave each run its own temporary directory so concurrent runs sharing a cwd cannot overwrite or delete each other's output. Both guarantees are preserved here — they now apply to the runs that actually produce schema output.

HarnessRunner.run() allocated the per-run .agentfield-out-* directory
before dispatching the provider even when schema is None. That directory
only ever holds .agentfield_output.json (Agent-Field#684, Agent-Field#891), and every consumer
of it is already inside an `if schema is not None` guard, so a text-only
run paid a filesystem write it never read back. When the project root
could not accept one — a read-only mount, an immutable CI workspace — a
plain permission_mode="plan" call failed during setup, before the coding
agent was invoked at all.

Allocate the directory only for schema-bearing runs. Their per-run
isolation and cleanup are unchanged, so concurrent runs sharing one cwd
still cannot overwrite or delete each other's output.

Refs Agent-Field#684, Agent-Field#891
Copilot AI lite review requested due to automatic review settings September 25, 2026 02:18

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown
Contributor

Performance

SDK Memory Δ Latency Δ Tests Status
Python 9.0 KB - 0.29 µs -17% ✓ ✓

✓ No regressions detected

@github-actions

Copy link
Copy Markdown
Contributor

📊 Coverage gate

Thresholds from .coverage-gate.toml: per-surface ≥ 84%, aggregate ≥ 85%, max per-surface regression ≤ 1.0 pp, max aggregate regression ≤ 0.50 pp.

Surface Current Baseline Δ
control-plane 87.90% 87.40% ↑ +0.50 pp 🟡
sdk-go 93.30% 92.00% ↑ +1.30 pp 🟢
sdk-python 94.72% 93.73% ↑ +0.99 pp 🟢
sdk-typescript 91.83% 90.42% ↑ +1.41 pp 🟢
web-ui 84.77% 84.79% ↓ -0.02 pp 🟡
aggregate 85.92% 85.75% ↑ +0.17 pp 🟡

✅ Gate passed

No surface regressed past the allowed threshold and the aggregate stayed above the floor.

@github-actions

Copy link
Copy Markdown
Contributor

📐 Patch coverage gate

Threshold: 80% on lines this PR touches vs origin/main (from .coverage-gate.toml:thresholds.min_patch).

Surface Touched lines Patch coverage Status
control-plane 0 — ➖ no changes
sdk-go 0 — ➖ no changes
sdk-python 0 — ➖ no changes
sdk-typescript 0 — ➖ no changes
web-ui 0 — ➖ no changes

✅ Patch gate passed

Every surface whose lines were touched by this PR has patch coverage at or above the threshold.

@santoshkumarradha santoshkumarradha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated the schema-free path and the existing schema-bearing harness flows locally. The focused tests, Ruff, compile checks, CLA, and CI are all green. This looks good to merge.

@santoshkumarradha
santoshkumarradha added this pull request to the merge queue Sep 25, 2026
Merged via the queue into Agent-Field:main with commit 8805c96 Sep 25, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants