From 16c3878f5db508862aeb8d37185990e8de580515 Mon Sep 17 00:00:00 2001 From: pbean Date: Wed, 15 Jul 2026 08:16:33 -0700 Subject: [PATCH] fix(adapters): bound dev/review stall nudges + diagnose result-less Stops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The idle wake nudge is delivered as a submitted turn, so a session that merely answers it ends in another result-less Stop — which refilled the per-silence nudge budget, re-armed the grace window, and repeated until session_timeout_min, burning a turn per cycle. Dev/review sessions now get the same monotonic cap injected workflow sessions already had: after [limits] dev_stall_nudges_cap (default 6) total nudges the session is declared stalled instead; post-kill reconcile still rescues a finished one whose terminal artifact is on disk. STALL_NUDGE_TEXT now also states that a prose reply cannot end the session, and every result-less Stop leaves a diagnostic breadcrumb (tasks//resultless-stops.jsonl: pending / not-terminal / stale-mtime / ambiguous / no-artifact / no-result-json) so the trigger — why a completed-looking session read as result-less — is answerable from the run dir. Closes #149 --- CHANGELOG.md | 12 +++ docs/FEATURES.md | 2 +- docs/game-engine-plugin-guide.md | 4 + docs/tui-guide.md | 1 + src/bmad_loop/adapters/base.py | 15 ++-- src/bmad_loop/adapters/generic.py | 101 ++++++++++++++++++------- src/bmad_loop/data/settings/core.toml | 6 ++ src/bmad_loop/engine.py | 4 +- src/bmad_loop/policy.py | 30 ++++++-- tests/test_generic_tmux.py | 105 +++++++++++++++++++++++++- tests/test_plugin_workflows.py | 12 +-- tests/test_policy.py | 13 ++++ 12 files changed, 255 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ef56fe..8526bac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -145,6 +145,18 @@ PATH)`, the TUI notifies `multiplexer backend unavailable — launch/attach disa ### Fixed +- **Dev/review sessions can no longer livelock on their own wake nudges (#149).** The idle + wake nudge is delivered as a submitted turn, so a session that merely _answers_ it ends in + another result-less Stop — which refilled the nudge budget, re-armed the grace window, and + repeated until `session_timeout_min`, burning a turn per cycle. Dev/review sessions now get the + same monotonic cap injected workflow sessions already had: after `limits.dev_stall_nudges_cap` + (default 6) total nudges the session is declared stalled instead (post-kill reconcile still + rescues a finished one whose terminal artifact is on disk). The nudge text now also states that + a prose reply cannot end the session. And each result-less Stop leaves a diagnostic breadcrumb + (`tasks//resultless-stops.jsonl`: pending / not-terminal / stale-mtime / ambiguous / + no-artifact / no-result-json) so _why_ a completed-looking session read as result-less is + answerable from the run dir. + - **Split-story keys (`2-6a-…`) are no longer silently skipped (#144).** The sprint-status parser rejected story numbers carrying BMAD's split-story letter suffix, dropping exactly the stories that were split to be loop-tractable — invisible to `run`/`--story`/the TUI tree, and diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 36972fa..cad7019 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -142,7 +142,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - Single policy file written by `init`, snapshotted at run start (applies to new runs and resumes; editable live from the TUI). - Sections: `[gates]`, `[limits]`, `[verify]`, `[notify]`, `[review]`, `[adapter]` (+ per-stage), `[sweep]`, `[scm]` (worktree isolation + merge-back), `[cleanup]` (run-dir retention + disk reclamation), `[plugins]` (trust allowlist + per-plugin `[plugins.]` config — e.g. the opt-in game-engine layer via `[plugins.unity]`, off by default), `[tui]` (`low_frame_rate` for slow/SSH links; persisted dashboard pane sizes). -- Tunable limits: `max_review_cycles`, `max_dev_attempts`, `max_followup_reviews`, `session_timeout_min`, `stop_without_result_nudges`, `dev_stall_grace_s`, `dev_stall_nudges`, `max_tokens_per_story`. +- Tunable limits: `max_review_cycles`, `max_dev_attempts`, `max_followup_reviews`, `session_timeout_min`, `stop_without_result_nudges`, `dev_stall_grace_s`, `dev_stall_nudges`, `dev_stall_nudges_cap`, `workflow_stall_nudges_cap`, `max_tokens_per_story`. ### TUI dashboard diff --git a/docs/game-engine-plugin-guide.md b/docs/game-engine-plugin-guide.md index 348c276..958bc84 100644 --- a/docs/game-engine-plugin-guide.md +++ b/docs/game-engine-plugin-guide.md @@ -242,6 +242,10 @@ The canonical example lives at `src/bmad_loop/data/plugins/unity/`: > (wake-nudges spent on grace expiry before it is called stalled). The grace window > measures genuine inactivity — pane output re-arms it — so raise these (not the > readiness knobs) if networked/PlayMode-heavy stories are being mis-stalled. +> `limits.dev_stall_nudges_cap` (default 6) additionally bounds the _total_ nudges +> a session may ever receive — a very long wait needing more wake cycles than that +> should raise the cap too (a stalled-but-finished session is still rescued +> post-kill when its terminal artifact is on disk). Each script's module docstring documents every env knob it reads — the authoritative source if a default ever changes. The [Game Engine MCP guide](game-engine-mcp-guide.md) diff --git a/docs/tui-guide.md b/docs/tui-guide.md index 1a0bbe7..61d1746 100644 --- a/docs/tui-guide.md +++ b/docs/tui-guide.md @@ -495,6 +495,7 @@ behavior. | `limits.stop_without_result_nudges` | int ≥ 0 | 1 | nudges when a session stops without result.json | | `limits.dev_stall_grace_s` | int ≥ 0 | 600 | idle grace for a dev session awaiting a background run (e.g. PlayMode); pane output or a re-invocation re-arms it · 0 = stall on first result-less Stop | | `limits.dev_stall_nudges` | int ≥ 0 | 2 | wake-nudges on grace expiry before stalling; a Stop response restores the budget · 0 = stall on grace expiry | +| `limits.dev_stall_nudges_cap` | int ≥ 0 | 6 | total (never-restored) wake-nudges per dev/review session — bounds the refill loop when the reply to a nudge is itself a result-less Stop | | `limits.max_tokens_per_story` | int ≥ 1 | 2000000 | cost-weighted budget | | `limits.cache_read_weight` | float 0.0–1.0 | 0.1 | cache-read weight in the budget; 1.0 = raw | | `verify.commands` | one per line | (none) | test/lint commands run before commit | diff --git a/src/bmad_loop/adapters/base.py b/src/bmad_loop/adapters/base.py index 4efbc5a..bd7fe9a 100644 --- a/src/bmad_loop/adapters/base.py +++ b/src/bmad_loop/adapters/base.py @@ -32,12 +32,15 @@ class SessionSpec: model: str = "" # empty = CLI default # fallback only; real dev/review/retro sessions get limits.session_timeout_min * 60 timeout_s: float = 90 * 60 - # total stall wake-nudges this session may ever receive; None = unbounded - # (today's behavior). Unlike the adapter's refillable per-silence budget, - # this cap is monotonic — a session that keeps ending its turn without a - # result cannot re-earn nudges forever. Set for injected workflow sessions - # so a forgotten completion marker degrades to "stalled" instead of - # livelocking until timeout_s. + # total stall wake-nudges this session may ever receive; None (the raw + # constructor default) = unbounded. Unlike the adapter's refillable + # per-silence budget, this cap is monotonic — a session that keeps ending + # its turn without a result cannot re-earn nudges forever, because the + # nudge is itself a submitted turn whose reply refills the budget (#149). + # The engine sets it for every session it drives: workflow_stall_nudges_cap + # for injected workflow sessions, dev_stall_nudges_cap otherwise, so a + # missing completion artifact degrades to "stalled" instead of livelocking + # until timeout_s. stall_nudges_cap: int | None = None diff --git a/src/bmad_loop/adapters/generic.py b/src/bmad_loop/adapters/generic.py index 555ecc7..ce878ed 100644 --- a/src/bmad_loop/adapters/generic.py +++ b/src/bmad_loop/adapters/generic.py @@ -54,7 +54,10 @@ "You appear idle in bmad-loop automation mode, which cannot re-invoke you when " "a background process finishes. If you are waiting on one (e.g. a Unity PlayMode " "run or a long test), check its status now and continue the workflow; if it is " - "done, finalize the work and end your turn. If you are stuck, say so and stop." + "done, finalize the work and end your turn. If you are stuck, say so and stop. " + "Note: a prose reply cannot end this session — only your workflow's completion " + "artifact (the spec's terminal status / result file) does; if the work is " + "already complete, write it before ending your turn." ) @@ -190,10 +193,11 @@ def wait_for_completion(self, handle: SessionHandle, spec: SessionSpec) -> Sessi # unresponsive session burns through it. Bounded overall by spec.timeout_s. stall_nudges_left = self._stall_nudges # monotonic total of stall nudges sent this session — never restored, - # unlike stall_nudges_left. When spec.stall_nudges_cap is set (injected - # workflow sessions), a session that keeps ending its turn without a - # result cannot ride the fresh-Stop refill forever: after cap total - # nudges it is declared stalled. cap=None (dev/review) skips the check. + # unlike stall_nudges_left. When spec.stall_nudges_cap is set (the + # engine sets it for every session it drives), a session that keeps + # ending its turn without a result cannot ride the fresh-Stop refill + # forever: after cap total nudges it is declared stalled. cap=None + # (raw constructor default) skips the check. stall_nudges_sent = 0 # internal observability counter: counts ticks where the liveness probe # raised a transport error (e.g. a 30s tmux hang). It deliberately does @@ -378,6 +382,25 @@ def _final( def _result_path(self, task_id: str) -> Path: return self.tasks_dir / task_id / "result.json" + def _note_resultless_stop(self, task_id: str, verdict: str, detail: str = "") -> None: + """Append a diagnostic breadcrumb when a Stop's artifact read-back gives + up empty: one JSON line ({ts, verdict, detail}) in + ``tasks//resultless-stops.jsonl``. Pure observability — the + #149 nudge livelock was undiagnosable because nothing recorded *why* + each Stop read as result-less. Best-effort: an unwritable run dir must + never break the completion loop.""" + try: + path = self.tasks_dir / task_id / "resultless-stops.jsonl" + path.parent.mkdir(parents=True, exist_ok=True) + line = json.dumps( + {"ts": time.time_ns(), "verdict": verdict, "detail": detail}, + ensure_ascii=False, + ) + with path.open("a", encoding="utf-8") as fh: + fh.write(line + "\n") + except OSError: + pass + def _read_result(self, task_id: str) -> dict | None: path = self._result_path(task_id) if not path.is_file(): @@ -392,8 +415,13 @@ def _await_result(self, task_id: str, grace_s: float = RESULT_GRACE_S) -> dict | deadline = time.monotonic() + grace_s while True: result = self._read_result(task_id) - if result is not None or time.monotonic() >= deadline: + if result is not None: return result + if time.monotonic() >= deadline: + self._note_resultless_stop( + task_id, "no-result-json", f"no readable {self._result_path(task_id)}" + ) + return None time.sleep(RESULT_POLL_S) def _window_alive(self, handle: SessionHandle) -> bool: @@ -490,6 +518,13 @@ def _synth_result( spec_path, story_key=story_key, dw_ids=dw_ids or None ) if not wait or time.monotonic() >= deadline: + if wait: + self._note_resultless_stop( + handle.task_id, + "no-artifact", + "no result artifact newer than session launch under: " + + ", ".join(str(d) for d in search_dirs), + ) return None time.sleep(RESULT_POLL_S) @@ -529,28 +564,42 @@ def _stories_synth_result( # >1 matching file — waiting can't make it collapse to one. Return now # (don't burn the grace); the engine's next _pick_next re-classifies # AMBIGUOUS and raises the actionable wedge for resolve. - return None - if ( - state.kind in (stories.KIND_PRESENT, stories.KIND_SENTINEL) - and state.path - and self._written_this_session(state.path, handle.launched_ns) - ): - try: - sr = devcontract.synthesize_result( - state.path, story_key=story_key or None, plan_halt=plan_halt + if wait: + self._note_resultless_stop( + handle.task_id, + "ambiguous", + f"{len(state.paths)} specs match id {story_key!r} under {base}", ) - except UnicodeDecodeError: - # A non-UTF-8 read is either a torn glimpse of a spec still - # being written (keep polling — a later pass sees the finished - # write) or a genuinely corrupt file: then the grace expires - # result-less and the next _pick_next re-classifies it as a - # wedge (resolve_story_spec degrades an undecodable PRESENT - # spec to status "" → pause for resolve), never a crash of - # the read-back poll. - sr = None - if sr is not None and sr.result_json is not None: - return sr + return None + # Classify this pass for the result-less breadcrumb; overwritten + # below when the spec is present but not (yet) this session's + # terminal output. + verdict, detail = state.kind, str(state.path or base) + if state.kind in (stories.KIND_PRESENT, stories.KIND_SENTINEL) and state.path: + if not self._written_this_session(state.path, handle.launched_ns): + verdict = "stale-mtime" + detail = f"{state.path} predates session launch" + else: + try: + sr = devcontract.synthesize_result( + state.path, story_key=story_key or None, plan_halt=plan_halt + ) + except UnicodeDecodeError: + # A non-UTF-8 read is either a torn glimpse of a spec still + # being written (keep polling — a later pass sees the finished + # write) or a genuinely corrupt file: then the grace expires + # result-less and the next _pick_next re-classifies it as a + # wedge (resolve_story_spec degrades an undecodable PRESENT + # spec to status "" → pause for resolve), never a crash of + # the read-back poll. + sr = None + if sr is not None and sr.result_json is not None: + return sr + verdict = "not-terminal" + detail = f"{state.path} has no terminal status (frontmatter {state.status!r})" if not wait or time.monotonic() >= deadline: + if wait: + self._note_resultless_stop(handle.task_id, verdict, detail) return None time.sleep(RESULT_POLL_S) diff --git a/src/bmad_loop/data/settings/core.toml b/src/bmad_loop/data/settings/core.toml index b9c68f7..5a0b08c 100644 --- a/src/bmad_loop/data/settings/core.toml +++ b/src/bmad_loop/data/settings/core.toml @@ -105,6 +105,12 @@ minimum = 0 default_ref = "LimitsPolicy.dev_stall_nudges" description = "times an idle dev session is nudged awake on grace expiry before it is called stalled (bmad-loop has no background-completion re-invocation); pane output re-arms the grace window and a fresh Stop restores the budget · 0 = stall on grace expiry" [[section.field]] +key = "dev_stall_nudges_cap" +kind = "int" +minimum = 0 +default_ref = "LimitsPolicy.dev_stall_nudges_cap" +description = "total (never-restored) stall nudges for a dev/review session before it is called stalled; bounds a session whose reply to the wake nudge is itself a result-less Stop that would refill the budget forever · 0 = stall on first grace expiry" +[[section.field]] key = "workflow_stall_nudges_cap" kind = "int" minimum = 0 diff --git a/src/bmad_loop/engine.py b/src/bmad_loop/engine.py index 9397f0b..134e2ff 100644 --- a/src/bmad_loop/engine.py +++ b/src/bmad_loop/engine.py @@ -2317,7 +2317,9 @@ def _run_session( model=cfg.model, timeout_s=self.policy.limits.session_timeout_min * 60, stall_nudges_cap=( - self.policy.limits.workflow_stall_nudges_cap if label is not None else None + self.policy.limits.workflow_stall_nudges_cap + if label is not None + else self.policy.limits.dev_stall_nudges_cap ), ) self.journal.set_active_log(task_id) diff --git a/src/bmad_loop/policy.py b/src/bmad_loop/policy.py index 6b3418a..e3458aa 100644 --- a/src/bmad_loop/policy.py +++ b/src/bmad_loop/policy.py @@ -90,13 +90,21 @@ class LimitsPolicy: # the full grace, nudge after nudge, drains the budget and stalls. 0 = stall # on grace expiry. dev_stall_nudges: int = 2 - # monotonic cap on stall wake-nudges for injected plugin-workflow sessions - # (SessionSpec.stall_nudges_cap). Dev/review sessions stay uncapped: their - # nudge budget is deliberately restored on every fresh Stop so a session - # awaiting a slow background process can wait up to session_timeout_min. A - # workflow session that keeps ending its turn without writing its completion - # marker would ride that refill forever; after this many total nudges it is - # declared stalled instead (non-blocking workflows then advance the phase). + # monotonic (never-restored) cap on total stall wake-nudges for a dev/review + # session (SessionSpec.stall_nudges_cap). The per-silence dev_stall_nudges + # budget is restored on every fresh Stop so a cooperative session awaiting a + # slow background process can keep waiting — but the wake nudge is itself a + # submitted turn, so a session that merely *answers* it ends in another + # result-less Stop and re-earns the budget: without this cap the loop rides + # the refill until session_timeout_min, burning a turn per cycle (#149). + # After this many total nudges the session is declared stalled instead + # (post-kill reconcile still rescues a finished one whose artifact is on + # disk). 0 = stall on first grace expiry. + dev_stall_nudges_cap: int = 6 + # same monotonic cap for injected plugin-workflow sessions: one that keeps + # ending its turn without writing its completion marker is declared stalled + # after this many total nudges (non-blocking workflows then advance the + # phase). workflow_stall_nudges_cap: int = 3 max_tokens_per_story: int = 2_000_000 # weight of cache-read tokens in the budget check (1.0 = count raw) @@ -556,6 +564,9 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy: ), dev_stall_grace_s=int(limits_d.get("dev_stall_grace_s", LimitsPolicy.dev_stall_grace_s)), dev_stall_nudges=int(limits_d.get("dev_stall_nudges", LimitsPolicy.dev_stall_nudges)), + dev_stall_nudges_cap=int( + limits_d.get("dev_stall_nudges_cap", LimitsPolicy.dev_stall_nudges_cap) + ), workflow_stall_nudges_cap=int( limits_d.get("workflow_stall_nudges_cap", LimitsPolicy.workflow_stall_nudges_cap) ), @@ -578,6 +589,10 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy: raise PolicyError(f"limits.dev_stall_grace_s must be >= 0: got {limits.dev_stall_grace_s}") if limits.dev_stall_nudges < 0: raise PolicyError(f"limits.dev_stall_nudges must be >= 0: got {limits.dev_stall_nudges}") + if limits.dev_stall_nudges_cap < 0: + raise PolicyError( + f"limits.dev_stall_nudges_cap must be >= 0: got {limits.dev_stall_nudges_cap}" + ) if limits.workflow_stall_nudges_cap < 0: raise PolicyError( f"limits.workflow_stall_nudges_cap must be >= 0: got {limits.workflow_stall_nudges_cap}" @@ -815,6 +830,7 @@ def _fold_deprecated_engine( stop_without_result_nudges = 1 dev_stall_grace_s = 600 # grace for a dev session that ended its turn awaiting a background process (e.g. a slow PlayMode/test run) before it is called stalled; each re-invocation resets it. 0 = fail fast on the first result-less Stop dev_stall_nudges = 2 # times an idle dev session is nudged awake on grace expiry before it is called stalled (bmad-loop has no background-completion re-invocation); pane output re-arms the grace window and a fresh Stop restores the budget. 0 = stall on grace expiry +dev_stall_nudges_cap = 6 # total (never-restored) stall nudges for a dev/review session before it is called stalled; bounds a session whose reply to the wake nudge is itself a result-less Stop that would refill the budget forever (#149). 0 = stall on first grace expiry workflow_stall_nudges_cap = 3 # total (never-restored) stall nudges for an injected plugin-workflow session before it is called stalled; bounds a session that finished its work but never wrote its completion marker. 0 = stall on first grace expiry max_tokens_per_story = 2000000 cache_read_weight = 0.1 # cache reads bill at ~0.1x input on all vendors; 1.0 = count raw diff --git a/tests/test_generic_tmux.py b/tests/test_generic_tmux.py index e5292cc..981d2ab 100644 --- a/tests/test_generic_tmux.py +++ b/tests/test_generic_tmux.py @@ -7,6 +7,7 @@ propagation / hook-signal waiting / kill end-to-end for any profile. """ +import json import shutil import subprocess import sys @@ -575,6 +576,102 @@ def find(artifacts, *, since_ns): assert calls["n"] == 1 +# ------------------------------- result-less-Stop diagnostics (#149) +# +# When a Stop's artifact read-back gives up empty, the adapter appends a +# {ts, verdict, detail} line to tasks//resultless-stops.jsonl so the +# WHY of a nudge/stall (issue #149's undiagnosable trigger) is readable +# straight from the run dir. + + +def _breadcrumbs(adapter, task_id="3-1-dev-1"): + path = adapter.tasks_dir / task_id / "resultless-stops.jsonl" + if not path.is_file(): + return [] + return [json.loads(line) for line in path.read_text(encoding="utf-8").splitlines()] + + +def test_resultless_stop_breadcrumb_scan_no_artifact(tmp_path, monkeypatch): + adapter, impl = make_dev_adapter(tmp_path) + monkeypatch.setattr(generic, "RESULT_GRACE_S", 0.0) + assert adapter._result_json(_dev_handle(), _dev_spec(tmp_path), wait=True) is None + (crumb,) = _breadcrumbs(adapter) + assert crumb["verdict"] == "no-artifact" + assert str(impl) in crumb["detail"] # names the searched dirs + + +def test_resultless_stop_breadcrumb_stories_pending(tmp_path, monkeypatch): + adapter, _ = make_dev_adapter(tmp_path) + monkeypatch.setattr(generic, "RESULT_GRACE_S", 0.0) + assert adapter._result_json(_dev_handle(), _stories_spec(tmp_path), wait=True) is None + (crumb,) = _breadcrumbs(adapter) + assert crumb["verdict"] == "pending" + + +def test_resultless_stop_breadcrumb_stories_ambiguous(tmp_path): + adapter, _ = make_dev_adapter(tmp_path) + _write_story_spec(tmp_path, "1", "foo", "---\nstatus: done\n---\n\ndone\n") + _write_story_spec(tmp_path, "1", "bar", "---\nstatus: done\n---\n\ndone\n") + assert adapter._result_json(_dev_handle(), _stories_spec(tmp_path), wait=True) is None + (crumb,) = _breadcrumbs(adapter) + assert crumb["verdict"] == "ambiguous" + assert "2 specs" in crumb["detail"] + + +def test_resultless_stop_breadcrumb_stories_stale_mtime(tmp_path, monkeypatch): + adapter, _ = make_dev_adapter(tmp_path) + monkeypatch.setattr(generic, "RESULT_GRACE_S", 0.0) + spec = _write_story_spec( + tmp_path, "1", "foo", "---\nstatus: done\n---\n\n## Auto Run Result\n\nStatus: done\n" + ) + handle = _dev_handle(launched_ns=spec.stat().st_mtime_ns + 1) + assert adapter._result_json(handle, _stories_spec(tmp_path), wait=True) is None + (crumb,) = _breadcrumbs(adapter) + assert crumb["verdict"] == "stale-mtime" + assert "predates session launch" in crumb["detail"] + + +def test_resultless_stop_breadcrumb_stories_not_terminal(tmp_path, monkeypatch): + adapter, _ = make_dev_adapter(tmp_path) + monkeypatch.setattr(generic, "RESULT_GRACE_S", 0.0) + _write_story_spec(tmp_path, "1", "foo", "---\nstatus: ready-for-dev\n---\n\nplanned only\n") + assert adapter._result_json(_dev_handle(), _stories_spec(tmp_path), wait=True) is None + (crumb,) = _breadcrumbs(adapter) + assert crumb["verdict"] == "not-terminal" + assert "'ready-for-dev'" in crumb["detail"] + + +def test_resultless_stop_breadcrumb_base_no_result_json(tmp_path): + adapter = GenericTmuxAdapter( + run_dir=tmp_path / "run", + policy=Policy(limits=LimitsPolicy()), + profile=get_profile("claude"), + ) + assert adapter._await_result("3-1-dev-1", grace_s=0.0) is None + (crumb,) = _breadcrumbs(adapter) + assert crumb["verdict"] == "no-result-json" + assert "result.json" in crumb["detail"] + + +def test_resultless_stop_breadcrumb_only_on_stop_readback(tmp_path): + """wait=False reads (the _final stall/crash re-checks) must not write + breadcrumbs — only the Stop-event read-back diagnoses a result-less Stop.""" + adapter, _ = make_dev_adapter(tmp_path) + assert adapter._result_json(_dev_handle(), _dev_spec(tmp_path), wait=False) is None + assert adapter._result_json(_dev_handle(), _stories_spec(tmp_path), wait=False) is None + assert _breadcrumbs(adapter) == [] + + +def test_resultless_stop_breadcrumb_write_failure_is_swallowed(tmp_path): + """The breadcrumb is best-effort observability: an unwritable tasks dir + must never break the completion loop.""" + adapter, _ = make_dev_adapter(tmp_path) + blocker = tmp_path / "blocker" + blocker.write_text("a file where the tasks dir should be") + adapter.tasks_dir = blocker / "tasks" # any write under it raises OSError + adapter._note_resultless_stop("3-1-dev-1", "pending", "detail") # must not raise + + def test_generic_dev_disables_nudges(tmp_path): adapter, _ = make_dev_adapter(tmp_path) assert adapter._stop_nudges == 0 @@ -1056,10 +1153,10 @@ def advance(call_n): def test_uncapped_spec_keeps_refilling_nudges_past_cap(tmp_path, monkeypatch): - """cap=None (dev/review sessions) preserves the pre-cap behavior byte- - identical: every fresh Stop restores the budget and nudging continues well - past any workflow cap — a legitimately slow background wait (e.g. a Unity - PlayMode run) may need every one of them, bounded only by spec.timeout_s.""" + """cap=None (the raw SessionSpec default — the engine now caps every + session it drives, dev/review included) preserves the uncapped adapter + contract byte-identical: every fresh Stop restores the budget and nudging + continues past any cap, bounded only by spec.timeout_s.""" adapter, _, clock, sent = _stall_loop_adapter(tmp_path, monkeypatch) def advance(call_n): diff --git a/tests/test_plugin_workflows.py b/tests/test_plugin_workflows.py index 080dc50..e3d78fe 100644 --- a/tests/test_plugin_workflows.py +++ b/tests/test_plugin_workflows.py @@ -224,10 +224,12 @@ def test_workflow_injects_a_session_at_post_dev_phase(project): def test_dev_and_review_sessions_carry_no_workflow_contract(project): - """The completion contract + stall-nudge cap are workflow-session-only: the - main dev/review sessions (their skills own their result conventions, and a - slow background wait may legitimately need unbounded re-nudging) must stay - byte-identical — no appended contract, no cap.""" + """The completion contract is workflow-session-only: the main dev/review + sessions (their skills own their result conventions) must stay + byte-identical — no appended contract. Their stall nudges are bounded by + the dev cap, not the workflow one: a session whose reply to the wake nudge + is itself a result-less Stop would otherwise refill the budget until + session timeout (#149).""" captured: list = [] setup_story(project) @@ -247,7 +249,7 @@ def inner(spec): assert summary.done == 1 assert len(captured) == 2 for spec in captured: - assert spec.stall_nudges_cap is None + assert spec.stall_nudges_cap == engine.policy.limits.dev_stall_nudges_cap assert "Completion signal" not in spec.prompt assert "bmad-dev-auto-result-" not in spec.prompt diff --git a/tests/test_policy.py b/tests/test_policy.py index 0978279..65c792d 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -280,6 +280,19 @@ def test_workflow_stall_nudges_cap_default_parse_and_template(): policy.loads("[limits]\nworkflow_stall_nudges_cap = -1\n") +def test_dev_stall_nudges_cap_default_parse_and_template(): + import tomllib + + assert policy.loads("").limits.dev_stall_nudges_cap == 6 + loaded = policy.loads("[limits]\ndev_stall_nudges_cap = 0\n") + assert loaded.limits.dev_stall_nudges_cap == 0 + # the emitted template documents the knob at its dataclass default + doc = tomllib.loads(policy.POLICY_TEMPLATE) + assert doc["limits"]["dev_stall_nudges_cap"] == policy.LimitsPolicy.dev_stall_nudges_cap + with pytest.raises(policy.PolicyError, match="dev_stall_nudges_cap"): + policy.loads("[limits]\ndev_stall_nudges_cap = -1\n") + + def test_max_followup_reviews_default_parse_and_template(): import tomllib