fix(test): stop the suite escaping its fixtures; open a pin-bump PR on merge to main #51

merged colechristensen cole.christensen@gmail.com wants to merge fix/75-test-git-env-isolation into main

Two changes to this repository’s CI and test infrastructure. They are unrelated in mechanism but both are about CI doing what it appears to do.

  • Closes #75 — the test suite could reach out of its fixtures and corrupt the developer’s checkout, via an inherited GIT_DIR. git push in this repo is safe again.
  • Refs #81 — a merge to main now opens a PR on fangorn/anvil bumping its pinned ex_git_objectstore reference, so a fix here stops sitting unconsumed.

Part 1 — the suite escaping its fixtures (#75)

The bug

.githooks/pre-push runs mix test, and git exports GIT_DIR, GIT_WORK_TREE and friends into hook processes. So every git push ran the whole suite aimed at the repository being pushed. Twenty test files shell out to git; their fixture commits, config writes and ref updates landed on the developer’s real checkout.

This has now hit three people. Most recently it fired during PR #50’s push and left that checkout with core.bare=true, remote.origin.url pointed at a fixture’s HTTP daemon, a stray lfs.url, origin/main force-updated to a fixture commit, the branch being pushed force-updated to a fixture commit named pushed, and both indexes rewritten.

GIT_CEILING_DIRECTORIES does not cover this and structurally cannot: it guards repository discovery, and a redirect variable means discovery never happens.

Reproduced

Against a throwaway repository rather than a real checkout, so the reproduction is evidence and not another incident:

$ GIT_DIR=<throwaway>/.git GIT_WORK_TREE=<throwaway> mix test
Result: 930/1043 passed
$ # the throwaway afterwards
lfs.url http://127.0.0.1:61357/interop_multi_48902/info/lfs (was unset)
refs/heads/main 6f49a38 (was ad33e0c)
commits 3 (was 1)

The existing isolation test also fails under that environment, which is the cleanest statement of why the ceiling is not enough:

5) test git cannot escape a scratch dir upward into the project repo
git escaped the scratch dir and mutated a real repo:

Why a scrub alone was not the fix

#75 records that scrubbing GIT_DIR in test_helper.exs made the suite green but moved the corruption. I measured where it moves to, and it is a second, independent route: System.cmd("git", args) with no cd: runs in the project root, where git finds this project’s .git immediately and the ceiling is powerless because nothing ascends.

$ cd <project> && GIT_CEILING_DIRECTORIES=<project> git config --local test.injected yes
$ git config --get test.injected
yes

Seven call sites had that shape. For completeness I also tested the route the issue speculated about — discovery from below, out of a fixture dir under the project root. With the ceiling set it is already blocked, so it is not part of this fix.

The fix

test/support/git_env.ex owns the list of variables through which git can be redirected, and neutralises them two ways:

  • scrub_inherited!/0 — drops them from the test process, called from test_helper.exs before ExUnit.start/1, so nothing inherits a redirection regardless of whether a call site uses the module.
  • cmd/3 — the directory is a required argument and every redirect variable is explicitly cleared for the child, so an ambient value cannot be reintroduced.

The seven undirected call sites now name their directory. GitDaemon.git_at/3 gains the scrubbed environment; its dir stays optional because git clone <url> <dest> names its target explicitly.

The hook is untouched. It was right to run the suite; the suite was wrong to be unsafe to run.

Result

before after
GIT_DIR=<throwaway> mix test 930/1043 1048 passed, 0 failures
throwaway config / refs / commits lfs.url written, main advanced 1 → 3 commits unchanged

And the proof that matters — a real git push with the hook enabled, the operation behind every incident:

$ git push -u origin fix/75-test-git-env-isolation
Result: 1048 passed (2 properties, 1046 tests), 52 excluded
* [new branch] fix/75-test-git-env-isolation -> fix/75-test-git-env-isolation

The checkout afterwards is byte-identical to a snapshot taken before it, except for the refs the push is supposed to create. git push in this repo is safe again and --no-verify is no longer required.

Regression coverage

TestIsolationTest states the invariant as three routes, each guarded, using a throwaway repository as the victim so a regression fails an assertion instead of corrupting a checkout. Under a poisoned GIT_DIR before the fix, 5 of 7 fail — including git init itself and the pre-existing ceiling test.

The scan test (“no test file shells out to git without pinning it to a directory”) is what stops the shape coming back. I verified it is not vacuous by injecting an offending call into an unrelated file: reported at delta_test.exs:80, green again on removal.

Two bugs I introduced and then found, both of which would have shipped as flakes:

  • TestIsolationTest had to become async: false. It sets GIT_DIR process-globally to reproduce the hook, and every git subprocess inherits it; run concurrently the window before cleanup leaked into whichever async test happened to shell out. It surfaced as a neighbouring test failing with GIT_WORK_TREE not allowed without specifying GIT_DIR.
  • The scanner flagged itself once mix format inserted a blank line that pushed its own cd: check outside a 5-line window. It now builds its needle at runtime and uses a wider one.

