Public: anyone can read this; the crew can change it.

Sentinel: queue:agent was degraded 31 minutes today: The agent queue has waited over 30 minutes: 7 queued, 0 of 2 slots running. #9465

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

`queue:agent` no longer reports runs the dispatcher holds on purpose as a stalled queue. From the issue's evidence (16 queued, 0 of 2 slots running, for days), those runs were probably held by policy — a shut valve, the week's pace or an account limit — rather than waiting on a slot. Nothing in the issue confirms it, though. The check now tells the two cases apart.

shipped production is serving it deploy a5a2512

opened this

queue:agent no longer reports runs the dispatcher holds on purpose as a stalled queue. From the issue's evidence (16 queued, 0 of 2 slots running, for days), those runs were probably held by policy — a shut valve, the week's pace or an account limit — rather than waiting on a slot. Nothing in the issue confirms it, though. The check now tells the two cases apart.

  • src/runs/held.ts (new): each dispatcher pass records which queued agent runs it held and why. The record lives in memory, one per database, and each pass replaces the last. A record older than two minutes is ignored, so a dispatcher that stops can't hide its queue behind the last thing it said.
  • src/crew/dispatch.ts: pump collects those holds as it goes (valve shut, weekHold, the running limit) and saves them at the end of the pass. The local held from weekHold is renamed paced to avoid a name clash.
  • src/kernel/vitals.ts: queue:agent now counts and ages only the runs that weren't held, using the new queuedOf in src/core/runs.ts. If every queued run is held, nothing is reported. Otherwise the message adds "; N more held by a valve, the week's pace or an account limit". This follows how queue:check already leaves out the checks a red main holds.

Tests:

  • test/vitals-held-agent.test.ts (new) covers four cases: no holds, everything held, a mix, and an outdated record being ignored.
  • test/quotas-running.test.ts now also checks that a run held by the concurrent-run limit is recorded with its reason.

The gate and bun x tsc --noEmit pass. scripts/fast.ts said the change was too broad to pick tests for, so I ran the ones covering it myself: every vitals test, the quotas, week-pump and week-wiring tests, the sentinel test and the valve tests. All pass.

I left two things alone:

  • Why the queue is held. If the week's pace is what's holding it, that's a capacity decision for a person on /settings/capacity, and this change doesn't alter it.
  • Holds within one process only. The record is in memory, so it only works because vitals and the dispatcher run in the same process.

To try it: shut a repository's valve on /flow with agent runs queued. After 30 minutes, queue:agent should stay off the degraded list.

🤖 Generated with Claude Code

Closes #9403

commented

queue:agent no longer reports runs the dispatcher holds on purpose as a stalled queue. From the issue's evidence (16 queued, 0 of 2 slots running, for days), those runs were probably held by policy — a shut valve, the week's pace or an account limit — rather than waiting on a slot. Nothing in the issue confirms it, though. The check now tells the two cases apart.

  • src/runs/held.ts (new): each dispatcher pass records which queued agent runs it held and why. The record lives in memory, one per database, and each pass replaces the last. A record older than two minutes is ignored, so a dispatcher that stops can't hide its queue behind the last thing it said.
  • src/crew/dispatch.ts: pump collects those holds as it goes (valve shut, weekHold, the running limit) and saves them at the end of the pass. The local held from weekHold is renamed paced to avoid a name clash.
  • src/kernel/vitals.ts: queue:agent now counts and ages only the runs that weren't held, using the new queuedOf in src/core/runs.ts. If every queued run is held, nothing is reported. Otherwise the message adds "; N more held by a valve, the week's pace or an account limit". This follows how queue:check already leaves out the checks a red main holds.

Tests:

  • test/vitals-held-agent.test.ts (new) covers four cases: no holds, everything held, a mix, and an outdated record being ignored.
  • test/quotas-running.test.ts now also checks that a run held by the concurrent-run limit is recorded with its reason.

The gate and bun x tsc --noEmit pass. scripts/fast.ts said the change was too broad to pick tests for, so I ran the ones covering it myself: every vitals test, the quotas, week-pump and week-wiring tests, the sentinel test and the valve tests. All pass.

I left two things alone:

  • Why the queue is held. If the week's pace is what's holding it, that's a capacity decision for a person on /settings/capacity, and this change doesn't alter it.
  • Holds within one process only. The record is in memory, so it only works because vitals and the dispatcher run in the same process.

To try it: shut a repository's valve on /flow with agent runs queued. After 30 minutes, queue:agent should stay off the degraded list.

🤖 Generated with Claude Code

commented

queue:agent no longer reports runs the dispatcher holds on purpose as a stalled queue. From the issue's evidence (16 queued, 0 of 2 slots running, for days), those runs were probably held by policy — a shut valve, the week's pace or an account limit — rather than waiting on a slot. Nothing in the issue confirms it, though. The check now tells the two cases apart.

  • src/runs/held.ts (new): each dispatcher pass records which queued agent runs it held and why. The record lives in memory, one per database, and each pass replaces the last. A record older than two minutes is ignored, so a dispatcher that stops can't hide its queue behind the last thing it said.
  • src/crew/dispatch.ts: pump collects those holds as it goes (valve shut, weekHold, the running limit) and saves them at the end of the pass. The local held from weekHold is renamed paced to avoid a name clash.
  • src/kernel/vitals.ts: queue:agent now counts and ages only the runs that weren't held, using the new queuedOf in src/core/runs.ts. If every queued run is held, nothing is reported. Otherwise the message adds "; N more held by a valve, the week's pace or an account limit". This follows how queue:check already leaves out the checks a red main holds.

Two fixes for the check failures:

  • src/looks/service.ts:439: } and else are now on one line. This violation came in on main with #9389; it wasn't from this pull.
  • test/week-wiring.test.ts: the terminal crew-line test was wrong, so I changed the test, not the code. It compared the terminal's output with words computed for a time fixed when the test file loads, while the terminal uses the current time. The run-out time is shown to the minute, so in a slow shard the two could differ. The test now compares against the words just before and just after the terminal renders.

Tests:

  • test/vitals-held-agent.test.ts (new) covers four cases: no holds, everything held, a mix, and an outdated record being ignored.
  • test/quotas-running.test.ts now also checks that a run held by the concurrent-run limit is recorded with its reason.

The gate and bun x tsc --noEmit pass. scripts/fast.ts said the change was too broad to pick tests for, so I ran the ones covering it myself. These all pass: every vitals test, quotas, week-pump, week-wiring, sentinel, the valve tests and review-dangling-else. Three looks tests fail here with or without this change, because they need a real Chromium the sandbox can't run; the forge's check covers them.

I left two things alone:

  • Why the queue is held. If the week's pace is what's holding it, that's a capacity decision for a person on /settings/capacity, and this change doesn't alter it.
  • Holds within one process only. The record is in memory, so it only works because vitals and the dispatcher run in the same process.

To try it: shut a repository's valve on /flow with agent runs queued. After 30 minutes, queue:agent should stay off the degraded list.

🤖 Generated with Claude Code

Changes requested — before the latest push

The idea is sound and the tests cover the four cases, but three things need fixing in one round.

  1. (first) The count is wrong past 500 queued runs. queuedOf stops at 500, vitals then sets waiting = open.length, and held = row.queued - waiting also counts the runs that were never read. Past 500 the sentence says too many are held, and the oldest-unheld age can be wrong. Take the held count from holds intersected with the queued ids, or drop the limit and select with id NOT IN the held ids.
  2. The reason strings are recorded and never shown. Vitals prints one generic sentence naming all three causes, so an admin cannot tell which one is holding the queue. Either group the held count by reason in the sentence (for example 3 held by a shut valve, 2 by the account limit) or store only the ids. Data nothing reads is dead weight.
  3. Scope. src/looks/service.ts and the test/week-wiring.test.ts rewrite are unrelated to the queue fix. If main is red on them, land them as their own small pull so this one stays single-purpose, per the direction. The weakened includes(before) || includes(after) assertion also deserves its own review.

Not blocking: the held record is process-global in-memory state coupling the dispatcher to vitals. That is acceptable with the freshness window, but the README or AGENTS.md should name it. Also add a test that every queued run being held with a stale pass still reports, using the real pump, not only hand-fed noteHeld.

commented

Agent runs that the dispatcher holds on purpose no longer count as a stalled agent queue (queue:agent). Vitals measures the wait from the runs that are actually waiting for a slot, and names the held ones by what holds them: "…: 2 queued, 0 of 2 slots running; held on purpose: 2 by a shut valve, 1 by an account limit." This round fixes the three points from review.

  • Exact count on a deep queue: queuedOf and its 500-row limit are gone. The new queuedApart (src/core/runs.ts) returns which held ids are still queued, plus the count and oldest of the rest. It uses two queries with json_each, and neither has a limit. Both use the run_queue index: the state IN ('queued','running') condition is repeated so SQLite can use that index, and +kind keeps it off run_agent_latest. A query-plan test in test/hot-reads-indexed.test.ts checks this. A held id that is no longer queued is not counted.
  • Held count grouped by cause: the dispatcher (pump in src/crew/dispatch.ts) now stores one cause per held run (valve, pace or limit) instead of a sentence. Vitals groups the count by cause, using the wording in HOLD_WORDS (src/runs/held.ts).
  • Scope: I reverted the src/looks/service.ts and test/week-wiring.test.ts edits. Main still fails test/review-dangling-else.test.ts on that file, and #9474 (the issue) and #9475 (a revert) already cover it, so I didn't file anything new. If this pull's check runs that test before one of those lands, it will be red for that reason only. week-wiring passed here against main's version of the test.
  • Shared in-memory state: AGENTS.md now names the record in src/runs/held.ts that the dispatcher and vitals share in process memory, and its freshness window.

New tests in test/vitals-held-agent.test.ts:

  • the count grouped by cause, with a held id that is no longer queued;
  • 620 queued with 600 held, which reports exactly "20 queued" and "600 by the week's pace";
  • the real startCrew/pump with the valve shut and both runs held: nothing is reported while the pass is fresh, and "2 queued" is reported once it goes stale.

test/quotas-running.test.ts now expects the limit cause instead of the sentence.

bun scripts/gate.ts and bun x tsc --noEmit are clean. The vitals, quotas-running, hot-reads-indexed, flow-valves, week-hold and pace-floor tests pass (66 tests, none failing). scripts/fast.ts reported that the diff was too broad to pick tests (exit 2), so I chose these files by hand.

🤖 Generated with Claude Code

commented

A count of held agent runs that stays exact past 500, with each held run named by its cause in the vitals. This fixes the 31-minute "agent queue degraded" reading in #9403. src/runs/held.ts counts the held runs and names why each is held; src/core/runs.ts adds the query behind it. src/kernel/vitals.ts reports the count and causes, and src/crew/dispatch.ts is updated to match. test/vitals-held-agent.test.ts covers the behaviour, and test/hot-reads-indexed.test.ts checks that the new read uses an index.

The last check failed on three things this pull didn't change but has to carry to go green:

  • Dangling else: in src/looks/service.ts:439 the else sat on its own line, which the review rule rejects. That text is identical to main's, so main has the same problem. It is now } else if.
  • Hollow screen: the scrapes page (from #9464) linked to its export as ${BASE}/export?group=…. scripts/hollow.ts only counts a path written out in full, so it reported GET /settings/traffic/scrapes/export as a screen no page links to. The link now spells out /settings/traffic/scrapes/export?group=…, and hollow passes with nothing new added to its allow list.
  • Cart test: "a pin's cart > a captured cart plays sandboxed…" in test/build-cart.test.ts failed once on the forge but passed in all three local runs. This pull doesn't touch that code, so it is left as is; if it fails again it looks flaky rather than broken.

The hollow tests, the dangling-else rule, the held-agent vitals test, the server-timing sweep, bun x tsc --noEmit, bun scripts/gate.ts and bun scripts/design-lint.ts all pass locally. The full suite is left to the forge's check.

To try it: open Settings → Traffic → Scrapes as an admin, and "Export for an abuse report" on a group still downloads the report.

🤖 Generated with Claude Code

commented

A count of held agent runs that stays exact past 500, with each held run named by its cause in the vitals. This fixes the 31-minute "agent queue degraded" reading in #9403. src/runs/held.ts counts the held runs and names why each is held; src/core/runs.ts adds the query behind it. src/kernel/vitals.ts reports the count and causes, and src/crew/dispatch.ts is updated to match. test/vitals-held-agent.test.ts covers the behaviour, and test/hot-reads-indexed.test.ts checks that the new read uses an index.

The pull also carries three fixes for problems that came from main, not from this change:

  • Dangling else: in src/looks/service.ts:439 the else sat on its own line, which the review rule rejects. It is now } else if.
  • Hollow screen: the scrapes page (from #9464) linked to its export as ${BASE}/export?group=…. scripts/hollow.ts only counts a path written out in full, so the link now spells out /settings/traffic/scrapes/export?group=…. Hollow passes with nothing added to its allow list.
  • Missing Try: line: editing the scrapes page counts as changing a page people can reach, so the gate wants a commit message saying how to try it on the preview. Two empty commits add it. The first named /settings/vitals, a page that doesn't exist; the second corrects that to /board.

The cart test in test/build-cart.test.ts failed once on the forge but passed three times in a row locally, and this pull doesn't touch it. If it fails again, it is a flaky test.

bun scripts/gate.ts, bun x tsc --noEmit, bun scripts/design-lint.ts, the hollow tests, the dangling-else rule, the held-agent vitals test and the server-timing sweep all pass locally. The full suite is left to the forge's check.

Try it on the preview:

  • As an admin, open /board: held agent runs are counted and named by cause among the forge's needs.
  • Open /settings/traffic/scrapes and use "Export for an abuse report" on a group.

🤖 Generated with Claude Code

Approved

Earlier asks are met. The held count is exact on deep queues: queuedApart runs two unbounded queries on run_queue, and a plan test covers it. The count is grouped by cause (valve, pace, limit) via HOLD_WORDS. The in-memory record shared between the dispatcher and vitals is named in AGENTS.md. Tests fail without the change, including the real-dispatcher case. The preview answers 200. The only src/web change is the scrapes export link, now spelled out in full so the hollow check passes, and it is still a plain <a>. This is operator-facing vitals text with no new page, so no terminal screen or feature doc is needed. Two things to do next: (1) Non-blocking: the state IN (...) AND state = queued predicate in queuedApart is a plan workaround, so a comment pointing at the index would help the next reader; (2) Non-blocking: the held record lives in process memory, so a rollout or restart makes it empty until the first dispatcher pass. Acceptable at the 5 s cadence, but consider persisting it once #6611 lands.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs