fix(adapters): session-qualify psmux TUI-side window ids

[dracic]

Closes #291

What

#254/#290 covered the engine seam but left the launcher's surfaces bare — and the TUI usually runs outside any psmux pane, where a bare @N replayed as a -t target resolves through psmux's most-recent-session fallback. The ctl-window prune could kill-window another server's identically-numbered window with rc 0, and the detached-resolve select/attach could land on the wrong server.

• new_parked_window, the window_id columns of list_windows, and current_window_id now emit session:@N (via the existing _qualified_window_id, sharing its degrade rules — including a new guard so an empty session name degrades on the compose side exactly like the parse side).
• current_window_id resolves session and id from one #{session_name}:#{window_id} expansion — a split probe can resolve the id while the session fails, which would break the prune's self-exclusion and kill the window it runs in.
• select_window resolves a qualified id to session:<index> just-in-time: psmux validates a scoped target's window part CLI-side against index/name only (never window_id), while the server resolves ids fine (FocusWindowById). On a resolve miss/failure it still sends the original target (best-effort contract) but now warns on stderr — psmux's can't find window exit 1 lands in a discarded pipe, so the miss was previously untraceable.
• tmux untouched: its ids are server-global (divergence pinned by test).

Every psmux behavior relied on here was read from the psmux source at tag v3.3.7, the tag matching the installed build.

Follow-ups (split out, not fixed here)

• psmux: per-window user options are session-scoped - the ctl-window project guard cannot discriminate and the parked-return mechanism is inert #310 — per-window user options are session-scoped on psmux, so the PROJECT_OPTION prune guard cannot discriminate between projects and the RETURN_OPTION parked-return mechanism is inert. Split out of psmux: TUI-side window ids are still bare — parked mint, list_windows fields, and prune kill targets replay @N cross-server #291 by scope review: this PR fixes id routing only; that needs a different state channel. The window-option verbs are deliberately untouched here — psmux drops the window slot of a -w target either way, so the qualified ids this PR introduces neither help nor harm those surfaces.
• A garbled current_window_id probe degrades to None, which disables the prune's self-exclusion; pre-existing on both backends and out of scope here — will be filed separately.

Verification

• pytest tests/test_psmux_backend.py tests/test_tui_launch.py tests/test_multiplexer.py tests/test_portability_guard.py -q → 134 passed (16 new tests, incl. ablation-checked degrade/passthrough gates)
• trunk check clean, pyright@1.1.411 clean on the touched module

Summary by CodeRabbit

• Bug Fixes
  • Improved psmux window targeting by consistently scoping TUI window IDs to their sessions.
  • Prevented selection/cleanup from targeting identically numbered windows belonging to different sessions.
  • Added safer handling when session-qualified IDs can’t be represented or resolved.
• Documentation
  • Clarified adapter and TUI expectations for backend-specific window ID formatting and matching behavior.
• Tests
  • Added coverage for session-qualified ID behavior across parked-window creation, listing, probing, and selection, plus tmux compatibility.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 405bac5f-1da9-4ca7-b4e7-dbef86c2918e

📥 Commits

Reviewing files that changed from the base of the PR and between 56b37b7 and d323fd6.

📒 Files selected for processing (6)

• CHANGELOG.md
• docs/adapter-authoring-guide.md
• src/bmad_loop/adapters/multiplexer.py
• src/bmad_loop/adapters/psmux_backend.py
• src/bmad_loop/tui/launch.py
• tests/test_psmux_backend.py

🚧 Files skipped from review as they are similar to previous changes (3)

• src/bmad_loop/tui/launch.py
• src/bmad_loop/adapters/multiplexer.py
• src/bmad_loop/adapters/psmux_backend.py

---

Walkthrough

psmux window identifiers are now session-qualified across parked-window creation, listing, current-window lookup, and selection. Qualified targets resolve to session-local indexes, while invalid or unqualifiable identifiers degrade safely. Contracts, documentation, changelog text, and tests were updated.

Changes

psmux window targeting

