feat(mux): seam-canonical window targets#141
Conversation
|
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 (9)
WalkthroughThe multiplexer seam now owns canonical ChangesCanonical target handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 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 |
Core hand-assembled tmux-shaped =session[:window] strings in 10 places, forcing every non-tmux backend to reverse-engineer tmux target syntax. The grammar is now owned by the seam: a concrete, overridable TerminalMultiplexer.target() encoder (default emits the same grammar, byte-identical), a module-level parse_target() decoder for native-id backends, and core formats every target via runs.session_target / launch.ctl_target. herdr's _parse_target delegates its grammar split; tmux backends keep passing tokens straight through. Zero existing test assertions changed.
5c197e7 to
a3998f8
Compare
|
@CodeRabbit review |
✅ Action performedReview finished.
|
|
@CodeRabbit review |
✅ Action performedReview finished.
|
What
Makes the window-target contract backend-neutral. Core (
runs.py,tui/launch.py,tui/app.py) used to hand-assemble tmux-shaped=session[:window]exact-match strings in 10 places and push them through the seam — so every non-tmux backend had to reverse-engineer tmux target syntax (herdr's_parse_targetdid; the psmux re-land #58 would have next). The grammar is now owned by the seam:TerminalMultiplexer.target(session, window=None)— new concrete encoder, default emits the now-seam-canonical=session[:window]token. Backends MAY override to emit native ids; the docstring pins the constraint (the token must stay a stable by-name reference — core formats targets ahead of use, e.g. a parked window's return target, so eager resolution goes stale).multiplexer.parse_target()— the matching module-level decoder:(session, window|None)for a canonical token,Nonefor a backend-native id. One tested helper instead of per-backend re-derivation.target()via two thin helpers that preserve layering:runs.session_target(run_id)andlaunch.ctl_target(window=None)(tui/app.pykeeps importing onlyruns/launch, never the mux)._parse_targetdelegates its grammar split toparse_target(); its workspace/tab/pane resolution (and strict/tolerant semantics) is untouched. herdr deliberately does not overridetarget()— pinned by a test.BaseTmuxBackendkeeps passing targets straight through (now noted in its class docstring).Zero behavior change, by construction
The default
target()emits byte-identical strings to the old f-strings, so no existing test assertion was edited — the exact-argv pins intest_tui_launch.py,test_runs.py(attach_argv with and withoutTMUX),test_cli.py,test_tui_app.py, and the herdr grammar-acceptance tests intest_herdr_backend.pyall pass unchanged and double as the compliance proof. New tests intest_multiplexer.pycover the encoder grammar, theparse_targetround-trip/edges/native-passthrough, and herdr's inherit-don't-override choice.Out-of-tree adapters are unaffected: all string-typed method signatures and semantics are unchanged,
target()is concrete and inherited. Adapters that already decode the grammar should migrate toparse_target()— it is now documented contract, not reverse-engineered tmux syntax.Verification
TMUXin the env and withenv -u TMUX(this dev box runs the agent inside tmux).grep -rn 'f"={' src/bmad_loop→ only backend files (tmux_base.py) and the seam default itself.trunk checkclean.Follow-up: a small separate PR renames the seam-backed-but-tmux-named helpers (
launch.tmux_available,app._tmux_missing,runs.tmux_sessions) so adapter authors aren't misled; kept out of this diff so the "zero assertions changed" property stays reviewable.Refs: #136, #137 (stacked base), #58, #140.
Summary by CodeRabbit
New Features
Documentation
Tests