Multiplexer selection: availability-aware defaults + dev-chosen backend (#87)#118
Merged
Conversation
…d writer The machine-scoped terminal-multiplexer choice (issue #87) persists as [mux] backend in policy.toml. MuxPolicy stays data-only (registered-name existence is checked at selection time); the writer is a targeted line replace against the template's commented anchor so hand edits and comments survive without a core TOML-writer dependency.
… platform defaults get_multiplexer now resolves by precedence — BMAD_LOOP_MUX_BACKEND env, the policy [mux] backend (installed via configure_multiplexer), the platform default when registered+available, the first available platform match — bottoming out at the historical fallback so a tmux-less POSIX host still returns TmuxMultiplexer for validate to report. Forced names bypass matches()/available() (explicit = trusted) and raise loudly when unregistered, naming the policy file for the policy path. detect_multiplexers() enumerates the registry (never raises) for bmad-loop mux and the validate preflight. Registration order now breaks ties only among available non-default backends — two same-platform backends (psmux / tmux-windows, issue #87) no longer collide by import order.
…ght detection main() installs the policy [mux] backend into the multiplexer seam before dispatch — the one point that covers probe/diagnose/attach/stop, which never load policy. `bmad-loop mux` lists registered backends (platform/available/version/selected+why); `mux set <name>` persists the choice via write_mux_backend (unregistered names error unless --force, unavailable ones persist with a warning — explicit = trusted). validate's preflight keeps its existing lines byte-compatible and adds an advisory multi-backend listing plus env/policy provenance notes. No prompts anywhere: runs are unattended.
…on hint policy.toml is per-machine-per-repo (it carries the [mux] backend choice and the TUI settings editor rewrites it), so init now gitignores it like runs/ and cache/. A gitignore entry doesn't untrack an already-committed file, so init best-effort detects a tracked policy.toml and prints the one-time 'git rm --cached' hint. CHANGELOG documents both the feature and the migration.
…ator contract core.toml gains the [mux] section (schema sync-tested against MuxPolicy). The porting guide documents the five-step precedence and a new 'availability discriminators' contract — psmux and tmux-windows both drive a binary named tmux, so their available() probes must be pairwise discriminating (the handoff for PR #85 and the psmux re-cut). ROADMAP reframes the two as sibling tmux-family backends behind selection rather than interim-vs-future. README/FEATURES/setup/tui guides pick up the mux command, the policy.toml gitignore, and the migration hint.
…B110) The preflight detect block and _configure_mux now assign a fallback in the except arm instead of try/except/pass; prettier reflowed the ROADMAP paragraph and table alignment.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe change adds machine-scoped multiplexer policy, availability-aware selection and diagnostics, new ChangesMultiplexer selection and policy
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant CLI
participant PolicyFile
participant MultiplexerRegistry
Developer->>CLI: run bmad-loop mux set name
CLI->>PolicyFile: write [mux].backend
CLI->>MultiplexerRegistry: configure selection
MultiplexerRegistry->>MultiplexerRegistry: probe registered backends
MultiplexerRegistry-->>CLI: backend diagnostics and selection reason
CLI-->>Developer: report selected or unavailable backend
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/bmad_loop/adapters/multiplexer.py (1)
294-352: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPlatform-default step doesn't verify
matches(sys.platform)for the candidate backend.
_factory_by_name(default)(Line 338) only checks registration by name; it never confirms the backend's ownmatches()predicate agrees with the current platform — unlike the "first-match" loop right below it (Lines 343-345), which does. Harmless today (onlytmuxships, and itsmatchescovers everything but win32), but a future same-platform backend registering under a name that collides with_PLATFORM_DEFAULTS/_DEFAULT_BACKENDfor the wrong platform would be selected here purely onavailable(), bypassing the platform check every other step enforces.🛡️ Proposed fix: fold the matches() check into the default lookup
default = _PLATFORM_DEFAULTS.get(sys.platform, _DEFAULT_BACKEND) - default_factory = _factory_by_name(default) - if default_factory is not None: - backend = _instance(default, default_factory) - if _usable(backend): - return backend, default, "platform-default" + for name, matches, factory in _BACKENDS: + if name == default and matches(sys.platform): + backend = _instance(name, factory) + if _usable(backend): + return backend, name, "platform-default" + break for name, matches, factory in _BACKENDS: if matches(sys.platform) and _usable(_instance(name, factory)): return instances[name], name, "first-match"🤖 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 `@src/bmad_loop/adapters/multiplexer.py` around lines 294 - 352, Update the platform-default selection in _select so the backend is considered only when its registered matches predicate returns true for sys.platform, in addition to passing _usable. Locate the default_factory/_instance block and retain the existing first-match and fallback behavior for candidates that do not match the platform.
🤖 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/multiplexer.py`:
- Around line 399-409: In the backend probing logic around the factory
invocation, isolate the availability probe from the version probe: keep
`factory()` and `_usable(backend)` in the block that sets `available`, then call
`backend.version()` in a separate guarded block that only leaves `version` as
`None` on failure. Do not reset `available` when `version()` raises, preserving
the result computed by `_usable` and preventing contradictory diagnostics.
In `@src/bmad_loop/policy.py`:
- Around line 904-980: Update write_mux_backend to preserve any inline comment
and spacing trailing the existing backend value when replacing or clearing the
key. Extract the suffix beginning at the first # after the assignment/value,
retain it alongside the original line ending, and append it to new_line; add or
update tests for backend lines such as `backend = "tmux" # pinned per teammate
X` to verify the comment survives.
---
Nitpick comments:
In `@src/bmad_loop/adapters/multiplexer.py`:
- Around line 294-352: Update the platform-default selection in _select so the
backend is considered only when its registered matches predicate returns true
for sys.platform, in addition to passing _usable. Locate the
default_factory/_instance block and retain the existing first-match and fallback
behavior for candidates that do not match the platform.
🪄 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: 05671e20-324a-41dd-8bf0-f6556992b2d7
📒 Files selected for processing (18)
CHANGELOG.mdREADME.mddocs/FEATURES.mddocs/ROADMAP.mddocs/adapter-authoring-guide.mddocs/porting-to-a-new-os.mddocs/setup-guide.mddocs/tui-guide.mdsrc/bmad_loop/adapters/multiplexer.pysrc/bmad_loop/cli.pysrc/bmad_loop/data/settings/core.tomlsrc/bmad_loop/install.pysrc/bmad_loop/policy.pytests/test_backend_registry.pytests/test_cli.pytests/test_install.pytests/test_policy.pytests/test_settings_schema.py
- detect_multiplexers: isolate the version() probe from the availability probe so a cosmetic version() failure can no longer overwrite a correctly-computed available=True (contradictory selected/available row) - write_mux_backend: preserve a hand-added trailing comment on the 'backend =' anchor line itself when replacing or clearing the value (names never contain '#', so '#' after '=' is unambiguously a comment) - _select platform-default step: enforce the backend's own matches() predicate like every other step, via a first-wins-preserving lookup (CodeRabbit's proposed loop would have scanned past a non-matching first registration to a later duplicate, breaking first-wins)
Collaborator
Author
|
Re the platform-default |
Collaborator
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #87 (dracic). Selection was first-
matches(sys.platform)-wins in registration order, so two same-platform backends — psmux and tmux-windows (#85), bothmatches == "win32", both driving a binary literally namedtmux— would collide by import order. Selection is now availability-aware, per-platform-defaulted, and dev-choosable.Selection precedence (
adapters/multiplexer.py::_select)BMAD_LOOP_MUX_BACKENDenv var (unchanged; message byte-compatible)[mux] backendin policy.toml, installed once per CLI invocation bymain()viaconfigure_multiplexer()— the one chokepoint that also covers probe/diagnose/attach/stop, which never load policypsmux, elsewhere:tmux) when registered andavailable()available()(registration order breaks ties)TmuxMultiplexer— a tmux-less POSIX host still selects tmux andvalidatereports it unavailableForced names (1–2) bypass
matches()/available()(explicit = trusted) and fail loudly when unregistered — the policy path names the policy file to edit.bmad-loop muxdetect_multiplexers()(never raises; feeds thevalidatepreflight too, which lists backends when >1 is registered and notes env/policy provenance)mux set <name>: persists the choice through a comment-preserving stdlib writer (policy.write_mux_backend, anchored on the template's commented key line — no tomlkit/tomli-w dependency in core);--clearreverts to auto,--forceallows a name that only registers on the target machine; unavailable-but-registered persists with a warningpolicy.toml is now gitignored by
initPolicy is per-machine-per-repo (it carries the
[mux] backendchoice; the TUI settings editor rewrites it). A.gitignoreentry does not untrack an already-committed file, soinitbest-effort detects a tracked policy.toml and prints the one-timegit rm --cachedhint; CHANGELOG carries the migration note. (bmad-loop's own worktree-clean preflight already exempted policy.toml via pathspec — this additionally stops inner dev sessions and plaingit statusfrom reading a policy edit as dirt.)Handoff to the Windows backend PRs
The
available()discriminators can't be implemented here (both backends are out-of-tree), so the porting guide gains an "Availability discriminators (same-platform backends)" contract: psmux →which("psmux") and which("tmux") and which("pwsh"); tmux-windows →which("tmux") and not which("psmux"); probes must be cheap and side-effect-free, factories plain constructors. ROADMAP reframes psmux/tmux-windows as sibling tmux-family backends behind selection. #85 and the psmux re-cut should rebase onto this and implement the contract; any test pinning "first match wins on win32" needs re-cutting against the new precedence.Deliberately unchanged
probe.py,diagnostics.py,runs.py,tui/launch.py,tui/data.py,_make_adapters,tmux_base.py— all keep callingget_multiplexer(); the chokepoint changes what it returns.test_match_based_selection_wins_by_orderwas rewritten (registration order now breaks ties only among available non-default backends — and the old form was nondeterministic across hosts with/without tmux).Verification
trunk checkcleandetect_multiplexersguards, writer round-trip/CRLF/byte-identical-diff,muxCLI exit codes, end-to-end chokepoint throughcli.maininit(gitignore gains policy.toml) →mux(platform default) →mux set tmux(anchor line replaced in place) →mux set nope(exit 1, known list) →mux set nope --force→ next invocation fails loud naming policy.toml →mux set --clear→ env override showsforced by BMAD_LOOP_MUX_BACKENDCloses #87.
Summary by CodeRabbit
bmad-loop muxandbmad-loop mux set/--clearto list and persist a terminal-multiplexer backend choice.[mux].bmad-loop initnow gitignores.bmad-loop/policy.toml, with guidance when it was previously tracked.