elastic / elastic/ai-github-actions
[autonomy-atomicity] Isolate ready-to-make-pr self-review state per test run
- Dominant language
- Python
- Stars
- 11
- Forks
- 16
- Avg merge
- 22h 9m
- Merged PRs (30d)
- 31
Description
Recommendation: create_issue
Category: Global mutable state / implicit ordering dependency in tests and safe-input scripts
Files/lines:
- `.github/workflows/gh-aw-fragments/safe-output-push-to-pr.md:17-18` reads PR context from the fixed path `/tmp/pr-context/pr.json`.
- `.github/workflows/gh-aw-fragments/safe-output-push-to-pr.md:77-88` writes and copies all self-review artifacts to the fixed directory `/tmp/self-review`.
- `.github/workflows/gh-aw-fragments/safe-output-create-pr.md:62-68` writes diff/stat/commit artifacts to the same fixed `/tmp/self-review` directory.
- `.github/workflows/gh-aw-fragments/safe-output-create-pr.md:97-99` reads review instructions from fixed `/tmp/pr-context/review-instructions.md`.
- `tests/test_safe_input_ready_to_make_pr.py:183-186`, `206`, `215`, `268-287`, `301-315` assert against those fixed `/tmp/self-review` files.
- `tests/test_safe_input_ready_to_make_pr.py:424-435` writes and deletes a fixed `/tmp/pr-context/pr.json`; `446-449` only cleans it after each push-guard test.
Evidence:
The extracted safe-input Python is executed in many independent `tmp_path` git repositories, but every case shares the same process-external artifact locations. The test suite currently runs serially via `.github/workflows/ci-tests.yml:24-25` (`uv run --extra test pytest tests/ -v`) and `Makefile:274-275`, which masks the coupling. Any future `pytest -n`, sharding, or overlapping invocation of the ready-to-make-pr/ready-to-push-to-pr checks in one job can race or read stale artifacts from another case/run.
Problem:
These tests are not atomic: repository fixtures are isolated, but the behavior under test is coupled through global `/tmp/self-review` and `/tmp/pr-context` state. That makes individual cases order-dependent and blocks safe test parallelization/sharding. It also makes regressions harder to localize because a failing test can leave shared files that affect a later test.
Suggested fix:
Parameterize the artifact root used by the safe-input snippets, for example `SELF_REVIEW_DIR = os.environ.get('SELF_REVIEW_DIR', '/tmp/self-review')` and `PR_CONTEXT_DIR = os.environ.get('PR_CONTEXT_DIR', '/tmp/pr-context')`. In tests, pass per-test directories under `tmp_path` via `subprocess.run(..., env={...})` and assert against those paths instead of hard-coded `/tmp/...`. Add an autouse fixture that cleans or allocates these directories for every test. Keep the production default paths unchanged for compatibility.
Duplicate check notes:
This is not the known MkDocs hook global temp-dir state (#784); it is a separate safe-input/self-review fixture coupling. It is also not the Claude PR review queue scoping issue (#1374) or the ready-to-make-pr suite duplication (#491); this finding is specifically about fixed `/tmp/self-review` and `/tmp/pr-context` state preventing atomic/parallel tests.
---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Trigger Autonomy Atomicity Analyzer](https://github.com/elastic/ai-github-actions/actions/runs/28186364821)
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
Contributor guide
Research direction
Start with the listed safe-output fragments and tests/test_safe_input_ready_to_make_pr.py, tracing every use of /tmp/self-review and /tmp/pr-context. Run the focused test file before and after isolating artifact paths under each test's tmp_path. Done means tests no longer share these artifacts, cleanup is per test, production defaults remain compatible, and parallel or sharded runs do not race.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- github-actions, python
- Domain
- ci-cd, testing
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100