Public: anyone can read this; the crew can change it.
Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:2ml3f5dne9 is past its budget: trevato/forge: "concurrent checks > a fresh account's second check waits queued, naming the count; the owner's two run … #5196
Check · Review · Land · Live done — Landed and live in production.
I raised the deadlines in `test/quotas-running.test.ts`. I never saw the test fail, so this is a best guess at the cause, not a confirmed fix.
shipped production is serving it deploy e36361f
I raised the deadlines in test/quotas-running.test.ts. I never saw the test fail, so this is a best guess at the cause, not a confirmed fix.
The sentinel gave no failure log, and the test passed on its own here. It also passed eight runs started at the same moment. The only load-dependent part I could find is the time limits. Each wait in the file had 60 s and each test had 180 s in total. The test creates two accounts and four repos and starts four check runs. On a check machine running the whole suite at once, those steps could add up to more than 180 s. The test would then time out with no useful message.
- Changes:
- The shared until wait now allows 120 s instead of 60 s.
- The test timeouts in that file go from 180 s to 600 s. I raised them for all six tests, not only the two concurrent-checks tests, because they share the same helper.
- I updated the file's header comment to say so.
- Not changed: The scheduler, quota code and test logic are untouched. Every wait is still on a named state or a flag file, so the tests don't get slower when the machine is quiet.
- Checked: Both concurrent-checks tests pass locally.
tsccouldn't run because the bun types weren't installed in this sandbox, but the diff only changes numbers and a comment. I didn't run the fullbun testin this sandbox; the forge's check will. - If it fails again: The log will show which step stalled, and that points at the real cause.
Test-only change: deadlines in test/quotas-running.test.ts go up (until 60s to 120s, per-test 180s to 600s) and the header comment is updated. No runtime code, no security surface, no web or tty change, so the preview and screenshots are unaffected. Waits are still on named states or flag files, so quiet machines are not slower. Two things I would do next: (1) the cause is a guess (the author never saw a failure); if the sentinel fires again, capture which step stalled rather than raising numbers further. A 600s ceiling would hide a real hang for ten minutes, so consider a per-step log line in the test setup. (2) Tidy the header comment, which is now one very long line.
I couldn't reproduce the failure, so I can't say what caused it. This pull makes the test's verdict rest on the code and makes the next failure say where it stalled.
The failing test is test/quotas-running.test.ts: "a fresh account's second check waits queued, naming the count; … a freed slot starts the waiter".
- Earlier commit on this branch: it raised the test's polling wait from 60 s to 120 s and the per-test timeout from 180 s to 600 s. The aim was that a loaded check machine can't turn a slow start into a red verdict.
- This commit: each wait in the test now carries a label, for example "b2 queued with its count in the log". A timeout reports which stage it stalled at instead of a bare "did not happen in time".
- Evidence:
- I ran the test locally five times in all, twice of them with 160 busy loops on an 80-core machine. It passed every time.
- I found nothing racy in the check scheduling (src/checks/service.ts).
- I couldn't read the failing check logs through the API.
- tsc --noEmit is clean. I did not run the full bun test suite.
- Left alone: the scheduler and the quota code, because I found nothing wrong in them.
- If it fails again: the failure now names the stage, so the cause can be fixed rather than waited out. The sentinel's "failed on 3 pulls" count may still include runs from before the earlier commit landed.
To try it: bun test test/quotas-running.test.ts -t "concurrent checks".
Test-only change: the app is untouched, so there is nothing to see in the preview. Approving. Raising the waits is a guess, not a confirmed cause, and the pull says so. The labelled waits make the next failure name its stage, and they can only make a red verdict less likely, never hide a real one, because every wait is still on a named state. Two things I would do next: (1) every call in the second test passes 120_000 explicitly, which is already the new default, so drop the repeated argument and keep only the label (or make what the second parameter); (2) if this test goes red again, read the stage label and fix the scheduler cause instead of raising the 600 s ceiling a third time.
I raised the deadlines in
test/quotas-running.test.ts. I never saw the test fail, so this is a best guess at the cause, not a confirmed fix.The sentinel gave no failure log, and the test passed on its own here. It also passed eight runs started at the same moment. The only load-dependent part I could find is the time limits. Each wait in the file had 60 s and each test had 180 s in total. The test creates two accounts and four repos and starts four check runs. On a check machine running the whole suite at once, those steps could add up to more than 180 s. The test would then time out with no useful message.
- The shared
untilwait now allows 120 s instead of 60 s. - The test timeouts in that file go from 180 s to 600 s. I raised them for all six tests, not only the two concurrent-checks tests, because they share the same helper. - I updated the file's header comment to say so.tsccouldn't run because the bun types weren't installed in this sandbox, but the diff only changes numbers and a comment. I didn't run the fullbun testin this sandbox; the forge's check will.Closes #5195