PR #2296 - petry-projects/.github-private

Problem

Test suites that drive dev-lead's real writer and handler functions read and write fixed files under /tmp. Any other bats session on the same machine (another worktree, another agent, a retry) uses the same files, so one session's value leaks into another's assertion. Tests 19, 24 and 30 of test_budget_breaker.bats failed intermittently on unmodified main for this reason.

The issue names two files and one suite. The survey for this change found more:

Six sidecar files, not two: the rate-limit reset, the failure reason, three timeout files, and the session log.
19 suites reach the writers. Eleven of them never name a path, so a text search does not find them; they were found by logging every default-path resolution during a full run.
Five suites use a fixed directory name under $BATS_TMPDIR, which is /tmp for every session.

This is part 1 of 2 and does not close the issue.

Design

One definition, in scripts/engine.sh: DEV_LEAD_SIDECAR_DIR="${DEV_LEAD_SIDECAR_DIR:-/tmp}". Every writer and reader names "$DEV_LEAD_SIDECAR_DIR/<file>", so the two sides cannot resolve different paths. The three handler scripts already source engine.sh first.
The 19 suites export DEV_LEAD_SIDECAR_DIR="$BATS_TEST_TMPDIR" in setup(). The five suites with fixed directories move to $BATS_TEST_TMPDIR.
A static check, scripts/check-fixed-tmp-paths.sh, lists every fixed path under /tmp named in tests/ and scripts/ and fails on one that is not accounted for. Exceptions are data (scripts/lib/fixed-tmp-paths.tsv: path, file, reason), and a row that no longer matches anything fails the check. It runs in the bats job through tests/test_check_fixed_tmp_paths.bats (16 tests, about 2 seconds).
No workflow reads these files in a run: block or a later step, so nothing outside the scripts needs the variable.

Deliberately left for part 2: the sixth sidecar, dev-lead-session-output.txt. Moving it touches lines that "fix(tests): make every negated bats assertion able to fail" (#2264) also rewrites in test_engine_writer.bats. This part has no hunk adjacent to that pull request and merges cleanly before or after it (checked with git merge-tree). The six exception rows for that file say "relocation pending" and part 2 deletes them.

Not fixed, and needs its own change: scripts/review-one-pr.sh keeps its working files in /tmp/cascade (57 fixed references). pr-review.yml uploads that directory and a prompt names it to the model, so a variable in the script alone would be honoured by one side only. It has an exception row marked "NOT isolated yet".

Risk

Production behaviour: with the variable unset, the 39 references resolve to the same 39 literal paths as on main, in order. No workflow sets it.
A new input to scripts that run with credentials. Each sidecar is read as data (cat, tr -d, tail through an HTML escape, a redaction filter); none is sourced or evaluated. Someone who controls the job environment can already write the /tmp files; the variable only lets them aim six fixed basenames at another directory. The name contains nothing that looks like a secret.
A new gate on main. A pull request that adds a fixed /tmp path and passed CI before this merged will fail the bats job after it. I ran the check on this branch merged with #2264, #2281 and #2293: clean. Other open pull requests were not checked.
A future suite that forgets the export silently uses /tmp; the static check reads text and cannot see that. Stated in the checker's header as a known limit.
The checker ran under GNU awk and BSD awk. Ubuntu's default mawk was not available locally; CI is the first run on it.

Test plan

Deterministic demonstration: a background loop wrote garbage to the default paths while the suites ran.

Suite	`main`	this branch
`test_budget_breaker`	34 ok, 5 failed	39 ok
`test_engine_fallback`	30 ok, 8 failed	38 ok
`test_engine_writer`	41 ok, 2 failed	43 ok

Four concurrent sessions, five rounds, 25 suites: identical results in every session, the only failures being two that also fail alone locally (grep -P on BSD grep, Python jsonschema missing).
Mutations: one writer put back on /tmp (5 tests red), one reader put back (11 red), a new fixed path added to two tests (the check reports it as shared). Restored to 99 of 99.
Re-run by me on this exact commit after rebasing: the new suite 16 of 16; test_budget_breaker 39, test_engine_fallback 38, test_engine_writer 43, test_rate_limit_samples 14, test_fix_issue 32, test_gemini_quota 44, all passing; shellcheck --severity=warning -x and actionlint clean; the checker exits 0.
Not run: CI; the secret scanner locally; e2e scenarios 06 and 08.

Rollback

Revert the squash commit. Paths return to literals and the gate disappears. Nothing persists between runs.

Monitoring

The bats job's duration (it has run close to its limit before): expect about 2 seconds more.
A bats failure on main in test_check_fixed_tmp_paths.bats after another pull request merges means that pull request added a fixed path; the failure names the file.

Refs #2271

🤖 Generated with Claude Code