Skip to content

fix(PG-956): the run-slot tests take a lock as the Mac does on CI's ubuntu-26.04 - #1076

Merged
nrjdalal merged 2 commits into
canaryfrom
PG-956-gauges-lock-flake
Nov 5, 2026
Merged

fix(PG-956): the run-slot tests take a lock as the Mac does on CI's ubuntu-26.04#1076
nrjdalal merged 2 commits into
canaryfrom
PG-956-gauges-lock-flake

Conversation

@nrjdalal

@nrjdalal nrjdalal commented Nov 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

TL;DR

  • 🧪 The run-slot lock tests stop failing now and then on GitHub's Linux runner. One such failure held up a ship.
  • 🔍 The cause was the runner's own mkdir command, which can tell two programs that each made the same folder. The tests now use the runner's other mkdir, which tells only one, as the Mac's does.

Summary

 tests/scripts/ops/run-slot-fake.ts, machine()
   slowChild("python3", bin)
+  if (GNU_MKDIR)             // Bun.which("gnumkdir"): ubuntu-26.04 only
+    bin/mkdir = exec gnumkdir "$@"
  • GitHub's ubuntu-latest is ubuntu-26.04, whose /usr/bin/mkdir is uutils coreutils 0.10.0 (rust-coreutils 0.10.0-1ubuntu2~26.04.1). Its create_dir checks path.exists(), then maps a failed mkdir(2) to success with Err(_) if path.is_dir() => Ok(()), even without -p. So when two processes race, both exit 0.
  • run-slot.sh takes every lock with a plain mkdir: the gauges refresh lock, the run slots and the model slots. On that runner, two looks can each hold the gauges lock and both run top: "eight looks with no reading yet run top once" got 6.
  • The image keeps GNU's mkdir as gnumkdir, and each fake machine's bin now runs that one wherever it exists. On the Mac and on GNU images nothing changes.
  • .agents/testing.md, Traps: one new trap, the racing mkdir and how a test avoids it.

Decisions:

Evidence

  • Cause, in an ubuntu:26.04 container: 8 racing mkdir L, 200 rounds each:

    mkdir rounds with more than one winner
    /usr/bin/mkdir (uutils 0.10.0) 33 of 200, and 38 of 200 on a second run
    gnumkdir (GNU 9.7) 0 of 200

  • No repro without uutils: the eight-look case ran exactly one top in 60 of 60 rounds on the Mac (BSD mkdir) and 80 of 80 on oven/bun:debian (GNU mkdir, bash 5.2).

  • Before / after in ubuntu:26.04 (--cpus 2, Linux bun 1.4.4):

    run canary's fake this head's fake
    probe: 60 rounds of 8 looks with no reading, rounds that ran top more than once 10 of 60 (2 or 3 tops) 0 of 60
    run-slot-gauges-lock.test.ts, 20 runs, the two "run top once" cases failed in 6 of 20: the stale-reading case (line 46) in runs 1, 14 and 15, the no-reading case (line 52) in runs 5, 7 and 13 failed in 0 of 20

  • At this head on the Mac: 20 runs in a row of bun run test:file tests/scripts/ops/run-slot-gauges-lock.test.ts: 20 of 20 passed, each 10 pass, 0 fail. bun run check-types:tests: no errors. bun run test:skills: all 13 passed.

  • Sightings on CI:

  • doc-sync: the change touches the test fake alone. git grep -n -iE "mkdir" over .agents, scripts/README.md, scripts/ops/README.md, scripts/hooks/README.md, docs and the core skills finds no line that calls mkdir atomic. The new trap in .agents/testing.md is the one place that says it is not on ubuntu-26.04.

Merge Danger

Door: two-way

A revert brings back the uutils mkdir in the fake, and the flake with it.

Blast Radius: small

Tests only: every run-slot*.test.ts machine on a runner that has gnumkdir. run-slot.sh and every other script are unchanged.

Links

🤖 Generated with Claude Code

@nrjdalal
fix(PG-956): the run-slot tests take a lock as the Mac does on CI's u…
1ba3be4 
…buntu-26.04

