Skip to content

fix(adapters): bound dev/review stall nudges + diagnose result-less Stops#151

Merged
pbean merged 1 commit into
mainfrom
fix/dev-stall-nudge-cap
Jul 15, 2026
Merged

fix(adapters): bound dev/review stall nudges + diagnose result-less Stops#151
pbean merged 1 commit into
mainfrom
fix/dev-stall-nudge-cap

Conversation

@pbean

@pbean pbean commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Closes #149.

The bug

The idle wake nudge (STALL_NUDGE_TEXT) is delivered as a submitted turn (send-keys + Enter), so a session that merely answers it ends in another result-less Stop — which refilled the per-silence nudge budget (generic.py, fresh-Stop refill), re-armed the grace window, and repeated until session_timeout_min (90 min default), burning a turn of tokens per cycle. The monotonic cap that fixes exactly this livelock already existed (workflow_stall_nudges_cap, 0.8.0) but was wired only for injected workflow sessions; dev/review sessions were deliberately left uncapped and thus exposed.

Note for reproducers: the issue's "~5 seconds" is the event-poll tick, not the idle threshold — the nudge fires after dev_stall_grace_s (default 600 s) of pane-log silence following a result-less Stop. Slow Stop hooks alone never trigger it at defaults; the loop needs a Stop whose artifact read-back comes up empty.

The fix

  • New [limits] dev_stall_nudges_cap (default 6): dev/review sessions now get the same monotonic never-restored cap; after that many total nudges the session is declared stalled instead of riding the refill. Post-kill reconcile still rescues a finished session whose terminal artifact is on disk; otherwise the normal stalled retry/defer ladder applies. Under default timing the cap leaves ~an hour of idle grace for legit slow background waits (Unity PlayMode runs, long tests).
  • Nudge text hardening: STALL_NUDGE_TEXT now states that a prose reply cannot end the session — only the workflow's completion artifact does — so a wake turn is steered toward finalizing rather than explaining.
  • Result-less-Stop diagnostics: every Stop whose artifact read-back gives up empty appends {ts, verdict, detail} to tasks/<task_id>/resultless-stops.jsonl (pending / not-terminal / stale-mtime / ambiguous / no-artifact / no-result-json), so the trigger half of Claude adapter: Stop hook + idle detection re-entrant loop prevents session from ending #149 — why a completed-looking session read as result-less — is answerable straight from the run dir next time.

Verification

  • Full suite green (2271 passed, 1 skipped), trunk clean.
  • New/updated tests: policy parse/template/validation for the new knob; engine wiring (dev/review specs now carry the dev cap); breadcrumb verdicts across stories mode (pending/ambiguous/stale-mtime/not-terminal), scan mode (no-artifact), base adapter (no-result-json); wait=False reads write nothing; breadcrumb write failure is swallowed.
  • Deterministic sandbox E2E (fake-claude profile, real tmux, dev_stall_grace_s=2, cap=2): a session that answers every nudge with a fresh result-less Stop received exactly 2 nudges, then session-end stalledstory-deferredrun-complete, with one no-artifact breadcrumb per Stop. Pre-fix this scenario nudges until the 90-minute timeout.

Summary by CodeRabbit

  • Bug Fixes

    • Prevented dev/review sessions from repeatedly cycling on idle wake nudges.
    • Sessions are now marked stalled after reaching the configured nudge limit.
    • Improved stall messages to clarify how sessions are completed.
    • Added diagnostic records to help explain result-less stops.
  • New Settings

    • Added dev_stall_nudges_cap, defaulting to 6, to control the maximum wake nudges per session.
  • Documentation

    • Updated configuration, settings, and plugin guides with the new limit and troubleshooting guidance.

…tops

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/<task_id>/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
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Dev/review sessions now use a configurable monotonic stall-nudge cap. Result-less Stop paths write JSONL diagnostic breadcrumbs, with policy, settings, engine wiring, tests, and documentation updated accordingly.

Changes

Stall nudge control and diagnostics

Layer / File(s) Summary
Configure and assign stall-nudge caps
src/bmad_loop/policy.py, src/bmad_loop/engine.py, src/bmad_loop/data/settings/core.toml, tests/test_policy.py, tests/test_plugin_workflows.py
Adds dev_stall_nudges_cap with default and validation, exposes it in settings, assigns caps by session type, and tests parsing and propagation.
Record result-less Stop outcomes
src/bmad_loop/adapters/generic.py, tests/test_generic_tmux.py
Writes best-effort JSONL breadcrumbs for missing or ambiguous completion artifacts and verifies diagnostic verdicts, write isolation, and failure handling.
Document bounded session behavior
src/bmad_loop/adapters/base.py, docs/*, CHANGELOG.md
Documents monotonic nudge limits, completion-artifact requirements, stalled sessions, and the new configuration key.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DevSession
  participant GenericAdapter
  participant ResultArtifact
  participant BreadcrumbFile
  DevSession->>GenericAdapter: receive idle wake-nudge
  GenericAdapter->>ResultArtifact: poll completion result
  ResultArtifact-->>GenericAdapter: result-less Stop
  GenericAdapter->>BreadcrumbFile: append diagnostic breadcrumb
  GenericAdapter-->>DevSession: continue until cap or completion
Loading

Suggested reviewers: dracic

Poem

I nudged the loop, then counted six,
No endless hops or tangled tricks.
When results hide beneath the floor,
I leave a breadcrumb by the door.
— A relieved rabbit 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 29.63% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: bounding dev/review stall nudges and adding diagnostics for result-less Stops.
Linked Issues check ✅ Passed The PR addresses #149 by breaking the idle/Stop loop with a finite nudge cap and preserving session termination.
Out of Scope Changes check ✅ Passed The docs, tests, policy, engine, and adapter edits all support the stall-nudge fix and diagnostics, with no clear unrelated changes.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/dev-stall-nudge-cap

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/bmad_loop/data/settings/core.toml`:
- Line 113: Remove the duplicate [[section.field]] declaration near
workflow_stall_nudges_cap in the settings definition, keeping only the
declaration that begins the intended field record. Ensure no empty field entry
is created before workflow_stall_nudges_cap.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e20fd183-aad5-439f-96a7-0e9bf69f904b

📥 Commits

Reviewing files that changed from the base of the PR and between 6f1abb4 and 16c3878.

📒 Files selected for processing (12)
  • CHANGELOG.md
  • docs/FEATURES.md
  • docs/game-engine-plugin-guide.md
  • docs/tui-guide.md
  • src/bmad_loop/adapters/base.py
  • src/bmad_loop/adapters/generic.py
  • src/bmad_loop/data/settings/core.toml
  • src/bmad_loop/engine.py
  • src/bmad_loop/policy.py
  • tests/test_generic_tmux.py
  • tests/test_plugin_workflows.py
  • tests/test_policy.py

Comment thread src/bmad_loop/data/settings/core.toml
@pbean pbean merged commit df42177 into main Jul 15, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Claude adapter: Stop hook + idle detection re-entrant loop prevents session from ending

1 participant