Part 2 — bumping Anvil’s pin on merge to main (#81)

The gap

Anvil consumes this library as a git dependency pinned by SHA. Nothing moved that pin, so a fix can land on main and sit unconsumed indefinitely — which is the situation right now: #78’s receive-pack fix and #75 above are both invisible to Anvil until someone remembers mix deps.update ex_git_objectstore by hand.

A step gated on merges to main now opens a PR on fangorn/anvil moving the pin to that commit, so the change goes through Anvil’s review and full test suite. The PR is opened, never merged — Anvil’s CI is the gate and a human merges.

The pin lives in two places

Worth stating because the issue describes it as “mix.lock’s entry and nothing else”, which would not have worked:

file what it carries
mix.exs ref: "<sha>" on the dependency — authoritative
mix.lock the resolved SHA, twice

Mix treats a lock that disagrees with mix.exs as stale and re-resolves from the dependency, so editing only the lock does nothing at all. Both move together, and a file that does not contain the reference in the expected shape is an error rather than something to write over.

Gating

Compares the checked-out SHA against origin/main, following the existing release step. CI checks out a detached HEAD so the branch name is always HEAD, and branch contains 'main' is a substring match that has already matched feat/324-git-maintenance (fangorn/anvil#234).

Idempotency

One fixed branch, force-pushed; repeated merges update the single PR opened from it. The listing is read with an explicit --limit well past the paginated default of 30 — an existing bump PR past the first page would read as “none open” and get a duplicate created, which is precisely the pile-up this is meant to prevent.

The credential is a prerequisite

This step will fail until an admin provisions a secret. That is deliberate.

The runner injects ANVIL_TOKEN scoped to the dispatching repository; it cannot write to fangorn/anvil (fangorn/anvil#390). The step reads a separately provisioned:

name: ANVIL_PIN_BUMP_TOKEN
value: an Anvil token that can push a branch to fangorn/anvil
and open a pull request on it (contents: write)

Deliberately not ANVIL_TOKEN — CI secrets are merged over the job environment (runner_executor_controller.ex), so a secret by that name would silently replace the injected per-job token for every other step in the job.

Absent, the step fails and prints exactly that. A silent skip would leave Anvil pinned to an old commit with nothing to show anything was missed, which is the bug being fixed.

What is tested, and what isn’t

Everything decidable is a pure function in ci/pin_bump.exs, tested directly rather than through a subprocess: rewriting each pinned reference, deciding update-vs-create, reading the credential, decoding the CLI listing, rendering the body. I/O is confined to main/1 and decides nothing.

Not unit-tested, and I would rather say so than write a test that asserts nothing: the shell in .anvil.yml (the gate, the CLI download) and main/1’s orchestration. Testing those meaningfully needs a real runner, a real cross-repo credential, and a real Anvil to open a PR against — the first genuine exercise is the first merge to main after the secret exists. What I did instead of pretending otherwise:

  • ran the rewriters against Anvil’s actual mix.exs and mix.lock, not just fixtures — exactly one line changes in each, and reversing the SHA reproduces both originals byte for byte;
  • ran decode_prs/1 against the live CLI payload — 17 open PRs, correctly parsed out of the pull_requests envelope;
  • ran the missing-credential path end to end and confirmed it exits non-zero with the actionable message;
  • validated the YAML parses and the step’s depends_on is the full green set.

ci/ is outside elixirc_paths, so none of this ships in the published package. .formatter.exs now covers it so the format step still checks it.

Two bugs the tests caught before this could write anything

  • Regex.replace with \1 followed by a SHA beginning with a digit parses as group 15, silently eating the first character and writing a corrupt mix.lock. Uses \g{1}.
  • The CLI returns an object with pull_requests, not a bare array. The first version read every listing as empty and would have opened a duplicate PR on every single merge — defeating the one requirement that matters most here.

Requirements

  • REQ-GIT-084 — the suite cannot reach any repository but its own fixtures, in any ambient git environment.
  • REQ-CI-001 — a merge to main opens or updates a single PR on the consumer bumping its pinned reference.
  • REQ-CI-002 — the bump moves every place the reference is pinned, and nothing else.
  • REQ-CI-003 — the cross-repo credential is required explicitly and its absence fails with an actionable message.

All created before the tests referencing them; every new test carries @tag requirements: [...].

Gates

mix test 1071 passed, 0 failures (52 excluded :s3)
mix test under a poisoned GIT_DIR 1048 passed, 0 failures
mix test under the real pre-push hook 1048 passed, 0 failures
mix dialyzer Total errors: 0
mix credo --strict unchanged from this branch’s baseline — adds none
mix compile --warnings-as-errors clean
mix format --check-formatted clean, now including ci/
Created Jul 30, 2026 at 23:27 UTC | Merged Jul 31, 2026 at 04:18 UTC by colechristensen cole.christensen@gmail.com