The runner's default mkdir is uutils 0.10.0, which reports success when
another process made the folder between its check and its own mkdir, so
two looks could each hold run-slot.sh's gauges lock and both run top
(expected 1, received 6). The fake machine's bin now runs GNU's mkdir,
which the image keeps as gnumkdir, wherever it exists; run-slot.sh itself
runs on the Mac, whose mkdir is atomic. uutils fixed it in 0.11.0, and
.agents/testing.md's Traps names the case.
@nrjdalal
Merge remote-tracking branch 'origin/canary' into PG-956-gauges-lock-…
0005bc9 
…flake

# Conflicts:
#	.agents/testing.md
@nrjdalal

nrjdalal commented Nov 5, 2026

Copy link
Copy Markdown
Owner Author

ReviewBot · round 1: 5/5 at 0005bc9 · 0 blocking, 0 should-fix, 3 nits

On GitHub's ubuntu-26.04 runner, mkdir is uutils 0.10.0, which can tell two racing callers that each made the folder. Each fake machine now runs GNU's gnumkdir as mkdir wherever the image has it, so run-slot.sh's mkdir locks get one holder there, as on the Mac. run-slot.sh itself is unchanged.

Spec (the right thing)

  • The cause is shown by a run: delivered, measured again here in ubuntu:26.04 under --cpus 2 (mkdir (uutils coreutils) 0.10.0, /usr/bin/gnumkdir GNU 9.7).
    • 8 racing mkdirs gave more than one winner in 24 of 200 rounds with uutils, and in 0 of 200 with gnumkdir.
    • The body's numbers, 33 and 38 of 200, are other runs of the same race.
  • The fix holds: delivered, measured on run-slot-gauges-lock.test.ts in the same container, 20 runs each.
    • Canary's fake failed in 1 of 20, run 10, where both "run top once" cases failed.
    • This head's fake failed in 0 of 20.
    • The fix removes the cause rather than the speed dependence, which meets the line's intent: the test no longer fails on a slow runner.
  • 20 runs in a row: delivered for the case the issue names.
    • The head ran the file 20 of 20 green here, the dead-lock case included.
    • The body's ubuntu runs show the two "run top once" cases at 0 of 20.
  • The change sits in run-slot-fake.ts, with nothing changed on the Mac or a GNU image: delivered.
    • The shim is written only when Bun.which("gnumkdir") finds one.
    • Every lock in run-slot.sh is a bare mkdir (run-slot.sh:1265, :1267, :1309, :1352, :1445), with no PATH= reset and no absolute path.
    • The fake's env() puts bin first on PATH (run-slot-fake.ts:147), so the shim reaches every lock.
  • One trap in .agents/testing.md: delivered, at lines 68-69, in Traps.
  • Evidence holds the red, the green and the Mac's 20 runs: delivered.
  • The dead-lock takeover case left out: the call is sound.

Nothing unasked.

Standards (built well)

No blocking or should-fix finding. Nits, each yours to take or leave:

  • [nit] run-slot-fake.ts:33 and .agents/testing.md:68 explain the uutils race in full twice. The comment could name the trap in .agents/testing.md and keep only what the shim does, so the explanation lives in one place.
  • [nit] .agents/testing.md:68 gives "38 of 200 rounds", one run's figure. Runs here and in the body gave 24, 33 and 38 of 200. A range, or the sentence with no count, would stay true.
  • [nit] In the body's Evidence, the Mac's 20 green runs show that the head keeps the Mac green, not that it fixes the flake, since canary's fake passes there too. One clause saying so would keep a reader from counting them as the green.

Threads

None opened, none open.

