feat(mux): herdr TUI-launch surface#137
Conversation
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.
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughHerdr now supports POSIX parked-window launches, return-file handling, target resolution, attachment, tab focus, window listing, cleanup, and backend-derived context detection. Documentation and tests describe and validate the completed TUI/CLI launch surface. ChangesHerdr TUI launch surface
Estimated code review effort: 4 (Complex) | ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/ROADMAP.md`:
- Line 12: Update the later herdr status paragraph in docs/ROADMAP.md to state
that herdr is available end-to-end on POSIX, including both the engine run path
and TUI-launch surface. Keep the remaining win32 agent.start launch follow-up
consistent with the status line.
In `@src/bmad_loop/adapters/herdr_backend.py`:
- Around line 829-862: Update new_parked_window to wrap the post-creation
sidecar update and pane run in failure cleanup: if either operation fails after
tab creation, best-effort close pane_id before re-raising the original error.
Preserve the existing HerdrError conversion for sidecar OSError and ensure
cleanup failures do not replace the original exception.
In `@src/bmad_loop/tui/app.py`:
- Around line 379-392: The attach flows in `_attach_to_target` and the
corresponding `cmd_attach` logic in `launch.py` call `runs.attach_target_argv()`
without handling `MultiplexerError` or `HerdrError`. Wrap both attach-argument
calls in the existing error-handling path, surface a user-facing error message,
and return without launching or crashing when either exception occurs.
🪄 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: 7befd407-eddf-4cbc-ac2f-ec79da408cf9
📒 Files selected for processing (11)
CHANGELOG.mddocs/ROADMAP.mddocs/porting-to-a-new-os.mdsrc/bmad_loop/adapters/herdr_backend.pysrc/bmad_loop/adapters/tmux_base.pysrc/bmad_loop/cli.pysrc/bmad_loop/tui/app.pysrc/bmad_loop/tui/launch.pytests/test_herdr_backend.pytests/test_herdr_integration.pytests/test_tui_launch.py
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.
|
@CodeRabbit review |
✅ Action performedReview finished.
|
…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.
1 participant
PR 2 of the herdr series (#136 = engine run path). Implements the full
tui/launch.pysurface on the herdr backend, so TUI-launched runs/sweeps/resolves, attach, prune, and the return-to-origin hop all work underBMAD_LOOP_MUX_BACKEND=herdr.What's here
Parked windows —
new_parked_windowcreates a tab and types a POSIXexec sh -c '<argv>; ec=$?; echo "[bmad-loop exited $ec — press enter]"; read -r; <trailer>'recipe (tmux-identical from the operator's seat). The tmux trailer reads the return option live viashow-options; herdr window options live in our sidecar JSON, which a one-lineshtrailer can't query — so the option methods mirror the parked window's return option into a per-window return file the trailercats. The value is resolved to a tab id at write time (Python owns the JSON parsing), keeping the trailer a dumbherdr tab focus. POSIX-only, like theexeclaunch (pwsh dialect = the win32 follow-up, withagent.start).Target parsing — one
_parse_targethelper resolves tmux-style=session[:window]specs (whattui/launch.pybuilds) vs native pane ids acrossselect_window,kill_window,set/unset/show_window_option,attach_target_argv,switch_client. Window-option sidecar keys are normalized to the native pane id, soset_return_pane("=bmad-loop-ctl:run-X", …)andreturn_attached_client()'s read by native id agree.Attach, finalized — outside herdr:
["herdr","terminal","attach",<terminal_id>](blocks; exits when the pane closes — verified live). Inside a herdr pane: the fire-and-forget["herdr","tab","focus",<tab_id>], mirroring tmux's in-TMUXswitch-client branch. Session-level=sessiontargets resolve to the workspace's active tab, except when that's the root shell and task tabs exist — then the newest tab (task tabs are--no-focus, so an agent workspace keeps root active while the window you want is the newest; ctl attach is unaffected becauseattach_planrunsselect_windowfirst).switch_client — a
tab focuson the target's tab; focusing a tab in another workspace flips workspace focus too (verified 0.7.3), so one verb covers the whole return hop. No "last client" concept in herdr →last_fallbackdegrades to honestFalse.list_windows tmux-field mapping —
window_id/pane_id→ pane id,window_name→ owning tab's label (baretab listjoin —tab list --workspace <absent>RAISESworkspace_not_found, same trap aspane list),session_name,@…options → sidecar. Backsctl_window/_ctl_window_candidatesverbatim.Seam-honesty migration (the two
TMUXenv reads from the plan, plus one same-pattern read found intui/app.py):launch.in_ctl_session(),cli.cmd_attach, andapp._attach_to_targetnow ask the seam (current_pane_id()/current_session()) instead of sniffingTMUX. To keep that behavior-preserving,BaseTmuxBackend._display_messagegained theTMUXguard: against a live server,display-messagewould answer for some other client's session and misreport a plain shell as inside tmux. Every call site already guarded onTMUX, so tmux behavior is unchanged (pinned by tests); under herdr these paths now work (HERDR_*env is the inside marker, no subprocess).Live re-verification (herdr 0.7.3, isolated
HERDR_SESSIONprobes)$ecexpanded, pane parks onread -r, Enter runs the trailer, pane+tab close, workspace survives on its root tab.herdr terminal attachexits when its pane closes → the tmux "trailer detaches the throwaway client" move is emulated by pane-close itself.tab focusworks headless, updatesactive_tab_id, and flips workspace focus cross-workspace.workspace creategets the numeric label"1"→ can never collide with<kind>-<run_id>window names.Degradations (ledger updated in the module docstring)
detach_clientstays a no-op (keybinding-only in herdr; no CLI verb — re-checkedterminal --help). The post-exit detach return is full-fidelity anyway (pane close endsterminal attach); only the mid-processRETURN_DETACHhand-back (sweep finishing decisions while a plain-terminal client watches) degrades to "stay attached until the parked window closes".switch_clienthas no last-client fallback.Tests
active_tab_id): parked argv+recipe shape, return-option round-trip incl.=targetnormalization + return-file mirror/unset/kill/kill-session cleanup, target-form matrix for select/attach/switch, session-level attach heuristic,list_windowsmapping, in_ctl_session under herdr, tmux_display_messageTMUX-guard pins.HAVE_HERDR, isolatedHERDR_SESSION):launch.start_detached(["validate", …])→ ctl workspace,ctl_windowdiscovery via the tab-label join, parked banner (regex on the expandedexited \d+, since the typed recipe contains the literal$ec), return-file write via=nametarget, Enter-unpark → pane gone + file consumed + workspace survives. Skips cleanly with herdr off PATH.TMUXin the env (this box runs inside tmux; CI doesn't — the guard change made several pre-existing tests' env assumptions explicit).BMAD_LOOP_MUX_BACKEND=herdr, herdr binary off PATH) on all touched test files: 539 passed. The new consumer paths are safer on win32 than before: herdrcurrent_*is env-only (no subprocess).trunk check(no filter): clean.Risks
=sessionactive-vs-newest-tab heuristic is behavior chosen for our two workspace shapes (agent: root+task; ctl: focused-by-select). A user who manually focuses the root tab of an agent workspace then attaches via CLI gets the newest task tab — intended, documented in_session_level_tab.agent.start) and pwsh parked dialect remain the follow-up (Feature Request: Support for Native Windows (Remove tmux dependency for Windows developers) #92 thread).Summary by CodeRabbit
New Features
Bug Fixes
Documentation