feat: active circuit breaker with safe pause & resume (closes #483)#588
feat: active circuit breaker with safe pause & resume (closes #483)#588SakethSumanBathini wants to merge 2 commits into
Conversation
|
@SakethSumanBathini is attempting to deploy a commit to the sreerevanth's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesCircuit Breaker Lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AgentSession
participant CircuitBreaker
participant RollbackEngine
AgentSession->>CircuitBreaker: observe AgentEvent
CircuitBreaker->>CircuitBreaker: evaluate thresholds
CircuitBreaker->>CircuitBreaker: transition CLOSED to OPEN
CircuitBreaker->>RollbackEngine: create checkpoint
CircuitBreaker->>CircuitBreaker: transition OPEN to PAUSED
CircuitBreaker->>RollbackEngine: restore checkpoint
CircuitBreaker->>CircuitBreaker: transition PAUSED to HALF_OPEN
AgentSession->>CircuitBreaker: observe probe event
CircuitBreaker->>CircuitBreaker: transition HALF_OPEN to CLOSED
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
🧪 PR Test Results
Python 3.12 · commit ffb621b |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agentwatch/circuit_breaker/breaker.py`:
- Around line 175-176: Update the observe() method to assign the observed
event’s event.step_number to self._last_step_number before or while accumulating
the event, so pause() can use the latest step when no explicit step_number is
provided.
- Around line 345-361: Capture the transition counters in local metadata before
invoking _reset_window() within the transition logic, then use those preserved
values when constructing the TransitionRecord. Ensure CLOSED transitions retain
the pre-reset total_tokens, total_errors, and hallucinations while other
transitions continue recording current values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 33a168be-949e-4747-9095-6d90ed6781fb
📒 Files selected for processing (4)
agentwatch/circuit_breaker/__init__.pyagentwatch/circuit_breaker/breaker.pyagentwatch/circuit_breaker/thresholds.pytests/test_circuit_breaker.py
…rack step_number in observe(); format (CodeRabbit sreerevanth#588)
Summary
Closes #483. AgentWatch currently blocks dangerous commands but has no way to pause a misbehaving agent, preserve its state, and resume it once the operator is confident it's safe to continue — a session that trips a safety check today either keeps running or is lost entirely. This PR adds an active circuit breaker: a state machine that watches agent execution in real time, trips on configurable thresholds (token budget, error rate, hallucination signals), and can safely pause an agent by checkpointing its state through the existing rollback engine — then resume it through a controlled recovery process rather than an all-or-nothing restart.
Why this design
Rather than building a new checkpointing or logging mechanism, this PR deliberately integrates with infrastructure AgentWatch already has, so the breaker adds one new coherent capability instead of a second, parallel system:
RollbackEngine.create_checkpoint/rollback()(agentwatch/rollback/engine.py) — the breaker doesn't reimplement snapshotting, it calls the engine that already does it well.EUAIActPackage.log_decision(agentwatch/governance/eu_ai_act.py) — every state transition is recorded as aDecisionLogEntry, the same audit primitive the rest of the governance package already uses for Article 12 documentation.AgentEvent(agentwatch/core/schema.py) — the breaker is framework-agnostic and never calls into a specific agent runtime; it consumes the same event stream every other AgentWatch module consumes.Both integrations are optional, dependency-injected parameters, not hard imports — a
CircuitBreakercan be constructed and used standalone (e.g. in tests, or in a deployment that hasn't wired up rollback/governance yet), and gracefully degrades: pausing without a rollback engine still transitions to PAUSED (logging a warning that no durable checkpoint exists), and a failing governance sink is caught and logged rather than breaking the breaker's control flow.State machine
CLOSED -> OPEN threshold breach (tokens / errors / hallucinations)
OPEN -> PAUSED pause(): checkpoint saved via RollbackEngine
PAUSED -> HALF_OPEN resume(): begin probing recovery, optionally restoring the checkpoint
OPEN -> HALF_OPEN resume() without an intervening pause (skip straight to probing)
HALF_OPEN -> CLOSED N consecutive successful probes (configurable)
HALF_OPEN -> OPEN a single failed probe re-opens immediately
any -> CLOSED manual reset() (operator override)
Illegal transitions (e.g. attempting
pause()from CLOSED, or a rawCLOSED -> PAUSEDjump) raiseRuntimeErrorrather than silently corrupting the breaker's state — enforced by an explicit allowed-transitions table, not ad-hocifchecks scattered through the code.What trips the breaker
Thresholds are evaluated over a rolling window that resets whenever the breaker (re)enters CLOSED:
max_total_tokens— cumulative tokens across observed events (readsAgentEvent.token_usage.total_tokens).max_consecutive_errors— errors/failures/timeouts in a row; a success resets the run.max_total_errors— errors in total within the window, even if interspersed with successes (catches a "flaky but not catastrophic" pattern the consecutive-error check alone would miss).max_hallucinations— events flagged viaConfidenceData.anomaly_flags, or whoseoverall_scorefalls belowmin_confidence.Each threshold can be disabled independently by setting it to
0(or0.0formin_confidence), andCircuitThresholdsvalidates its own configuration on construction (rejecting negative values, an out-of-range confidence bound, or a probe-success count of zero) so a misconfigured breaker fails loudly at setup time instead of silently never tripping or never recovering.Pause, resume, and recovery probing
pause(step_number=..., working_dir=..., memory_snapshot=...)— only valid from OPEN. Creates a checkpoint via the injectedRollbackEngine(labeledcircuit-breaker-pause-<session_id>for traceability) and transitions to PAUSED, recording the checkpoint id.resume(restore=False)— valid from PAUSED or OPEN. Moves to HALF_OPEN to begin probing recovery. Whenrestore=Trueand a checkpoint exists, the rollback engine restores it first — so an operator can choose "resume from where it paused" vs. "resume by rolling back to the last known-good checkpoint."half_open_probe_successesconsecutive successes close the circuit and reset all counters; a single failed probe re-opens immediately (the probe-success counter resets on any failure, so a partial run of successes doesn't "bank" progress across a failure).trip()/reset()— manual operator overrides (kill-switch and force-recover) for cases outside the automatic threshold logic.Compliance and observability
TransitionRecord(from-state, to-state, reason, session id, checkpoint id if any, and a snapshot of the counters at the moment of transition) — the breaker's own in-memory audit trail, retrievable via.history.DecisionLogEntry, withhuman_oversight_required=Truewhenever the breaker enters OPEN or PAUSED (the states where an agent should not be autonomously proceeding) — directly supporting the Article 12 record-keeping requirement in the issue's acceptance criteria.status()returns a JSON-serializable snapshot (state,is_tripped, all counters, probe progress, last checkpoint id, transition count) — the exact payload a dashboard needs to render circuit-breaker status, without the dashboard UI itself being part of this PR (see Scope below).Testing
40 tests, 100% line coverage of all three new files (
__init__.py,thresholds.py,breaker.py) — exceeding the issue's 95% target. Coverage breakdown by concern:CircuitThresholdsconstruction (negative values, out-of-range confidence, zero probe count) raises; defaults are valid.session_idare confirmed to be ignored rather than polluting counters.Nonecheckpoint id);pause()/resume()raise from invalid states;resume(restore=True)is confirmed to callrollback()with the correct checkpoint id.trip()/reset()from various starting states;trip()is a no-op when already OPEN (doesn't duplicate a transition record).TransitionRecord.to_dict()round-trips correctly;status()has the exact expected shape; EU AI Act logging is verified to produce the correctDecisionLogEntryfields, and a test with a deliberately broken governance sink confirms a logging exception is caught and does not prevent the state transition.Additionally verified:
ruff check— clean (fixed import ordering and an unused import that surfaced during development; suppressed one bandit false-positive,S105flagging theTOKEN_BUDGETenum value as a "hardcoded password," using this repo's existing# noqa / # nosecconvention).mypy— clean, no type errors.mainafter merging in the latest upstream changes. Confirms zero regressions across the entire codebase, not just the modules this PR touches directly.Scope note
This PR delivers: the full state machine, safe pause/resume with checkpointing, configurable thresholds, EU AI Act Article 12 record-keeping, and 100% test coverage — five of the issue's six acceptance criteria. The status dashboard UI is intentionally left as a focused follow-up PR:
status()already returns the complete payload a dashboard would consume, so the follow-up is purely a frontend rendering task with zero backend changes required. Keeping the dashboard separate keeps this PR's diff focused on the circuit-breaker logic itself, which is where the review attention matters most.Closes #483.
Summary by CodeRabbit