Uh oh!
There was an error while loading. Please reload this page.
fix(dispatch-capacity): degrade state when a placed slot outlasts agentHoldTimeoutMs (#419) - #423
Conversation
…ntHoldTimeoutMs (#419) Before this, `dispatchCapacity` had two hold bounds surfaced and neither applied to the failure that actually happened. `waitWarnMs` concerns queue WAIT time and never fires with `waiting: 0`; `agentlessHoldTimeoutMs` bounds a slot held WITHOUT a placed agent, so a slot with `placedAgents: 1` was exempt. The reaper for occupied slots did exist (`agentHoldTimeoutMs`, 4h default) but was never exposed next to the agentless one and never fed the state derivation — so an operator reading /healthz saw `dispatchCapacity.state: "healthy"` while two occupants had held their slots 13.5 hours (27x the agentless timeout, 3.4x the placed one). Changes: - Publish `agentHoldTimeoutMs` on `FactoryDispatchCapacityStatus` and `FactoryPublicDispatchCapacityHealth` so a reader can check `slotHeldForMs`/`heldForMs` against BOTH reap deadlines, not just the agentless one. - Add `occupiedOccupants` (aggregate count) and per-occupant `pastOccupiedDeadline`, mirroring the discipline of `agentlessOccupants` and `pastReapDeadline`. Anchors on `heldForMs` — the clock the reaper uses — so a placement whose spawn took hours is not confused with a placement that outran its deadline. `>=`, not `>`: agrees with the reaper's `nowMs < dueAtMs` skip boundary. - Extend `deriveDispatchCapacityState` / `dispatchCapacityState` to degrade to `stalled` when either wedge shape is present, and thread the same signal through `normalizePublicHealth` so a record that arrives over the wire without the aggregate count still projects the wedge (mirror of the #318 discipline for the agentless shape). - Ablation tests: must-fire (13.5h placed-agent occupant degrades state, populates `occupiedOccupants`, reaches `status: 'degraded'` and `degradedSubsystems` without moving `ok`), must-not-fire (a placed- agent occupant one millisecond inside the deadline stays healthy), and a wire-record variant that proves an old producer without the count still degrades. Reclamation itself already survives a restart: `heldSinceAtMs` is carried on the durable lifecycle and `inFlightRecordFromLifecycle` re-derives it on rehydration, then the finally-block at the end of the adopt path re-arms `#rescheduleHeldAgentDeadlineSweep`. The existing `agentHoldTimeoutMs` reaper (`#sweepHeldAgentDeadlines`) fires immediately after boot for any record already past deadline. This PR is the missing observational half: without it, the reaper's absence in a running daemon is invisible on /healthz. Related but distinct: #331 (orphan-reaper `dispatchCallActive` guard can never fire — separate composed-key bug in `#dispatchInFlight` lookup). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@coderabbitai review Requested for exact head |
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Two review threads on #423 (cubic-dev-ai P3): - src/orchestrator/public-health.test.ts: the placed-wedge tests covered 4h-1ms (must-not-fire) and 13.5h (must-fire) but not the exact `>=` boundary. Add an at-deadline case that mirrors the existing agentless boundary test, so a regression to `>` on the shared reap-instant convention would surface. - src/types.ts: the `agentHoldTimeoutMs` doc guided readers toward `slotHeldForMs`, which is the wrong clock for the placed shape and would let a slow-spawn slot read as an occupied wedge. Call out `heldForMs` explicitly, and why the two clocks are distinct. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai review Requested for exact head |
Closes#419.
The defect
dispatchCapacityhad two hold bounds surfaced and neither applied to the failure that actually happens.waitWarnMs(30 min) concerns queue WAIT time and never fires withwaiting: 0.agentlessHoldTimeoutMs(30 min) bounds a slot held WITHOUT a placed agent. A slot withplacedAgents: 1was exempt.The reaper for occupied slots did exist (
agentHoldTimeoutMs, 4 h default) but was never exposed next to the agentless one and never fed the state derivation. So an operator reading/healthzsawdispatchCapacity.state: "healthy"while two occupants had held their slots 13.5 hours — 27× the agentless timeout, 3.4× the placed-agent timeout — withbatchSize: 2and both slots consumed. Total dispatch stop, green subsystem.The fix
agentHoldTimeoutMsonFactoryDispatchCapacityStatusandFactoryPublicDispatchCapacityHealthso a reader can checkslotHeldForMs/heldForMsagainst both reap deadlines.occupiedOccupants(aggregate) and per-occupantpastOccupiedDeadline. Mirrors the discipline ofagentlessOccupants/pastReapDeadline. Anchored onheldForMs(the clock the reaper uses) so a placement whose spawn took hours is not confused with a placement that outran its deadline.>=, not>: agrees with the reaper'snowMs < dueAtMsskip boundary.deriveDispatchCapacityState/dispatchCapacityStateto degrade tostalledwhen either wedge shape is present, and thread the same signal throughnormalizePublicHealthso an over-the-wire record without the aggregate count still projects the wedge (mirror of the fix(factory): arm the never-placed reaper independently of startup fleet health (#315) #318 discipline).dispatchCapacitywas already inDISPATCH_GATING_SUBSYSTEMSand the top-levelstatuscomputation already checks it. The state change flipsstatus: "ok" → "degraded"and adds"dispatchCapacity"todegradedSubsystems.ok(container-liveness) is deliberately left alone — recycling a wedged Factory destroys the durable evidence of the wedge and re-imports the lock into the replacement (the Dispatch is permanently wedged: one agent-less lifecycle holds the only batch slot and cannot be reaped, so every queued row spins at 1 Hz forever #303 doc-comment discipline).Reclamation across a restart
Already survives:
heldSinceAtMsrides on the durable lifecycle,inFlightRecordFromLifecyclere-derives it on rehydration, and thefinallyblock at the end of the adopt path calls#rescheduleHeldAgentDeadlineSweepeven if a broker call above it threw. The existingagentHoldTimeoutMsreaper (#sweepHeldAgentDeadlines) fires immediately post-boot for any record already past deadline (#412 comment thread confirms the 2026-08-31T11:14Z restart cleared the stranded slots). This PR is the missing observational half: without it, the reaper's absence in a running daemon is invisible on/healthz.Ablation
Three tests in
src/orchestrator/public-health.test.ts:state: 'stalled',occupiedOccupants: 2,degradedSubsystems: ['dispatchCapacity'],status: 'degraded'.okstaystrue.state: 'healthy', nooccupiedOccupants, nopastOccupiedDeadline, top-levelstatus: 'ok'.slotHeldForMsis deliberately far past the bound to prove the diagnostic anchors onheldForMs.normalizePublicHealth— mirror of the fix(factory): arm the never-placed reaper independently of startup fleet health (#315) #318 fix for the agentless shape.Full suite: 750/750 pass across
orchestrator/public-health,orchestrator/factory,config/schema,cli/diagnose.Overlap with related issues
dispatchCallActiveguard can never fire): distinct. That is a composed-key bug in the#dispatchInFlightlookup path — the reaper reads a bare issue key while writers compose${issue}:${dryRun}:${phase}. Unrelated to this fix; not addressed here.readinessReconcile).Test plan
vitest run src/orchestrator/public-health.test.ts— 58/58 pass, incl. 3 new dispatchCapacity reports healthy while both slots are held 13.5h by offline agents: no max hold for an occupied slot #419 testsvitest run src/orchestrator/factory.test.ts src/config/schema.test.ts src/cli/diagnose.test.ts— 750/750 pass🤖 Generated with Claude Code