fix: silent-PASS gate, duplicate context writers, verify-merge entry selection

.github/workflows/ci.yml

@@ -3,11 +3,16 @@ name: CI
 # `merge_group` is mandatory. Without it, a check required by the merge queue
 # never runs for a queue entry and the queue stalls until
 # check_response_timeout_minutes expires.
+# No `push: [main]`. The queue fast-forwards main to the queue head verbatim, so
+# a push build re-runs on a SHA the merge_group run already validated — and
+# rewrites the same required `ci` context on it, several minutes later. Observed
+# on b23f79d: the queue's success at 03:04:37 was replaced by a pending, then a
+# second success at 03:08:39. That is the "one context, two writers" defect from
+# scenario 6 in a second form, and it happens on every merge. It also inflated
+# the per-PR cost by a third.
 on:
   pull_request:
   merge_group:
-  push:
-    branches: [main]
 
 # Collapse redundant PR runs, but never cancel a merge_group run: a cancelled
 # required check reads as a failure to the queue and evicts the PR.
@@ -19,7 +24,11 @@ jobs:
   ci:
     name: ci
     runs-on: ubuntu-latest
-    timeout-minutes: 20
+    # Must stay BELOW the ruleset's check_response_timeout_minutes (15), or a
+    # slow run gets evicted by the queue while Actions is still going and the
+    # eviction is recorded as a check failure — which would confound scenario 5,
+    # whose entire purpose is measuring that timeout.
+    timeout-minutes: 12
     steps:
       - uses: actions/checkout@v4
         with:

.gitignore

@@ -2,3 +2,5 @@
 # after an entry merges, so they have to be recorded locally for
 # scripts/verify-merge.sh to compare against afterwards.
 .mq-queue-log
+.mq-watch.pid
+ci/config.env.tmp

README.md

