From 7f5728b3078cab47ab1330435b20ead3c2f9e1d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Davor=20Raci=C4=87?= Date: Mon, 3 Aug 2026 11:10:50 +0200 Subject: [PATCH 1/4] fix(validate): name the platform so a WSL-interop mis-pick is visible (#332) WSL appends the Windows PATH to its own, so a bash prompt can reach a Windows-installed bmad-loop. That interpreter reports win32, takes the psmux platform default and never sees the distro's tmux, while validate printed a green "multiplexer PsmuxMultiplexer available" and nothing named the platform. Selection is unchanged - psmux is the correct pick for a win32 interpreter. What changes is what the operator can see: - validate reports the multiplexer selection reason on every host, not only for a forced env/policy choice. A `fallback` reason is emitted as a warning, since its own label says no available backend matches this platform. - a win32 interpreter working on a \wsl.localhost\... project raises host.wsl-interop, naming the backend actually selected and the remedy. - a detection failure now reports under mux.backends-detected at warning instead of being swallowed. It was the one probe here that failed silently, and silence reads as "selection failed": mux.selection vanishes while mux.backend, which comes from an independent call, still prints healthy. - diagnose gains sys.platform and win32_on_wsl_path in its Environment block. The field is named for what it observes, not "wsl interop": cd into a distro path from native PowerShell reaches the same state, so neither surface may claim where the operator is standing. It is tri-state - no project in hand renders "-" rather than a fabricated "no", the answer a triager would read to rule this out - and `collect` requires the project so the verdict cannot be defaulted by omission. The project path and sys.executable stay out: the egress redactor leaves a Linux username standing in a distro path and carries the project name past the identifier gate. Detection reads the project path only. Probed on a live interop launch, WSL_DISTRO_NAME and WSL_INTEROP are absent from the child and PWD, when present, carries the Windows-side parent's value - WSL hands a Windows binary the Windows environment block, so an env marker is not merely missing but can be present and wrong. Path.resolve() preserves both bridge spellings and dereferences a mapped drive back to them, so the resolved path callers pass still matches; a caller-supplied \?\UNC\... prefix is the one shape left out. The UNC shape covers a project on the distro filesystem; one under /mnt/c gets a genuine Windows path and is covered by the selection line instead. Closes #332 --- .github/ISSUE_TEMPLATE/bug-report.yaml | 4 + CHANGELOG.md | 21 +++ docs/FEATURES.md | 1 + docs/ROADMAP.md | 2 +- docs/multiplexer-backends.md | 7 +- docs/porting-to-a-new-os.md | 31 ++-- docs/setup-guide.md | 9 +- src/bmad_loop/checks.py | 1 + src/bmad_loop/cli.py | 6 +- src/bmad_loop/diagnostics.py | 45 ++++- src/bmad_loop/documents.py | 11 +- src/bmad_loop/platform_util.py | 32 ++++ src/bmad_loop/runsetup.py | 87 ++++++++- tests/test_cli.py | 235 ++++++++++++++++++++++++- tests/test_diagnostics.py | 78 ++++++-- tests/test_platform_util.py | 65 +++++++ 16 files changed, 587 insertions(+), 48 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug-report.yaml b/.github/ISSUE_TEMPLATE/bug-report.yaml index 1be9a149..aaf83205 100644 --- a/.github/ISSUE_TEMPLATE/bug-report.yaml +++ b/.github/ISSUE_TEMPLATE/bug-report.yaml @@ -99,9 +99,13 @@ body: id: platform attributes: label: Operating System + description: On WSL, run `bmad-loop mux` and read the `selection:` line — `platform default for linux` + is the supported setup, `platform default for win32` means you are on the Windows build + and should say so below. options: - macOS - Windows + - Windows (WSL) - Linux - Other validations: diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dc168ca..4b64f503 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -152,6 +152,27 @@ whose seams had diverged enough that several ports needed a different fix, and t ### Fixed +- **A native-Windows install driven from a WSL shell now says so (#332).** WSL appends the Windows + `PATH` to its own, so a bash prompt can reach a Windows-installed `bmad-loop`: that interpreter + reports `win32`, takes the psmux platform default, and never sees the distro's tmux — while + `validate` printed a green `multiplexer PsmuxMultiplexer available` and nothing named the platform. + `validate` now reports the multiplexer selection reason for **every** host (it was emitted only for + a forced `BMAD_LOOP_MUX_BACKEND`/`[mux] backend` choice), so `platform default for win32` is on + screen wherever the mismatch happens; the same un-gating makes a `fallback` selection — no + available backend matches this platform — a warning rather than a green line. A `win32` interpreter + working on a `\\wsl.localhost\...` project additionally raises a `host.wsl-interop` **warning** + naming the fix (install with the WSL/Linux Python) and the backend it actually chose. That warning + covers the project-on-the-distro shape only; a project under `/mnt/c` gets a genuine Windows path + and no warning, and is covered by the selection line instead. Nothing changes which backend is + selected — psmux is correct for a `win32` interpreter — nor validate's exit code. `diagnose` gains + `sys.platform` and `win32 on WSL distro path` in its Environment block, the latter rendering `—` + rather than a fabricated `no` when there was no project to check against. +- **A multiplexer-detection failure is reported instead of swallowed.** `validate` caught and + discarded any exception from backend detection, so `mux.selection` and the backend inventory + vanished with nothing said — while `mux.backend` above them, which comes from an independent + selection call, still printed a healthy backend. It now reports under `mux.backends-detected` at + **warning** carrying the error. + - **A seed path naming the project root is refused at load, in every source that feeds it (#456).** A root-naming entry made `provision_worktree`'s seed loop resolve source to the repo root and destination to the worktree — both pass its containment checks — so it copied the whole project diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 666c463c..28b7e38c 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -183,6 +183,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - `bmad-loop init` installs the three `bmad-loop-*` skills (`bmad-loop-setup`, `bmad-loop-resolve`, `bmad-loop-sweep`, into `.claude/skills/` and/or `.agents/skills/`), the hook relay, `.bmad-loop/policy.toml`, and a gitignore covering the runs dir, plugin caches, and policy.toml itself (per-machine config). Flags: `--cli` (repeatable), `--no-skills`, `--force-skills`. - `bmad-loop validate` preflights every prerequisite: BMAD config, sprint-status, git, the selected terminal-multiplexer backend (listing all detected when more than one is registered), CLI binary, hook registration, and the review skills the installed dev primitive actually invokes (reporting which name it resolved) — derived from its `customize.toml` review layers (or from `step-04-review.md` on releases that name reviewers inline), so both the merged `bmad-review` topology and the standalone-hunter one validate, and configured layers naming an uninstalled skill are caught — plus its `customize.toml`. +- The preflight also **names the multiplexer selection reason wherever selection resolves** (`mux.selection`, e.g. `platform default for win32`), not only when a `BMAD_LOOP_MUX_BACKEND`/`[mux] backend` choice forced it. A `fallback` selection is reported as a warning (its own label says no available backend matches this platform); a selection that outright failed is carried by `mux.preflight`, and a detection that failed by `mux.backends-detected` at warning — so a missing `mux.selection` line is always explained by another. On top of that, `host.wsl-interop` warns when a **native-Windows interpreter is working on a `\\wsl.localhost\...` project** ([#332](https://github.com/bmad-code-org/bmad-loop/issues/332) — see [multiplexer-backends.md](multiplexer-backends.md) for why WSL can hand a bash prompt the Windows build). Both are diagnostics only: neither changes which backend is selected (psmux _is_ correct for a `win32` interpreter) and neither flips validate's exit code. `bmad-loop diagnose` carries the same two facts in its Environment block as `sys.platform` and `win32 on WSL distro path` (`yes`/`no`/`—`, the last meaning the verdict was not checked — never a fabricated `no`). - Non-invasive: drives the upstream dev primitive unmodified — there is no fork to keep in sync — and review is just a re-invocation of it on the `done` spec. Your standard BMAD install is never modified. ### Command reference diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index f4424c18..fd6a8705 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -26,7 +26,7 @@ matching seam, `ProcessHost` (`src/bmad_loop/process_host.py`): `terminate` / `f `is_alive` / `identity` (a PID-reuse guard) plus `hook_interpreter()` (so hook registration never branches on platform), registered the same way (`register_process_host`, `BMAD_LOOP_PROCESS_HOST`); `WindowsProcessHost` already ships. `bmad-loop validate` runs a -`_platform_preflight()` that reports the selected backend's readiness and names the process +`_platform_preflight(project)` that reports the selected backend's readiness and names the process host — so a new OS surfaces in preflight by registering, not by a `validate` edit. The Unity plugin's `/proc`/`/tmp`/`cp -a`/symlink primitives degrade off Linux (with `psutil` from the optional `non-linux` extra) and its pid lifecycle now delegates to `ProcessHost`; everything is diff --git a/docs/multiplexer-backends.md b/docs/multiplexer-backends.md index b3c3a6dc..fe3d4dfe 100644 --- a/docs/multiplexer-backends.md +++ b/docs/multiplexer-backends.md @@ -57,7 +57,12 @@ names carry over. It is selected automatically when available; `available()` req releases can force-kill a recycled PID during teardown, so they report unavailable and selection falls through). Native Windows is still experimental — see the [roadmap](ROADMAP.md#native-windows-multiplexer-backend) for the remaining work. WSL is -unaffected: it _is_ Linux and uses tmux. +unaffected: it _is_ Linux and uses tmux — provided bmad-loop was installed with the +distro's own Python. WSL appends the Windows `PATH` to its own, so a Windows-installed +bmad-loop is reachable from the bash prompt; that process reports `win32` and takes the +psmux default no matter how Linux the shell looks. `bmad-loop validate` names the +selection reason on every host, and warns (`host.wsl-interop`) when a `win32` interpreter +is working on a `\\wsl.localhost\...` project (#332). Two model differences matter if you port a backend or read psmux argv. psmux runs one server per session, so window ids are minted per server and the backend session-qualifies every id it diff --git a/docs/porting-to-a-new-os.md b/docs/porting-to-a-new-os.md index 706aa9ac..2552847f 100644 --- a/docs/porting-to-a-new-os.md +++ b/docs/porting-to-a-new-os.md @@ -11,12 +11,12 @@ The OS-specific work is quarantined behind four seams. Porting to a new OS is core `.py` modules or their call sites. Each seam selects its implementation by platform from a registry, with an env-var override for tests. -| # | Seam | Contract / registry | Override env var | -| --- | -------------------- | ------------------------------------------------------- | ------------------------------------------------------- | -| 1 | Terminal multiplexer | `TerminalMultiplexer` / `register_multiplexer` | `BMAD_LOOP_MUX_BACKEND` (or `bmad-loop mux set `) | -| 2 | Process lifecycle | `ProcessHost` / `register_process_host` | `BMAD_LOOP_PROCESS_HOST` | -| 3 | Hook interpreter | `ProcessHost.hook_interpreter()` | (rides on seam 2) | -| 4 | Validate preflight | `_platform_preflight()` (no new code — reads seams 1–2) | — | +| # | Seam | Contract / registry | Override env var | +| --- | -------------------- | -------------------------------------------------------------- | ------------------------------------------------------- | +| 1 | Terminal multiplexer | `TerminalMultiplexer` / `register_multiplexer` | `BMAD_LOOP_MUX_BACKEND` (or `bmad-loop mux set `) | +| 2 | Process lifecycle | `ProcessHost` / `register_process_host` | `BMAD_LOOP_PROCESS_HOST` | +| 3 | Hook interpreter | `ProcessHost.hook_interpreter()` | (rides on seam 2) | +| 4 | Validate preflight | `_platform_preflight(project)` (no new code — reads seams 1–2) | — | The **one** bundled caveat: a backend you ship _in this repo_ needs its import added to the relevant `_load_builtin_*` loader so it self-registers (one line). An @@ -260,15 +260,22 @@ A new OS overrides this on its `ProcessHost`; nothing else changes. ## Seam 4 — validate preflight -`_platform_preflight()` (`src/bmad_loop/cli.py`, called from `cmd_validate`) asks -the selected multiplexer for its `available()` / `version()` and names the selected -process host. A new OS therefore surfaces its readiness in `bmad-loop validate` -**by registering** (seams 1–2) — not by adding a `win32` block to `validate`. The -process host is named in the output so a misselection (e.g. the Windows host picked -on Linux) is visible at a glance. +`_platform_preflight(project)` (`src/bmad_loop/cli.py`, called from `cmd_validate`) +asks the selected multiplexer for its `available()` / `version()` and names the +selected process host. A new OS therefore surfaces its readiness in `bmad-loop +validate` **by registering** (seams 1–2) — not by adding a `win32` block to +`validate`. The process host is named in the output so a misselection (e.g. the +Windows host picked on Linux) is visible at a glance. There is no new code to write for this seam — it reads seams 1 and 2. +The one `sys.platform` branch that does live here is not a port seam and is not a +precedent for one: the `host.wsl-interop` check (#332) reports that the _interpreter +itself_ is the wrong build for the shell that launched it — a native-Windows +`bmad-loop` reached from a WSL prompt. No registration can express that, because +every seam is correctly selected for the interpreter that is running; what is wrong +is which interpreter the operator got. Readiness questions still register. + --- ## Helper scripts (plugins) diff --git a/docs/setup-guide.md b/docs/setup-guide.md index 265d3e9a..ca10d4d6 100644 --- a/docs/setup-guide.md +++ b/docs/setup-guide.md @@ -42,7 +42,14 @@ of the README. not yet at the Linux/macOS/WSL support tier — the remaining native-Windows work (window hosting, attach/detach, Unity cache paths) is tracked in [the roadmap](ROADMAP.md#native-windows-multiplexer-backend); the port path is in - [Porting bmad-loop to a new OS](porting-to-a-new-os.md). + [Porting bmad-loop to a new OS](porting-to-a-new-os.md). Inside WSL, install with the + **Linux** interpreter — a Windows-installed bmad-loop is reachable from the bash prompt + and silently behaves as Windows + ([why](multiplexer-backends.md#psmux-native-windows-experimental)). To check: + `bmad-loop mux` should read `platform default for linux`; if it names `win32` you are + running the Windows build, and `bmad-loop validate` says so too. (A forced choice — + `BMAD_LOOP_MUX_BACKEND` or `[mux] backend` — is reported as the reason _instead_ of the + platform, so clear it before reading that line as a platform check.) ## Installed via the BMAD-method installer? (recommended) diff --git a/src/bmad_loop/checks.py b/src/bmad_loop/checks.py index ee4bb67a..a9260f6c 100644 --- a/src/bmad_loop/checks.py +++ b/src/bmad_loop/checks.py @@ -66,6 +66,7 @@ "mux.selection", "mux.external-backend", "host.process", + "host.wsl-interop", "notify.desktop-unavailable", "skills.base", "skills.base-missing", diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 5e760a9c..3f1a1ea2 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -326,7 +326,7 @@ def cmd_validate(args: argparse.Namespace) -> int: except verify.GitError: pass - report.extend(_platform_preflight()) + report.extend(_platform_preflight(project)) # #231: notify.desktop defaults to true but only fires when a platform notifier # exists (osascript/PowerShell/notify-send). When none does, the setting is @@ -2768,7 +2768,9 @@ def cmd_diagnose(args: argparse.Namespace) -> int: return 1 pseudo = sanitize.Pseudonymizer() - diag = diagnostics.collect(run_dirs, pseudo=pseudo, cap=args.max_journal_entries) + diag = diagnostics.collect( + run_dirs, pseudo=pseudo, cap=args.max_journal_entries, project=project + ) repairs: list[tuple[str, int]] = [] fail_rules: list[str] | None = None report = "" diff --git a/src/bmad_loop/diagnostics.py b/src/bmad_loop/diagnostics.py index 1d79cdc1..221cbd24 100644 --- a/src/bmad_loop/diagnostics.py +++ b/src/bmad_loop/diagnostics.py @@ -35,6 +35,7 @@ import json import platform import re +import sys from collections import Counter from dataclasses import dataclass, field from datetime import datetime, timezone @@ -151,6 +152,18 @@ class EnvInfo: package_version: str multiplexer: str tmux_version: str | None + # `platform.system()` above says "Windows" for both a native shell and a WSL + # interop launch. `sys_platform` carries the raw token instead, so a dump can be + # matched character-for-character against validate's `platform default for {token}` + # line; `win32_on_wsl_path` is the #332 condition itself. Named for exactly what it + # observes — a win32 interpreter working a distro path — and NOT "wsl interop": + # `cd \\wsl.localhost\...` from native PowerShell reaches the same state, so this + # must not claim where the operator is standing (the matching `host.wsl-interop` + # finding is worded to the same limit). `None` means the verdict was not checked, + # which is not the same answer as `False`. Additive fields — no SCHEMA_VERSION bump; + # `--json` evolution is additive-only, see the contract note in machine.py. + sys_platform: str + win32_on_wsl_path: bool | None @dataclass @@ -240,8 +253,22 @@ class Diagnostics: # ----------------------------------------------------------------- collectors -def collect_env() -> EnvInfo: +def collect_env(project: Path | None) -> EnvInfo: + """Host facts for the dump's Environment block. + + ``project`` is used only for the #332 verdict, and is required — here *and* on + ``collect`` — so no caller can default the answer by omission: without a project + the verdict is ``None`` ("not checked"), which must stay distinguishable from a + checked ``False``. Rendering them alike is how a triager wrongly rules #332 out. + + The project *path* itself is never emitted: it is a redaction hazard — the + redactor leaves the Linux username in a ``\\\\wsl.localhost\\...\\home\\\\...`` + path standing (it compares against the *Windows* account), and ``collect_env`` + has no pseudonymizer to alias it against — so the boolean is what ships. Same + reason ``sys.executable`` is absent despite naming the exact mismatch: the venv + path carries the project name past the redactor.""" from .adapters.multiplexer import fold_version, get_multiplexer + from .platform_util import is_wsl_unc_path mux = "none" tmux_v = None @@ -265,6 +292,10 @@ def collect_env() -> EnvInfo: package_version=__version__, multiplexer=mux, tmux_version=tmux_v, + sys_platform=sys.platform, + win32_on_wsl_path=( + None if project is None else sys.platform == "win32" and is_wsl_unc_path(project) + ), ) @@ -522,6 +553,7 @@ def collect( pseudo: sanitize.Pseudonymizer, cap: int = DEFAULT_JOURNAL_CAP, generated_at: str | None = None, + project: Path | None, ) -> Diagnostics: runs: list[RunDiag] = [] for run_dir in run_dirs: @@ -533,7 +565,7 @@ def collect( schema_version=SCHEMA_VERSION, generated_at=generated_at or datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ"), tool_version=__version__, - env=collect_env(), + env=collect_env(project), runs=runs, ) @@ -627,6 +659,15 @@ def render_markdown( out.append(_fmt_kv("bmad-loop version", e.package_version)) out.append(_fmt_kv("python", e.python_version)) out.append(_fmt_kv("os", f"{e.os} {e.os_release}")) + out.append(_fmt_kv("sys.platform", e.sys_platform)) + # "—" for the unchecked verdict, the same spelling `tmux` uses below for absent: + # "no" would read as a checked negative and rule #332 out for a triager. + out.append( + _fmt_kv( + "win32 on WSL distro path", + "—" if e.win32_on_wsl_path is None else "yes" if e.win32_on_wsl_path else "no", + ) + ) out.append(_fmt_kv("multiplexer", e.multiplexer)) out.append(_fmt_kv("tmux", e.tmux_version or "—")) out.append(_fmt_kv("schema / generated", f"v{d.schema_version} @ {d.generated_at}")) diff --git a/src/bmad_loop/documents.py b/src/bmad_loop/documents.py index 83ab0079..f9a6d489 100644 --- a/src/bmad_loop/documents.py +++ b/src/bmad_loop/documents.py @@ -77,7 +77,16 @@ def validate_document( no finding at all. A check id missing from ``findings`` means "did not run", never "passed" — check ``ok`` for the verdict, not the absence of an id. - **``mux.backends-detected`` is gated on more than one registered backend**, - so a lone-tmux host carries no backend inventory. Same rule as above. + so a lone-tmux host carries no backend inventory. Same rule as above. The one + exception to its ``ok`` severity is a detection failure, which reports under + the same id at ``warning`` with an empty inventory. + - **``mux.selection`` is not gated on the reason.** It used to appear only for a + forced ``env``/``policy`` choice and now names the reason wherever selection + resolves. It is absent whenever no backend row is *selected*, which has two + causes: a forced name matching no registered backend (``mux.preflight`` + carries that failure) and ``_select`` bottoming out at its historical tmux + fallback with tmux unregistered (nothing else reports that — the inventory + simply holds no selected row). Same rule as above. ``findings`` stays flat and in emission order rather than grouped by severity: grouping would destroy the cross-severity ordering (the order the gates ran) diff --git a/src/bmad_loop/platform_util.py b/src/bmad_loop/platform_util.py index d3a0fd2e..35796a60 100644 --- a/src/bmad_loop/platform_util.py +++ b/src/bmad_loop/platform_util.py @@ -160,6 +160,38 @@ def names_tree_root(value: str | Path) -> bool: return bool(parts) and all(part.strip(" .") == "" and part != ".." for part in parts) +def is_wsl_unc_path(value: str | Path) -> bool: + """True if ``value`` addresses a WSL distro's filesystem through the Windows UNC + bridge — ``\\\\wsl.localhost\\\\...`` or its legacy ``\\\\wsl$\\\\...`` + spelling, matched case-insensitively and with either separator, since Windows + accepts ``//wsl$/...`` as readily as the backslash form. + + Used to spot a native-Windows interpreter working a distro path — the #332 + mis-pick, where WSL's appended Windows ``PATH`` lands a bash prompt on a ``win32`` + build that takes the win32 defaults and never sees the distro's tmux. + + The path is the signal because the obvious alternative does not survive: probed on + a live interop launch (Windows 11, WSL2, Ubuntu-24.04, 2026-08), ``WSL_DISTRO_NAME`` + and ``WSL_INTEROP`` were absent from the child and ``PWD``, when present at all, + carried the *Windows*-side parent's value rather than the distro cwd — WSL hands a + Windows binary the *Windows* environment block. So an env marker is not merely + missing, it can be present and wrong. Nothing pins that observation, so re-probe + before adding an env-marker check rather than assuming one would work. + + Platform-blind by design (reads no ``sys.platform``): the "is this interpreter the + wrong one" half stays at the call site. + + Callers pass a resolved path (``cli._project``), and resolution is what decides the + coverage: measured on Windows 11 / CPython 3.13, ``Path.resolve()`` leaves both + bridge spellings untouched, folds ``//wsl.localhost/...`` into the backslash form, + and dereferences a mapped drive or ``subst`` alias back to the UNC spelling — so all + of those match. It does *not* strip a ``\\\\?\\UNC\\...`` prefix the input already + carried (``ntpath.realpath`` only strips one it added itself), and that spelling is + the one shape left unmatched. It falls back to the ``mux.selection`` line, which + names the platform regardless.""" + return str(value).replace("/", "\\").lower().startswith(("\\\\wsl.localhost\\", "\\\\wsl$\\")) + + def _retry_on_sharing_violation(op: Callable[[], None]) -> None: """Run ``op``, retrying the transient Windows sharing violation a concurrent handle on the file triggers (WinError 5/32). Gated to win32 so a real POSIX diff --git a/src/bmad_loop/runsetup.py b/src/bmad_loop/runsetup.py index cc01f2af..e0feb675 100644 --- a/src/bmad_loop/runsetup.py +++ b/src/bmad_loop/runsetup.py @@ -41,7 +41,7 @@ from .checks import Finding from .journal import Journal, save_state from .model import RunState -from .platform_util import atomic_replace +from .platform_util import atomic_replace, is_wsl_unc_path from .runs import RUNS_DIR if TYPE_CHECKING: @@ -155,11 +155,14 @@ def mux_reason_label(reason: str) -> str: "policy": f"set by [mux] backend in {policy_mod.POLICY_FILE}", "platform-default": f"platform default for {sys.platform}", "first-match": "first available platform match", - "fallback": "fallback (no registered backend is available)", + # not "no registered backend is available": `_select` reaches `fallback` when no + # *available* backend matches this platform — an available backend registered for + # another platform leaves the reason here just the same. + "fallback": "fallback (no available backend matches this platform)", }.get(reason, reason) -def platform_preflight() -> list[Finding]: +def platform_preflight(project: Path) -> list[Finding]: """Probe the platform-selected seams — the terminal multiplexer and the process host — for `cmd_validate`, returning the findings in emission order. @@ -167,6 +170,11 @@ def platform_preflight() -> list[Finding]: a new OS or transport surfaces here by *registering* rather than by adding a ``sys.platform`` branch to validate. The process host is named so a misselection (e.g. the Windows host picked on Linux) is visible at a glance. + + ``project`` is read only to name the host/interpreter mismatch behind #332 — a + win32 interpreter working on a WSL UNC path. Selection is unaffected by it: for + a win32 interpreter psmux *is* the right pick, so this warns rather than + re-chooses. """ from .adapters.multiplexer import ( detect_multiplexers, @@ -211,7 +219,14 @@ def platform_preflight() -> list[Finding]: try: infos = detect_multiplexers() - except Exception: # detection is advisory; never break validate + except Exception as e: + # Advisory, so it must not abort validate — but it must not be silent either. + # Two findings below read `infos`: `mux.selection` vanishes entirely, and the + # #332 warning degrades to its no-backend wording. Without this line the report + # shows a healthy `mux.backend` (independent, from `get_multiplexer`) above a + # warning naming no backend — which reads as "selection failed" when what + # actually failed was detection. + found.append(Finding("mux.backends-detected", "warning", f"mux detection failed: {e}")) infos = [] if len(infos) > 1: # a lone tmux needs no listing; keep single-backend output stable listed = ", ".join( @@ -248,14 +263,23 @@ def platform_preflight() -> list[Finding]: ) ) chosen = next((i for i in infos if i.selected), None) - if chosen and chosen.reason in ("env", "policy"): - # detail keeps the raw enum, not mux_reason_label's prose: the label is - # wording ("set by [mux] backend in .bmad-loop/policy.toml"), the enum is - # the value MuxBackendInfo.reason actually carries. + if chosen: + # Emitted for EVERY reason, not just the forced ones (#332): the reason that + # most needs naming is `platform-default`, which is how a win32 interpreter + # silently lands on psmux. detail keeps the raw enum, not mux_reason_label's + # prose: the label is wording ("set by [mux] backend in + # .bmad-loop/policy.toml"), the enum is the value MuxBackendInfo.reason + # actually carries. + # + # Severity follows the reason. `fallback` is the one `_select` returns when no + # *available* backend matches this platform, and its label says exactly that — so + # emitting it at "ok" would print a green line whose own text contradicts it. + # It stays a warning rather than a problem because `mux.backend` above already + # carries the problem for that host; this line only names how it got there. found.append( Finding( "mux.selection", - "ok", + "warning" if chosen.reason == "fallback" else "ok", f"multiplexer selection {mux_reason_label(chosen.reason)}", {"backend": chosen.name, "reason": chosen.reason}, ) @@ -286,6 +310,51 @@ def platform_preflight() -> list[Finding]: except Exception as e: # a bad BMAD_LOOP_PROCESS_HOST must report, not crash found.append(Finding("host.process", "problem", f"process host preflight failed: {e}")) + # A `warning`, never a `problem`: nothing here is broken from this interpreter's + # point of view — the selected backend is the right win32 pick and every seam above + # reports healthy — so the verdict and the exit code must not flip. What is wrong is + # the interpreter, and only the operator can swap it. + if sys.platform == "win32" and is_wsl_unc_path(project): + # State only what the evidence supports. A win32 interpreter on a distro path + # is NOT proof of a WSL shell: `cd \\wsl.localhost\...` from native PowerShell + # reaches the same condition, and nothing here can separate the two (the interop + # env markers do not survive the boundary — see `is_wsl_unc_path`). So the + # finding reports the mismatch it can actually see and leaves the WSL remedy + # conditional, rather than telling an operator where they are standing. + # + # The backend clause names what was *actually* chosen instead of assuming psmux + # — a forced `env`/`policy` choice would otherwise contradict the `mux.selection` + # line above — and is dropped entirely when selection failed (`chosen is None`, + # e.g. a forced unknown name), since there is no backend to name and inventing + # one is the exact failure this check exists to stop. + picked = ( + f"{chosen.name} was selected and the distro's own tmux is invisible to it" + if chosen + else "the distro's own tmux is invisible to it" + ) + found.append( + Finding( + "host.wsl-interop", + "warning", + "the native-Windows build (this interpreter reports win32) is working on a " + f"WSL distro path — {picked}; if you are running from a WSL shell, install " + "bmad-loop with the WSL/Linux Python instead", + # `project` is deliberately NOT carried here. `validate --json` is not a + # sanitized surface, and a distro path ends in the *Linux* username, + # which the egress redactor does not know (it compares against the + # Windows account). The caller passed the path in; it needs no echo. + # + # `selection_resolved` because a null backend has two causes a consumer + # would otherwise conflate: selection failed, or detection did (which + # `mux.backends-detected` reports at `warning`). + { + "backend": chosen.name if chosen else None, + "platform": sys.platform, + "selection_resolved": chosen is not None, + }, + ) + ) + return found diff --git a/tests/test_cli.py b/tests/test_cli.py index a2db599b..7db9c446 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -4,6 +4,7 @@ import io import json import sys +from pathlib import Path import pytest import yaml @@ -3427,10 +3428,13 @@ def _patch_preflight(monkeypatch, backend): monkeypatch.setattr(ph_mod, "get_process_host", lambda: _FakeHost()) -def _preflight_notes_problems(): +def _preflight_notes_problems(project=Path("C:/p")): """The preflight's pre-#205 (notes, problems) shape, rebuilt from its findings — - these tests assert the *seam probing*, which the Finding refactor did not change.""" - found = cli._platform_preflight() + these tests assert the *seam probing*, which the Finding refactor did not change. + + The default project is a plain drive path so the #332 WSL check stays quiet; + the tests that care pass their own.""" + found = cli._platform_preflight(project) return ( [f.message for f in found if f.severity != "problem"], [f.message for f in found if f.severity == "problem"], @@ -3970,7 +3974,10 @@ def _make_validate_pass(project, monkeypatch, capsys, *, policy=CLAUDE_ONLY_POLI monkeypatch.setattr( cli, "_platform_preflight", - lambda: [cli.Finding("mux.backend", "ok", "multiplexer TmuxBackend available (tmux 3.4)")], + # arity follows runsetup.platform_preflight, which takes the project (#332) + lambda _project: [ + cli.Finding("mux.backend", "ok", "multiplexer TmuxBackend available (tmux 3.4)") + ], ) capsys.readouterr() # drop `init`'s chatter — the next read must see only the document @@ -4213,7 +4220,7 @@ def test_validate_json_mux_detail_keeps_the_rows_the_text_flattens(mux_registry, ) mux_registry.register_multiplexer("beta", lambda p: False, lambda: _MuxStub(avail=False)) - found = cli._platform_preflight() + found = cli._platform_preflight(Path("C:/p")) listing = next(f for f in found if f.check == "mux.backends-detected") # unchanged text assert "alpha*" in listing.message and "beta (unavailable)" in listing.message @@ -4239,7 +4246,7 @@ def test_platform_preflight_selection_detail_keeps_the_raw_reason(mux_registry, monkeypatch.setenv("BMAD_LOOP_MUX_BACKEND", "alpha") mux_registry.get_multiplexer.cache_clear() - selection = next(f for f in cli._platform_preflight() if f.check == "mux.selection") + selection = next(f for f in cli._platform_preflight(Path("C:/p")) if f.check == "mux.selection") assert selection.detail == {"backend": "alpha", "reason": "env"} assert "forced by BMAD_LOOP_MUX_BACKEND" in selection.message # prose stays in the message @@ -4258,7 +4265,9 @@ def test_external_backend_failure_is_a_warning_not_a_note(mux_registry, monkeypa monkeypatch.setattr(mux_registry, "_EXTERNAL_ERRORS", {"brokenmux": "ImportError: no ghost"}) monkeypatch.setattr(mux_registry, "_EXTERNALS_LOADED", True) # no rescan over the stub - finding = next(f for f in cli._platform_preflight() if f.check == "mux.external-backend") + finding = next( + f for f in cli._platform_preflight(Path("C:/p")) if f.check == "mux.external-backend" + ) assert finding.severity == "warning" assert finding.detail == {"entry_point": "brokenmux", "error": "ImportError: no ghost"} @@ -4532,7 +4541,7 @@ def test_platform_preflight_folds_a_multi_line_version(mux_registry): lambda: _MuxStub(avail=True, version="alpha 1.2\nextra build line"), ) - finding = next(f for f in cli._platform_preflight() if f.check == "mux.backend") + finding = next(f for f in cli._platform_preflight(Path("p")) if f.check == "mux.backend") assert finding.detail["version"] == "alpha 1.2; extra build line" assert "\n" not in finding.message @@ -4550,7 +4559,7 @@ def test_platform_preflight_bounds_an_over_long_version(mux_registry): lambda: _MuxStub(avail=True, version="alpha 1.2 " + "x" * 300), ) - finding = next(f for f in cli._platform_preflight() if f.check == "mux.backend") + finding = next(f for f in cli._platform_preflight(Path("p")) if f.check == "mux.backend") assert len(finding.detail["version"]) == VERSION_MAX_CHARS assert finding.detail["version"].endswith("…") @@ -4679,6 +4688,214 @@ def test_platform_preflight_notes_forced_selection_provenance(mux_registry, monk assert any("forced by BMAD_LOOP_MUX_BACKEND" in n for n in notes) +# ------------------------------------ native-Windows-from-WSL visibility (#332) +# +# A WSL shell reaches a native-Windows bmad-loop through interop (WSL appends the +# Windows PATH to its own). That interpreter reports win32, takes the psmux +# platform default, and never sees the distro's tmux — while the operator is +# looking at a bash prompt. Selection is right for what the interpreter is; the +# bug is that nothing named the platform. These pin the naming. + +WSL_UNC_PROJECT = Path("\\\\wsl.localhost\\Ubuntu-24.04\\home\\u\\p") + + +@pytest.fixture +def fake_platform(monkeypatch): + """Patch `sys.platform` for a preflight call and un-poison what that call caches. + + The patch is *process-wide* — spelled `sys` deliberately, because `cli.runsetup.sys` + is the same module object and the qualified spelling would imply a module-local + scope that does not exist. Every `sys.platform` reader on the path under test sees + it: `mux_reason_label`'s label, `_PLATFORM_DEFAULTS`, and the process host's + `matches()`. + + `platform_preflight` reaches `get_process_host()`, which is `lru_cache(maxsize=1)` + and selects on `sys.platform`. Patching the platform without clearing the cache + leaves the *Windows* process host cached for every later test in the worker — + silently, and worst on a POSIX CI worker. (`get_multiplexer` has the same shape; + the `mux_registry` fixture already clears it.) Clearing on the way in and on the + way out keeps the poisoning inside one test in both directions. Within the test, + constructing `WindowsProcessHost` on a POSIX worker is harmless: a raise becomes a + `host.process` problem finding, which no assertion here reads.""" + from bmad_loop.process_host import get_process_host + + def _set(platform_name: str) -> None: + get_process_host.cache_clear() + monkeypatch.setattr(sys, "platform", platform_name) + + yield _set + get_process_host.cache_clear() + + +def test_platform_preflight_names_the_platform_default_selection(mux_registry): + """The reason that most needs naming is `platform-default` — it is how a win32 + interpreter silently lands on psmux — and it used to be the one the gate dropped.""" + default = mux_registry._PLATFORM_DEFAULTS.get(sys.platform, mux_registry._DEFAULT_BACKEND) + mux_registry.register_multiplexer(default, lambda p: True, lambda: _MuxStub(avail=True)) + mux_registry.get_multiplexer.cache_clear() + + selection = next(f for f in cli._platform_preflight(Path("C:/p")) if f.check == "mux.selection") + assert selection.severity == "ok" + assert selection.detail == {"backend": default, "reason": "platform-default"} + assert f"platform default for {sys.platform}" in selection.message + + +def test_platform_preflight_selection_warns_when_the_reason_is_fallback(mux_registry): + """`fallback` is what `_select` returns when no available backend matches this + platform, and the label says so — so un-gating this finding must not print that + sentence at `ok`.""" + mux_registry.register_multiplexer("alpha", lambda p: True, lambda: _MuxStub(avail=False)) + mux_registry.get_multiplexer.cache_clear() + + selection = next(f for f in cli._platform_preflight(Path("C:/p")) if f.check == "mux.selection") + assert selection.detail == {"backend": "alpha", "reason": "fallback"} + assert selection.severity == "warning" + assert "no available backend matches this platform" in selection.message + + +def test_wsl_interop_warns_when_a_win32_interpreter_works_a_distro_path( + mux_registry, fake_platform +): + mux_registry.register_multiplexer("psmux", lambda p: True, lambda: _MuxStub(avail=True)) + mux_registry.get_multiplexer.cache_clear() + fake_platform("win32") + finding = next( + f for f in cli._platform_preflight(WSL_UNC_PROJECT) if f.check == "host.wsl-interop" + ) + assert "WSL/Linux Python" in finding.message + assert "psmux was selected" in finding.message + # The evidence is `win32` + a distro path. That is NOT proof of a WSL shell — + # `cd \\wsl.localhost\...` from native PowerShell reaches the same condition — so + # the finding must not assert where the operator is standing. Exactly one mention, + # and it is the conditional remedy rather than a claim about the session. + assert finding.message.count("WSL shell") == 1 + assert "if you are running from a WSL shell" in finding.message + # The project path must not ride along: `validate --json` is unsanitized and a + # distro path ends in the Linux username. + assert finding.detail == { + "backend": "psmux", + "platform": "win32", + "selection_resolved": True, + } + assert str(WSL_UNC_PROJECT) not in str(finding.detail) + finding.message + + +def test_wsl_interop_names_the_backend_actually_selected(mux_registry, fake_platform, monkeypatch): + """A forced choice must not be described as psmux — the `mux.selection` line in the + same report would say otherwise, in the one check whose point is to stop misleading.""" + mux_registry.register_multiplexer("herdr", lambda p: False, lambda: _MuxStub(avail=True)) + monkeypatch.setenv("BMAD_LOOP_MUX_BACKEND", "herdr") + mux_registry.get_multiplexer.cache_clear() + fake_platform("win32") + findings = cli._platform_preflight(WSL_UNC_PROJECT) + warning = next(f for f in findings if f.check == "host.wsl-interop") + selection = next(f for f in findings if f.check == "mux.selection") + assert warning.detail == {"backend": "herdr", "platform": "win32", "selection_resolved": True} + assert "herdr was selected" in warning.message + assert "psmux" not in warning.message + assert selection.detail == {"backend": "herdr", "reason": "env"} + + +def test_wsl_interop_names_no_backend_when_selection_failed( + mux_registry, fake_platform, monkeypatch +): + """A forced unknown name makes `_select` raise, so no row is selected. The warning + must then name no backend at all — inventing one ("the win32 default") is the exact + confident-wrong output this check exists to remove — and `mux.selection` is absent + because there is nothing to name; `mux.preflight` carries that failure instead.""" + monkeypatch.setenv("BMAD_LOOP_MUX_BACKEND", "nosuchbackend") + mux_registry.get_multiplexer.cache_clear() + fake_platform("win32") + findings = cli._platform_preflight(WSL_UNC_PROJECT) + + warning = next(f for f in findings if f.check == "host.wsl-interop") + # `selection_resolved` because a null backend has a second cause a consumer would + # otherwise conflate with this one: detection raising (`mux.backends-detected`). + assert warning.detail == { + "backend": None, + "platform": "win32", + "selection_resolved": False, + } + assert "was selected" not in warning.message + assert "win32 default" not in warning.message + assert not [f for f in findings if f.check == "mux.selection"] + assert [f for f in findings if f.check == "mux.preflight"] + + +def test_wsl_interop_is_a_warning_so_the_validate_verdict_never_flips(mux_registry, fake_platform): + """Severity asserted through the real preflight, not a hand-built Finding: every + seam is healthy from this interpreter's point of view, so only the operator's + choice of interpreter is wrong and the exit code must not move.""" + from bmad_loop.checks import ValidationReport + + mux_registry.register_multiplexer("psmux", lambda p: True, lambda: _MuxStub(avail=True)) + mux_registry.get_multiplexer.cache_clear() + fake_platform("win32") + findings = cli._platform_preflight(WSL_UNC_PROJECT) + assert next(f for f in findings if f.check == "host.wsl-interop").severity == "warning" + + report = ValidationReport() + report.extend(findings) + assert report.passed is True + + +@pytest.mark.parametrize( + ("platform_name", "project"), + [ + # win32 on a real Windows path: psmux works, cwd is genuinely Windows. + ("win32", Path("C:/p")), + # the WSL/Linux interpreter on the same distro path — the correct install. + ("linux", WSL_UNC_PROJECT), + ("linux", Path("/home/u/p")), + # UNC, but somebody else's file server: the predicate matches wsl only. + ("win32", Path("\\\\fileserver\\share\\p")), + ], +) +def test_wsl_interop_stays_silent_off_the_interop_shape( + mux_registry, fake_platform, platform_name, project +): + fake_platform(platform_name) + assert not [f for f in cli._platform_preflight(project) if f.check == "host.wsl-interop"] + + +def test_wsl_interop_survives_the_render_and_the_json_projection(mux_registry, fake_platform): + """The other tests read the finding straight off `_platform_preflight`, which cannot + catch it being dropped between there and a surface the operator actually sees — and + every validate-rendering test stubs the preflight out wholesale.""" + mux_registry.register_multiplexer("psmux", lambda p: True, lambda: _MuxStub(avail=True)) + mux_registry.get_multiplexer.cache_clear() + fake_platform("win32") + + report = cli.ValidationReport() + report.extend(cli._platform_preflight(WSL_UNC_PROJECT)) # asserts the id is registered + doc = cli.validate_document(report, False, "") + + rows = [f for f in doc["findings"] if f["check"] == "host.wsl-interop"] + assert len(rows) == 1 + assert rows[0]["severity"] == "warning" + assert rows[0]["detail"]["selection_resolved"] is True + assert doc["ok"] is True # a warning never moves the verdict + assert str(WSL_UNC_PROJECT) not in json.dumps(doc) # unsanitized surface + + +def test_mux_detection_failure_reports_instead_of_vanishing(mux_registry, monkeypatch): + """Detection is advisory, so it must not abort validate — but silence here reads as + "selection failed": `mux.selection` disappears while `mux.backend` (independent, from + `get_multiplexer`) still names a healthy backend right above it.""" + mux_registry.register_multiplexer("alpha", lambda p: True, lambda: _MuxStub(avail=True)) + mux_registry.get_multiplexer.cache_clear() + monkeypatch.setattr( + mux_registry, "detect_multiplexers", lambda: (_ for _ in ()).throw(RuntimeError("boom")) + ) + + findings = cli._platform_preflight(Path("C:/p")) + detected = next(f for f in findings if f.check == "mux.backends-detected") + assert detected.severity == "warning" + assert "boom" in detected.message + assert not [f for f in findings if f.check == "mux.selection"] # the vanishing it explains + assert [f for f in findings if f.check == "mux.backend"] # selection itself was fine + + # ---------------------------------------- bmad-loop confirm (#335 part 3, #356) # # The exit from `awaiting-operator`. Out of band by construction: the run that diff --git a/tests/test_diagnostics.py b/tests/test_diagnostics.py index af17a70f..1f06846b 100644 --- a/tests/test_diagnostics.py +++ b/tests/test_diagnostics.py @@ -9,6 +9,8 @@ import json import re +import sys +from pathlib import Path import pytest @@ -170,7 +172,9 @@ def _seed_run(root, run_id="20260627-120000-aaaa", *, extra_journal=None, sweeps def _render_all(run_dirs): pseudo = sanitize.Pseudonymizer() - diag = diagnostics.collect(run_dirs, pseudo=pseudo) + # project=None: these tests are about run payloads, not the #332 verdict, and the + # parameter is required precisely so "not checked" is stated rather than defaulted. + diag = diagnostics.collect(run_dirs, pseudo=pseudo, project=None) md = diagnostics.render_markdown(diag, pseudo=pseudo) js = diagnostics.render_json(diag, pseudo=pseudo) return diag, pseudo, md + "\n" + js @@ -196,6 +200,59 @@ def test_known_safe_values_survive(project): assert "input_tokens" in combined # token count keys survive +def test_env_names_the_platform_and_the_win32_on_wsl_path_verdict(project, monkeypatch): + """#332: `platform.system()` says "Windows" for both a native shell and a WSL + interop launch, so the raw sys.platform token plus the verdict are what explain + which backend default the run took.""" + from bmad_loop.adapters.multiplexer import get_multiplexer + + # `collect_env` reaches `get_multiplexer()`, an lru_cache(maxsize=1) that selects + # on `sys.platform`; without these clears the patched window caches the Windows + # pick for every later test in the worker. + get_multiplexer.cache_clear() + monkeypatch.setattr(diagnostics.sys, "platform", "win32") + pseudo = sanitize.Pseudonymizer() + unc = Path("\\\\wsl.localhost\\Ubuntu-24.04\\home\\u\\p") + diag = diagnostics.collect([_seed_run(project.project)], pseudo=pseudo, project=unc) + monkeypatch.undo() + get_multiplexer.cache_clear() + assert diag.env.sys_platform == "win32" + assert diag.env.win32_on_wsl_path is True + md = diagnostics.render_markdown(diag, pseudo=pseudo) + # the rendered *value*, not just the label: asserting the label alone passes + # equally for the "no" verdict, which is the answer this test exists to reject. + assert "**sys.platform:** win32" in md + assert "**win32 on WSL distro path:** yes" in md + # The label must not claim a WSL *shell*: `cd \\wsl.localhost\...` from native + # PowerShell reaches this same state, and the sibling `host.wsl-interop` finding is + # worded to that limit — the two surfaces must make the same claim. + assert "wsl interop" not in md.lower() + # the boolean ships; the path it was derived from never does — the redactor + # leaves the Linux username in a \\wsl.localhost\...\home\ path standing. + assert str(unc) not in md + assert sanitize.assert_no_leak(diagnostics.render_json(diag, pseudo=pseudo)) == [] + + +def test_env_win32_on_wsl_path_is_false_for_a_plain_project(project): + """Both new fields render for every host, not only the interop one.""" + pseudo = sanitize.Pseudonymizer() + diag = diagnostics.collect([_seed_run(project.project)], pseudo=pseudo, project=project.project) + assert diag.env.win32_on_wsl_path is False + assert diag.env.sys_platform == sys.platform + assert "**win32 on WSL distro path:** no" in diagnostics.render_markdown(diag, pseudo=pseudo) + + +def test_env_unchecked_verdict_renders_as_absent_not_as_a_negative(project): + """`None` (no project in hand) must not render "no": that is the sentence a triager + reads to rule #332 *out*, and it would be a fabricated negative.""" + pseudo = sanitize.Pseudonymizer() + diag = diagnostics.collect([_seed_run(project.project)], pseudo=pseudo, project=None) + assert diag.env.win32_on_wsl_path is None + md = diagnostics.render_markdown(diag, pseudo=pseudo) + assert "**win32 on WSL distro path:** —" in md + assert "**win32 on WSL distro path:** no" not in md + + def test_pseudonymization_is_stable_and_correlates(project): run_dir = _seed_run(project.project) diag, _pseudo, combined = _render_all([run_dir]) @@ -335,7 +392,7 @@ def test_all_runs_scope(project): def test_legend_reverses_locally_but_never_ships(project): run_dir = _seed_run(project.project) pseudo = sanitize.Pseudonymizer() - diag = diagnostics.collect([run_dir], pseudo=pseudo) + diag = diagnostics.collect([run_dir], pseudo=pseudo, project=None) combined = diagnostics.render_markdown(diag, pseudo=pseudo) + diagnostics.render_json( diag, pseudo=pseudo ) @@ -352,7 +409,7 @@ def test_unreadable_run_does_not_crash(project): run_dir.mkdir(parents=True) (run_dir / "state.json").write_text("{ this is not valid json") pseudo = sanitize.Pseudonymizer() - diag = diagnostics.collect([run_dir], pseudo=pseudo) + diag = diagnostics.collect([run_dir], pseudo=pseudo, project=None) assert len(diag.runs) == 1 assert diag.runs[0].warnings # flagged as unreadable # still renders without raising @@ -375,7 +432,7 @@ def _seed_routing_gap(project): def test_routing_gap_is_repaired_end_to_end(project): run_dir = _seed_routing_gap(project) pseudo = sanitize.Pseudonymizer() - diag = diagnostics.collect([run_dir], pseudo=pseudo) + diag = diagnostics.collect([run_dir], pseudo=pseudo, project=None) reps: list[tuple[str, int]] = [] js = diagnostics.render_json(diag, pseudo=pseudo, repairs=reps) # must not raise alias = next(a for ns, orig, a in pseudo.entries() if orig == STORY_KEY) @@ -394,7 +451,8 @@ def test_render_json_keys_are_sorted(project): round-trip cannot detect the flag being dropped.""" run_dir = _seed_run(project.project) pseudo = sanitize.Pseudonymizer() - js = diagnostics.render_json(diagnostics.collect([run_dir], pseudo=pseudo), pseudo=pseudo) + diag = diagnostics.collect([run_dir], pseudo=pseudo, project=None) + js = diagnostics.render_json(diag, pseudo=pseudo) def hook(pairs): keys = [k for k, _ in pairs] @@ -409,7 +467,7 @@ def test_no_repairs_on_fully_routed_run(project): silently normalize a new per-field routing gap (CI keeps catching them).""" run_dir = _seed_run(project.project) pseudo = sanitize.Pseudonymizer() - diag = diagnostics.collect([run_dir], pseudo=pseudo) + diag = diagnostics.collect([run_dir], pseudo=pseudo, project=None) reps: list[tuple[str, int]] = [] md = diagnostics.render_markdown(diag, pseudo=pseudo, repairs=reps) js = diagnostics.render_json(diag, pseudo=pseudo, repairs=reps) @@ -429,7 +487,7 @@ def test_repair_note_is_inside_verified_bytes(project): """The disclosure appended after repair is itself covered by the self-check.""" run_dir = _seed_routing_gap(project) pseudo = sanitize.Pseudonymizer() - diag = diagnostics.collect([run_dir], pseudo=pseudo) + diag = diagnostics.collect([run_dir], pseudo=pseudo, project=None) js = diagnostics.render_json(diag, pseudo=pseudo) extras = [(orig, f"{ns}:{alias}") for ns, orig, alias in pseudo.entries()] assert sanitize.assert_no_leak(js, extra=extras) == [] @@ -494,7 +552,7 @@ def version(self): monkeypatch.setattr(mux_mod, "get_multiplexer", lambda: _TwoLineMux()) - env = diagnostics.collect_env() + env = diagnostics.collect_env(Path("p")) assert env.tmux_version == "tmux 3.3.7; psmux 3.3.7" assert env.multiplexer == "_TwoLineMux" @@ -511,7 +569,7 @@ def version(self): monkeypatch.setattr(mux_mod, "get_multiplexer", lambda: _ChattyMux()) - env = diagnostics.collect_env() + env = diagnostics.collect_env(Path("p")) assert env.tmux_version is not None assert len(env.tmux_version) == mux_mod.VERSION_MAX_CHARS assert "\n" not in env.tmux_version @@ -545,7 +603,7 @@ def version(self): monkeypatch.setattr(mux_mod, "get_multiplexer", lambda: _HomeyMux()) - env = diagnostics.collect_env() + env = diagnostics.collect_env(Path("p")) assert env.tmux_version is not None assert env.tmux_version.endswith("…") # the cut really fired assert home not in env.tmux_version diff --git a/tests/test_platform_util.py b/tests/test_platform_util.py index fd954ac0..e4d72da6 100644 --- a/tests/test_platform_util.py +++ b/tests/test_platform_util.py @@ -11,6 +11,7 @@ import stat import subprocess import sys +from pathlib import Path import pytest @@ -103,6 +104,26 @@ def test_names_tree_root_catches_the_win32_trim_aliases(value): assert platform_util.names_tree_root(value) is True +# --------------------------------------------------------------- is_wsl_unc_path + + +@pytest.mark.parametrize( + "value", + [ + "\\\\wsl.localhost\\Ubuntu-24.04\\home\\u\\p", + "\\\\wsl$\\Ubuntu\\home\\u\\p", # legacy prefix, still minted by older Windows + "\\\\WSL.LOCALHOST\\Ubuntu\\home\\u", # UNC hosts are case-insensitive + "\\\\wsl$\\Ubuntu", # distro root, no further path + "\\\\wsl.localhost\\Ubuntu\\", # trailing separator, no path component + "//wsl.localhost/Ubuntu/home/u/p", # Windows accepts either separator + "\\\\wsl.localhost/Ubuntu\\home", # mixed separators + Path("\\\\wsl.localhost\\Ubuntu\\home\\u\\p"), # a Path, not just a str + ], +) +def test_is_wsl_unc_path_matches_the_interop_bridge(value): + assert platform_util.is_wsl_unc_path(value) is True + + @pytest.mark.parametrize( "value", [ @@ -132,6 +153,50 @@ def test_names_tree_root_accepts_anything_naming_a_child(value): assert platform_util.names_tree_root(value) is False +@pytest.mark.parametrize( + "value", + [ + "C:\\projects\\p", # a real Windows drive path — psmux is right for it + "C:/projects/p", + "/home/u/p", # the same project seen from inside the distro + "\\\\fileserver\\share\\p", # UNC, but not WSL: must not warn + "\\\\wslfoo\\share\\p", # host merely *starts* with wsl + "\\\\wsl.localhost", # no distro component at all + "wsl.localhost\\Ubuntu\\home", # not a UNC path + "", + ], +) +def test_is_wsl_unc_path_ignores_everything_else(value): + assert platform_util.is_wsl_unc_path(value) is False + + +def test_is_wsl_unc_path_is_platform_blind(monkeypatch): + """The predicate answers "is this path inside a distro", never "am I on Windows" + — the sys.platform half lives at the runsetup call site. Pinned so the two halves + stay separable and this truth table needs no platform monkeypatching.""" + monkeypatch.setattr(sys, "platform", "linux") + assert platform_util.is_wsl_unc_path("\\\\wsl.localhost\\Ubuntu\\home\\u") is True + + +@pytest.mark.skipif(sys.platform != "win32", reason="UNC resolution is a Windows behavior") +@pytest.mark.parametrize( + "spelling", + [ + "\\\\wsl.localhost\\{d}\\home", + "\\\\wsl$\\{d}\\home", + "//wsl.localhost/{d}/home", + ], +) +def test_is_wsl_unc_path_survives_the_resolve_the_caller_applies(spelling): + """The one shape production actually sees. `cli._project` hands the preflight a + `Path(...).resolve()`, and every other test here passes an unresolved literal — so + a `Path.resolve()` semantics change (it has moved across 3.6/3.8/3.13) could + normalize the bridge prefix away and silently disable the whole #332 check with + this file still green. The distro need not exist: resolution is non-strict.""" + raw = spelling.format(d="Ubuntu-24.04") + assert platform_util.is_wsl_unc_path(Path(raw).resolve()) is True + + # ---------------------------------------------------------------- atomic_replace From 7a9f2e76657b515332d9db0b0edf21ef32450c2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Davor=20Raci=C4=87?= Date: Fri, 7 Aug 2026 17:58:09 +0200 Subject: [PATCH 2/4] fix(validate): adopt the observable win32-on-wsl-path contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check id host.wsl-interop claimed more than the evidence supports — a native PowerShell session on a \wsl.localhost\ path satisfies the same predicate without any WSL interop — so the id, the docs and the diagnose fields now all state only the observable mismatch: a win32 interpreter working a WSL distro path. The corrected fallback selection label stays. Also lands the remaining review patches: the extended-length \?\UNC\wsl.localhost\ spelling no longer slips past the predicate (with a resolve-survival row pinning the realpath premise), diagnose's verdict is a plain bool behind a required project parameter, the information-free selection_resolved detail key is gone, and the preflight commentary plus its test cluster are condensed without losing coverage. Closes #332 --- CHANGELOG.md | 5 +- docs/FEATURES.md | 2 +- docs/multiplexer-backends.md | 2 +- docs/porting-to-a-new-os.md | 2 +- src/bmad_loop/checks.py | 2 +- src/bmad_loop/diagnostics.py | 33 ++++------ src/bmad_loop/platform_util.py | 13 ++-- src/bmad_loop/runsetup.py | 43 +++++-------- tests/test_cli.py | 109 ++++++++++----------------------- tests/test_diagnostics.py | 42 +++++-------- tests/test_platform_util.py | 12 ++++ 11 files changed, 103 insertions(+), 162 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b64f503..32d4ec87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -160,13 +160,12 @@ whose seams had diverged enough that several ports needed a different fix, and t a forced `BMAD_LOOP_MUX_BACKEND`/`[mux] backend` choice), so `platform default for win32` is on screen wherever the mismatch happens; the same un-gating makes a `fallback` selection — no available backend matches this platform — a warning rather than a green line. A `win32` interpreter - working on a `\\wsl.localhost\...` project additionally raises a `host.wsl-interop` **warning** + working on a `\\wsl.localhost\...` project additionally raises a `host.win32-on-wsl-path` **warning** naming the fix (install with the WSL/Linux Python) and the backend it actually chose. That warning covers the project-on-the-distro shape only; a project under `/mnt/c` gets a genuine Windows path and no warning, and is covered by the selection line instead. Nothing changes which backend is selected — psmux is correct for a `win32` interpreter — nor validate's exit code. `diagnose` gains - `sys.platform` and `win32 on WSL distro path` in its Environment block, the latter rendering `—` - rather than a fabricated `no` when there was no project to check against. + `sys.platform` and `win32 on WSL distro path` (`yes`/`no`) in its Environment block. - **A multiplexer-detection failure is reported instead of swallowed.** `validate` caught and discarded any exception from backend detection, so `mux.selection` and the backend inventory vanished with nothing said — while `mux.backend` above them, which comes from an independent diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 28b7e38c..61b786f7 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -183,7 +183,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - `bmad-loop init` installs the three `bmad-loop-*` skills (`bmad-loop-setup`, `bmad-loop-resolve`, `bmad-loop-sweep`, into `.claude/skills/` and/or `.agents/skills/`), the hook relay, `.bmad-loop/policy.toml`, and a gitignore covering the runs dir, plugin caches, and policy.toml itself (per-machine config). Flags: `--cli` (repeatable), `--no-skills`, `--force-skills`. - `bmad-loop validate` preflights every prerequisite: BMAD config, sprint-status, git, the selected terminal-multiplexer backend (listing all detected when more than one is registered), CLI binary, hook registration, and the review skills the installed dev primitive actually invokes (reporting which name it resolved) — derived from its `customize.toml` review layers (or from `step-04-review.md` on releases that name reviewers inline), so both the merged `bmad-review` topology and the standalone-hunter one validate, and configured layers naming an uninstalled skill are caught — plus its `customize.toml`. -- The preflight also **names the multiplexer selection reason wherever selection resolves** (`mux.selection`, e.g. `platform default for win32`), not only when a `BMAD_LOOP_MUX_BACKEND`/`[mux] backend` choice forced it. A `fallback` selection is reported as a warning (its own label says no available backend matches this platform); a selection that outright failed is carried by `mux.preflight`, and a detection that failed by `mux.backends-detected` at warning — so a missing `mux.selection` line is always explained by another. On top of that, `host.wsl-interop` warns when a **native-Windows interpreter is working on a `\\wsl.localhost\...` project** ([#332](https://github.com/bmad-code-org/bmad-loop/issues/332) — see [multiplexer-backends.md](multiplexer-backends.md) for why WSL can hand a bash prompt the Windows build). Both are diagnostics only: neither changes which backend is selected (psmux _is_ correct for a `win32` interpreter) and neither flips validate's exit code. `bmad-loop diagnose` carries the same two facts in its Environment block as `sys.platform` and `win32 on WSL distro path` (`yes`/`no`/`—`, the last meaning the verdict was not checked — never a fabricated `no`). +- The preflight also **names the multiplexer selection reason wherever selection resolves** (`mux.selection`, e.g. `platform default for win32`), not only when a `BMAD_LOOP_MUX_BACKEND`/`[mux] backend` choice forced it. A `fallback` selection is reported as a warning (its own label says no available backend matches this platform); a selection that outright failed is carried by `mux.preflight`, and a detection that failed by `mux.backends-detected` at warning — so a missing `mux.selection` line is always explained by another. On top of that, `host.win32-on-wsl-path` warns when a **native-Windows interpreter is working on a `\\wsl.localhost\...` project** ([#332](https://github.com/bmad-code-org/bmad-loop/issues/332) — see [multiplexer-backends.md](multiplexer-backends.md) for why WSL can hand a bash prompt the Windows build). Both are diagnostics only: neither changes which backend is selected (psmux _is_ correct for a `win32` interpreter) and neither flips validate's exit code. `bmad-loop diagnose` carries the same two facts in its Environment block as `sys.platform` and `win32 on WSL distro path` (`yes`/`no`). - Non-invasive: drives the upstream dev primitive unmodified — there is no fork to keep in sync — and review is just a re-invocation of it on the `done` spec. Your standard BMAD install is never modified. ### Command reference diff --git a/docs/multiplexer-backends.md b/docs/multiplexer-backends.md index fe3d4dfe..447a10a2 100644 --- a/docs/multiplexer-backends.md +++ b/docs/multiplexer-backends.md @@ -61,7 +61,7 @@ unaffected: it _is_ Linux and uses tmux — provided bmad-loop was installed wit distro's own Python. WSL appends the Windows `PATH` to its own, so a Windows-installed bmad-loop is reachable from the bash prompt; that process reports `win32` and takes the psmux default no matter how Linux the shell looks. `bmad-loop validate` names the -selection reason on every host, and warns (`host.wsl-interop`) when a `win32` interpreter +selection reason on every host, and warns (`host.win32-on-wsl-path`) when a `win32` interpreter is working on a `\\wsl.localhost\...` project (#332). Two model differences matter if you port a backend or read psmux argv. psmux runs one server diff --git a/docs/porting-to-a-new-os.md b/docs/porting-to-a-new-os.md index 2552847f..1d0e732a 100644 --- a/docs/porting-to-a-new-os.md +++ b/docs/porting-to-a-new-os.md @@ -270,7 +270,7 @@ Windows host picked on Linux) is visible at a glance. There is no new code to write for this seam — it reads seams 1 and 2. The one `sys.platform` branch that does live here is not a port seam and is not a -precedent for one: the `host.wsl-interop` check (#332) reports that the _interpreter +precedent for one: the `host.win32-on-wsl-path` check (#332) reports that the _interpreter itself_ is the wrong build for the shell that launched it — a native-Windows `bmad-loop` reached from a WSL prompt. No registration can express that, because every seam is correctly selected for the interpreter that is running; what is wrong diff --git a/src/bmad_loop/checks.py b/src/bmad_loop/checks.py index a9260f6c..df0cffa4 100644 --- a/src/bmad_loop/checks.py +++ b/src/bmad_loop/checks.py @@ -66,7 +66,7 @@ "mux.selection", "mux.external-backend", "host.process", - "host.wsl-interop", + "host.win32-on-wsl-path", "notify.desktop-unavailable", "skills.base", "skills.base-missing", diff --git a/src/bmad_loop/diagnostics.py b/src/bmad_loop/diagnostics.py index 221cbd24..dcb76b01 100644 --- a/src/bmad_loop/diagnostics.py +++ b/src/bmad_loop/diagnostics.py @@ -158,12 +158,11 @@ class EnvInfo: # line; `win32_on_wsl_path` is the #332 condition itself. Named for exactly what it # observes — a win32 interpreter working a distro path — and NOT "wsl interop": # `cd \\wsl.localhost\...` from native PowerShell reaches the same state, so this - # must not claim where the operator is standing (the matching `host.wsl-interop` - # finding is worded to the same limit). `None` means the verdict was not checked, - # which is not the same answer as `False`. Additive fields — no SCHEMA_VERSION bump; + # must not claim where the operator is standing (the matching `host.win32-on-wsl-path` + # finding is worded to the same limit). Additive fields — no SCHEMA_VERSION bump; # `--json` evolution is additive-only, see the contract note in machine.py. sys_platform: str - win32_on_wsl_path: bool | None + win32_on_wsl_path: bool @dataclass @@ -253,13 +252,14 @@ class Diagnostics: # ----------------------------------------------------------------- collectors -def collect_env(project: Path | None) -> EnvInfo: +def collect_env(project: Path) -> EnvInfo: """Host facts for the dump's Environment block. - ``project`` is used only for the #332 verdict, and is required — here *and* on - ``collect`` — so no caller can default the answer by omission: without a project - the verdict is ``None`` ("not checked"), which must stay distinguishable from a - checked ``False``. Rendering them alike is how a triager wrongly rules #332 out. + ``project`` feeds only the #332 verdict. It is required — here *and* on + ``collect`` — so a new caller must consciously supply the project it actually + diagnosed; the type cannot force that choice to be *right* (any ``Path`` + yields a confident verdict), and the sole production caller, ``cmd_diagnose``, + passes the resolved project. The project *path* itself is never emitted: it is a redaction hazard — the redactor leaves the Linux username in a ``\\\\wsl.localhost\\...\\home\\\\...`` @@ -293,9 +293,7 @@ def collect_env(project: Path | None) -> EnvInfo: multiplexer=mux, tmux_version=tmux_v, sys_platform=sys.platform, - win32_on_wsl_path=( - None if project is None else sys.platform == "win32" and is_wsl_unc_path(project) - ), + win32_on_wsl_path=sys.platform == "win32" and is_wsl_unc_path(project), ) @@ -553,7 +551,7 @@ def collect( pseudo: sanitize.Pseudonymizer, cap: int = DEFAULT_JOURNAL_CAP, generated_at: str | None = None, - project: Path | None, + project: Path, ) -> Diagnostics: runs: list[RunDiag] = [] for run_dir in run_dirs: @@ -660,14 +658,7 @@ def render_markdown( out.append(_fmt_kv("python", e.python_version)) out.append(_fmt_kv("os", f"{e.os} {e.os_release}")) out.append(_fmt_kv("sys.platform", e.sys_platform)) - # "—" for the unchecked verdict, the same spelling `tmux` uses below for absent: - # "no" would read as a checked negative and rule #332 out for a triager. - out.append( - _fmt_kv( - "win32 on WSL distro path", - "—" if e.win32_on_wsl_path is None else "yes" if e.win32_on_wsl_path else "no", - ) - ) + out.append(_fmt_kv("win32 on WSL distro path", "yes" if e.win32_on_wsl_path else "no")) out.append(_fmt_kv("multiplexer", e.multiplexer)) out.append(_fmt_kv("tmux", e.tmux_version or "—")) out.append(_fmt_kv("schema / generated", f"v{d.schema_version} @ {d.generated_at}")) diff --git a/src/bmad_loop/platform_util.py b/src/bmad_loop/platform_util.py index 35796a60..09fcdefd 100644 --- a/src/bmad_loop/platform_util.py +++ b/src/bmad_loop/platform_util.py @@ -185,11 +185,14 @@ def is_wsl_unc_path(value: str | Path) -> bool: coverage: measured on Windows 11 / CPython 3.13, ``Path.resolve()`` leaves both bridge spellings untouched, folds ``//wsl.localhost/...`` into the backslash form, and dereferences a mapped drive or ``subst`` alias back to the UNC spelling — so all - of those match. It does *not* strip a ``\\\\?\\UNC\\...`` prefix the input already - carried (``ntpath.realpath`` only strips one it added itself), and that spelling is - the one shape left unmatched. It falls back to the ``mux.selection`` line, which - names the platform regardless.""" - return str(value).replace("/", "\\").lower().startswith(("\\\\wsl.localhost\\", "\\\\wsl$\\")) + of those match. An extended-length ``\\\\?\\UNC\\...`` prefix the input already + carried (``ntpath.realpath`` only strips one it added itself) is folded down to the + plain UNC form here, so that spelling matches too. A spelling this still misses + degrades gracefully: the ``mux.selection`` line names the platform regardless.""" + text = str(value).replace("/", "\\").lower() + if text.startswith("\\\\?\\unc\\"): + text = "\\\\" + text[len("\\\\?\\unc\\") :] + return text.startswith(("\\\\wsl.localhost\\", "\\\\wsl$\\")) def _retry_on_sharing_violation(op: Callable[[], None]) -> None: diff --git a/src/bmad_loop/runsetup.py b/src/bmad_loop/runsetup.py index e0feb675..6a5d0d53 100644 --- a/src/bmad_loop/runsetup.py +++ b/src/bmad_loop/runsetup.py @@ -310,23 +310,17 @@ def platform_preflight(project: Path) -> list[Finding]: except Exception as e: # a bad BMAD_LOOP_PROCESS_HOST must report, not crash found.append(Finding("host.process", "problem", f"process host preflight failed: {e}")) - # A `warning`, never a `problem`: nothing here is broken from this interpreter's - # point of view — the selected backend is the right win32 pick and every seam above - # reports healthy — so the verdict and the exit code must not flip. What is wrong is - # the interpreter, and only the operator can swap it. + # A `warning`, never a `problem`: every seam above is healthy for this interpreter, + # so the verdict and the exit code must not flip — what is wrong is the interpreter, + # and only the operator can swap it. if sys.platform == "win32" and is_wsl_unc_path(project): - # State only what the evidence supports. A win32 interpreter on a distro path - # is NOT proof of a WSL shell: `cd \\wsl.localhost\...` from native PowerShell - # reaches the same condition, and nothing here can separate the two (the interop - # env markers do not survive the boundary — see `is_wsl_unc_path`). So the - # finding reports the mismatch it can actually see and leaves the WSL remedy - # conditional, rather than telling an operator where they are standing. - # - # The backend clause names what was *actually* chosen instead of assuming psmux - # — a forced `env`/`policy` choice would otherwise contradict the `mux.selection` - # line above — and is dropped entirely when selection failed (`chosen is None`, - # e.g. a forced unknown name), since there is no backend to name and inventing - # one is the exact failure this check exists to stop. + # State only what the evidence supports: win32 + a distro path is NOT proof of a + # WSL shell — `cd \\wsl.localhost\...` from native PowerShell reaches the same + # condition, and the interop env markers do not survive the boundary (see + # `is_wsl_unc_path`) — so the WSL remedy stays conditional. The backend clause + # names what was *actually* chosen (a forced choice would otherwise contradict + # `mux.selection` above) and is dropped when selection failed (`chosen is None`): + # inventing a backend is the exact failure this check exists to stop. picked = ( f"{chosen.name} was selected and the distro's own tmux is invisible to it" if chosen @@ -334,24 +328,15 @@ def platform_preflight(project: Path) -> list[Finding]: ) found.append( Finding( - "host.wsl-interop", + "host.win32-on-wsl-path", "warning", "the native-Windows build (this interpreter reports win32) is working on a " f"WSL distro path — {picked}; if you are running from a WSL shell, install " "bmad-loop with the WSL/Linux Python instead", - # `project` is deliberately NOT carried here. `validate --json` is not a + # `project` is deliberately NOT carried here: `validate --json` is not a # sanitized surface, and a distro path ends in the *Linux* username, - # which the egress redactor does not know (it compares against the - # Windows account). The caller passed the path in; it needs no echo. - # - # `selection_resolved` because a null backend has two causes a consumer - # would otherwise conflate: selection failed, or detection did (which - # `mux.backends-detected` reports at `warning`). - { - "backend": chosen.name if chosen else None, - "platform": sys.platform, - "selection_resolved": chosen is not None, - }, + # which the egress redactor does not know. + {"backend": chosen.name if chosen else None, "platform": sys.platform}, ) ) diff --git a/tests/test_cli.py b/tests/test_cli.py index 7db9c446..ee76cd27 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -4690,33 +4690,22 @@ def test_platform_preflight_notes_forced_selection_provenance(mux_registry, monk # ------------------------------------ native-Windows-from-WSL visibility (#332) # -# A WSL shell reaches a native-Windows bmad-loop through interop (WSL appends the -# Windows PATH to its own). That interpreter reports win32, takes the psmux -# platform default, and never sees the distro's tmux — while the operator is -# looking at a bash prompt. Selection is right for what the interpreter is; the -# bug is that nothing named the platform. These pin the naming. +# WSL interop can hand a bash prompt a native-Windows bmad-loop (WSL appends the +# Windows PATH to its own). That interpreter reports win32 and takes the psmux +# default — correctly, for what it is. These pin the naming that makes the +# mismatch visible. WSL_UNC_PROJECT = Path("\\\\wsl.localhost\\Ubuntu-24.04\\home\\u\\p") @pytest.fixture def fake_platform(monkeypatch): - """Patch `sys.platform` for a preflight call and un-poison what that call caches. - - The patch is *process-wide* — spelled `sys` deliberately, because `cli.runsetup.sys` - is the same module object and the qualified spelling would imply a module-local - scope that does not exist. Every `sys.platform` reader on the path under test sees - it: `mux_reason_label`'s label, `_PLATFORM_DEFAULTS`, and the process host's - `matches()`. - - `platform_preflight` reaches `get_process_host()`, which is `lru_cache(maxsize=1)` - and selects on `sys.platform`. Patching the platform without clearing the cache - leaves the *Windows* process host cached for every later test in the worker — - silently, and worst on a POSIX CI worker. (`get_multiplexer` has the same shape; - the `mux_registry` fixture already clears it.) Clearing on the way in and on the - way out keeps the poisoning inside one test in both directions. Within the test, - constructing `WindowsProcessHost` on a POSIX worker is harmless: a raise becomes a - `host.process` problem finding, which no assertion here reads.""" + """Patch `sys.platform` process-wide (`cli.runsetup.sys` is the same module + object — a module-qualified patch would silently miss the other readers on the + path under test: `mux_reason_label`, `_PLATFORM_DEFAULTS`, the process host's + `matches()`) and clear `get_process_host`'s lru_cache in both directions — without + the clears, the patched window caches the Windows host for every later test in + the worker. (`get_multiplexer` has the same shape; `mux_registry` clears it.)""" from bmad_loop.process_host import get_process_host def _set(platform_name: str) -> None: @@ -4728,8 +4717,8 @@ def _set(platform_name: str) -> None: def test_platform_preflight_names_the_platform_default_selection(mux_registry): - """The reason that most needs naming is `platform-default` — it is how a win32 - interpreter silently lands on psmux — and it used to be the one the gate dropped.""" + """`platform-default` is how a win32 interpreter silently lands on psmux — and + it used to be the one reason the emission gate dropped.""" default = mux_registry._PLATFORM_DEFAULTS.get(sys.platform, mux_registry._DEFAULT_BACKEND) mux_registry.register_multiplexer(default, lambda p: True, lambda: _MuxStub(avail=True)) mux_registry.get_multiplexer.cache_clear() @@ -4741,9 +4730,8 @@ def test_platform_preflight_names_the_platform_default_selection(mux_registry): def test_platform_preflight_selection_warns_when_the_reason_is_fallback(mux_registry): - """`fallback` is what `_select` returns when no available backend matches this - platform, and the label says so — so un-gating this finding must not print that - sentence at `ok`.""" + """The fallback label itself says no available backend matches this platform — + printing that sentence green would contradict it.""" mux_registry.register_multiplexer("alpha", lambda p: True, lambda: _MuxStub(avail=False)) mux_registry.get_multiplexer.cache_clear() @@ -4753,34 +4741,28 @@ def test_platform_preflight_selection_warns_when_the_reason_is_fallback(mux_regi assert "no available backend matches this platform" in selection.message -def test_wsl_interop_warns_when_a_win32_interpreter_works_a_distro_path( - mux_registry, fake_platform -): +def test_win32_on_wsl_path_warns_and_keeps_the_path_out(mux_registry, fake_platform): mux_registry.register_multiplexer("psmux", lambda p: True, lambda: _MuxStub(avail=True)) mux_registry.get_multiplexer.cache_clear() fake_platform("win32") finding = next( - f for f in cli._platform_preflight(WSL_UNC_PROJECT) if f.check == "host.wsl-interop" + f for f in cli._platform_preflight(WSL_UNC_PROJECT) if f.check == "host.win32-on-wsl-path" ) assert "WSL/Linux Python" in finding.message assert "psmux was selected" in finding.message - # The evidence is `win32` + a distro path. That is NOT proof of a WSL shell — - # `cd \\wsl.localhost\...` from native PowerShell reaches the same condition — so - # the finding must not assert where the operator is standing. Exactly one mention, - # and it is the conditional remedy rather than a claim about the session. + # win32 + a distro path is NOT proof of a WSL shell (native PowerShell reaches the + # same condition), so the only mention is the conditional remedy. assert finding.message.count("WSL shell") == 1 assert "if you are running from a WSL shell" in finding.message # The project path must not ride along: `validate --json` is unsanitized and a # distro path ends in the Linux username. - assert finding.detail == { - "backend": "psmux", - "platform": "win32", - "selection_resolved": True, - } + assert finding.detail == {"backend": "psmux", "platform": "win32"} assert str(WSL_UNC_PROJECT) not in str(finding.detail) + finding.message -def test_wsl_interop_names_the_backend_actually_selected(mux_registry, fake_platform, monkeypatch): +def test_win32_on_wsl_path_names_the_backend_actually_selected( + mux_registry, fake_platform, monkeypatch +): """A forced choice must not be described as psmux — the `mux.selection` line in the same report would say otherwise, in the one check whose point is to stop misleading.""" mux_registry.register_multiplexer("herdr", lambda p: False, lambda: _MuxStub(avail=True)) @@ -4788,15 +4770,15 @@ def test_wsl_interop_names_the_backend_actually_selected(mux_registry, fake_plat mux_registry.get_multiplexer.cache_clear() fake_platform("win32") findings = cli._platform_preflight(WSL_UNC_PROJECT) - warning = next(f for f in findings if f.check == "host.wsl-interop") + warning = next(f for f in findings if f.check == "host.win32-on-wsl-path") selection = next(f for f in findings if f.check == "mux.selection") - assert warning.detail == {"backend": "herdr", "platform": "win32", "selection_resolved": True} + assert warning.detail == {"backend": "herdr", "platform": "win32"} assert "herdr was selected" in warning.message assert "psmux" not in warning.message assert selection.detail == {"backend": "herdr", "reason": "env"} -def test_wsl_interop_names_no_backend_when_selection_failed( +def test_win32_on_wsl_path_names_no_backend_when_selection_failed( mux_registry, fake_platform, monkeypatch ): """A forced unknown name makes `_select` raise, so no row is selected. The warning @@ -4808,37 +4790,14 @@ def test_wsl_interop_names_no_backend_when_selection_failed( fake_platform("win32") findings = cli._platform_preflight(WSL_UNC_PROJECT) - warning = next(f for f in findings if f.check == "host.wsl-interop") - # `selection_resolved` because a null backend has a second cause a consumer would - # otherwise conflate with this one: detection raising (`mux.backends-detected`). - assert warning.detail == { - "backend": None, - "platform": "win32", - "selection_resolved": False, - } + warning = next(f for f in findings if f.check == "host.win32-on-wsl-path") + assert warning.detail == {"backend": None, "platform": "win32"} assert "was selected" not in warning.message assert "win32 default" not in warning.message assert not [f for f in findings if f.check == "mux.selection"] assert [f for f in findings if f.check == "mux.preflight"] -def test_wsl_interop_is_a_warning_so_the_validate_verdict_never_flips(mux_registry, fake_platform): - """Severity asserted through the real preflight, not a hand-built Finding: every - seam is healthy from this interpreter's point of view, so only the operator's - choice of interpreter is wrong and the exit code must not move.""" - from bmad_loop.checks import ValidationReport - - mux_registry.register_multiplexer("psmux", lambda p: True, lambda: _MuxStub(avail=True)) - mux_registry.get_multiplexer.cache_clear() - fake_platform("win32") - findings = cli._platform_preflight(WSL_UNC_PROJECT) - assert next(f for f in findings if f.check == "host.wsl-interop").severity == "warning" - - report = ValidationReport() - report.extend(findings) - assert report.passed is True - - @pytest.mark.parametrize( ("platform_name", "project"), [ @@ -4851,17 +4810,18 @@ def test_wsl_interop_is_a_warning_so_the_validate_verdict_never_flips(mux_regist ("win32", Path("\\\\fileserver\\share\\p")), ], ) -def test_wsl_interop_stays_silent_off_the_interop_shape( +def test_win32_on_wsl_path_stays_silent_off_the_shape( mux_registry, fake_platform, platform_name, project ): fake_platform(platform_name) - assert not [f for f in cli._platform_preflight(project) if f.check == "host.wsl-interop"] + assert not [f for f in cli._platform_preflight(project) if f.check == "host.win32-on-wsl-path"] -def test_wsl_interop_survives_the_render_and_the_json_projection(mux_registry, fake_platform): +def test_win32_on_wsl_path_survives_the_render_and_the_json_projection(mux_registry, fake_platform): """The other tests read the finding straight off `_platform_preflight`, which cannot - catch it being dropped between there and a surface the operator actually sees — and - every validate-rendering test stubs the preflight out wholesale.""" + catch it being dropped before a surface the operator actually sees — every + validate-rendering test stubs the preflight out wholesale. The severity and the + unmoved `ok` verdict pin that a warning never flips validate's exit code.""" mux_registry.register_multiplexer("psmux", lambda p: True, lambda: _MuxStub(avail=True)) mux_registry.get_multiplexer.cache_clear() fake_platform("win32") @@ -4870,10 +4830,9 @@ def test_wsl_interop_survives_the_render_and_the_json_projection(mux_registry, f report.extend(cli._platform_preflight(WSL_UNC_PROJECT)) # asserts the id is registered doc = cli.validate_document(report, False, "") - rows = [f for f in doc["findings"] if f["check"] == "host.wsl-interop"] + rows = [f for f in doc["findings"] if f["check"] == "host.win32-on-wsl-path"] assert len(rows) == 1 assert rows[0]["severity"] == "warning" - assert rows[0]["detail"]["selection_resolved"] is True assert doc["ok"] is True # a warning never moves the verdict assert str(WSL_UNC_PROJECT) not in json.dumps(doc) # unsanitized surface diff --git a/tests/test_diagnostics.py b/tests/test_diagnostics.py index 1f06846b..fc7f97b7 100644 --- a/tests/test_diagnostics.py +++ b/tests/test_diagnostics.py @@ -170,11 +170,14 @@ def _seed_run(root, run_id="20260627-120000-aaaa", *, extra_journal=None, sweeps return run_dir +# a plain relative project for tests that are about run payloads, not the #332 +# verdict — any non-WSL path yields the same checked `False` +ANY_PROJECT = Path("p") + + def _render_all(run_dirs): pseudo = sanitize.Pseudonymizer() - # project=None: these tests are about run payloads, not the #332 verdict, and the - # parameter is required precisely so "not checked" is stated rather than defaulted. - diag = diagnostics.collect(run_dirs, pseudo=pseudo, project=None) + diag = diagnostics.collect(run_dirs, pseudo=pseudo, project=ANY_PROJECT) md = diagnostics.render_markdown(diag, pseudo=pseudo) js = diagnostics.render_json(diag, pseudo=pseudo) return diag, pseudo, md + "\n" + js @@ -224,8 +227,8 @@ def test_env_names_the_platform_and_the_win32_on_wsl_path_verdict(project, monke assert "**sys.platform:** win32" in md assert "**win32 on WSL distro path:** yes" in md # The label must not claim a WSL *shell*: `cd \\wsl.localhost\...` from native - # PowerShell reaches this same state, and the sibling `host.wsl-interop` finding is - # worded to that limit — the two surfaces must make the same claim. + # PowerShell reaches this same state, and the sibling `host.win32-on-wsl-path` + # finding is worded to that limit — the two surfaces must make the same claim. assert "wsl interop" not in md.lower() # the boolean ships; the path it was derived from never does — the redactor # leaves the Linux username in a \\wsl.localhost\...\home\ path standing. @@ -242,17 +245,6 @@ def test_env_win32_on_wsl_path_is_false_for_a_plain_project(project): assert "**win32 on WSL distro path:** no" in diagnostics.render_markdown(diag, pseudo=pseudo) -def test_env_unchecked_verdict_renders_as_absent_not_as_a_negative(project): - """`None` (no project in hand) must not render "no": that is the sentence a triager - reads to rule #332 *out*, and it would be a fabricated negative.""" - pseudo = sanitize.Pseudonymizer() - diag = diagnostics.collect([_seed_run(project.project)], pseudo=pseudo, project=None) - assert diag.env.win32_on_wsl_path is None - md = diagnostics.render_markdown(diag, pseudo=pseudo) - assert "**win32 on WSL distro path:** —" in md - assert "**win32 on WSL distro path:** no" not in md - - def test_pseudonymization_is_stable_and_correlates(project): run_dir = _seed_run(project.project) diag, _pseudo, combined = _render_all([run_dir]) @@ -392,7 +384,7 @@ def test_all_runs_scope(project): def test_legend_reverses_locally_but_never_ships(project): run_dir = _seed_run(project.project) pseudo = sanitize.Pseudonymizer() - diag = diagnostics.collect([run_dir], pseudo=pseudo, project=None) + diag = diagnostics.collect([run_dir], pseudo=pseudo, project=ANY_PROJECT) combined = diagnostics.render_markdown(diag, pseudo=pseudo) + diagnostics.render_json( diag, pseudo=pseudo ) @@ -409,7 +401,7 @@ def test_unreadable_run_does_not_crash(project): run_dir.mkdir(parents=True) (run_dir / "state.json").write_text("{ this is not valid json") pseudo = sanitize.Pseudonymizer() - diag = diagnostics.collect([run_dir], pseudo=pseudo, project=None) + diag = diagnostics.collect([run_dir], pseudo=pseudo, project=ANY_PROJECT) assert len(diag.runs) == 1 assert diag.runs[0].warnings # flagged as unreadable # still renders without raising @@ -432,7 +424,7 @@ def _seed_routing_gap(project): def test_routing_gap_is_repaired_end_to_end(project): run_dir = _seed_routing_gap(project) pseudo = sanitize.Pseudonymizer() - diag = diagnostics.collect([run_dir], pseudo=pseudo, project=None) + diag = diagnostics.collect([run_dir], pseudo=pseudo, project=ANY_PROJECT) reps: list[tuple[str, int]] = [] js = diagnostics.render_json(diag, pseudo=pseudo, repairs=reps) # must not raise alias = next(a for ns, orig, a in pseudo.entries() if orig == STORY_KEY) @@ -451,7 +443,7 @@ def test_render_json_keys_are_sorted(project): round-trip cannot detect the flag being dropped.""" run_dir = _seed_run(project.project) pseudo = sanitize.Pseudonymizer() - diag = diagnostics.collect([run_dir], pseudo=pseudo, project=None) + diag = diagnostics.collect([run_dir], pseudo=pseudo, project=ANY_PROJECT) js = diagnostics.render_json(diag, pseudo=pseudo) def hook(pairs): @@ -467,7 +459,7 @@ def test_no_repairs_on_fully_routed_run(project): silently normalize a new per-field routing gap (CI keeps catching them).""" run_dir = _seed_run(project.project) pseudo = sanitize.Pseudonymizer() - diag = diagnostics.collect([run_dir], pseudo=pseudo, project=None) + diag = diagnostics.collect([run_dir], pseudo=pseudo, project=ANY_PROJECT) reps: list[tuple[str, int]] = [] md = diagnostics.render_markdown(diag, pseudo=pseudo, repairs=reps) js = diagnostics.render_json(diag, pseudo=pseudo, repairs=reps) @@ -487,7 +479,7 @@ def test_repair_note_is_inside_verified_bytes(project): """The disclosure appended after repair is itself covered by the self-check.""" run_dir = _seed_routing_gap(project) pseudo = sanitize.Pseudonymizer() - diag = diagnostics.collect([run_dir], pseudo=pseudo, project=None) + diag = diagnostics.collect([run_dir], pseudo=pseudo, project=ANY_PROJECT) js = diagnostics.render_json(diag, pseudo=pseudo) extras = [(orig, f"{ns}:{alias}") for ns, orig, alias in pseudo.entries()] assert sanitize.assert_no_leak(js, extra=extras) == [] @@ -552,7 +544,7 @@ def version(self): monkeypatch.setattr(mux_mod, "get_multiplexer", lambda: _TwoLineMux()) - env = diagnostics.collect_env(Path("p")) + env = diagnostics.collect_env(ANY_PROJECT) assert env.tmux_version == "tmux 3.3.7; psmux 3.3.7" assert env.multiplexer == "_TwoLineMux" @@ -569,7 +561,7 @@ def version(self): monkeypatch.setattr(mux_mod, "get_multiplexer", lambda: _ChattyMux()) - env = diagnostics.collect_env(Path("p")) + env = diagnostics.collect_env(ANY_PROJECT) assert env.tmux_version is not None assert len(env.tmux_version) == mux_mod.VERSION_MAX_CHARS assert "\n" not in env.tmux_version @@ -603,7 +595,7 @@ def version(self): monkeypatch.setattr(mux_mod, "get_multiplexer", lambda: _HomeyMux()) - env = diagnostics.collect_env(Path("p")) + env = diagnostics.collect_env(ANY_PROJECT) assert env.tmux_version is not None assert env.tmux_version.endswith("…") # the cut really fired assert home not in env.tmux_version diff --git a/tests/test_platform_util.py b/tests/test_platform_util.py index e4d72da6..7db84827 100644 --- a/tests/test_platform_util.py +++ b/tests/test_platform_util.py @@ -118,6 +118,12 @@ def test_names_tree_root_catches_the_win32_trim_aliases(value): "//wsl.localhost/Ubuntu/home/u/p", # Windows accepts either separator "\\\\wsl.localhost/Ubuntu\\home", # mixed separators Path("\\\\wsl.localhost\\Ubuntu\\home\\u\\p"), # a Path, not just a str + "\\\\?\\UNC\\wsl.localhost\\Ubuntu\\home\\u\\p", # extended-length UNC spelling + "\\\\?\\unc\\wsl$\\Ubuntu\\home\\u", # extended-length, legacy host, lowercase + # not a shape Win32 accepts (a forward-slash device path addresses a host + # named `?`) — the separator fold over-matches it, which can only add the + # warning, never suppress it + "//?/UNC/wsl.localhost/Ubuntu/home/u", ], ) def test_is_wsl_unc_path_matches_the_interop_bridge(value): @@ -163,6 +169,8 @@ def test_names_tree_root_accepts_anything_naming_a_child(value): "\\\\wslfoo\\share\\p", # host merely *starts* with wsl "\\\\wsl.localhost", # no distro component at all "wsl.localhost\\Ubuntu\\home", # not a UNC path + "\\\\?\\UNC\\fileserver\\share\\p", # extended-length, but not a WSL host + "\\\\?\\C:\\p", # extended-length drive path, not UNC at all "", ], ) @@ -185,6 +193,10 @@ def test_is_wsl_unc_path_is_platform_blind(monkeypatch): "\\\\wsl.localhost\\{d}\\home", "\\\\wsl$\\{d}\\home", "//wsl.localhost/{d}/home", + # resolve() must keep, not strip, an extended prefix the input carried — + # the premise the predicate's fold rests on; if a future CPython starts + # stripping it, the fold goes dead and only this row notices + "\\\\?\\UNC\\wsl.localhost\\{d}\\home", ], ) def test_is_wsl_unc_path_survives_the_resolve_the_caller_applies(spelling): From 6bde3b1bf308324261aae8225fe15e93d843e5e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Davor=20Raci=C4=87?= Date: Fri, 7 Aug 2026 18:25:43 +0200 Subject: [PATCH 3/4] docs(validate): stop overclaiming that a missing mux.selection is always explained Review follow-ups: the FEATURES sentence and the validate --json contract docstring now carry the silent unregistered-tmux exception and the detection-failure third cause, and document that the detection-failure warning carries no detail. The issue form points at diagnose's sys.platform for when a forced backend replaces the selection line. The diagnose env test pins the JSON field names directly and un-poisons the mux cache when a mid-test raise skips the inline clears. --- .github/ISSUE_TEMPLATE/bug-report.yaml | 3 ++- docs/FEATURES.md | 2 +- src/bmad_loop/documents.py | 14 ++++++++------ tests/test_diagnostics.py | 25 ++++++++++++++++++------- 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug-report.yaml b/.github/ISSUE_TEMPLATE/bug-report.yaml index aaf83205..4a0ebd50 100644 --- a/.github/ISSUE_TEMPLATE/bug-report.yaml +++ b/.github/ISSUE_TEMPLATE/bug-report.yaml @@ -101,7 +101,8 @@ body: label: Operating System description: On WSL, run `bmad-loop mux` and read the `selection:` line — `platform default for linux` is the supported setup, `platform default for win32` means you are on the Windows build - and should say so below. + and should say so below. A forced `BMAD_LOOP_MUX_BACKEND`/`[mux] backend` choice replaces + that line with the forced reason; `bmad-loop diagnose` reports `sys.platform` either way. options: - macOS - Windows diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 61b786f7..7d377501 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -183,7 +183,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - `bmad-loop init` installs the three `bmad-loop-*` skills (`bmad-loop-setup`, `bmad-loop-resolve`, `bmad-loop-sweep`, into `.claude/skills/` and/or `.agents/skills/`), the hook relay, `.bmad-loop/policy.toml`, and a gitignore covering the runs dir, plugin caches, and policy.toml itself (per-machine config). Flags: `--cli` (repeatable), `--no-skills`, `--force-skills`. - `bmad-loop validate` preflights every prerequisite: BMAD config, sprint-status, git, the selected terminal-multiplexer backend (listing all detected when more than one is registered), CLI binary, hook registration, and the review skills the installed dev primitive actually invokes (reporting which name it resolved) — derived from its `customize.toml` review layers (or from `step-04-review.md` on releases that name reviewers inline), so both the merged `bmad-review` topology and the standalone-hunter one validate, and configured layers naming an uninstalled skill are caught — plus its `customize.toml`. -- The preflight also **names the multiplexer selection reason wherever selection resolves** (`mux.selection`, e.g. `platform default for win32`), not only when a `BMAD_LOOP_MUX_BACKEND`/`[mux] backend` choice forced it. A `fallback` selection is reported as a warning (its own label says no available backend matches this platform); a selection that outright failed is carried by `mux.preflight`, and a detection that failed by `mux.backends-detected` at warning — so a missing `mux.selection` line is always explained by another. On top of that, `host.win32-on-wsl-path` warns when a **native-Windows interpreter is working on a `\\wsl.localhost\...` project** ([#332](https://github.com/bmad-code-org/bmad-loop/issues/332) — see [multiplexer-backends.md](multiplexer-backends.md) for why WSL can hand a bash prompt the Windows build). Both are diagnostics only: neither changes which backend is selected (psmux _is_ correct for a `win32` interpreter) and neither flips validate's exit code. `bmad-loop diagnose` carries the same two facts in its Environment block as `sys.platform` and `win32 on WSL distro path` (`yes`/`no`). +- The preflight also **names the multiplexer selection reason wherever selection resolves** (`mux.selection`, e.g. `platform default for win32`), not only when a `BMAD_LOOP_MUX_BACKEND`/`[mux] backend` choice forced it. A `fallback` selection is reported as a warning (its own label says no available backend matches this platform); a selection that outright failed is carried by `mux.preflight`, and a detection that failed by `mux.backends-detected` at warning — so a missing `mux.selection` line is normally explained by another finding (the historical unregistered-tmux fallback is the one silent exception; see the `--json` contract note in `documents.py`). On top of that, `host.win32-on-wsl-path` warns when a **native-Windows interpreter is working on a `\\wsl.localhost\...` project** ([#332](https://github.com/bmad-code-org/bmad-loop/issues/332) — see [multiplexer-backends.md](multiplexer-backends.md) for why WSL can hand a bash prompt the Windows build). Both are diagnostics only: neither changes which backend is selected (psmux _is_ correct for a `win32` interpreter) and neither flips validate's exit code. `bmad-loop diagnose` carries the same two facts in its Environment block as `sys.platform` and `win32 on WSL distro path` (`yes`/`no`). - Non-invasive: drives the upstream dev primitive unmodified — there is no fork to keep in sync — and review is just a re-invocation of it on the `done` spec. Your standard BMAD install is never modified. ### Command reference diff --git a/src/bmad_loop/documents.py b/src/bmad_loop/documents.py index f9a6d489..cafea8a6 100644 --- a/src/bmad_loop/documents.py +++ b/src/bmad_loop/documents.py @@ -66,7 +66,7 @@ def validate_document( failure to produce one (see machine.py on parsing non-empty stdout whatever the exit code). - Three things a consumer has to know: + Four things a consumer has to know: - **``message`` is not contracted.** Several problems are a bare ``str(e)`` from the config, policy, profile and sprint-status exceptions, so their @@ -79,14 +79,16 @@ def validate_document( - **``mux.backends-detected`` is gated on more than one registered backend**, so a lone-tmux host carries no backend inventory. Same rule as above. The one exception to its ``ok`` severity is a detection failure, which reports under - the same id at ``warning`` with an empty inventory. + the same id at ``warning`` and carries no ``detail`` at all — read the + severity, never index ``detail["backends"]`` without a null check. - **``mux.selection`` is not gated on the reason.** It used to appear only for a forced ``env``/``policy`` choice and now names the reason wherever selection - resolves. It is absent whenever no backend row is *selected*, which has two + resolves. It is absent whenever no backend row is *selected*, which has three causes: a forced name matching no registered backend (``mux.preflight`` - carries that failure) and ``_select`` bottoming out at its historical tmux - fallback with tmux unregistered (nothing else reports that — the inventory - simply holds no selected row). Same rule as above. + carries that failure), a detection failure (the ``warning`` above carries + it), and ``_select`` bottoming out at its historical tmux fallback with tmux + unregistered (nothing else reports that — the inventory simply holds no + selected row). Same rule as above. ``findings`` stays flat and in emission order rather than grouped by severity: grouping would destroy the cross-severity ordering (the order the gates ran) diff --git a/tests/test_diagnostics.py b/tests/test_diagnostics.py index fc7f97b7..6db8d054 100644 --- a/tests/test_diagnostics.py +++ b/tests/test_diagnostics.py @@ -213,12 +213,16 @@ def test_env_names_the_platform_and_the_win32_on_wsl_path_verdict(project, monke # on `sys.platform`; without these clears the patched window caches the Windows # pick for every later test in the worker. get_multiplexer.cache_clear() - monkeypatch.setattr(diagnostics.sys, "platform", "win32") - pseudo = sanitize.Pseudonymizer() - unc = Path("\\\\wsl.localhost\\Ubuntu-24.04\\home\\u\\p") - diag = diagnostics.collect([_seed_run(project.project)], pseudo=pseudo, project=unc) - monkeypatch.undo() - get_multiplexer.cache_clear() + try: + monkeypatch.setattr(diagnostics.sys, "platform", "win32") + pseudo = sanitize.Pseudonymizer() + unc = Path("\\\\wsl.localhost\\Ubuntu-24.04\\home\\u\\p") + diag = diagnostics.collect([_seed_run(project.project)], pseudo=pseudo, project=unc) + finally: + # pytest undoes the patch on its own, but only at teardown — a raise in + # `collect` would leave the Windows pick cached past this test without this. + monkeypatch.undo() + get_multiplexer.cache_clear() assert diag.env.sys_platform == "win32" assert diag.env.win32_on_wsl_path is True md = diagnostics.render_markdown(diag, pseudo=pseudo) @@ -233,7 +237,14 @@ def test_env_names_the_platform_and_the_win32_on_wsl_path_verdict(project, monke # the boolean ships; the path it was derived from never does — the redactor # leaves the Linux username in a \\wsl.localhost\...\home\ path standing. assert str(unc) not in md - assert sanitize.assert_no_leak(diagnostics.render_json(diag, pseudo=pseudo)) == [] + # the `--json` document is its own contract: pin the field names and values + # there too, not only the markdown labels. + js = diagnostics.render_json(diag, pseudo=pseudo) + payload = json.loads(js) + assert payload["env"]["sys_platform"] == "win32" + assert payload["env"]["win32_on_wsl_path"] is True + assert str(unc) not in js + assert sanitize.assert_no_leak(js) == [] def test_env_win32_on_wsl_path_is_false_for_a_plain_project(project): From 03ac27e872f1193b14928b8bcc21646bf0b631f2 Mon Sep 17 00:00:00 2001 From: t Date: Sat, 8 Aug 2026 22:14:54 -0700 Subject: [PATCH 4/4] test(diagnostics): make the #332 leak guard and the twin's platform half bite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two assertions in the new diagnose coverage passed for reasons unrelated to what they claim to prove. Both measured by ablation, not inspection. The JSON leak guard was vacuous twice over. `assert str(unc) not in js` compares a raw `\\wsl.localhost\...` spelling against bytes `json.dumps` has already escaped, so the doubled backslashes can never match — the same trap `cli.py` names at the diagnose egress guard. Its belt-and-braces partner is no better: `sanitize`'s absolute-path rules know POSIX `/home/`, `/Users/`, `/root/` and `C:\Users\`, never a backslash `\home\` under a UNC host, and the username rule compares the Windows account — so `assert_no_leak` returns `[]` for this shape whatever leaked. Adding a raw-path field to `EnvInfo` left both green while the rendered document carried the Linux username in full. Assert over the decoded `payload["env"]` values instead, which are the bytes a consumer actually reads; that reddens on the same ablation. This is what CodeRabbit asked for on the pre-merge head, and the reason its concern was not yet addressed. `collect_env`'s gate is a twin of `platform_preflight`'s, but only one of them was pinned on both halves: deleting `sys.platform == "win32"` from `collect_env` reddened nothing in this file, because there was no "off-win32 + distro path" row to catch it. runsetup's twin has carried that row all along. Add it. The implementation is correct in both cases and is unchanged here — this is test-only. --- tests/test_diagnostics.py | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/tests/test_diagnostics.py b/tests/test_diagnostics.py index 6db8d054..a86c8e56 100644 --- a/tests/test_diagnostics.py +++ b/tests/test_diagnostics.py @@ -243,8 +243,43 @@ def test_env_names_the_platform_and_the_win32_on_wsl_path_verdict(project, monke payload = json.loads(js) assert payload["env"]["sys_platform"] == "win32" assert payload["env"]["win32_on_wsl_path"] is True - assert str(unc) not in js - assert sanitize.assert_no_leak(js) == [] + # Assert over the *decoded* values, never the rendered bytes. Two independent + # reasons a substring scan of `js` cannot carry this guard, both measured: + # - `json.dumps` doubles every backslash, so the raw `\\wsl.localhost\...` + # spelling never appears in the rendered bytes it is compared against + # (the same trap `cli.py` already names at its diagnose egress guard); + # - `sanitize`'s absolute-path rules know POSIX `/home/`, `/Users/`, `/root/` + # and `C:\Users\` — never a backslash `\home\` under a UNC host — and its + # username rule compares the *Windows* account, so `assert_no_leak` returns + # `[]` for this shape no matter what leaked. + # Ablation: adding a raw-path field to `EnvInfo` leaves both of those green and + # only the check below reddens. + assert all(str(unc) not in str(v) for v in payload["env"].values()) + assert sanitize.assert_no_leak(js) == [] # general backstop; blind to this shape + + +def test_env_win32_on_wsl_path_is_false_off_win32(project, monkeypatch): + """The *platform* half of the twin gate, pinned. What #332 names is a mismatched + interpreter, not a path shape: the very distro path a win32 interpreter warns about + is a perfectly ordinary mount for a Linux one. `runsetup`'s twin already carries + this row (`test_win32_on_wsl_path_stays_silent_off_the_shape`'s `linux` case); + without it here, deleting `sys.platform == "win32"` from `collect_env` reddens + nothing in this file — measured, which is why the row exists.""" + from bmad_loop.adapters.multiplexer import get_multiplexer + + # Same cache dance as the win32 test above, and for the same reason: `collect_env` + # reaches `get_multiplexer()`, whose lru_cache selects on the patched `sys.platform`. + get_multiplexer.cache_clear() + try: + monkeypatch.setattr(diagnostics.sys, "platform", "linux") + pseudo = sanitize.Pseudonymizer() + unc = Path("\\\\wsl.localhost\\Ubuntu-24.04\\home\\u\\p") + diag = diagnostics.collect([_seed_run(project.project)], pseudo=pseudo, project=unc) + finally: + monkeypatch.undo() + get_multiplexer.cache_clear() + assert diag.env.sys_platform == "linux" + assert diag.env.win32_on_wsl_path is False def test_env_win32_on_wsl_path_is_false_for_a_plain_project(project):