diff --git a/CHANGELOG.md b/CHANGELOG.md index aa01e055..554aad83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,17 @@ breaking changes may land in a minor release. ### Added +- Add `[mux] honor_ambient_psmux_data_dir` (default off): on psmux, use an + absolute `PSMUX_DATA_DIR` your profile exports into every shell as the + session registry instead of the derived per-project root; `bmad-loop mux` + says which source won (#729). Such a root can be shared by several projects, + so there a same-named session is killed, attached, read as live or adopted + by a launch only when its project tag proves it this project's; a refusal is + warned about, and a launch that would adopt one fails with the reason. The + same check applies without the opt-in when no state root can be derived and + an ambient `PSMUX_DATA_DIR` stays in force; a teardown kill it refuses is + journalled (`session-kill-refused`). A running TUI refuses launches after + the switch is flipped either way, until restarted. - Add `[environment] probes` (+ `probe_timeout_s`): operator health checks run before `[verify]` commands, before each dev and review session launch, and before a failed attempt is charged; a failing, hanging or unrunnable probe @@ -36,6 +47,13 @@ breaking changes may land in a minor release. ### Fixed +- Kill a resumed run's stale psmux session in the registry it predates (the + displaced or pre-#537 default root, or the derived one after opting in to + your own), tag-proven only; a same-named survivor there made the resumed + session's create fail on psmux's cross-registry name mutex. A registry that + cannot be listed, or a session left standing, is journalled and warned about. + A TUI-launched resume sweeps the TUI's displaced root too (forwarded to the + child), except a share-root shape older PowerShell would corrupt. - Escalate an environment fault at the review-budget rescue gate instead of deferring the story as unconverged (DW-523). diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 51a6ae34..9b3d9ad8 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -769,7 +769,7 @@ verdict unverifiable rather than certifying a different launch configuration. - Single policy file written by `init`, stamped into the run at every engine start — `run`, `sweep`, `resume` — so it always describes the policy that process enforces (applies to new runs and resumes; editable live from the TUI). - Rewrites of it are confined and permission-honoring (#593, #597). Such a write walks the components below the project no-follow and lands through the directory handle that walk produced — `O_DIRECTORY|O_NOFOLLOW` opens with `dir_fd` on POSIX, `NtCreateFile` relative to the handle above with reparse points refused on Windows (`win32_at`) — so a symlink or junction planted at `.bmad-loop/` is refused rather than followed; refusing a link at the file alone never covered its parent, and `mkdir(parents=True, exist_ok=True)` accepts a symlinked directory. A host with neither arm degrades to a documented check-then-write; `init`'s one-time seeding of a missing policy predates any session and stays a plain write. A `policy.toml` an operator marked read-only is refused with a `PermissionError` instead of being replaced and left still reading `0444`. The confined walk also covers story specs inside the checkout, park records, the decisions store, and the sweep's triage cache and bundle intent document (DW-269); the read-only refusal reaches further — story specs, `sprint-status.yaml`, park records, the decisions store, hook `settings.json` — but `sprint-status.yaml` deliberately keeps its symlink-following writer (an operator may keep the board behind a link) and the hook-settings and worktree-provisioning writers keep their own pre-existing link checks rather than the descriptor walk. The read-only refusal deliberately skips machine-minted state (run archives, stop requests, the config-digest stamp) — those are channels, not operator signals. -- Sections — all 16: `[gates]`, `[limits]`, `[verify]` (+ `env_fault_rc`), `[environment]` (operator health probes + `probe_timeout_s`), `[notify]`, `[review]`, `[stories]` (which planning pipeline drives the loop: sprint-status or a typed `stories.yaml`), `[dev]` (see below), `[adapter]` (+ per-stage `[adapter.dev|review|triage]`), `[sweep]`, `[scm]` (worktree isolation + merge-back), `[cleanup]` (run-dir retention + disk reclamation), `[plugins]` (trust allowlist + per-plugin `[plugins.]` config — e.g. the opt-in game-engine layer via `[plugins.unity]`, off by default), `[tui]` (`low_frame_rate` for slow/SSH links; persisted dashboard pane sizes), `[operator]` (whether a dev session may park a story at `awaiting-operator`, and whether a review pass may demote a `done` story to one — `on_review_demotion`), `[mux]` (machine-scoped multiplexer backend choice). +- Sections — all 16: `[gates]`, `[limits]`, `[verify]` (+ `env_fault_rc`), `[environment]` (operator health probes + `probe_timeout_s`), `[notify]`, `[review]`, `[stories]` (which planning pipeline drives the loop: sprint-status or a typed `stories.yaml`), `[dev]` (see below), `[adapter]` (+ per-stage `[adapter.dev|review|triage]`), `[sweep]`, `[scm]` (worktree isolation + merge-back), `[cleanup]` (run-dir retention + disk reclamation), `[plugins]` (trust allowlist + per-plugin `[plugins.]` config — e.g. the opt-in game-engine layer via `[plugins.unity]`, off by default), `[tui]` (`low_frame_rate` for slow/SSH links; persisted dashboard pane sizes), `[operator]` (whether a dev session may park a story at `awaiting-operator`, and whether a review pass may demote a `done` story to one — `on_review_demotion`), `[mux]` (machine-scoped multiplexer backend choice, plus `honor_ambient_psmux_data_dir`: on psmux, use an absolute `PSMUX_DATA_DIR` your profile exports into every shell as the session registry instead of the derived per-project root — a yes/no switch, never a path; off by default; see [multiplexer-backends.md](multiplexer-backends.md)). - `[dev] skill` names the inner dev skill the orchestrator drives. `"bmad-dev-auto"` — the generic upstream dev primitive — is the only accepted value; the field is retained as the seam for a future alternative dev skill, and any other value is rejected at load. It is **not** the name sessions are dispatched with: upstream renamed the primitive to `bmad-build-auto`, so the invoked name is resolved from what is actually installed and a project on either era works with this field untouched. It has no entry in the core settings schema, so it is edited in the file rather than from the TUI settings editor. - Tunable limits: `max_review_cycles`, `max_dev_attempts`, `artifact_file_max_mb`, `artifact_payload_max_mb` (binary MiB, exactly 1,048,576 raw bytes each), `max_followup_reviews`, `session_timeout_min`, `git_timeout_s`, `teardown_grace_s` (one shared budget bounding the verified window kill _and_ the follow-on reap of any straggler descendant the session detached — e.g. a `setsid` background writer — combined; whatever remains after the window dies is what the straggler reap gets, before the worktree is merged and removed), `stop_without_result_nudges`, `dev_stall_grace_s`, `dev_stall_nudges`, `dev_stall_nudges_cap`, `workflow_stall_nudges_cap`, `max_tokens_per_story`. @@ -810,7 +810,7 @@ verdict unverifiable rather than certifying a different launch configuration. - `bmad-loop init` — install skills, hooks, policy, gitignore. - `bmad-loop validate` — preflight all prerequisites. `--render-probe` also executes the dev skill's render command in a throwaway copy (`skills.dev-render-probe`; see above). `--json` instead emits a stable machine-readable document (schema-versioned; the `ok` verdict, the queue `mode`/`spec_folder`, per-severity `counts`, and every check as a flat emission-ordered finding with a stable `check` id, `severity`, human `message` and structured `detail`) per the [contract below](#machine-readable-output---json); a failing check still emits the whole document, at exit 1 — the nonzero code is the verdict, not a failure to produce one. -- `bmad-loop mux` — list registered terminal-multiplexer backends (platform · availability · version · which is selected and why; a backend whose binary is present but crashed the version probe gets a `warning:` on stderr carrying the probe's own failure, since the `-` in the VERSION column cannot tell that apart from a binary that reports no version); on a backend with a registry namespace it also prints the **registry root** this project's sessions live in plus the export that reaches them from a bare client — on psmux that is `//_mux`, so a plain `psmux ls` shows none of them and says so rather than erroring ([#537](https://github.com/bmad-code-org/bmad-loop/issues/537)); `mux set ` persists a machine-scoped choice into policy.toml (`--clear` reverts to auto, `--force` allows a name only registered on the target machine). Bundled backend: `tmux`; external backends (e.g. the herdr adapter) register via the `bmad_loop.mux_backends` entry-point group — see [Terminal multiplexer backends](multiplexer-backends.md). +- `bmad-loop mux` — list registered terminal-multiplexer backends (platform · availability · version · which is selected and why; a backend whose binary is present but crashed the version probe gets a `warning:` on stderr carrying the probe's own failure, since the `-` in the VERSION column cannot tell that apart from a binary that reports no version); on a backend with a registry namespace it also prints the **registry root** this project's sessions live in plus the export that reaches them from a bare client — on psmux that is `//_mux`, so a plain `psmux ls` shows none of them and says so rather than erroring ([#537](https://github.com/bmad-code-org/bmad-loop/issues/537)) — or, with `[mux] honor_ambient_psmux_data_dir = true` and an absolute `PSMUX_DATA_DIR` set, your own root, labelled as honoured ([#729](https://github.com/bmad-code-org/bmad-loop/issues/729)); `mux set ` persists a machine-scoped choice into policy.toml (`--clear` reverts to auto, `--force` allows a name only registered on the target machine). Bundled backend: `tmux`; external backends (e.g. the herdr adapter) register via the `bmad_loop.mux_backends` entry-point group — see [Terminal multiplexer backends](multiplexer-backends.md). - `bmad-loop adapters` — list registered coding-CLI adapter **kinds** (name · builtin/external · whether the family drives a multiplexer · which profiles select it), the CLI axis's counterpart to `mux`. Unlike `mux` there is no global choice to persist: a kind is selected per profile by its `adapter` field. A profile referencing an unregistered kind, and any out-of-tree adapter/profile package that failed to load, get a `warning:` on stderr; `validate` reports the same as `adapter.kind` / `adapter.external` / `adapter.external-profile`. - `bmad-loop run` — drive the dev → review → verify → commit loop. - `bmad-loop sweep` — triage + execute open deferred-work entries. diff --git a/docs/multiplexer-backends.md b/docs/multiplexer-backends.md index 86260dd7..c49df09e 100644 --- a/docs/multiplexer-backends.md +++ b/docs/multiplexer-backends.md @@ -220,9 +220,9 @@ Two further consequences: but only while psmux's `exit-empty` is on; it is on by default, and `psmux show-options -g exit-empty` says which you have. With it off, an empty session stays. -Setting `PSMUX_DATA_DIR` yourself does **not** move bmad-loop's registry. bmad-loop derives the -root from the project and the state root and exports it over whatever it finds, saying so once on -stderr when it replaced something. Your value is left alone for your own psmux sessions — it is +By default, setting `PSMUX_DATA_DIR` yourself does **not** move bmad-loop's registry. bmad-loop +derives the root from the project and the state root and exports it over whatever it finds, saying +so once on stderr when it replaced something. Your value is left alone for your own psmux sessions — it is psmux's variable, not bmad-loop's, so bmad-loop overrides it rather than refusing to run. That is deliberate, and the reason is worth having: an honoured export would make the registry a @@ -240,8 +240,39 @@ $env:PSMUX_DATA_DIR = '' psmux ls ``` -One registry serving both bmad-loop and your own psmux is a reasonable thing to want and is not -available today; it needs a preference you state rather than one bmad-loop guesses at. +One registry serving both bmad-loop and your own psmux is a preference you state rather than one +bmad-loop guesses at ([#729](https://github.com/bmad-code-org/bmad-loop/issues/729)). If your +profile exports `PSMUX_DATA_DIR` into **every** shell, turn on the switch in the project's +`policy.toml`: + +```toml +[mux] +honor_ambient_psmux_data_dir = true +``` + +bmad-loop then uses your value as the registry, a pane child inherits and honours it, and a clean +process carrying your profile honours it too, so every process agrees. Not under `PSMUX_BARE_ENV`, +which bmad-loop does not support: a bare pane inherits no `PSMUX_DATA_DIR`, so a run started there +derives the registry instead. `bmad-loop mux` says +`your own $PSMUX_DATA_DIR, honoured` and prints the derived root it would otherwise use. Leave the +switch off for a value you typed into one shell: a process started without it would derive, and the +session would read as gone there. Rules, all fixed: + +- It is a yes/no switch, never a path. `policy.toml` is writable by the sessions bmad-loop drives, + so a policy-supplied root would let one aim cleanup's kills at a registry of its choosing. +- With no `PSMUX_DATA_DIR` set, or a relative or empty one, the switch changes nothing: the derived + root is used. +- A value shaped like a bmad-loop-derived root (`<16-hex project key>\_mux`) is never treated as + your pin. That is what a pane child inherits when the outer process derived, and it re-derives + like a clean process does. +- The TUI's control session gets its own name in your registry, so turning the switch on while an + old control session still runs in the derived root does not collide with it. +- A running TUI keeps watching the registry it started with. After you flip the switch either way, + it refuses to launch a run until you restart it (`bmad-loop tui`). A run started then would land + in a registry the TUI no longer watches. +- Sessions started in the derived root before you turned the switch on are still reached by + `bmad-loop cleanup`'s tag-scoped sweep. Your registry is shared with your own sessions, so cleanup + claims a session there only by its ownership tag. Two consequences of deriving, both benign: diff --git a/src/bmad_loop/adapters/generic.py b/src/bmad_loop/adapters/generic.py index ddc982cc..1a5a6eca 100644 --- a/src/bmad_loop/adapters/generic.py +++ b/src/bmad_loop/adapters/generic.py @@ -803,14 +803,60 @@ def __init__( # --------------------------------------------------------- multiplexer def _ensure_session(self, cwd: Path) -> None: - if not self.mux.has_session(self.session_name): + if self.mux.has_session(self.session_name): + # Reusing it is right for this run's own session (a resume), and + # wrong for a same-named one of another project's in a shared + # registry (#729): every window would open inside it, under its tag. + refusal = runs.foreign_session_refusal( + self.session_name, self.mux, self.run_dir.parents[2] + ) + if refusal is not None: + raise MultiplexerError( + f"refusing to launch into the existing session {self.session_name}: " + f"{refusal} — stop or rename that run, or turn off " + "[mux] honor_ambient_psmux_data_dir" + ) + else: self.mux.new_session(self.session_name, cwd, PANE_COLUMNS, PANE_LINES) # Tag the session with its project so a cleanup in another project # never prunes this run (run_dir = /.bmad-loop/runs/). project = self.run_dir.parents[2] - self.mux.set_session_option( - self.session_name, runs.PROJECT_OPTION, runs.project_tag(project) - ) + try: + self.mux.set_session_option( + self.session_name, runs.PROJECT_OPTION, runs.project_tag(project) + ) + except Exception as tag_fault: + # Left standing, the untagged session would block this run id + # for good in a shared registry (#729): the ownership gate reads + # it as foreign, and the kill and cleanup paths refuse it. It is + # the one this call just minted, so tear it down by that exact + # name — straight through the backend, since the gate would + # refuse an untagged session by construction — and re-raise. + # The backend kill is best-effort and silent by contract, so + # whether it landed is read back rather than assumed. + listing_faults: list[str] = [] + try: + self.mux.kill_session(self.session_name) + key = self.mux.session_name_key(self.session_name) + survived = any( + self.mux.session_name_key(n) == key + for n in self.mux.list_sessions_reporting(on_fault=listing_faults.append) + ) + except Exception as kill_fault: + listing_faults.append(str(kill_fault)) + survived = True + if survived or listing_faults: + why = ( + f"could not be confirmed gone ({listing_faults[0]})" + if listing_faults + else "is still there after tearing it down" + ) + raise MultiplexerError( + f"tagging the new session {self.session_name} failed ({tag_fault}), " + f"and the untagged session {why} — remove it by hand before " + "resuming this run" + ) from tag_fault + raise def interactive_argv(self, spec: SessionSpec) -> list[str]: extra = self.extra_args diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 99809cbb..bc8fd734 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -182,9 +182,10 @@ def _configure_mux(project: Path) -> None: keep-diagnostics-working rule as the policy read above. It also *overrides* an ambient ``PSMUX_DATA_DIR`` rather than honouring it — - the root is derived, always, so that two processes given one project cannot + the root is derived, so that two processes given one project cannot disagree about where its sessions live (the full argument is in that - function). Overriding an operator's variable silently is how someone loses an + function) — unless policy ``[mux] honor_ambient_psmux_data_dir`` says the + operator's value is a persistent pin (#729). Overriding an operator's variable silently is how someone loses an hour to `psmux ls` showing nothing, so it is said once, here, at the only point that runs ahead of every command. stderr, not stdout: the ``--json`` contract is one object on stdout and nothing else, and this is the @@ -193,10 +194,10 @@ def _configure_mux(project: Path) -> None: path = _policy_path(project) try: - name = policy_mod.load(path).mux.backend or None + mux_policy = policy_mod.load(path).mux except (policy_mod.PolicyError, OSError): - name = None - configure_multiplexer(name, origin=path) + mux_policy = policy_mod.MuxPolicy() + configure_multiplexer(mux_policy.backend or None, origin=path) # Automatic selection probes availability before returning its cached # instance. Give that probe the derived root first: psmux's version probe # reaches `_run`, which must reject an empty/relative ambient value, and a @@ -237,8 +238,12 @@ def _configure_mux(project: Path) -> None: os.environ[runs.PSMUX_DATA_DIR] = ambient if not namespaced: return - root = runs.export_psmux_registry_root(project) + root = runs.export_psmux_registry_root( + project, honor_ambient=mux_policy.honor_ambient_psmux_data_dir + ) if root is not None: + # An honoured value is the operator's own stated preference, so it gets + # no note; `bmad-loop mux` still says which source won. if ambient is not None and ambient != root: print( f"note: using bmad-loop's own psmux registry {root} — your " @@ -1270,17 +1275,30 @@ def _print_registry(project: Path) -> None: derived = str(runs.mux_registry_root(project)) except (runs.StateRootError, OSError, RuntimeError): derived = None - # bmad-loop always derives, so a mismatch is not an operator's honoured - # export — that is not a thing any more — but the one case the export - # degrades on: an underivable state root, where it leaves whatever it found - # rather than inventing a root. Saying "derived" there would be a lie about - # the one situation an operator most needs told. - origin = ( - "derived from the project" - if root == derived - else f"NOT bmad-loop's — ${runs.PSMUX_DATA_DIR} as found, " - "because no state root could be derived here" - ) + # A root other than the derived one is either the operator's value honoured + # on their opt-in — asked of the same pure rule the export used — or the one + # case the export degrades on: an underivable state root, where it leaves + # whatever it found rather than inventing a root. Saying "derived" there + # would be a lie about the one situation an operator most needs told. + try: + honor = policy_mod.load(_policy_path(project)).mux.honor_ambient_psmux_data_dir + except (policy_mod.PolicyError, OSError): + honor = False + if root == derived: + origin = "derived from the project" + elif derived is not None and ( + runs.resolve_psmux_registry_root(derived, root, honor_ambient=honor) == root + ): + origin = ( + f"your own ${runs.PSMUX_DATA_DIR}, honoured by " + "[mux] honor_ambient_psmux_data_dir — the derived root would be " + f"{derived}" + ) + else: + origin = ( + f"NOT bmad-loop's — ${runs.PSMUX_DATA_DIR} as found, " + "because no state root could be derived here" + ) print(f"registry: {root} ({origin})") # A single-quoted PowerShell literal, whose only escape is doubling the quote: # an unescaped `C:\Users\O'Brien\...` ends the string mid-path and the line @@ -6299,6 +6317,9 @@ def main(argv: list[str] | None = None) -> int: description="Deterministic orchestrator for the BMAD implementation phase", ) parser.add_argument("--version", action="version", version=f"bmad-loop {__version__}") + # Hidden: composed by the TUI launcher (`tui/launch.py` `start_detached`) for a + # detached child, never typed by hand. See the handling after `parse_args`. + parser.add_argument("--displaced-registry-root", help=argparse.SUPPRESS) sub = parser.add_subparsers(dest="command", required=True) def add(name: str, func, help: str, *, aliases=()) -> argparse.ArgumentParser: @@ -6741,6 +6762,23 @@ def add(name: str, func, help: str, *, aliases=()) -> argparse.ArgumentParser: # session stopped. `cmd_relay` is total, so nothing is lost by not wrapping it. if args.func is cmd_relay: return cmd_relay(args) + if args.displaced_registry_root: + # The launcher's displaced psmux registry, forwarded because its detached + # child inherits the derived root and so displaces nothing of its own — + # without it a TUI-launched resume or cleanup never sweeps the operator's + # pre-#537 registry. Recorded HERE, ahead of `_configure_mux`, because the + # record is first-wins (`note_displaced_registry`) and the export inside + # `_configure_mux` records what it displaces. The value comes from the launcher's own process record, never + # from policy.toml, and every kill in a legacy registry stays tag-proven, so + # the option gives a caller no reach beyond setting PSMUX_DATA_DIR itself. + # After the relay branch, which ignores it: a hook must never exit 2. + if not Path(args.displaced_registry_root).is_absolute(): + parser.error( + f"--displaced-registry-root must be absolute: {args.displaced_registry_root!r}" + ) + from .adapters.psmux_backend import note_displaced_registry + + note_displaced_registry(args.displaced_registry_root) try: # Install the policy [mux] backend choice before dispatch: several # handlers (probe/diagnose/attach/stop/cleanup/tui) reach the mux diff --git a/src/bmad_loop/data/settings/core.toml b/src/bmad_loop/data/settings/core.toml index 06d9744d..c953c225 100644 --- a/src/bmad_loop/data/settings/core.toml +++ b/src/bmad_loop/data/settings/core.toml @@ -516,3 +516,9 @@ kind = "select" default_ref = "MuxPolicy.backend" label = "backend" description = "force a registered transport backend by name (see `bmad-loop mux`); leave blank to auto-select — BMAD_LOOP_MUX_BACKEND outranks it · takes effect on the next bmad-loop invocation" +[[section.field]] +key = "honor_ambient_psmux_data_dir" +kind = "switch" +default_ref = "MuxPolicy.honor_ambient_psmux_data_dir" +label = "honour your own PSMUX_DATA_DIR" +description = "psmux only: on — an absolute PSMUX_DATA_DIR your profile exports into every shell is used as the registry instead of bmad-loop's derived per-project one · off (default) — it is overridden · leave off for a value typed into one shell · takes effect on the next bmad-loop invocation; a running TUI refuses launches until restarted" diff --git a/src/bmad_loop/engine.py b/src/bmad_loop/engine.py index 0dd67838..f7d07a88 100644 --- a/src/bmad_loop/engine.py +++ b/src/bmad_loop/engine.py @@ -95,6 +95,7 @@ clear_graceful_stop, consume_stop_request, deferred_stash_path, + drain_refused_kills, events_dir_for, graceful_stop_requested, kill_session, @@ -1096,6 +1097,24 @@ def _unresolved_escalation_key(self) -> str | None: None, ) + def _kill_run_session(self) -> None: + """Tear down this run's agent session, journaling a kill the + ownership gate refused (`runs.foreign_session_refusal`). + + `kill_session` reports a refusal on stderr and in a queue only the TUI + drains, so an unattended run would otherwise leave no durable trace of + it — and a degrade must be visible (AGENTS.md: journal it). It matters + beyond a foreign session: the gate fails closed when the shared registry + cannot be listed, so a transient listing fault leaves this run's own + agent window live. One `session-kill-refused` entry per drained reason; + a refusal with none queued is the control-session alias arm. + + `is False`, not falsy: only a refusal answers ``False``.""" + if kill_session(self.state.run_id) is False: + refusals = drain_refused_kills() or ["run id aliases a control session"] + for refusal in refusals: + self.journal.append("session-kill-refused", detail=refusal) + def _run_inner(self) -> RunSummary: self._install_stop_signals() try: @@ -1158,7 +1177,7 @@ def _run_inner(self) -> RunSummary: # _owns_signals); stop already kills it, and pause/interrupt # leave it for resume to reuse. if self._owns_signals and self.policy.adapter.cleanup_session_on_finish: - kill_session(self.state.run_id) + self._kill_run_session() except RunPaused as pause: self.state.paused_reason = pause.reason self.state.paused_stage = pause.stage @@ -1196,13 +1215,13 @@ def _run_inner(self) -> RunSummary: # a gracefully stopped child sweep is a clean completion from the # parent's perspective (the parent journals sweep-auto-finished). if self._owns_signals and self.policy.adapter.cleanup_session_on_finish: - kill_session(self.state.run_id) + self._kill_run_session() else: # Hard stop: the loop was interrupted inside adapter.run() (a # signal), or unwound on either side of it because a hard stop # request was honored — so the agent window may still be live. # Tear the whole run session down. - kill_session(self.state.run_id) + self._kill_run_session() if self._is_nested: raise # nested auto-sweep: let the owner record the stop self.state.stopped = True @@ -1229,7 +1248,7 @@ def _run_inner(self) -> RunSummary: # engine disappear with stale state. self._stopping = True # swallow stop signals landing mid-teardown try: - kill_session(self.state.run_id) + self._kill_run_session() except ( BaseException ): # nosec B110 - best-effort teardown; the stop must still record @@ -1255,7 +1274,7 @@ def _run_inner(self) -> RunSummary: except OSError: pass try: - kill_session(self.state.run_id) + self._kill_run_session() except ( Exception ): # nosec B110 - best-effort teardown; a crashing run must still record diff --git a/src/bmad_loop/policy.py b/src/bmad_loop/policy.py index 1e541f5b..b92b4f9d 100644 --- a/src/bmad_loop/policy.py +++ b/src/bmad_loop/policy.py @@ -365,6 +365,11 @@ class MuxPolicy: # data-only, and a plugin backend may not be importable in every context that # parses policy. Machine-specific: `bmad-loop init` gitignores policy.toml. backend: str = "" + # Honour an operator's own absolute PSMUX_DATA_DIR instead of overriding it + # with the derived per-project registry root (#729). A whether, never a + # where: policy.toml is writable by driven sessions, so it must not name a + # root — see runs.resolve_psmux_registry_root. + honor_ambient_psmux_data_dir: bool = False @dataclass(frozen=True) @@ -1355,7 +1360,15 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy: f"{sorted(OPERATOR_ON_REVIEW_DEMOTION_MODES)}:" f" got {operator.on_review_demotion!r}" ) - mux = MuxPolicy(backend=_typed_str(mux_d, "mux", "backend", MuxPolicy.backend).strip()) + mux = MuxPolicy( + backend=_typed_str(mux_d, "mux", "backend", MuxPolicy.backend).strip(), + honor_ambient_psmux_data_dir=_typed_bool( + mux_d, + "mux", + "honor_ambient_psmux_data_dir", + MuxPolicy.honor_ambient_psmux_data_dir, + ), + ) if mux.backend and not _MUX_NAME_RE.match(mux.backend): raise PolicyError( f"mux.backend must be a backend name (letters, digits, . _ -): got {mux.backend!r}" @@ -1640,6 +1653,12 @@ def _fold_deprecated_engine( # `bmad-loop mux` lists backends and shows the selection; `bmad-loop mux set # ` writes this key. Takes effect on the next bmad-loop invocation. # backend = "tmux" +# psmux only: true = use an absolute PSMUX_DATA_DIR your profile exports into +# every shell as the session registry, instead of bmad-loop's derived +# per-project one. Leave false for a value typed into a single shell. +# Takes effect on the next bmad-loop invocation; a running TUI refuses launches +# until restarted. +# honor_ambient_psmux_data_dir = false """ diff --git a/src/bmad_loop/runs.py b/src/bmad_loop/runs.py index fce51818..71146c3b 100644 --- a/src/bmad_loop/runs.py +++ b/src/bmad_loop/runs.py @@ -88,6 +88,9 @@ # selection, which probes a subprocess, so it cannot be routed through a backend # instance. See `export_psmux_registry_root`. PSMUX_DATA_DIR = "PSMUX_DATA_DIR" +# What `project_tag` returns: the parent directory name of every derived +# registry root. See `resolve_psmux_registry_root`. +_PROJECT_TAG_RE = re.compile(r"[0-9a-f]{16}") RUNS_DIR = Path(".bmad-loop") / "runs" ARCHIVE_DIR = Path(".bmad-loop") / "archive" PID_FILE = "engine.pid" @@ -603,7 +606,74 @@ def mux_registry_root(project: Path) -> Path: return project_state_root(project) / MUX_REGISTRY_DIR -def export_psmux_registry_root(project: Path) -> str | None: +def resolve_psmux_registry_root(derived: str, ambient: str | None, *, honor_ambient: bool) -> str: + """The registry root a process settles on: ``derived`` unless the operator + opted in (``[mux] honor_ambient_psmux_data_dir``) AND ``ambient`` is a value + worth honouring. Pure, so every process given one project, one policy file + and one environment reaches one answer — the outer process and a pane child + alike, which is what closes both horns of #729: + + - **Transient pin** (typed into one shell): leave the flag off. Everything + derives, a pane child included, so a clean process without the pin finds + the session. + - **Persistent pin** (a profile exports it into every shell): turn it on. + The outer process honours the pin and exports it, a pane child inherits + and honours it (not under ``PSMUX_BARE_ENV``, where a pane inherits no + ``PSMUX_DATA_DIR``; see ``_warn_if_bare_env``), and a clean process + carrying the profile pin honours it too. + + Whether the pin is persistent is not in the environment; the flag is the + operator saying so, from a per-project file both processes read. It is a + *whether* and never a *where*: ``policy.toml`` is written by the sessions + this orchestrator drives, so a policy-sourced path would let a driven + session aim the cleanup path's kills at a registry of its choosing. + + Not honoured, even with the flag on: + + - an empty or relative value — psmux panics on it, and no shell-relative + path can be one registry for two processes. ``Path.is_absolute`` and not + ``os.path.isabs``: below 3.13 the latter accepts a drive-relative + ``\\registry`` on Windows, which psmux rejects; + - a value shaped like a bmad-loop-derived root, + ``/``:data:`MUX_REGISTRY_DIR`. That is what an outer process + exports when it derived, and a pane child inheriting it must not mistake + it for a pin: a child moved to another state root or another + ``--project`` re-derives, exactly as a clean process with no pin does. + """ + if not (honor_ambient and ambient and Path(ambient).is_absolute()): + return derived + pin = Path(ambient) + if pin.name == MUX_REGISTRY_DIR and _PROJECT_TAG_RE.fullmatch(pin.parent.name): + return derived + return ambient + + +# The project this process configured its registry for +# (`export_psmux_registry_root`), or None before that ran. Process-local, set +# once ahead of dispatch like the backend's displaced-root record; it is what +# lets the by-name session operations prove ownership without a project +# parameter (see `foreign_session_refusal`). +_SETTLED_PROJECT: Path | None = None + + +def settled_project() -> Path | None: + """The project this process configured its registry for, or ``None`` + before :func:`export_psmux_registry_root` ran (library or test use).""" + return _SETTLED_PROJECT + + +def displaced_psmux_registry_root() -> str | None: + """The registry root this process's :func:`export_psmux_registry_root` + displaced, or ``None`` when it displaced nothing. Read by the TUI launcher, + so a detached child — which inherits the derived root and so displaces + nothing itself — still learns it (``--displaced-registry-root``). Imported + lazily for the same reason as the export's own call into the psmux leaf.""" + from .adapters import psmux_backend + + return psmux_backend._DISPLACED_ROOT + + +def export_psmux_registry_root(project: Path, *, honor_ambient: bool = False) -> str | None: """Point this process — and everything it spawns — at ``project``'s registry by exporting ``PSMUX_DATA_DIR``. Returns the value in force afterwards, or ``None`` when no root could be derived. @@ -616,17 +686,19 @@ def export_psmux_registry_root(project: Path) -> str | None: unreadable registry as ``False`` / ``[]`` — a live run reading itself as gone. One export ahead of dispatch covers every verb in-process. - **The root is always derived, and an ambient value never changes it.** That - is the whole rule, and the absence of an exception is the point: - :func:`mux_registry_root` is a pure function of (project, state root), so any - two bmad-loop processes given the same project and the same state root agree - — which is the entire property #537 exists to establish. A value already in - the environment is *overridden*, and the caller says so + **The root is derived, and an ambient value does not change it by + default.** :func:`mux_registry_root` is a pure function of (project, state + root), so any two bmad-loop processes given the same project and the same + state root agree — which is the entire property #537 exists to establish. A + value already in the environment is *overridden*, and the caller says so (:func:`cli._configure_mux` reports it once on stderr; ``bmad-loop mux`` - discloses it). + discloses it). The one exception is an operator's stated preference, + ``honor_ambient`` (policy ``[mux] honor_ambient_psmux_data_dir``), decided + by :func:`resolve_psmux_registry_root` — see there for why a boolean and + not a path, and which values it still refuses to honour. - **Why an operator's own ``PSMUX_DATA_DIR`` is not honoured**, since honouring - it is the obvious kindness and it was tried: + **Why an operator's own ``PSMUX_DATA_DIR`` is not honoured by default**, + since honouring it is the obvious kindness and it was tried: - It would make the registry a function of the launch *shell*. A TUI started from the Start menu carries no profile environment and derives; a run @@ -665,11 +737,10 @@ def export_psmux_registry_root(project: Path) -> str | None: Without that the override would strand exactly the sessions it displaced, with cleanup reporting a clean machine. - Wanting one registry to serve both is a real request and is deliberately not - answered here. It needs a stated operator preference rather than a guess at - one — and it must be a policy *whether*, never a *where*: ``policy.toml`` is - written by the sessions this orchestrator drives, so a policy-sourced root - would let a driven session choose which registry the cleanup path kills in. + Wanting one registry to serve both is answered by that stated preference + rather than a guess at one (#729). Honouring displaces the derived root + instead, and it is handed to the same sweep: sessions started before the + flag was turned on live there. **No ``BMAD_LOOP_*`` knob for the root either.** It is derived state, not configuration; ``BMAD_LOOP_STATE_DIR`` already relocates it transitively — @@ -692,6 +763,11 @@ def export_psmux_registry_root(project: Path) -> str | None: diagnostics down with it. ``None`` means "no root established": psmux keeps whatever it had, which is also the root cleanup sweeps as the legacy one. """ + global _SETTLED_PROJECT + # Recorded before anything can fail: the ownership gate needs the project + # on the degrade arm too, where the registry in force is not one derived + # for it (see `foreign_session_refusal`). + _SETTLED_PROJECT = project try: root = str(mux_registry_root(project)) except (StateRootError, OSError, RuntimeError): @@ -702,6 +778,13 @@ def export_psmux_registry_root(project: Path) -> str | None: # value psmux would panic on. return None displaced = os.environ.get(PSMUX_DATA_DIR) + derived = root + root = resolve_psmux_registry_root(derived, displaced, honor_ambient=honor_ambient) + if root != derived: + # Honoured: what is displaced now is the derived root, which holds any + # session started before the operator turned the flag on. Same sweep, + # same reason as the override arm below. + displaced = derived os.environ[PSMUX_DATA_DIR] = root if displaced is not None and displaced != root: # The variable is now gone, and it was the only record of where a @@ -1231,7 +1314,15 @@ def _session_liveness(run_id: str) -> str: if not mux_usable(mux): # forced-aware, like every other observer gate return "unknown" try: - return "alive" if mux.has_session(session_name(run_id)) else "unknown" + if not mux.has_session(session_name(run_id)): + return "unknown" + # A same-named session in a shared registry may be another project's: + # it proves nothing about this run, which is exactly 'unknown'. + refusal = foreign_session_refusal(session_name(run_id), mux) + if refusal is not None: + _warn_foreign_session(f"reading {session_name(run_id)} as this run's", refusal) + return "unknown" + return "alive" except (OSError, MultiplexerError): # The seam raises MultiplexerError (not OSError) on a backend failure; a # dead query proves nothing about a legacy run, so degrade to 'unknown' @@ -1329,7 +1420,108 @@ def discover_runs(project: Path) -> tuple[list[RunInfo], str | None]: # ----------------------------------------------------------- stop / delete / archive -def kill_session(run_id: str, mux: TerminalMultiplexer | None = None) -> None: +def foreign_session_refusal( + name: str, mux: TerminalMultiplexer | None = None, project: Path | None = None +) -> str | None: + """Why the session called ``name`` may NOT be treated as ``project``'s, or + ``None`` when it may (or does not exist). + + A by-name operation — a kill, an attach, a liveness read, an adapter adopting + a session it finds already there — is sound only where the registry itself + proves ownership: the derived per-project root, whose every session is this + project's. An operator's honoured root (#729) is shared by every project that + honours it, and run ids are unique per project only, so there + ``bmad-loop-`` may be a neighbour's. In such a registry the session must + carry this project's tag (:data:`PROJECT_OPTION` in :func:`accepted_tags`) — + the proof the cleanup sweep already demands (``require_tag``). + + Gated: a namespacing backend whose root in force is set and is not the + derived one. That reaches past the opt-in: on the flag-off degrade arm (no + state root derivable, so :func:`export_psmux_registry_root` leaves an + ambient ``PSMUX_DATA_DIR`` in force) the root in force is the operator's, + and it is gated just the same. Ungated: tmux (no namespace), the derived + root, and psmux's own default registry (``registry_root()`` ``None`` — the + same underivable arm with nothing ambient). That last one is shared too, and + :func:`_registry_proves_ownership` demands the tag for cleanup there, but a + cleanup sweep only narrows what it claims; this gate would refuse kills and + attaches by run id. The arm predates #729, startup warns about it, and gating + it would change its pre-#729 kill and attach behaviour. + + ``project`` defaults to the one this process configured + (``export_psmux_registry_root``); with neither, ownership cannot be proven + and the answer is a refusal. A listing that faults is a refusal too: "could + not ask" never reads as "absent". + """ + mux = mux or get_multiplexer() + try: + if not mux.has_registry_namespace(): + return None + root = mux.registry_root() + except MultiplexerError as exc: + return f"the multiplexer could not be asked which registry is in force: {exc}" + if root is None: + return None + project = project or _SETTLED_PROJECT + if project is None: + return ( + f"the registry {root} is not one derived for a known project, so " + f"ownership of {name} cannot be proven" + ) + try: + if root == str(mux_registry_root(project)): + return None + except (StateRootError, OSError, RuntimeError): + pass # no derived root: the one in force proves nothing, so ask the tag + key = mux.session_name_key(name) + faults: list[str] = [] + present: list[str] = [] + tags: dict[str, str] = {} + try: + listed = mux.list_sessions_reporting(on_fault=faults.append) + present = [n for n in listed if mux.session_name_key(n) == key] + tags = mux.session_options(PROJECT_OPTION) if present and not faults else {} + except MultiplexerError as exc: + faults.append(str(exc)) + if faults: + return ( + f"the shared registry {root} could not be listed ({faults[0]}), so " + f"ownership of {name} cannot be proven" + ) + if not present: + return None + tag = next((v for s, v in tags.items() if mux.session_name_key(s) == key), "") + try: + mine = accepted_tags(project) + except (OSError, RuntimeError) as exc: + return ( + f"this project's tag could not be computed ({exc}), so ownership of " + f"{name} cannot be proven" + ) + if tag in mine: + return None + whose = "untagged" if not tag else "tagged for another project" + return f"{name} in the shared registry {root} is {whose}, so it is not this project's" + + +def _warn_foreign_session(what: str, refusal: str) -> None: + print(f"warning: {what} refused — {refusal}", file=sys.stderr) + + +# Kills `kill_session` refused since the last `drain_refused_kills`, one line +# each. stderr is the CLI's channel; the TUI cannot read it (Textual captures +# it for the app's whole run), so its cleanup worker drains this instead. +_REFUSED_KILLS: list[str] = [] + + +def drain_refused_kills() -> list[str]: + """The kills :func:`kill_session` refused since the last call, oldest + first, and forget them — for a frontend whose stderr nobody reads.""" + drained = list(_REFUSED_KILLS) + del _REFUSED_KILLS[: len(drained)] + return drained + + +def kill_session(run_id: str, mux: TerminalMultiplexer | None = None) -> bool: """Kill a run's agent session (bmad-loop-); a no-op when it is already gone or the multiplexer is unavailable. @@ -1355,10 +1547,28 @@ def kill_session(run_id: str, mux: TerminalMultiplexer | None = None) -> None: deliberately so: a by-name kill in a shared registry without tag proof could take another project's same-named session (run ids are unique per project only). The legacy sweep in :func:`prune_sessions`, which does - demand the tag, is the path that reaches it.""" + demand the tag, is the path that reaches it — and, for one run's session + on a resume, :func:`kill_displaced_session`. + + In the primary registry the kill is also refused, with a warning, when that + registry is shared (an operator's honoured root, #729) and the session's + tag does not prove it this project's — :func:`foreign_session_refusal`. + + Returns ``False`` when the kill was refused (either rule above), ``True`` + when it was sent — sent, not landed: the backend kill is best-effort.""" if run_id_aliases_control_session(run_id): - return + return False + if mux is None: + # The primary registry: a by-name kill there needs ownership proof when + # it is shared (`foreign_session_refusal`). An explicit `mux` is a legacy + # registry whose callers judged the tag already. + refusal = foreign_session_refusal(session_name(run_id)) + if refusal is not None: + _warn_foreign_session(f"killing {session_name(run_id)}", refusal) + _REFUSED_KILLS.append(refusal) + return False (mux or get_multiplexer()).kill_session(session_name(run_id)) + return True CTL_SESSION = "bmad-loop-ctl" @@ -1413,6 +1623,11 @@ def ctl_session_for(project: Path, mux: TerminalMultiplexer | None = None) -> st state root spelled in two such casings of the same non-ASCII name stays split, as it is for every other digest of an operator-supplied path. + When the operator's own root is honoured (``[mux] + honor_ambient_psmux_data_dir``), that settled root is digested alongside + the derived one: the name stays per project, and changes with the + registry it lives in. + The degrade arm (namespaced transport, underivable state root) answers the fixed name: that arm runs on the transport's shared default registry, where a shared session scoped by per-window project tags is the correct, @@ -1423,7 +1638,22 @@ def ctl_session_for(project: Path, mux: TerminalMultiplexer | None = None) -> st if not mux.has_registry_namespace(): return CTL_SESSION try: - scope = os.path.normcase(str(mux_registry_root(project).resolve())) + derived = mux_registry_root(project) + scope = os.path.normcase(str(derived.resolve())) + # An honoured operator root (#729) is a different physical registry, so + # it gets a different name — or turning the opt-in on would mint the + # same name in the new registry while the old one's control session + # still holds psmux's mutex. Not honoured, the export settled the + # derived root and the name is byte-identical to before. Compared as + # identities, resolved and case-folded like `scope`, so another spelling + # of the derived registry is still that registry. The root is the + # backend's, not the environment's, so a bound instance answers for its + # own registry. + settled = mux.registry_root() + if settled and Path(settled).is_absolute(): + pinned = os.path.normcase(str(Path(settled).resolve())) + if pinned != scope: + scope += "\0" + pinned except (StateRootError, OSError, RuntimeError): return CTL_SESSION return f"{CTL_SESSION}-{hashlib.sha256(os.fsencode(scope)).hexdigest()[:16]}" @@ -1670,7 +1900,10 @@ def _registry_proves_ownership(project: Path) -> bool: ``PSMUX_DATA_DIR`` it found in force, and psmux honours any absolute value (``src/paths.rs``, source-read at v3.3.8) — so on that arm every verb, including the kill, addresses the operator's own registry while this project's - run dirs go on looking like ownership. + run dirs go on looking like ownership. The operator's opt-in + (``[mux] honor_ambient_psmux_data_dir``) puts every verb in their registry + on purpose, and it is just as shared: it misses the derived root, so the tag + is demanded there too. ``registry_root()`` answering ``None`` covers two cases, and they get **opposite** answers — conflating them was a defect, not caution. A backend @@ -1734,8 +1967,8 @@ def prune_sessions( :func:`prunable_sessions`' untagged run-dir fallback is evidence only where the registry has already restricted the listing to this project. A legacy registry is shared by every project by definition; the primary one is shared - whenever the derivation failed — an ambient ``PSMUX_DATA_DIR`` left in - force, or nothing in force at all, where a namespacing backend runs on its + whenever the derivation failed or the operator opted into their own root — an + ambient ``PSMUX_DATA_DIR`` left or honoured in force, or nothing in force at all, where a namespacing backend runs on its own shared default registry. What that strictness leaves standing in a legacy registry is reported by :func:`legacy_registry_leftovers`, which the cleanup frontends print: a @@ -1745,8 +1978,10 @@ def prune_sessions( project, require_tag=not _registry_proves_ownership(project) ) if not dry_run: - for run_id in prunable: - kill_session(run_id) + # A kill the ownership gate refuses (a listing that faulted between the + # partition and the kill) is not reported as removed; it warned. + prunable = [run_id for run_id in prunable if kill_session(run_id) is not False] + unknown &= set(prunable) # the killed subset, by contract try: legacies = _legacy_registries() except MultiplexerError: @@ -1765,6 +2000,88 @@ def prune_sessions( return prunable, live, unknown +def kill_displaced_session(project: Path, run_id: str) -> list[str]: + """Kill ``run_id``'s agent session in every legacy registry where its + ownership tag proves it this project's, and return one line per thing that + still stands in the resume's way: a registry that could not be listed, or a + same-named session left standing because nothing proves it ours. ``[]`` + means every registry answered and none holds a blocker. + + For a resume: :func:`kill_session` reaches only the registry this process + addresses, and psmux's duplicate-server guard is a mutex keyed on the + session name across every registry (psmux/psmux#599), so a same-named + session left standing in a registry this process no longer addresses — the + one it displaced (#537), psmux's default from before the per-project root, + or the derived one an opt-in to the operator's own root moved away from + (#729) — makes the resumed session's create fail. + + **Tag-proven, never by name.** A legacy registry is shared and run ids are + unique per project only, so the rule is the one :func:`prune_sessions`' + legacy pass applies (``require_tag=True``): the session's + :data:`PROJECT_OPTION` tag must be one of :func:`accepted_tags`. The + partition's *liveness* arm is deliberately not reused: by the time a resume + gets here it has refused a live engine and published its own pid, so the + run reads alive and every stale session would be spared. The session is + this run's, and this process is its engine. + + Listed through ``list_sessions_reporting`` so a failed listing reaches the + returned lines instead of folding into "no session". A failed tag read + still warns on the backend's own channel and proves nothing, so the session + is left standing and named here. The kill itself is best-effort and silent, + so the registry is re-listed after it and a survivor is named too. + """ + if run_id_aliases_control_session(run_id): + return [] + try: + legacies = _legacy_registries() + except MultiplexerError as exc: + return [f"no legacy registry could be checked: backend selection failed: {exc}"] + blockers: list[str] = [] + try: + mine = accepted_tags(project) + except (OSError, RuntimeError) as exc: + return [ + f"this project's tag could not be computed ({exc}), so no legacy registry was swept" + ] + for legacy in legacies: + label = legacy.registry_root() or DEFAULT_REGISTRY_LABEL + listing_faults: list[str] = [] + # Through the registry's own name key: on psmux `bmad-loop-R1` IS + # `bmad-loop-r1`, and the mutex folds the same way. + target = legacy.session_name_key(session_name(run_id)) + try: + names = legacy.list_sessions_reporting(on_fault=listing_faults.append) + ours = [n for n in names if legacy.session_name_key(n) == target] + tags = legacy.session_options(PROJECT_OPTION) if ours else {} + killed = False + for name in ours: + if tags.get(name, "") in mine: + kill_session(run_id, legacy) + killed = True + else: + blockers.append( + f"{label}: {name} left standing — its ownership tag does not " + "prove it this project's" + ) + # The kill is best-effort and silent by contract, so whether it + # landed is read back rather than assumed. + survivors = ( + [ + n + for n in legacy.list_sessions_reporting(on_fault=listing_faults.append) + if legacy.session_name_key(n) == target + ] + if killed + else [] + ) + except MultiplexerError as exc: + blockers.append(f"{label}: could not be listed: {exc}") + continue + blockers += [f"{label}: could not be listed: {fault}" for fault in listing_faults] + blockers += [f"{label}: {name} still standing after the kill" for name in survivors] + return blockers + + #: How a frontend names psmux's OWN default registry, the one root #: :meth:`~.adapters.multiplexer.TerminalMultiplexer.registry_root` deliberately #: answers ``None`` for (respelling its home cascade in Python is a second thing diff --git a/src/bmad_loop/runsetup.py b/src/bmad_loop/runsetup.py index 26071570..2e1ce52c 100644 --- a/src/bmad_loop/runsetup.py +++ b/src/bmad_loop/runsetup.py @@ -1529,8 +1529,18 @@ def compose_resume( resolved_sweep_options, ) # drop any stale agent session so the run spins up a fresh one (a stopped or - # interrupted run can leave a lingering bmad-loop- session behind). + # interrupted run can leave a lingering bmad-loop- session behind) — in + # this registry, and, tag-proven, in any registry the session predates: a + # same-named one left there blocks the new session's create. runs.kill_session(run_dir.name) + for blocker in runs.kill_displaced_session(project, run_dir.name): + journal.append("displaced-session-not-cleared", detail=blocker) + print( + f"warning: run {run_dir.name}: a stale agent session in an older " + f"registry may block this resume — {blocker}; if the resume fails to " + "create its session, check it with `bmad-loop cleanup`", + file=sys.stderr, + ) adapters = make_adapters(project, run_dir, policy, profiles=profiles) if state.run_type == "sweep": assert resolved_sweep_options is not None diff --git a/src/bmad_loop/tui/app.py b/src/bmad_loop/tui/app.py index e29d9173..c6cd018b 100644 --- a/src/bmad_loop/tui/app.py +++ b/src/bmad_loop/tui/app.py @@ -550,7 +550,7 @@ def action_attach(self) -> None: return session = runs.session_name(run_id) win_id = launch.ctl_window_id(self.project, run_id) - ok, agent_live = self._mux_guarded(lambda: launch.session_exists(session)) + ok, agent_live = self._mux_guarded(lambda: launch.agent_session_exists(session)) if not ok: return # A sweep blocked on a decision prompt has no agent session — the @@ -563,6 +563,17 @@ def action_attach(self) -> None: elif agent_live: target = runs.session_target(run_id) else: + # Textual captures stderr, so agent_session_exists' warning about a + # same-named session of another project's never reaches the screen. + ok, refusal = self._mux_guarded(lambda: runs.foreign_session_refusal(session)) + if not ok: + return + if refusal is not None: + # markup=False: the refusal carries a registry path. + self.notify( + f"not attaching: {refusal}", severity="warning", timeout=10, markup=False + ) + return self.notify( f"nothing to attach: no live agent session ({session}) and no " f"{launch.ctl_session(self.project)} window for this run (runs started outside " @@ -1560,6 +1571,15 @@ def _stop_run_worker(self, run_id: str, run_dir: Path) -> None: except (OSError, StopRunError, ProcessHostError) as e: self.call_from_thread(self.notify, f"stop failed: {e}", severity="error") return + # A backstop kill the shared-registry ownership gate refused leaves the + # session standing and warns on stderr, which Textual swallows. + for refusal in runs.drain_refused_kills(): + self.call_from_thread( + self.notify, + f"session not removed: {refusal}", + severity="warning", + markup=False, + ) self.call_from_thread(self.notify, f"run {run_id} stopped") def action_graceful_stop_run(self) -> None: @@ -1797,6 +1817,15 @@ def _cleanup_sessions_worker(self) -> None: # window(s)" toast reads as a successful window sweep. self.call_from_thread(self.notify, f"ctl window prune failed: {e}", severity="error") windows, survived, unverifiable = [], [], [] + # A kill the shared-registry ownership gate refused is left out of the + # count below and warned on stderr, which Textual swallows: say it here. + for refusal in runs.drain_refused_kills(): + self.call_from_thread( + self.notify, + f"session not removed: {refusal}", + severity="warning", + markup=False, + ) if unknown: self.call_from_thread( self.notify, diff --git a/src/bmad_loop/tui/launch.py b/src/bmad_loop/tui/launch.py index a23ed820..c3122652 100644 --- a/src/bmad_loop/tui/launch.py +++ b/src/bmad_loop/tui/launch.py @@ -21,9 +21,11 @@ from enum import StrEnum from pathlib import Path +from .. import policy as policy_mod from .. import runs from ..adapters.multiplexer import ( MultiplexerError, + TerminalMultiplexer, get_multiplexer, mux_usable, ) @@ -66,6 +68,24 @@ def session_exists(session: str) -> bool: return get_multiplexer().has_session(session) +def agent_session_exists(session: str) -> bool: + """:func:`session_exists` for a run's AGENT session, the one an attach lands + on: in a registry shared with other projects (#729) a same-named session + may be another project's, and it does not exist as far as this project is + concerned. Saying why is the difference between that and "no session". + + Kept off :func:`session_exists` itself, which also answers for the control + session — a per-project name already (`runs.ctl_session_for`), whose + session carries no project tag at all.""" + if not session_exists(session): + return False + refusal = runs.foreign_session_refusal(session, get_multiplexer()) + if refusal is not None: + print(f"warning: treating {session} as absent — {refusal}", file=sys.stderr) + return False + return True + + # Run-dir sidecar naming the ctl-session window start_detached minted last for # this run. `-` is not unique across the four kinds, so the window # listing alone cannot tell a live resume window from the parked run window it @@ -717,7 +737,7 @@ def attach_plan(project: Path, run_id: str) -> tuple[list[str], str | None] | No nothing to attach to.""" session = runs.session_name(run_id) win_id = ctl_window_id(project, run_id) - agent_live = session_exists(session) + agent_live = agent_session_exists(session) if win_id is not None and ( decision_pending(runs.run_dir_for(project, run_id)) or not agent_live ): @@ -880,6 +900,104 @@ def cli_argv(*tail: str) -> list[str]: return [sys.executable, "-m", "bmad_loop.cli", *tail] +def _registry_drift(project: Path, mux: TerminalMultiplexer) -> str | None: + """Why a run launched from this process would land in a registry this + process does not watch, or ``None`` when it would not. + + The registry is settled once per process (`cli._configure_mux`), but the + detached child re-reads ``[mux] honor_ambient_psmux_data_dir`` from + policy.toml, which the settings editor can rewrite under a running TUI. A + TUI that started honouring the operator's root and then had the switch + turned off would launch children into the derived root while it goes on + querying the old one: it could not see, attach to or stop what it started. + So the child's answer is predicted here with the same pure rule it will + apply (`runs.resolve_psmux_registry_root`), from the root it inherits — + this process's root in force — and a disagreement refuses the launch. + The prediction assumes inheritance. Under `PSMUX_BARE_ENV` a pane child + inherits no `PSMUX_DATA_DIR` (psmux re-adds only its allowlist), so it + derives. bmad-loop does not support that mode, and + `PsmuxMultiplexer._warn_if_bare_env` says so. + + Asked only of a process that configured its registry for this project + (`runs.settled_project`), which every CLI entry does: there is nothing to + disagree with otherwise. The other direction (switch turned ON) leaves the + child where this process is — it inherits the derived root, which the rule + never honours as a pin — but no longer where the operator is: a TUI that + overrode the operator's root R recorded it as displaced, and with the + switch now on, every shell carrying R would honour it while this TUI and + its children stay in the derived registry. That is refused too. A TUI + started without R in its environment (from the Start menu, say) displaced + nothing, cannot know R, and so has nothing to refuse.""" + if runs.settled_project() != project: + return None + try: + if not mux.has_registry_namespace(): + return None + root = mux.registry_root() + except MultiplexerError: + return None # selection already proved usable; the launch reports its own faults + if root is None: + return None + try: + derived = str(runs.mux_registry_root(project)) + except (runs.StateRootError, OSError, RuntimeError): + return None # the child cannot derive either, and keeps the root it inherits + fault: Exception | None = None + try: + honor = policy_mod.load(project / policy_mod.POLICY_FILE).mux.honor_ambient_psmux_data_dir + except (policy_mod.PolicyError, OSError) as exc: + honor = False # what the child's `_configure_mux` falls back to as well + fault = exc + child = runs.resolve_psmux_registry_root(derived, root, honor_ambient=honor) + if child == root: + if honor and root == derived: + displaced = runs.displaced_psmux_registry_root() + if ( + displaced + and runs.resolve_psmux_registry_root(derived, displaced, honor_ambient=True) + == displaced + ): + return ( + "[mux] honor_ambient_psmux_data_dir was turned on since this TUI " + f"started: a new run would stay in the derived registry {root}, while " + f"shells carrying your PSMUX_DATA_DIR now use {displaced} — restart " + "the TUI (bmad-loop tui), then launch" + ) + return None + if fault is not None: + # The switch may not have changed at all: the child cannot read the + # policy either, so it falls back to off. Name the real cause. + return ( + f"policy.toml could not be read ({fault}); a new run would use the registry " + f"{child}, but this TUI watches {root} — fix the policy, then launch" + ) + return ( + f"[mux] honor_ambient_psmux_data_dir changed since this TUI started: a new run " + f"would use the registry {child}, but this TUI watches {root} and could not " + "see, attach to or stop it — restart the TUI (bmad-loop tui), then launch" + ) + + +def _forwardable_displaced_root(displaced: str | None, in_force: str | None) -> str | None: + """The spelling of this process's displaced registry root to hand a detached + child (see :func:`start_detached`), or ``None`` when there is nothing to + forward, or nothing that would survive the trip. + + ``str(Path(...))`` drops a trailing separator everywhere but on a root. A + value that still ends in one *and* contains whitespace — a share root such + as ``\\\\srv\\my share\\`` — is the shape Windows PowerShell older than 7.3 + corrupts on the way into a parked window's argv (ADR 0001 §6, "Argv fidelity + on psmux"), so it is not forwarded: a sweep that misses that root is the + outcome before this forwarding existed, while a corrupted value would name + a registry nobody used.""" + if not displaced or not os.path.isabs(displaced) or displaced == in_force: + return None + normalized = str(Path(displaced)) + if normalized.endswith(("/", "\\")) and any(c.isspace() for c in normalized): + return None + return normalized + + def start_detached(project: Path, argv_tail: list[str], run_id: str, kind: str) -> str | None: """Run a bmad-loop command in a new window of the control session. @@ -901,6 +1019,14 @@ def start_detached(project: Path, argv_tail: list[str], run_id: str, kind: str) each button separately kept finding the path nobody gated (resolve was the fourth); gating the mutation cannot. Ahead of the mux probes so the refusal needs no transport to be phrased. + + Forwards this process's displaced psmux registry root, as the hidden + top-level ``--displaced-registry-root`` ahead of the subcommand. The child + inherits the derived root and so displaces nothing itself; without the + option a TUI-launched resume or cleanup would never sweep the operator's + pre-#537 registry, which only this process recorded. A root of the shape + older PowerShell corrupts in transit is not forwarded + (:func:`_forwardable_displaced_root`). """ if runs.run_id_aliases_control_session(run_id): raise LaunchError( @@ -913,6 +1039,20 @@ def start_detached(project: Path, argv_tail: list[str], run_id: str, kind: str) "multiplexer backend unavailable (binary missing, version unsupported, " "or a required helper absent)" ) + drift = _registry_drift(project, mux) + if drift is not None: + raise LaunchError(drift) + argv = cli_argv(*argv_tail) + try: + forwarded = ( + _forwardable_displaced_root(runs.displaced_psmux_registry_root(), mux.registry_root()) + if mux.has_registry_namespace() + else None + ) + except MultiplexerError as e: + raise LaunchError(f"multiplexer registry query failed: {e}") from e + if forwarded is not None: + argv = cli_argv(f"--displaced-registry-root={forwarded}", *argv_tail) ctl = _ensure_ctl_session(project) try: win_id = ( @@ -920,7 +1060,7 @@ def start_detached(project: Path, argv_tail: list[str], run_id: str, kind: str) ctl, f"{kind}-{run_id}", project, - cli_argv(*argv_tail), + argv, RETURN_OPTION, ) or None diff --git a/tests/conftest.py b/tests/conftest.py index 86f8d09a..6fa6030a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -999,7 +999,12 @@ def _isolate_mux_registry(monkeypatch): and for a sharper reason: it is written by the same export, it survives in module state rather than in the environment (which is the whole point of it), and a leftover value makes `legacy_registries()` hand every later test - an extra registry to sweep.""" + an extra registry to sweep. + + `runs._SETTLED_PROJECT` is reset on the same rule: the export records the + project it configured, and the ownership gate reads it, so a leftover would + judge a later test's sessions against an earlier test's project. Its record + of refused kills (`runs._REFUSED_KILLS`) likewise.""" from bmad_loop.adapters import psmux_backend monkeypatch.delenv(runs.PSMUX_DATA_DIR, raising=False) @@ -1007,6 +1012,8 @@ def _isolate_mux_registry(monkeypatch): monkeypatch.delenv("TMUX_PANE", raising=False) monkeypatch.delenv("PSMUX_BARE_ENV", raising=False) monkeypatch.setattr(psmux_backend, "_DISPLACED_ROOT", None) + monkeypatch.setattr(runs, "_SETTLED_PROJECT", None) + monkeypatch.setattr(runs, "_REFUSED_KILLS", []) def seed_project_files(root: Path) -> ProjectPaths: diff --git a/tests/test_cli.py b/tests/test_cli.py index d7749b74..a4d24d7a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -17501,6 +17501,56 @@ def test_main_stays_quiet_when_it_overrode_nothing( assert capsys.readouterr().err == "" +def test_main_honours_an_operators_registry_on_the_policy_opt_in( + force_psmux_backend, tmp_path, capsys, monkeypatch +): + """The seam half of #729: `_configure_mux` reads `[mux] + honor_ambient_psmux_data_dir` from the project's policy.toml and hands it to + the export, so the handler runs in the operator's registry — and says + nothing, because that is what the operator asked for. + + Ablate the `honor_ambient=` argument in `_configure_mux` and the handler + sees the derived root.""" + theirs = str(tmp_path / "their-own-registry") + (tmp_path / cli.POLICY_FILE).parent.mkdir(parents=True, exist_ok=True) + (tmp_path / cli.POLICY_FILE).write_text( + "[mux]\nhonor_ambient_psmux_data_dir = true\n", encoding="utf-8" + ) + monkeypatch.setenv(runs.PSMUX_DATA_DIR, theirs) + seen = {} + + def handler(args): + seen["root"] = os.environ.get(runs.PSMUX_DATA_DIR) + return 0 + + monkeypatch.setattr(cli, "cmd_list", handler) + assert cli.main(["list", "--project", str(tmp_path)]) == 0 + assert seen["root"] == theirs + assert capsys.readouterr().err == "" + + +def test_mux_says_when_the_registry_was_honoured( + force_psmux_backend, tmp_path, capsys, monkeypatch +): + """`bmad-loop mux` names which source won, and on the opt-in that is the + operator's own value — with the derived root beside it, so turning the flag + off is not a guess about where sessions will go. + + Ablate the honoured arm in `_print_registry` and this reads as the degrade + message instead.""" + theirs = str(tmp_path / "their-own-registry") + (tmp_path / cli.POLICY_FILE).parent.mkdir(parents=True, exist_ok=True) + (tmp_path / cli.POLICY_FILE).write_text( + "[mux]\nhonor_ambient_psmux_data_dir = true\n", encoding="utf-8" + ) + monkeypatch.setenv(runs.PSMUX_DATA_DIR, theirs) + + assert cli.main(["mux", "--project", str(tmp_path)]) == 0 + out = capsys.readouterr().out + assert f"registry: {theirs} (your own ${runs.PSMUX_DATA_DIR}, honoured" in out + assert str(runs.mux_registry_root(tmp_path)) in out + + def test_main_warns_when_it_has_no_registry_of_its_own( force_psmux_backend, tmp_path, capsys, monkeypatch ): @@ -17633,6 +17683,77 @@ def has_registry_namespace(self): assert os.environ[runs.PSMUX_DATA_DIR] == str(runs.mux_registry_root(tmp_path)) +def test_main_records_a_forwarded_displaced_registry(force_psmux_backend, tmp_path, monkeypatch): + """A detached child inherits its launcher's derived root and so displaces + nothing; the launcher's own record reaches it as the hidden top-level + `--displaced-registry-root`, and from there the legacy sweep names it — the + registry an operator's pre-#537 sessions live in. + + Ablate the `note_displaced_registry` call in `main` and `_DISPLACED_ROOT` + stays `None`.""" + from bmad_loop.adapters import multiplexer as multiplexer_mod + from bmad_loop.adapters import psmux_backend + + theirs = str(tmp_path / "their-own-registry") + monkeypatch.setenv(runs.PSMUX_DATA_DIR, str(runs.mux_registry_root(tmp_path))) + monkeypatch.setattr(cli, "cmd_list", lambda _args: 0) + + rc = cli.main(["--displaced-registry-root=" + theirs, "list", "--project", str(tmp_path)]) + + assert rc == 0 + assert psmux_backend._DISPLACED_ROOT == theirs + legacy = multiplexer_mod.get_multiplexer().legacy_registries() + assert theirs in [r.registry_root() for r in legacy] + + +def test_main_refuses_a_relative_displaced_registry(tmp_path, capsys, monkeypatch): + """psmux panics on a relative `PSMUX_DATA_DIR`, and the option is a value no + operator types: a relative one is a malformed launch, refused as a usage + error before anything is recorded or dispatched. + + Ablate the `is_absolute()` check and `main` returns 0 with the value + recorded.""" + from bmad_loop.adapters import psmux_backend + + monkeypatch.setattr(cli, "cmd_list", lambda _args: 0) + + with pytest.raises(SystemExit) as exc: + cli.main(["--displaced-registry-root=relative-root", "list", "--project", str(tmp_path)]) + + assert exc.value.code == cli.ExitCode.USAGE + assert psmux_backend._DISPLACED_ROOT is None + assert capsys.readouterr().out == "" + + +def test_main_forwarded_root_precedes_the_exports_own(force_psmux_backend, tmp_path, monkeypatch): + """The record is first-wins, so the forwarded value is noted ahead of + `_configure_mux`, whose export notes whatever it displaces. Given an ambient + root of its own as well, the child keeps the launcher's record. + + Ablate by moving the handling after `_configure_mux` and the export's + displaced value wins.""" + from bmad_loop.adapters import psmux_backend + + forwarded = str(tmp_path / "launchers-displaced") + monkeypatch.setenv(runs.PSMUX_DATA_DIR, str(tmp_path / "ambient-in-the-child")) + monkeypatch.setattr(cli, "cmd_list", lambda _args: 0) + + rc = cli.main(["--displaced-registry-root=" + forwarded, "list", "--project", str(tmp_path)]) + + assert rc == 0 + assert psmux_backend._DISPLACED_ROOT == forwarded + + +def test_relay_ignores_a_displaced_registry_root(monkeypatch): + """`relay` stays first to dispatch: a top-level option it has no use for, + however malformed, must not turn a hook into a usage error. + + Ablate by moving the option handling ahead of the relay branch and this + exits 2.""" + monkeypatch.setattr(cli, "cmd_relay", lambda _args: 0) + assert cli.main(["--displaced-registry-root=relative-root", "relay", "Stop"]) == 0 + + def test_main_leaves_psmux_data_dir_alone_when_no_backend_can_be_selected( tmp_path, capsys, monkeypatch ): diff --git a/tests/test_engine.py b/tests/test_engine.py index 58bab330..58d7d293 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -16039,6 +16039,71 @@ def boom(): assert not journal.exists() or "run-crash" not in journal.read_text() +def _loop_finishes(): + return None + + +def _loop_hard_stops(): + raise RunStopped() + + +def _loop_crashes(): + raise RuntimeError("boom") + + +@pytest.mark.parametrize( + "loop", + [_loop_finishes, _loop_hard_stops, _loop_crashes], + ids=["finish", "hard-stop", "crash"], +) +@pytest.mark.parametrize("kill_result", [False, None], ids=["refused", "sent"]) +def test_teardown_kill_refusal_is_journaled(project, monkeypatch, loop, kill_result): + """A teardown kill the ownership gate refused (`runs.foreign_session_refusal`) + is journaled as `session-kill-refused`, with the drained reason, on every + teardown arm — not left on stderr and in a queue only the TUI drains. + + The `None` row is the control: engine tests stub `kill_session` with + `lambda rid: None`, and only a `False` answer is a refusal — so a queued + reason does not journal unless the kill actually reported one. + + Ablation: make `_kill_run_session` call `kill_session` and return, and the + refused rows fail; test the result with `not` instead of `is False`, and + the sent rows fail.""" + killed = [] + + def kill(rid): + killed.append(rid) + return kill_result + + monkeypatch.setattr("bmad_loop.engine.kill_session", kill) + monkeypatch.setattr("bmad_loop.engine.drain_refused_kills", lambda: ["x is untagged"]) + engine, _ = make_engine(project, []) + monkeypatch.setattr(engine, "_loop", loop) + + engine.run() + + assert killed == ["test-run"] + refused = [e for e in engine.journal.entries() if e["kind"] == "session-kill-refused"] + if kill_result is False: + assert [e["detail"] for e in refused] == ["x is untagged"] + else: + assert refused == [] + + +def test_teardown_kill_refusal_without_a_reason_names_the_alias(project, monkeypatch): + """A refusal with no reason queued is `kill_session`'s control-session alias + arm, which refuses before the gate runs; it still journals, and says so.""" + monkeypatch.setattr("bmad_loop.engine.kill_session", lambda rid: False) + monkeypatch.setattr("bmad_loop.engine.drain_refused_kills", lambda: []) + engine, _ = make_engine(project, []) + monkeypatch.setattr(engine, "_loop", _loop_crashes) + + engine.run() + + refused = [e for e in engine.journal.entries() if e["kind"] == "session-kill-refused"] + assert [e["detail"] for e in refused] == ["run id aliases a control session"] + + def test_run_crash_after_finish_clears_finished(project, monkeypatch): """A post-loop step that throws after finished=True is recorded as a crash and the finished flag is cleared, so status classification reads CRASHED diff --git a/tests/test_generic_tmux.py b/tests/test_generic_tmux.py index 1a0f2667..e9e4ee0b 100644 --- a/tests/test_generic_tmux.py +++ b/tests/test_generic_tmux.py @@ -157,6 +157,215 @@ def fake_run(argv, **kwargs): ] +class _SharedRegistryMux: + """A shared (honoured, #729) registry already holding `name`, tagged `tag`.""" + + def __init__(self, name, tag): + self._name, self._tag = name, tag + self.created: list[str] = [] + self.killed: list[str] = [] + + def has_registry_namespace(self): + return True + + def registry_root(self): + return "/shared-registry" + + def session_name_key(self, name): + return name + + def has_session(self, name): + return name == self._name + + def list_sessions_reporting(self, *, on_fault=None): + return [self._name] + + def session_options(self, _option): + return {self._name: self._tag} if self._tag else {} + + def new_session(self, name, *_args): + self.created.append(name) + + def kill_session(self, name): + self.killed.append(name) + + +@pytest.mark.parametrize("tag", ["0123456789abcdef", ""], ids=["foreign", "untagged"]) +def test_ensure_session_refuses_to_adopt_another_projects_session(tmp_path, tag): + """In a registry shared with another project, an existing same-named + session may be that project's: adopting it would open this run's windows + inside it, under its tag. The launch fails with a clear error instead. + + Ablate the gate in `_ensure_session` and it returns as if the session were + this run's own.""" + run_dir = tmp_path / ".bmad-loop" / "runs" / "RID" # parents[2] == project + mux = _SharedRegistryMux("bmad-loop-RID", tag) + adapter = GenericTmuxAdapter( + run_dir=run_dir, + policy=Policy(limits=LimitsPolicy()), + profile=get_profile("claude"), + mux=mux, + ) + + with pytest.raises(MultiplexerError, match="refusing to launch into the existing session"): + adapter._ensure_session(tmp_path) + assert mux.created == [] + assert mux.killed == [] # a session it FOUND is never torn down + + +class _TagFailingMux: + """No session exists yet; minting one works, tagging it raises. Records the + session lifecycle so a teardown is observable, and models the backend kill + as what it is by contract: best-effort, so it can fail silently (`stuck`), + raise (`kill_fault`), or leave a registry that cannot be read back + (`list_fault`).""" + + def __init__(self, *, kill_fault=None, stuck=False, list_fault=None): + self.sessions: list[str] = [] + self.created: list[str] = [] + self.killed: list[str] = [] + self.tag_fault = MultiplexerError("set-option failed: transient") + self._kill_fault = kill_fault + self._stuck = stuck + self._list_fault = list_fault + + def has_session(self, name): + return name in self.sessions + + def session_name_key(self, name): + return name + + def new_session(self, name, *_args): + self.created.append(name) + self.sessions.append(name) + + def set_session_option(self, name, option, value): + raise self.tag_fault + + def kill_session(self, name): + self.killed.append(name) + if self._kill_fault is not None: + raise self._kill_fault + if not self._stuck: + self.sessions.remove(name) + + def list_sessions_reporting(self, *, on_fault=None): + if self._list_fault is not None: + assert on_fault is not None + on_fault(self._list_fault) + return [] + return list(self.sessions) + + +def _tag_failing_adapter(tmp_path, mux): + return GenericTmuxAdapter( + run_dir=tmp_path / ".bmad-loop" / "runs" / "RID", + policy=Policy(limits=LimitsPolicy()), + profile=get_profile("claude"), + mux=mux, + ) + + +def test_ensure_session_tears_down_a_session_it_could_not_tag(tmp_path): + """Left standing, an untagged session blocks its run id for good in a shared + registry (#729): the ownership gate reads it as foreign and the kill and + cleanup paths refuse it. The session this call just minted is torn down by + that exact name, confirmed gone, and the ORIGINAL error propagates. + + Ablate the teardown and `mux.killed` is empty.""" + mux = _TagFailingMux() + adapter = _tag_failing_adapter(tmp_path, mux) + + with pytest.raises(MultiplexerError) as caught: + adapter._ensure_session(tmp_path) + + assert caught.value is mux.tag_fault + assert mux.created == mux.killed == ["bmad-loop-RID"] + assert mux.sessions == [] + + +@pytest.mark.parametrize( + ("teardown", "said"), + [ + ("raises", "could not be confirmed gone (kill-session failed: gone wrong)"), + ("silent", "is still there after tearing it down"), + ("unlistable", "could not be confirmed gone (list-sessions failed: rc 1)"), + ], +) +def test_ensure_session_says_so_when_the_teardown_did_not_land(tmp_path, teardown, said): + """The backend kill is best-effort and silent by contract, so a teardown is + read back, not assumed: a kill that raised, one that silently left the + session, and a registry that cannot be listed each raise one error naming + the tag fault and the leftover, chained from the tag fault. + + Ablate the read-back (trust the kill) and the `silent` and `unlistable` rows + re-raise the bare tag fault instead.""" + kill_fault = MultiplexerError("kill-session failed: gone wrong") + mux = _TagFailingMux( + kill_fault=kill_fault if teardown == "raises" else None, + stuck=teardown == "silent", + list_fault="list-sessions failed: rc 1" if teardown == "unlistable" else None, + ) + adapter = _tag_failing_adapter(tmp_path, mux) + + with pytest.raises(MultiplexerError) as caught: + adapter._ensure_session(tmp_path) + + assert caught.value is not mux.tag_fault + assert caught.value.__cause__ is mux.tag_fault + assert said in str(caught.value) + assert "transient" in str(caught.value) and "remove it by hand" in str(caught.value) + + +def test_ensure_session_reports_a_silent_teardown_failure_on_the_real_backend( + tmp_path, monkeypatch, force_tmux_backend +): + """The bundled backend, not a double: `set-option` fails, `kill-session` fails + silently (the seam's `check=False`), and the listing still names the session. + The operator is told it is still there. + + Ablate the read-back and the bare tag fault surfaces instead.""" + project = tmp_path + run_dir = project / ".bmad-loop" / "runs" / "RID" + adapter = GenericTmuxAdapter( + run_dir=run_dir, policy=Policy(limits=LimitsPolicy()), profile=get_profile("claude") + ) + name = adapter.session_name + monkeypatch.setattr(tmux_base.shutil, "which", lambda _b: "/usr/bin/tmux") + + def fake_run(argv, **kwargs): + verb = argv[1] + if verb == "has-session": + return subprocess.CompletedProcess(argv, 1, stdout="", stderr="") + if verb in ("set-option", "kill-session"): + return subprocess.CompletedProcess(argv, 1, stdout="", stderr="server busy") + if verb == "list-sessions": + return subprocess.CompletedProcess(argv, 0, stdout=f"{name}\n", stderr="") + return subprocess.CompletedProcess(argv, 0, stdout="", stderr="") + + monkeypatch.setattr(tmux_base.subprocess, "run", fake_run) + + with pytest.raises(MultiplexerError, match="is still there after tearing it down") as caught: + adapter._ensure_session(project) + assert isinstance(caught.value.__cause__, MultiplexerError) + + +def test_ensure_session_reuses_its_own_session_in_a_shared_registry(tmp_path): + """The other half: this run's own tagged session (a resume) is reused.""" + run_dir = tmp_path / ".bmad-loop" / "runs" / "RID" + mux = _SharedRegistryMux("bmad-loop-RID", runs.project_tag(tmp_path)) + adapter = GenericTmuxAdapter( + run_dir=run_dir, + policy=Policy(limits=LimitsPolicy()), + profile=get_profile("claude"), + mux=mux, + ) + + adapter._ensure_session(tmp_path) + assert mux.created == [] + assert mux.killed == [] # a session it FOUND is never torn down + + def make_spec(tmp_path, task_id="1-1-a-dev-1", timeout_s=30.0, model="sonnet") -> SessionSpec: return SessionSpec( task_id=task_id, @@ -727,6 +936,10 @@ def __init__(self, screen=""): def has_session(self, name): return True + def has_registry_namespace(self): + # tmux-shaped: the shared-registry ownership gate (#729) stays out. + return False + def send_text(self, window_id, text): # The contract/stall nudges reach the mux too; recording them keeps that # off the host binary as well, which is the same promise as has_session. @@ -4605,6 +4818,10 @@ def has_session(self, name): # window that died under a session that is still very much there. return True + def has_registry_namespace(self): + # tmux-shaped: the shared-registry ownership gate (#729) stays out. + return False + @pytest.mark.parametrize( "task_id_kind", ["absolute", "parent-traversal", "empty", "windows-reserved"] diff --git a/tests/test_policy.py b/tests/test_policy.py index a0399b32..410d2457 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -796,6 +796,7 @@ def test_array_policy_fields_keep_unset_distinct_from_empty(): ("cleanup", "clean_tmp"), ("tui", "low_frame_rate"), ("operator", "enabled"), + ("mux", "honor_ambient_psmux_data_dir"), ] @@ -1554,6 +1555,12 @@ def test_mux_backend_parses_and_strips(): assert pol.mux.backend == "psmux" +def test_mux_honor_ambient_psmux_data_dir_defaults_off_and_parses(): + assert policy.loads("").mux.honor_ambient_psmux_data_dir is False + pol = policy.loads("[mux]\nhonor_ambient_psmux_data_dir = true\n") + assert pol.mux.honor_ambient_psmux_data_dir is True + + def test_mux_backend_rejects_junk(): with pytest.raises(policy.PolicyError, match="mux.backend"): policy.loads('[mux]\nbackend = "not a name!"\n') diff --git a/tests/test_portability_guard.py b/tests/test_portability_guard.py index a0d4068e..3d512c5c 100644 --- a/tests/test_portability_guard.py +++ b/tests/test_portability_guard.py @@ -1446,6 +1446,14 @@ "story-escalation-resolved", # runsetup.py "composition-unwind-failed", + # A resume that could not clear its run's same-named session from a + # displaced psmux registry. No new diagnostics routing: its only field, + # `detail`, is already in `_JOURNAL_DROP_FIELDS`. + "displaced-session-not-cleared", + # engine.py: a teardown kill the ownership gate refused. No new + # diagnostics routing: its only field, `detail`, is already in + # `_JOURNAL_DROP_FIELDS`. + "session-kill-refused", "run-start", # stories_engine.py "checkpoint-pause", diff --git a/tests/test_runs.py b/tests/test_runs.py index fbc40ed8..606791e4 100644 --- a/tests/test_runs.py +++ b/tests/test_runs.py @@ -7144,6 +7144,185 @@ def test_export_psmux_registry_root_converges_a_pane_child_that_moves_the_state_ assert child == clean == clean_pinned == str(runs.mux_registry_root(tmp_path)) +# An absolute path that is not a derived root, spelled for the host OS. +_PIN = os.path.abspath(os.path.join(os.sep, "operator", "pin")) + + +@pytest.mark.parametrize( + ("ambient", "honor", "expected"), + [ + (None, False, "derived"), + (None, True, "derived"), + (_PIN, False, "derived"), + (_PIN, True, "ambient"), + ("", True, "derived"), + ("relative/root", True, "derived"), + (".", True, "derived"), + ( + os.path.join(os.path.dirname(_PIN), "0123456789abcdef", runs.MUX_REGISTRY_DIR), + True, + "derived", + ), + (os.path.join(os.path.dirname(_PIN), runs.MUX_REGISTRY_DIR), True, "ambient"), + ("\\registry", True, "derived"), # rooted but drive-relative on Windows + ], +) +def test_resolve_psmux_registry_root(ambient, honor, expected): + """The whole decision, as a table. Honoured only on the opt-in AND an + absolute value that is not shaped like a derived root + (`<16-hex project tag>/_mux`); everything else derives. An operator's own + directory merely named `_mux` is still theirs. + + Ablate any one conjunct in `resolve_psmux_registry_root` and a row fails: + the flag (row 3), absoluteness (rows 5-7, and on Windows the + drive-relative row 10, which `os.path.isabs` accepts below 3.13), the + derived-root exclusion (row 8), its tag-shape narrowing (row 9).""" + derived = os.path.abspath(os.path.join(os.sep, "state", "proj", runs.MUX_REGISTRY_DIR)) + got = runs.resolve_psmux_registry_root(derived, ambient, honor_ambient=honor) + assert got == (ambient if expected == "ambient" else derived) + + +@pytest.mark.parametrize("honor", [False, True]) +def test_the_policy_flag_closes_the_horn_it_names(tmp_path, monkeypatch, honor): + """#729's measured table, one row per process, for both settings of the flag. + + The outer process runs under S1 with the pin R in its environment; the pane + child inherits what the outer exported and moves to S2. The flag is the + operator saying which kind of pin R is, so it names the clean process the + child has to agree with: + + - off — a *transient* pin, typed into one shell: a clean S2 process has no + pin and derives D2; + - on — a *persistent* pin, exported by the profile: a clean S2 process + carries R and honours it. + + Ablate the flag (always derive, or always honour) and one parametrization + fails.""" + pinned = str(tmp_path / "pinned") + monkeypatch.setenv(envvars.STATE_DIR, str(tmp_path / "S1")) + monkeypatch.setenv(runs.PSMUX_DATA_DIR, pinned) + d1 = str(runs.mux_registry_root(tmp_path)) + outer = runs.export_psmux_registry_root(tmp_path, honor_ambient=honor) + + # the pane child: carries whatever the outer exported, now under S2 + monkeypatch.setenv("TMUX", "/tmp/psmux-1000/default,123,0") + monkeypatch.setenv(envvars.STATE_DIR, str(tmp_path / "S2")) + child = runs.export_psmux_registry_root(tmp_path, honor_ambient=honor) + + monkeypatch.delenv("TMUX", raising=False) + if honor: # persistent horn: a clean S2 process WITH the profile pin + monkeypatch.setenv(runs.PSMUX_DATA_DIR, pinned) + else: # transient horn: a clean S2 process with NO pin + monkeypatch.delenv(runs.PSMUX_DATA_DIR, raising=False) + clean = runs.export_psmux_registry_root(tmp_path, honor_ambient=honor) + + d2 = str(runs.mux_registry_root(tmp_path)) + assert outer == (pinned if honor else d1) + assert child == clean == (pinned if honor else d2) + + +def test_honouring_never_adopts_an_inherited_derived_root(tmp_path, monkeypatch): + """With the flag on and no pin at all, the outer process derives D1 and + exports it, so every pane child inherits D1 — which is not the operator's + pin and must not be honoured as one. A child moved to S2, or one run for + another project, re-derives exactly as a clean process does. + + Ablate the derived-root exclusion in `resolve_psmux_registry_root` and both + asserts fail.""" + monkeypatch.setenv(envvars.STATE_DIR, str(tmp_path / "S1")) + monkeypatch.delenv(runs.PSMUX_DATA_DIR, raising=False) + d1 = runs.export_psmux_registry_root(tmp_path, honor_ambient=True) + assert d1 == str(runs.mux_registry_root(tmp_path)) + + monkeypatch.setenv(envvars.STATE_DIR, str(tmp_path / "S2")) + assert runs.export_psmux_registry_root(tmp_path, honor_ambient=True) == str( + runs.mux_registry_root(tmp_path) + ) + + other = tmp_path / "other" + other.mkdir() + monkeypatch.setenv(runs.PSMUX_DATA_DIR, d1) + assert runs.export_psmux_registry_root(other, honor_ambient=True) == str( + runs.mux_registry_root(other) + ) + + +@pytest.mark.parametrize("ambient", ["", "relative/root", "."]) +def test_honouring_still_overrides_a_value_psmux_would_panic_on(tmp_path, monkeypatch, ambient): + """The flag honours a pin, not a typo: a relative or empty value is replaced + by the derived root exactly as with the flag off, so `PsmuxMultiplexer._run`'s + refusal stays reserved for the degrade arm it was written for. + + Ablate the `os.path.isabs` conjunct in `resolve_psmux_registry_root` and + this fails.""" + monkeypatch.setenv(runs.PSMUX_DATA_DIR, ambient) + derived = str(runs.mux_registry_root(tmp_path)) + assert runs.export_psmux_registry_root(tmp_path, honor_ambient=True) == derived + assert os.environ[runs.PSMUX_DATA_DIR] == derived + + +def test_ctl_session_for_follows_an_honoured_registry(tmp_path, monkeypatch): + """psmux's duplicate-server mutex is keyed on the session name alone, so the + control session in an honoured registry must not reuse the name the derived + registry's one holds: turning the opt-in on while the old control session + lives would otherwise make every TUI launch fail. With the derived root + settled, the name is unchanged. + + Ablate the honoured-root arm in `ctl_session_for` and the first assert + fails.""" + derived = str(runs.mux_registry_root(tmp_path)) + monkeypatch.setenv(runs.PSMUX_DATA_DIR, derived) + as_derived = runs.ctl_session_for(tmp_path, _NamespaceStub(True)) + monkeypatch.setenv(runs.PSMUX_DATA_DIR, str(tmp_path / "pinned")) + as_honoured = runs.ctl_session_for(tmp_path, _NamespaceStub(True)) + monkeypatch.setenv(runs.PSMUX_DATA_DIR, os.path.join(derived, "..", runs.MUX_REGISTRY_DIR)) + respelled = runs.ctl_session_for(tmp_path, _NamespaceStub(True)) + monkeypatch.delenv(runs.PSMUX_DATA_DIR) + unset = runs.ctl_session_for(tmp_path, _NamespaceStub(True)) + + assert as_honoured != as_derived + assert as_derived == unset == respelled + assert runs.is_ctl_session_name(as_honoured) + + +def test_ctl_session_for_reads_the_backends_root(tmp_path, monkeypatch): + """The honoured root is the one the backend's verbs resolve through, not + whatever the environment says: a bound instance answers for its own + registry, so its control session is named for that registry. + + Ablate (read `PSMUX_DATA_DIR` from the environment again) and the bound + instance is named for the derived registry instead.""" + derived = str(runs.mux_registry_root(tmp_path)) + pinned = str(tmp_path / "pinned") + monkeypatch.setenv(runs.PSMUX_DATA_DIR, pinned) + as_honoured = runs.ctl_session_for(tmp_path, _NamespaceStub(True)) + monkeypatch.setenv(runs.PSMUX_DATA_DIR, derived) + as_derived = runs.ctl_session_for(tmp_path, _NamespaceStub(True)) + + bound = runs.ctl_session_for(tmp_path, _NamespaceStub(True, root=pinned)) + + assert as_honoured != as_derived # the premise: the two roots name differently + assert bound == as_honoured + + +def test_honouring_hands_the_derived_root_to_the_sweep(tmp_path, monkeypatch): + """Turning the flag on moves the registry from the derived root to the pin, + and sessions started before the switch are still in the derived one. It is + the displaced root now, so cleanup's tag-scoped legacy pass reaches it. + + Ablate the `displaced = derived` arm in `export_psmux_registry_root` and + nothing is recorded.""" + from bmad_loop.adapters import psmux_backend + + monkeypatch.setattr(psmux_backend, "_DISPLACED_ROOT", None) + pinned = str(tmp_path / "pinned") + monkeypatch.setenv(runs.PSMUX_DATA_DIR, pinned) + + assert runs.export_psmux_registry_root(tmp_path, honor_ambient=True) == pinned + assert os.environ[runs.PSMUX_DATA_DIR] == pinned + assert psmux_backend._DISPLACED_ROOT == str(runs.mux_registry_root(tmp_path)) + + def test_pinned_state_env_resolves_rather_than_forwards(tmp_path, monkeypatch): """What travels is the answer this process reached, not the override it was handed. Forwarding only when the operator set something leaves the common case @@ -7175,15 +7354,20 @@ def boom(): class _NamespaceStub: - """Duck-typed mux answering only the namespace question — all - ctl_session_for consults.""" + """Duck-typed mux answering only what ctl_session_for consults: the + namespace question, and the registry root — the environment's, as the + primary psmux instance answers, unless bound to its own.""" - def __init__(self, namespaced): + def __init__(self, namespaced, root=None): self._namespaced = namespaced + self._root = root def has_registry_namespace(self): return self._namespaced + def registry_root(self): + return self._root or os.environ.get(runs.PSMUX_DATA_DIR) + def test_ctl_session_for_is_fixed_without_a_registry_namespace(tmp_path): """tmux keeps the machine-shared `bmad-loop-ctl` byte-identically — the @@ -7714,6 +7898,9 @@ def session_name_key(self, name): def list_sessions(self): return list(self._sessions) + def list_sessions_reporting(self, *, on_fault=None): + return self.list_sessions() + def session_options(self, _option): return dict(self._tags) @@ -7904,6 +8091,153 @@ def test_prune_sessions_refuses_an_untagged_legacy_session_claimed_only_by_a_run assert legacy.killed == [] +class _RemovingMux(_RegistryMux): + """A registry whose kill lands: the session leaves the listing.""" + + def kill_session(self, name): + super().kill_session(name) + key = self.session_name_key(name) + self._sessions = [s for s in self._sessions if self.session_name_key(s) != key] + + +def test_kill_displaced_session_kills_only_this_runs_tag_proven_session(tmp_path, monkeypatch): + """A resume must clear a same-named session in a registry this process no + longer addresses, or psmux's name mutex (it spans registries) rejects the + resumed session's create. Exactly that one session, and only on its tag: + another run of ours stays, and a neighbour's same-named session and an + untagged one stay too, even though this project holds a dead `r1` run dir + the primary pass's untagged fallback would accept as proof. + + Each one left standing is named, because it will still block the resume. + Ablate the `kill_session(run_id, legacy)` call and `mine.killed` is empty; + ablate the tag check and the other two are killed.""" + ours = runs.project_tag(tmp_path) + (_make_state_run(tmp_path, "r1") / "engine.pid").write_text(str(_dead_pid())) + mine = _RemovingMux( + ["bmad-loop-r1", "bmad-loop-r2"], {"bmad-loop-r1": ours, "bmad-loop-r2": ours} + ) + neighbour = _RegistryMux( + ["bmad-loop-r1"], {"bmad-loop-r1": "0123456789abcdef"}, root=str(tmp_path / "n") + ) + untagged = _RegistryMux(["bmad-loop-r1"], {}, root=str(tmp_path / "u")) + monkeypatch.setattr(runs, "_legacy_registries", lambda: [mine, neighbour, untagged]) + + blockers = runs.kill_displaced_session(tmp_path, "r1") + assert mine.killed == ["bmad-loop-r1"] + assert neighbour.killed == untagged.killed == [] + assert len(blockers) == 2 + assert str(tmp_path / "n") in blockers[0] and str(tmp_path / "u") in blockers[1] + assert all("bmad-loop-r1 left standing" in b for b in blockers) + + +def test_kill_displaced_session_kills_even_though_the_resumed_run_reads_alive( + tmp_path, monkeypatch +): + """By the time a resume sweeps, it has refused a live engine and published + its own pid, so the run reads alive. Reusing the prune partition's liveness + arm would spare every stale session — the one case this exists for. + + Ablate by routing the claim back through `prunable_sessions` and this + fails.""" + monkeypatch.setattr(runs, "engine_liveness", lambda _d: "alive") + legacy = _RemovingMux(["bmad-loop-r1"], {"bmad-loop-r1": runs.project_tag(tmp_path)}) + monkeypatch.setattr(runs, "_legacy_registries", lambda: [legacy]) + + assert runs.kill_displaced_session(tmp_path, "r1") == [] + assert legacy.killed == ["bmad-loop-r1"] + + +def test_kill_displaced_session_reports_a_registry_it_could_not_ask(tmp_path, monkeypatch): + """Could not ask is not "nothing there". A listing the backend folds into + `[]` reaches the sink, one that raises is caught, and a backend that cannot + be selected is a line of its own; registries that did answer are still + swept. + + Ablate the `on_fault` sink (call `list_sessions()`) and the first line is + lost; ablate either `except MultiplexerError` arm and this raises.""" + + class _Folding(_RegistryMux): + def list_sessions_reporting(self, *, on_fault=None): + assert on_fault is not None + on_fault("list-sessions failed: rc 1") + return [] + + class _Raising(_RegistryMux): + def list_sessions_reporting(self, *, on_fault=None): + raise MultiplexerError("registry unreadable") + + folding = _Folding([], {}, root=str(tmp_path / "fold")) + raising = _Raising([], {}, root=str(tmp_path / "raise")) + fine = _RemovingMux(["bmad-loop-r1"], {"bmad-loop-r1": runs.project_tag(tmp_path)}) + monkeypatch.setattr(runs, "_legacy_registries", lambda: [folding, raising, fine]) + + blockers = runs.kill_displaced_session(tmp_path, "r1") + assert len(blockers) == 2 + assert str(tmp_path / "fold") in blockers[0] and "rc 1" in blockers[0] + assert str(tmp_path / "raise") in blockers[1] and "registry unreadable" in blockers[1] + assert fine.killed == ["bmad-loop-r1"] + + def boom(): + raise MultiplexerError("no backend") + + monkeypatch.setattr(runs, "_legacy_registries", boom) + assert [ + "backend selection failed: no backend" in f + for f in runs.kill_displaced_session(tmp_path, "r1") + ] == [True] + + +def test_kill_displaced_session_names_a_session_its_kill_did_not_remove(tmp_path, monkeypatch): + """The kill is best-effort and silent, and a survivor still holds psmux's + name mutex, so it is read back and named rather than reported as cleared. + + Ablate the re-listing and this returns `[]`.""" + stuck = _RegistryMux( + ["bmad-loop-r1"], {"bmad-loop-r1": runs.project_tag(tmp_path)}, root=str(tmp_path / "s") + ) + monkeypatch.setattr(runs, "_legacy_registries", lambda: [stuck]) + + blockers = runs.kill_displaced_session(tmp_path, "r1") + assert stuck.killed == ["bmad-loop-r1"] + assert blockers == [f"{tmp_path / 's'}: bmad-loop-r1 still standing after the kill"] + + +def test_kill_displaced_session_matches_through_the_registrys_name_key(tmp_path, monkeypatch): + """psmux folds session-name case, so a resume of `r1` must find a stale + `bmad-loop-R1` — its mutex blocks `bmad-loop-r1` all the same. + + Ablate the `session_name_key` comparison (match names exactly) and nothing + is killed.""" + tag = runs.project_tag(tmp_path) + folding = _RemovingMux(["bmad-loop-R1"], {"bmad-loop-R1": tag}, fold=True) + monkeypatch.setattr(runs, "_legacy_registries", lambda: [folding]) + + assert runs.kill_displaced_session(tmp_path, "r1") == [] + assert folding.killed == ["bmad-loop-r1"] + + +def test_kill_displaced_session_reports_an_uncomputable_tag(tmp_path, monkeypatch): + """Without this project's tag nothing can be proven ours, so nothing is + killed — and the resume is told why in a line of its own, which + `compose_resume` journals and warns, instead of the fault raising out of it. + Same guard `foreign_session_refusal` puts on the same call. + + Ablate the `except (OSError, RuntimeError)` arm and this raises.""" + legacy = _RemovingMux(["bmad-loop-r1"], {"bmad-loop-r1": runs.project_tag(tmp_path)}) + monkeypatch.setattr(runs, "_legacy_registries", lambda: [legacy]) + + def boom(_project): + raise OSError("state dir unreadable") + + monkeypatch.setattr(runs, "accepted_tags", boom) + + blockers = runs.kill_displaced_session(tmp_path, "r1") + assert len(blockers) == 1 + assert "tag could not be computed (state dir unreadable)" in blockers[0] + assert "no legacy registry was swept" in blockers[0] + assert legacy.killed == [] + + def test_prune_sessions_still_claims_an_untagged_session_in_the_primary_registry( tmp_path, monkeypatch ): @@ -8371,6 +8705,9 @@ def test_legacy_leftovers_dry_run_keeps_what_the_legacy_pass_cannot_claim(tmp_pa monkeypatch.setattr(runs, "get_multiplexer", lambda: ours) monkeypatch.setattr(runs, "mux_sessions", ours.list_sessions) monkeypatch.setattr(runs, "session_project_tags", lambda: {}) + # As `cli._configure_mux` leaves a process: the derived root in force AND + # the project it was derived for, which is what lets the kill gate see it. + monkeypatch.setattr(runs, "_SETTLED_PROJECT", tmp_path) # untagged over there: the run dir proves nothing in a shared registry legacy = _RegistryMux(["bmad-loop-dup"], {}) monkeypatch.setattr(runs, "_legacy_registries", lambda: [legacy]) @@ -9680,3 +10017,244 @@ def checked_save(target, state): monkeypatch.setattr(runs, "save_state", checked_save) runs.rearm_for_reverify(run_dir, project_root=spec_path.parents[2]) + + +# ------------------------------------- by-name operations in a shared registry (#729) + + +class _SharedRegistryMux: + """A namespacing backend whose registry in force is `root` — an operator's + honoured root another project may share. Only what the ownership gate and + the by-name verbs reach.""" + + def __init__(self, root, sessions, tags, *, list_fault=None, namespaced=True): + self._root = root + self._sessions = list(sessions) + self._tags = dict(tags) + self._list_fault = list_fault + self._namespaced = namespaced + self.killed: list[str] = [] + + def has_registry_namespace(self): + return self._namespaced + + def registry_root(self): + return self._root + + def session_name_key(self, name): + return name + + def list_sessions_reporting(self, *, on_fault=None): + if self._list_fault is not None: + assert on_fault is not None + on_fault(self._list_fault) + return [] + return list(self._sessions) + + def session_options(self, _option): + return dict(self._tags) + + def has_session(self, name): + return name in self._sessions + + def kill_session(self, name): + self.killed.append(name) + + +def _shared(monkeypatch, project, mux): + """Point the module-level seam at `mux` and record `project` as configured, + as `cli._configure_mux` does ahead of every command.""" + monkeypatch.setattr(runs, "get_multiplexer", lambda: mux) + monkeypatch.setattr(runs, "_SETTLED_PROJECT", project) + return mux + + +_FOREIGN_TAG = "0123456789abcdef" + + +def _foreign_r1(tmp_path): + return _SharedRegistryMux( + str(tmp_path / "shared"), ["bmad-loop-r1"], {"bmad-loop-r1": _FOREIGN_TAG} + ) + + +def _own_r1(tmp_path): + return _SharedRegistryMux( + str(tmp_path / "shared"), ["bmad-loop-r1"], {"bmad-loop-r1": runs.project_tag(tmp_path)} + ) + + +@pytest.mark.parametrize( + ("case", "refused"), + [ + ("ours", False), + ("foreign", True), + ("untagged", True), + ("absent", False), + ("listing-fault", True), + ("no-project", True), + ], +) +def test_foreign_session_refusal_in_a_shared_registry(tmp_path, monkeypatch, case, refused): + """In a registry that is not this project's derived root, a same-named + session is this project's only on its tag. "Could not ask" and "no project + configured" refuse rather than read as absent. + + Ablate the tag comparison and `foreign`/`untagged` stop refusing; ablate the + listing-fault arm and `listing-fault` stops refusing; ablate the + no-project arm and `no-project` stops refusing.""" + name = "bmad-loop-r1" + tag = {"ours": runs.project_tag(tmp_path), "foreign": _FOREIGN_TAG}.get(case) + mux = _SharedRegistryMux( + str(tmp_path / "shared"), + [] if case == "absent" else [name], + {name: tag} if tag else {}, + list_fault="list-sessions failed: rc 1" if case == "listing-fault" else None, + ) + _shared(monkeypatch, None if case == "no-project" else tmp_path, mux) + + assert (runs.foreign_session_refusal(name) is not None) is refused + + +@pytest.mark.parametrize("where", ["derived", "default", "tmux"]) +def test_foreign_session_refusal_stays_out_of_a_registry_that_proves_ownership( + tmp_path, monkeypatch, where +): + """The derived root proves ownership by construction, so no tag is read + there; psmux's own default registry (the pre-existing degrade arm) and tmux + keep their historical by-name behaviour. A foreign tag on the session makes + the point: it is never consulted. + + Ablate the `root == derived` early return and the `derived` row refuses.""" + name = "bmad-loop-r1" + root = {"derived": str(runs.mux_registry_root(tmp_path)), "default": None}.get(where) + mux = _SharedRegistryMux(root, [name], {name: _FOREIGN_TAG}, namespaced=where != "tmux") + _shared(monkeypatch, tmp_path, mux) + + assert runs.foreign_session_refusal(name) is None + + +def test_kill_session_leaves_another_projects_session_in_a_shared_registry( + tmp_path, monkeypatch, capsys +): + """The chokepoint every by-name kill routes through: resume's stale-session + drop, stop's backstop, the engine's teardowns. In a shared registry a + neighbour's tagged `bmad-loop-r1` stays, and the refusal is said. + + Ablate the gate in `kill_session` and it is killed.""" + mux = _shared(monkeypatch, tmp_path, _foreign_r1(tmp_path)) + + runs.kill_session("r1") + + assert mux.killed == [] + assert "killing bmad-loop-r1 refused" in capsys.readouterr().err + + +def test_kill_session_still_kills_its_own_session_in_a_shared_registry(tmp_path, monkeypatch): + mux = _shared(monkeypatch, tmp_path, _own_r1(tmp_path)) + + runs.kill_session("r1") + + assert mux.killed == ["bmad-loop-r1"] + + +def test_stop_run_backstop_leaves_another_projects_session(tmp_path, monkeypatch, capsys): + """Stop's backstop kill goes through the gated chokepoint, so stopping this + project's `r1` never takes a neighbour's same-named session with it.""" + mux = _shared(monkeypatch, tmp_path, _foreign_r1(tmp_path)) + run_dir = _make_state_run(tmp_path, "r1") # no engine.pid -> legacy/dead + + assert runs.stop_run(run_dir) is True + assert mux.killed == [] + assert "refused" in capsys.readouterr().err + + +def test_session_liveness_does_not_read_another_projects_session_as_alive(tmp_path, monkeypatch): + """A same-named session in a shared registry proves nothing about this run: + 'unknown', not 'alive', which would refuse this run's resume and delete + and hide it from cleanup. + + Ablate the gate in `_session_liveness` and the first assert reads 'alive'.""" + monkeypatch.setattr(runs, "mux_usable", lambda _m: True) + _shared(monkeypatch, tmp_path, _foreign_r1(tmp_path)) + assert runs._session_liveness("r1") == "unknown" + + _shared(monkeypatch, tmp_path, _own_r1(tmp_path)) + assert runs._session_liveness("r1") == "alive" + + +def test_export_records_the_project_it_configured(tmp_path, monkeypatch): + """The gate's project comes from the export, recorded even when the + derivation fails (the degrade arm still needs a project to prove a tag + against). + + Ablate the assignment in `export_psmux_registry_root` and both fail.""" + monkeypatch.delenv(runs.PSMUX_DATA_DIR, raising=False) + runs.export_psmux_registry_root(tmp_path) + assert runs._SETTLED_PROJECT == tmp_path + + other = tmp_path / "other" + monkeypatch.setattr(runs, "_SETTLED_PROJECT", None) + + def boom(_project): + raise runs.StateRootError("no state root") + + monkeypatch.setattr(runs, "mux_registry_root", boom) + assert runs.export_psmux_registry_root(other) is None + assert runs._SETTLED_PROJECT == other + + +def test_foreign_session_refusal_matches_through_the_registrys_name_key(tmp_path, monkeypatch): + """psmux folds session-name case, so a foreign-tagged `bmad-loop-R1` IS the + session a kill of `bmad-loop-r1` would reach. + + Ablate the `session_name_key` comparisons (match names exactly) and the gate + sees no such session and lets the kill through.""" + mux = _SharedRegistryMux( + str(tmp_path / "shared"), ["bmad-loop-R1"], {"bmad-loop-R1": _FOREIGN_TAG} + ) + mux.session_name_key = lambda name: name.lower() + _shared(monkeypatch, tmp_path, mux) + + assert runs.foreign_session_refusal("bmad-loop-r1") is not None + + +def test_kill_session_reports_whether_it_sent_the_kill(tmp_path, monkeypatch): + """`False` for a refused kill, which is also queued for a frontend that cannot + read stderr (the TUI's cleanup worker drains it). + + Ablate the `_REFUSED_KILLS.append` and the drain comes back empty.""" + _shared(monkeypatch, tmp_path, _foreign_r1(tmp_path)) + assert runs.kill_session("r1") is False + drained = runs.drain_refused_kills() + assert len(drained) == 1 and "tagged for another project" in drained[0] + assert runs.drain_refused_kills() == [] + + _shared(monkeypatch, tmp_path, _own_r1(tmp_path)) + assert runs.kill_session("r1") is True + + +def test_prune_sessions_does_not_count_a_refused_kill_as_removed(tmp_path, monkeypatch): + """The partition judged the session ours, then the ownership gate refused the + kill (a listing that faulted in between): cleanup must not report it as + removed while it runs on. + + Ablate the `kill_session(run_id) is not False` filter in `prune_sessions` and + `r1` is reported killed.""" + monkeypatch.setattr(runs, "prunable_sessions", lambda _p, *_a, **_k: (["r1"], [], {"r1"})) + monkeypatch.setattr(runs, "_legacy_registries", lambda: []) + monkeypatch.setattr(runs, "kill_session", lambda _run_id: False) + + assert runs.prune_sessions(tmp_path) == ([], [], set()) + + +def test_session_liveness_says_why_it_did_not_read_alive(tmp_path, monkeypatch, capsys): + """'unknown' alone cannot tell a refused probe from an absent session, so the + refusal is warned about. + + Ablate the warning in `_session_liveness` and stderr is empty.""" + monkeypatch.setattr(runs, "mux_usable", lambda _m: True) + _shared(monkeypatch, tmp_path, _foreign_r1(tmp_path)) + + assert runs._session_liveness("r1") == "unknown" + assert "reading bmad-loop-r1 as this run's refused" in capsys.readouterr().err diff --git a/tests/test_runsetup.py b/tests/test_runsetup.py index f96abcae..239df3fb 100644 --- a/tests/test_runsetup.py +++ b/tests/test_runsetup.py @@ -665,6 +665,114 @@ def test_resume_defaults_old_sweep_options_to_unrestricted(tmp_path, monkeypatch assert composed.engine.kwargs["min_severity"] is None +def test_resume_sweeps_the_displaced_registry_and_reports_what_it_could_not_ask( + tmp_path, monkeypatch, capsys +): + """The resume's stale-session sweep reaches the registries the session may + predate (psmux's name mutex spans registries, so a survivor there blocks the + new session's create), and a registry it could not ask is journalled and + warned about rather than folded into "nothing there". + + Ablate the `runs.kill_displaced_session` call in `compose_resume` and + `swept` stays empty.""" + run_dir = tmp_path / runs.RUNS_DIR / RUN_ID + run_dir.mkdir(parents=True) + state = RunState(run_id=RUN_ID, project=str(tmp_path), started_at="now", run_type="sweep") + swept = [] + monkeypatch.setattr(runs, "kill_session", lambda _run_id: None) + + def displaced(project, run_id): + swept.append((project, run_id)) + return ["C:\\old: could not be listed: boom"] + + monkeypatch.setattr(runs, "kill_displaced_session", displaced) + journal = Journal(run_dir) + + runsetup.compose_resume( + project=tmp_path, + paths=_fake_paths(tmp_path), + run_dir=run_dir, + state=state, + policy=policy_mod.loads(""), + journal=journal, + sweep_factory=lambda _trigger, *, started: None, + make_adapters=_accepting_adapters, + engine_cls=_CapturingEngine, + stories_engine_cls=_CapturingEngine, + sweep_engine_cls=_CapturingEngine, + ) + + assert swept == [(tmp_path, RUN_ID)] + details = [ + e.get("detail") + for e in journal.entries() + if e.get("kind") == "displaced-session-not-cleared" + ] + assert details == ["C:\\old: could not be listed: boom"] + assert "could not be listed: boom" in capsys.readouterr().err + + +class _SharedRegistryWithForeignR1: + """A shared (honoured, #729) registry holding another project's tagged + session under this run's name — just what the ownership gate reads.""" + + def __init__(self, root): + self._root = root + self.killed: list[str] = [] + + def has_registry_namespace(self): + return True + + def registry_root(self): + return self._root + + def session_name_key(self, name): + return name + + def list_sessions_reporting(self, *, on_fault=None): + return [runs.session_name(RUN_ID)] + + def session_options(self, _option): + return {runs.session_name(RUN_ID): "0123456789abcdef"} + + def kill_session(self, name): + self.killed.append(name) + + +def test_resume_leaves_another_projects_same_named_session_alone(tmp_path, monkeypatch, capsys): + """Resume drops a stale agent session by name. In a registry shared with + another project, `bmad-loop-` may be that project's live session, and + killing it would end its coding process: the real `runs.kill_session` gate + refuses and says so. + + Ablate the gate in `runs.kill_session` and `mux.killed` holds the + neighbour's session.""" + run_dir = tmp_path / runs.RUNS_DIR / RUN_ID + run_dir.mkdir(parents=True) + state = RunState(run_id=RUN_ID, project=str(tmp_path), started_at="now", run_type="sweep") + mux = _SharedRegistryWithForeignR1(str(tmp_path / "shared")) + monkeypatch.setattr(runs, "get_multiplexer", lambda: mux) + monkeypatch.setattr(runs, "_SETTLED_PROJECT", tmp_path) + monkeypatch.setattr(runs, "kill_displaced_session", lambda _project, _run_id: []) + + runsetup.compose_resume( + project=tmp_path, + paths=_fake_paths(tmp_path), + run_dir=run_dir, + state=state, + policy=policy_mod.loads(""), + journal=Journal(run_dir), + sweep_factory=lambda _trigger, *, started: None, + make_adapters=_accepting_adapters, + engine_cls=_CapturingEngine, + stories_engine_cls=_CapturingEngine, + sweep_engine_cls=_CapturingEngine, + ) + + assert mux.killed == [] + assert "refused" in capsys.readouterr().err + + def test_missing_sweep_options_requires_current_state_marker(tmp_path): run_dir = tmp_path / runs.RUNS_DIR / RUN_ID run_dir.mkdir(parents=True) diff --git a/tests/test_tui_app.py b/tests/test_tui_app.py index a842a766..128c8ca2 100644 --- a/tests/test_tui_app.py +++ b/tests/test_tui_app.py @@ -3426,6 +3426,43 @@ async def test_delete_unknown_pid_warns_but_does_not_block(project_tree, monkeyp assert "cannot be undone" in app.screen._warning +async def test_cleanup_says_which_kills_the_ownership_gate_refused(project, monkeypatch): + """A kill refused in a shared registry is left out of the removal count and + warned about on stderr, which Textual captures: the worker drains the refusals + and toasts each one. + + Rendered (`notifications=True`), because the refusal carries a registry path: + with markup on, `[red]` would be eaten as a style tag. + + Ablate the drain loop in `_cleanup_sessions_worker` and no toast names it; + drop its `markup=False` and the rendered path loses `[red]`.""" + from bmad_loop import runs + + def prune(_project): + runs._REFUSED_KILLS.append( + "bmad-loop-r1 in the shared registry C:\\[red]\\shared is tagged for another project" + ) + return [], [], set() + + monkeypatch.setattr(launch, "mux_available", lambda: True) + monkeypatch.setattr(runs, "prune_sessions", prune) + monkeypatch.setattr(launch, "prune_ctl_windows", lambda _p: ([], [], [])) + make_run(project.project, "20260611-100000-aaaa") + app = BmadLoopApp(project.project) + async with app.run_test(notifications=True) as pilot: + await until(pilot, lambda: isinstance(app.screen, DashboardScreen)) + await pilot.press("c") + await until(pilot, lambda: isinstance(app.screen, ConfirmModal)) + await click(pilot, await ready(pilot, "#ok")) + await until( + pilot, + lambda: any( + "session not removed" in text and "C:\\[red]\\shared" in text + for text, _severity in rendered_toasts(app) + ), + ) + + async def test_cleanup_unknown_sessions_notifies(project, monkeypatch): # cleanup still prunes 'unknown' sessions (unknown never blocks cleanup) but # must say so instead of silently killing a possibly-live engine's session. @@ -3742,6 +3779,43 @@ async def test_attach_without_agent_session_notifies(project, monkeypatch): await until(pilot, lambda: any("no live agent session" in m for m in notifications(app))) +async def test_attach_to_another_projects_session_says_why(project, monkeypatch): + """The session EXISTS, and is another project's: the attach must not land on + it, and since Textual captures stderr, `agent_session_exists`' warning never + reaches the screen — the handler says it in a toast instead of "no live + agent session". + + Ablate the refusal toast in `action_attach` and only the generic message + appears; regress the handler to plain `session_exists` and it attaches.""" + attached: list[str] = [] + monkeypatch.setattr(launch, "mux_available", lambda: True) + monkeypatch.setattr(launch, "session_exists", lambda session: True) + monkeypatch.setattr(launch, "ctl_window_id", lambda proj, run_id: None) + monkeypatch.setattr( + "bmad_loop.tui.app.runs.foreign_session_refusal", + lambda session, *_mux: ( + f"{session} in the shared registry C:\\[red]\\shared is tagged for another project" + ), + ) + make_run(project.project, "20260611-100000-aaaa") + app = BmadLoopApp(project.project) + monkeypatch.setattr(app, "_attach_to_target", lambda target, **_k: attached.append(target)) + # notifications=True mounts the toast rack, so what is asserted is what the + # operator sees: with markup on, `[red]` would be eaten as a style tag. + async with app.run_test(notifications=True) as pilot: + await until(pilot, lambda: isinstance(app.screen, DashboardScreen)) + await until(pilot, lambda: dashboard(app).selected_run_id is not None) + await pilot.press("a") + await until( + pilot, + lambda: any( + "not attaching" in text and "C:\\[red]\\shared" in text + for text, _severity in rendered_toasts(app) + ), + ) + assert attached == [] + + async def test_attach_multiplexer_error_notifies(project, monkeypatch): # attach_target_argv is a server round-trip on server-backed backends (e.g. # the external herdr adapter), so it can raise after the availability/session @@ -5695,6 +5769,37 @@ async def test_stop_run_stops_and_kills_ctl_window(project_tree, monkeypatch): assert kills == [(project_tree.project, "20260611-100000-aaaa")] +async def test_stop_run_says_when_its_backstop_kill_was_refused(project_tree, monkeypatch): + """A hard stop whose backstop kill the shared-registry ownership gate refused + leaves the session standing and warns on stderr, which Textual captures: the + stop worker drains the refusal and toasts it beside "stopped". + + Ablate the drain loop in `_stop_run_worker` and no toast names it.""" + from bmad_loop import runs + + def stop(_run_dir): + runs._REFUSED_KILLS.append("bmad-loop-x in the shared registry S is untagged") + return True + + monkeypatch.setattr(launch, "mux_available", lambda: True) + monkeypatch.setattr(data, "liveness", lambda run_dir: "alive") + monkeypatch.setattr(runs, "stop_run", stop) + monkeypatch.setattr(launch, "kill_ctl_window", lambda proj, rid: None) + make_run(project_tree.project, "20260611-100000-aaaa", alive=True) + app = BmadLoopApp(project_tree.project) + async with app.run_test() as pilot: + await until(pilot, lambda: dashboard(app).selected_run_id == "20260611-100000-aaaa") + await pilot.press("x") + await until(pilot, lambda: isinstance(app.screen, ConfirmModal)) + await click(pilot, await ready(pilot, "#ok")) + await until( + pilot, + lambda: any( + "session not removed" in m and "is untagged" in m for m in notifications(app) + ), + ) + + @pytest.mark.parametrize("live", ["dead", "unknown"]) async def test_stop_run_not_live_warns_without_calling(project, monkeypatch, live): """`x` is the hard stop: it only ever fires at a *provably alive* engine, so the diff --git a/tests/test_tui_launch.py b/tests/test_tui_launch.py index df68076a..250f60ae 100644 --- a/tests/test_tui_launch.py +++ b/tests/test_tui_launch.py @@ -762,6 +762,9 @@ def __init__(self): def has_registry_namespace(self): return True + def registry_root(self): + return os.environ.get(runs.PSMUX_DATA_DIR) # as the primary psmux instance answers + def has_session(self, name): return False @@ -1823,6 +1826,9 @@ def available(self): def has_registry_namespace(self): return True + def registry_root(self): + return os.environ.get(runs.PSMUX_DATA_DIR) # as the primary psmux instance answers + def has_session(self, session): self.sessions.append(session) return True @@ -2189,6 +2195,318 @@ def test_attach_plan_none_when_nothing_to_attach(monkeypatch): assert launch.attach_plan(Path("/proj"), "RID") is None +class _SharedRegistryWithForeignSession: + """A shared (honoured, #729) registry where `bmad-loop-RID` is another + project's tagged session.""" + + def has_registry_namespace(self): + return True + + def registry_root(self): + return "/shared-registry" + + def session_name_key(self, name): + return name + + def has_session(self, name): + return name == "bmad-loop-RID" + + def list_sessions_reporting(self, *, on_fault=None): + return ["bmad-loop-RID"] + + def session_options(self, _option): + return {"bmad-loop-RID": "0123456789abcdef"} + + +def test_attach_plan_will_not_attach_to_another_projects_session(monkeypatch, tmp_path, capsys): + """In a registry shared with another project, `bmad-loop-RID` may be that + project's live coding session; attaching the operator to it is the by-name + hazard. `agent_session_exists` reads it as absent and says why, so with no + ctl window there is nothing to attach. + + Ablate the gate in `agent_session_exists` and the plan attaches to it.""" + monkeypatch.setattr(launch, "get_multiplexer", lambda: _SharedRegistryWithForeignSession()) + monkeypatch.setattr(runs, "_SETTLED_PROJECT", tmp_path) + monkeypatch.setattr(launch, "ctl_window_id", lambda proj, rid: None) + monkeypatch.setattr(launch, "decision_pending", lambda rd: False) + + assert launch.attach_plan(tmp_path, "RID") is None + assert "treating bmad-loop-RID as absent" in capsys.readouterr().err + + +def test_session_exists_stays_a_plain_existence_check_in_a_shared_registry(monkeypatch, tmp_path): + """`session_exists` also answers for the control session, which carries no + project tag (its name is already per project), so the shared-registry + ownership gate must not reach it: gated, a prune in an operator's honoured + root read its own ctl session as absent and swept nothing. + + Ablate by moving the gate back into `session_exists` and this fails.""" + + class _UntaggedCtl(_SharedRegistryWithForeignSession): + def has_session(self, name): + return name == "ctl-under-test" + + def list_sessions_reporting(self, *, on_fault=None): + return ["ctl-under-test"] + + def session_options(self, _option): + return {} + + monkeypatch.setattr(launch, "get_multiplexer", lambda: _UntaggedCtl()) + monkeypatch.setattr(runs, "_SETTLED_PROJECT", tmp_path) + + assert launch.session_exists("ctl-under-test") is True + assert launch.agent_session_exists("ctl-under-test") is False + + +class _HonouredRegistry: + """A namespacing backend whose registry in force is `root`.""" + + def __init__(self, root): + self._root = root + + def has_registry_namespace(self): + return True + + def registry_root(self): + return self._root + + +def _write_honour_flag(project: Path, on: bool) -> None: + from bmad_loop import policy as policy_mod + + path = project / policy_mod.POLICY_FILE + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + f"[mux]\nhonor_ambient_psmux_data_dir = {'true' if on else 'false'}\n", encoding="utf-8" + ) + + +@pytest.mark.parametrize( + ("root", "flag_now", "drift"), + [ + ("pinned", False, True), # started honouring, switch turned off since + ("pinned", True, False), # still honouring + ("derived", True, False), # switch turned on since, but nothing was displaced to honour + ("derived", False, False), # unchanged + ], +) +def test_registry_drift_predicts_where_a_child_would_settle( + monkeypatch, tmp_path, root, flag_now, drift +): + """The detached child re-reads the switch from policy.toml and settles its + registry from the root it inherits (this process's). A TUI that started + honouring the operator's root and then had the switch turned off would + launch into a registry it does not watch; the other three combinations + land where the TUI looks. + + Ablate the `child == root` comparison (never refuse) and the first row + fails.""" + in_force = ( + str(tmp_path / "pinned") if root == "pinned" else str(runs.mux_registry_root(tmp_path)) + ) + _write_honour_flag(tmp_path, flag_now) + monkeypatch.setattr(runs, "_SETTLED_PROJECT", tmp_path) + + refusal = launch._registry_drift(tmp_path, _HonouredRegistry(in_force)) + + assert (refusal is not None) is drift + if drift: + assert "restart the TUI" in refusal and in_force in refusal + + +def test_registry_drift_names_an_unreadable_policy(monkeypatch, tmp_path): + """A policy.toml that cannot be read is not a switch turned off: the child + falls back to off as well, so the launch is still refused, but the refusal + names the real cause rather than claiming the switch changed — and a + restart would not help, fixing the policy would. + + Ablate the fault arm and the refusal says the switch changed.""" + from bmad_loop import policy as policy_mod + + pinned = str(tmp_path / "pinned") + path = tmp_path / policy_mod.POLICY_FILE + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("[mux\nhonor_ambient_psmux_data_dir = true\n", encoding="utf-8") + monkeypatch.setattr(runs, "_SETTLED_PROJECT", tmp_path) + + refusal = launch._registry_drift(tmp_path, _HonouredRegistry(pinned)) + + assert refusal is not None + assert refusal.startswith("policy.toml could not be read (") + assert "fix the policy, then launch" in refusal and pinned in refusal + assert "changed since this TUI started" not in refusal + + +def test_registry_drift_refuses_after_the_switch_was_turned_on(monkeypatch, tmp_path): + """The switch turned ON under a TUI that overrode the operator's root R: + its children inherit the derived root and stay there, while every shell + carrying R would now honour it — runs started from the two places would land + in two registries. The TUI cannot follow without a restart, so it refuses. + + Ablate the flip-ON arm in `_registry_drift` and this returns None.""" + from bmad_loop.adapters import psmux_backend + + derived = str(runs.mux_registry_root(tmp_path)) + theirs = str(tmp_path / "their-own-registry") + _write_honour_flag(tmp_path, True) + monkeypatch.setattr(runs, "_SETTLED_PROJECT", tmp_path) + monkeypatch.setattr(psmux_backend, "_DISPLACED_ROOT", theirs) + + refusal = launch._registry_drift(tmp_path, _HonouredRegistry(derived)) + + assert refusal is not None + assert refusal.startswith("[mux] honor_ambient_psmux_data_dir was turned on") + assert derived in refusal and theirs in refusal and "restart the TUI" in refusal + + +@pytest.mark.parametrize("displaced", ["none", "unhonourable"]) +def test_registry_drift_has_nothing_to_refuse_without_an_honourable_displaced_root( + monkeypatch, tmp_path, displaced +): + """The control: a TUI started without R in its environment displaced nothing + and cannot know R, and a displaced value the rule would not honour anyway (a + derived-registry shape) leaves every shell on the derived root too. + + Ablate the `resolve_psmux_registry_root(...) == displaced` condition and the + second row refuses.""" + from bmad_loop.adapters import psmux_backend + + derived = runs.mux_registry_root(tmp_path) + if displaced == "unhonourable": + other = tmp_path / "elsewhere" / derived.parent.name / derived.name + monkeypatch.setattr(psmux_backend, "_DISPLACED_ROOT", str(other)) + _write_honour_flag(tmp_path, True) + monkeypatch.setattr(runs, "_SETTLED_PROJECT", tmp_path) + + assert launch._registry_drift(tmp_path, _HonouredRegistry(str(derived))) is None + + +class _ParkingRegistry(_HonouredRegistry): + """A namespacing (or not) backend that records the argv each parked window + would run.""" + + def __init__(self, root, *, namespaced=True): + super().__init__(root) + self._namespaced = namespaced + self.argvs: list[list[str]] = [] + + def has_registry_namespace(self): + return self._namespaced + + def new_parked_window(self, session, name, cwd, argv, return_opt): + self.argvs.append(list(argv)) + return "@7" + + def set_window_option(self, window, option, value): + pass + + +def _park(monkeypatch, tmp_path, mux) -> list[str]: + monkeypatch.setattr(launch, "get_multiplexer", lambda: mux) + monkeypatch.setattr(launch, "mux_usable", lambda _m: True) + monkeypatch.setattr(launch, "_ensure_ctl_session", lambda _p: "ctl") + launch.start_detached(tmp_path, ["resume", "--project", str(tmp_path)], "RID", "resume") + (argv,) = mux.argvs + return argv + + +def test_start_detached_forwards_the_displaced_registry(monkeypatch, tmp_path): + """The child inherits the derived root and so displaces nothing; without + the launcher's record a TUI-launched resume or cleanup never sweeps the + operator's pre-#537 registry. Forwarded top-level, ahead of the subcommand. + + Ablate the forwarding in `start_detached` and the option is absent.""" + from bmad_loop.adapters import psmux_backend + + theirs = str(tmp_path / "their-own-registry") + monkeypatch.setattr(psmux_backend, "_DISPLACED_ROOT", theirs) + + argv = _park(monkeypatch, tmp_path, _ParkingRegistry(str(tmp_path / "derived"))) + + assert argv == launch.cli_argv( + f"--displaced-registry-root={theirs}", "resume", "--project", str(tmp_path) + ) + + +@pytest.mark.parametrize("case", ["nothing-displaced", "namespace-less", "displaced-in-force"]) +def test_start_detached_omits_it_without_a_displaced_root(monkeypatch, tmp_path, case): + """Nothing to forward — nothing displaced, a transport with no registry, or + a displaced root that is the one in force — leaves the argv byte-identical + to the one before the option existed. + + Ablate the `has_registry_namespace()` gate and the second row forwards.""" + from bmad_loop.adapters import psmux_backend + + in_force = str(tmp_path / "derived") + if case != "nothing-displaced": + displaced = in_force if case == "displaced-in-force" else str(tmp_path / "theirs") + monkeypatch.setattr(psmux_backend, "_DISPLACED_ROOT", displaced) + mux = _ParkingRegistry(in_force, namespaced=case != "namespace-less") + + argv = _park(monkeypatch, tmp_path, mux) + + assert argv == launch.cli_argv("resume", "--project", str(tmp_path)) + + +def test_start_detached_skips_a_corruptible_displaced_root(monkeypatch, tmp_path): + """A share root with whitespace keeps its trailing separator through + normalisation, and that is the shape Windows PowerShell older than 7.3 + corrupts in a parked window's argv (ADR 0001 §6). Not forwarding it is the + behaviour before the option existed; forwarding it would name a registry + nobody used. The positive control — the same share one level down — is + forwarded, so the skip is the shape and not the share. + + `isabs` is answered for these two literals so the win32 shape runs on POSIX + too. Ablate the skip and the first launch forwards.""" + from bmad_loop.adapters import psmux_backend + + share_root = r"\\srv\my share" + "\\" + below = r"\\srv\my share\registry" + real_isabs = os.path.isabs + monkeypatch.setattr(os.path, "isabs", lambda p: p in (share_root, below) or real_isabs(p)) + monkeypatch.setattr(psmux_backend, "_DISPLACED_ROOT", share_root) + + argv = _park(monkeypatch, tmp_path, _ParkingRegistry(str(tmp_path / "derived"))) + assert argv == launch.cli_argv("resume", "--project", str(tmp_path)) + + monkeypatch.setattr(psmux_backend, "_DISPLACED_ROOT", below) + argv = _park(monkeypatch, tmp_path, _ParkingRegistry(str(tmp_path / "derived"))) + assert argv[3] == f"--displaced-registry-root={below}" + + +def test_registry_drift_is_not_asked_of_an_unconfigured_process(monkeypatch, tmp_path): + """Nothing to disagree with when this process never settled a registry for + the project (library or test use): the live psmux tests drive launches + under an isolated root without a CLI entry. + + Ablate the `settled_project()` precondition and this refuses.""" + _write_honour_flag(tmp_path, False) + monkeypatch.setattr(runs, "_SETTLED_PROJECT", None) + + assert launch._registry_drift(tmp_path, _HonouredRegistry(str(tmp_path / "pinned"))) is None + + +def test_start_detached_refuses_a_launch_into_a_registry_it_does_not_watch(monkeypatch, tmp_path): + """The refusal sits at the one mutation every TUI launch converges on, ahead + of the control-session mint, and reaches the operator as a LaunchError. + + Ablate the `_registry_drift` call in `start_detached` and the ctl session is + minted.""" + minted: list[Path] = [] + _write_honour_flag(tmp_path, False) + monkeypatch.setattr(runs, "_SETTLED_PROJECT", tmp_path) + monkeypatch.setattr( + launch, "get_multiplexer", lambda: _HonouredRegistry(str(tmp_path / "pinned")) + ) + monkeypatch.setattr(launch, "mux_usable", lambda _m: True) + monkeypatch.setattr(launch, "_ensure_ctl_session", lambda p: minted.append(p) or "ctl") + + with pytest.raises(launch.LaunchError, match="restart the TUI"): + launch.start_detached(tmp_path, ["run"], "20260611-100000-aaaa", "run") + assert minted == [] + + def test_run_captured_merges_streams(monkeypatch): def fake(argv, **kwargs): assert argv[:3] == [sys.executable, "-m", "bmad_loop.cli"]