@@ -67,6 +67,26 @@ Watch any of them with:
 scripts/watch-queue.sh        # queue refs, per-SHA contexts, PR auto-merge state
 ```
 
+## Teardown — required, not optional
+
+Every scenario needs these two steps. Skipping them is what caused all the
+self-inflicted damage in `docs/results.md`:
+
+```bash
+scripts/watch-queue.sh stop                    # or it polls gh for hours
+gh pr close <n> --comment 'scenario fixture'   # for every PR carrying an override
+```
+
+**Close override PRs; never merge them.** Under a merge queue enqueueing *is*
+merging, so there is no "queue it but don't land it" — a scenario PR that carries
+a `ci/config.env` override makes that override the repo default the moment it
+goes green. `scripts/queue-prs.sh` now skips such PRs unless you name them
+explicitly, but closing them is the actual discipline.
+
+Do **not** put `watch-queue.sh` at the end of a chained command block: it is the
+last element, so the block never returns and the watcher is never reaped. Run it
+in its own shell, or accept the 45-minute budget it now self-imposes.
+
 `watch-queue.sh` also records every queue head it sees to `.mq-queue-log`
 (gitignored) and fetches the objects, because GitHub deletes those refs seconds
 after an entry merges — without that, scenario 8's tree comparison is impossible.

ci/check.sh

@@ -24,6 +24,26 @@ SLEEP_SECONDS="${_env_sleep:-${SLEEP_SECONDS:-60}}"
 FORCE_FAIL="${_env_force:-${FORCE_FAIL:-0}}"
 QUEUE_ONLY_FAIL="${_env_qonly:-${QUEUE_ONLY_FAIL:-0}}"
 
+# Validate the knobs before anything uses them. `[ "$n" -gt "$m" ]` returns 2 on
+# a non-integer operand, and `if` treats 2 as false — so a stray character
+# (CRLF checkout, a typo, a sed-mangled value) SILENTLY SKIPS the item-count
+# gate and the script prints PASS. Same for the sleep. `set -e` does not help:
+# it is explicitly exempt inside an `if` condition. Fail loudly instead.
+case "${MAX_ITEMS}${SLEEP_SECONDS}" in
+  ''|*[!0-9]*)
+    echo "FATAL: non-integer knob (MAX_ITEMS='${MAX_ITEMS}' SLEEP_SECONDS='${SLEEP_SECONDS}')" >&2
+    echo "       refusing to run: a bad value would skip the gate and report PASS" >&2
+    exit 2 ;;
+esac
+for _b in "FORCE_FAIL=$FORCE_FAIL" "QUEUE_ONLY_FAIL=$QUEUE_ONLY_FAIL"; do
+  case "${_b#*=}" in
+    0|1) ;;
+    *) echo "FATAL: ${_b%%=*} must be 0 or 1, got '${_b#*=}'" >&2
+       echo "       (FORCE_FAIL=true would run entirely green and read as 'no eviction')" >&2
+       exit 2 ;;
+  esac
+done
+
 # --- context detection -------------------------------------------------------
 # Actions sets GITHUB_*; Jenkins multibranch sets BRANCH_NAME / GIT_COMMIT.
 event="${GITHUB_EVENT_NAME:-}"
@@ -52,10 +72,19 @@ esac
 # Jenkins PR job uses "merging the PR with the current target branch revision"),
 # so a PR run must show 2 parents. One parent on a PR run would mean the branch
 # is being tested in isolation and the queue gate is worthless.
-parents="$(git rev-list --parents -1 HEAD 2>/dev/null | cut -d' ' -f2- || echo '')"
+# `cut -s` so a parentless line yields empty instead of echoing HEAD back as if
+# it were its own parent (root commit, or a shallow graft).
+parents="$(git rev-list --parents -1 HEAD 2>/dev/null | cut -s -d' ' -f2- || echo '')"
 nparents="$(printf '%s' "$parents" | wc -w | tr -d ' ')"
 merge_build=no
 [ "$nparents" -ge 2 ] && merge_build=yes
+# A shallow clone truncates HEAD's parents, so a genuine merge commit reports
+# one parent. Report "unknown", not "no": "no" asserts the CI is testing the
+# branch in isolation, and a false alarm there invites someone to "fix" the PR
+# discovery strategy and destroy the real gate.
+if [ "$(git rev-parse --is-shallow-repository 2>/dev/null || echo false)" = "true" ]; then
+  merge_build=unknown
+fi
 
 echo "=============================================="
 echo " ci/check.sh"

ci/config.env

@@ -10,20 +10,21 @@
 # Keep this comfortably above the item count on main, or every item PR fails
 # merged-with-target for reasons unrelated to the scenario under test.
 #
-# Beware: an override passed to scripts/mk-pr.sh lands on main with the PR. That
-# is how main ended up at MAX_ITEMS=4 with exactly 4 items after scenario 7 — the
-# next item PR would have failed at 5. To replay a semantic-conflict scenario,
-# use a per-branch override on PRs you intend to CLOSE, not merge.
+# Beware: an override passed to scripts/mk-pr.sh lands on main with the PR,
+# because under a merge queue enqueueing IS merging. Scenario 7 passed
+# MAX_ITEMS=3 and patched to 4, and merging that silently LOWERED the default
+# from 8 — reverting the deliberate raise made 3h earlier in PR #8. Nothing
+# announced it. scripts/queue-prs.sh now skips PRs that touch this file.
 MAX_ITEMS=12
 
 # Padding so several PRs can be queued while the first is still building.
 # Set to 0 for local runs.
 #
-# Keep the committed default SHORT. Scenario 2 raised it to 180 on three PR
-# branches; all three merged, so 180 silently became the repo default and every
-# later run — PR, queue entry and push alike — paid three minutes. Raise it
-# per-branch for a scenario, never here.
-SLEEP_SECONDS=180
+# Keep the committed default SHORT. This leaked to 180 THREE times: scenario 2
+# (PR #14), then again from scenario 3 (PR #18) less than four hours after the
+# fix that restored it. Both times every later run — PR, queue entry and push
+# alike — paid three minutes. Raise it per-branch for a scenario, never here.
+SLEEP_SECONDS=60
 
 # 1 = always fail, on PR and in the queue.
 FORCE_FAIL=0

docs/results.md

@@ -78,9 +78,10 @@ each queue ref is the *previous entry's head*, not `main`:
 | `pr-15-192ec44…` | `192ec44` | **pr-14's queue head** |
 | `pr-16-407b5d6…` | `407b5d6` | **pr-15's queue head** |
 
-So entry 3 was validated against a tree containing PRs 1 and 2 — the queue did
-not serialize and re-base one at a time, which is what scenario 4 saw when the
-PRs were too far apart in time to overlap.
+So entry 3 was validated against a tree containing PRs 1 and 2. Note this is not
+special to `SLEEP_SECONDS=180`: scenario 4 speculated at 60s too (that entry's
+claim to the contrary has been retracted). The long check made the stacking
+*easy to observe*, nothing more.
 
 **Timings** (all three enqueued within 32s):
 
@@ -91,9 +92,14 @@ PRs were too far apart in time to overlap.
 | #16 | 22:41:55 | 22:46:01 | 4m06s |
 
 #14 and #15 merged in the **same second** — the queue merged a batch, not a
-sequence. Three PRs landed in 4m38s wall-clock against a 180s check; serialized
-they would have taken roughly 3 × (180s + 2min wait) ≈ 15 min. That ratio is the
-argument for speculation on a busy repo.
+sequence. Three PRs landed in **4m38s** wall-clock against a 180s check.
+
+Serialized comparison, measured rather than modelled: a lone entry with a 180s
+check took **~4m15s** end to end (juliet's second entry, born ~03:00:52, merged
+03:05:08), so three of them ≈ **12m45s** → **~2.75× speedup**. Do *not* model it
+as `3 × (check + min_entries_to_merge_wait_minutes)`: scenario 1 shows a 60s
+check plus a 2-minute wait producing 2m09s total, i.e. the wait overlaps the
+check rather than adding to it.
 
 **Resulting history** is a clean first-parent spine of merge commits, each PR's
 own commit hanging off it:
@@ -126,15 +132,49 @@ by having `mk-pr.sh` print the pre-override values and a restore reminder;
 the real rule is **close scenario PRs rather than merging them** when they carry
 overrides.
 
-### 3. Mid-queue eviction — _pending_
+### 3. Mid-queue eviction — **PASS** (2026-08-06, merge-commit era)
+
+PRs #18 `hotel`, #19 `india` (`QUEUE_ONLY_FAIL=1`), #20 `juliet`, all at
+`SLEEP_SECONDS=180`, enqueued together.
+
+**India was green as a PR and red only in the queue** — exactly the split
+`QUEUE_ONLY_FAIL` exists to create:
+
+```
+Public CI  pass   .../job/PR-19/1/
+ci         pass   3m3s
+```
 
-PR2 with `QUEUE_ONLY_FAIL=1`; enqueue PR1, PR2, PR3.
-Expected: PR2 green as a PR, red in the queue, evicted; PR1 merges; PR3 rebuilt
-on a fresh entry without PR2.
+…yet it never merged. Timeline: `added_to_merge_queue` 02:56:39 →
+`removed_from_merge_queue` **03:00:51**, no `merged` event. **Eviction latency
+4m12s**, i.e. the 180s check plus queue overhead — the queue does not wait for
+anything else once a required check reports failure.
 
-- PR2 PR-level result / queue result:
-- Did PR3 get a new queue branch:
-- Eviction latency:
+**The rebuild is the finding.** Queue heads recorded by `watch-queue.sh`:
+
+| entry | base | contains |
+|---|---|---|
+| `pr-18-14e500a` | `main` | hotel |
+| `pr-19-6d3f547` | pr-18's head | hotel + india |
+| `pr-20-88c9660` | pr-19's head | hotel + india + juliet |
+| **`pr-20-6d3f547`** | **pr-18's head** | **hotel + juliet — india dropped** |
+
+Juliet's first entry was speculative on top of india. When india failed, that
+entry was discarded and juliet was **re-entered 7 seconds later** (03:00:58)
+against hotel alone, then merged as `b23f79d`. So a mid-queue failure costs the
+PRs behind it a rebuild, not a rejection — they are never blamed for a neighbour's
+breakage, and `main` never sees india's tree.
+
+**Cost note for production sizing:** juliet was built twice — once speculatively
+with india, once without. That is the price of speculation, and it scales with
+how often entries fail. On a repo where a queue build costs an hour rather than
+three minutes, `max_entries_to_build: 5` means up to five concurrent builds and a
+re-run of everything behind any failure.
+
+**Keep #19 open or close it — do not merge it.** Its branch carries
+`QUEUE_ONLY_FAIL=1`; merging would make queue-only failure the repo default and
+break every subsequent queue entry. Same class of leak as scenario 2's
+`SLEEP_SECONDS`.
 
 ### 4. Semantic conflict — **PASS** (2026-08-05, squash-era)
 
@@ -150,13 +190,20 @@ one file, enqueued back to back.
 - **Would PR-level CI have caught it? No.** Git saw no conflict either — the two
   PRs touch different files. Only the queue caught it. This is the value prop,
   demonstrated end to end.
-- Aftermath, worth knowing: `bravo` is now permanently unmergeable (`main` is at
-  the 2-item limit), so its author has to rebase and deal with it. The queue
-  converts a latent broken-`main` into a stuck PR — which is the trade.
-- Note these two did **not** co-batch: `pr-5`'s entry was based on `main` *after*
-  `alpha` merged, i.e. the queue serialized them rather than speculating. With
-  `min_entries_to_merge: 1` and a 60s check, `alpha` merged before `bravo` was
-  enqueued. Scenario 2 needs `SLEEP_SECONDS=180` to force real overlap.
+- Aftermath: `bravo` was unmergeable *until the limit moved* — `MAX_ITEMS` went
+  to 8 at 18:51 the same evening, and it was simply closed at 22:37. The queue
+  converts a latent broken-`main` into a stuck PR, which is the trade; "stuck"
+  lasts only as long as the constraint does.
+- **These two DID co-batch** — an earlier version of this entry claimed the
+  opposite and it was wrong, with consequences. `bravo` was
+  `added_to_merge_queue` at **17:57:19**, its entry was built at 17:57:37, and
+  `alpha` merged at **17:59:02** — so bravo was enqueued 1m43s *before* alpha
+  landed, and `ab47802` in its ref name is alpha's **queue head**, not "main
+  after alpha merged". Speculation was already happening at `SLEEP_SECONDS=60`.
+- The retracted claim caused real damage: it produced the conclusion "scenario 2
+  needs `SLEEP_SECONDS=180` to force real overlap", scenario 2 was then run at
+  180, and that override leaked onto `main` twice (see scenario 2). A wrong
+  inference here became a config bug three hours later.
 
 ### 5. Jenkins gating / timeout — _pending_
 

scripts/fake-queue-branch.sh

@@ -19,10 +19,15 @@ cd "$(dirname "$0")/.."
 base="$(git symbolic-ref --short refs/remotes/origin/HEAD 2>/dev/null | sed 's|^origin/||' || echo main)"
 
 if [ "${1:-}" = "--clean" ]; then
-  git ls-remote origin 'refs/heads/gh-readonly-queue/*' 'refs/heads/mq-sim/*' \
+  # Scope to the probe namespace ONLY. `*` in an ls-remote pattern matches across
+  # '/', so 'refs/heads/gh-readonly-queue/*' matches every REAL queue entry —
+  # running --clean during a scenario would delete live merge-queue refs
+  # mid-validation. Never widen these globs. No `|| true`: a refused delete must
+  # be visible, not swallowed.
+  git ls-remote origin 'refs/heads/gh-readonly-queue/*/pr-999-*' 'refs/heads/mq-sim/*' \
     | awk '{print $2}' | while read -r ref; do
         echo "deleting ${ref}"
-        git push origin --delete "${ref#refs/heads/}" || true
+        git push origin --delete "${ref#refs/heads/}"
       done
   exit 0
 fi

scripts/mk-pr.sh

@@ -19,6 +19,20 @@ branch="mq-test/${slug}"
 
 git fetch origin "$base"
 
+# A dirty tree silently contaminates the scenario: uncommitted ci/config.env
+# edits survive the switch, get staged by the unconditional `git add` below, and
+# ship in a PR whose commit message says only "add item".
+if ! git diff --quiet || ! git diff --cached --quiet; then
+  echo "dirty working tree — commit or stash first, or the changes ride along in the PR" >&2
+  git status --short >&2
+  exit 1
+fi
+
+# Any early exit below would otherwise strand the caller on a detached HEAD or
+# on the test branch, where their next unrelated commit would land.
+_orig="$(git symbolic-ref --short -q HEAD || git rev-parse HEAD)"
+trap 'git switch -q "$_orig" 2>/dev/null || true' EXIT
+
 # Re-runnable: a scenario gets replayed a lot, so rebuild the branch from the
 # current base instead of failing on leftovers from the previous run.
 git switch --detach "origin/${base}"
@@ -46,17 +60,21 @@ for kv in "$@"; do
   mv -f ci/config.env.tmp ci/config.env
   echo "set ${key}=${val}"
 done
-git add ci/config.env
+# Only when overrides were actually requested — an unconditional add would sweep
+# in any unrelated config change that reached the tree.
+if [ $# -gt 0 ]; then git add ci/config.env; fi
 
 git commit -m "test(${slug}): add item$([ $# -gt 0 ] && echo " + $*")"
 
 if [ -n "$restore" ]; then
   cat >&2 <<EOF
 
 !! ci/config.env overrides are COMMITTED to this branch, so they become the repo
-!! default if this PR merges. That is how main ended up with MAX_ITEMS=4 after
-!! scenario 7 and SLEEP_SECONDS=180 after scenario 2.
-!! Either close this PR when the scenario ends, or follow up with a PR restoring:
+!! default if this PR merges — and under a merge queue, enqueueing IS merging.
+!! This has leaked three times: MAX_ITEMS lowered 8 -> 4 (scenario 7, reverting a
+!! deliberate raise) and SLEEP_SECONDS 60 -> 180 twice (scenarios 2 and 3).
+!! CLOSE this PR when the scenario ends. scripts/queue-prs.sh will skip it unless
+!! you name it explicitly. If it does merge, restore:
 !!   ${restore}
 
 EOF

scripts/queue-prs.sh

@@ -32,7 +32,22 @@ if [ "${#prs[@]}" -eq 0 ]; then
   exit 1
 fi
 
+# Under a merge queue, enqueue and merge are the SAME action: --auto is the
+# enqueue path, and a green entry merges. So a PR carrying a ci/config.env
+# override cannot be "queued but not merged" — enqueueing it is deciding to make
+# its override the repo default. That is how MAX_ITEMS=4 (reverting a deliberate
+# 8) and SLEEP_SECONDS=180 (twice) reached main. Overrides are skipped unless
+# named explicitly on the command line.
+explicit=$#
 for pr in "${prs[@]}"; do
+  # Ask GitHub what the PR touches rather than diffing local refs, which may not
+  # be fetched and go stale after a force-push.
+  if [ "$explicit" -eq 0 ] && \
+     gh pr view "$pr" --json files -q '.files[].path' | grep -qx 'ci/config.env'; then
+    echo "== SKIP #${pr}: changes ci/config.env — merging it would change the repo default."
+    echo "   Close it when the scenario ends, or pass the number explicitly to force."
+    continue
+  fi
   echo "== enqueue #${pr}"
   gh pr merge "$pr" --merge --auto
 done

scripts/verify-merge.sh

@@ -57,10 +57,25 @@ check "merge commit has 2 parents" "2" "$nparents"
 check "second parent is the PR head" "$head_sha" "$p2"
 
 # Which queue entry validated this PR? Ref names are gh-readonly-queue/<base>/pr-<n>-<base-sha>.
-queue_sha="$(grep -E "/pr-${pr}-" "$QUEUE_LOG" 2>/dev/null | tail -1 | cut -d' ' -f1 || true)"
+# A missing comparison is a FAILED verification, never a pass. This check is the
+# whole reason the script exists ("landed tree == validated tree"); if it cannot
+# run, exiting 0 would report success having verified nothing — the same
+# silent-green shape the Jenkinsfile was fixed for in scenario 6.
+# A PR can have MORE THAN ONE queue entry: a speculative entry is discarded and
+# rebuilt when something ahead of it fails (scenario 3 produced two for pr-20).
+# `tail -1` would pick whichever the watcher happened to log last — ordered by
+# ls-remote's lexicographic ref names, not by which one merged — and comparing
+# against a discarded entry reports FAIL on the repo's headline claim. Prefer
+# the entry whose head IS the merge commit; fall back only if none matches.
+queue_sha="$(awk -v p="/pr-${pr}-" -v m="$merge_sha" '$2 ~ p && $1 == m {print $1}' "$QUEUE_LOG" 2>/dev/null | tail -1 || true)"
 if [ -z "$queue_sha" ]; then
-  note "  SKIP  tree comparison: no queue head for pr-${pr} in ${QUEUE_LOG}"
+  queue_sha="$(awk -v p="/pr-${pr}-" '$2 ~ p {print $1}' "$QUEUE_LOG" 2>/dev/null | tail -1 || true)"
+  [ -z "$queue_sha" ] || note "  NOTE  no logged entry matches the merge commit; using last-seen entry"
+fi
+if [ -z "$queue_sha" ]; then
+  note "  FAIL  tree comparison NOT PERFORMED: no queue head for pr-${pr} in ${QUEUE_LOG}"
   note "        (run scripts/watch-queue.sh during the queue run to capture it)"
+  fail=1
 else
   note "  queue head   : $queue_sha"
   if git cat-file -e "${queue_sha}^{commit}" 2>/dev/null; then
@@ -73,7 +88,10 @@ else
     check "landed tree == validated tree" \
       "$(git rev-parse "${queue_sha}^{tree}")" "$(git rev-parse "${merge_sha}^{tree}")"
   else
-    note "  SKIP  tree comparison: object ${queue_sha} not present locally"
+    note "  FAIL  tree comparison NOT PERFORMED: object ${queue_sha} was pruned"
+    note "        (queue heads are unreachable objects; watch-queue.sh now pins"
+    note "         them under refs/mq-queue/* so gc cannot collect them)"
+    fail=1
   fi
 fi