Skip to content

review-one-pr.sh keeps its working files in the fixed directory /tmp/cascade, so concurrent test sessions (and concurrent reviews on one machine) share it #2302

Open

Description

@don-petry

Problem

scripts/review-one-pr.sh reads and writes its working files under /tmp/cascade, by 57 fixed references. Every test that runs the script writes that directory for real, so two bats sessions on one machine (two worktrees, two agents, a retry) share it, in the same way the dev-lead sidecar files did before "fix(tests): give the dev-lead sidecars a per-test directory, and gate fixed /tmp paths" (#2296). Split out of "Test suites share fixed files under /tmp" (#2271), where it was found.

It was not fixed there because a variable in the script alone would be honoured by one side only:

  • .github/workflows/pr-review.yml uploads /tmp/cascade/** as the run's artifact;
  • prompts/cascade-action.md names the directory to the model;
  • the script itself.

The static check from #2296 (scripts/check-fixed-tmp-paths.sh) carries an exception row for it in scripts/lib/fixed-tmp-paths.tsv, marked "NOT isolated yet".

Scope

  1. One definition of the directory (an overridable default, as DEV_LEAD_SIDECAR_DIR is for the dev-lead sidecars) that the script, the workflow's artifact upload and the prompt all take their path from.
  2. The suites that run review-one-pr.sh point it at $BATS_TEST_TMPDIR. Which suites those are has not been traced; log every default-path resolution during a full run to find the ones that never name the path.
  3. Remove the exception row; the static check then fails if a literal comes back.

Acceptance criteria

  1. With the variable unset, the script, the artifact upload and the prompt resolve to /tmp/cascade, byte for byte as today.
  2. The affected suites pass while a background loop writes garbage into the default directory, and fail the same way on main first (to show the test is real).
  3. scripts/lib/fixed-tmp-paths.tsv has no row for /tmp/cascade.
  4. pr-review.yml is a channel-released reusable: the pull request states which tags must move.

Activity

  1. github-actions commented on Oct 11, 2026

    @github-actions
    Contributor

    Thanks for the thorough write-up and for splitting this out of #2271, the breakdown of what has to move together (script, artifact upload, prompt) is really helpful. One clarifying question to pin down the failure: do you have a concrete reproduction of the collision, for example two concurrent bats runs where one run's files in /tmp/cascade changed the other's result? If so, please say what you expected and what you saw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugBug reports

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions