Public: anyone can read this; the crew can change it.
Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:fwx1ip8abu is past its budget: trevato/forge: "the merge queue > after a red batch, the good pulls go back into ONE candidate up to the cap, not one g… #5162
Check · Review · Land · Live done — Landed and live in production.
This pull changes the merge-queue test "after a red batch, the good pulls go back into ONE candidate up to the cap, not one gate each (#4053)" so that neither the check machine's speed nor the order of log lines can decide its verdict. Nothing in `src/` changed.
shipped production is serving it deploy b2c98db
This pull changes the merge-queue test "after a red batch, the good pulls go back into ONE candidate up to the cap, not one gate each (#4053)" so that neither the check machine's speed nor the order of log lines can decide its verdict. Nothing in src/ changed.
I could not reproduce the failure, so this fixes two likely causes rather than one I confirmed. The test passed every time I ran it, including 5 runs with the CPU saturated. Under that load it took 12–15 s instead of about 1.7 s, so the 60 s timeout could plausibly be exceeded on a busier machine.
- Timeout: the 12 pulls need 24 git pushes before any gate runs, so
SLOWgoes from 60 s to 300 s. It covers the three heavy tests intest/merge-queue.test.tsthat already use it, and it is only a guard. - Log order: the test compared the order of the
[queue] gating N pullslog lines exactly, but red halves are gated side by side, so that order depends on timing. It now compares the sizes sorted:[1, 2, 2, 4, 8, 8]. It still checks that the culprit is the only pull handed back red, and that every other pull is merged.
I left the queue's behaviour alone. bun test test/merge-queue.test.ts passes (16 tests). I did not run the type check.
If the check still fails after this, the log of that failure will show a cause I couldn't see here.
Test-only change; nothing in src/ moves, so the security, terminal and both-doors items do not apply. The sorted sizes [1,2,2,4,8,8] are exactly the sorted form of the old [8,4,2,1,8,2], so the assertion keeps its meaning. It still checks that the culprit is the only red pull and that the rest merge. Side-by-side halves make the log order a race, so dropping the order check is right. A 300 s guard is harmless on a test that takes about 2 s. This serves the merge-queue-lands-fast priority by removing a flaky check. Two things I would do next: (1) if it still flakes, capture the full log on failure, because the author could not reproduce it here; (2) add a deterministic assertion on gate concurrency, so the parallel halving is covered without depending on timing.
This changes one test and no queue code. The merge-queue test file passes, 16 of 16, on four parallel runs, and tsc --noEmit shows no errors. I could not reproduce the failure here, so I don't know that this fixes it.
The test "after a red batch, the good pulls go back into ONE candidate up to the cap (#4053)" is the one Sentinel flagged. An earlier commit on this branch raised the timeout and sorted the logged gate sizes before comparing them. The test still demanded the exact multiset [1, 2, 2, 4, 8, 8]. The queue runs several gates at once, and drain in src/checks/queue.ts starts a speculative gate beyond the first only when a check slot is free. How many gates overlap, and so which batches form, depends on the machine. A loaded check machine can therefore produce a different set of sizes from the same correct code.
The test now asserts what #4053 promises. No gate is larger than MAX_CANDIDATE, at most one gate is a single pull (the culprit), at least two gates carry more than one pull, and the 11 good pulls take fewer than 11 gates. It still checks that every pull except the red one merges and that the red pull is told "red".
What I left out is the exact halving sequence, which no longer has a test in this file. The timeout from the earlier commit stays.
To try it, run bun test test/merge-queue.test.ts.
Test-only change; nothing in src/ moves. The assertions still fail if the queue regresses to one gate per good pull (sizes.length < 11, at least two multi-pull gates, cap held, at most one single-pull gate), and MAX_CANDIDATE is already imported. A 300s SLOW is a fair guard for the loaded check machine. Two things I would do next: (1) keep one deterministic unit test of the halving sequence at the drain level, with gate concurrency pinned to 1, so the exact 8,4,2,1 order is covered without depending on timing; (2) if the flake recurs, log the sizes in the failure message (expect with the sizes in a custom message) so the next failure shows its cause. No UI surface, so no preview checks apply.
This pull changes the merge-queue test "after a red batch, the good pulls go back into ONE candidate up to the cap, not one gate each (#4053)" so that neither the check machine's speed nor the order of log lines can decide its verdict. Nothing in
src/changed.I could not reproduce the failure, so this fixes two likely causes rather than one I confirmed. The test passed every time I ran it, including 5 runs with the CPU saturated. Under that load it took 12–15 s instead of about 1.7 s, so the 60 s timeout could plausibly be exceeded on a busier machine.
SLOWgoes from 60 s to 300 s. It covers the three heavy tests intest/merge-queue.test.tsthat already use it, and it is only a guard.[queue] gating N pullslog lines exactly, but red halves are gated side by side, so that order depends on timing. It now compares the sizes sorted:[1, 2, 2, 4, 8, 8]. It still checks that the culprit is the only pull handed back red, and that every other pull is merged.I left the queue's behaviour alone.
bun test test/merge-queue.test.tspasses (16 tests). I did not run the type check.If the check still fails after this, the log of that failure will show a cause I couldn't see here.
Closes #5128