ref:main

CI: reused workspaces make builds nondeterministic — same commit fails, then passes #50

open Opened by cole.christensen@gmail.com

Links

No links yet.

The same commit failed and then passed with no change to the tree. A build’s result depends on what the runner slot happened to contain, which makes both outcomes untrustworthy.

Evidence

run commit result
0d7f0b6584 72bf1986 failed — compile: ** (Mix) Unknown package mochiweb in lockfile
74dfb0ae50 72bf1986 passed — re-run, identical commit, no push in between

The branch (fangorn/fleet fix/dns-doubled-mail-hostname) changed only .anvil.yml, one new scripts/*.py, and a tofu/*.tf file. Its mix.lock is byte-identical to main’s, and main passed with that lock in run 0b7589960f. mochiweb is present in both locks. So nothing in the commit explains the failure.

Earlier instance, same class

Previously a run reported main @ 538c7a4d in its metadata while executing a test file that only existed on wip/tree-load-async. Same underlying shape: the workspace contents did not match the commit the run claimed to be building.

Mechanism

src/runner/workspace.rs reuses a slot per work_dir/<owner>/<repo>/<slot> and deliberately does not clean it:

// NOTE: no `git clean -fdx` — preserves build caches (deps, _build, node_modules, target/)

(workspace.rs:388, and the same rationale at :99 and :567)

That is a reasonable trade for build speed, but it means deps/ and _build/ can carry state from a different branch into the current build. A dependency tree left by a branch with different deps is enough to make mix reject a lock entry that is legitimately required on this branch — which matches the observed error exactly.

run_git_lenient (:434) also warns and continues when a git operation fails, so a failed fetch or checkout leaves a stale tree and the build proceeds against it rather than stopping.

Why it matters

Both failure directions are bad:

  • Spurious red blocks a merge and trains everyone to re-run CI instead of reading it — which then hides real failures.
  • Spurious green is worse: a pass may come from stale artifacts rather than the committed tree, so CI stops being evidence that the commit builds.

We hit this twice today, and the second time it blocked merging a fix for a live production DNS fault.

Possible directions

Not prescriptive — the caching is worth keeping, so the aim is invalidating it correctly rather than removing it:

  1. Record the ref/SHA a slot last built. When it changes, clear the dependency tree (deps/, _build/) while keeping the heavier caches (node_modules/, target/).
  2. Key cache reuse on a hash of the lock files (mix.lock, package-lock.json, Cargo.lock) so a lock change invalidates the derived state.
  3. After checkout, assert HEAD equals the requested SHA and fail loudly otherwise — this covers the wrong-tree instance above.
  4. Escalate run_git_lenient to checked for operations whose failure invalidates the workspace (fetch, checkout, reset); a warning is not enough when the consequence is building the wrong code.