Checked

  • First round: the PR's whole history read. It holds no comments, reviews or threads.
  • Read in full: 1ba3be4 and the merge 0005bc9, against the merge base 6b8a5ee.
    • The merge's one conflict, .agents/testing.md, was resolved by keeping both sides (show --remerge-diff).
    • merge-tree names only that file, so the merge holds no edit outside its conflicts.
  • code-review from 6b8a5ee, with Standards and Spec read fresh and the spec's lines in the Spec prompt.
    • Spec found every line delivered, and Done-when 3 partial only if the dead-lock case counts. Its reasons are above.
    • Standards found no breach. Its nits are above.
  • writing-for-agents read on .agents/testing.md:68-69: positive phrasing, in Traps beside the other runner traps.
  • Changed-term grep:
    • git grep -n -iE 'mkdir' over .agents, scripts/README.md, scripts/ops/README.md and the skills finds no line that calls mkdir atomic on Linux.
    • Shell mkdir locks exist only in run-slot.sh. pr-merge's are Node fs.mkdir, which uutils does not touch.
    • No list grep applies, since the diff extends no enumerated kind.
  • check-types: green on the local merge with canary 756e588 (env NODE_ENV=production SKIP_ENV_VALIDATION=true bun run check-types).
  • touched tests on a merge with canary 756e588: 8166 pass, 0 fail (128 files, from state.py touched-tests).
  • git merge-tree --write-tree --name-only 0005bc9 origin/canary: clean.
  • The head's checks, read just before posting: Audit, Lint, and Build and test green at 0005bc9.

On this 5/5 candidate, the fresh full read found nothing blocking or should-fix. Its nits are listed above for you to take or leave.

Not verified

  • The uutils source and its 0.11.0 fix: the claim about how it reports success rests on the race measured above, not on a read of the code.
  • A run on GitHub's own runner image. The container is ubuntu:26.04 from Docker Hub, with the same uutils version.

ReviewBot · round 1. Push back on any finding you think is wrong.

@nrjdalal
nrjdalal marked this pull request as ready for review November 5, 2026 23:49
@nrjdalal
nrjdalal merged commit 87d60fd into canary Nov 5, 2026
20 checks passed
@nrjdalal

nrjdalal commented Nov 5, 2026

Copy link
Copy Markdown
Owner Author

