diff --git a/CHANGELOG.md b/CHANGELOG.md index 911200efb..3191ea482 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -183,6 +183,11 @@ breaking changes may land in a minor release. ### Fixed +- Let `resolve` re-arm, adopt or abandon a story left escalated after a `resume` cleared + the run's pause and its engine exited before run end (#958). Warn on `resume` and hint + in `status` and the TUI header when a story is still escalated; restore the pause + (journaling `resume-aborted`) when a resume fails before its engine starts; report a + live engine before the pause stage. - Pause a Claude session seen blocked by a usage limit (credit exhaustion or a session-window limit) with a `usage limit:` reason instead of charging a dev attempt; a live pane sighting is exempt from the token veto. Add the profile fields diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 788a7008d..66b6831b2 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -128,12 +128,14 @@ bmad-loop drives a BMAD sprint unattended: a deterministic Python loop launches - **Properties:** - A CRITICAL escalation marks the story `escalated`, sends a `CRITICAL escalation: ` notice and pauses the run at `escalation`. - A PREFERENCE escalation is journaled as `preference-escalation` and the run continues. + - Resolve also admits an unfinished story run with no pause stage and a story still escalated, as a resume whose engine exited before run end leaves it; `resume` warns of each escalated story it skips, and `status` and the TUI header name the resolve that clears it. + - A resume that fails before its engine starts puts back the pause it cleared and journals `resume-aborted`. - `--abandon` (in-progress) closes an escalated replay run unmerged, keeping its branch and parking dirty work on a preserve ref. - Re-arming resets the story to `pending` with fresh dev-attempt and review-cycle budgets. - `--reverify` re-runs `[verify]` on a deferred or environment-fault story's kept work without a dev session; on a finished run it replays a kept worktree unit in a new run. - In tickets mode the re-arm moves the ticket's plan through `tickets.py mark` and clears its `blocked_*` fields; a refused move puts the plan back byte for byte. - **Config:** `bmad-loop resolve --story`, `--no-interactive`, `--restore-patch`, `--adopt-branch`, `--reverify`, `--resume` / `--no-resume`, `--force`, `--abandon` -- **Tested by:** `tests/test_engine.py`, `tests/test_cli.py`, `tests/test_escalation.py`, `tests/test_resolve.py` +- **Tested by:** `tests/test_engine.py`, `tests/test_cli.py`, `tests/test_escalation.py`, `tests/test_resolve.py`, `tests/test_runs.py`, `tests/test_tui_app.py` - **Docs:** [Escalations](escalations-guide.md#escalations), [Resolving an escalation](escalations-guide.md#resolving-an-escalation), [Replay a finished run's kept unit](escalations-guide.md#resolve-reverify-finished), [Abandon a replay run](escalations-guide.md#abandon-a-replay-run), [Resolve in tickets mode](escalations-guide.md#resolve-in-tickets-mode), [resolve CLI reference](reference/cli.md#resolve) ### Human-only actions diff --git a/docs/escalations-guide.md b/docs/escalations-guide.md index 84f57288b..9aa2068fd 100644 --- a/docs/escalations-guide.md +++ b/docs/escalations-guide.md @@ -5,12 +5,12 @@ A run pauses when it needs a human: an escalation, an environment fault, a merge ## Where a pause shows up - **Notification.** A desktop notice (`[notify] desktop`, default on) and a line appended to `.bmad-loop/runs//ATTENTION` (`[notify] file`, default on). If neither channel works, the run warns that no alert channel is configured. Displayed text is capped at 2000 characters; the full reason is in `journal.jsonl`. -- **`bmad-loop status `** shows the pause stage, the paused story and the reason. A parked attempt's recovery ref appears as `[ref]` (`preserve_ref` in `status --json`). +- **`bmad-loop status `** shows the pause stage, the paused story and the reason. A parked attempt's recovery ref appears as `[ref]` (`preserve_ref` in `status --json`). A run with no pause stage and a story still escalated prints `escalated: story '' awaiting resolve — bmad-loop resolve `. - **The TUI** shows the same, and its keys launch the commands below: `R` resolve, `V` re-verify, `e` resume, `p` review the paused run. | Pause stage | Cause | Clear it with | | ------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------- | -| `escalation` | a CRITICAL escalation, or a story still escalated at run end | [`bmad-loop resolve`](#resolving-an-escalation) | +| `escalation` | a CRITICAL escalation, or a story still escalated at run end; with no pause stage, a story a resume left escalated | [`bmad-loop resolve`](#resolving-an-escalation) | | `environment` | an environment probe failed before a session launched | fix the environment, then `bmad-loop resume` ([details](#environment-faults)) | | `story-gate` | a ledger `gate:`, or a ledger that cannot be read; in tickets mode also a `tickets.toml` that cannot be read for its `v6_key` claims while an unlanded entry carries a v6 story-key gate | see the [sweep guide](sweep-guide.md#hard-gates) | | `spec-approval`, `epic-boundary` | a configured gate or a retrospective | `bmad-loop resume` | @@ -32,11 +32,11 @@ Escalations come in two severities: - **CRITICAL**: the story moves to `escalated`, `story-escalated` is journaled, you get a `CRITICAL escalation: ` notice, and the run pauses at `escalation`. - **PREFERENCE**: journaled as `preference-escalation`; the run continues. -A story stays escalated until you resolve it. A plain `bmad-loop resume` skips it and runs the rest of the queue, then pauses again with `story '' is still escalated — run bmad-loop resolve `. A run is never marked finished while a story is escalated. +A story stays escalated until you resolve it. A plain `bmad-loop resume` warns `story '' is still escalated — resume skips it`, skips it and runs the rest of the queue, then pauses again with `story '' is still escalated — run bmad-loop resolve `. A run is never marked finished while a story is escalated. A resume clears the pause before its engine starts, so an engine that exits before run end (killed, stopped, crashed) leaves the run with no pause stage and the story still escalated; resolve it the same way. ## Resolving an escalation -`bmad-loop resolve ` works on a run paused at `escalation` whose engine is not running. `--story ` picks a story other than the paused one. By default it asks before it re-arms and then resumes the run itself: +`bmad-loop resolve ` works on a run paused at `escalation`, or on an unfinished run with no pause stage and a story still escalated, whose engine is not running. Any other pause stage, a finished run, and a sweep with no pause stage are refused. `--story ` picks a story other than the default: the paused story, or with no pause stage the first escalated one (`--story` must then name an escalated story). By default it asks before it re-arms and then resumes the run itself: - `--resume` re-arms and resumes without asking. - `--no-resume` only re-arms and prints `resume when ready: bmad-loop resume `. diff --git a/docs/reference/cli.md b/docs/reference/cli.md index b3a2798df..3508e3587 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -210,6 +210,8 @@ bmad-loop resume [--project PROJECT] [--accept-baseline] run_id - Refused with exit 1 for a live engine, a finished run, a sweep whose ledger cannot be read, and the same git-floor, isolation, skill and plugin-trust gates as `run`. Does not require a clean worktree. - Refused with exit 1, before any side effect, for a story run whose work source this version does not know (the run was started by a newer bmad-loop) or cannot run yet. Do not downgrade bmad-loop while a run is in progress. - Discards any pending stop request and re-reads the policy and config before continuing. +- For a story run, prints `warning: run : story '' is still escalated — resume skips it; resolve it with bmad-loop resolve ` on stderr for each escalated story, then continues. +- Clears the pause before the engine starts. A resume that fails before that (an adapter, profile or multiplexer error, `error: …`) puts the cleared pause fields back and journals `resume-aborted` with `error` set to the exception class, unless the run finished or was paused again meanwhile; a restore that fails prints `could not restore the pause this resume cleared` and the original error still ends the command. After a resolve re-arm the story is already `pending`, so a plain `resume` re-drives it. ### `resolve` @@ -222,7 +224,7 @@ bmad-loop resolve [--project PROJECT] [--story STORY] [--no-interactive] | Option | Description | | -------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------- | -| `--story STORY` | Story to resolve. Default: the run's paused story. | +| `--story STORY` | Story to resolve. Default: the paused story, or with no pause stage the first escalated story. | | `--no-interactive` | Skip the resolve agent (the spec was fixed by hand), then re-arm and resume. With `--abandon`: skip the confirm prompt. | | `--restore-patch PATH` | Re-arm the spec to `in-review` and re-apply this saved patch, so review resumes on the attempted change. | | `--adopt-branch` | Commit and merge the kept worktree branch as-is and mark the story done. Worktree isolation only; not for sweeps. | @@ -234,7 +236,8 @@ bmad-loop resolve [--project PROJECT] [--story STORY] [--no-interactive] - `--restore-patch`, `--adopt-branch`, `--reverify` and `--abandon` are mutually exclusive. - Tickets mode: `--story` takes a ticket ref. The re-arm moves the plan through `tickets.py mark` to `ready-for-dev`, or `in-review` for a restore, and a refused move aborts it. Refused with exit 1 when the ticket store's layout changed since the run started. See the [tickets guide](../tickets-guide.md#parking-and-resolve). - `--reverify --story KEY` on a finished sprint run creates a new replay run for a kept DEFERRED worktree unit; the finished run is not modified. -- Refuses with exit 1, leaving the run unchanged, when the run is not paused at an escalation (`--reverify --story` accepts any pause), the engine is live, there is no escalated story, or a DEFERRED story is targeted without `--reverify`. Declining the confirm prompt exits 0 and leaves the run paused. +- Acts on a run paused at `escalation`, or on an unfinished story run with no pause stage and a story still escalated (a resume whose engine exited before run end leaves this). With no pause stage, `--story` must name an escalated story. +- Refuses with exit 1, leaving the run unchanged, in this order: the engine is live (`still live`; stop it first), the run is neither of the shapes above (any other pause stage, a finished run, a sweep with no pause stage; `--reverify --story` accepts any pause), there is no escalated story, or a DEFERRED story is targeted without `--reverify`. Declining the confirm prompt exits 0 and leaves the run paused. ### `decisions` @@ -298,6 +301,7 @@ bmad-loop status [--project PROJECT] [--json] [run_id] ``` - The status line (`status: …`) is `finished`, `PAUSED () — `, `in progress — graceful stop pending (will stop after the current item)`, or `in progress (or interrupted)`. +- For an unfinished story run with no pause stage, each story still escalated adds `escalated: story '' awaiting resolve — bmad-loop resolve ` below the status line. Not in `--json`. - For a tickets run, the footer is the tickets board, read live through the project's `tickets.py status` and pinned to the run's initiative. It shows the run's selectors, the ticket counts by state, then one block per epic in build order: done count, epic status and any recorded closure, and each ticket's state, plan status, flags and this run's phase. A tree that cannot be read prints `tickets board unavailable: `, and `status` still exits 0. Marks or store commits still owed after landing are listed last. - Exits 1 with `no runs found`, or on a bad or ambiguous ref (stdout stays empty under `--json`). diff --git a/docs/tui-guide.md b/docs/tui-guide.md index 6e20f88b5..198319cb5 100644 --- a/docs/tui-guide.md +++ b/docs/tui-guide.md @@ -232,6 +232,9 @@ Situational banners below that: a crash; the error follows on the next line. - `✖ engine gone — run was interrupted · press e to resume`: the recorded engine pid is dead. +- `escalated: story '' awaiting resolve — press R (bmad-loop resolve )`: + an unfinished run with no pause stage still has an escalated story, as a + resume whose engine exited before run end leaves it. - `⚠ auto-sweep not run: () — deferred work is untouched`: an auto-sweep was refused; run `bmad-loop sweep` with a clean worktree. A sweep that ran shows a dim `auto-sweep ran: ` line. @@ -422,6 +425,9 @@ selects the new run. `e` resumes the selected unfinished run: paused, stopped, crashed or interrupted. It refuses finished runs and runs whose state is unreadable. The confirmation shows `paused at — ` for a paused run, or `run is not paused — it looks interrupted`. +For a story run it adds a yellow +`story '' is still escalated — resume skips it; resolve it with bmad-loop resolve ` +line for each escalated story. If the original engine still appears to be running, the confirmation adds, in bold red, `engine.pid may still be live — resuming could double-drive this run`. @@ -457,8 +463,9 @@ with `run is live — stop it first`. ## Resolving an escalation (`R`) -`R` works only on a run paused at the `escalation` stage and refuses a run -whose engine may still be live. A CRITICAL escalation parks its story at +`R` works on a run paused at the `escalation` stage, or on an unfinished story +run with no pause stage and a story still escalated (it resolves the first such +story), and refuses a run whose engine may still be live. A CRITICAL escalation parks its story at `escalated`, which a plain resume skips; `R` is how you un-stick it. 1. Confirming launches `bmad-loop resolve ` in a control window and diff --git a/src/bmad_loop/checkpoints.py b/src/bmad_loop/checkpoints.py index c6fc3191c..8cc132caa 100644 --- a/src/bmad_loop/checkpoints.py +++ b/src/bmad_loop/checkpoints.py @@ -38,6 +38,7 @@ from . import gates from .engine import RunPaused +from .escalation import unresolved_escalation_keys from .model import ( PAUSE_ESCALATION, PAUSE_PLAN_CHECKPOINT, @@ -125,15 +126,9 @@ def _unresolved_escalation_key(self) -> str | None: from disk every pick (see ``_repause_inrun_escalation``), so reaching run end means the human fixed it by hand; its stale ESCALATED record must not re-pause the run, whose only way past would be a re-arm that re-drives the - fixed story.""" - return next( - ( - k - for k, t in self.state.tasks.items() - if t.phase == Phase.ESCALATED and t.attempt > 0 - ), - None, - ) + fixed story. ``escalation.unresolved_escalation_keys`` holds the narrowing + for the stories and tickets sources, so resolve's admission shares it.""" + return next(iter(unresolved_escalation_keys(self.state)), None) def _pause_wedged( self, diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index ba09120ae..dceac6cfd 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -80,9 +80,13 @@ validate_document, ) from .engine import Engine, _publication_refusal -from .escalation import display_pause_reason +from .escalation import ( + cleared_pause_escalation_keys, + display_pause_reason, + resume_skipped_escalation_keys, +) from .journal import Journal, load_state, save_state, state_lock -from .model import RunState, StoryTask +from .model import PauseSnapshot, RunState, StoryTask from .platform_util import ( MAX_SEGMENT, LockUnavailableError, @@ -5443,6 +5447,9 @@ def _prepare_resume_locked(project: Path, run_dir: Path, *, accept_baseline: boo # Likewise a tickets run paused for an epic's closure: this resume answers that # pause, so the engine acknowledges the closure instead of pausing for it again. tickets_source.resume_epic_closure(state) + # What clear_pause() is about to clear, so _resume_paused_run can put it back + # if the resume fails before its engine starts (see _restore_aborted_resume). + pause = state.pause_snapshot() state.clear_pause() runs.write_pid(run_dir) # Persist before the engine starts: status, the TUI and diagnose only ever @@ -5456,7 +5463,7 @@ def _prepare_resume_locked(project: Path, run_dir: Path, *, accept_baseline: boo # SweepEngine and _make_adapters are handed in from this module's namespace so # the test suite's `monkeypatch.setattr(cli, "SweepEngine"/"Engine"/..., ...)` # still applies. - return paths, state, pol, journal, new_digest, profiles, sweep_options, plugin_grants + return paths, state, pol, journal, new_digest, profiles, sweep_options, plugin_grants, pause class _CtlLaunch: @@ -5661,42 +5668,59 @@ def _resume_paused_run(project: Path, run_dir: Path, *, accept_baseline: bool = prepared = _prepare_resume_locked(project, run_dir, accept_baseline=accept_baseline) if isinstance(prepared, int): return prepared - paths, state, pol, journal, new_digest, profiles, sweep_options, plugin_grants = prepared + paths, state, pol, journal, new_digest, profiles, sweep_options, plugin_grants, pause = ( + prepared + ) _claim_ctl_window(project, run_dir, state) # Adapter construction and the engine lifetime are deliberately outside the # state hold. The pid/state publication above makes a rival control command # observe this process as live while these unbounded operations proceed. - composed = runsetup.compose_resume( - project=project, - paths=paths, - run_dir=run_dir, - state=state, - policy=pol, - journal=journal, - sweep_factory=_sweep_factory(project, paths, new_digest, plugin_grants=plugin_grants), - make_adapters=_make_adapters, - engine_cls=Engine, - stories_engine_cls=StoriesEngine, - tickets_engine_cls=TicketsEngine, - sweep_engine_cls=SweepEngine, - profiles=profiles, - sweep_options=sweep_options, - plugin_grants=plugin_grants, - ) - _warn_bypass_dropped( - pol, - project, - ( - ("triage", "dev", "review") - if state.run_type == "sweep" - else worksource.launch_roles( - worksource.resumed_traits_for(state.source).name, pol.gates.retrospective + try: + composed = runsetup.compose_resume( + project=project, + paths=paths, + run_dir=run_dir, + state=state, + policy=pol, + journal=journal, + sweep_factory=_sweep_factory(project, paths, new_digest, plugin_grants=plugin_grants), + make_adapters=_make_adapters, + engine_cls=Engine, + stories_engine_cls=StoriesEngine, + tickets_engine_cls=TicketsEngine, + sweep_engine_cls=SweepEngine, + profiles=profiles, + sweep_options=sweep_options, + plugin_grants=plugin_grants, + ) + _warn_bypass_dropped( + pol, + project, + ( + ("triage", "dev", "review") + if state.run_type == "sweep" + else worksource.launch_roles( + worksource.resumed_traits_for(state.source).name, pol.gates.retrospective + ) + ), + profiles=profiles, + journal=composed.journal, + ) + except BaseException as e: + # make_adapters raises SystemExit for every refusal; anything escaping + # before the engine starts must not strand the pause cleared above. A + # restore that fails is reported, never raised: the resume's own failure + # is the one the operator has to act on. + try: + _restore_aborted_resume(run_dir, journal, pause, e) + except Exception as restore_error: + print( + f"warning: run {run_dir.name}: could not restore the pause this resume" + f" cleared ({type(restore_error).__name__}); the run is left unpaused", + file=sys.stderr, ) - ), - profiles=profiles, - journal=composed.journal, - ) + raise summary = composed.engine.run() print(summary.render()) return ( @@ -5706,6 +5730,34 @@ def _resume_paused_run(project: Path, run_dir: Path, *, accept_baseline: bool = ) +def _restore_aborted_resume( + run_dir: Path, journal: Journal, pause: PauseSnapshot, error: BaseException +) -> None: + """Put back the pause a resume cleared when it fails before its engine starts. + + `_prepare_resume_locked` clears the pause fields and saves before the adapters + are built, and only the engine writes them back (on its `RunPaused`). An + adapter build that raises — `make_adapters` turns every refusal into a + `SystemExit` — therefore stranded a run paused at an escalation with no + pause stage and its story still escalated (#958). Restoring exactly the fields `clear_pause()` cleared returns the run to the + pause it was in. + + Only when nothing moved in between: the run still exists, has not finished, + and carries no pause of its own (a concurrent re-pause wins). Through + resolve's re-arm the escalated task is already PENDING, so the restored pause + plus a plain `resume` re-drives it. The journal row names the exception class + only: its text can carry profile or adapter content (see `run-resume`).""" + with state_lock(run_dir): + if not runs.is_run(run_dir): + return + fresh = load_state(run_dir) + if fresh.finished or fresh.paused_reason is not None: + return + fresh.restore_pause(pause) + save_state(run_dir, fresh) + journal.append("resume-aborted", error=type(error).__name__) + + def cmd_resume(args: argparse.Namespace) -> int: project = _project(args) try: @@ -5748,6 +5800,22 @@ def cmd_resume(args: argparse.Namespace) -> int: if (refusal := runs.unreadable_sweep_ledger(project, run_dir)) is not None: print(refusal, file=sys.stderr) return ExitCode.FAILURE + # A story still escalated is skipped and re-paused on (DW-386), and a resume + # that ends early leaves it escalated behind a run that reads as not paused + # (#958) — so name it, and the resolve that clears it. A warning, never a + # refusal, and HERE rather than in `_resume_paused_run` for the reason the + # gates above give: resolve's re-arm reaches that helper with the story already + # re-armed. An unreadable state skips the warning; the resume below reports it. + try: + skipped = resume_skipped_escalation_keys(load_state(run_dir)) + except (OSError, KeyError, ValueError): + skipped = [] + for key in skipped: + print( + f"warning: run {args.run_id}: story '{key}' is still escalated — resume skips " + f"it; resolve it with bmad-loop resolve {args.run_id}", + file=sys.stderr, + ) # No `verify.worktree_clean` gate here, unlike cmd_run / cmd_sweep — on purpose # (DW-360). Those gates prove a run LAUNCHES from a clean tree; resume continues # a run that already passed one. A dirty tree at resume is often the @@ -5953,7 +6021,7 @@ def _hand_back_ctl_window( def cmd_resolve(args: argparse.Namespace) -> int: - from .model import PAUSE_ESCALATION, Phase + from .model import Phase # DW-535: `--abandon` closes the run, so "resume after this?" has no meaning — # pairing it with either answer is a usage error, refused before even the run lookup. @@ -6001,16 +6069,9 @@ def cmd_resolve(args: argparse.Namespace) -> int: # gate (a finished run is not paused); every paused-run branch below is untouched. if getattr(args, "reverify", False) and state.finished: return _resolve_reverify_finished(args, project, run_dir, state) - # DW-522: `--reverify --story ` may name a worktree unit under ANY pause - # stage (an isolated defer never pauses the run); `reverify_refusal` decides - # whether the named story qualifies. Everything else needs the escalation pause. - reverify_named = bool(getattr(args, "reverify", False) and args.story) - if not _resolve_pause_admits(state.paused_stage, reverify_named=reverify_named): - print( - runs.not_escalation_pause_message(args.run_id, state.paused_stage), - file=sys.stderr, - ) - return 1 + # Liveness comes ahead of the pause gate below: a resume clears the pause before + # its engine starts (#958), so a resumed engine that is still running reads as + # unpaused, and the true refusal is "still live", not "not paused". # Not a cleanup path, so the "unknown must not block" invariant does not apply: # an unverifiable-but-live pid must not be re-driven. A provably-live engine # always blocks (--force never bypasses it); unknown blocks unless the operator @@ -6034,7 +6095,18 @@ def cmd_resolve(args: argparse.Namespace) -> int: "proceeding anyway (--force)", file=sys.stderr, ) - story_key = args.story or state.paused_story_key + # DW-522: `--reverify --story ` may name a worktree unit under ANY pause + # stage (an isolated defer never pauses the run); `reverify_refusal` decides + # whether the named story qualifies. Everything else needs the escalation pause, + # or no pause with the story still escalated (`runs.escalation_resolvable`, #958). + reverify_named = bool(getattr(args, "reverify", False) and args.story) + if not _resolve_pause_admits(state, reverify_named=reverify_named): + print( + runs.not_escalation_pause_message(args.run_id, state.paused_stage), + file=sys.stderr, + ) + return 1 + story_key = args.story or runs.default_escalation_key(state) task = state.tasks.get(story_key) if story_key else None reverify = getattr(args, "reverify", False) if story_key is not None and task is not None and task.phase == Phase.DEFERRED and not reverify: @@ -6046,10 +6118,13 @@ def cmd_resolve(args: argparse.Namespace) -> int: file=sys.stderr, ) return 1 + # Unpaused (#958), only an unresolved escalation is resolvable: a named stories + # attempt-0 wedge is lifted by fixing its disk state and resuming, never re-armed. if ( story_key is None or task is None or task.phase not in ((Phase.ESCALATED, Phase.DEFERRED) if reverify else (Phase.ESCALATED,)) + or (not reverify_named and not runs.escalation_story_admitted(state, story_key)) ): print(f"no escalated story to resolve in run {args.run_id}", file=sys.stderr) return 1 @@ -6059,8 +6134,8 @@ def cmd_resolve(args: argparse.Namespace) -> int: # this flow reaches it the interactive session has run and `runs.rearm_escalation` # has spent the escalation, so a refusal there is a refusal after the side # effects. LAST in this gate block, not first: the refusals above answer "this - # gesture does not apply to this run at all" (alias, not paused at an escalation, - # live engine, no escalated story) and must not be displaced by a ledger-repair + # gesture does not apply to this run at all" (alias, live engine, not paused at an + # escalation, no escalated story) and must not be displaced by a ledger-repair # steer that would not help. Mirrors `cmd_resume`, where the live-engine refusal # also wins outright and the ledger gate follows it. if (refusal := runs.unreadable_sweep_ledger(project, run_dir)) is not None: @@ -6470,7 +6545,7 @@ def cmd_resolve(args: argparse.Namespace) -> int: # mutation boundary, so repeat every state/liveness precondition from # the snapshot left by the preceding writer before restamping anything. fresh_state = load_state(run_dir, pin=state) - if fresh_state.paused_stage != PAUSE_ESCALATION: + if not runs.escalation_resolvable(fresh_state): print( runs.not_escalation_pause_message(args.run_id, fresh_state.paused_stage), file=sys.stderr, @@ -6580,7 +6655,7 @@ def _resolve_adopt( and merges the branch through its COMMITTING recovery arm. No interactive session runs, and review, the `[verify]` commands and `pre_commit_gate` workflows are NOT re-run.""" - from .model import PAUSE_ESCALATION, Phase + from .model import Phase if (refusal := runs.adopt_refusal(state, task, story_key)) is not None: print(f"error: {refusal}", file=sys.stderr) @@ -6601,7 +6676,7 @@ def _resolve_adopt( # Same mutation-boundary re-check as the re-arm path: the checks above # ran lock-free, so repeat them against the state left by the last writer. fresh_state = load_state(run_dir, pin=state) - if fresh_state.paused_stage != PAUSE_ESCALATION: + if not runs.escalation_resolvable(fresh_state): print( runs.not_escalation_pause_message(args.run_id, fresh_state.paused_stage), file=sys.stderr, @@ -6666,7 +6741,7 @@ def _resolve_abandon( a usage error), re-checks pause, liveness, phase and generation under the run lock and hands the transaction to `unitreplay.abandon_replay_run`. No session and no engine run; the board, the ledger and the origin run are never written.""" - from .model import PAUSE_ESCALATION, Phase + from .model import Phase from .unitreplay import AbandonError, abandon_refusal, abandon_replay_run if (refusal := abandon_refusal(state, task, story_key)) is not None: @@ -6697,7 +6772,7 @@ def _resolve_abandon( with state_lock(run_dir): # The checks above ran lock-free: repeat them against the last writer's state. fresh_state = load_state(run_dir, pin=state) - if fresh_state.paused_stage != PAUSE_ESCALATION: + if not runs.escalation_resolvable(fresh_state): print( runs.not_escalation_pause_message(args.run_id, fresh_state.paused_stage), file=sys.stderr, @@ -6752,16 +6827,15 @@ def _resolve_abandon( return 0 -def _resolve_pause_admits(paused_stage: str | None, *, reverify_named: bool) -> bool: - """Whether `resolve` may act on a run paused at ``paused_stage``: the escalation - pause always; any pause for `--reverify --story ` (DW-522), whose - `runs.reverify_refusal` then admits only a worktree unit off the escalation pause. - One rule for `cmd_resolve`'s entry gate and `_resolve_reverify`'s locked re-check.""" - from .model import PAUSE_ESCALATION - +def _resolve_pause_admits(state: RunState, *, reverify_named: bool) -> bool: + """Whether `resolve` may act on ``state``'s pause: `runs.escalation_resolvable` + (the escalation pause, or no pause with a story still escalated, #958); any + pause for `--reverify --story ` (DW-522), whose `runs.reverify_refusal` + then admits only a worktree unit off the escalation pause. One rule for + `cmd_resolve`'s entry gate and `_resolve_reverify`'s locked re-check.""" if reverify_named: - return paused_stage is not None - return paused_stage == PAUSE_ESCALATION + return state.paused_stage is not None + return runs.escalation_resolvable(state) def _resolve_reverify( @@ -6835,7 +6909,7 @@ def _resolve_reverify( # Same mutation-boundary re-check as the re-arm path: the checks above # ran lock-free, so repeat them against the state left by the last writer. fresh_state = load_state(run_dir, pin=state) - if not _resolve_pause_admits(fresh_state.paused_stage, reverify_named=explicit_story): + if not _resolve_pause_admits(fresh_state, reverify_named=explicit_story): print( runs.not_escalation_pause_message(args.run_id, fresh_state.paused_stage), file=sys.stderr, @@ -7897,6 +7971,10 @@ def cmd_status(args: argparse.Namespace) -> int: print("status: in progress — graceful stop pending (will stop after the current item)") else: print("status: in progress (or interrupted)") + # #958: a story left escalated behind a run that reads as not paused — the + # same predicate as resolve's admission, so the hint never names a refusal. + for key in cleared_pause_escalation_keys(state): + print(f"escalated: story '{key}' awaiting resolve — bmad-loop resolve {state.run_id}") if state.run_type == "sweep": print(_sweep_options_line(run_dir, state)) if state.sweeps_refused: diff --git a/src/bmad_loop/engine.py b/src/bmad_loop/engine.py index 4a103aece..40a2c3121 100644 --- a/src/bmad_loop/engine.py +++ b/src/bmad_loop/engine.py @@ -55,6 +55,7 @@ review_exhausted, review_retry_or_exhaust, session_failure_reason, + unresolved_escalation_keys, ) from .frontmatter import FrontmatterWriteError from .install import dev_primitive_or_default @@ -1164,11 +1165,9 @@ def run(self) -> RunSummary: def _unresolved_escalation_key(self) -> str | None: """The first ESCALATED task that still blocks ``finished`` at run end (DW-386), or None. ``StoriesEngine`` narrows it: its pick-time wedges are - disk projections a drained schedule has already cleared.""" - return next( - (k for k, t in self.state.tasks.items() if t.phase == Phase.ESCALATED), - None, - ) + disk projections a drained schedule has already cleared. The predicate is + ``escalation.unresolved_escalation_keys``, which resolve's admission shares.""" + return next(iter(unresolved_escalation_keys(self.state)), None) def _kill_run_session(self) -> None: """Tear down this run's agent session, journaling a kill the diff --git a/src/bmad_loop/escalation.py b/src/bmad_loop/escalation.py index 31f4385ec..47921c3e3 100644 --- a/src/bmad_loop/escalation.py +++ b/src/bmad_loop/escalation.py @@ -13,8 +13,16 @@ from .adapters.base import SessionResult from .gates import NOTICE_FULL_DETAIL_SOURCE, NOTICE_TRUNCATION_MARKER, notice_line -from .model import ENV_FAULT_CLAIM_LIMIT, PAUSE_ESCALATION, RunState, StoryTask, VerifyOutcome +from .model import ( + ENV_FAULT_CLAIM_LIMIT, + PAUSE_ESCALATION, + Phase, + RunState, + StoryTask, + VerifyOutcome, +) from .policy import Policy +from .worksource import SOURCE_STORIES, SOURCE_TICKETS SEVERITY_CRITICAL = "CRITICAL" SEVERITY_PREFERENCE = "PREFERENCE" @@ -186,6 +194,53 @@ def display_pause_reason(state: RunState) -> str: return display_critical_reason(reason, source) +# The sources driven by a `CheckpointFlowMixin` engine (`runsetup._engine_cls_for`), +# whose pick-time wedges are ESCALATED tasks at ``attempt == 0``. +_CHECKPOINT_FLOW_SOURCES = frozenset({SOURCE_STORIES, SOURCE_TICKETS}) + + +def unresolved_escalation_keys(state: RunState) -> list[str]: + """The ESCALATED tasks that still block ``finished`` at run end (DW-386), in + task order — the one predicate the engine's run-end re-pause and resolve's + admission share, so the two cannot drift. + + A stories or tickets run counts only an in-run escalation (``attempt > 0``): + its pick-time wedges (``attempt == 0``) are re-classified from disk every + pick, so a stale ESCALATED record of one is not an escalation a human still + owes (``CheckpointFlowMixin._unresolved_escalation_key``).""" + checkpoint_flow = state.source in _CHECKPOINT_FLOW_SOURCES + return [ + key + for key, task in state.tasks.items() + if task.phase == Phase.ESCALATED and (task.attempt > 0 or not checkpoint_flow) + ] + + +def resume_skipped_escalation_keys(state: RunState) -> list[str]: + """The escalated stories a ``resume`` of ``state`` skips and re-pauses on + (DW-386), in task order — what ``resume`` and the TUI's resume confirm warn + about. Empty for a finished run and for a sweep: a sweep's migrate/triage + ESCALATED tasks are cleared by a plain resume, so there is nothing to warn of.""" + if state.finished or state.run_type == "sweep": + return [] + return unresolved_escalation_keys(state) + + +def cleared_pause_escalation_keys(state: RunState) -> list[str]: + """The escalated stories of a run whose pause fields are null (#958): a resume + clears the pause before its engine starts, so an exit before run end leaves a + story ESCALATED behind a run that reads as not paused. + + The one predicate behind ``runs.escalation_resolvable``'s null-stage admission + and the ``status`` / TUI header hint, so the hint never names a run resolve + refuses. A run paused at the escalation stage already says so, and any other + stage (``environment``, a gate, a checkpoint) refuses resolve, so both are + empty here.""" + if state.paused_stage is not None: + return [] + return resume_skipped_escalation_keys(state) + + def preference_escalations(result_json: dict[str, Any] | None) -> list[dict[str, Any]]: return [ e diff --git a/src/bmad_loop/model.py b/src/bmad_loop/model.py index e6f9efbd1..1ccf12901 100644 --- a/src/bmad_loop/model.py +++ b/src/bmad_loop/model.py @@ -1268,6 +1268,21 @@ def env_fault_site_reverifiable(task: StoryTask) -> bool: return latest is not None and latest.status == "completed" +@dataclass(frozen=True) +class PauseSnapshot: + """The six fields :meth:`RunState.clear_pause` clears, captured just before a + resume clears them so a resume that fails before its engine starts can put + them back (:meth:`RunState.restore_pause`). Without it that failure left an + escalated story behind a run with no pause stage, which ``resolve`` refused.""" + + paused_reason: str | None + paused_stage: str | None + paused_story_key: str | None + stopped: bool + crashed: bool + crash_error: str | None + + @dataclass class RunState: run_id: str @@ -1556,6 +1571,26 @@ def clear_pause(self) -> None: self.crashed = False self.crash_error = None + def pause_snapshot(self) -> PauseSnapshot: + """Exactly the fields :meth:`clear_pause` clears, for :meth:`restore_pause`.""" + return PauseSnapshot( + paused_reason=self.paused_reason, + paused_stage=self.paused_stage, + paused_story_key=self.paused_story_key, + stopped=self.stopped, + crashed=self.crashed, + crash_error=self.crash_error, + ) + + def restore_pause(self, snapshot: PauseSnapshot) -> None: + """Undo a :meth:`clear_pause` from the :meth:`pause_snapshot` taken before it.""" + self.paused_reason = snapshot.paused_reason + self.paused_stage = snapshot.paused_stage + self.paused_story_key = snapshot.paused_story_key + self.stopped = snapshot.stopped + self.crashed = snapshot.crashed + self.crash_error = snapshot.crash_error + def cache_read_weight(self) -> float: """The run's cache-read weight from its persisted policy snapshot; the product default (policy.LimitsPolicy.cache_read_weight = 0.1) when the diff --git a/src/bmad_loop/resolve.py b/src/bmad_loop/resolve.py index e48ef481e..fe2b15829 100644 --- a/src/bmad_loop/resolve.py +++ b/src/bmad_loop/resolve.py @@ -27,9 +27,10 @@ from .engine import _session_task_id from .escalation import critical_escalations from .journal import TASK_CYCLE_ARTIFACTS, Journal -from .model import RunState, StoryTask +from .model import PAUSE_ESCALATION, RunState, StoryTask from .platform_util import safe_segment from .runs import ( + journal_entries_or_none, live_stories_root, rebase_recorded_project_path, redrive_base_ref, @@ -294,6 +295,27 @@ def _gather_escalations( return list(found.values()), sum(1 for key in answered if key not in found) +def escalation_reason(state: RunState, run_dir: Path, story_key: str) -> str | None: + """The reason ``story_key`` escalated, for the resolve context's ``paused_reason``. + + The run's ``paused_reason`` answers only while the run is paused at the escalation + stage on THIS story. A resume clears the pause before its engine starts, so a run + left with the story still escalated has none (#958), and resolving ``--story B`` + while paused on A must not hand B's session A's reason. Otherwise the reason is + the last ``story-escalated`` journal row for the story; None when the journal is + unreadable or holds no such row, which the skill reports as a malformed context.""" + if state.paused_stage == PAUSE_ESCALATION and state.paused_story_key == story_key: + return state.paused_reason + entries = journal_entries_or_none(run_dir) + if entries is None: + return None + for entry in reversed(entries): + if entry.get("kind") == "story-escalated" and entry.get("story_key") == story_key: + reason = entry.get("reason") + return reason if isinstance(reason, str) else None + return None + + def build_context( state: RunState, run_dir: Path, @@ -401,7 +423,7 @@ def build_context( # spelling for every reader — and consumed by an agent, which accepts '/'. "spec_file": context_spec_path.as_posix() if context_spec_path is not None else None, "baseline_commit": task.baseline_commit if task else None, - "paused_reason": state.paused_reason, + "paused_reason": escalation_reason(state, run_dir, story_key), # Where the escalation's environment fault was detected (DW-523), a closed # `model.ENV_FAULT_SITES` slug; None when the escalation is not one. Set, # it tells the session the spec needs no change: the environment does. diff --git a/src/bmad_loop/runs.py b/src/bmad_loop/runs.py index dcca48447..97d8a536b 100644 --- a/src/bmad_loop/runs.py +++ b/src/bmad_loop/runs.py @@ -51,6 +51,7 @@ get_multiplexer, mux_usable, ) +from .escalation import cleared_pause_escalation_keys, unresolved_escalation_keys from .frontmatter import auto_dev_baseline_of, parse_frontmatter, status_of from .journal import STATE_FILE, VERIFY_DIR, Journal, load_state, save_state, state_lock from .model import ( @@ -4636,6 +4637,50 @@ def not_escalation_pause_message(run_id: str, paused_stage: str | None) -> str: return message +def escalation_resolvable(state: RunState) -> bool: + """Whether a resolve path (re-arm, adopt, abandon) admits ``state``. + + A run paused at the escalation stage is admitted, as before. So is a run + whose pause fields are null while an escalation it owes is still unresolved + (#958): ``resume`` clears the pause before its engine starts, and only a + run-end or in-run ``RunPaused`` writes it back, so any exit in between — a + kill, a stop, a crash, an adapter-build failure — leaves an ESCALATED task + behind a run that reads as not paused. Every later resume re-pauses on it, + so without this admission the run has no way out. + + The null-stage arm excludes a finished run, and a sweep: a sweep's + migrate/triage ESCALATED tasks are cleared by a plain resume, so a sweep + resolve keeps stage-only admission. Any other stage (``environment``, a + story gate, a checkpoint) keeps its refusal: a resolve that resumes would + clear that pause and silently approve what it gates.""" + if state.paused_stage == PAUSE_ESCALATION: + return True + return bool(cleared_pause_escalation_keys(state)) + + +def default_escalation_key(state: RunState) -> str | None: + """The story a resolve path acts on when no story is named: the paused story + at the escalation stage, else the first unresolved escalation under the + null-stage admission of :func:`escalation_resolvable` (#958), else None.""" + if state.paused_stage == PAUSE_ESCALATION: + return state.paused_story_key + if escalation_resolvable(state): + return unresolved_escalation_keys(state)[0] + return None + + +def escalation_story_admitted(state: RunState, story_key: str) -> bool: + """Whether a resolve path may act on ``story_key`` in ``state``, which + :func:`escalation_resolvable` already admits. At the escalation stage any + story passes here and meets the path's own per-task refusals; under the + null-stage admission (#958) only an unresolved escalation does, so a named + attempt-0 stories wedge or a non-escalated task is refused rather than + re-armed past the run's other stages.""" + if state.paused_stage == PAUSE_ESCALATION: + return True + return story_key in unresolved_escalation_keys(state) + + def validate_restore_latch( state: RunState, task: StoryTask, story_key: str, *, worktree_isolation: bool = False ) -> str | None: @@ -6417,14 +6462,18 @@ def adopt_escalated_branch(run_dir: Path, story_key: str | None = None) -> str: the existing commit-window recovery path, so no session runs and review is deliberately NOT re-run. Does not clear the pause; the caller resumes. + Admission is `escalation_resolvable`: paused at the escalation stage, or not + paused while the story is still an unresolved escalation (#958), and the + default story is `default_escalation_key`. + Returns the adopted branch. Raises RearmError, with state untouched, when the - run is not paused at an escalation, the story is not ESCALATED, or - `adopt_refusal` names a reason.""" + run is not admitted, the story is not ESCALATED (or, unpaused, not an + unresolved escalation), or `adopt_refusal` names a reason.""" with state_lock(run_dir): state = load_state(run_dir) - if state.paused_stage != PAUSE_ESCALATION: + if not escalation_resolvable(state): raise RearmError(not_escalation_pause_message(run_dir.name, state.paused_stage)) - key = story_key or state.paused_story_key + key = story_key or default_escalation_key(state) if key is None: raise RearmError(f"run {run_dir.name} has no escalated story to resolve") task = state.tasks.get(key) @@ -6432,6 +6481,8 @@ def adopt_escalated_branch(run_dir: Path, story_key: str | None = None) -> str: raise RearmError(f"run {run_dir.name} has no task for story {key}") if task.phase != Phase.ESCALATED: raise RearmError(f"story {key} is not escalated (phase: {task.phase})") + if not escalation_story_admitted(state, key): + raise RearmError(not_escalation_pause_message(run_dir.name, state.paused_stage)) if (refusal := adopt_refusal(state, task, key)) is not None: raise RearmError(refusal) # Deliberate direct assignment, not a state-machine transition: ESCALATED has @@ -6465,7 +6516,13 @@ def _rearm_escalation_locked( resolution_recorded: bool, project_root: Path | None = None, ) -> RearmOutcome: - """Re-arm an escalation-paused story so the next resume re-drives it. + """Re-arm an escalated story so the next resume re-drives it. + + Admission is `escalation_resolvable`: the run is paused at the escalation stage, + or not paused while the story is still an unresolved escalation — a resume + clears the pause before its engine starts, so an exit before run end leaves it + null with the story still ESCALATED (#958). The default story is + `default_escalation_key`. Flips the escalated task out of its terminal ESCALATED phase back to PENDING — which makes `_finish_inflight` reset the tree to the story's @@ -6579,15 +6636,16 @@ def _rearm_escalation_locked( Returns the authoritative re-arm outcome: the story key, the ordered notices whose journal appends succeeded during this call, and whether one of the appended - records holds the combined re-arm/resume gesture. Raises RearmError when the run is not paused at - the escalation stage, the target story is not escalated, or a supplied + records holds the combined re-arm/resume gesture. Raises RearmError when the run is not + admitted, the target story is not escalated (or, unpaused, not an unresolved + escalation), or a supplied `restore_patch` fails `validate_restore_latch` (the shared precondition set — sentinel wedge, spec-less escalation, worktree isolation). """ state = load_state(run_dir) - if state.paused_stage != PAUSE_ESCALATION: + if not escalation_resolvable(state): raise RearmError(not_escalation_pause_message(run_dir.name, state.paused_stage)) - key = story_key or state.paused_story_key + key = story_key or default_escalation_key(state) if key is None: raise RearmError(f"run {run_dir.name} has no escalated story to resolve") task = state.tasks.get(key) @@ -6595,6 +6653,8 @@ def _rearm_escalation_locked( raise RearmError(f"run {run_dir.name} has no task for story {key}") if task.phase != Phase.ESCALATED: raise RearmError(f"story {key} is not escalated (phase: {task.phase})") + if not escalation_story_admitted(state, key): + raise RearmError(not_escalation_pause_message(run_dir.name, state.paused_stage)) # Patch-restore preconditions (T1 guard + spec-less wedge + worktree isolation), # rejected here before any task mutation so the escalation stays armed for a # corrected resolve. `cli._resolve_restore_patch` runs the same validator ahead diff --git a/src/bmad_loop/tui/app.py b/src/bmad_loop/tui/app.py index 0fdb29ac0..ba9b9ecb2 100644 --- a/src/bmad_loop/tui/app.py +++ b/src/bmad_loop/tui/app.py @@ -804,7 +804,10 @@ def action_resolve_run(self) -> None: markup=False, ) return - if state.paused_stage != "escalation": + # Admission is `runs.escalation_resolvable`, the CLI's own rule: paused at + # the escalation stage, or not paused while a story is still escalated — + # a resume clears the pause before its engine starts (#958). + if not runs.escalation_resolvable(state): self.notify( "resolve is only available for a run paused at an escalation" + (f"\n{pointer}" if pointer else ""), @@ -817,7 +820,7 @@ def action_resolve_run(self) -> None: if _engine_possibly_live(run_dir): self.notify(f"run {run_id} may still be live — stop it first", severity="warning") return - story = state.paused_story_key or "?" + story = runs.default_escalation_key(state) or "?" self.push_screen( ConfirmModal( @@ -1656,7 +1659,7 @@ def _do_rearm( fresh_state = load_state(run_dir, pin=recorded) fresh_task = fresh_state.tasks.get(story_key) if ( - fresh_state.paused_stage != PAUSE_ESCALATION + not runs.escalation_resolvable(fresh_state) or fresh_task is None or fresh_task.phase != Phase.ESCALATED ): diff --git a/src/bmad_loop/tui/screens/modals.py b/src/bmad_loop/tui/screens/modals.py index 5f3f8ad5d..5dc7a37bc 100644 --- a/src/bmad_loop/tui/screens/modals.py +++ b/src/bmad_loop/tui/screens/modals.py @@ -22,7 +22,7 @@ from textual.widgets import Button, Checkbox, Input, Label, Select, Static from ... import stories, tickets_source, worksource -from ...escalation import display_pause_reason +from ...escalation import display_pause_reason, resume_skipped_escalation_keys from ...model import RunState from .. import data, widgets @@ -490,6 +490,14 @@ def __init__(self, run_id: str, state: RunState, engine_alive: bool): body.append(f" — {display_pause_reason(state)}", style="yellow") else: body.append("run is not paused — it looks interrupted", style="dim") + # The same fact `bmad-loop resume` warns of: a still-escalated story is + # skipped and re-paused on (DW-386); resolve (R) is what clears it. + for key in resume_skipped_escalation_keys(state): + body.append( + f"\nstory '{key}' is still escalated — resume skips it; " + f"resolve it with bmad-loop resolve {run_id}", + style="yellow", + ) warning = ( "engine.pid may still be live — resuming could double-drive this run" if engine_alive diff --git a/src/bmad_loop/tui/widgets.py b/src/bmad_loop/tui/widgets.py index 782756984..abcd29a58 100644 --- a/src/bmad_loop/tui/widgets.py +++ b/src/bmad_loop/tui/widgets.py @@ -26,7 +26,7 @@ from textual.widgets.tree import TreeNode from .. import policy -from ..escalation import display_pause_reason +from ..escalation import cleared_pause_escalation_keys, display_pause_reason from ..journal import UNREADABLE_LINE_KIND from ..model import ( PAUSE_ENVIRONMENT, @@ -352,6 +352,14 @@ def show_run( "\n✖ engine gone — run was interrupted · press e to resume", style="bold red", ) + # #958: a story left escalated behind a run that reads as not paused — + # `cmd_status`'s hint, under resolve's own admission predicate. + for key in cleared_pause_escalation_keys(state): + text.append( + f"\nescalated: story '{key}' awaiting resolve — press R " + f"(bmad-loop resolve {state.run_id})", + style="bold yellow", + ) if decision is not None and status not in ( data.FINISHED, data.INTERRUPTED, diff --git a/src/bmad_loop/unitreplay.py b/src/bmad_loop/unitreplay.py index 7e7ff6a67..5b98ea6ca 100644 --- a/src/bmad_loop/unitreplay.py +++ b/src/bmad_loop/unitreplay.py @@ -74,7 +74,7 @@ from . import runs, runsetup, verify, worksource from .bmadconfig import ProjectPaths from .journal import Journal, load_state, save_state, state_lock -from .model import PAUSE_ESCALATION, Phase, RunState, StoryTask +from .model import Phase, RunState, StoryTask from .platform_util import pinned_root_identity, root_identity_record, safe_segment from .policy import Policy from .workspace import ( @@ -369,16 +369,20 @@ def abandon_replay_run(run_dir: Path, story_key: str, *, paths: ProjectPaths) -> between the journal and the save, with a second ``replay-abandoned`` row). ``paths`` is the current config: ``paths.repo_root`` is the repository the worktree is registered with (as `WorktreeFlow.reopen_unit` reads it). Liveness and the task generation are the CLI caller's locked checks. + Admission is `runs.escalation_resolvable`, so a replay run a resume left + unpaused with its unit still escalated (#958) is closable too. Raises AbandonError.""" with state_lock(run_dir): state = load_state(run_dir) - if state.paused_stage != PAUSE_ESCALATION: + if not runs.escalation_resolvable(state): raise AbandonError(runs.not_escalation_pause_message(run_dir.name, state.paused_stage)) task = state.tasks.get(story_key) if task is None: raise AbandonError(f"run {run_dir.name} has no task for story {story_key}") if (refusal := abandon_refusal(state, task, story_key)) is not None: raise AbandonError(refusal) + if not runs.escalation_story_admitted(state, story_key): + raise AbandonError(runs.not_escalation_pause_message(run_dir.name, state.paused_stage)) repo_root = paths.repo_root # The recorded mount is launch-time: after a project move, the bare path names # a tree that is gone and would send a standing worktree down the diff --git a/tests/conftest.py b/tests/conftest.py index 9557ff397..4178b3a5c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3499,6 +3499,16 @@ def escalated_run( return EscalatedRun(run_dir=run_dir, state=state, task=task) +def clear_escalation_pause(run: EscalatedRun) -> EscalatedRun: + """Persist ``run`` with its pause cleared and its task still ESCALATED: the + state a resume leaves when it clears the pause and its engine exits before the + run ends (#958). Mutates and returns ``run`` so a caller can chain it on + `escalated_run`.""" + run.state.clear_pause() + save_state(run.run_dir, run.state) + return run + + # DW-522: an in-place run paused on a story whose attempt committed above its # baseline — shared by the `rearm_for_reverify` (tests/test_runs.py) and # `resolve --reverify` (tests/test_cli.py) tests. diff --git a/tests/test_cli.py b/tests/test_cli.py index aff4d9d4a..f50653e33 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -31,6 +31,7 @@ FakeTicketTree, _reverify_run, assert_run_state_lock_held, + clear_escalation_pause, escalated_run, fake_find, fake_next_view, @@ -5099,6 +5100,354 @@ def test_resolve_refuses_environment_pause_with_resume_hint(tmp_path, capsys): assert "bmad-loop resume" not in err +def _cleared_escalated_run(project, run_id="r1", **kw): + """`_escalated_run` after a resume cleared its pause and the engine exited before + run end: unpaused, the task still ESCALATED (#958).""" + return clear_escalation_pause(escalated_run(project, run_id, **kw)).run_dir + + +def test_resolve_no_interactive_rearms_after_resume_cleared_pause(tmp_path, monkeypatch, capsys): + """#958: a resume cleared the pause and its engine never reached run end, so the + run is unpaused while the story stays escalated. `resolve` re-arms it. + + Ablation: restore `_resolve_pause_admits`' stage-only rule and this exits 1 with + "not paused at an escalation (stage: none)".""" + from bmad_loop.journal import load_state + from bmad_loop.model import Phase + + run_dir = _cleared_escalated_run(tmp_path) + monkeypatch.setattr(cli, "_resume_paused_run", lambda *_a: pytest.fail("resumed")) + rc = cli.main(["resolve", "--project", str(tmp_path), "r1", "--no-interactive", "--no-resume"]) + out = capsys.readouterr() + assert rc == 0, out.err + assert "resume when ready" in out.out + assert load_state(run_dir).tasks["s1"].phase == Phase.PENDING + + +def test_resolve_default_story_after_cleared_pause(tmp_path, monkeypatch, capsys): + """Unpaused, the default story is the first unresolved escalation, and a named + `--story` must be one: a non-escalated task is refused. + + Ablation: drop the `escalation_story_admitted` clause in `cmd_resolve` and the + attempt-0 wedge row re-arms.""" + from bmad_loop.journal import load_state, save_state + from bmad_loop.model import Phase, StoryTask + + run_dir = _cleared_escalated_run(tmp_path, story_key="s2") + state = load_state(run_dir) + state.tasks = { + "s1": StoryTask(story_key="s1", epic=1, phase=Phase.DONE, attempt=1), + **state.tasks, + } + save_state(run_dir, state) + monkeypatch.setattr(cli, "_resume_paused_run", lambda *_a: pytest.fail("resumed")) + args = ["resolve", "--project", str(tmp_path), "r1", "--no-interactive", "--no-resume"] + + assert cli.main([*args, "--story", "s1"]) == 1 + assert "no escalated story to resolve in run r1" in capsys.readouterr().err + + assert cli.main(args) == 0, capsys.readouterr().err + saved = load_state(run_dir) + assert saved.tasks["s2"].phase == Phase.PENDING + assert saved.tasks["s1"].phase == Phase.DONE + + +def test_resolve_cleared_pause_refuses_a_named_stories_wedge(tmp_path, monkeypatch, capsys): + """A stories run's attempt-0 wedge is lifted by a hand fix and a resume; naming it + on an unpaused run that also holds an in-run escalation is refused, not re-armed. + + Ablation: drop the `escalation_story_admitted` clause in `cmd_resolve` and this + exits 0 with the wedge re-armed.""" + from bmad_loop.journal import load_state, save_state + from bmad_loop.model import Phase, StoryTask + + run_dir = _cleared_escalated_run(tmp_path, source="stories") + state = load_state(run_dir) + state.tasks["w"] = StoryTask(story_key="w", epic=1, phase=Phase.ESCALATED, attempt=0) + save_state(run_dir, state) + monkeypatch.setattr(cli, "_resume_paused_run", lambda *_a: pytest.fail("resumed")) + rc = cli.main( + [ + "resolve", + "--project", + str(tmp_path), + "r1", + "--story", + "w", + "--no-interactive", + "--no-resume", + ] + ) + assert rc == 1 + assert "no escalated story to resolve in run r1" in capsys.readouterr().err + assert load_state(run_dir).tasks["w"].phase == Phase.ESCALATED + + +def test_resolve_environment_pause_still_refused_with_escalated_task(tmp_path, monkeypatch, capsys): + """Task-keyed admission applies only to a null stage: an environment pause is + lifted by a plain resume, even with an ESCALATED task on record. + + Ablation: admit any stage with an unresolved escalation in + `runs.escalation_resolvable` and this re-arms.""" + from bmad_loop.journal import load_state, save_state + from bmad_loop.model import Phase + + run_dir = _escalated_run(tmp_path, "r1") + state = load_state(run_dir) + state.paused_stage = "environment" + save_state(run_dir, state) + monkeypatch.setattr(cli, "_resume_paused_run", lambda *_a: pytest.fail("resumed")) + rc = cli.main(["resolve", "--project", str(tmp_path), "r1", "--no-interactive", "--no-resume"]) + err = capsys.readouterr().err + assert rc == 1 + assert "not paused at an escalation (stage: environment)" in err + assert "an environment pause needs no resolve" in err + assert load_state(run_dir).tasks["s1"].phase == Phase.ESCALATED + + +def test_resolve_cleared_pause_refuses_sweep_and_finished(tmp_path, monkeypatch, capsys): + """An unpaused sweep keeps stage-only admission, and a finished run still refuses + with the existing message.""" + from bmad_loop.journal import load_state, save_state + + _cleared_escalated_run(tmp_path, "r1", run_type="sweep") + finished = _cleared_escalated_run(tmp_path, "r2") + state = load_state(finished) + state.finished = True + save_state(finished, state) + monkeypatch.setattr(cli, "_resume_paused_run", lambda *_a: pytest.fail("resumed")) + for run_id in ("r1", "r2"): + rc = cli.main( + ["resolve", "--project", str(tmp_path), run_id, "--no-interactive", "--no-resume"] + ) + assert rc == 1 + assert f"run {run_id} is not paused at an escalation (stage: none)" in ( + capsys.readouterr().err + ) + + +def test_resolve_reports_live_engine_before_stage(tmp_path, monkeypatch, capsys): + """A resumed engine that is still running reads as unpaused, because the resume + cleared the pause before it started: resolve says "still live", not "not paused". + + Ablation: move the liveness block back below the pause gate and this reports + "not paused at an escalation (stage: none)".""" + from bmad_loop import runs + from bmad_loop.model import Phase, StoryTask + + _make_run_with_state( + tmp_path, "r1", tasks={"s1": StoryTask(story_key="s1", epic=1, phase=Phase.DEV_RUNNING)} + ) + monkeypatch.setattr(runs, "engine_liveness", lambda _rd: "alive") + assert cli.main(["resolve", "--project", str(tmp_path), "r1"]) == 1 + err = capsys.readouterr().err + assert "run r1 is still live — stop it first" in err + assert "not paused at an escalation" not in err + + +def test_resume_warns_on_escalated_task(tmp_path, monkeypatch, capsys): + """A resume skips a still-escalated story and re-pauses on it, so it names the + story and the resolve that clears it — paused at the escalation or, after an + earlier resume cleared the pause, not (#958). A warning: the resume proceeds. + + Ablation: drop the warning loop in `cmd_resume` and neither line prints.""" + from bmad_loop import runs + + reached: list[str] = [] + monkeypatch.setattr(runs, "engine_liveness", lambda _rd: "dead") + monkeypatch.setattr( + cli, "_resume_paused_run", lambda _p, run_dir, **_k: reached.append(run_dir.name) or 0 + ) + _escalated_run(tmp_path, "r1") + _cleared_escalated_run(tmp_path, "r2", story_key="7-7") + for run_id, story in (("r1", "s1"), ("r2", "7-7")): + assert cli.main(["resume", "--project", str(tmp_path), run_id]) == 0 + assert ( + f"warning: run {run_id}: story '{story}' is still escalated — resume skips it; " + f"resolve it with bmad-loop resolve {run_id}" + ) in capsys.readouterr().err + assert reached == ["r1", "r2"] + + +def test_resume_no_warning_for_sweep(tmp_path, monkeypatch, capsys): + """A sweep's migrate/triage ESCALATED tasks are cleared by a plain resume, so a + sweep resume warns of nothing. + + Ablation: drop the sweep clause of `resume_skipped_escalation_keys` and the + warning prints here.""" + from bmad_loop import runs + + monkeypatch.setattr(runs, "engine_liveness", lambda _rd: "dead") + monkeypatch.setattr(runs, "unreadable_sweep_ledger", lambda *_a: None) + monkeypatch.setattr(cli, "_resume_paused_run", lambda *_a, **_k: 0) + _escalated_run(tmp_path, "r1", run_type="sweep") + assert cli.main(["resume", "--project", str(tmp_path), "r1"]) == 0 + assert "still escalated" not in capsys.readouterr().err + + +def _failing_resume_run(project, monkeypatch, *, before_raise=None, **state_fields): + """An escalation-paused run under a real BMAD config whose resume fails while + building its adapters — `make_adapters`' own failure shape, a `SystemExit` — + after `_prepare_resume_locked` cleared and saved the pause. `state_fields` + override the saved state; `before_raise(run_dir)` runs inside the failing build.""" + from conftest import install_base_skills + + from bmad_loop.journal import load_state, save_state + + install_bmad_config(project) + install_base_skills(project) + run_dir = _escalated_run(project.project) + if state_fields: + state = load_state(run_dir) + for name, value in state_fields.items(): + setattr(state, name, value) + save_state(run_dir, state) + monkeypatch.setattr(runs, "engine_liveness", lambda _rd: "dead") + + def failing_build(*_a, **_k): + if before_raise is not None: + before_raise(run_dir) + raise SystemExit("error: x") + + monkeypatch.setattr(cli, "_make_adapters", failing_build) + return run_dir + + +def _journal_kinds(run_dir): + return [ + json.loads(line)["kind"] for line in (run_dir / "journal.jsonl").read_text().splitlines() + ] + + +def test_resume_restores_pause_when_adapter_build_fails(project, monkeypatch, capsys): + """A resume clears the pause before its adapters are built; one whose build + fails puts the pause back and journals `resume-aborted` with the exception class + only, so the run stays paused at its escalation and `resolve` still takes it. + + Ablation: drop the `except BaseException` arm in `_resume_paused_run` and the + pause stays cleared and no `resume-aborted` row lands.""" + from bmad_loop.journal import load_state + from bmad_loop.model import Phase + + run_dir = _failing_resume_run(project, monkeypatch) + before = load_state(run_dir) + with pytest.raises(SystemExit, match="error: x"): + cli.main(["resume", "--project", str(project.project), "r1"]) + after = load_state(run_dir) + assert (after.paused_reason, after.paused_stage, after.paused_story_key) == ( + before.paused_reason, + before.paused_stage, + before.paused_story_key, + ) + assert after.paused_stage == "escalation" + rows = [ + json.loads(line) + for line in (run_dir / "journal.jsonl").read_text().splitlines() + if json.loads(line)["kind"] == "resume-aborted" + ] + assert [{k: v for k, v in r.items() if k != "ts"} for r in rows] == [ + {"kind": "resume-aborted", "error": "SystemExit"} + ] + capsys.readouterr() + monkeypatch.setattr(cli, "_resume_paused_run", lambda *_a: pytest.fail("resumed")) + rc = cli.main( + ["resolve", "--project", str(project.project), "r1", "--no-interactive", "--no-resume"] + ) + assert rc == 0, capsys.readouterr().err + assert load_state(run_dir).tasks["s1"].phase == Phase.PENDING + + +def test_resume_restore_keeps_stopped_crashed_flags(project, monkeypatch): + """The restore puts back every field `clear_pause()` cleared, the stop and crash + flags with the pause fields. + + Ablation: drop the `stopped`/`crashed`/`crash_error` lines from + `RunState.restore_pause` and they come back cleared.""" + from bmad_loop.journal import load_state + + run_dir = _failing_resume_run( + project, monkeypatch, stopped=True, crashed=True, crash_error="RuntimeError: boom" + ) + with pytest.raises(SystemExit): + cli.main(["resume", "--project", str(project.project), "r1"]) + after = load_state(run_dir) + assert (after.stopped, after.crashed, after.crash_error) == (True, True, "RuntimeError: boom") + assert after.paused_stage == "escalation" + + +@pytest.mark.parametrize("change", ["re-paused", "finished"]) +def test_resume_restore_skips_when_state_changed(project, monkeypatch, change): + """A run that moved while the adapters were being built — re-paused by a rival, + or finished — keeps its new state: the restore writes nothing and journals no + `resume-aborted`. + + Ablation: drop the `finished` / `paused_reason` check in + `_restore_aborted_resume` and the escalation pause overwrites the new state.""" + from bmad_loop.journal import load_state, save_state + + def move(run_dir): + state = load_state(run_dir) + if change == "re-paused": + state.paused_reason, state.paused_stage = "environment fault", "environment" + else: + state.finished = True + save_state(run_dir, state) + + run_dir = _failing_resume_run(project, monkeypatch, before_raise=move) + with pytest.raises(SystemExit): + cli.main(["resume", "--project", str(project.project), "r1"]) + after = load_state(run_dir) + if change == "re-paused": + assert (after.paused_reason, after.paused_stage) == ("environment fault", "environment") + else: + assert after.finished and after.paused_reason is None and after.paused_stage is None + assert "resume-aborted" not in _journal_kinds(run_dir) + + +def test_resume_restore_failure_keeps_the_resume_error(project, monkeypatch, capsys): + """A restore that cannot read the state warns and lets the resume's own failure + propagate, rather than replacing it with the restore's fault. + + Ablation: drop the `try`/`except Exception` around `_restore_aborted_resume` and + the KeyError from the unreadable state escapes instead of the SystemExit.""" + + def corrupt(run_dir): + (run_dir / "state.json").write_text("{}", encoding="utf-8") + + _failing_resume_run(project, monkeypatch, before_raise=corrupt) + with pytest.raises(SystemExit, match="error: x"): + cli.main(["resume", "--project", str(project.project), "r1"]) + assert ( + "warning: run r1: could not restore the pause this resume cleared (KeyError); " + "the run is left unpaused" + ) in capsys.readouterr().err + + +def test_status_hints_escalated_story_after_cleared_pause(tmp_path, capsys): + """#958: a run that reads as not paused while a story is still escalated names + the story and the resolve that clears it. + + Ablation: drop the hint loop in `cmd_status` and the line is absent.""" + _cleared_escalated_run(tmp_path, "r1", story_key="7-7") + assert cli.main(["status", "--project", str(tmp_path), "r1"]) == 0 + out = capsys.readouterr().out + assert "status: in progress (or interrupted)" in out + assert "escalated: story '7-7' awaiting resolve — bmad-loop resolve r1" in out + + +def test_status_no_hint_when_paused_at_escalation(tmp_path, capsys): + """A run paused at the escalation already says so on its status line; an + unpaused sweep is resolved by a plain resume. Neither gets the hint. + + Ablation: drop the `paused_stage` clause of `cleared_pause_escalation_keys` and + the paused row prints it; drop its sweep clause and the sweep row does.""" + _escalated_run(tmp_path, "r1") + _cleared_escalated_run(tmp_path, "r2", run_type="sweep") + for run_id in ("r1", "r2"): + assert cli.main(["status", "--project", str(tmp_path), run_id]) == 0 + assert "awaiting resolve" not in capsys.readouterr().out + + # resolve refuses 'unknown' too, not just 'alive' — re-driving a possibly-live engine. @pytest.mark.parametrize( "liveness,msg", @@ -5407,6 +5756,32 @@ def test_resolve_adopt_branch_moves_to_committing_and_resumes(tmp_path, monkeypa assert "review" in err and "[verify]" in err and "pre_commit_gate" in err +def test_resolve_adopt_branch_admits_cleared_escalation_pause(tmp_path, monkeypatch, capsys): + """#958: a resume clears the pause before its engine starts, so an engine exit + before run end leaves the pause null with the task still ESCALATED. + `resolve --adopt-branch` admits that run at its entry gate and under the lock, + defaulting to the unresolved escalation. + + Ablation, performed: restore the stage-only `paused_stage != PAUSE_ESCALATION` + re-check in `_resolve_adopt` and this reddens on rc.""" + from bmad_loop.journal import load_state, save_state + from bmad_loop.model import Phase + + run_dir = _adoptable_run(tmp_path) + state = load_state(run_dir) + state.clear_pause() + save_state(run_dir, state) + resumed = [] + monkeypatch.setattr(cli, "_resume_paused_run", lambda proj, rd: resumed.append(rd) or 0) + + rc = cli.main(["resolve", "--project", str(tmp_path), "r1", "--adopt-branch", "--resume"]) + + assert rc == 0, capsys.readouterr().err + assert resumed == [run_dir] + task = load_state(run_dir).tasks["s1"] + assert task.phase == Phase.COMMITTING and task.adopt_pending + + def test_resolve_adopt_branch_force_unknown_proceeds(tmp_path, monkeypatch, capsys): """`--force` is the only way past an unverifiable engine pid, so the adopt leg's under-lock liveness re-check must honor it exactly as the re-arm leg does. diff --git a/tests/test_engine.py b/tests/test_engine.py index 13fcfc0ea..8e78e56da 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -17170,6 +17170,21 @@ def test_resume_past_an_escalation_pauses_instead_of_finishing(project): assert paused["stage"] == PAUSE_ESCALATION and paused["story_key"] == "1-1-a" +def test_unresolved_escalation_key_matches_the_shared_seam(project): + """The run-end re-pause (DW-386) and resolve's admission (#958) read one + predicate: `Engine._unresolved_escalation_key` is the first key of + `escalation.unresolved_escalation_keys`, attempt-0 tasks included.""" + from bmad_loop.escalation import unresolved_escalation_keys + + engine, _ = make_engine(project, []) + assert engine._unresolved_escalation_key() is None + for key, phase, attempt in (("a", Phase.DONE, 1), ("b", Phase.ESCALATED, 0)): + engine.state.tasks[key] = StoryTask(story_key=key, epic=1, phase=phase, attempt=attempt) + engine.state.tasks["c"] = StoryTask(story_key="c", epic=1, phase=Phase.ESCALATED, attempt=2) + assert unresolved_escalation_keys(engine.state) == ["b", "c"] + assert engine._unresolved_escalation_key() == "b" + + def test_run_without_an_escalation_still_finishes(project): """DW-386's guard is inert when no task is ESCALATED.""" write_sprint(project, {"1-1-a": "ready-for-dev"}) diff --git a/tests/test_engine_worktree.py b/tests/test_engine_worktree.py index eab2be296..6fa49a7c0 100644 --- a/tests/test_engine_worktree.py +++ b/tests/test_engine_worktree.py @@ -12676,6 +12676,33 @@ def test_abandon_closes_an_escalated_replay_run_and_keeps_its_branch( assert _state_bytes(engine.run_dir) == origin_before +def test_abandon_admits_cleared_escalation_pause(project, tmp_path, monkeypatch, capsys): + """#958: a resume clears the pause before its engine starts, so an engine exit + before run end leaves the replay run unpaused with its unit still ESCALATED. + `--abandon` still closes it: rc 0, worktree removed, branch kept, run finished. + + Ablation, performed: restore the stage-only `paused_stage != PAUSE_ESCALATION` + gate in `abandon_replay_run` and this reddens on rc.""" + _engine, replay_dir = _escalated_replay(project, tmp_path, monkeypatch) + state = load_state(replay_dir) + state.clear_pause() + save_state(replay_dir, state) + task = state.tasks["1-1-a"] + unit, branch = Path(task.worktree_path), task.branch + sha = _branch_sha(project, branch) + capsys.readouterr() + + assert _abandon(project, replay_dir, "--no-interactive") == 0, capsys.readouterr().err + + assert f"kept on branch {branch}" in capsys.readouterr().out + assert not unit.exists() and _registered_under(project, replay_dir) == [] + assert _branch_sha(project, branch) == sha + replay = load_state(replay_dir) + assert replay.finished and replay.paused_stage is None + assert replay.tasks["1-1-a"].phase == Phase.ESCALATED + assert Journal(replay_dir).entries()[-1]["kind"] == "replay-abandoned" + + def test_abandon_parks_uncommitted_work_on_an_abandon_ref(project, tmp_path, monkeypatch, capsys): """An untracked file in the kept worktree is parked on `refs/attempt-preserve-dirty/--abandon` before the removal, named in diff --git a/tests/test_escalation.py b/tests/test_escalation.py index 6a818060c..ea7039231 100644 --- a/tests/test_escalation.py +++ b/tests/test_escalation.py @@ -21,8 +21,9 @@ preference_escalations, review_retry_or_exhaust, session_failure_reason, + unresolved_escalation_keys, ) -from bmad_loop.model import PAUSE_ESCALATION, PAUSE_STORY_GATE, RunState, StoryTask +from bmad_loop.model import PAUSE_ESCALATION, PAUSE_STORY_GATE, Phase, RunState, StoryTask from bmad_loop.policy import LimitsPolicy, NotifyPolicy, Policy, ReviewPolicy from bmad_loop.verify import VerifyOutcome @@ -809,3 +810,47 @@ def test_decide_reverify_never_retries(): VerifyOutcome.escalate("x"), ): assert decide_reverify(_reverify_task(origin), outcome).action != Action.RETRY + + +# ----------------------------------------------------------- unresolved_escalation_keys + + +def _escalation_state(source: str, *tasks: tuple[str, Phase, int]) -> RunState: + state = RunState(run_id="r1", project="/p", started_at="now", source=source) + for key, phase, attempt in tasks: + state.tasks[key] = StoryTask(story_key=key, epic=1, phase=phase, attempt=attempt) + return state + + +def test_unresolved_escalation_keys_sprint_counts_every_escalated_task_in_order(): + """A sprint run's run-end re-pause (DW-386) counts any ESCALATED task, including + one at attempt 0, and resolve's admission reads the keys in task order.""" + state = _escalation_state( + "sprint-status", + ("b", Phase.ESCALATED, 0), + ("a", Phase.DONE, 1), + ("c", Phase.ESCALATED, 2), + ("d", Phase.DEFERRED, 1), + ) + assert unresolved_escalation_keys(state) == ["b", "c"] + + +@pytest.mark.parametrize("source", ["stories", "tickets"]) +def test_unresolved_escalation_keys_checkpoint_flow_skips_attempt0_wedges(source): + """Stories and tickets runs re-classify a pick-time wedge (attempt 0) from disk + every pick, so only an in-run escalation (attempt > 0) is unresolved. + + Ablation: drop the attempt narrowing and the wedge key comes back.""" + state = _escalation_state( + source, + ("w", Phase.ESCALATED, 0), + ("x", Phase.ESCALATED, 1), + ("y", Phase.PENDING, 1), + ("z", Phase.ESCALATED, 3), + ) + assert unresolved_escalation_keys(state) == ["x", "z"] + + +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) == [] diff --git a/tests/test_model.py b/tests/test_model.py index cf3c075ae..c4ec5a7dc 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -1443,6 +1443,26 @@ def test_clear_pause_also_clears_crashed(): assert state.paused is False +def test_restore_pause_undoes_clear_pause(): + """`pause_snapshot` captures exactly the fields `clear_pause` clears, so a resume + that fails before its engine starts can put the pause back. Every field is set + to a non-default value: a field the snapshot missed would come back cleared.""" + state = _state( + paused_reason="CRITICAL escalation", + paused_stage="escalation", + paused_story_key="s1", + stopped=True, + crashed=True, + crash_error="RuntimeError: boom", + ) + before = state.to_dict() + snapshot = state.pause_snapshot() + state.clear_pause() + assert state.to_dict() != before + state.restore_pause(snapshot) + assert state.to_dict() == before + + def test_cache_read_weight_from_snapshot(): state = _state(policy_snapshot={"limits": {"cache_read_weight": 0.5}}) assert state.cache_read_weight() == 0.5 diff --git a/tests/test_portability_guard.py b/tests/test_portability_guard.py index f0ccc4cf1..c83d81881 100644 --- a/tests/test_portability_guard.py +++ b/tests/test_portability_guard.py @@ -259,6 +259,8 @@ # relying on a reviewer to find it by grep. SAVE_STATE_CALLERS = { ("cli.py", "_prepare_resume_locked"), + # a resume that failed before its engine started puts its cleared pause back + ("cli.py", "_restore_aborted_resume"), ("engine.py", "_save"), ("runs.py", "_rearm_escalation_locked"), ("runs.py", "_rearm_for_reverify_locked"), @@ -274,6 +276,9 @@ } RUN_STATE_TRANSACTIONS = { ("cli.py", "_resume_paused_run"), + # the failed-resume restore: reload, re-check finished / a concurrent re-pause, + # then put back the pause `_prepare_resume_locked` cleared + ("cli.py", "_restore_aborted_resume"), ("cli.py", "cmd_resolve"), ("cli.py", "_hand_back_ctl_window"), ("cli.py", "_resolve_adopt"), @@ -1398,6 +1403,10 @@ # #920: the accepting process could not publish its control window; # `error` is a drop field. "ctl-window-claim-failed", + # A resume whose adapter build raised before its engine started put the + # cleared pause back; `error` is the exception class name only (a drop + # field), never its text. + "resume-aborted", "run-resume", # The resume of a tickets run paused for a refined ticket's story-file # approval records that approval (`tickets_source.approve_paused_story_file`). diff --git a/tests/test_resolve.py b/tests/test_resolve.py index 35d2b1563..fc18e5231 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -7,16 +7,17 @@ import pytest import yaml -from conftest import escalated_run, git, json_recursion_payload +from conftest import clear_escalation_pause, escalated_run, git, json_recursion_payload from bmad_loop import devcontract, platform_util, resolve, runs, verify from bmad_loop.engine import _session_task_id -from bmad_loop.journal import JOURNAL_FILE, TASK_CYCLE_ARTIFACTS, load_state, save_state +from bmad_loop.journal import JOURNAL_FILE, TASK_CYCLE_ARTIFACTS, Journal, load_state, save_state from bmad_loop.model import ( PAUSE_ESCALATION, Phase, RunState, SessionRecord, + StoryTask, ) from bmad_loop.platform_util import safe_segment @@ -2489,6 +2490,129 @@ def test_rearm_rejects_unescalated_story(tmp_path): runs.rearm_escalation(run_dir, isolated_redrive=False, resolution_recorded=True) +# #958: a resume clears the pause before its engine starts, so an engine that exits +# before run end leaves the run unpaused with the story still ESCALATED. + + +def _cleared_run(tmp_path, **kw): + run = clear_escalation_pause(escalated_run(tmp_path, **kw)) + assert run.state.paused_stage is None and run.task.phase == Phase.ESCALATED + return run + + +def test_rearm_admits_cleared_pause_escalated_task(tmp_path): + """The #958 shape re-arms, defaulting to the unresolved escalated story.""" + run = _cleared_run(tmp_path) + outcome = runs.rearm_escalation(run.run_dir, isolated_redrive=False, resolution_recorded=False) + assert outcome.story_key == "s1" + assert load_state(run.run_dir).tasks["s1"].phase == Phase.PENDING + + +def test_rearm_cleared_pause_story_must_be_unresolved(tmp_path): + """Unpaused, a named story must be an unresolved escalation: a task that is + not escalated keeps its refusal, and an attempt-0 stories wedge is not admitted. + + Ablation: drop the `escalation_story_admitted` check in + `_rearm_escalation_locked` and the wedge row re-arms.""" + run = _cleared_run(tmp_path, source="stories") + run.state.tasks["s2"] = StoryTask(story_key="s2", epic=1, phase=Phase.DONE, attempt=1) + run.state.tasks["w"] = StoryTask(story_key="w", epic=1, phase=Phase.ESCALATED, attempt=0) + save_state(run.run_dir, run.state) + with pytest.raises(runs.RearmError, match="not escalated"): + runs.rearm_escalation(run.run_dir, "s2", isolated_redrive=False, resolution_recorded=False) + with pytest.raises(runs.RearmError, match=r"not paused at an escalation \(stage: none\)"): + runs.rearm_escalation(run.run_dir, "w", isolated_redrive=False, resolution_recorded=False) + with pytest.raises(runs.RearmError, match="no task for story"): + runs.rearm_escalation( + run.run_dir, "ghost", isolated_redrive=False, resolution_recorded=False + ) + assert load_state(run.run_dir).tasks["w"].phase == Phase.ESCALATED + + +def test_rearm_cleared_pause_refuses_sweep(tmp_path): + """A sweep's ESCALATED tasks are cleared by a plain resume, so an unpaused + sweep keeps stage-only admission. + + Ablation: drop the sweep exclusion in `runs.escalation_resolvable` and this + re-arms.""" + run = _cleared_run(tmp_path, run_type="sweep") + with pytest.raises(runs.RearmError, match=r"not paused at an escalation \(stage: none\)"): + runs.rearm_escalation(run.run_dir, isolated_redrive=False, resolution_recorded=False) + assert load_state(run.run_dir).tasks["s1"].phase == Phase.ESCALATED + + +def test_rearm_cleared_pause_refuses_finished(tmp_path): + """Ablation: drop the finished exclusion in `runs.escalation_resolvable` and + this re-arms.""" + run = _cleared_run(tmp_path) + run.state.finished = True + save_state(run.run_dir, run.state) + with pytest.raises(runs.RearmError, match=r"not paused at an escalation \(stage: none\)"): + runs.rearm_escalation(run.run_dir, isolated_redrive=False, resolution_recorded=False) + assert load_state(run.run_dir).tasks["s1"].phase == Phase.ESCALATED + + +@pytest.mark.parametrize("source", ["stories", "tickets"]) +def test_rearm_cleared_pause_ignores_stories_attempt0_wedge(tmp_path, source): + """A checkpoint-flow run's pick-time wedge (attempt 0) is lifted by a hand fix + and a resume, so an unpaused run holding only that wedge is not admitted. + + Ablation: drop the attempt narrowing in `unresolved_escalation_keys` and this + re-arms.""" + run = _cleared_run(tmp_path, source=source, attempt=0) + with pytest.raises(runs.RearmError, match=r"not paused at an escalation \(stage: none\)"): + runs.rearm_escalation(run.run_dir, isolated_redrive=False, resolution_recorded=False) + assert load_state(run.run_dir).tasks["s1"].phase == Phase.ESCALATED + + +def test_rearm_cleared_pause_keeps_the_other_stage_refusals(tmp_path): + """Admission by task applies only to a null stage: an environment pause (or any + other non-escalation stage) with an escalated task still refuses.""" + run = escalated_run(tmp_path) + for stage in ("environment", "story-gate"): + run.state.paused_stage = stage + save_state(run.run_dir, run.state) + with pytest.raises(runs.RearmError, match=f"stage: {stage}"): + runs.rearm_escalation(run.run_dir, isolated_redrive=False, resolution_recorded=False) + assert load_state(run.run_dir).tasks["s1"].phase == Phase.ESCALATED + + +def test_build_context_reason_reads_the_journal_after_a_cleared_pause(tmp_path): + """Unpaused (#958), the run carries no `paused_reason`, so the context takes the + story's LAST `story-escalated` row; none at all reads as None. + + Ablation: return `state.paused_reason` unconditionally and this reddens.""" + run_dir, state, _ = _escalated_run(tmp_path) + key = "6-4-cli-list-command" + state.clear_pause() + save_state(run_dir, state) + assert _context_reason(state, run_dir, key) is None + journal = Journal(run_dir) + journal.append("story-escalated", story_key=key, reason="first escalation") + journal.append("story-escalated", story_key="other", reason="not this story") + journal.append("story-escalated", story_key=key, reason="latest escalation") + assert _context_reason(state, run_dir, key) == "latest escalation" + + +def test_build_context_reason_for_another_story_than_the_paused_one(tmp_path): + """Resolving `--story B` while paused on A hands B's session B's own reason, never + A's; the paused story keeps the run's `paused_reason`.""" + run_dir, state, _ = _escalated_run(tmp_path) + key = "6-4-cli-list-command" + state.tasks["b"] = StoryTask(story_key="b", epic=6, phase=Phase.ESCALATED, attempt=1) + save_state(run_dir, state) + Journal(run_dir).append("story-escalated", story_key="b", reason="b's escalation") + assert _context_reason(state, run_dir, "b") == "b's escalation" + assert _context_reason(state, run_dir, key) == ( + "CRITICAL escalation from review session: names not unique" + ) + + +def _context_reason(state, run_dir, story_key): + path = _context(state, run_dir, story_key, isolation="") + return json.loads(path.read_text(encoding="utf-8"))["paused_reason"] + + # ------------------------------------------------- _gather_escalations diff --git a/tests/test_runs.py b/tests/test_runs.py index 026cd304f..e631c56ff 100644 --- a/tests/test_runs.py +++ b/tests/test_runs.py @@ -4599,6 +4599,44 @@ def _adoptable(tmp_path): return run_dir, wt +def test_adopt_admits_cleared_escalation_pause(tmp_path): + """#958: a resume clears the pause before its engine starts, so an engine exit + before run end leaves the pause null with the task still ESCALATED. Adoption + admits that run and defaults to the unresolved escalation, as at the pause. + + Ablation, performed: restore the stage-only `paused_stage != PAUSE_ESCALATION` + gate in `adopt_escalated_branch` and this reddens.""" + run_dir, _wt = _adoptable(tmp_path) + state = load_state(run_dir) + state.clear_pause() + save_state(run_dir, state) + + assert runs.adopt_escalated_branch(run_dir) == "bmad-loop/r1/1-1-a" + + task = load_state(run_dir).tasks["1-1-a"] + assert task.phase == Phase.COMMITTING and task.adopt_pending is True + + +def test_adopt_refuses_a_finished_run_with_its_escalation_still_recorded(tmp_path): + """#958: the null-stage admission excludes a finished run, whose ESCALATED task + is a closed record (an abandoned replay run keeps one), not an escalation a + human still owes: refused as not paused, state untouched. + + Ablation, performed: drop `not state.finished` from `escalation_resolvable`'s + null-stage arm and this reddens (the adoption goes through).""" + run_dir, _wt = _adoptable(tmp_path) + state = load_state(run_dir) + state.clear_pause() + state.finished = True + save_state(run_dir, state) + before = (run_dir / "state.json").read_bytes() + + with pytest.raises(runs.RearmError, match=r"not paused at an escalation \(stage: none\)"): + runs.adopt_escalated_branch(run_dir) + + assert (run_dir / "state.json").read_bytes() == before + + def test_adopt_under_lock_refuses_a_task_no_longer_escalated(tmp_path): """DW-386: the locked re-check, not the CLI's lock-free early exit, is what stops a stale adopt gesture from latching a task a rival already moved. diff --git a/tests/test_stories_engine.py b/tests/test_stories_engine.py index 19eded563..883156b4f 100644 --- a/tests/test_stories_engine.py +++ b/tests/test_stories_engine.py @@ -610,6 +610,22 @@ def test_bare_resume_does_not_leapfrog_a_wedged_story(project): assert not any(s.role == "dev" for s in radapter.sessions) # story 2 never dispatched +def test_unresolved_escalation_key_matches_the_shared_seam(project): + """`CheckpointFlowMixin._unresolved_escalation_key` and resolve's admission + (#958) share `escalation.unresolved_escalation_keys`: a stories run skips an + attempt-0 wedge in both.""" + from bmad_loop.escalation import unresolved_escalation_keys + + setup_stories(project, [entry("1")]) + engine, _ = make_engine(project, []) + assert engine.state.source == "stories" + engine.state.tasks["w"] = StoryTask(story_key="w", epic=1, phase=Phase.ESCALATED, attempt=0) + assert engine._unresolved_escalation_key() is None + engine.state.tasks["x"] = StoryTask(story_key="x", epic=1, phase=Phase.ESCALATED, attempt=1) + assert unresolved_escalation_keys(engine.state) == ["x"] + assert engine._unresolved_escalation_key() == "x" + + def test_bare_resume_finishes_after_a_wedge_is_fixed_by_hand(project): """A pick-time wedge's designed exit is a hand fix plus a bare resume. The run-end never-finish-over-ESCALATED guard (DW-386) must not re-pause on the diff --git a/tests/test_tui_app.py b/tests/test_tui_app.py index c6b9efdb4..528cad632 100644 --- a/tests/test_tui_app.py +++ b/tests/test_tui_app.py @@ -4788,6 +4788,124 @@ async def test_resolve_refused_on_environment_pause_with_resume_hint(project_tre assert launched == [] # warned, never launched +def _cleared_escalated_run(root: Path, *, run_type: str = "story", **kw) -> Path: + """A run whose pause a resume cleared before its engine exited early, its story + still ESCALATED (#958): no pause fields, one in-run escalation.""" + return make_run( + root, + "20260611-100000-aaaa", + run_type=run_type, + tasks={"7-7": StoryTask(story_key="7-7", epic=7, phase=Phase.ESCALATED, attempt=1)}, + **kw, + ) + + +async def test_resolve_admits_cleared_pause_escalated_run(project_tree, monkeypatch): + """#958: `R` admits an unpaused run whose story is still escalated, by the CLI's + own rule, and its confirm names that story rather than "?". + + Ablation: restore the stage-only `paused_stage != "escalation"` check and `R` + warns "only available" instead of confirming.""" + monkeypatch.setattr(launch, "mux_available", lambda: True) + monkeypatch.setattr(data, "liveness", lambda run_dir: "dead") + _cleared_escalated_run(project_tree.project) + app = BmadLoopApp(project_tree.project) + async with app.run_test() as pilot: + await until(pilot, lambda: isinstance(app.screen, DashboardScreen)) + await until(pilot, lambda: dashboard(app).selected_run_id is not None) + await pilot.press("R") + await until(pilot, lambda: isinstance(app.screen, ConfirmModal)) + assert "open the resolve agent for 7-7?" in app.screen._body.plain + assert not any("only available" in m for m in notifications(app)) + + +@pytest.mark.parametrize( + ("run_type", "extra", "refusal"), + [ + ("sweep", {}, "only available"), + ( + "story", + {"paused_stage": PAUSE_ENVIRONMENT, "paused_reason": "environment fault"}, + "an environment pause needs no resolve", + ), + ], + ids=["cleared-sweep", "environment-pause"], +) +async def test_resolve_refuses_escalated_task_outside_admission( + project_tree, monkeypatch, run_type, extra, refusal +): + """An unpaused sweep keeps stage-only admission (its ESCALATED tasks are cleared + by a plain resume), and an environment pause still refuses with its own hint + though a story is escalated — a resolve would lift that pause unseen. + + Ablation: drop the sweep clause of `resume_skipped_escalation_keys` and the + sweep row confirms; move the environment refusal below admission and that row + gets the resolve confirm.""" + launched: list[str] = [] + monkeypatch.setattr(launch, "mux_available", lambda: True) + monkeypatch.setattr(data, "liveness", lambda run_dir: "dead") + monkeypatch.setattr(launch, "start_resolve_detached", lambda proj, rid: launched.append(rid)) + _cleared_escalated_run(project_tree.project, run_type=run_type, **extra) + app = BmadLoopApp(project_tree.project) + async with app.run_test() as pilot: + await until(pilot, lambda: isinstance(app.screen, DashboardScreen)) + await until(pilot, lambda: dashboard(app).selected_run_id is not None) + await pilot.press("R") + await until(pilot, lambda: any(refusal in m for m in notifications(app))) + assert isinstance(app.screen, DashboardScreen), "no confirm" + assert launched == [] + + +async def test_do_rearm_admits_cleared_pause(project_tree, monkeypatch): + """#958: the re-arm's locked re-check admits an unpaused run whose story is still + escalated, so the gesture re-arms and resumes it. + + Ablation: restore `fresh_state.paused_stage != PAUSE_ESCALATION` and this + toasts "no longer paused at escalation" with neither recorder filled.""" + from bmad_loop import runs + + install_bmad_config(project_tree) + rearms: list = [] + resumes: list = [] + monkeypatch.setattr(launch, "mux_available", lambda: True) + monkeypatch.setattr(launch, "resume_detached", lambda proj, rid: resumes.append(rid)) + monkeypatch.setattr(data, "liveness", lambda run_dir: "dead") + monkeypatch.setattr( + runs, + "rearm_escalation", + lambda rd, sk, **_k: rearms.append(sk) or _rearm_outcome(sk), + ) + run_dir = _cleared_escalated_run(project_tree.project) + app = BmadLoopApp(project_tree.project) + async with app.run_test() as pilot: + await until(pilot, lambda: isinstance(app.screen, DashboardScreen)) + app._do_rearm("20260611-100000-aaaa", run_dir, "7-7") + await pilot.pause() + await until(pilot, lambda: rearms == ["7-7"] and resumes == ["20260611-100000-aaaa"]) + assert not any("no longer paused" in m for m in notifications(app)) + + +def test_resume_confirm_names_a_skipped_escalated_story(project_tree): + """The resume confirm states what `bmad-loop resume` warns of: a still-escalated + story is skipped, and resolve clears it — but not for a sweep, whose escalated + tasks a plain resume clears. + + Ablation: drop the loop in `ConfirmResumeModal.__init__` and the story row + reads no warning; drop the sweep clause and the sweep row shows one.""" + from bmad_loop.journal import load_state + + run_dir = _cleared_escalated_run(project_tree.project) + body = ConfirmResumeModal(run_dir.name, load_state(run_dir), engine_alive=False)._body.plain + assert ( + "story '7-7' is still escalated — resume skips it; " + "resolve it with bmad-loop resolve 20260611-100000-aaaa" + ) in body + + sweep_dir = _cleared_escalated_run(project_tree.project, run_type="sweep") + body = ConfirmResumeModal(sweep_dir.name, load_state(sweep_dir), engine_alive=False)._body.plain + assert "still escalated" not in body + + # ------------------------------------------------- stories mode: board + badges @@ -7733,6 +7851,43 @@ def _state(tasks): assert "done 1" in content # the park did not absorb the done story +async def test_header_hints_an_escalated_story_after_a_cleared_pause(project_tree): + """#958: `cmd_status`'s hint, in the header — a run that reads as not paused + while a story is still escalated names it and `R`. Not for a run paused at the + escalation (its badge already says so), nor for a sweep, whose escalated tasks + a plain resume clears. + + Ablation: drop the hint loop in `show_run` and the first row lacks it; drop the + `paused_stage` or sweep clause of `cleared_pause_escalation_keys` and the + paused or sweep row shows it.""" + escalated = StoryTask(story_key="7-7", epic=7, phase=Phase.ESCALATED, attempt=1) + + def _state(**kw): + return RunState( + run_id="r1", + project=str(project_tree.project), + started_at="now", + tasks={"7-7": escalated}, + **kw, + ) + + app = BmadLoopApp(project_tree.project) + async with app.run_test(): + header = dashboard(app).query_one("#runheader", RunHeader) + + header.show_run("r1", data.INTERRUPTED, _state()) + assert "escalated: story '7-7' awaiting resolve — press R" in str(header.content) + + paused = _state( + paused_stage="escalation", paused_reason="CRITICAL escalation", paused_story_key="7-7" + ) + header.show_run("r1", data.PAUSED, paused) + assert "awaiting resolve" not in str(header.content) + + header.show_run("r1", data.INTERRUPTED, _state(run_type="sweep")) + assert "awaiting resolve" not in str(header.content) + + async def test_header_shows_a_refused_auto_sweep_apart_from_one_that_ran(project_tree): """DW-366: a refused auto-sweep gets its own warning line (reason slug plus the `bmad-loop sweep` hint), and a delivered one gets only a dim `ran` line, so diff --git a/tests/test_tui_launch.py b/tests/test_tui_launch.py index 76bb121f4..0fb2a63bd 100644 --- a/tests/test_tui_launch.py +++ b/tests/test_tui_launch.py @@ -24,6 +24,7 @@ from bmad_loop import envvars, runs from bmad_loop.adapters import tmux_base from bmad_loop.adapters.multiplexer import PARKED_BANNER, MultiplexerError, get_multiplexer +from bmad_loop.model import PauseSnapshot from bmad_loop.tui import launch # Every test here asserts tmux-specific argv/behaviour through the multiplexer @@ -1339,7 +1340,7 @@ def mint(argv, **kwargs): def prepare(_project, _run_dir, *, accept_baseline=False): engine["live"] = True # the pid this child publishes under the lock - return (None,) * 8 + return (*(None,) * 8, PauseSnapshot(None, None, None, False, False, None)) def compose(**_kwargs): raise Drove