Layer / File(s)	Summary
Identifier contracts and documentation docs/adapter-authoring-guide.md, src/bmad_loop/adapters/multiplexer.py, src/bmad_loop/adapters/psmux_backend.py, src/bmad_loop/tui/launch.py	Backend and TUI documentation defines consistent session:@N`` forms for psmux identifiers and matching current/listed window IDs.
Qualified identifier production src/bmad_loop/adapters/psmux_backend.py	Parked, listed, and current psmux window IDs are qualified, with degradation for empty or colon-containing sessions and rejection of malformed probes.
Session-scoped selection and validation src/bmad_loop/adapters/psmux_backend.py, tests/test_psmux_backend.py, CHANGELOG.md	Qualified targets resolve through session-scoped window listings before selection; unresolved targets warn and fall back to the original target, with psmux and tmux coverage.

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

Sequence Diagram(s)

sequenceDiagram
  participant TUI
  participant PsmuxMultiplexer
  participant PsmuxCLI
  TUI->>PsmuxMultiplexer: submit session:`@N` target
  PsmuxMultiplexer->>PsmuxCLI: list session windows
  PsmuxCLI-->>PsmuxMultiplexer: window_id and window_index
  PsmuxMultiplexer->>PsmuxCLI: select session:window_index
  PsmuxCLI-->>TUI: selected window or warning fallback

Loading

Possibly related issues

• psmux: TUI-side window ids are still bare — parked mint, list_windows fields, and prune kill targets replay @N cross-server #291 — Directly covers psmux TUI-side session-qualified window IDs.
• adapter seam: document the qualified-id obligation once, not per backend #311 — Covers qualified-ID replay, translation, and cross-API consistency.

Possibly related PRs

• bmad-code-org/bmad-loop#290 — Extends the earlier psmux session-qualified ID contract across additional entry points.

Poem

I hop through sessions, precise and bright,
session:@N`` keeps windows right.
Lost targets warn, then safely stay—
Tmux remains bare along the way.
Psmux paths now know where to go.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 4.17% 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 summarizes the main change: session-qualifying psmux TUI-side window IDs.
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

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Summary

This PR completes the psmux session-qualification fix started in #254 by extending it to the TUI-launcher surfaces (new_parked_window, list_windows window-id columns, and current_window_id). Without the fix, a bare @N id replayed from outside any pane resolves through psmux's most-recent-session fallback, which could cause kill-window to destroy a window in an unrelated server.

• new_parked_window and list_windows window-id columns now go through _qualified_window_id, producing session:@N with the same degrade rules (empty or colon-bearing session names fall back to bare ids) so current_window_id and list_windows stay in sync for the prune's self-exclusion comparison.
• current_window_id is re-implemented as a single #{session_name}:#{window_id} probe, eliminating a split-probe gap where the id could resolve while the session fails, breaking the prune's own-window guard.
• select_window translates qualified ids back to session:<index> just-in-time, working around psmux's CLI-side check that validates window targets against index/name only (never window_id); misses warn on stderr and fall back to the unresolved target.

Confidence Score: 5/5

Safe to merge — the changes are narrowly scoped to the psmux backend's window-id composition surfaces, leave tmux untouched, and are validated by 16 new ablation-checked tests on top of the existing 118.

Every new code path — single-probe current_window_id, qualified new_parked_window, selective list_windows column rewriting, and the select_window index-translation with its miss/failure degradation — has a corresponding test that asserts the exact output and spawn sequence. The degrade rules (empty session, colon-bearing session, falsy id) are tested on both the compose side and the parse side, and a cross-check test confirms the two stay in sync for the prune comparison. The _window_index lookup intentionally calls the parent's list_windows to get bare ids that match the regex-extracted bare id — a subtle but correct choice. No pre-existing interface changed; the tmux backend is explicitly confirmed bare by a new divergence test.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename	Overview
src/bmad_loop/adapters/psmux_backend.py	Core of the fix: adds new_parked_window, list_windows, current_window_id, select_window, and _window_index overrides; all degrade cases and the single-probe contract are correctly implemented
tests/test_psmux_backend.py	16 new tests cover all new code paths: session-qualified ids, colon-session degrade, empty-session degrade, falsy-id passthrough, single-probe enforcement, list_windows column selectivity, select_window index resolution, resolve miss/failure degradation, =prefix handling, name-token passthrough, and tmux divergence
src/bmad_loop/tui/launch.py	Documentation-only changes: docstrings for select_ctl_window_id, set_return_pane, and start_detached updated to reflect backend-specific id forms; no logic changes
src/bmad_loop/adapters/multiplexer.py	Abstract contract updates: list_windows and current_window_id docstrings now document the symmetry requirement; new_parked_window docstring updated to reflect psmux now also qualifies it
docs/adapter-authoring-guide.md	Adapter guide updated to reflect that new_parked_window, list_windows window_id columns, and current_window_id are now all session-qualified on psmux
CHANGELOG.md	New changelog entry accurately describes the bug (cross-server kill-window), the fix (session:@n qualification), and the select_window index-translation approach