Merged by bun run pr:merge at 0005bc9.

  • open: open
  • author: by nrjdalal
  • held: not in held-prs.json
  • ready: ready for review
  • base: into canary
  • up to date: canary has 8 commits 0005bc9 lacks; it merges clean with canary 25501e1
  • keeps canary: no merge of canary into the PR dropped a line git's own merge keeps
  • ci: 20 checks passed
  • body: the PR template
  • review: ## ReviewBot · round 1: 5/5 at 0005bc9 · 0 blocking, 0 should-fix, 3 nits
  • types: the merged tree aa4421e (fix(PG-956): the run-slot tests take a lock as the Mac does on CI's ubuntu-26.04 #1076 at 0005bc9 onto canary 25501e1) type-checks: check-types 1s, build 0s, metrics 0s, check-types:tests 4s

@nrjdalal
nrjdalal deleted the PG-956-gauges-lock-flake branch November 5, 2026 23:50
This was referenced Nov 5, 2026
Merged
Merged
Merged
nrjdalal added a commit that referenced this pull request Nov 6, 2026
@nrjdalal
fix(PG-1077): a look counts run-slot's slots 2 to 4 on one gauges rea…
c88ae1b 
…ding (#1089)

## TL;DR

- 🎛️ When run-slot counts how many runs the machine can take, it now uses one CPU reading for the whole count. Before, a new reading that arrived in the middle could give a count that matched neither the old reading nor the new one.
- 🧪 This fixes a test that failed now and then on GitHub's Linux runner. When that test fails now, it shows what each check printed.

## Summary

```diff
 capacity()                                   # slots, room and every take read it
+  local LANES=unread
   for s in run-slot run-slot-2 run-slot-3 run-slot-4
     slot_open "$s"
-      ! lanes_open                           # slots 2, 3 and 4: each read the gauges again
+      ! lanes_now                            # the first check reads them, the rest share its verdict
+lanes_now()
+  open|shut: the count's verdict; unread: lanes_open once and keep it; unset: lanes_open, as before
```

- `run-slot.sh slots` prints `capacity() / 2`. Each of slots 2 to 4 called `lanes_open`, and each `lanes_open` read `gauges()` again. In the dead-lock test the stale reading says 30% idle (shut) and the new one 80% (open). A look whose checks straddled the new reading counted slot 2 shut and slots 3 and 4 open, and printed `3`, or `2` when it landed between slots 3 and 4.
- The fix is a `local` in `capacity()`, which bash's dynamic scope hands to `slot_open`'s checks. Outside a count, `LANES` is unset and every check runs `lanes_open` as before, so a waiter still reads the gauges on every poll.
- `scripts/ops/README.md`, beside the takeover line: one line on the single verdict.

**Decisions:**
- **The fix is in run-slot.sh, not the test.** A real look on the Mac can count a mixed reading too: the bug isn't specific to Linux, only easier to hit on a 2-CPU runner.
- **The take loop is unchanged.** It calls `slot_open` slot by slot to take the first open one, so a reading that lands in the middle only changes which open slot it takes. Its counts come from `capacity()`, which now reads once.
- **The dead-lock test now fails on the list of bad looks, not on a boolean.** `expect(looks.filter(neither 1 nor 4)).toEqual([])` prints each bad look's slot line and stderr, so a CI failure says what a look printed. It stays whatever the cause turns out to be.

## Evidence

- **The new test**, "a look counts slots 2 to 4 on one reading, even when a new one lands between their checks". It sets a stale 30% reading, a lock a live refresher holds, and a fake `stat`. `stat` runs on the lock only in `gauges_lock`, once its mkdir finds the lock held, so the fake writes a fresh 80% reading after the look's first lanes check has read the stale one, as a refresher's rename would.

  | | canary's `run-slot.sh` | this head |
  | --- | --- | --- |
  | Mac | 10 pass, 1 fail: expected `["1"]`, received `["3"]` | passes |
  | `ubuntu:26.04` container | 10 pass, 1 fail: received `["3"]` | passes |

- **20 runs in a row** of `bun run test:file tests/scripts/ops/run-slot-gauges-lock.test.ts` on the Mac: 20 of 20, each 11 pass, 0 fail. Of the same file in `ubuntu:26.04` (`--cpus 2 --memory=1g`, Linux bun 1.4.4): 20 of 20, each 11 pass, 0 fail.
- **Every run-slot test** (`tests/scripts/ops/run-slot*.test.ts`) on the Mac: 424 pass, 0 fail. `bun run check-types:tests`: no errors. `bun run test:skills`: all 13 passed.
- **The first look, which caught nothing:**
  - the dead-lock case alone in `ubuntu:26.04` went 0 bad rounds in 60 with GNU's mkdir, and 0 in 60 with all 104 `gnu*` tools on PATH;
  - 60 runs of the whole file with a `bash -x` trace per look went 60 of 60 green.
  So it isn't a uutils tool, and a run can't be relied on to catch it. The deterministic test above is the evidence.
- **Sightings on CI:** #959 at 3006e0a, Suite (5), job 127180040487, ubuntu-26.04: the dead-lock case's `slots.every(1 or 4)` received false. In `ubuntu:26.04` at #1076's head it failed 1 of 20 runs.
- **doc-sync:**
  - `git grep -n -E "lanes_open|capacity\(\)|run-slot.sh slots|one reading"` over `.agents`, `docs`, the READMEs, `AGENTS.md` and the skills finds `scripts/ops/README.md:22, 26, 41`;
  - lines 22 and 41 name `lanes_open` for what it opens, which is unchanged;
  - the new line sits after line 28's takeover line.

## Merge Danger

**Door:** two-way

A revert brings back the per-slot readings and the flake.

**Blast Radius:** small

`run-slot.sh`'s slot count only. A count reads the gauges once instead of up to three times, and the verdict is the same whenever no reading lands in the middle. Every caller outside `capacity()` is unchanged.

## Links

- Closes #1077
- #956 and #1076: the mkdir race in the same file, fixed there
- #959: the CI sighting

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Review • 5/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

run-slot gauges lock test flakes: eight looks with no reading run top 6 times

1 participant

@nrjdalal