Skip to content

TUI: ctl_window_id admits an untagged control window on run-id collision, so two projects can attach to or kill each other's #531

Description

@pbean

Residual of #482, deferred by PR #518's "Known limitation, not fixed here" section. Filed rather than folded in: the remedy is a seam change, and the exposure #518 leaves is strictly narrower than what it replaced.

What #518 shipped, so the residual is stated against the right baseline

ctl_window_id (src/bmad_loop/tui/launch.py:339) gained a project argument and now filters the shared bmad-loop-ctl listing by the per-window PROJECT_OPTION tag. So the plain "name scan over a shared session" framing in PR #518's body is broader than what actually landed — that text was written before the tag scoping was added mid-review, and the tagged path is project-scoped.

What remains is the untagged fallback, and the no-record path that cannot break its tie.

The mechanism

mine  = runs.project_tag(project)
local = runs.is_run(runs.run_dir_for(project, run_id))      # launch.py:383
...
if tag == mine:
    tagged.append(win_id)
elif not tag and local:                                      # launch.py:413
    untagged.append(win_id)
matches = tagged or untagged                                 # launch.py:417
recorded = _read_ctl_window(project, run_id)
return recorded if recorded in matches else matches[0]       # launch.py:424

An untagged window is admitted on local alone — ownership proved by run-id collision on the filesystem, not by identity. If project A's control window for <id> is untagged and project B also holds a run dir for that same <id>, the window is a legal candidate for B. With nothing tagged, matches is that untagged list in listing order, and with no usable record B answers matches[0] — which can be A's window.

Consequences follow the same three verbs as #482, but across a project boundary: a attaches to the neighbour's window, set_return_pane stamps the return target on it, and x kills a live orchestrator in another project.

This is the same weak-ownership shape as #419, which reaches it through prunable_sessions; here it feeds targeting rather than pruning.

Why a window can be untagged — and which causes are already filed

  1. psmux refuses the tag value. PsmuxMultiplexer._transportable() rejects e.g. a non-breaking space or a spaced UNC path; set_window_option warns and returns without raising. Already tracked in Untagged sessions are weak ownership: they leak once their run dir is gone, and can be pruned by the wrong project on a run-id collision (the fallback behind #320) #419, including ctl_window_id as a second, targeting consumer (added there during the fix(tui): resolve a run's control window by identity, not listing order #518 gate).
  2. The tmux write silently fails. BaseTmuxBackend.set_window_option (src/bmad_loop/adapters/tmux_base.py:388-392) runs with check=False and swallows SubprocessError/OSError, so a failed tag write is indistinguishable from a successful one. Not covered by Untagged sessions are weak ownership: they leak once their run dir is gone, and can be pruned by the wrong project on a run-id collision (the fallback behind #320) #419, which is scoped to psmux's transportability predicate.
  3. Pre-upgrade windows minted before the per-window tag existed carry none, and _ctl_window_candidates documents them as exactly that case.

Causes 2 and 3 are why this is not simply a subset of #419: they are reachable on POSIX/tmux with an ordinary project path.

Bounds

  • Accidental collision is remote: runs.new_run_id() (src/bmad_loop/runs.py:61-62) is time.strftime("%Y%m%d-%H%M%S") + secrets.token_hex(2), so two projects must mint in the same second and draw the same 4 hex digits.
  • --run-id is caller-supplied and validated for shape only, so a script reusing a fixed id across two projects reproduces it deterministically. The TUI also pre-assigns ids.
  • The #518 record narrows the exposure to the no-record path: a recorded id must pass the listing/name/project re-prove, so where a record exists and holds, the answer is this project's own window.

Not a regression

Before #518, ctl_window_id took no project argument at all — no tag, no run-dir gate, first name match from any project. Every path here is strictly narrower than that.

Direction

Same remedy as #419's direction (1): make the project identity transportable by construction so the untagged bucket stops being reachable for ordinary paths. #518 took the first step — runs.project_tag now percent-encodes behind a %enc% prefix when a value cannot ride the listing (a str.splitlines() separator, or a surrogateescaped byte), conditionally, so transportable paths stay byte-identical and nothing already stored is stranded. What is left is that project_tag asks the listing's questions, not PsmuxMultiplexer._transportable's, and that an untagged window is admitted on run-dir collision at all. Closing the latter needs a positive ownership proof for the untagged bucket rather than a filesystem coincidence.

Cause 2 above is separable and much smaller: a tag write that fails is currently indistinguishable from one that succeeded, so the lookup cannot tell "untagged, pre-upgrade" from "tagging broke just now".

One note for completeness, not a defect

kill_ctl_window (launch.py:630) and prune_ctl_windows (launch.py:688) kill the window but leave the run dir's ctl-window record file in place. That is by design and bounded by the re-prove: a record whose window is gone from the listing, or whose row no longer carries this run's name, is ignored rather than replayed. Listed here so the lifecycle is written down somewhere, not because it needs fixing.

Refs: #482, #419, #526, PR #518.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:adaptersCoding-CLI adapters and profilesarea:tuiTextual TUIbugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions