Skip to content

feat(mux): herdr cross-platform multiplexer backend#136

Merged
pbean merged 11 commits into
mainfrom
feat/herdr-mux-backend
Jul 14, 2026
Merged

feat(mux): herdr cross-platform multiplexer backend#136
pbean merged 11 commits into
mainfrom
feat/herdr-mux-backend

Conversation

@pbean

@pbean pbean commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds herdr (herdr.dev) as a new TerminalMultiplexer
backend — the first non-tmux-family, native-Windows-capable transport for
bmad-loop. herdr is a cross-platform, agent-aware terminal workspace manager
(headless server + CLI + ndjson socket; Unix socket on POSIX, named pipe on
Windows
). Implementing the seam over it gives bmad-loop a path toward native
Windows (#92) plus herdr's agent-status sidebar for watching runs — landed as
one registration line + one sanctioned probe.py gate fix, zero portability-guard
allowlist changes
, no core edits.

This is PR 1 of 2: the engine run path. The TUI-launch surface (parked
windows, attach/detach, full window options) is a deliberate follow-up (PR 2);
those methods degrade to raises/no-ops here.

Opt-in only. _PLATFORM_DEFAULTS is untouched — POSIX keeps tmux as the
platform default; herdr activates via bmad-loop mux set herdr /
BMAD_LOOP_MUX_BACKEND=herdr, or on win32 by first-match until psmux ships.

What's in the PR (phase-per-commit)

Commit Phase Contents
ef5cde6 1 adapters/herdr_backend.py core: _HerdrClient transport, server lifecycle, sidecar options, full HerdrMultiplexer ABC + conformance tests
f75c8b5 2 _PanePoller pipe_pane emulation (content-hash-gated tee) + tests
2d9c5d3 3 one registration line (after tmux) + probe.py which("tmux")backend.available() gate fix + selection tests
847e875 4 HAVE_HERDR-gated integration + one stories E2E, mirroring the tmux live tests
2c079ee 5 docs (porting / roadmap / authoring), CHANGELOG, degradation-ledger docstring

Design (characterized against live herdr 0.7.3 / protocol 16)

Mapping: bmad-loop session → herdr workspace (label == session name);
window → tab (one shell pane; native window id = its root_pane.pane_id,
w1:p1-shaped); launched command → typed exec <argv> via pane run (type +
Enter atomically) so process-exit == pane-close == tab-close == tmux-identical
window death
. All herdr subprocess I/O is funnelled through _HerdrClient so a
socket transport can swap in later without touching HerdrMultiplexer.

Phase-0 exit-criteria decisions:

  1. Launch = typed exec (not agent.start, which splits the active tab — reserved as the Windows launch path since POSIX exec doesn't exist there).
  2. _pane_counts_as_live() = server-answered pane get presence — the pane vanishes on exit (no linger, either mechanism), so presence is authoritative.
  3. Attach = ["herdr","terminal","attach",<terminal_id>] (pane_id→terminal_id via pane get); full surface is PR 2.
  4. Error taxonomy = JSON-code (server answered → may map to []/False) vs non-JSON Error: Os / TimeoutExpired / FileNotFoundError (→ raise). Liveness prefers enumeration (workspace list rc=0 proves the server answered; absent label = honest []).
  5. JSON = structured-by-default (create echoes new ids); pane read is raw text (--format text strips ANSI).

Phase-0 traps baked into the impl: CLI revision is unusable (stays 0) → poller content-hash-gates, not revision-gates; duplicate workspace labels are allowed → first-match resolution + has_session guard; pane list --workspace <absent> raises → enumerate-then-filter for liveness; no server autostart on any CLI verb → lazy _ensure_server() is mandatory; no absolute headless resize → new_session geometry is advisory (default 54×23).

Degradation ledger (PR-1 scope)

  • pipe_pane — herdr has no tee, so a per-window _PanePoller daemon snapshots
    pane read --source recent-unwrapped into the log whenever content changes
    (content-hash-gated). This drives the two log consumers a tmux tee would:
    generic._log_activity_key's dev-stall re-arm and probe's marker discovery.
  • new_parked_windowraises HerdrError (only tui/launch.py calls it → PR 2).
  • attach_target_argv — returns the herdr terminal attach <terminal_id> argv;
    full =session:window parsing + switch_client are PR 2.
  • detach_client — no-op (herdr detach is a keybinding, no CLI verb).
  • Session/window options — no native herdr equivalent → cross-process JSON
    sidecar (~/.bmad-loop/herdr-state.json, override BMAD_LOOP_HERDR_STATE,
    atomic_replace); entries for a gone workspace are pruned on next enumeration.
  • new_session geometry — advisory (no absolute headless resize).
  • ProtocolSUPPORTED_PROTOCOL = 16; fail below, warn-once above.

Selection behavior

  • POSIX + both installed: tmux still wins (platform default). herdr only on explicit force.
  • win32: _PLATFORM_DEFAULTS untouched — herdr picked by first-match when available; once psmux/tmux-windows registers for win32 the default reasserts, no code change.
  • available() = shutil.which("herdr") — a different binary family, so pairwise-discriminating against the tmux family by construction (no mux set tie-break needed). Never probes/starts the server.
  • ⚠️ Registering herdr with matches=True means it also matches win32; the Windows CI leg (no tmux/herdr binary) therefore no longer bottom-falls-back to tmux for the 27 tmux-argv surface tests. Phase 3 pins those to tmux via a shared force_tmux_backend fixture (env-forced, no-op on POSIX) — see commit 2d9c5d3.

Live smoke (executed on herdr 0.7.3, Linux; isolated HERDR_SESSION, torn down)

Real bmad-loop CLI on a fake-CLI stories sandbox + a direct-backend live-session demo:

bmad-loop mux                → herdr row: available ✓, herdr 0.7.3; tmux * platform default
bmad-loop mux set herdr      → written to .bmad-loop/policy.toml
bmad-loop mux                → selection: herdr (set by [mux] backend)
bmad-loop validate           → multiplexer HerdrMultiplexer available (herdr 0.7.3);
                               mux backends: tmux, herdr*; process host: PosixProcessHost
bmad-loop run (fake CLI)     → run …-e8a8: 2 done, 0 deferred; stories 1 & 2 = done; +2 commits
  log growth                 → runs/<id>/logs/1-dev-1.log 604 B, 2-dev-1.log 478 B
                               (tee'd: `exec …/fake-cli.sh '/bmad-dev-auto …'` + pane content)
bmad-loop mux set --clear    → reverts to auto; mux → tmux platform default

Workspace/agent-status visibility + @bmad_project prune (a finished run self-cleans
its workspace, so demonstrated against a held-live session):

herdr workspace list         → workspace w1  label='bmad-loop-smokedemo01'   (sidebar data)
herdr pane list              → w1:p1, w1:p2  agent_status='unknown'          (exec-launched;
                               semantic agent_status rides the agent.start/Windows path)
session_options(@bmad_project) → {'bmad-loop-smokedemo01': '<project path>'} (sidecar round-trip)
bmad-loop cleanup --dry-run  → would kill session bmad-loop-smokedemo01
teardown                     → kill_session → list_sessions() == []; session stop+delete rc=0

Testing

  • uv run pytest -q2285 passed, 1 skipped (full suite).
  • trunk check (no filter) → clean (one isort autofix to test_herdr_poller.py, in this PR).
  • black / ruff → clean.
  • Gated tests (tests/test_herdr_integration.py, tests/test_herdr_e2e.py): 6 passed with herdr installed; 6 skipped cleanly with herdr off PATH (and on win32 — the POSIX exec launch is PR-6).
  • Portability guard: zero allowlist changes.

Risks

  1. Pane linger breaking window-death completion for SessionEnd-less CLIs (codex) — Phase-0 saw no linger (pane vanishes); crash-detection integration test pins it.
  2. Unknown user shell for the typed exec launch (fish/pwsh) — POSIX-only by design; agent.start is the Windows path.
  3. herdr 0.x churn — fail-below/warn-above protocol guard; version surfaced in mux/validate; all I/O isolated in _HerdrClient.
  4. Windows unverified (no Windows box) — PR 1 is POSIX-verified / Windows-by-construction; the Phase-0 characterization must be re-run on a real Windows host before claiming Feature Request: Support for Native Windows (Remove tmux dependency for Windows developers) #92; defaults untouched so nothing auto-selects wrongly.
  5. Snapshot logs vs alt-screen TUIs — content-hash gating + recent-unwrapped source; integration test asserts growth.

Follow-up (PR 2 — TUI-launch surface)

new_parked_window (POSIX exit-status window), full select_window /
set|show|unset_window_option, list_windows field mapping for
tui/launch.py, attach_target_argv/switch_client finalization, and the two
direct TMUX env reads → seam-honest current_*. Windows launch path
(agent.start) + a Windows-host re-run of Phase 0 also live beyond PR 1.

Summary by CodeRabbit

  • New Features
    • Added an opt-in herdr terminal multiplexer backend for non-tmux-family setups (via environment or persistent mux configuration).
    • Provides session/window/pane lifecycle, exec-style launches, pane logging, and terminal attachment on supported platforms; tmux remains the default where available.
  • Documentation
    • Expanded authoring/porting and platform guidance for implementing and using the herdr backend; updated the roadmap.
  • Bug Fixes
    • Improved probe preflight fallback to use the selected backend’s availability rather than assuming tmux.
  • Tests
    • Added comprehensive herdr unit/integration/e2e and deterministic backend-selection test coverage.

pbean added 5 commits July 13, 2026 17:46
Implement HerdrMultiplexer behind the TerminalMultiplexer seam, characterized
against herdr 0.7.3 / protocol 16 (plan Phase-0 Findings). Not a tmux-family
subclass — herdr is a different binary family, so the contract is implemented
fresh.

Mapping: bmad-loop session -> herdr workspace (label == session name);
window -> tab (one shell pane), native window id == the tab's root pane id;
launch via typed 'exec <argv>' (pane run = type+Enter) so process exit ==
pane close == tab close == tmux-identical window death.

- _HerdrClient isolates all subprocess I/O (_run/_herdr/_herdr_json) plus the
  server lifecycle (ensure_server: single warm status read, autostart when
  down, fail-below/warn-above protocol guard) so a socket transport can swap in.
- Raisers (has_session/new_session/new_window/set_session_option/send_text/
  list_window_ids/window_alive) never leak a raw subprocess error; liveness
  enumerations are honest ([] means 'no windows', transport failure raises).
- Duplicate workspace labels tolerated (first-match) + has_session dup-guard.
- Session/window options live in a cross-process sidecar JSON (atomic_replace,
  BMAD_LOOP_HERDR_STATE override); dead-workspace entries pruned on enumerate.
- Degradations: pipe_pane = stub no-op (Phase 2), new_parked_window raises
  (PR 2), detach_client no-op, geometry advisory.

Not registered (Phase 3); pipe_pane poller not implemented (Phase 2); no
consumer files touched. Gate: test_herdr_backend + test_multiplexer green,
black clean, portability guard passes with zero allowlist changes.
herdr has no pipe-pane/tee, so emulate it with a per-window _PanePoller
daemon: poll `pane read --source recent-unwrapped` and append a fresh
snapshot to the log whenever the content changes (content-hash gated —
the CLI revision is unusable, it stays 0). This drives the two consumers
a tmux tee would: generic._log_activity_key re-arms the dev-stall grace
on log growth, and probe finds completion markers in the log text.

- pipe_pane primes one synchronous read; a pane_not_found / unreachable
  read starts no tee (tolerant of the crash-on-launch race, like tmux
  swallowing TmuxError).
- Poller retired by kill_window (exact pane) / kill_session (workspace
  prefix) or self-retires after N consecutive pane_not_founds; a
  transport hiccup is neither growth nor death (streak preserved).
- FakeHerdr now answers `pane read` (raw text + bare pane_not_found);
  new tests/test_herdr_poller.py covers growth/skip, marker discovery,
  kill + self-retire, hiccup tolerance, no thread leak.
Register the herdr backend as a builtin AFTER tmux in
_load_builtin_backends with matches=lambda platform: True. On POSIX the
tmux platform-default still wins (herdr is opt-in via BMAD_LOOP_MUX_BACKEND
/ the [mux] backend policy); on native Windows herdr is the first platform
match (tmux doesn't match, psmux out-of-tree). _PLATFORM_DEFAULTS untouched.

Sanctioned consumer fix: probe.py's launch gate now checks the SELECTED
backend's available() instead of a hardcoded which("tmux"), so a Windows
host running herdr (or a future psmux) can still probe. Names the backend
in the fallback-to-scan warning.

Registry/selection tests (fresh_registry fixture): tmux still wins on POSIX
when both available; herdr first-match when tmux absent; env override forces
herdr; win32 first-match + unavailable-fallback both select herdr; a herdr
detect_multiplexers row. Portability guard passes with ZERO allowlist
changes (argv ["herdr",...], Popen via detach_kwargs(), sidecar under
Path.home()).

Collateral: herdr now matches win32, so get_multiplexer() no longer
bottom-falls-back to tmux on the Windows CI leg — it returns herdr. The 27
tmux-argv tests that drove the seam expecting tmux (test_tui_launch,
test_runs attach_argv, test_tui_data legacy-liveness) now pin tmux by name
via a shared force_tmux_backend conftest fixture (a no-op on POSIX). The two
"auto-default is tmux" registry tests pin sys.platform to a POSIX value.
Verified via a faithful win32 selection simulation (herdr removed from PATH):
was 27 failed, now 0. Full linux suite: 2279 passed, 1 skipped.
Mirror test_generic_tmux.py's three live tests through HerdrMultiplexer
against a real herdr 0.7.3 server (tests/test_herdr_integration.py):
fake-CLI end-to-end -> completed (claude/codex/gemini), crash detection
(pins the Phase-0 O1 pane-exit finding), and pipe_pane log growth under a
producing pane. Plus one stories happy-path lifted from test_stories_e2e.py's
fake-claude recipe through the real bmad-loop binary (tests/test_herdr_e2e.py).

All behind HAVE_HERDR (skipped off win32 / without herdr), each test on its
own isolated HERDR_SESSION with a guaranteed session stop+delete teardown and
the herdr backend forced by name. 6 pass with herdr installed, 6 skip cleanly
without it.
Document the shipped herdr multiplexer backend across the porting/authoring docs,
add a terse CHANGELOG entry, and round out the backend's module-docstring
degradation ledger with the attach story.

- docs/porting-to-a-new-os.md: herdr as the first shipped "implement
  TerminalMultiplexer fresh" worked example + a different-binary-family
  availability-discriminator note (which("herdr") is pairwise-discriminating by
  construction; available()/version()/ctor never touch the server).
- docs/ROADMAP.md: herdr as the third (shipped, non-tmux-family) native-Windows
  candidate; POSIX keeps tmux as platform default, win32 picks herdr only by
  first-match, _PLATFORM_DEFAULTS untouched so psmux reasserts once it ships.
- docs/adapter-authoring-guide.md: one paragraph mapping the herdr transport
  (session->workspace, window->tab, exec launch, sidecar options, poller pipe_pane).
- herdr_backend.py: add the attach_target_argv bullet to the degradation ledger.
- CHANGELOG: terse [Unreleased] entry for the opt-in herdr backend (run path;
  TUI-launch surface deferred to a follow-up PR).

Also applies a trunk-surfaced isort fix to tests/test_herdr_poller.py (the
cross-file FakeHerdr import belongs in the third-party group) — behaviorally
identical; first full 'trunk check' runs in this phase.

Verification: uv run pytest -q -> 2285 passed, 1 skipped; trunk check (no filter)
clean; black/ruff clean; live fake-CLI stories run over herdr + direct-backend
session demo (workspace/agent-status visibility + @bmad_project prune) green.
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@pbean, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f5917d81-e6ad-4f24-8a1f-bae04dffb2a0

📥 Commits

Reviewing files that changed from the base of the PR and between 2f9154a and d42be23.

📒 Files selected for processing (13)
  • CHANGELOG.md
  • docs/ROADMAP.md
  • docs/porting-to-a-new-os.md
  • src/bmad_loop/adapters/herdr_backend.py
  • src/bmad_loop/adapters/tmux_base.py
  • src/bmad_loop/cli.py
  • src/bmad_loop/tui/app.py
  • src/bmad_loop/tui/launch.py
  • tests/test_cli.py
  • tests/test_herdr_backend.py
  • tests/test_herdr_integration.py
  • tests/test_tui_app.py
  • tests/test_tui_launch.py

Walkthrough

Adds an opt-in herdr terminal multiplexer backend with server management, workspace/tab mapping, pane polling, sidecar options, backend registration, probe integration, documentation, and extensive fake, integration, and end-to-end coverage.

Changes

Herdr backend implementation

Layer / File(s) Summary
Client, protocol, and state handling
src/bmad_loop/adapters/herdr_backend.py, src/bmad_loop/platform_util.py
Adds herdr subprocess execution, protocol checks, JSON parsing, atomic sidecar persistence, and cross-platform advisory locking.
Workspace, tab, pane, and polling lifecycle
src/bmad_loop/adapters/herdr_backend.py
Maps sessions and windows to herdr entities, launches commands, manages options and attachment, and emulates pane logging with poller threads.
Backend registration and selection behavior
src/bmad_loop/adapters/multiplexer.py, src/bmad_loop/probe.py, docs/*, CHANGELOG.md, tests/test_backend_registry.py
Registers herdr, updates probe fallback behavior, documents selection and implementation details, and tests precedence across platforms.
Backend contract and failure conformance
tests/test_herdr_backend.py
Covers command construction, liveness, transport failures, options, protocol handling, degradation behavior, attachment, and context accessors.
Poller and real-server validation
tests/test_herdr_*.py, tests/conftest.py, tests/test_platform_util.py, tests/test_runs.py, tests/test_tui_*.py, tests/test_generic_tmux.py
Tests poller lifecycle, real herdr execution, crash handling, log growth, locking, session isolation, and deterministic tmux-specific paths.

Estimated code review effort: 5 (Critical) | ~90 minutes

Possibly related PRs

Suggested reviewers: dracic

Sequence Diagram(s)

sequenceDiagram
  participant Run
  participant HerdrMultiplexer
  participant HerdrServer
  participant PanePoller
  participant LogFile
  Run->>HerdrMultiplexer: create session and window
  HerdrMultiplexer->>HerdrServer: create workspace and tab
  HerdrMultiplexer->>HerdrServer: run typed exec argv
  HerdrMultiplexer->>PanePoller: start pane polling
  PanePoller->>HerdrServer: read recent pane content
  PanePoller->>LogFile: append changed snapshots
  Run->>HerdrMultiplexer: send text and inspect liveness
Loading

Poem

I hop through herdr tabs with carrots in tow,
Pollers catch pane changes as bright markers glow.
Sidecars lock safely, servers wake on demand,
Tmux keeps its throne across POSIX land.
New paths are tested from start to the end—
A rabbit applauds this backend friend.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.59% 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 is concise and accurately summarizes the main change: adding the herdr cross-platform multiplexer backend.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/herdr-mux-backend

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.

Phase 5 is the first push that runs the Windows CI legs; they surfaced 8
pre-existing win32-only failures latent since Phase 3 registered herdr with
matches=True. Two classes, both test-only:

1. Path-separator (test_herdr_backend.py x2): str(Path("/work")) is '\\work'
   on win32, but the argv assertions hardcoded '/work'. Assert str(Path(...))
   so the expected --cwd matches the backend's stringified Path on either OS.

2. Selection collateral (test_generic_tmux.py x1, test_tui_app.py x5): herdr
   *matches* win32, so on the Windows leg get_multiplexer() falls back to herdr
   (unavailable there) instead of tmux; these tmux-surface tests then drive
   herdr through the seam and its Popen('herdr server') fails. Phase 3's
   force_tmux_backend fixture already fixed this class for test_tui_launch /
   test_runs / test_tui_data but missed test_generic_tmux's non-gated unit test
   (default mux = get_multiplexer()) and the 5 test_tui_app attach tests. Apply
   the same fixture (a no-op on POSIX, pins tmux on win32).

Verified: the 8 tests pass on Linux; the 6 selection-collateral tests still pass
with BMAD_LOOP_MUX_BACKEND=herdr forced (proving force_tmux_backend engages and
selects tmux, the win32 behavior); full affected files 200 passed; trunk clean.

@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: 2

🧹 Nitpick comments (4)
tests/test_herdr_integration.py (1)

59-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

_teardown_session duplicated with tests/test_herdr_e2e.py.

Identical stop/delete teardown logic in both files. See consolidated comment.

🤖 Prompt for 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.

In `@tests/test_herdr_integration.py` around lines 59 - 63, Remove the duplicated
_teardown_session helper from tests/test_herdr_integration.py and reuse the
shared teardown implementation already defined for the herdr tests, preserving
the existing stop-then-delete best-effort behavior.
tests/test_herdr_poller.py (1)

26-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicates test_herdr_backend.py's fake fixture.

Same subprocess/shutil/env setup as tests/test_herdr_backend.py lines 173-180, plus two extra monkeypatch.setattr calls. See consolidated comment for a fixture-composition suggestion.

🤖 Prompt for 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.

In `@tests/test_herdr_poller.py` around lines 26 - 38, Remove the duplicated
subprocess, shutil, and environment setup from the fake fixture in
tests/test_herdr_poller.py, and reuse or compose the existing fake fixture from
test_herdr_backend.py. Preserve the poller-specific POLL_INTERVAL_S and
POLL_NOT_FOUND_LIMIT overrides in the poller tests.
tests/test_herdr_backend.py (1)

173-184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

fake fixture duplicated with tests/test_herdr_poller.py.

This fixture (subprocess.run/shutil.which patch, BMAD_LOOP_HERDR_STATE env, HERDR_ENV clear) is copy-pasted almost verbatim in tests/test_herdr_poller.py (lines 26-38), which only adds two extra monkeypatch.setattr calls for polling globals. Risk of drift if one is updated and the other isn't. See consolidated comment.

🤖 Prompt for 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.

In `@tests/test_herdr_backend.py` around lines 173 - 184, Consolidate the
duplicated fake fixture setup shared by tests/test_herdr_backend.py and
tests/test_herdr_poller.py into one reusable fixture or helper, preserving the
existing subprocess.run, shutil.which, BMAD_LOOP_HERDR_STATE, and HERDR_ENV
setup. Keep the poller-specific global patches in the poller test fixture while
reusing the common setup.
tests/test_herdr_e2e.py (1)

37-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicates _teardown_session from tests/test_herdr_integration.py.

See consolidated comment.

🤖 Prompt for 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.

In `@tests/test_herdr_e2e.py` around lines 37 - 39, Remove the duplicate
_teardown_session helper from tests/test_herdr_e2e.py and reuse the existing
implementation from tests/test_herdr_integration.py, updating imports or
references as needed while preserving the current teardown behavior.
🤖 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/adapters/herdr_backend.py`:
- Around line 105-146: Protect every shared sidecar read-modify-write cycle with
one OS-level advisory lock, including the flows in session_options,
set_session_option, set_window_option/unset_window_option, and _drop_state.
Update _load_state and _save_state or introduce a shared locking helper so the
lock spans the complete load, mutation, and save sequence, while preserving
set_session_option’s HerdrError wrapping and sentinel callers’ existing error
behavior.

In `@tests/test_herdr_integration.py`:
- Around line 170-218: Wrap the poller lifecycle and growth/marker assertions in
test_herdr_pipe_pane_log_grows_under_real_pane with try/finally, ensuring
mux.kill_session(session) always executes after pipe_pane starts the poller.
Keep the existing assertions and final _pollers cleanup check, but perform
cleanup before the fixture can tear down the Herdr session.

---

Nitpick comments:
In `@tests/test_herdr_backend.py`:
- Around line 173-184: Consolidate the duplicated fake fixture setup shared by
tests/test_herdr_backend.py and tests/test_herdr_poller.py into one reusable
fixture or helper, preserving the existing subprocess.run, shutil.which,
BMAD_LOOP_HERDR_STATE, and HERDR_ENV setup. Keep the poller-specific global
patches in the poller test fixture while reusing the common setup.

In `@tests/test_herdr_e2e.py`:
- Around line 37-39: Remove the duplicate _teardown_session helper from
tests/test_herdr_e2e.py and reuse the existing implementation from
tests/test_herdr_integration.py, updating imports or references as needed while
preserving the current teardown behavior.

In `@tests/test_herdr_integration.py`:
- Around line 59-63: Remove the duplicated _teardown_session helper from
tests/test_herdr_integration.py and reuse the shared teardown implementation
already defined for the herdr tests, preserving the existing stop-then-delete
best-effort behavior.

In `@tests/test_herdr_poller.py`:
- Around line 26-38: Remove the duplicated subprocess, shutil, and environment
setup from the fake fixture in tests/test_herdr_poller.py, and reuse or compose
the existing fake fixture from test_herdr_backend.py. Preserve the
poller-specific POLL_INTERVAL_S and POLL_NOT_FOUND_LIMIT overrides in the poller
tests.
🪄 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: 44a21071-5e19-42c3-90c1-6d4dff57968e

📥 Commits

Reviewing files that changed from the base of the PR and between e4934a1 and 2c079ee.

📒 Files selected for processing (16)
  • CHANGELOG.md
  • docs/ROADMAP.md
  • docs/adapter-authoring-guide.md
  • docs/porting-to-a-new-os.md
  • src/bmad_loop/adapters/herdr_backend.py
  • src/bmad_loop/adapters/multiplexer.py
  • src/bmad_loop/probe.py
  • tests/conftest.py
  • tests/test_backend_registry.py
  • tests/test_herdr_backend.py
  • tests/test_herdr_e2e.py
  • tests/test_herdr_integration.py
  • tests/test_herdr_poller.py
  • tests/test_runs.py
  • tests/test_tui_data.py
  • tests/test_tui_launch.py

Comment thread src/bmad_loop/adapters/herdr_backend.py
Comment thread tests/test_herdr_integration.py
pbean added 2 commits July 13, 2026 20:46
CodeRabbit (PR #136) confirmed: the sidecar's load->mutate->save cycles were
unguarded across processes — atomic_replace only prevents torn reads, so the
engine's session tag (generic._ensure_session) racing the TUI/CLI enumeration's
prune-rewrite (runs.session_project_tags -> session_options) could silently
lose updates, the serialization the tmux server gave set-option for free.

- new platform_util.file_lock(path, blocking=True): exclusive OS advisory lock
  (fcntl.flock POSIX / msvcrt.locking win32), kernel-released on process death,
  blocking=False as a deterministic exclusion probe for tests
- all five cycles (set_session_option, set/unset_window_option, _drop_state,
  session_options' prune) hold the lock on a sibling .lock file — never the
  data file, whose inode atomic_replace swaps out; raiser/sentinel split
  preserved (lock failure -> HerdrError vs skipped write, never unguarded)
- session_options' read stays lock-free (atomic swap = consistent snapshot);
  its prune re-loads under the lock before saving; the residual outside-the-
  lock liveness snapshot race is documented (bounded by prunable_sessions'
  run-dir fallback)
- try/finally around the pipe_pane growth test so an assertion failure can't
  leak the poller past the fixture's server teardown (transport failures never
  self-retire a tee, so a leaked one spins pane-reads for the worker's life)
- dedup _teardown_session (e2e now imports integration's copy) and the fake
  fixture (test_herdr_backend.install_fake_herdr helper; the poller fixture
  keeps only its poll-cadence patches on top)
@pbean

pbean commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator Author

The 4 review-body nitpicks (duplicated _teardown_session in test_herdr_integration/test_herdr_e2e, duplicated fake fixture in test_herdr_backend/test_herdr_poller) are all valid and addressed in 2f9154a: the e2e module now imports the integration module's _teardown_session (the cross-test-module import pattern this PR already uses), and the shared fixture body is extracted to test_herdr_backend.install_fake_herdr with the poller fixture keeping only its poll-cadence patches on top.

@pbean

pbean commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator Author

@CodeRabbit review

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Complete the herdr backend's tui/launch.py surface (PR 2 of the herdr
series): parked orchestrator windows via a typed 'exec sh -c' recipe with
a per-window return file the trailer consumes, one _parse_target helper
for tmux-style '=session[:window]' targets across select/kill/option/
attach/switch methods (window-option sidecar keys normalized to the
native pane id), list_windows tmux-field mapping (window_id/window_name/
@options via a tab-label join), finalized attach_target_argv (terminal
attach outside herdr, fire-and-forget tab focus inside), and
switch_client via tab focus.

Seam-honesty migration: the TUI/CLI attach flows and in_ctl_session now
ask the seam (current_pane_id/current_session) instead of sniffing TMUX;
BaseTmuxBackend._display_message gains the TMUX guard that keeps 'not
inside' honest (behavior-preserving at every call site, pinned by tests).

Unit tests extend FakeHerdr with tabs/focus/active_tab_id; a new
HAVE_HERDR-gated live test drives launch.start_detached(validate)
end-to-end (park -> banner -> return-file -> unpark -> workspace
survives). Full suite green with and without TMUX in the env; win32-sim
(forced herdr, no binary) green on all touched files.
CodeRabbit round-2 fixes on the TUI-launch surface:

- new_parked_window: the herdr launch is three steps (tab create ->
  sidecar write -> typed recipe) where tmux gets one atomic new-window;
  a failure after tab create now rolls the fresh tab back via the
  existing kill_window teardown (close + sidecar prune + return-file
  cleanup) instead of leaving an untracked idle shell in the ctl
  workspace until workspace close.
- TUI attach: attach_target_argv is a server round-trip on herdr and
  can raise MultiplexerError after the availability/session pre-gates
  pass; _attach_to_target now surfaces it as an error toast instead of
  crashing the app. The cmd_attach half of the finding was declined:
  cli.main's backstop already prints 'error: <msg>' + rc 1 (a new
  regression test pins that).
- ROADMAP: the herdr status paragraph now matches the header line
  (shipped end-to-end on POSIX: run path + TUI-launch surface).

Full suite 2310 green with and without TMUX in the env; trunk clean.
@pbean pbean merged commit fce9e43 into main Jul 14, 2026
9 checks passed
pbean added a commit that referenced this pull request Jul 14, 2026
…herdr

The herdr backend (engine run path #136, TUI-launch surface #137) moves out
of core to pbean/bmad-loop-adapter-herdr before ever shipping in a release.
The adapter registers through the bmad_loop.mux_backends entry-point
discovery (previous commit): co-install it with bmad-loop and 'herdr'
appears in `bmad-loop mux` exactly as before. Core now bundles only tmux —
_load_builtin_backends() is tmux-only again.

Removed: adapters/herdr_backend.py plus its four test files; they continue
as the adapter repo's backend.py + suite, where the live-herdr integration
and E2E tests still run. test_backend_registry gains
test_win32_bottoms_out_at_tmux_with_no_externals pinning the win32 fallback
reversal now that nothing bundled matches win32.

Kept deliberately: the seam docstrings, probe.py gate comment, and
tui/launch.py note that cite herdr as the external worked example, and the
win32-matching rationale comments in tests — an installed external backend
may match win32 (the herdr adapter does), so force_tmux_backend and the
registry/launch tests pin selection against exactly that leak.

Docs: multiplexer-backends.md's herdr section becomes 'External backends'
(install recipe + adapter-repo links); README / FEATURES / docs index /
setup guide / TUI guide / ROADMAP / porting guide / authoring guide now name
herdr as an external adapter and link its operator guide for the seat-level
differences (detach chord ctrl+b q, polled logs, state sidecar). CHANGELOG
[Unreleased] herdr bullets collapse into one extraction entry.

BREAKING CHANGE: bmad_loop.adapters.herdr_backend is gone (it never shipped
in a release); install bmad-loop-adapter-herdr alongside bmad-loop instead.
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.

1 participant