Sequence Diagram

sequenceDiagram
    participant TUI as tui/launch.py
    participant Mux as PsmuxMultiplexer
    participant Base as BaseTmuxBackend
    participant Psmux as psmux CLI

    Note over TUI,Psmux: start_detached / new_parked_window
    TUI->>Mux: new_parked_window(session, name, cwd, argv, opt)
    Mux->>Base: super().new_parked_window(...)
    Base->>Psmux: "new-window -P -F #{window_id}"
    Psmux-->>Base: "@2"
    Base-->>Mux: "@2"
    Mux->>Mux: "_qualified_window_id(session, @2)"
    Note right of Mux: Degrades to bare if session empty or has colon
    Mux-->>TUI: "session:@2"

    Note over TUI,Psmux: prune / _ctl_window_candidates
    TUI->>Mux: current_window_id()
    Mux->>Psmux: "display-message -p #{session_name}:#{window_id}"
    Note right of Mux: Single probe — avoids split-probe gap
    Psmux-->>Mux: "session:@2"
    Mux->>Mux: _QUALIFIED_ID.fullmatch
    Mux-->>TUI: "session:@2"

    TUI->>Mux: list_windows(session, [window_id, window_name])
    Mux->>Base: super().list_windows(...)
    Base->>Psmux: "list-windows -F #{window_id} #{window_name}"
    Psmux-->>Base: "@1 shell, @2 run-x"
    Base-->>Mux: "[(@1,shell), (@2,run-x)]"
    Mux->>Mux: _qualified_window_id for each window_id column
    Mux-->>TUI: "[(session:@1,shell), (session:@2,run-x)]"
    Note right of TUI: current==session:@2 found in rows, prune skips own window

    Note over TUI,Psmux: select_ctl_window_id
    TUI->>Mux: "select_window(session:@2)"
    Mux->>Mux: "_QUALIFIED_ID.fullmatch → session, id=@2"
    Mux->>Mux: "_window_index(session, @2)"
    Mux->>Base: super().list_windows(session, [window_id, window_index])
    Base->>Psmux: "list-windows -t =session -F #{window_id} #{window_index}"
    Psmux-->>Base: "@2 1"
    Base-->>Mux: "[(@2,1)]"
    Mux-->>Mux: "index=1"
    Mux->>Psmux: select-window -t session:1
    Note right of Mux: Index form passes psmux CLI-side check

Loading

_{Reviews (2): Last reviewed commit: "docs(adapters): mark the select-window r..." | Re-trigger Greptile}

[pbean]

Reviewed and validated. Approach is right, scope is clean, and the tmux path is provably untouched. Rebased onto main (CHANGELOG only) and pushed one review commit; details below.

Independently re-verified against the psmux source at v3.3.7

I didn't take the source claims on faith — the select_window override is the one piece of new machinery here, so its premise had to hold:

