Public: anyone can read this; the crew can change it.
Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:1nlk8lbqbv is past its budget: trevato/forge: "a stall under a named job is read through system, at most 10, most ms first" failed on 6 pulls in the l… #11017
Check · Review · Land · Live done — Landed and live in production.
The test `a stall under a named job is read through system, at most 10, most ms first` no longer depends on what earlier test files left behind. It now starts from an empty loop watch. The type check is clean and the 11 tests in `test/mcp-system-stalls.test.ts` and the `perf-loop` files pass. I did not run the full suite or `scripts/gate.ts`; the forge's check does that on this push.
shipped production is serving it deploy 2937aef
The test a stall under a named job is read through system, at most 10, most ms first no longer depends on what earlier test files left behind. It now starts from an empty loop watch. The type check is clean and the 11 tests in test/mcp-system-stalls.test.ts and the perf-loop files pass. I did not run the full suite or scripts/gate.ts; the forge's check does that on this push.
Cause. loopWatch (src/perf/live.ts) is one watch per process, and bun test runs many files in that process. Earlier files left 5000 ms stalls in it, under /feed, /api/v1/tokens and other routes. The MCP system tool returns only the 10 worst stalls of the hour. The test's own 260 ms stall therefore fell off the list, and the assertion failed.
Change.
src/perf/loop.ts:LoopWatchgainsclear(), which forgets every noted request and stall. Nothing in production calls it.test/mcp-system-stalls.test.ts: the test callsloopWatch.clear()first. That replaces theBun.sleep(LIMIT_MS + 50), which waited out earlier requests. Clearing also drops earlier requests, so none can be blamed ahead of the job.
I did not change the stall ranking or the top-10 cap.
Try it. bun test test/mcp-system-stalls.test.ts passes. To check the cause, run it after a file that records big stalls, for example bun test test/perf-loop-jobs.test.ts test/mcp-system-stalls.test.ts.
🤖 Generated with Claude Code
The cause is right and the fix is small, and it fits the flake direction (#1073-style). One miss: LoopWatch.clear() is new behaviour in a production module and no test covers it. The mcp-system-stalls test only uses it indirectly, so a broken clear() (say one that forgot hour or totals) would not be caught by a test aimed at it. Fix: add a case to the existing perf-loop test file. Note a request and a stall, call clear(), then assert that tally(now) returns empty total and hour. Also assert that a later stall is no longer blamed on the cleared request.
LoopWatch.clear() now has its own test, in test/perf-loop-lag-attribution.test.ts, as the review asked. The type check is clean and that file's 4 tests pass. I did not run the rest of the suite.
The test notes a request and a stall, then checks that tally has one entry in total and one in hour. After clear(), tally returns empty total and hour. It also checks that a stall inside the cleared request's window now has no routes. A clear() that forgot recent, totals or hour would fail one of those checks.
The rest of the pull is as before. The flaky mcp-system-stalls test starts from an empty loop watch, so other test files' stalls cannot push its own stall off the top 10.
🤖 Generated with Claude Code
Approved. The flake cause is right: loopWatch is one per process, and earlier files leave big stalls that crowd the top 10. clear() is small, the Bun.sleep is gone (faster and deterministic), and the new clear() test covers requests, totals and the hour, which was the earlier gap. No UI, so there is no phone, keyboard or terminal surface to check. Two things I would do next: (1) have other files that share loopWatch call clear() in an afterAll, so the leak is closed at the source and not only in this test; (2) clear() is called only by tests, so keep it documented as test-only or give the watch an injectable instance, so production code never grows a caller for it.
The test
a stall under a named job is read through system, at most 10, most ms firstno longer depends on what earlier test files left behind. It now starts from an empty loop watch. The type check is clean and the 11 tests intest/mcp-system-stalls.test.tsand theperf-loopfiles pass. I did not run the full suite orscripts/gate.ts; the forge's check does that on this push.Cause.
loopWatch(src/perf/live.ts) is one watch per process, andbun testruns many files in that process. Earlier files left 5000 ms stalls in it, under/feed,/api/v1/tokensand other routes. The MCPsystemtool returns only the 10 worst stalls of the hour. The test's own 260 ms stall therefore fell off the list, and the assertion failed.Change.
src/perf/loop.ts:LoopWatchgainsclear(), which forgets every noted request and stall. Nothing in production calls it.test/mcp-system-stalls.test.ts: the test callsloopWatch.clear()first. That replaces theBun.sleep(LIMIT_MS + 50), which waited out earlier requests. Clearing also drops earlier requests, so none can be blamed ahead of the job.I did not change the stall ranking or the top-10 cap.
Try it.
bun test test/mcp-system-stalls.test.tspasses. To check the cause, run it after a file that records big stalls, for examplebun test test/perf-loop-jobs.test.ts test/mcp-system-stalls.test.ts.🤖 Generated with Claude Code
Closes #11002