From 16eecce455b4c7d0e5b5ae50bce8c2bf0a7b443e Mon Sep 17 00:00:00 2001 From: t Date: Sat, 31 Oct 2026 17:14:23 -0700 Subject: [PATCH 1/4] fix(adapters): keep a latched usage-limit or account StopFailure's classification when the window dies (#959) --- CHANGELOG.md | 2 + src/bmad_loop/adapters/base.py | 17 +- src/bmad_loop/adapters/generic.py | 124 +++++++++++++-- tests/test_conftest.py | 2 +- tests/test_generic_tmux.py | 250 ++++++++++++++++++++++++++++++ tests/test_stories_e2e.py | 60 +++++++ 6 files changed, 437 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90675e150..84d1e1660 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -207,6 +207,8 @@ breaking changes may land in a minor release. - Name a displaced psmux registry that belongs to another project (or this one before a rename) as such in `cleanup` and the TUI, instead of calling its sessions `not migrated` (#906). +- Pause a Claude session uncharged, instead of charging a dev attempt for a crash, when + its window dies after a usage-limit or account StopFailure or during a quota wait (#959). ### Security diff --git a/src/bmad_loop/adapters/base.py b/src/bmad_loop/adapters/base.py index 9994227ff..fd5900216 100644 --- a/src/bmad_loop/adapters/base.py +++ b/src/bmad_loop/adapters/base.py @@ -335,8 +335,10 @@ class SessionResult: # wait loop WITHHELD the stall wake nudge (its Enter could answer the prompt, # #727) and ended the session `stalled`. Set only on that `stalled` exit, # from a latched parked hook event or a `parked_prompt_patterns` match on the - # visible pane; never on a `crashed` (window death outranks it) or completed - # result. Every engine/sweep site PAUSEs on it like `env_fault` (which + # visible pane; never on a completed result, and never on a `crashed` one + # (window death outranks it) except a result-less window death after a + # latched account StopFailure, which `wait_for_completion` labels parked + # with the same evidence the live exit carries (#959). Every engine/sweep site PAUSEs on it like `env_fault` (which # outranks it), so re-arm restores the attempt instead of a retry relaunching # into the same prompt. `parked_evidence` names what matched. APPENDED. parked: bool = False @@ -361,7 +363,10 @@ class SessionResult: # Usage-limit evidence (#610): the line a `usage_limit_patterns` / # `usage_wait_patterns` match found LIVE. Two sources: the tmux adapters' # visible pane — at stall-grace expiry (ending the session stalled + parked) - # or the instant the session timeout fired, never on a `crashed` result — and + # or the instant the session timeout fired, never on a `crashed` result + # except a result-less window death after a latched blocked or wait usage + # StopFailure or during an open quota wait, which `wait_for_completion` + # labels with that latched evidence (#959) — and # opencode-http's provider-error line, formatted from a typed SSE frame for # this session while it ran (DW-592), carried onto a `timeout`, `stalled` or # `crashed` result its wait returns (ENV_FAULT_STATUSES, the verdicts @@ -398,6 +403,12 @@ class SessionResult: # `wait_for_completion` returns; journaled on `session-end` present-only. # APPENDED. stop_failure: str | None = None + # The `[hooks.stop_failure_types]` action the profile maps `stop_failure`'s + # error to (`account`, `rate_limit`, `transient`, `ignore`), None when the + # error is unmapped or there is no uncleared StopFailure. Stamped with + # `stop_failure` on every result `wait_for_completion` returns. Diagnostic, + # plus #959 usage-relaunch routing. APPENDED. + stop_failure_action: str | None = None class CodingCLIAdapter(ABC): diff --git a/src/bmad_loop/adapters/generic.py b/src/bmad_loop/adapters/generic.py index 00920fe28..19944e544 100644 --- a/src/bmad_loop/adapters/generic.py +++ b/src/bmad_loop/adapters/generic.py @@ -254,10 +254,29 @@ class _StopFailureLatch: ``(evidence, matched_line)``: the quota-wait admission takes it as the wait line when the pane shows none (a backend without ``capture_pane`` never does). A ``Stop`` clears both: a turn that ended proves the failure was - recovered. `wait_hit` also clears when a quota wait admits or ends.""" + recovered. `wait_hit` also clears when a quota wait admits or ends. + + `kind` classifies the latest StopFailure for a window that dies after it + (#959): ``"account"``, ``"blocked"`` or ``"wait"`` (None for an unmapped, + transient, ignored or unmatched error). `evidence` is the label the live + exit would carry and `resets` its parsed reset (`UsageReset.value`), so the + funnel can relabel a result-less ``crashed`` exit as the same uncharged + pause. `baseline` is the transcript's (mtime_ns, size) stat'ed fresh at the + latch (None when none was named or the stat failed; the next heartbeat + sample then adopts one). A Stop clears all four, and so does a heartbeat + sample whose key differs from `baseline`: a CLI that wrote its transcript + after the failure recovered by itself.""" error: str | None = None wait_hit: tuple[str, str] | None = None + kind: str | None = None + evidence: str | None = None + resets: str | None = None + baseline: tuple[int, int] | None = None + + def clear_kind(self) -> None: + self.kind = self.evidence = self.resets = None + self.baseline = None @dataclass @@ -1457,8 +1476,14 @@ def wait_for_completion(self, handle: SessionHandle, spec: SessionSpec) -> Sessi result = self._wait_loop(handle, spec, quota, notice, stop_failure) if stop_failure.error is not None: # The one funnel every wait-loop exit passes (#865): a StopFailure no - # later Stop cleared rides out on the result, whatever ended it. - result = dataclasses.replace(result, stop_failure=stop_failure.error) + # later Stop cleared rides out on the result, whatever ended it, + # with its mapped action. + result = dataclasses.replace( + result, + stop_failure=stop_failure.error, + stop_failure_action=self._stop_failure_types.get(stop_failure.error), + ) + result = self._relabel_dead_window(handle, result, quota, stop_failure) if quota.waiting: self._end_quota_wait(handle, quota, "session-exit") if notice.open: @@ -1467,6 +1492,46 @@ def wait_for_completion(self, handle: SessionHandle, spec: SessionSpec) -> Sessi result = dataclasses.replace(result, quota_wait_s=round(quota.closed_s, 3)) return result + def _relabel_dead_window( + self, + handle: SessionHandle, + result: SessionResult, + quota: _QuotaWait, + stop_failure: _StopFailureLatch, + ) -> SessionResult: + """A window that died while the session waited on a usage limit or an + operator keeps that classification (#959): a result-less ``crashed`` + exit after a latched account StopFailure is ``parked``, after a + blocked or wait one ``usage_limited``, and during a pane-only quota + wait ``usage_limited`` with the wait class. The status stays + ``crashed``; only labels are added, so routing gives the same + uncharged pause as the live exit instead of charging a crash. A crash + whose read-back found a result is never relabeled: the result wins.""" + if result.status != "crashed" or result.result_json is not None: + return result + kind = stop_failure.kind + if kind in ("blocked", "wait"): + result = dataclasses.replace( + result, + usage_limited=stop_failure.evidence, + usage_limit_class=USAGE_LIMIT_BLOCKED if kind == "blocked" else USAGE_LIMIT_WAIT, + usage_limit_resets=stop_failure.resets, + ) + elif kind == "account": + result = dataclasses.replace(result, parked=True, parked_evidence=stop_failure.evidence) + elif quota.waiting and quota.evidence is not None: + kind = "quota-wait" + result = dataclasses.replace( + result, + usage_limited=quota.evidence, + usage_limit_class=USAGE_LIMIT_WAIT, + usage_limit_resets=quota.resets, + ) + else: + return result + self._note_lifecycle(handle.task_id, "crash-relabeled", kind=kind, error=stop_failure.error) + return result + def _wait_loop( self, handle: SessionHandle, @@ -1770,6 +1835,16 @@ def still_working() -> bool: return True return self._quota_wait_tool_open(handle, path) + def latch_kind(kind: str, evidence: str, resets: str | None) -> None: + """Classify the StopFailure just latched (#959), with a fresh + transcript stat as the activity baseline that clears it.""" + stop_failure.kind = kind + stop_failure.evidence = evidence + stop_failure.resets = resets + stop_failure.baseline = ( + self._transcript_activity_key(transcript_path) if transcript_path else None + ) + def stall_exit( parked_now: str | None, usage_now: str | None, @@ -1777,9 +1852,11 @@ def stall_exit( class_now: str | None = None, ) -> SessionResult: """The non-completion exit of a stalled session (DW-348, #610): - resolve, then re-probe liveness. A now-dead window is ``crashed`` - (never parked or usage-limited: nothing waits on a human once the CLI - is gone). Otherwise ``stalled`` with ``accept_result=False``: an + resolve, then re-probe liveness. A now-dead window is ``crashed``, + unlabeled here: nothing waits on a human once the CLI is gone. The + funnel (`_relabel_dead_window`) labels it parked or usage-limited + when a latched StopFailure or an open quota wait classified it + (#959). Otherwise ``stalled`` with ``accept_result=False``: an artifact on disk cannot upgrade the stall to completed (it may be stale or mid-write; only a Stop or window death vouches for it), and ``parked_now`` / ``usage_now`` / ``resets_now`` / ``class_now`` @@ -2017,6 +2094,21 @@ def stall_exit( parked_key is None or idle.last_key != parked_key ): parked_evidence = parked_kind = parked_key = None + if stop_failure.kind is not None and sampled: + # A transcript write after the StopFailure is a CLI that + # recovered by itself (#959): a window that dies later is + # no longer that failure's. Only a stat taken this tick + # counts, as for the parked notice above. + if stop_failure.baseline is None: + stop_failure.baseline = idle.last_key + elif idle.last_key != stop_failure.baseline: + self._note_lifecycle( + handle.task_id, + "stop-failure-kind-cleared", + error=stop_failure.error, + stop_failure_kind=stop_failure.kind, + ) + stop_failure.clear_kind() if quota.waiting: # The transcript moved: the CLI continued after the reset. # The timeout resumes from the remaining time frozen at @@ -2681,6 +2773,9 @@ def stall_exit( action = self._stop_failure_types.get(error) message = event.stop_message stop_failure.error = error + # The latest StopFailure owns the classification (#959): an + # unmapped, transient, ignored or unmatched one leaves none. + stop_failure.clear_kind() self._note_lifecycle( handle.task_id, "stop-failure", @@ -2765,7 +2860,9 @@ def stall_exit( said = event.stop_error_details else: said = "no message" - return stall_exit(f"StopFailure({error}): {said}", None) + evidence = f"StopFailure({error}): {said}" + latch_kind("account", evidence, None) + return stall_exit(evidence, None) if action == "rate_limit": # A 429 and a subscription limit share `rate_limit`: the # message decides, against the profile's usage lists. @@ -2776,16 +2873,14 @@ def stall_exit( ) continue evidence, line, usage_class = hit + reset = self._usage_reset(handle, line, _local_now()) + resets = reset.value if reset is not None else None if usage_class == USAGE_LIMIT_BLOCKED: - reset = self._usage_reset(handle, line, _local_now()) - return stall_exit( - evidence, - evidence, - reset.value if reset is not None else None, - USAGE_LIMIT_BLOCKED, - ) + latch_kind("blocked", evidence, resets) + return stall_exit(evidence, evidence, resets, USAGE_LIMIT_BLOCKED) # A wait limit: the CLI continues by itself after the # reset, so the quota wait may admit on this line. + latch_kind("wait", evidence, resets) stop_failure.wait_hit = (evidence, line) self._note_lifecycle( handle.task_id, "stop-failure-quota-wait-pending", evidence=evidence @@ -2819,6 +2914,7 @@ def stall_exit( # recovered from (#865). stop_failure.error = None stop_failure.wait_hit = None + stop_failure.clear_kind() if notice.open: self._note_session_unparked(handle, notice, "stop") # A turn ENDED — the one canonical event that proves the CLI did diff --git a/tests/test_conftest.py b/tests/test_conftest.py index 3e10b094d..694a85e65 100644 --- a/tests/test_conftest.py +++ b/tests/test_conftest.py @@ -1685,7 +1685,7 @@ def _declares_loadgroup(addopts: object) -> bool: "test_tmux_locale_e2e.py": {"test_e2e_": 1}, "test_launch_probe.py": {"test_e2e_": 1}, "test_stories_e2e.py": { - "test_e2e_": 19, + "test_e2e_": 20, "test_reap_e2e_": 3, # Not asserted, and deliberately so — see the residual note above. Today these are # the local-process identity harness defs: `test_detach_gate_*`, diff --git a/tests/test_generic_tmux.py b/tests/test_generic_tmux.py index f6e018974..90ab226dd 100644 --- a/tests/test_generic_tmux.py +++ b/tests/test_generic_tmux.py @@ -15056,6 +15056,256 @@ def test_unmatched_rate_limit_stop_failure_keeps_the_stall_timing(tmp_path, monk assert result.stop_failure == "rate_limit" +def _decide_dev(result, attempt=1): + from bmad_loop.escalation import decide_dev + from bmad_loop.model import StoryTask + + return decide_dev( + StoryTask(story_key="3-1", epic=3, attempt=attempt), + result, + None, + Policy(limits=LimitsPolicy()), + ) + + +def _dies_at(q, call): + """Kill `q`'s window from wait call `call` on (a `_drive_quota` script).""" + alive = {"v": True} + q.adapter._window_alive = lambda handle: alive["v"] + + def script(call_n): + if call_n >= call: + alive["v"] = False + + return script + + +def test_account_stop_failure_then_window_death_is_parked_not_charged(tmp_path, monkeypatch): + """#959: the user's own StopFailure hook kills the window, so the stall + re-probe finds it dead. The latched account error still labels the crash + parked, with the live exit's evidence, and the dev decider pauses it + uncharged instead of retrying. + + Ablation: drop the account arm of `_relabel_dead_window` and this fails on + `parked is False` (and a RETRY).""" + adapter, mux = _parked_adapter( + tmp_path, + monkeypatch, + [_stop_failure_event("billing_error", "Credit balance is too low")], + alive=lambda handle: False, + ) + result = adapter.wait_for_completion(_dev_handle(), _dev_spec(tmp_path)) + assert (result.status, result.parked) == ("crashed", True) + assert result.parked_evidence == "StopFailure(billing_error): Credit balance is too low" + assert result.usage_limited is None + assert result.stop_failure == "billing_error" + assert result.stop_failure_action == "account" + assert result.stop_seen is False + assert mux.sent == [] + (crumb,) = _lifecycle_events(adapter, "crash-relabeled") + assert (crumb["kind"], crumb["error"]) == ("account", "billing_error") + from bmad_loop.escalation import Action + + decision = _decide_dev(result) + assert decision.action == Action.PAUSE + assert decision.reason.startswith("parked: dev session crashed (StopFailure(billing_error)") + + +def test_blocked_rate_limit_stop_failure_then_window_death_is_usage_limited(tmp_path, monkeypatch): + """#959: a blocked usage StopFailure, then a dead window at the stall + re-probe: the crash carries the usage-limit labels, so the env-fault + classifier's usage shortcut stamps it and the decider pauses uncharged. + + Ablation: drop the blocked/wait arm of `_relabel_dead_window` and this fails + on `usage_limited is None` (and a RETRY).""" + adapter, _ = _parked_adapter( + tmp_path, + monkeypatch, + [_stop_failure_event("rate_limit", USAGE_CREDITS_LINE)], + alive=lambda handle: False, + ) + spec = _dev_spec(tmp_path) + result = adapter.wait_for_completion(_dev_handle(), spec) + pattern = adapter.profile.usage_limit_patterns[0] + evidence = f"StopFailure(rate_limit) message matched {pattern!r}: {USAGE_CREDITS_LINE.strip()}" + assert (result.status, result.parked) == ("crashed", False) + assert result.usage_limited == evidence + assert result.usage_limit_class == USAGE_LIMIT_BLOCKED + assert result.usage_limit_resets is None + assert result.stop_failure_action == "rate_limit" + result = adapter._classify_env_fault(_dev_handle(), spec, result) + assert result.env_fault is True + from bmad_loop.escalation import Action + + decision = _decide_dev(result) + assert decision.action == Action.PAUSE + assert decision.usage_limit is not None + assert decision.usage_limit.limit_class == USAGE_LIMIT_BLOCKED + + +def test_wait_rate_limit_stop_failure_then_window_death_is_usage_limited(tmp_path, monkeypatch): + """#959: a wait-class usage StopFailure latches its reset; the window dies + before any quota wait admits. The crash is usage-limited WAIT with the + reset carried. + + Ablation: drop the blocked/wait arm of `_relabel_dead_window` and this fails + on `usage_limited is None`.""" + q = _quota_adapter(tmp_path, monkeypatch, screen="") + result = _drive_quota( + q, + _quota_spec(tmp_path), + events={2: _stop_failure_event("rate_limit", USAGE_SESSION_LINE)}, + script=_dies_at(q, 3), + ) + pattern = q.adapter.profile.usage_wait_patterns[0] + evidence = f"StopFailure(rate_limit) message matched {pattern!r}: {USAGE_SESSION_LINE.strip()}" + assert _lifecycle_events(q.adapter, "quota-wait-started") == [] + assert (result.status, result.parked) == ("crashed", False) + assert result.usage_limited == evidence + assert result.usage_limit_class == USAGE_LIMIT_WAIT + assert result.usage_limit_resets == QUOTA_RESETS + (crumb,) = _lifecycle_events(q.adapter, "crash-relabeled") + assert (crumb["kind"], crumb["error"]) == ("wait", "rate_limit") + result = q.adapter._classify_env_fault(_dev_handle(), _quota_spec(tmp_path), result) + from bmad_loop.escalation import Action + + decision = _decide_dev(result) + assert decision.action == Action.PAUSE + assert decision.usage_limit is not None + + +def test_pane_quota_wait_then_window_death_is_usage_limited(tmp_path, monkeypatch): + """#959: no StopFailure (a pre-2.1.78 claude, or another CLI); the pane's + wait line opened a quota wait and the window died inside it. The crash + carries the wait's evidence and reset as a WAIT usage limit. + + Ablation: drop the quota-wait arm of `_relabel_dead_window` and this fails + on `usage_limited is None`.""" + q = _quota_adapter(tmp_path, monkeypatch) + result = _drive_quota(q, _quota_spec(tmp_path), script=_dies_at(q, 8)) + assert len(_lifecycle_events(q.adapter, "quota-wait-started")) == 1 + assert result.status == "crashed" + assert result.usage_limited == q.wait_evidence() + assert result.usage_limit_class == USAGE_LIMIT_WAIT + assert result.usage_limit_resets == QUOTA_RESETS + assert result.stop_failure is None + assert result.stop_failure_action is None + (crumb,) = _lifecycle_events(q.adapter, "crash-relabeled") + assert (crumb["kind"], crumb["error"]) == ("quota-wait", None) + # The open wait is still closed for the journal on the way out. + assert q.quota_kinds() == ["quota-wait-started", "quota-wait-ended"] + + +def test_unmatched_rate_limit_then_window_death_stays_charged(tmp_path, monkeypatch): + """An ordinary 429 (`rate_limit` matching neither usage list) leaves no + classification: the dead window is a plain crash, charged as today.""" + q = _quota_adapter(tmp_path, monkeypatch, screen="") + result = _drive_quota( + q, + _quota_spec(tmp_path), + events={2: _stop_failure_event("rate_limit", "API Error: 429 Too Many Requests")}, + script=_dies_at(q, 3), + ) + assert result.status == "crashed" + assert (result.usage_limited, result.parked) == (None, False) + assert result.stop_failure == "rate_limit" + assert result.stop_failure_action == "rate_limit" + assert _lifecycle_events(q.adapter, "crash-relabeled") == [] + from bmad_loop.escalation import Action + + assert _decide_dev(result).action == Action.RETRY + + +def test_a_stop_clears_the_stop_failure_kind(tmp_path, monkeypatch): + """A Stop after a wait-class StopFailure proves the turn recovered: the + window dying afterwards is a plain crash. (An account StopFailure exits at + once, so no Stop can follow it.)""" + q = _quota_adapter(tmp_path, monkeypatch, screen="") + result = _drive_quota( + q, + _quota_spec(tmp_path), + events={ + 2: _stop_failure_event("rate_limit", USAGE_SESSION_LINE), + 3: _stop_event("3-1-dev-1", "sess", str(q.transcript)), + }, + script=_dies_at(q, 4), + ) + assert result.status == "crashed" + assert (result.usage_limited, result.parked) == (None, False) + assert (result.stop_failure, result.stop_failure_action) == (None, None) + assert _lifecycle_events(q.adapter, "crash-relabeled") == [] + + +def test_transcript_activity_clears_a_wait_kind(tmp_path, monkeypatch): + """A transcript write after a wait-class StopFailure is a CLI that went on + by itself: the classification clears (crumbed), and a later window death is + a plain crash. The error itself stays latched. + + Ablation: drop the `stop-failure-kind-cleared` clear and this fails on a + usage-limited result.""" + q = _quota_adapter(tmp_path, monkeypatch, screen="") + dies = _dies_at(q, 5) + + def script(call_n): + if call_n == 3: + _grow(q.transcript, _assistant_line()) + dies(call_n) + + result = _drive_quota( + q, + _quota_spec(tmp_path), + events={2: _stop_failure_event("rate_limit", USAGE_SESSION_LINE)}, + script=script, + ) + (cleared,) = _lifecycle_events(q.adapter, "stop-failure-kind-cleared") + assert (cleared["error"], cleared["stop_failure_kind"]) == ("rate_limit", "wait") + assert result.status == "crashed" + assert (result.usage_limited, result.parked) == (None, False) + assert result.stop_failure == "rate_limit" + assert _lifecycle_events(q.adapter, "crash-relabeled") == [] + + +def test_crash_with_a_result_json_is_not_relabeled(tmp_path, monkeypatch): + """The crash-path read-back still wins: a window that dies after a latched + wait-class StopFailure but with the terminal spec on disk completes, with + no usage-limit or parked label.""" + q = _quota_adapter(tmp_path, monkeypatch, screen="") + dies = _dies_at(q, 3) + + def script(call_n): + if call_n == 3: + _grow(q.log, b"worked on the story\n" * 200) # the #261 proof of work + q.finish_spec() + dies(call_n) + + result = _drive_quota( + q, + _quota_spec(tmp_path), + events={2: _stop_failure_event("rate_limit", USAGE_SESSION_LINE)}, + script=script, + ) + assert result.status == "completed" + assert result.result_json is not None + assert (result.usage_limited, result.parked, result.usage_limit_class) == (None, False, None) + assert _lifecycle_events(q.adapter, "crash-relabeled") == [] + + +@pytest.mark.parametrize("alive", [True, False]) +def test_stop_failure_action_is_stamped_on_every_exit(tmp_path, monkeypatch, alive): + """The mapped action rides out with the error on the live (stalled) exit + and the dead-window (crashed) one alike.""" + adapter, _ = _parked_adapter( + tmp_path, + monkeypatch, + [_stop_failure_event("authentication_failed", "Invalid API key")], + alive=lambda handle: alive, + ) + result = adapter.wait_for_completion(_dev_handle(), _dev_spec(tmp_path)) + assert result.status == ("stalled" if alive else "crashed") + assert result.parked is True + assert (result.stop_failure, result.stop_failure_action) == ("authentication_failed", "account") + + def test_rate_limit_stop_failure_match_timeout_is_crumbed(tmp_path, monkeypatch): adapter, _ = _parked_adapter( tmp_path, monkeypatch, [_stop_failure_event("rate_limit", USAGE_CREDITS_LINE)] diff --git a/tests/test_stories_e2e.py b/tests/test_stories_e2e.py index 0f0ec7f6e..8329e9b2b 100644 --- a/tests/test_stories_e2e.py +++ b/tests/test_stories_e2e.py @@ -333,6 +333,11 @@ """ assert TIMEOUT_FAKE_CLI.count(_PARENT_START) == 1 STOP_FAILURE_FAKE_CLI = TIMEOUT_FAKE_CLI.replace(_PARENT_START, _PARENT_START + _STOP_FAILURE_EVENT) +# #959: the CLI's window dies right after the StopFailure (as when an operator's own +# StopFailure hook kills it to rotate accounts). The watcher's 1 s poll reads the +# event only after the window is gone, so the stall re-probe finds it dead. +assert STOP_FAILURE_FAKE_CLI.count("sleep 30\nexit 0\n") == 1 +STOP_FAILURE_DIES_FAKE_CLI = STOP_FAILURE_FAKE_CLI.replace("sleep 30\nexit 0\n", "exit 0\n") # PROFILE_TOML with the canonical StopFailure mapped and one account error, as # claude's own profile maps it. @@ -2067,6 +2072,61 @@ def test_e2e_stop_failure_account_error_pauses_uncharged( ) +def test_e2e_stop_failure_then_window_death_pauses_uncharged( + tmp_path, monkeypatch, private_tmux_server +): + """#959 end to end through the real binary + real tmux, zero tokens: the CLI + fires `StopFailure(billing_error)` and its window dies at once. The session + ends `crashed`, but the latched account error labels it parked, so the engine + PAUSEs uncharged instead of retrying: two dev attempts are allowed and exactly + one session ran.""" + root = tmp_path / "sbx" + story = "1-1-stopfail-dies" + _scaffold_sprint( + root, + story, + fake_cli=STOP_FAILURE_DIES_FAKE_CLI, + extra_policy="\n[limits]\nmax_dev_attempts = 2\nteardown_grace_s = 5\n", + profile_toml=STOP_FAILURE_PROFILE_TOML, + ) + # inherited by the `bmad-loop run` subprocess (_run passes no env=) + monkeypatch.setenv("BMAD_LOOP_SESSION_TIMEOUT_S", "120") + run_id: str | None = None + try: + proc = _run(root, "run", timeout=90) + assert proc.returncode == 0, proc.stderr or proc.stdout + run_id = _run_id(root) + run_dir = root / ".bmad-loop" / "runs" / run_id + + state = json.loads((run_dir / "state.json").read_text(encoding="utf-8")) + assert state.get("paused_stage") == "escalation", state + reason = str(state.get("paused_reason", "")) + assert reason.startswith("parked: dev session crashed (StopFailure(billing_error)"), reason + + journal = [ + json.loads(ln) + for ln in (run_dir / "journal.jsonl").read_text(encoding="utf-8").splitlines() + if ln.strip() + ] + ends = [j for j in journal if j["kind"] == "session-end"] + assert len(ends) == 1, [j["kind"] for j in journal] + [end] = ends + assert end["status"] == "crashed", end + assert end["parked"] is True, end + assert end["stop_failure"] == "billing_error", end + decisions = [j for j in journal if j["kind"] == "dev-decision"] + assert [d["action"] for d in decisions] == ["pause"], decisions + assert state["tasks"][story]["attempt"] == 1, state["tasks"][story] + finally: + # The paused run left its session for a resume that never comes. + if run_id is not None: + subprocess.run( + ["tmux", "-S", str(private_tmux_server), "kill-session"] + + ["-t", f"=bmad-loop-{run_id}"], + capture_output=True, + ) + + @pytest.mark.parametrize( "force_live_reap_assertion", [False, True], ids=["reaped", "live-assertion"] ) From 5934ebce5d66ab2a3b6234de387492cf892e6ec8 Mon Sep 17 00:00:00 2001 From: t Date: Sat, 31 Oct 2026 17:27:59 -0700 Subject: [PATCH 2/4] feat(engine): opt-in uncharged relaunch for usage-limited sessions, capped per story (#959) --- docs/reference/configuration.md | 2 + src/bmad_loop/adapters/base.py | 9 + src/bmad_loop/data/settings/core.toml | 13 ++ src/bmad_loop/engine.py | 116 ++++++++++- src/bmad_loop/escalation.py | 59 +++++- src/bmad_loop/model.py | 33 ++++ src/bmad_loop/policy.py | 31 +++ src/bmad_loop/runs.py | 3 + src/bmad_loop/sweep.py | 5 + tests/test_engine.py | 273 ++++++++++++++++++++++++++ tests/test_escalation.py | 137 +++++++++++++ tests/test_model.py | 29 +++ tests/test_policy.py | 31 +++ tests/test_portability_guard.py | 5 + 14 files changed, 735 insertions(+), 11 deletions(-) diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index ccca5365c..e6b772cfc 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -103,6 +103,8 @@ Cycle/attempt caps, timeout & token budget. | `session_budget_mode` | string | `"warn"` | `enforce` / `off` / `warn` | mid-session guard on the weighted per-session spend, sampled every ~30s · off: no sampling · warn: one ATTENTION + breadcrumb · enforce: wrap-up nudge then over_budget termination (retry→defer) | | `max_tokens_per_session` | integer | `4000000` | ≥ 1 | weighted (cache-discounted) token cap a single session may spend before the budget guard trips; healthy sessions run ~1-2.5M weighted, so the default trips only true runaways | | `session_budget_grace_s` | integer | `240` | ≥ 0 | enforce mode: seconds a tripped session gets to wrap up after the nudge before it is terminated over_budget · 0 = terminate at trip, no nudge | +| `usage_limit_action` | string | `"pause"` | `pause` / `relaunch` | what a session that ends on a usage limit or an account StopFailure does · pause: the uncharged usage-limit pause `resolve` clears · relaunch: relaunch it at once as an uncharged retry (BMAD_LOOP_USAGE_RELAUNCH=<n> in the session env, for account rotation), capped by max_usage_relaunches | +| `max_usage_relaunches` | integer | `3` | ≥ 0 | relaunch mode: uncharged usage relaunches per story (dev, fix and review together) · at the cap the session takes the usage-limit pause instead · 0 = always pause | ### `[verify]` diff --git a/src/bmad_loop/adapters/base.py b/src/bmad_loop/adapters/base.py index fd5900216..c6235940f 100644 --- a/src/bmad_loop/adapters/base.py +++ b/src/bmad_loop/adapters/base.py @@ -252,6 +252,15 @@ class SessionSpec: # keep the plain timeout. The engine sets limits.quota_wait_max_min * 60 for # every session it drives. Kept LAST so positional constructions stay valid. quota_wait_max_s: float = 0.0 + # Usage relaunch (#959, `limits.usage_limit_action = "relaunch"`), set by the + # engine on primary story sessions (dev, fix, review) only; False on every + # other session and on constructions outside the engine. + # `usage_relaunch_mode`: a usage limit or account StopFailure relaunches this + # session. `usage_relaunch_left`: relaunch mode with relaunch budget left, so + # the session should end at once on a limit rather than wait it out. + # Appended after `quota_wait_max_s` so positional constructions stay valid. + usage_relaunch_mode: bool = False + usage_relaunch_left: bool = False @dataclass(frozen=True) diff --git a/src/bmad_loop/data/settings/core.toml b/src/bmad_loop/data/settings/core.toml index c1f272a4b..f5002be1a 100644 --- a/src/bmad_loop/data/settings/core.toml +++ b/src/bmad_loop/data/settings/core.toml @@ -227,6 +227,19 @@ kind = "int" minimum = 0 default_ref = "LimitsPolicy.session_budget_grace_s" description = "enforce mode: seconds a tripped session gets to wrap up after the nudge before it is terminated over_budget · 0 = terminate at trip, no nudge" +[[section.field]] +key = "usage_limit_action" +kind = "select" +options_ref = "USAGE_LIMIT_ACTIONS" +default_ref = "LimitsPolicy.usage_limit_action" +label = "usage limit action" +description = "what a session that ends on a usage limit or an account StopFailure does · pause: the uncharged usage-limit pause `resolve` clears · relaunch: relaunch it at once as an uncharged retry (BMAD_LOOP_USAGE_RELAUNCH= in the session env, for account rotation), capped by max_usage_relaunches" +[[section.field]] +key = "max_usage_relaunches" +kind = "int" +minimum = 0 +default_ref = "LimitsPolicy.max_usage_relaunches" +description = "relaunch mode: uncharged usage relaunches per story (dev, fix and review together) · at the cap the session takes the usage-limit pause instead · 0 = always pause" [[section]] name = "verify" diff --git a/src/bmad_loop/engine.py b/src/bmad_loop/engine.py index 31df3871d..0e4461369 100644 --- a/src/bmad_loop/engine.py +++ b/src/bmad_loop/engine.py @@ -60,6 +60,7 @@ session_failure_reason, unresolved_escalation_keys, usage_limit_pause, + usage_relaunch, ) from .frontmatter import FrontmatterWriteError from .install import dev_primitive_or_default @@ -2693,6 +2694,7 @@ def _finish_inflight(self) -> None: task.reverify_replayed = "" task.generation += 1 task.review_cycle = 0 + task.review_usage_relaunches = 0 # refunds never exceed review_cycle (#959) task.followup_reviews_spent = 0 task.salvage_refile_pending = False self._save() @@ -3714,6 +3716,10 @@ def _dev_phase(self, task: StoryTask, resume_result: SessionResult | None = None result_json=result.result_json, already_env_fault=env_fault, ) + if decision.uncharged: + # An uncharged usage relaunch (#959): refund this attempt before + # the journal and the save below, so both record the same count. + task.dev_usage_relaunches += 1 self._journal_dev_decision(task, result, decision, env_fault=env_fault) if decision.action == Action.PROCEED: # DEV_VERIFY + spec_file is not itself proof of acceptance: this @@ -3745,10 +3751,17 @@ def _dev_phase(self, task: StoryTask, resume_result: SessionResult | None = None # `task.attempt` are known-good for this decision, and it is before # `_rollback_or_pause` can raise `RunPaused` and skip the notice for # exactly the attempt whose loss most needs announcing. + if decision.uncharged: + title = ( + f"dev relaunch: {task.story_key} (usage limit, relaunch " + f"{task.usage_relaunches}/{self.policy.limits.max_usage_relaunches})" + ) + else: + title = f"dev retry: {task.story_key} (attempt {task.attempt})" gates.notify( self.policy, self.run_dir, - f"dev retry: {task.story_key} (attempt {task.attempt})", + title, decision.reason.strip() or "dev attempt rejected with no reason recorded", ) if outcome is not None and outcome.fixable: @@ -3873,6 +3886,10 @@ def _journal_dev_decision( # non-completed result to the parked PAUSE. parked=result.parked, parked_evidence=result.parked_evidence, + # An uncharged usage relaunch (#959) and the story's running + # count of them (dev and review together). + uncharged=decision.uncharged, + usage_relaunches=task.usage_relaunches, ) else: self.journal.append( @@ -3887,6 +3904,8 @@ def _journal_dev_decision( produced_work=result.produced_work, parked=result.parked, parked_evidence=result.parked_evidence, + uncharged=decision.uncharged, + usage_relaunches=task.usage_relaunches, env_fault_site=decision.env_site, ) @@ -4003,7 +4022,12 @@ def _review_and_commit( # replay branch below skips the re-increment, so no extra budget is # burned. resume_result is nulled after it is consumed, so every later # iteration falls back to the normal budget guard. - while resume_result is not None or task.review_cycle < self.policy.limits.max_review_cycles: + # The budget counts charged cycles only: an uncharged usage relaunch + # (#959) bumps `review_cycle` for session identity and refunds it. + while ( + resume_result is not None + or task.charged_review_cycles < self.policy.limits.max_review_cycles + ): if resume_result is None: # DW-523: probe before the cycle is charged; a pause here resumes # through the DEV_VERIFY or completed-pass replay arms. @@ -4055,6 +4079,19 @@ def _review_and_commit( result_json=result.result_json, already_env_fault=result.env_fault, ) + if decision.uncharged: + # An uncharged usage relaunch (#959): refund this cycle before the + # journal, and persist it, so the next pass's budget check and + # state.json agree. + task.review_usage_relaunches += 1 + self._save() + gates.notify( + self.policy, + self.run_dir, + f"review relaunch: {task.story_key} (usage limit, relaunch " + f"{task.usage_relaunches}/{self.policy.limits.max_usage_relaunches})", + decision.reason, + ) if decision.action != Action.SALVAGE and task.salvage_refile_pending: task.salvage_refile_pending = False self._save() @@ -4065,7 +4102,11 @@ def _review_and_commit( return if decision.action == Action.RETRY: self.journal.append( - "review-retry", story_key=task.story_key, reason=decision.reason + "review-retry", + story_key=task.story_key, + reason=decision.reason, + uncharged=decision.uncharged, + usage_relaunches=task.usage_relaunches, ) continue if decision.action == Action.SALVAGE: @@ -4092,7 +4133,11 @@ def _review_and_commit( self._defer(task, fallback.reason) return self.journal.append( - "review-retry", story_key=task.story_key, reason=fallback.reason + "review-retry", + story_key=task.story_key, + reason=fallback.reason, + uncharged=fallback.uncharged, + usage_relaunches=task.usage_relaunches, ) continue @@ -4240,7 +4285,10 @@ def _review_and_commit( ) if gated.action == Action.PAUSE: self._escalate_decision(task, gated) - if outcome.fixable and task.review_cycle < self.policy.limits.max_review_cycles: + if ( + outcome.fixable + and task.charged_review_cycles < self.policy.limits.max_review_cycles + ): # failing verify commands are dev work, not review work: a # re-review of the same tree cannot make them pass. Repair # with the failing output as feedback, then re-review. This @@ -6997,7 +7045,10 @@ def _env_gate_decision( return self._env_gate_claim(task, decision, claim, role=role) if not self.policy.environment.probes: return decision - charges = decision.action in (Action.RETRY, Action.DEFER) or decision.budget_exhausted + # An uncharged usage relaunch (#959) charges nothing, so it never probes. + charges = ( + decision.action in (Action.RETRY, Action.DEFER) and not decision.uncharged + ) or decision.budget_exhausted if not charges: return decision site = f"probe:decision:{role}" @@ -8584,6 +8635,16 @@ def _run_session( # them so the generic adapter can stamp them onto the synthesized # result.json, keeping verify_dev_bundle's dw_ids cross-check live. env["BMAD_LOOP_DW_IDS"] = ",".join(task.dw_ids) + # Usage relaunch (#959) applies to primary story sessions only — dev + # (including the fix phase) and review. Injected workflow, sweep-triage + # and retro sessions have no relaunch arm. + relaunch_session = label is None and role in ("dev", "review") + relaunch_mode = self.policy.limits.usage_limit_action == "relaunch" + if relaunch_session and task.usage_relaunches > 0: + # The story's uncharged usage relaunches so far: the rotation signal + # a wrapper `binary` or `pre_session` plugin keys an account switch on. + # Engine-injected, so not registered in `envvars.py`. + env["BMAD_LOOP_USAGE_RELAUNCH"] = str(task.usage_relaunches) if role == "dev" and not self.policy.review.enabled: # signals that the orchestrator will run no follow-up review session. # bmad-build-auto always self-reviews inline (step-03 → step-04) and @@ -8725,6 +8786,15 @@ def _run_session( # usage-limit quota wait cap (DW-396): the timeout freezes while the # CLI waits out a usage-window reset, for at most this long in total. quota_wait_max_s=float(self.policy.limits.quota_wait_max_min * 60), + # usage relaunch mode (#959), primary story sessions only: whether a + # usage limit relaunches this session, and whether relaunch budget is + # left for it (the adapter exits at once rather than waiting). + usage_relaunch_mode=relaunch_session and relaunch_mode, + usage_relaunch_left=( + relaunch_session + and relaunch_mode + and task.usage_relaunches < self.policy.limits.max_usage_relaunches + ), # Launch-state snapshot of a review session's spec (#276 M1); None for # every other session and on a crash-resume (process-transient). spec_snapshot=spec_snapshot, @@ -9505,7 +9575,9 @@ def _fix_phase(self, task: StoryTask, reason: str) -> Decision: # Empty whenever the last attempt completed: the verify failure `reason` # already describes that, and the callers' own wording is the honest one. session_failure = "" - while task.attempt < self.policy.limits.max_dev_attempts: + # Charged attempts only: an uncharged usage relaunch (#959) bumps + # `attempt` for session identity and refunds it. + while task.charged_attempts < self.policy.limits.max_dev_attempts: preserve_chain_snapshot = self._preserves_dispatched_spec_snapshot_for_repair(task) task.attempt += 1 feedback = self._write_feedback(task, reason) @@ -9586,6 +9658,17 @@ def _fix_phase(self, task: StoryTask, reason: str) -> Decision: verification_stage=verified.stage, verification_sequence=verified.sequence, ) + critical_reason = critical_session_reason("fix", result.result_json) + # relaunch mode (#959): a usage-limited repair is relaunched uncharged, + # ahead of the env_fault/parked escalates below that would pause this + # same result. Never over a CRITICAL, which outranks it as in + # `decide_dev`. Refunded before the journal so `fix-decision` and + # state.json record the same count. + relaunch = ( + usage_relaunch(task, result, self.policy) if critical_reason is None else None + ) + if relaunch is not None: + task.dev_usage_relaunches += 1 self.journal.append( "fix-decision", story_key=task.story_key, @@ -9602,6 +9685,9 @@ def _fix_phase(self, task: StoryTask, reason: str) -> Decision: parked_evidence=result.parked_evidence, # No work at all (#727): what routed it to the no-work escalate below. produced_work=result.produced_work, + # An uncharged usage relaunch (#959) and the story's running count. + uncharged=relaunch is not None, + usage_relaunches=task.usage_relaunches, ) # CRITICAL routing, deliberately AFTER the emit and the journal record # above, and deliberately AHEAD of the env-fault/retryable arms below. @@ -9616,9 +9702,21 @@ def _fix_phase(self, task: StoryTask, reason: str) -> Decision: # session reporting the same thing fired one. The hook is named for # the verification, the verification ran, and a plugin correlating # verify passes cannot have half of them silently withheld. - critical_reason = critical_session_reason("fix", result.result_json) if critical_reason is not None: self._escalate(task, critical_reason) + if relaunch is not None: + # Relaunch the repair the way a failed repair continues: the tree + # is kept and the next pass gets the same feedback. Uncharged, so + # no environment probe and no end-of-loop budget gate. + self._save() + gates.notify( + self.policy, + self.run_dir, + f"fix relaunch: {task.story_key} (usage limit, relaunch " + f"{task.usage_relaunches}/{self.policy.limits.max_usage_relaunches})", + relaunch.reason, + ) + continue if result.status != "completed" and result.env_fault: # A fix session whose CLI lost its API connection (#194) did no # repair work — another attempt cannot fix the run environment, so @@ -9674,7 +9772,7 @@ def _fix_phase(self, task: StoryTask, reason: str) -> Decision: return Decision(Action.PROCEED) # A failed repair is charged — the next attempt, or the budget-spent # DEFER below — so re-probe the environment first (DW-523). - budget_left = task.attempt < self.policy.limits.max_dev_attempts + budget_left = task.charged_attempts < self.policy.limits.max_dev_attempts gated = self._env_gate_decision( task, Decision( diff --git a/src/bmad_loop/escalation.py b/src/bmad_loop/escalation.py index 576298b66..6bb2580ba 100644 --- a/src/bmad_loop/escalation.py +++ b/src/bmad_loop/escalation.py @@ -78,6 +78,10 @@ class Decision: # Set by a decider whose PAUSE reason is the usage-limit reason (DW-591): # the facts the engine records with the pause. None on every other decision. usage_limit: UsageLimitPause | None = None + # A RETRY that charges no budget (#959): a usage relaunch from + # :func:`usage_relaunch`. The engine adds it to the story's refund counter + # before journaling, and the environment seam never re-probes for it. + uncharged: bool = False def _escalation_list(result_json: dict[str, Any] | None) -> list[Any]: @@ -423,6 +427,46 @@ def session_failure_reason(role: str, result: SessionResult) -> str: return reason +USAGE_RELAUNCH_REASON_PREFIX = "usage relaunch " + + +def usage_relaunch(task: StoryTask, result: SessionResult, policy: Policy) -> Decision | None: + """The uncharged RETRY for a session that ended on a usage limit (#959), or + None when it does not qualify. + + Qualifies only in relaunch mode (``limits.usage_limit_action = + "relaunch"``), while the story's uncharged relaunches stay under + ``limits.max_usage_relaunches``, for a non-completed session the adapter saw + behind a usage limit (``usage_limited``, any adapter) or a StopFailure mapped + to the account action. At the cap it returns None, so the caller falls + through to the usage-limit pause the same result gets in pause mode: a + repeated or forged limit signal buys at most the cap, never a loop. + + The reason never starts with ``"usage limit: "``: :func:`pause_title` keys on + that prefix, and this is a relaunch, not a pause.""" + limits = policy.limits + if limits.usage_limit_action != "relaunch": + return None + if task.usage_relaunches >= limits.max_usage_relaunches: + return None + if result.status == "completed": + return None + if result.usage_limited is None and result.stop_failure_action != "account": + return None + evidence = ( + result.usage_limited + or result.parked_evidence + or result.stop_failure + or f"session {result.status}" + ) + return Decision( + Action.RETRY, + f"{USAGE_RELAUNCH_REASON_PREFIX}({task.usage_relaunches + 1}/" + f"{limits.max_usage_relaunches}): {evidence}", + uncharged=True, + ) + + def decide_dev( task: StoryTask, result: SessionResult, @@ -434,10 +478,16 @@ def decide_dev( if critical_reason is not None: return Decision(Action.PAUSE, critical_reason) - budget_left = task.attempt < policy.limits.max_dev_attempts + budget_left = task.charged_attempts < policy.limits.max_dev_attempts exhausted = _exhausted_action(task) if result.status != "completed": + relaunch = usage_relaunch(task, result, policy) + if relaunch is not None: + # relaunch mode (#959): an uncharged RETRY ahead of the env_fault and + # parked arms that would pause this same result, and ahead of the + # budget, so it relaunches even on the last attempt. + return relaunch if result.env_fault: # A transport/API failure (the CLI never reached the API, #194): the # attempt did no real work, so pause for a human instead of charging @@ -536,6 +586,11 @@ def decide_review_session(task: StoryTask, result: SessionResult, policy: Policy return Decision(Action.PAUSE, critical_reason) if result.status != "completed": + relaunch = usage_relaunch(task, result, policy) + if relaunch is not None: + # relaunch mode (#959): an uncharged RETRY ahead of the pause arms and + # the `review.on_timeout` routing (see decide_dev). + return relaunch if result.env_fault: # transport/API failure (#194): pause rather than charge a review # cycle for a session that never reached the API (see decide_dev). @@ -578,7 +633,7 @@ def review_retry_or_exhaust(task: StoryTask, policy: Policy, reason: str) -> Dec cycle budget lasts, then plateau-defer (or re-escalate mid re-drive). Module- level so the engine can fall back through it when a SALVAGE attempt turns out not to be applicable (#271).""" - if task.review_cycle < policy.limits.max_review_cycles: + if task.charged_review_cycles < policy.limits.max_review_cycles: return Decision(Action.RETRY, reason) return review_exhausted(task, reason) diff --git a/src/bmad_loop/model.py b/src/bmad_loop/model.py index 110c6242b..a19e2bb08 100644 --- a/src/bmad_loop/model.py +++ b/src/bmad_loop/model.py @@ -818,11 +818,40 @@ class StoryTask: # keep it); "" when no T1 in-window mark is owed or the plan cannot be read. ticket_premark_status: str = "" ticket_premark_digest: str = "" + # Uncharged usage relaunches (#959, `limits.usage_limit_action = "relaunch"`): + # how many of `attempt` (dev + fix sessions) and `review_cycle` (review + # sessions) were relaunches of a session that ended on a usage limit or an + # account StopFailure. The counters still bump on every launch, so session + # task ids and resume identity are untouched; these refunds are subtracted + # only in budget checks (`charged_attempts` / `charged_review_cycles`). + # Invariant: each refund is <= its counter, so every site that resets + # `attempt` (or `review_cycle`) to 0 zeroes its refund too. Their sum is + # capped per story by `limits.max_usage_relaunches`. + dev_usage_relaunches: int = 0 + review_usage_relaunches: int = 0 @property def terminal(self) -> bool: return self.phase in TERMINAL_PHASES + @property + def charged_attempts(self) -> int: + """Dev attempts charged against `limits.max_dev_attempts`: every launch + minus the uncharged usage relaunches (#959).""" + return self.attempt - self.dev_usage_relaunches + + @property + def charged_review_cycles(self) -> int: + """Review cycles charged against `limits.max_review_cycles`: every launch + minus the uncharged usage relaunches (#959).""" + return self.review_cycle - self.review_usage_relaunches + + @property + def usage_relaunches(self) -> int: + """Uncharged usage relaunches this story has spent, dev and review + together: the count `limits.max_usage_relaunches` caps (#959).""" + return self.dev_usage_relaunches + self.review_usage_relaunches + def record_session(self, record: SessionRecord) -> None: self.sessions.append(record) if record.usage: @@ -853,6 +882,8 @@ def to_dict(self, mount_project: Path | None = None) -> dict[str, Any]: "attempt": self.attempt, "review_cycle": self.review_cycle, "followup_reviews_spent": self.followup_reviews_spent, + "dev_usage_relaunches": self.dev_usage_relaunches, + "review_usage_relaunches": self.review_usage_relaunches, "generation": self.generation, "escalations_resolved_upto": self.escalations_resolved_upto, "followup_review_recommended": self.followup_review_recommended, @@ -1112,6 +1143,8 @@ def from_dict(cls, d: dict[str, Any]) -> "StoryTask": attempt=int(d.get("attempt", 0)), review_cycle=int(d.get("review_cycle", 0)), followup_reviews_spent=int(d.get("followup_reviews_spent", 0)), + dev_usage_relaunches=int(d.get("dev_usage_relaunches", 0)), + review_usage_relaunches=int(d.get("review_usage_relaunches", 0)), generation=int(d.get("generation", 0)), escalations_resolved_upto=int(d.get("escalations_resolved_upto", 0)), followup_review_recommended=bool(d.get("followup_review_recommended", False)), diff --git a/src/bmad_loop/policy.py b/src/bmad_loop/policy.py index bd1bd27ad..988258477 100644 --- a/src/bmad_loop/policy.py +++ b/src/bmad_loop/policy.py @@ -32,6 +32,8 @@ GATE_MODES = {"none", "per-epic", "per-story-spec-approval"} RETRO_MODES = {"never", "notify", "auto"} SESSION_BUDGET_MODES = {"off", "warn", "enforce"} +# [limits] usage_limit_action: what a usage-limited session end does (#959). +USAGE_LIMIT_ACTIONS = frozenset({"pause", "relaunch"}) SWEEP_AUTO_MODES = {"never", "per-epic", "run-end"} REVIEW_TRIGGER_MODES = {"always", "recommended"} REVIEW_ON_TIMEOUT_MODES = {"retry", "salvage-if-done", "defer"} @@ -199,6 +201,18 @@ class LimitsPolicy: # enforce mode: seconds a tripped session gets to wrap up after the nudge # before it is terminated over_budget. 0 = terminate at trip, no nudge. session_budget_grace_s: int = 240 + # what a dev/review/fix session that ended without completing because of a + # usage limit (or an account-level StopFailure) does (#959). "pause" = the + # uncharged usage-limit pause `resolve` clears. "relaunch" = relaunch the + # session at once as an UNCHARGED retry (attempt/review_cycle still bump for + # session identity; a refund counter keeps the budget whole), so a wrapper + # or pre_session plugin can rotate accounts keyed on BMAD_LOOP_USAGE_RELAUNCH. + usage_limit_action: str = "pause" + # relaunch mode: the per-story cap on uncharged usage relaunches (dev, fix + # and review together). At the cap the session falls back to the pause, so + # a repeated or forged limit signal can never loop unbounded. 0 = relaunch + # mode never relaunches (always pauses). + max_usage_relaunches: int = 3 @dataclass(frozen=True) @@ -1081,6 +1095,12 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy: session_budget_grace_s=_typed_int( limits_d, "limits", "session_budget_grace_s", LimitsPolicy.session_budget_grace_s ), + usage_limit_action=_typed_str( + limits_d, "limits", "usage_limit_action", LimitsPolicy.usage_limit_action + ).strip(), + max_usage_relaunches=_typed_int( + limits_d, "limits", "max_usage_relaunches", LimitsPolicy.max_usage_relaunches + ), ) if limits.max_review_cycles < 1 or limits.max_dev_attempts < 1: raise PolicyError("limits.max_review_cycles and limits.max_dev_attempts must be >= 1") @@ -1138,6 +1158,15 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy: f"limits.session_budget_mode must be one of {sorted(SESSION_BUDGET_MODES)}: " f"got {limits.session_budget_mode!r}" ) + if limits.usage_limit_action not in USAGE_LIMIT_ACTIONS: + raise PolicyError( + f"limits.usage_limit_action must be one of {sorted(USAGE_LIMIT_ACTIONS)}: " + f"got {limits.usage_limit_action!r}" + ) + if limits.max_usage_relaunches < 0: + raise PolicyError( + f"limits.max_usage_relaunches must be >= 0: got {limits.max_usage_relaunches}" + ) if limits.max_tokens_per_story < 1: raise PolicyError( f"limits.max_tokens_per_story must be >= 1: got {limits.max_tokens_per_story}" @@ -1576,6 +1605,8 @@ def _refuse_removed_engine(engine_d: dict[str, Any]) -> None: session_budget_mode = "warn" # off | warn | enforce — weighted per-SESSION cap sampled every ~30s mid-session; warn = one ATTENTION + breadcrumb; enforce = wrap-up nudge (best-effort courtesy; the kill is the guarantee) then over_budget termination (retry→defer). Live-verified on claude; other transcript parsers sample best-effort (mid-turn flush unverified); inert where no mid-session usage signal exists (usage_parser "none", copilot's shutdown-only flush) max_tokens_per_session = 4000000 # weighted cap a single session may spend before the guard trips; healthy sessions run ~1-2.5M weighted, so the default trips only true runaways (#158) session_budget_grace_s = 240 # enforce mode: seconds a tripped session gets to wrap up after the nudge before over_budget termination. 0 = terminate at trip, no nudge +usage_limit_action = "pause" # pause | relaunch — a session that ends on a usage limit or account StopFailure: pause = the uncharged usage-limit pause; relaunch = relaunch it at once as an uncharged retry (BMAD_LOOP_USAGE_RELAUNCH= in the session env, for account rotation) (#959) +max_usage_relaunches = 3 # relaunch mode: uncharged usage relaunches per story (dev, fix and review together); at the cap the session pauses instead. 0 = always pause [verify] # Deterministic gates run by the orchestrator after a clean review, before commit. diff --git a/src/bmad_loop/runs.py b/src/bmad_loop/runs.py index a23c0042a..c8b34c137 100644 --- a/src/bmad_loop/runs.py +++ b/src/bmad_loop/runs.py @@ -6797,6 +6797,7 @@ def _rearm_escalation_locked( # engine._finish_inflight): a clean re-attempt against the corrected spec. task.phase = Phase.PENDING task.attempt = 0 + task.dev_usage_relaunches = 0 # refunds never exceed attempt (#959) # A new generation of this task. `attempt` going back to 0 (and the next # dispatch bumping it to 1) would otherwise re-mint a session task_id # byte-equal to one the abandoned attempt already recorded, and @@ -6817,6 +6818,7 @@ def _rearm_escalation_locked( if resolution_recorded: task.escalations_resolved_upto = len(task.sessions) task.review_cycle = 0 + task.review_usage_relaunches = 0 # refunds never exceed review_cycle (#959) task.followup_reviews_spent = 0 # human-resolved re-drive gets a fresh damping budget task.defer_reason = None task.env_fault_site = None # the re-armed story starts with no fault on record (DW-523) @@ -7953,6 +7955,7 @@ def latch_for_reverify(task: StoryTask, origin: str) -> None: task.defer_reason = None task.generation += 1 # #705: no session id the earlier attempt minted is reused task.review_cycle = 0 + task.review_usage_relaunches = 0 # refunds never exceed review_cycle (#959) task.followup_reviews_spent = 0 task.salvage_refile_pending = False task.adopt_pending = False diff --git a/src/bmad_loop/sweep.py b/src/bmad_loop/sweep.py index 88ec0105e..86ed1d6af 100644 --- a/src/bmad_loop/sweep.py +++ b/src/bmad_loop/sweep.py @@ -3272,7 +3272,9 @@ def _reset_superseded_bundle_state(self, task: StoryTask) -> None: task.restore_patch = None task.preserve_from_attempt = False task.attempt = 0 + task.dev_usage_relaunches = 0 # refunds never exceed attempt (#959) task.review_cycle = 0 + task.review_usage_relaunches = 0 # refunds never exceed review_cycle (#959) task.followup_reviews_spent = 0 task.defer_reason = None _rearm_generation(task) @@ -4687,6 +4689,7 @@ def _ensure_migration(self, text: str) -> None: ) if task.phase == Phase.ESCALATED: task.attempt = 0 # the human resumed deliberately; fresh budget + task.dev_usage_relaunches = 0 # refunds never exceed attempt (#959) _rearm_generation(task) # ...and into a fresh session-id namespace task.migration_commit_escalated = False if dirty: @@ -5259,9 +5262,11 @@ def _ensure_triage(self, open_now: set[str], cycle: int = 1) -> TriagePlan: self.journal.append("resume-restart", story_key=triage_key, phase=str(task.phase)) if selector_cache_mismatch: task.attempt = 0 + task.dev_usage_relaunches = 0 # refunds never exceed attempt (#959) _rearm_generation(task) elif task.phase == Phase.ESCALATED: task.attempt = 0 # the human resumed deliberately; fresh budget + task.dev_usage_relaunches = 0 # refunds never exceed attempt (#959) _rearm_generation(task) # ...and into a fresh session-id namespace task.phase = Phase.PENDING # deliberate reset, not a normal transition diff --git a/tests/test_engine.py b/tests/test_engine.py index 814368697..5afbdd379 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -26824,3 +26824,276 @@ def test_rollback_discards_frontmatter_only_plan(project_v7): assert tree.discards == ["1.1"] assert tree.marks == [] + + +# --------------------------------------------------------------------------- +# #959: opt-in uncharged usage relaunch (`limits.usage_limit_action = "relaunch"`) + + +def _relaunch_policy(*, review: ReviewPolicy | None = None, **limits) -> Policy: + return Policy( + gates=GatesPolicy(mode="none"), + notify=QUIET, + scm=ScmPolicy(rollback_on_failure=True), + limits=LimitsPolicy(usage_limit_action="relaunch", **limits), + **({"review": review} if review is not None else {}), + ) + + +def _rows(engine, kind: str) -> list[dict]: + return [e for e in engine.journal.entries() if e["kind"] == kind] + + +def test_relaunch_mode_relaunches_a_usage_limited_dev_session_uncharged(project): + """#959: in relaunch mode a usage-limited dev session is relaunched as an + uncharged RETRY, even on the last attempt (`max_dev_attempts=1`): the second + session runs, the story commits, `attempt` still bumps (session identity) and + the refund keeps the charged count at 1. ABLATION, performed: drop the + `usage_relaunch` arm in `decide_dev` and the run pauses on the usage limit + after one session.""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + engine, adapter = make_engine( + project, + [_usage_limited(env_fault=True), dev_effect(project, "1-1-a", followup_review=False)], + policy=_relaunch_policy(max_dev_attempts=1), + ) + summary = engine.run() + + assert summary.done == 1 and not summary.paused and summary.deferred == 0 + assert [s.role for s in adapter.sessions] == ["dev", "dev"] + assert adapter.sessions[0].task_id != adapter.sessions[1].task_id + task = engine.state.tasks["1-1-a"] + assert (task.attempt, task.dev_usage_relaunches, task.charged_attempts) == (2, 1, 1) + on_disk = load_state(engine.run_dir).tasks["1-1-a"] + assert on_disk.dev_usage_relaunches == 1 + first, second = _rows(engine, "dev-decision") + assert first["action"] == "retry" and first["uncharged"] is True + assert first["usage_relaunches"] == 1 + assert first["reason"].startswith("usage relaunch (1/3): ") + assert second["action"] == "proceed" and second["uncharged"] is False + + +def test_relaunch_cap_pauses_uncharged(project): + """#959: at `max_usage_relaunches` the usage-limited session takes today's + uncharged usage-limit pause instead of another relaunch. ABLATION, performed: + drop the cap check in `usage_relaunch` and a third session is launched (the + script runs out and the run crashes instead of pausing).""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + engine, adapter = make_engine( + project, + [_usage_limited(env_fault=True), _usage_limited(env_fault=True)], + policy=_relaunch_policy(max_usage_relaunches=1), + ) + summary = engine.run() + + assert summary.paused and summary.deferred == 0 + assert [s.role for s in adapter.sessions] == ["dev", "dev"] + assert engine.state.paused_reason is not None + assert engine.state.paused_reason.startswith("usage limit: dev session stalled") + task = engine.state.tasks["1-1-a"] + assert task.phase == Phase.ESCALATED + assert (task.attempt, task.dev_usage_relaunches) == (2, 1) + first, second = _rows(engine, "dev-decision") + assert first["action"] == "retry" and first["uncharged"] is True + assert second["action"] == "pause" and second["uncharged"] is False + # the relaunch session knew it was the last one: no budget left for another + assert adapter.sessions[0].usage_relaunch_left is True + assert adapter.sessions[1].usage_relaunch_left is False + assert adapter.sessions[1].usage_relaunch_mode is True + + +def test_review_relaunch_on_the_last_cycle_still_runs(project): + """#959: a usage-limited review on the last allowed cycle is relaunched; the + review loop's budget counts charged cycles, so the relaunch runs and the + story converges. ABLATION, performed: make the review `while` read + `task.review_cycle` and the relaunch never runs (the story defers).""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + engine, adapter = make_engine( + project, + [ + dev_effect(project, "1-1-a"), + _usage_limited(env_fault=True), + review_effect(project, "1-1-a", clean=True), + ], + policy=_relaunch_policy(max_review_cycles=1), + ) + summary = engine.run() + + assert summary.done == 1 and summary.deferred == 0 and not summary.paused + assert [s.role for s in adapter.sessions] == ["dev", "review", "review"] + task = engine.state.tasks["1-1-a"] + assert (task.review_cycle, task.review_usage_relaunches) == (2, 1) + assert task.charged_review_cycles == 1 + (retry,) = _rows(engine, "review-retry") + assert retry["uncharged"] is True and retry["usage_relaunches"] == 1 + assert retry["reason"].startswith("usage relaunch (1/3): ") + + +def test_fix_phase_relaunch_is_uncharged(project, monkeypatch): + """#959: a usage-limited repair session in `_fix_phase` is relaunched + uncharged: with `max_dev_attempts=2` the dev attempt plus one charged repair + fit, so the relaunched repair still runs and the story commits. ABLATION, + performed: drop the fix-phase relaunch arm and the usage-limited repair + pauses the run on the usage limit.""" + write_sprint(project, {"epic-1": "backlog", "1-1-a": "ready-for-dev"}) + engine, adapter = make_engine( + project, + [ + dev_effect(project, "1-1-a", followup_review=False), + _usage_limited(env_fault=True), + dev_effect(project, "1-1-a"), + ], + policy=_relaunch_policy(review=ReviewPolicy(enabled=False), max_dev_attempts=2), + ) + calls = iter( + [ + [verify.CommandResult("pytest -q", 0, "first", "first-out", "")], + [verify.CommandResult("pytest -q", 1, "review fail", "", "review fail")], + [verify.CommandResult("pytest -q", 0, "fixed", "fixed-out", "")], + [verify.CommandResult("pytest -q", 0, "final", "final-out", "")], + ] + ) + monkeypatch.setattr( + verify, + "run_verify_commands", + scripted_verify_runner(project.repo_root, lambda: next(calls)), + ) + summary = engine.run() + + assert summary.done == 1 and summary.deferred == 0 and not summary.paused + assert [s.role for s in adapter.sessions] == ["dev", "dev", "dev"] + task = engine.state.tasks["1-1-a"] + assert (task.attempt, task.dev_usage_relaunches, task.charged_attempts) == (3, 1, 2) + relaunched, repaired = _rows(engine, "fix-decision") + assert relaunched["uncharged"] is True and relaunched["usage_relaunches"] == 1 + assert relaunched["ok"] is False + assert repaired["uncharged"] is False and repaired["ok"] is True + + +def test_uncharged_relaunch_skips_environment_probes(project, tmp_path): + """#959: an uncharged relaunch charges nothing, so the decision seam never + re-probes the environment for it. The environment dies during the + usage-limited session, yet the relaunch still launches: a dev retry + dispatches inside `_dev_phase`, past the dispatch gate, so nothing probes + between the two sessions. ABLATION, performed: drop `not decision.uncharged` + from `_env_gate_decision` and the decision is reclassified at + `probe:decision:dev` (a PAUSE, one session).""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + rig = _env_rig(tmp_path) + policy = dataclasses.replace( + _relaunch_policy(), environment=EnvironmentPolicy(probes=(rig.probe,)) + ) + engine, adapter = make_engine( + project, + [ + rig.dies_during(_usage_limited(env_fault=False)), + dev_effect(project, "1-1-a", followup_review=False), + ], + policy=policy, + ) + engine.run() + + assert [s.role for s in adapter.sessions] == ["dev", "dev"] + first = _rows(engine, "dev-decision")[0] + assert first["action"] == "retry" and first["uncharged"] is True + assert "env_fault_site" not in first + assert not _reclassified(engine) + sites = [e["site"] for e in _rows(engine, "env-probe-failed")] + assert "probe:decision:dev" not in sites + assert engine.state.tasks["1-1-a"].dev_usage_relaunches == 1 + + +def test_usage_relaunch_env_var_only_after_a_relaunch(project): + """#959: `BMAD_LOOP_USAGE_RELAUNCH=` rides the session env only once the + story has relaunched (n > 0) — absent on the first session, "1" on the + relaunch. Pause mode sets no relaunch flags. ABLATION, performed: drop the + `> 0` guard and the first session carries "0".""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + engine, adapter = make_engine( + project, + [_usage_limited(env_fault=True), dev_effect(project, "1-1-a", followup_review=False)], + policy=_relaunch_policy(), + ) + assert engine.run().done == 1 + + first, second = adapter.sessions + assert "BMAD_LOOP_USAGE_RELAUNCH" not in first.env + assert second.env["BMAD_LOOP_USAGE_RELAUNCH"] == "1" + assert (first.usage_relaunch_mode, first.usage_relaunch_left) == (True, True) + assert (second.usage_relaunch_mode, second.usage_relaunch_left) == (True, True) + + write_sprint(project, {"1-1-b": "ready-for-dev"}) + paused_engine, paused_adapter = make_engine(project, [_usage_limited(env_fault=True)]) + assert paused_engine.run().paused + (only,) = paused_adapter.sessions + assert (only.usage_relaunch_mode, only.usage_relaunch_left) == (False, False) + assert "BMAD_LOOP_USAGE_RELAUNCH" not in only.env + + +def test_relaunch_flags_only_on_primary_story_sessions(project): + """#959: only primary story sessions (dev, fix, review) get the relaunch + flags and env var; an injected plugin-workflow session — here a `review` + role run after the story already relaunched once — gets the defaults. + ABLATION, performed: drop `label is None` from the primary-session test and + the workflow session carries both flags and the env var.""" + from bmad_loop.plugins import PluginRegistry + from bmad_loop.plugins.model import LoadedPlugin, PluginManifest, WorkflowSpec + + write_sprint(project, {"1-1-a": "ready-for-dev"}) + manifest = PluginManifest( + name="wf", + api_version=1, + workflows=( + WorkflowSpec( + name="doc", + stage="post_dev_phase", + role="review", + prompt="/doc {story_key}", + blocking=False, + ), + ), + ) + engine, adapter = make_engine( + project, + [ + _usage_limited(env_fault=True), + dev_effect(project, "1-1-a", followup_review=False), + SessionResult(status="completed", result_json={}), + ], + policy=_relaunch_policy(), + registry=PluginRegistry([LoadedPlugin(manifest=manifest)]), + ) + engine.run() + + assert len(adapter.sessions) == 3 + dev_relaunch, workflow = adapter.sessions[1], adapter.sessions[2] + assert "wf.doc" in workflow.task_id + assert dev_relaunch.usage_relaunch_mode and dev_relaunch.usage_relaunch_left + assert dev_relaunch.env["BMAD_LOOP_USAGE_RELAUNCH"] == "1" + assert (workflow.usage_relaunch_mode, workflow.usage_relaunch_left) == (False, False) + assert "BMAD_LOOP_USAGE_RELAUNCH" not in workflow.env + + +def test_rearm_resets_usage_relaunch_counters(project): + """#959: `runs.rearm_escalation` resets `attempt` and `review_cycle`, so it + zeroes both refund counters too (refunds never exceed their counter) and the + re-driven story gets a fresh relaunch budget. ABLATION, performed: drop the + two rearm reset lines and the counters survive the re-arm.""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + engine, _ = make_engine( + project, + [_usage_limited(env_fault=True), _usage_limited(env_fault=True)], + policy=_relaunch_policy(max_usage_relaunches=1), + ) + assert engine.run().paused + task = engine.state.tasks["1-1-a"] + task.review_cycle = 1 + task.review_usage_relaunches = 1 + engine._save() + + rearm_escalation(engine.run_dir, isolated_redrive=False, resolution_recorded=True) + + rearmed = load_state(engine.run_dir).tasks["1-1-a"] + assert (rearmed.attempt, rearmed.dev_usage_relaunches) == (0, 0) + assert (rearmed.review_cycle, rearmed.review_usage_relaunches) == (0, 0) + assert rearmed.usage_relaunches == 0 diff --git a/tests/test_escalation.py b/tests/test_escalation.py index d9a62ca62..fe0436b9a 100644 --- a/tests/test_escalation.py +++ b/tests/test_escalation.py @@ -965,3 +965,140 @@ def test_unresolved_escalation_keys_checkpoint_flow_skips_attempt0_wedges(source def test_unresolved_escalation_keys_empty_without_an_escalated_task(): state = _escalation_state("sprint-status", ("a", Phase.DONE, 1), ("b", Phase.PENDING, 0)) assert unresolved_escalation_keys(state) == [] + + +# --- #959: opt-in uncharged usage relaunch ------------------------------------- + +RELAUNCH = dataclasses.replace( + POLICY, limits=dataclasses.replace(POLICY.limits, usage_limit_action="relaunch") +) +_ACCOUNT_EVIDENCE = "StopFailure(authentication_failed): account suspended" + + +def _account_stop_failure() -> SessionResult: + """A dead window after an account StopFailure, as S1 relabels it.""" + return SessionResult( + status="crashed", + parked=True, + parked_evidence=_ACCOUNT_EVIDENCE, + stop_failure="authentication_failed", + stop_failure_action="account", + ) + + +@pytest.mark.parametrize("attempt", range(1, POLICY.limits.max_dev_attempts + 1)) +@pytest.mark.parametrize("status", ["stalled", "timeout", "crashed"]) +def test_dev_usage_limited_relaunches_uncharged_in_relaunch_mode(attempt, status): + """Relaunch mode turns a usage-limited dev session into an uncharged RETRY, + even on the last attempt, ahead of the env_fault and parked pause arms. + ABLATION: drop `uncharged=True` from `usage_relaunch`'s Decision and the + uncharged assertion fails.""" + for result in _usage_limited_results(status): + decision = decide_dev(_task(attempt=attempt), result, None, RELAUNCH) + assert decision.action == Action.RETRY + assert decision.uncharged is True + assert not decision.budget_exhausted + assert decision.reason.startswith("usage relaunch (1/3): ") + assert _USAGE_EVIDENCE in decision.reason + + +def test_dev_account_stop_failure_relaunches(): + """An account StopFailure qualifies with no usage-limit sighting at all; the + reason names the StopFailure evidence.""" + result = _account_stop_failure() + assert result.usage_limited is None + decision = decide_dev(_task(attempt=2), result, None, RELAUNCH) + assert decision.action == Action.RETRY + assert decision.uncharged is True + assert decision.reason == f"usage relaunch (1/3): {_ACCOUNT_EVIDENCE}" + # a parked result with no account action and no usage sighting does not qualify + permission = SessionResult(status="stalled", parked=True, parked_evidence="permission") + assert escalation.usage_relaunch(_task(attempt=1), permission, RELAUNCH) is None + + +def test_relaunch_cap_falls_back_to_the_usage_pause(): + """At the per-story cap (dev and review refunds together) the result gets + the same uncharged usage-limit PAUSE it gets in pause mode. ABLATION: skip + the cap check in `usage_relaunch` and the PAUSE assertions fail.""" + at_cap = _task(attempt=3, review_cycle=1, dev_usage_relaunches=2, review_usage_relaunches=1) + assert at_cap.usage_relaunches == RELAUNCH.limits.max_usage_relaunches + for result in [*_usage_limited_results("stalled"), _account_stop_failure()]: + expected = decide_dev(at_cap, result, None, POLICY) + assert expected.action == Action.PAUSE + decision = decide_dev(at_cap, result, None, RELAUNCH) + assert decision == expected + assert decision.uncharged is False + review = decide_review_session(at_cap, result, RELAUNCH) + assert review == decide_review_session(at_cap, result, POLICY) + assert review.action == Action.PAUSE and review.uncharged is False + # a cap of 0: relaunch mode never relaunches + zero = dataclasses.replace( + RELAUNCH, limits=dataclasses.replace(RELAUNCH.limits, max_usage_relaunches=0) + ) + for result in _usage_limited_results("stalled"): + decision = decide_dev(_task(attempt=1), result, None, zero) + assert decision.action == Action.PAUSE + assert decision.uncharged is False + assert escalation.usage_relaunch(_task(attempt=1), result, zero) is None + + +def test_pause_mode_never_relaunches(): + """The default `usage_limit_action = "pause"` keeps today's pause.""" + assert POLICY.limits.usage_limit_action == "pause" + for result in [*_usage_limited_results("stalled"), _account_stop_failure()]: + assert escalation.usage_relaunch(_task(attempt=1), result, POLICY) is None + decision = decide_dev(_task(attempt=1), result, None, POLICY) + assert decision.action == Action.PAUSE + assert decision.uncharged is False + + +def test_completed_session_never_relaunches(): + """A completed session is judged on its work, never relaunched, even when it + carries a usage-limit sighting.""" + completed = dataclasses.replace(COMPLETED, usage_limited=_USAGE_EVIDENCE) + assert escalation.usage_relaunch(_task(attempt=1), completed, RELAUNCH) is None + account = dataclasses.replace(COMPLETED, stop_failure_action="account") + assert escalation.usage_relaunch(_task(attempt=1), account, RELAUNCH) is None + decision = decide_dev(_task(attempt=1), completed, FAILING, RELAUNCH) + assert decision.action == Action.RETRY + assert decision.uncharged is False + + +@pytest.mark.parametrize("review_cycle", [1, POLICY.limits.max_review_cycles]) +def test_review_usage_limited_relaunches(review_cycle): + """The review-side twin, ahead of the pause arms and the on_timeout routing, + and even on the last review cycle.""" + for result in [*_usage_limited_results("timeout"), _account_stop_failure()]: + decision = decide_review_session(_task(review_cycle=review_cycle), result, RELAUNCH) + assert decision.action == Action.RETRY + assert decision.uncharged is True + assert decision.reason.startswith("usage relaunch (1/3): ") + + +def test_relaunch_reason_never_reads_as_a_usage_pause(): + """`pause_title` keys on the `usage limit: ` prefix; a relaunch reason must not + carry it, and its count names the next relaunch out of the cap.""" + for result in [*_usage_limited_results("stalled"), _account_stop_failure()]: + task = _task(attempt=1, dev_usage_relaunches=1, review_usage_relaunches=1) + decision = decide_dev(task, result, None, RELAUNCH) + assert decision.reason.startswith("usage relaunch (3/3): ") + assert not decision.reason.startswith("usage limit: ") + assert pause_title(decision.reason, "9-0-x") != "Paused (usage limit): 9-0-x" + + +def test_charged_attempts_subtract_refunds(): + """The budget checks count charged sessions only. ABLATION: revert + `decide_dev`'s `budget_left` to `task.attempt` and the dev RETRY becomes a + DEFER.""" + crashed = SessionResult(status="crashed") + dev_task = _task(attempt=POLICY.limits.max_dev_attempts, dev_usage_relaunches=1) + decision = decide_dev(dev_task, crashed, None, POLICY) + assert decision.action == Action.RETRY + assert decision.uncharged is False # a plain crash is charged + # without the refund the same attempt count exhausts the budget + spent = decide_dev(_task(attempt=POLICY.limits.max_dev_attempts), crashed, None, POLICY) + assert spent.action == Action.DEFER + review_task = _task(review_cycle=POLICY.limits.max_review_cycles, review_usage_relaunches=1) + assert review_retry_or_exhaust(review_task, POLICY, "r").action == Action.RETRY + full = _task(review_cycle=POLICY.limits.max_review_cycles) + assert review_retry_or_exhaust(full, POLICY, "r").action == Action.DEFER diff --git a/tests/test_model.py b/tests/test_model.py index df1127ff3..dbd398f04 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -733,6 +733,35 @@ def test_followup_reviews_spent_defaults_zero_for_legacy_state(): assert StoryTask.from_dict(doc).followup_reviews_spent == 0 +def test_usage_relaunch_counters_round_trip_and_default_on_old_state(): + task = StoryTask( + story_key="1-1-a", + epic=1, + attempt=3, + review_cycle=4, + dev_usage_relaunches=2, + review_usage_relaunches=1, + ) + back = StoryTask.from_dict(task.to_dict()) + assert back.dev_usage_relaunches == 2 + assert back.review_usage_relaunches == 1 + # the refunds are subtracted only in the charged views; the raw counters stand + assert back.attempt == 3 and back.review_cycle == 4 + assert back.charged_attempts == 1 + assert back.charged_review_cycles == 3 + assert back.usage_relaunches == 3 + # state.json from before the fields existed reads as no refunds + doc = task.to_dict() + del doc["dev_usage_relaunches"] + del doc["review_usage_relaunches"] + old = StoryTask.from_dict(doc) + assert old.dev_usage_relaunches == 0 + assert old.review_usage_relaunches == 0 + assert old.charged_attempts == 3 + assert old.charged_review_cycles == 4 + assert old.usage_relaunches == 0 + + def test_generation_round_trips(): task = StoryTask(story_key="1-1-a", epic=1, generation=2) assert StoryTask.from_dict(task.to_dict()).generation == 2 diff --git a/tests/test_policy.py b/tests/test_policy.py index abad56051..6f5af0a4c 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -775,6 +775,37 @@ def test_quota_wait_max_min_default_parse_and_template(): policy.loads("[limits]\nquota_wait_max_min = -1\n") +def test_usage_limit_action_default_parse_and_template(): + import tomllib + + assert policy.loads("").limits.usage_limit_action == "pause" + loaded = policy.loads('[limits]\nusage_limit_action = "relaunch"\n') + assert loaded.limits.usage_limit_action == "relaunch" + # surrounding whitespace is stripped before the membership check + padded = policy.loads('[limits]\nusage_limit_action = " relaunch "\n') + assert padded.limits.usage_limit_action == "relaunch" + assert policy.USAGE_LIMIT_ACTIONS == frozenset({"pause", "relaunch"}) + # the emitted template documents the knob at its dataclass default + doc = tomllib.loads(policy.POLICY_TEMPLATE) + assert doc["limits"]["usage_limit_action"] == policy.LimitsPolicy.usage_limit_action + assert policy.loads(policy.POLICY_TEMPLATE).limits.usage_limit_action == "pause" + with pytest.raises(policy.PolicyError, match="usage_limit_action"): + policy.loads('[limits]\nusage_limit_action = "rotate"\n') + + +def test_max_usage_relaunches_default_parse_and_template(): + import tomllib + + assert policy.loads("").limits.max_usage_relaunches == 3 + assert policy.loads("[limits]\nmax_usage_relaunches = 0\n").limits.max_usage_relaunches == 0 + assert policy.loads("[limits]\nmax_usage_relaunches = 5\n").limits.max_usage_relaunches == 5 + # the emitted template documents the knob at its dataclass default + doc = tomllib.loads(policy.POLICY_TEMPLATE) + assert doc["limits"]["max_usage_relaunches"] == policy.LimitsPolicy.max_usage_relaunches + with pytest.raises(policy.PolicyError, match="max_usage_relaunches"): + policy.loads("[limits]\nmax_usage_relaunches = -1\n") + + def test_dev_contract_nudge_default_parse_and_template(): import tomllib diff --git a/tests/test_portability_guard.py b/tests/test_portability_guard.py index df95e91be..6eb749778 100644 --- a/tests/test_portability_guard.py +++ b/tests/test_portability_guard.py @@ -1111,6 +1111,11 @@ # `run-paused` (DW-591): a usage-limit pause's resolved reset — always an # ISO-8601 instant (`usagereset.resolved_reset`), never the raw clause. "usage_limit_reset_at", + # `dev-decision` / `fix-decision` / `review-retry` (#959): whether the + # routing was an uncharged usage relaunch (a bare boolean), and the story's + # running count of them (an int). + "uncharged", + "usage_relaunches", "verification_sequence", "verification_stage", "via", From 928e43bac581db71f693684725ba772bdeaf6ffe Mon Sep 17 00:00:00 2001 From: t Date: Sat, 31 Oct 2026 17:40:28 -0700 Subject: [PATCH 3/4] feat(adapters): exit at once on a usage limit in relaunch mode; document account rotation (#959) --- CHANGELOG.md | 4 + docs/FEATURES.md | 7 +- docs/adapter-authoring-guide.md | 49 ++++++++- docs/escalations-guide.md | 12 ++- docs/plugin-authoring-guide.md | 47 +++++++++ docs/reference/run-state.md | 20 ++-- src/bmad_loop/adapters/generic.py | 71 +++++++++++-- tests/test_conftest.py | 2 +- tests/test_generic_tmux.py | 164 ++++++++++++++++++++++++++++++ tests/test_stories_e2e.py | 89 ++++++++++++++++ 10 files changed, 439 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84d1e1660..a621be87a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,10 @@ breaking changes may land in a minor release. place (#865). - Add the `hooks.events-incomplete` validate warning: a profile hook event whose relay is not registered, with the `init` that registers it or the CLI version it needs. +- Add opt-in `[limits] usage_limit_action = "relaunch"`: a session that ends on a usage + limit relaunches the story at once without charging an attempt, up to + `max_usage_relaunches` per story (default 3), then pauses as before. The relaunched + session gets `BMAD_LOOP_USAGE_RELAUNCH` for account-rotation wrappers (#959). ### Changed diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 4acba2b2e..53dcbc915 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -108,10 +108,12 @@ bmad-loop drives a BMAD sprint unattended: a deterministic Python loop launches - When a Claude Code turn ends on an API error (its `StopFailure` hook), the adapter acts at once and never counts it as a completion: a transient error gets the stall nudge right away and is charged if never recovered, and an account error (billing, authentication, an account hold) pauses the run uncharged with a reason naming the error. - `validate --probes` launches each role's CLI once with no prompt and fails when its first screen shows a startup dialog that would park an unattended session. - While the pane shows the CLI waiting out a usage-limit reset, the session timeout and stall nudge are held for at most `[limits] quota_wait_max_min` (default 300; 0 = off) minutes; past that the session pauses with a usage-limit reason. + - With `[limits] usage_limit_action = "relaunch"`, a dev, fix or review session that ends on a usage limit or a Claude Code account error is relaunched at once without charging an attempt or review cycle, at most `[limits] max_usage_relaunches` (default 3) times per story; past that it pauses uncharged as in the default `"pause"` mode. + - In relaunch mode the session ends at the limit instead of waiting for its reset, a Claude Code `rate_limit` error naming no known limit counts as one, and the relaunched session gets `BMAD_LOOP_USAGE_RELAUNCH=` so a wrapper can switch accounts. - Before an attempt is reset, its commits go to an `attempt-preserve/-` branch and uncommitted work to a `refs/attempt-preserve-dirty/*` snapshot. -- **Config:** `[limits] max_dev_attempts`, `[limits] max_review_cycles`, `[limits] session_timeout_min`, `[limits] dev_stall_grace_s`, `[limits] dev_stall_nudges`, `[limits] dev_stall_nudges_cap`, `[limits] quota_wait_max_min`, `[scm] preserve_keep` +- **Config:** `[limits] max_dev_attempts`, `[limits] max_review_cycles`, `[limits] session_timeout_min`, `[limits] dev_stall_grace_s`, `[limits] dev_stall_nudges`, `[limits] dev_stall_nudges_cap`, `[limits] quota_wait_max_min`, `[limits] usage_limit_action`, `[limits] max_usage_relaunches`, `[scm] preserve_keep` - **Tested by:** `tests/test_escalation.py`, `tests/test_engine.py`, `tests/test_generic_tmux.py`, `tests/test_panematch.py`, `tests/test_sweep.py`, `tests/test_verify.py`, `tests/test_recovery_flow.py`, `tests/test_stories_e2e.py` -- **Docs:** [Preserved attempts](escalations-guide.md#preserved-attempts), [StopFailure signals](adapter-authoring-guide.md#stopfailure-signals), [[limits] reference](reference/configuration.md#limits), [[scm] reference](reference/configuration.md#scm) +- **Docs:** [Preserved attempts](escalations-guide.md#preserved-attempts), [Usage-limit pauses](escalations-guide.md#usage-limit-pauses), [Account rotation](plugin-authoring-guide.md#account-rotation), [StopFailure signals](adapter-authoring-guide.md#stopfailure-signals), [[limits] reference](reference/configuration.md#limits), [[scm] reference](reference/configuration.md#scm) ### Plateau-defer @@ -175,6 +177,7 @@ bmad-loop drives a BMAD sprint unattended: a deterministic Python loop launches - A pattern match is overruled when the session's tracked token usage is nonzero; that session is charged like any failure. - A session that times out, stalls or crashes after a live usage-limit sighting (its profile's `usage_limit_patterns` or `usage_wait_patterns` on the pane, or an opencode 2.x quota error) pauses with a `usage limit:` reason and is never charged; the reason names the reset time when the line gives one. - A Claude Code `StopFailure` reporting `rate_limit` is matched by its message against the same usage lists: a blocking limit pauses as a usage limit, and a waiting limit can start the quota wait without a pane capture. + - A session whose window dies after a Claude Code usage-limit or account `StopFailure`, or during a quota wait, ends `crashed` but pauses uncharged like the live case; a crash after an unclassified error is still charged. - `bmad-loop resolve --at-reset` waits until a usage-limit pause's reset plus `--margin-min` (default 2), then re-arms and resumes; credit exhaustion and an unknown or too-distant reset are refused. - **Config:** `[environment] probes`, `[environment] probe_timeout_s`, `[verify] env_fault_rc`, `[limits] quota_wait_max_min`; profile keys `env_fault_patterns`, `usage_limit_patterns`, `usage_wait_patterns` - **Tested by:** `tests/test_engine.py`, `tests/test_cli.py`, `tests/test_escalation.py`, `tests/test_env_fault_patterns.py`, `tests/test_env_fault.py`, `tests/test_generic_tmux.py`, `tests/test_opencode_http.py`, `tests/test_usagereset.py` diff --git a/docs/adapter-authoring-guide.md b/docs/adapter-authoring-guide.md index 24445cc1b..513bc689f 100644 --- a/docs/adapter-authoring-guide.md +++ b/docs/adapter-authoring-guide.md @@ -532,13 +532,44 @@ The generic adapter acts on it when it arrives, by action: offers none, so a backend that cannot capture its pane can still admit the wait. No match leaves `stop-failure-rate-limit-unmatched` and the usual stall timing. A match that blows the per-pattern timeout leaves `stop-failure-probe-failed` and counts as no - match. + match. In usage-relaunch mode (see below) a wait match and an unmatched value end the + session at once instead. - **`ignore`**: nothing beyond the crumb. Every event leaves a `stop-failure` lifecycle crumb (error, action, details, a message excerpt); an unmapped value also leaves `stop-failure-unmapped` and keeps the usual stall timing. A later `Stop` clears the latch. `SessionResult.stop_failure` carries the last -uncleared error, as does the heartbeat and the `session-end` journal entry. +uncleared error, as does the heartbeat and the `session-end` journal entry, and +`SessionResult.stop_failure_action` its mapped action. + +**A window that dies after the event.** An operator's own `StopFailure` hook may kill the +window, for example to switch accounts. The session then ends `crashed`, but an `account` +error, or a `rate_limit` that matched a usage list, keeps its classification: the result +is labeled `parked` (account) or `usage_limited` (blocked or wait class), and the run gets +the same uncharged pause as the live exit. The adapter leaves a `crash-relabeled` +lifecycle crumb (`kind`, `error`). A `Stop` clears the classification, and so does a +transcript write after the event, since the CLI recovered by itself. A crash whose result +file exists is completed as before and never relabeled. An unmatched `rate_limit` stays an +unlabeled, charged crash outside usage-relaunch mode. + +**Usage-relaunch mode.** With `[limits] usage_limit_action = "relaunch"`, primary dev and +review sessions are launched with two flags on `SessionSpec`: `usage_relaunch_mode`, and +`usage_relaunch_left` while the story has relaunches left under +`limits.max_usage_relaunches`. Then: + +- a `rate_limit` that matches neither usage list counts as a wait-class usage limit. The + session ends `stalled` + `parked` + `usage_limited` at once, with the evidence + `StopFailure(): ` and no reset. The + `stop-failure-rate-limit-unmatched` crumb carries `relaunch_mode=True`. A window that + dies after it is relabeled `usage_limited` the same way (`crash-relabeled`, + `kind="unmatched"`). This holds at the cap too, where the engine pauses uncharged at + once instead of after a stall grace. +- with relaunches left, a `rate_limit` that matched `usage_wait_patterns` ends the session + at the event instead of waiting for the quota wait (`quota-wait-skipped`, + `reason="usage-relaunch"`). + +The engine relaunches such a session without charging an attempt (see +[Usage-limit pauses](escalations-guide.md#usage-limit-pauses)). The shipped `claude` profile maps the documented values: `overloaded`, `server_error`, `unknown`, `max_output_tokens` and `invalid_request` (a context-shape failure that a fresh @@ -691,8 +722,9 @@ matched evidence. **When the session timeout fires**, one capture against both l `usage_limited` on the `timeout` result. Neither gate takes a usage match from a session that is visibly working: a transcript written within the last 90 s (as of the last heartbeat) or an unanswered tool call means the line is left over from a limit it -recovered from, and the stall or timeout is an ordinary one. A dead window is never -usage-limited. +recovered from, and the stall or timeout is an ordinary one. A dead window is +usage-limited only when it died during a quota wait or after a usage `StopFailure` (see +[StopFailure signals](#stopfailure-signals)). A `usage_limited` result is classified as an environment fault with no token-usage overrule and no tail scan, and pauses with a `usage limit:` reason (see @@ -727,6 +759,15 @@ line whose parsed reset lies beyond the remaining cap is not waited on: the sess the same way at once (`quota-wait-skipped`, `reason="reset-beyond-cap"`). An unresolved reset is admitted, and the cap bounds it. +A window that dies during the wait ends +`crashed` with the wait's evidence and reset as a wait-class `usage_limited` +(`crash-relabeled`, `kind="quota-wait"`). + +In usage-relaunch mode with relaunches left (`SessionSpec.usage_relaunch_left`), no wait +is admitted: where it would open, the session ends `stalled` + `parked` + `usage_limited` +at once (`quota-wait-skipped`, `reason="usage-relaunch"`), so the engine can relaunch the +story uncharged. At the cap the wait admits as usual. + The `opencode-http` adapter runs its own wait loop with no pane, so neither the usage gate nor the quota wait applies there. diff --git a/docs/escalations-guide.md b/docs/escalations-guide.md index 52a3b2473..27a966843 100644 --- a/docs/escalations-guide.md +++ b/docs/escalations-guide.md @@ -279,7 +279,7 @@ A session the adapter saw blocked by a coding-CLI usage limit (out of credits, o usage limit: dev session stalled (; resets