Repository navigation
fix(supervisor): re-arm guards on host stall instead of killing - #1536
Conversation
Co-Authored-By: brainlayerCodex running gpt-6.1-sol <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 47 minutes. View limit details
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Guard re-arm behavior src/brainlayer/supervisor_guard.py, tests/test_supervisor_guard_stalls.py |
The guard rearms after wait overshoots greater than five seconds, up to three times and 300 seconds of cumulative grace. A real heartbeat resets both limits. Tests cover re-arm limits, timeout behavior, EOF, quiescence, and diagnostic failures. |
Calendar admission deadline src/brainlayer/maintenance_calendar.py, tests/test_calendar_recovery_priority.py |
Calendar admission adds the maximum stall grace to its deadline. A regression test checks that admission succeeds after the recovery, exit, and grace allowances. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Sequence Diagram(s)
sequenceDiagram
participant SupervisorGuard
participant HeartbeatPipe
participant ParentProcess
participant DiagnosticSink
SupervisorGuard->>HeartbeatPipe: wait for heartbeat input
HeartbeatPipe-->>SupervisorGuard: input after scheduling stall
SupervisorGuard->>DiagnosticSink: emit guard-stall-rearm
SupervisorGuard->>HeartbeatPipe: wait for re-armed timeout
HeartbeatPipe-->>SupervisorGuard: real heartbeat
SupervisorGuard->>SupervisorGuard: reset stall-rearm counters
SupervisorGuard->>ParentProcess: kill after timeout when heartbeat is absent
Possibly related issues
- Issue 1445: This PR extends the calendar admission deadline for guard stall grace. It does not implement the issue’s indexing and finalization budget changes.
- Issue 1422: This PR extends calendar custody timing with bounded stall grace. It does not implement the backup receipt deadline or startup sequencing.
Possibly related PRs
- EtanHey/brainlayer#1464: This PR modifies the heartbeat guard and its timeout behavior established by that change.
- EtanHey/brainlayer#1525: This PR changes guard behavior for the heartbeat-starvation condition and adds tests for it.
Merge Risk: 
Low · previous review bebe9
-
Repeated prolonged stalls can cause a full-calendar run to time out while the previous guard retains custody.
Pre-merge checks: 
3 · 
2
-
Failed checks: 2 warnings Passed checks: 3 passed-
Full details: Title check
-
Explanation
The title names the main change: guards now re-arm on host stalls instead of killing the parent. It follows the conventional-commit style and matches the supervisor_guard changes, the stall constants, and the re-arm tests in the changeset. It is 63 characters, within a reasonable length.
-
Full details: Description check
-
Explanation
The description covers the problem, scope, re-arm and grace rules, EOF and quiescence handling, validation, and handoff notes, which are the substance this check needs. It does not follow the repository template. It has no 'Change and evidence' heading. It has no 'Crash sweep / feature batch' section, and the crash-sweep gate instructions are not addressed beyond a historical note. The Loop-record line is present but its values are for the author workflow. The description is largely complete but misses required template sections.
Resolution
Add the template's required sections. Use the '## Change and evidence' heading, and describe the change and the exact-head checks run. Add the '## Crash sweep / feature batch' section. State whether the crash-sweep label is needed, and report the exact-head CI status for the batch commit SHA with the verify-release gate result. Keep the Loop-record line in the template's format.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 18 functions across 4 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit waits while seconds sprawl,
The guard grants grace, but counts it all.
A heartbeat hops; the caps reset,
The calendar waits past the sunset.
Then quiet timeouts close the gate,
And plans resume without delay.
Comment @coderabbitai help to get the list of available commands.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 8568ee0b-02e7-4246-a1b6-f2e93f0f8faf) |
Cross-vendor review — #1536 @ ab3e525VERDICT: OK Answers to the brief
Findings
Required before merge
— Claude reviewer (cross-vendor) |
Co-Authored-By: brainlayerCodex running gpt-6.1-sol <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: d71c3022-080d-47bb-bb9e-9e43497db129) |
BrainLayer ratchetEvery Value below was measured by this run. A row this machine cannot measure says Hosted success gives no verdict on mapped bytes, search p50/p95, idle CPU, or fallback replay debt.
🟢 GREEN measured, within budget · 🔴 RED measured, out of budget — a finding to clear before merge · ⚪ n/a not measurable on this machine, never guessed. No RED rows. Measured on Linux/x86_64 · measured |
# Conflicts: # src/brainlayer/supervisor_guard.py
A prior full calendar's guard can now outlive guard_timeout by up to MAX_STALL_GRACE_S of host-stall re-arms (this PR). The full-calendar admission deadline counted guard_timeout only, so the next invocation could give up with calendar-admission-timeout while the prior guard was still legitimately alive. Add MAX_STALL_GRACE_S to the bound. Found by the #1537 x #1536 combined-timing re-check (R-1537-codex.md). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 2071f1b0-c348-4bf1-9b89-6beafdcddb29) |
R-1536-delta-sol P2: MAX_STALL_GRACE_S capped the granted windows, not the elapsed extension. Each re-arm happens after an overshoot > 5 s that was never counted, so three 36 s overshoots at timeout=120 stretched the guard to 528 s against the calendar admission's 420 s allowance. The first stall of an incident now fixes stall_grace_end = missed deadline + MAX_STALL_GRACE_S; every re-arm deadline is min(now + timeout, end) and no re-arm happens at or past it. A real heartbeat clears the incident. grace_used_s now reports how far the new deadline sits past the missed one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 9bc6627f-ccc9-448c-a376-4a0061a9c8ae) |
|
Review record: Codex Sol r1 (CHANGES REQUESTED: per-grant stall accounting, fixed in 21312f4 + 07e9843) → r2 (CHANGES REQUESTED: readable-past-end and unbounded final wake latency). The lead check accepts r2 as a pre-existing, bounded run_guard ordering plus the existing finite admission-failure policy; it is tracked as a follow-up row. Full text: R-1536-r2-sol.md (lead check appended). Canon #9 cap reached; merging on green. |
Problem and scope
This is a backstop for a host-wide stall that freezes the guard too. It would NOT have stopped the 2026-10-28 04:50 kill: that guard's observed wait overshoot was only 1.2 seconds, below the 5-second stall threshold. The separate 120-second timeout PR (
fix/guard-timeout-120) is the primary fix for the 10-28 incident. Do not ship this PR alone as the incident fix.The shared daemon/calendar guard detects starvation when its own monotonic
selectwait exceeds the requested wait by more than 5 seconds. This avoids wall-clock adjustment false positives and coordination across guards. It does not detect sleep on platforms where monotonic time pauses.Each re-arm grants a full heartbeat timeout window, shortened only by the remaining cumulative grace allowance. Grace is capped at BOTH three re-arms AND 300 seconds of granted windows per incident, whichever is reached first. At timeout 120 seconds, the grants are 120, 120, then 60 seconds; at timeout 900 seconds, the single grant is 300 seconds. Host time during which the guard cannot run is not counted as a granted window. Only a real
Hresets both caps; registration traffic does not.EOF is consumed before considering stall grace: a dead parent's registered process groups are reaped without a re-arm or timeout kill. A responsive guard still kills a silent parent on its normal timeout, including operation-overrun heartbeat suppression. Shutdown (
Q) deadlines never receive grace. Heartbeat defaults and operation budgets are unchanged by this PR.guard-stall-rearmuses the existing bounded asynchronous diagnostics sink, recording observed wait overshoot, re-arm count, cumulative granted seconds, parent PID/birth and guard PID. Last-heartbeat age continues to describe a real heartbeat.Round 1 response
Claude cross-vendor review of
ab3e525e94dc551117923fa1584f456154f18d2d: VERDICT OK, with follow-ups in review comment. This is the one authorized follow-up round in the same PR:GUARD_TIMEOUT_*block alongside theseSTALL_*/MAX_STALL_*constants when rebasing.Loop-record: implementer=codex/gpt-6.1-sol/high reviewer=claude/unknown rounds=1/0 defects=2 role=worker
The round-1 verdict covers the prior head. The lead owns verification of this follow-up head and merge. Do not merge from this worker.
Validation
ruff check,ruff format --check, andgit diff --checkpassed.test_cumulative_stall_grace_is_capped_at_300_seconds(120/900),test_real_heartbeat_resets_cumulative_stall_grace,test_stall_coinciding_with_eof_reaps_without_rearm.40d5a576bf714ec149778d71945607d20fe89c09.size:M because the expanded regression coverage proves both grace bounds, reset and EOF behavior at the 120/900-second timeout limits.
CodeRabbit local review: SKIPPED — rate-limit exhaustion (exit 1), including the follow-up attempt; no paid retry. The original PR review was also rate-limited. Manual diff inspection and scoped tests supplement the lead-routed cross-vendor review, not replace it. Neither Bugbot nor Greptile was requested by this worker.
Evidence and handoff
Preserve the earlier superseded crash-sweep gate failure:
CHANGES_RESULT: cancelled, emptySWEEPS_REQUIRED, exit 1 before running a sweep. No manual retry was requested to erase that result. Current-head CI state is reported in the follow-up handoff; old-head green does not certify the new head.Worktree disposition: RETAINED by the W2 implementer for the open PR and lead review; T3 owns eventual retirement.
— brainlayerCodex (worker) · codex/gpt-6.1-sol
🤖 Generated with Claude Code.
Note
Re-arm
supervisor_guard.run_guarddeadlines on host stall instead of killingSTALL_OVERSHOOT_S(5s), the guard moves its deadline instead of killing the parent, with at most 3 re-arms and an absolute grace end 300s (MAX_STALL_GRACE_S) after the missed deadline.Hheartbeat resets the re-arm count and grace end; EOF, quiesced shutdown, and silent parents still get the normal timeout kill. Failed stall diagnostics do not disable enforcement.MAX_STALL_GRACE_Sso admission can outwait a prior guard that used its stall grace.run_guardsimulator in test_supervisor_guard_stalls.py and a calendar admission test in test_calendar_recovery_priority.py.supervisor_guard.run_guardstall handling and the re-arm constants insupervisor_guard.Macroscope summarized 07e9843.
Note
Medium Risk
Changes supervisor parent-kill timing and extends full-calendar admission waits by up to 300s; bounded by tests but affects critical maintenance custody paths.
Overview
When the supervisor guard’s
selectwait overshoots the requested timeout by more than 5 seconds, it now treats that as host starvation rather than a silent parent: it re-arms the heartbeat deadline (up to 3 times and within a 300s absolute grace window from the first miss), emitsguard-stall-rearmdiagnostics, and only then falls back to the existingguard-timeout/ parent kill. RealHheartbeats reset stall counters; EOF is handled before stall logic so dead parents still exit cleanly; quiesced (Q) guards never re-arm.Full calendar admission budgets in
_admit_calendarnow addMAX_STALL_GRACE_Sso a scheduled run can keep waiting for custody while a prior owner’s guard finishes its stall grace, avoiding prematurecalendar-admission-timeout.Adds deterministic
test_supervisor_guard_stalls.pycoverage for re-arm caps, grace accounting, heartbeat/registration/shutdown edge cases, plus a calendar priority test that admission succeeds when the prior guard “expires late.”Reviewed by Cursor Bugbot for commit 07e9843. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit