Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ breaking changes may land in a minor release.

- Count `plugin-hook-error` entries in `diagnose`'s plugin-errors total (#779).

- Register Windows hook commands with forward-slash paths so Git Bash no longer strips
their separators and stalls every session (#773); re-run `bmad-loop init` to migrate.
`validate` warns (`hooks.relay-stale`) while a backslash registration remains. Paths
with spaces remain unsupported under the PowerShell fallback.

- Replace stale installed relay hooks when a project moves between Windows and POSIX.

- Report stale or unverifiable Codex hook trust in `validate` and `probe-adapter`
Expand Down
24 changes: 14 additions & 10 deletions src/bmad_loop/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ def cmd_validate(args: argparse.Namespace) -> int:
{"binary": tool, "path": resolved, "returncode": rc},
)

registered_relay_paths: set[Path] = set()
registered_relays: set[tuple[Path, str]] = set()
for profile in profiles:
# Keyed on the adapter KIND, not on `hookless`. httpx is the bundled
# opencode family's optional extra — a fact about one adapter class, which
Expand Down Expand Up @@ -742,7 +742,7 @@ def cmd_validate(args: argparse.Namespace) -> int:
{"profile": profile.name, "config_path": str(hook_config)},
)
if isinstance(parsed, dict):
registered_relay_paths.update(
registered_relays.update(
install.registered_relay_paths(
parsed, profile.hooks.dialect, profile.hooks.events, project
)
Expand Down Expand Up @@ -808,18 +808,22 @@ def cmd_validate(args: argparse.Namespace) -> int:
# installation in this process cannot repair an older path in a hook config.
# Compare with the command init would write now: an old executable can remain
# usable after switching installations, while still running an outdated relay.
expected_relay = None
if registered_relay_paths:
# The comparison is on the registered TEXT, the same test init's merge_hooks
# applies: on Windows both `Path` equality and `str(Path)` normalize
# separators, so a pre-#773 backslash registration (which Git Bash mangles,
# stalling every session) would otherwise never be flagged.
expected_text = None
if registered_relays:
hook_profile = next(profile for profile in profiles if not profile.hookless)
try:
expected_relay = install.relay_executable(
expected_text = install.relay_executable_text(
install._hook_command(project, hook_profile, "Stop")
)
except ProfileError:
# No current executable to compare. The registered path still gets
# its own presence check below; do not call it stale by inference.
pass
for relay in sorted(registered_relay_paths):
for relay, spelling in sorted(registered_relays):
if not relay.is_file():
report.fail(
"hooks.relay-present",
Expand All @@ -840,13 +844,13 @@ def cmd_validate(args: argparse.Namespace) -> int:
f"registered hook executable available: {relay}",
{"path": str(relay)},
)
if expected_relay is not None and relay != expected_relay:
if expected_text is not None and spelling != expected_text:
report.warn(
"hooks.relay-stale",
f"registered hook executable {relay} differs from this "
f"installation's {expected_relay} — re-run `bmad-loop init` "
f"registered hook executable {spelling} differs from this "
f"installation's {expected_text} — re-run `bmad-loop init` "
"to update the hook registration",
{"path": str(relay), "expected_path": str(expected_relay)},
{"path": spelling, "expected_path": expected_text},
)

# Adapter-kind validity is enforced against the LIVE registry, never a
Expand Down
31 changes: 24 additions & 7 deletions src/bmad_loop/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -1027,6 +1027,16 @@ def relay_executable(command: str) -> Path | None:
A foreign path remains unusable on this host, but must still be recognized
as managed so init and worktree provisioning replace its stale hook.
"""
text = relay_executable_text(command)
return Path(text) if text is not None else None


def relay_executable_text(command: str) -> str | None:
"""Return a relay command's executable exactly as registered.

`Path` normalizes the spelling (on Windows it treats `\\` and `/` alike), so
callers asking "would init write something different" compare this text.
"""
for posix in (os.name != "nt", os.name == "nt"):
try:
parts = shlex.split(command, posix=posix)
Expand All @@ -1042,7 +1052,7 @@ def relay_executable(command: str) -> Path | None:
for flavor in (PurePosixPath, PureWindowsPath):
executable = flavor(raw)
if executable.name in {"bmad-loop", "bmad-loop.exe"} and executable.is_absolute():
return Path(raw)
return raw
return None


Expand Down Expand Up @@ -1197,23 +1207,30 @@ def relay_registered(config: dict, dialect: str, events: Iterable[str]) -> bool:

def registered_relay_paths(
config: dict, dialect: str, events: Iterable[str], project: Path
) -> list[Path]:
"""Paths invoked by the actual managed commands in a hook config."""
) -> list[tuple[Path, str]]:
"""Paths invoked by the actual managed commands in a hook config.

Each path is paired with its registered spelling: the console relay's
executable text as written in the command, or the legacy script path with
the project directory substituted. The `Path` answers presence questions;
the spelling answers whether init would now write something different.
"""
container = hook_event_container(config, dialect)
paths: list[Path] = []
paths: list[tuple[Path, str]] = []
for event in events:
handlers = container.get(event)
if not isinstance(handlers, list):
continue
for handler in handlers:
for command in _commands_in_handler(handler):
executable = relay_executable(command)
executable = relay_executable_text(command)
if executable is not None:
paths.append(executable)
paths.append((Path(executable), executable))
else:
script = _legacy_relay_script(command)
if script is not None:
paths.append(Path(script.replace("$CLAUDE_PROJECT_DIR", str(project))))
path = Path(script.replace("$CLAUDE_PROJECT_DIR", str(project)))
paths.append((path, str(path)))
return paths


Expand Down
17 changes: 14 additions & 3 deletions src/bmad_loop/process_host.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,20 @@ def hook_interpreter(self) -> str:
return self.shell_quote(str(Path(sys.executable).absolute()))

def shell_quote(self, arg: str) -> str:
# POSIX single-quoting breaks Windows paths; list2cmdline is the stdlib's
# Windows argument quoter (the inverse of how CreateProcess parses argv).
return subprocess.list2cmdline([arg])
# Hook runners hand these commands to a shell, not CreateProcess: Claude
# Code uses Git Bash on Windows (PowerShell without it), and Git Bash eats
# an unquoted backslash — `C:\Users\me\bmad-loop.exe` runs as
# `C:Usersmebmad-loop.exe` and every session stalls to timeout (#773). So
# separators become forward slashes, which Windows and Python accept and
# sh, PowerShell and cmd.exe pass through unchanged; list2cmdline then
# double-quotes a path with spaces as before. Every caller passes a path
# (interpreter, relay executable, hook script), so no backslash here is an
# escape. Known gap: on the PowerShell fallback a double-quoted executable
# followed by arguments is a parse error without `&` (which would break
# sh), so paths WITH spaces stay unsupported there. Claude's exec-form
# `args` would avoid shells entirely but is Claude-only and changes the
# registered JSON shape older versions mis-run.
return subprocess.list2cmdline([arg.replace("\\", "/")])


def _proc_starttime(pid: int) -> float | None:
Expand Down
46 changes: 46 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,52 @@ def write_script_launcher(directory: Path, name: str, body: str) -> Path:
return launcher


class _WindowsLauncher:
"""``Path(sys.argv[0]).absolute()`` as a Windows install presents it, on any
test host: ``str`` is the Windows path the relay registration quotes, while
``is_file``/``os.access`` consult a real launcher file."""

def __init__(self, windows_path: str, real: Path) -> None:
self._windows, self._real = windows_path, real
self.name = "bmad-loop.exe" if os.name == "nt" else "bmad-loop"

def absolute(self) -> _WindowsLauncher:
return self

def is_file(self) -> bool:
return self._real.is_file()

def __fspath__(self) -> str:
return os.fspath(self._real)

def __str__(self) -> str:
return self._windows


def windows_relay_builder(monkeypatch: pytest.MonkeyPatch, tmp_path: Path, windows_path: str):
"""The real `install._hook_command`, run as a Windows install at
``windows_path`` would run it (WindowsProcessHost quoting), on any test host.
The Windows identity is patched in only for the duration of each call, so the
result can stand in for `_hook_command` wherever a caller imported it."""
import bmad_loop.install as install_mod
from bmad_loop.process_host import WindowsProcessHost

launcher = tmp_path / "windows-launcher" / "bmad-loop"
launcher.parent.mkdir(exist_ok=True)
launcher.write_text("#!/bin/sh\n", encoding="utf-8")
launcher.chmod(0o755)
real = install_mod._hook_command

def build(project, profile, canonical_event):
with monkeypatch.context() as m:
m.setattr(install_mod.sys, "argv", [windows_path])
m.setattr(install_mod, "Path", lambda raw: _WindowsLauncher(raw, launcher))
m.setattr(install_mod, "get_process_host", WindowsProcessHost)
return real(project, profile, canonical_event)

return build


# ---- host-shell verify/lifecycle stub commands (single platform-detection spot) ----
# The engine runs verify/plugin-lifecycle commands via the host shell (`sh -c` on
# POSIX, `cmd /c` on Windows), so tests that assert on that machinery need commands
Expand Down
77 changes: 74 additions & 3 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import subprocess
import sys
import types
from pathlib import Path
from pathlib import Path, PureWindowsPath

import pytest
import yaml
Expand Down Expand Up @@ -9713,7 +9713,8 @@ def test_validate_warns_when_registered_relay_uses_another_installation(
config = project.project / ".claude/settings.json"
data = json.loads(config.read_text())
current_command = data["hooks"]["Stop"][0]["hooks"][0]["command"]
current = install_mod.relay_executable(current_command)
# The registered text: on Windows str(Path) would respell init's forward slashes.
current = install_mod.relay_executable_text(current_command)
assert current is not None

previous = project.project / "previous-install" / "bmad-loop"
Expand All @@ -9737,7 +9738,7 @@ def test_validate_warns_when_registered_relay_uses_another_installation(
f"installation's {current} — re-run `bmad-loop init` "
"to update the hook registration"
),
"detail": {"path": str(previous), "expected_path": str(current)},
"detail": {"path": str(previous), "expected_path": current},
}
]
assert any(
Expand All @@ -9748,6 +9749,76 @@ def test_validate_warns_when_registered_relay_uses_another_installation(
)


def _respell_registered_relay(project, respell) -> tuple[str, str]:
"""Rewrite the committed Stop relay's executable text; return (old, new) spellings."""
from bmad_loop import install as install_mod

config = project.project / ".claude/settings.json"
data = json.loads(config.read_text())
command = data["hooks"]["Stop"][0]["hooks"][0]["command"]
current = install_mod.relay_executable_text(command)
assert current is not None
respelled = respell(current)
assert respelled != current
data["hooks"]["Stop"][0]["hooks"][0]["command"] = command.replace(current, respelled, 1)
config.write_text(json.dumps(data), encoding="utf-8")
git(project.project, "add", "-A")
git(project.project, "commit", "-q", "-m", "respell hook registration")
return current, respelled


def _single_relay_stale(doc) -> dict:
findings = [f for f in doc["findings"] if f["check"] == "hooks.relay-stale"]
assert len(findings) == 1, findings
return findings[0]


def test_validate_warns_when_relay_spelling_differs_but_path_is_equal(project, capsys, monkeypatch):
"""`Path` equality normalizes the spelling; init's merge_hooks compares text and
would rewrite this registration, so validate must call it stale (#773)."""
_make_validate_pass(project, monkeypatch, capsys)

def respell(current: str) -> str:
head, _, name = current.rpartition("/")
return f"{head}/./{name}"

current, respelled = _respell_registered_relay(project, respell)
assert Path(respelled) == Path(current) # the premise: a Path comparison sees no change

rc, doc = _validate_json(project.project, capsys)
assert rc == 0 # advisory: the relay still runs
assert _single_relay_stale(doc) == {
"check": "hooks.relay-stale",
"severity": "warning",
"message": (
f"registered hook executable {respelled} differs from this "
f"installation's {current} — re-run `bmad-loop init` "
"to update the hook registration"
),
"detail": {"path": respelled, "expected_path": current},
}


@pytest.mark.skipif(
sys.platform != "win32", reason="Windows path spelling; runs on the Windows leg"
)
def test_validate_warns_on_backslash_windows_relay_registration(project, capsys, monkeypatch):
"""A pre-#773 backslash registration stalls sessions under Git Bash; `Path` and
`str(Path)` both hide it on Windows, so only the registered text flags it."""
_make_validate_pass(project, monkeypatch, capsys)
current, backslashed = _respell_registered_relay(
project, lambda current: str(PureWindowsPath(current))
)
assert "\\" in backslashed

rc, doc = _validate_json(project.project, capsys)
assert rc == 0
finding = _single_relay_stale(doc)
assert finding["severity"] == "warning"
assert finding["detail"] == {"path": backslashed, "expected_path": current}
assert backslashed in finding["message"] and current in finding["message"]


def test_validate_json_reports_null_hook_handlers_without_crashing(project, capsys):
from bmad_loop.install import install_into

Expand Down
28 changes: 27 additions & 1 deletion tests/test_codex_trust.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from pathlib import Path

import pytest
from conftest import install_bmad_config, write_script_launcher
from conftest import install_bmad_config, windows_relay_builder, write_script_launcher

from bmad_loop import cli, codex_trust, probe
from bmad_loop.adapters.profile import ProfileError, get_profile
Expand Down Expand Up @@ -77,6 +77,32 @@ def test_trust_rejects_installed_relay_at_wrong_path(tmp_path, monkeypatch):
assert codex_trust.project_hook_trust(tmp_path, get_profile("codex")).status == "untrusted"


def test_trust_reads_backslash_windows_relay_as_stale_and_forward_slash_as_current(
tmp_path, monkeypatch
):
"""After #773 the expected Windows relay names its executable with forward
slashes. A pre-#773 backslash registration (broken under Git Bash anyway) is
still recognized as the relay but differs from the command init writes now:
it reads untrusted — re-run init — without querying Codex or raising."""
windows_exe = r"C:\Users\me\.local\bin\bmad-loop.exe"
build = windows_relay_builder(monkeypatch, tmp_path, windows_exe)
monkeypatch.setattr(codex_trust, "_hook_command", build)
old = _config(tmp_path, {e: rf"{windows_exe} relay {e}" for e in ("SessionStart", "Stop")})
monkeypatch.setattr(codex_trust, "_hooks_list", lambda *_: pytest.fail("stale relay"))
result = codex_trust.project_hook_trust(tmp_path, get_profile("codex"))
assert result.status == "untrusted" and "not usable" in result.reason

profile = get_profile("codex")
current = _config(tmp_path, {e: build(tmp_path, profile, e) for e in ("SessionStart", "Stop")})
assert current != old
assert current["hooks"]["Stop"][0]["hooks"][0]["command"] == (
"C:/Users/me/.local/bin/bmad-loop.exe relay Stop"
)
monkeypatch.setattr(codex_trust, "resolved_codex_binary", lambda *_: "codex-stub")
monkeypatch.setattr(codex_trust, "_hooks_list", lambda *_: _rpc(tmp_path, current))
assert codex_trust.project_hook_trust(tmp_path, get_profile("codex")).status == "trusted"


def test_trust_degrades_when_installed_relay_disappears(tmp_path, monkeypatch):
_config(tmp_path)

Expand Down
Loading
Loading