• cli_window_exists (src/main.rs:93-118) compares the target's window part only against #{window_index} and #{window_name}. Never #{window_id}. The premise holds.
• It has exactly one call site — the select-window handler (src/main.rs:1752). That is the load-bearing fact, and it settles the design two ways at once: select_window really is the only verb needing the index translation, and kill-window has no CLI-side validation at all, so the prune — the destructive path this PR exists to fix — takes the qualified id straight to the server. No prune regression.
• parse_target strips exactly one = (src/cli.rs:527), so the ==session failure the =-strip comment describes is real, not theoretical.
• Session→server routing is an exact-string port-file lookup (extract_session_from_target, src/cli.rs:634) — so recomposing session:<index> without the = introduces no prefix-match hazard. Worth stating explicitly since dropping an exact-match marker normally would.

I also treated the #310 deferral as a regression risk rather than a note, because current_window_id now feeds qualified ids to the option verbs via return_attached_client:

• set-option server-side (src/server/connection.rs:2088) filters the -t value out of its positional args entirely — target wholly ignored.
• show-options does read -t, but only through parse_target(...).window, which yields Some(2) for both @2 and session:@2 (window_is_id is discarded, src/cli.rs:602).

So both option verbs behave identically before and after. Qualifying is strictly better — right server, same pre-existing scope bug. The split to #310 is correct, and its "do this after #291" ordering is right.

Linux/macOS: zero impact

multiplexer.py's diff is docstring-only, tui/launch.py's three hunks are docstrings, and TmuxMultiplexer/PsmuxMultiplexer are siblings under BaseTmuxBackend with every override on the psmux leaf.

test_tmux_backend_keeps_bare_tui_ids is a placement assertion, so I ran the inverse ablation it needs — re-implementing the qualification in the shared base — and confirmed it goes red. It isn't passing merely because nothing qualifies anywhere.

Scope

Exactly #291's three named surfaces plus the current_window_id symmetry hazard #291's own implementation notes flagged. The double-qualification hazard those notes warned about does not materialize: select_ctl_window, attach_plan, kill_ctl_window, and action_attach all compose ctl_target() from window names, never from an id.

The one reach past the TUI surfaces is the not session guard in _qualified_window_id, which also covers the engine seam. That's justified beyond the symmetry argument given for it: composing ":@N" yields a target parse_target resolves to a session literally named default, so the guard prevents an actively wrong target, not just an asymmetric one.

Verification

• Full suite 3111 passed, 30 skipped post-rebase. The 30 is main's own baseline (main alone: 3091/30) — this PR adds 20 passing tests and zero new skips.
• pyright@1.1.411 clean; trunk check clean.
• Ablation: 9/9 held. I ablated each new gate individually — empty-session degrade, list_windows column filter, select_window resolve gate, = strip, send-on-resolve-miss, the one-probe current_window_id, the malformed-id check, _window_index session scoping — and confirmed each guarding test turns red, plus the inverse placement ablation above. The negative assertions here are trustworthy.

One finding (addressed in d323fd6, comment-only)

The resolve-miss warning is inert at its only caller, and the comment claimed otherwise.

select_window prints the miss to sys.stderr, justified as "or that is untraceable". But the only production path reaching the qualified-id branch is app.py:494 _launch_resolve → select_ctl_window_id — the sole caller of that function (select_ctl_window passes a name token and returns before the warning). That runs inside the Textual app, and tui/app.py:1114-1117 documents that Textual captures sys.stderr for the app's whole run — run_tui pre-trips the forced-backend warning before App.run() for exactly this reason. So the miss is untraceable either way, and the cited pipe_pane precedent (tmux_base.py:322) is an engine-path warning where stderr is a real terminal — not comparable.

Not a correctness bug: the degrade is safe (futile target sent, base swallows the rc 1, the window simply isn't focused). The defect was a diagnostic that cannot fire carrying a rationale that would mislead the next reader.

Fixed by correcting the comment rather than the plumbing — the print is worth keeping for the CLI-side callers tui/launch.py stays textual-free to allow, and making the miss TUI-visible would mean giving a best-effort -> None focus seam a return value. Wrong altitude for a rare cosmetic miss.

Rebase note

CHANGELOG.md was the only conflict (#302's entry from #308 landed in the same ### Fixed slot); resolved keep-both, #291 above #302 per the newest-first order this PR already used. +363/-28 unchanged across the rebase.

Minor: the description says 16 new tests; it's 20.