From 2b2c324259e5ec12fa1c1fd1e9ea9ea4734fa2fb Mon Sep 17 00:00:00 2001 From: t Date: Sun, 25 Oct 2026 10:22:22 -0700 Subject: [PATCH 1/4] fix(adapters): run Codex sessions with --no-daemon so tools and hooks keep the session env --- CHANGELOG.md | 5 ++ docs/reference/cli.md | 1 + src/bmad_loop/checks.py | 2 + src/bmad_loop/cli.py | 67 +++++++++++++++ src/bmad_loop/codex_trust.py | 31 ++++++- src/bmad_loop/data/profiles/codex.toml | 8 +- tests/test_cli.py | 112 ++++++++++++++++++++++++- tests/test_codex_trust.py | 47 +++++++++++ tests/test_generic_tmux.py | 21 ++++- tests/test_probe.py | 12 +++ tests/test_profile.py | 2 + 11 files changed, 300 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e28886818..ee8b97377 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -107,6 +107,7 @@ breaking changes may land in a minor release. `in_tool`, and the TUI shows `· idle · in ` (#858). - Add `[verify] command_timeout_min` (default 30): per-command verify timeout, previously fixed at 30 minutes (#857). +- `validate` reports `adapter.codex-version` (#901). ### Changed @@ -310,6 +311,10 @@ breaking changes may land in a minor release. a failed pin falls back to the window and is crumbed `pane-pin-failed` (#948). - Refuse to type a nudge into a pane in copy or clock mode, which swallowed it while it counted as sent; the nudge is now recorded as not delivered (#949). +- Run Codex sessions with `--no-daemon` so tools and hooks see the session's + `BMAD_LOOP_*` environment instead of a shared app-server daemon's; Codex >= 0.156 is + now required. A Codex run paused across this upgrade prints the config-changed + advisory once on resume (#901). ### Security diff --git a/docs/reference/cli.md b/docs/reference/cli.md index cadbc962f..b3a2798df 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -625,6 +625,7 @@ Every finding's `check` is one of these ids. Severity is `ok`, `warning` or `pro | `adapter.httpx` | ok / problem | `httpx` is importable for an opencode-http profile | | `adapter.launch-args-unservable` | warning | an opencode-http profile's `launch_args` cannot form a valid serve command | | `adapter.opencode-version` | ok / warning | the OpenCode server API an opencode-http binary's `--version` reports is supported | +| `adapter.codex-version` | ok / warning | a Codex binary accepts `--no-daemon` (>= 0.156) and the profile's `launch_args` pass it | | `adapter.hookless` | ok | the profile is hookless, so hook checks are skipped | | `adapter.kind` | ok / problem | the profile's `adapter` kind is registered | | `adapter.external` | warning | a third-party adapter entry point failed to load | diff --git a/src/bmad_loop/checks.py b/src/bmad_loop/checks.py index 654d58daf..48c24a53e 100644 --- a/src/bmad_loop/checks.py +++ b/src/bmad_loop/checks.py @@ -73,6 +73,8 @@ "adapter.launch-args-unservable", # opencode-http: the server API major `--version` reports (#847) "adapter.opencode-version", + # codex: `--version` accepts --no-daemon and the profile passes it (#901) + "adapter.codex-version", "adapter.kind", "adapter.external", "adapter.external-profile", diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 12882fdc8..ba09120ae 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -689,6 +689,67 @@ def _check_opencode_version(report: ValidationReport, profile_name: str, resolve ) +def _check_codex_version(report: ValidationReport, profile: CLIProfile, resolved: str) -> None: + """``adapter.codex-version``: whether ``resolved`` accepts ``--no-daemon`` + (Codex >= :data:`codex_trust.CODEX_MIN_VERSION`) and whether ``profile`` + passes it. Without it a session can attach to a shared app-server daemon that + runs tools and hooks with the daemon's env, so they never see BMAD_LOOP_*. + OK or a warning, never a problem: validate's rc is a compatibility contract. + + Called only once ``binary_runs`` saw ``--version`` exit 0, like + :func:`_check_opencode_version`, so an unreadable second read warns here.""" + from . import probe as probe_mod + from .codex_trust import CODEX_MIN_VERSION, NO_DAEMON_ARG, parse_codex_version + + floor = ".".join(str(part) for part in CODEX_MIN_VERSION) + first_line = probe_mod.binary_version_text(resolved) + if first_line is None: + report.warn( + "adapter.codex-version", + f"{profile.name}: `--version` exited 0 once, but a second read failed, " + f"timed out, or printed nothing — sessions need Codex >= {floor} for " + f"{NO_DAEMON_ARG}", + {"profile": profile.name, "path": resolved, "output": None}, + ) + return + detail: dict[str, object] = {"profile": profile.name, "path": resolved, "output": first_line} + version = parse_codex_version(first_line) + if version is None: + report.warn( + "adapter.codex-version", + f"{profile.name}: `--version` printed {first_line[:200]!r}, which names no " + f"Codex version — sessions need Codex >= {floor} for {NO_DAEMON_ARG}", + detail, + ) + return + found = ".".join(str(part) for part in version) + detail["version"] = found + if version < CODEX_MIN_VERSION: + detail["verdict"] = "below-floor" + report.warn( + "adapter.codex-version", + f"{profile.name}: codex {found} is older than {floor}: it rejects " + f"{NO_DAEMON_ARG} and every session exits at launch; upgrade Codex", + detail, + ) + elif NO_DAEMON_ARG not in profile.launch_args: + detail["verdict"] = "no-daemon-missing" + report.warn( + "adapter.codex-version", + f"{profile.name}: launch_args lack {NO_DAEMON_ARG}, so sessions can attach " + "to a shared Codex app-server daemon whose tools and hooks never see the " + "BMAD_LOOP_* environment", + detail, + ) + else: + detail["verdict"] = "ok" + report.ok( + "adapter.codex-version", + f"{profile.name}: codex {found} runs sessions with {NO_DAEMON_ARG}", + detail, + ) + + def _report_bmad_config(report: ValidationReport, paths: bmadconfig.ProjectPaths) -> None: """The `bmad-config` ok line, plus the v7 warnings. A v6 config reports exactly as it always has; a v7 one names the era and the active initiative, warns when @@ -1090,6 +1151,12 @@ def cmd_validate(args: argparse.Namespace) -> int: resolved_binary = runnable.get(profile.binary) if resolved_binary is not None: _check_opencode_version(report, profile.name, resolved_binary) + if profile.hooks.dialect == "codex-hooks-json": + # The same packaged-provenance gate as the opencode read above; never + # a problem, for the same rc contract. + resolved_binary = runnable.get(profile.binary) + if resolved_binary is not None: + _check_codex_version(report, profile, resolved_binary) if profile.hookless: report.ok( "adapter.hookless", diff --git a/src/bmad_loop/codex_trust.py b/src/bmad_loop/codex_trust.py index eb022fbe1..4d1564cdd 100644 --- a/src/bmad_loop/codex_trust.py +++ b/src/bmad_loop/codex_trust.py @@ -10,6 +10,7 @@ import json import os import queue +import re import shutil import subprocess import threading @@ -25,7 +26,15 @@ _RELAY_MARKER = "bmad-loop" _PROBE_MARKER = "bmad_loop_probe_hook.py" _TIMEOUT_S = 5.0 -_SAFE_BYPASS_ARG = "--dangerously-bypass-approvals-and-sandbox" +# Runs the client with an in-process app-server instead of a shared background +# daemon, so tools and hooks inherit the session window's BMAD_LOOP_* env. +NO_DAEMON_ARG = "--no-daemon" +# The first Codex that accepts NO_DAEMON_ARG; older ones exit at launch on it. +CODEX_MIN_VERSION: tuple[int, int, int] = (0, 156, 0) +# Launch args that leave hook discovery unchanged: the bypass switch changes +# approvals, and NO_DAEMON_ARG changes where the app-server is hosted. +_HOOK_DISCOVERY_SAFE_ARGS = frozenset({"--dangerously-bypass-approvals-and-sandbox", NO_DAEMON_ARG}) +_VERSION_RE = re.compile(r"(\d+)\.(\d+)\.(\d+)") # The ``unverifiable`` reason when the ``hooks/list`` query itself failed (spawn # error, timeout, protocol breakage) rather than Codex returning a verdict. # Exported so callers can tell a transient query failure from a real answer. @@ -38,9 +47,19 @@ class TrustResult: reason: str +def parse_codex_version(text: str) -> tuple[int, int, int] | None: + """The first ``X.Y.Z`` in ``codex --version`` output, or None when there is + none. Codex prints ``codex-cli 0.164.0``.""" + match = _VERSION_RE.search(text) + if match is None: + return None + major, minor, patch = (int(group) for group in match.groups()) + return major, minor, patch + + def hook_discovery_args_safe(args: tuple[str, ...] | None) -> bool: """Whether extra launch args can leave hook discovery unchanged.""" - return args is None or all(arg == _SAFE_BYPASS_ARG for arg in args) + return args is None or all(arg in _HOOK_DISCOVERY_SAFE_ARGS for arg in args) def resolved_codex_binary(binary: str, env: dict[str, str]) -> str | None: @@ -196,9 +215,13 @@ def project_hook_trust( """Fail closed unless Codex reports every required configured hook trusted.""" if profile.hooks.dialect != "codex-hooks-json": return TrustResult("unverifiable", "hook trust applies only to Codex hooks") - # The default bypass switch changes approvals, not hook configuration. Other + # The bypass switch changes approvals, and --no-daemon changes where the + # app-server is hosted; neither changes which config or hooks are discovered, + # and the `_hooks_list` oracle already runs its own `app-server --stdio`. Other # launch arguments can select a different config and cannot be mirrored here. - if profile.launch_args or not hook_discovery_args_safe(profile.bypass_args): + if not hook_discovery_args_safe(profile.launch_args) or not hook_discovery_args_safe( + profile.bypass_args + ): return TrustResult("unverifiable", "hook trust cannot verify profile launch arguments") config_path = (project / profile.hooks.config_path).resolve() try: diff --git a/src/bmad_loop/data/profiles/codex.toml b/src/bmad_loop/data/profiles/codex.toml index 7a9c4ee0c..0cb172d72 100644 --- a/src/bmad_loop/data/profiles/codex.toml +++ b/src/bmad_loop/data/profiles/codex.toml @@ -1,17 +1,21 @@ -# OpenAI Codex CLI (>= 0.139). Slash commands are NOT expanded when passed as +# OpenAI Codex CLI (>= 0.156). Slash commands are NOT expanded when passed as # the initial prompt argument; the $skill-name mention syntax attaches the # skill instead (skills live in .agents/skills/) — live-verified 2026-06-11. # "use subagents as needed" is required or Codex runs the skill's parallel # phases (e.g. review layers) without spawning subagents. Codex defines # SessionEnd and SubagentStop hook events, but bmad-loop maps only SessionStart # and Stop; the adapter's window-death fallback covers crashes. +# `--no-daemon` keeps tools and hooks in this client's process, so they inherit +# the window's BMAD_LOOP_* env. A shared app-server daemon (Codex's default since +# 0.157) runs them with the daemon's env instead. name = "codex" binary = "codex" +launch_args = ["--no-daemon"] prompt_template = "Use the ${skill} skill now, and use subagents as needed: {args}" bypass_args = ["--dangerously-bypass-approvals-and-sandbox"] model_flag = "--model" usage_parser = "codex-rollout" -first_run_note = "run `codex` once in the project and accept BOTH prompts: workspace trust, then 'Hooks need review' -> 'Trust all and continue' (untrusted hooks silently never fire); requires Codex >= 0.139" +first_run_note = "run `codex` once in the project and accept BOTH prompts: workspace trust, then 'Hooks need review' -> 'Trust all and continue' (untrusted hooks silently never fire); requires Codex >= 0.156" skill_tree = ".agents/skills" # Project-scoped Codex config (MCP servers under [mcp_servers]) — gitignored, so # a worktree checkout omits it and isolated sessions lose their MCP servers. diff --git a/tests/test_cli.py b/tests/test_cli.py index 7d7787836..aff4d9d4a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -595,7 +595,7 @@ def test_dry_run_sprint_output_is_pinned(project, capsys): out, err = capsys.readouterr() review = ( - 'codex "Use the $bmad-build-auto skill now, and use subagents as needed: ' + 'codex --no-daemon "Use the $bmad-build-auto skill now, and use subagents as needed: ' '" --dangerously-bypass-approvals-and-sandbox --model gpt-5-codex' ) assert rc == 0 @@ -11466,6 +11466,116 @@ def test_validate_opencode_version_warns_when_second_read_fails(project, capsys, assert "refuse to start" in findings[0]["message"] +def _codex_version_findings( + project, capsys, monkeypatch, version_line: str | None, *, overlay_launch_args=None +): + """`adapter.codex-version` findings for a codex project whose binary resolves, + runs, and prints ``version_line`` for `--version`. With ``overlay_launch_args`` + the triage role runs a project overlay profile `codex-mine` with those launch + args beside the shipped `codex`. + + Validates twice, the second time with the check stubbed out, and asserts the + rc is the same: the check warns and never fails, whatever it finds. The rc + itself is not asserted: other gates legitimately vary by machine.""" + from importlib import resources + + install_bmad_config(project) + policy = '[adapter]\nname = "codex"\n' + if overlay_launch_args is not None: + shipped = ( + resources.files("bmad_loop.data") + .joinpath("profiles/codex.toml") + .read_text(encoding="utf-8") + ) + lines = [ + ( + f"launch_args = {json.dumps(list(overlay_launch_args))}" + if line.startswith("launch_args") + else 'name = "codex-mine"' if line.startswith("name =") else line + ) + for line in shipped.splitlines() + ] + overlay = project.project / ".bmad-loop" / "profiles" / "codex-mine.toml" + overlay.parent.mkdir(parents=True, exist_ok=True) + overlay.write_text("\n".join(lines) + "\n", encoding="utf-8") + policy += '[adapter.triage]\nname = "codex-mine"\n' + _write_policy(project.project, policy) + monkeypatch.setattr(cli.shutil, "which", lambda tool, *_a, **_kw: f"/usr/bin/{tool}") + monkeypatch.setattr(probe_mod, "binary_runs", lambda *_a, **_kw: 0) + monkeypatch.setattr(probe_mod, "binary_version_text", lambda *_a, **_kw: version_line) + rc = cli.main(["validate", "--project", str(project.project), "--json"]) + captured = capsys.readouterr() + assert captured.out, captured.err + doc = json.loads(captured.out) + findings = [f for f in doc["findings"] if f["check"] == "adapter.codex-version"] + assert all(f["severity"] != "problem" for f in findings) + monkeypatch.setattr(cli, "_check_codex_version", lambda *_a, **_kw: None) + assert cli.main(["validate", "--project", str(project.project), "--json"]) == rc + capsys.readouterr() + return findings + + +def test_validate_codex_version_ok_on_current(project, capsys, monkeypatch): + """#901: a Codex that accepts --no-daemon, under the shipped profile.""" + findings = _codex_version_findings(project, capsys, monkeypatch, "codex-cli 0.164.0") + assert [f["severity"] for f in findings] == ["ok"] + assert findings[0]["detail"] == { + "profile": "codex", + "path": "/usr/bin/codex", + "output": "codex-cli 0.164.0", + "version": "0.164.0", + "verdict": "ok", + } + + +def test_validate_codex_version_warns_below_floor(project, capsys, monkeypatch): + """A Codex older than 0.156 rejects --no-daemon, so every session exits at + launch: a warning, never a problem (validate's rc is a compatibility contract).""" + findings = _codex_version_findings(project, capsys, monkeypatch, "codex-cli 0.155.1") + assert [f["severity"] for f in findings] == ["warning"] + assert findings[0]["detail"]["verdict"] == "below-floor" + assert "older than 0.156" in findings[0]["message"] + assert "upgrade Codex" in findings[0]["message"] + + +def test_validate_codex_version_warns_when_overlay_drops_no_daemon(project, capsys, monkeypatch): + """A project overlay profile without --no-daemon lets its sessions attach to a + shared daemon. The shipped profile beside it stays ok. + + Ablation target: drop the launch_args test and the overlay reads ok.""" + findings = _codex_version_findings( + project, capsys, monkeypatch, "codex-cli 0.164.0", overlay_launch_args=() + ) + by_profile = {f["detail"]["profile"]: f for f in findings} + assert by_profile["codex"]["severity"] == "ok" + assert by_profile["codex-mine"]["severity"] == "warning" + assert by_profile["codex-mine"]["detail"]["verdict"] == "no-daemon-missing" + assert "BMAD_LOOP_* environment" in by_profile["codex-mine"]["message"] + + +def test_validate_codex_version_overlay_keeping_no_daemon_is_ok(project, capsys, monkeypatch): + findings = _codex_version_findings( + project, capsys, monkeypatch, "codex-cli 0.164.0", overlay_launch_args=("--no-daemon",) + ) + assert sorted(f["detail"]["profile"] for f in findings) == ["codex", "codex-mine"] + assert {f["severity"] for f in findings} == {"ok"} + + +@pytest.mark.parametrize("version_line", [None, "codex-cli unknown"]) +def test_validate_codex_version_warns_on_unreadable_version( + project, capsys, monkeypatch, version_line +): + """The check runs only after `binary_runs` saw `--version` exit 0, so a read + that now fails or names no version is reported by no other finding. + + Ablation target: return early on an unreadable read and this sees no finding.""" + findings = _codex_version_findings(project, capsys, monkeypatch, version_line) + assert [f["severity"] for f in findings] == ["warning"] + assert findings[0]["detail"]["output"] == version_line + assert "verdict" not in findings[0]["detail"] + assert "0.156" in findings[0]["message"] + + def test_validate_warns_when_no_desktop_notifier(project, monkeypatch, capsys): """#231: notify.desktop defaults on, but when this platform resolves no notifier the setting is silently inert — validate must surface it as a diff --git a/tests/test_codex_trust.py b/tests/test_codex_trust.py index 66fe41ace..fdf977f2b 100644 --- a/tests/test_codex_trust.py +++ b/tests/test_codex_trust.py @@ -549,3 +549,50 @@ def test_continuous_unrelated_messages_cannot_extend_rpc_deadline(tmp_path, monk monkeypatch.setattr(codex_trust, "_TIMEOUT_S", 0.1) with pytest.raises(TimeoutError): codex_trust._hooks_list(str(script), tmp_path, {}) + + +def test_trust_accepts_no_daemon_launch_arg(tmp_path, monkeypatch): + """The shipped profile launches with `--no-daemon`, which changes where the + app-server is hosted, not which hooks are discovered: it resolves trusted, + not unverifiable. Ablation target: drop NO_DAEMON_ARG from the safe set.""" + data = _config(tmp_path) + monkeypatch.setattr(codex_trust, "resolved_codex_binary", lambda *_: "codex-stub") + monkeypatch.setattr(codex_trust, "_hooks_list", lambda *_: _rpc(tmp_path, data)) + profile = get_profile("codex") + assert profile.launch_args == (codex_trust.NO_DAEMON_ARG,) + assert codex_trust.project_hook_trust(tmp_path, profile).status == "trusted" + + +def test_trust_still_refuses_unknown_launch_args(tmp_path, monkeypatch): + data = _config(tmp_path) + monkeypatch.setattr(codex_trust, "resolved_codex_binary", lambda *_: "codex-stub") + monkeypatch.setattr(codex_trust, "_hooks_list", lambda *_: _rpc(tmp_path, data)) + profile = replace( + get_profile("codex"), launch_args=(codex_trust.NO_DAEMON_ARG, "--profile", "x") + ) + result = codex_trust.project_hook_trust(tmp_path, profile) + assert result.status == "unverifiable" + assert "launch arguments" in result.reason + + +def test_hook_discovery_args_safe_accepts_no_daemon(): + assert codex_trust.hook_discovery_args_safe(None) + assert codex_trust.hook_discovery_args_safe(("--no-daemon",)) + assert codex_trust.hook_discovery_args_safe( + ("--no-daemon", "--dangerously-bypass-approvals-and-sandbox") + ) + assert not codex_trust.hook_discovery_args_safe(("--no-daemon", "-c", "x=1")) + + +@pytest.mark.parametrize( + ("text", "expected"), + [ + ("codex-cli 0.164.0", (0, 164, 0)), + ("0.156.0", (0, 156, 0)), + ("codex-cli 0.155.1-alpha.2", (0, 155, 1)), + ("codex-cli", None), + ("", None), + ], +) +def test_parse_codex_version(text, expected): + assert codex_trust.parse_codex_version(text) == expected diff --git a/tests/test_generic_tmux.py b/tests/test_generic_tmux.py index 1c1240bd2..ff8bed4e4 100644 --- a/tests/test_generic_tmux.py +++ b/tests/test_generic_tmux.py @@ -391,12 +391,31 @@ def test_build_command_codex_renders_skill_mention(tmp_path): cmd = adapter.build_command(make_spec(tmp_path)) binary = shutil.which("codex") or "codex" assert cmd.startswith( - f"{binary} 'Use the $bmad-dev-auto skill now, and use subagents as needed: 1-1-a'" + f"{binary} --no-daemon 'Use the $bmad-dev-auto skill now, and use subagents as needed: 1-1-a'" ) assert "--dangerously-bypass-approvals-and-sandbox" in cmd assert cmd.endswith("--model sonnet") +def test_build_command_codex_passes_no_daemon_before_the_prompt(tmp_path): + """#901: `--no-daemon` keeps tools and hooks in the window's env, and it is a + top-level Codex flag, so it sits right after the binary.""" + adapter = make_adapter(tmp_path, profile_name="codex") + argv = adapter.interactive_argv(make_spec(tmp_path)) + assert argv[1] == "--no-daemon" + assert argv[2].startswith("Use the $bmad-dev-auto skill now") + + +def test_codex_extra_args_keep_no_daemon(tmp_path): + """A user `extra_args` replaces the profile's bypass_args, never its + launch_args, so `--no-daemon` survives it.""" + adapter = make_adapter(tmp_path, profile_name="codex", extra_args=("--custom-flag",)) + argv = adapter.interactive_argv(make_spec(tmp_path)) + assert argv[1] == "--no-daemon" + assert "--custom-flag" in argv + assert "--dangerously-bypass-approvals-and-sandbox" not in argv + + def test_build_command_gemini_uses_interactive_flag(tmp_path): adapter = make_adapter(tmp_path, profile_name="gemini") cmd = adapter.build_command(make_spec(tmp_path)) diff --git a/tests/test_probe.py b/tests/test_probe.py index a0c78ab64..4c1d625c2 100644 --- a/tests/test_probe.py +++ b/tests/test_probe.py @@ -1239,3 +1239,15 @@ def test_cli_json_without_workspace_reports_null(tmp_path, capsys): err_contains="ok:", ) assert doc["workspace_trust"] is None + + +def test_probe_argv_carries_codex_launch_args(monkeypatch): + """probe-adapter launches Codex as a session would: the shipped + `--no-daemon` launch arg sits right after the binary, ahead of the prompt.""" + from bmad_loop import codex_trust + + monkeypatch.setattr(codex_trust, "resolved_codex_binary", lambda *_: "/bin/codex") + argv = probe._probe_argv(get_profile("codex"), "codex", probe.Hints()) + assert argv[:2] == ["/bin/codex", "--no-daemon"] + assert argv[2] == probe.PROBE_PROMPT + assert "--dangerously-bypass-approvals-and-sandbox" in argv[3:] diff --git a/tests/test_profile.py b/tests/test_profile.py index c65113f91..98f0a5358 100644 --- a/tests/test_profile.py +++ b/tests/test_profile.py @@ -41,6 +41,8 @@ def test_builtin_profiles_load(): assert {"claude", "codex", "gemini", "opencode-http"} <= set(profiles) assert profiles["claude"].usage_parser == "claude-jsonl" assert profiles["codex"].hooks.dialect == "codex-hooks-json" + # tools and hooks run in the client, not a shared app-server daemon (#901) + assert profiles["codex"].launch_args == ("--no-daemon",) assert "SessionEnd" not in profiles["codex"].hooks.events # codex has no such hook assert profiles["gemini"].hooks.events["AfterAgent"] == "Stop" assert profiles["gemini"].launch_args == ("-i",) From ce3dee4b2db82b4329237fdeec69adc02b8d843f Mon Sep 17 00:00:00 2001 From: t Date: Sun, 25 Oct 2026 10:34:02 -0700 Subject: [PATCH 2/4] feat(adapters): journal session-start-missing and name the Codex daemon in triage failures --- CHANGELOG.md | 3 + docs/reference/configuration.md | 1 + src/bmad_loop/adapters/generic.py | 58 +++++++++ src/bmad_loop/codex_trust.py | 7 + src/bmad_loop/data/settings/core.toml | 6 + src/bmad_loop/policy.py | 12 ++ src/bmad_loop/sweep.py | 14 +- tests/test_generic_tmux.py | 181 ++++++++++++++++++++++++++ tests/test_policy.py | 19 +++ tests/test_portability_guard.py | 16 +++ tests/test_sweep.py | 48 +++++++ 11 files changed, 364 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee8b97377..a6875ab66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -108,6 +108,9 @@ breaking changes may land in a minor release. - Add `[verify] command_timeout_min` (default 30): per-command verify timeout, previously fixed at 30 minutes (#857). - `validate` reports `adapter.codex-version` (#901). +- Journal `session-start-missing` when a session's CLI reports no SessionStart within + `limits.session_start_grace_s` (default 120 s); Codex failures with no hook events + name the shared-daemon cause in sweep escalation text (#901). ### Changed diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 6a2e9f5ea..a3f33a1b2 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -93,6 +93,7 @@ Cycle/attempt caps, timeout & token budget. | `teardown_grace_s` | integer | `20` | ≥ 0 | verified session teardown: poll the killed window up to this long, then force-kill its pane pids and re-kill the window · 0 = single unverified best-effort kill | | `stop_without_result_nudges` | integer | `1` | ≥ 0 | | | `dev_stall_grace_s` | integer | `600` | ≥ 0 | silence grace for a dev/review session, armed at launch and re-armed by transport activity or fresh Stop/idle evidence, before bounded nudges/stalled classification · also the transcript-idle notice threshold (journal session-idle/session-active, TUI idle age) · 0 = no launch timer (and no idle notice), but a result-less turn end still fails fast | +| `session_start_grace_s` | integer | `120` | ≥ 0 | seconds a session whose CLI maps SessionStart may run without one before a journal-only session-start-missing notice; 0 = off | | `dev_stall_nudges` | integer | `2` | ≥ 0 | best-effort wake nudges for a silent dev/review session on grace expiry before it is called stalled; transport activity re-arms the grace and fresh Stop/idle evidence restores this budget · 0 = stall on grace expiry | | `dev_stall_nudges_cap` | integer | `6` | ≥ 0 | total (never-restored) best-effort stall nudges for a dev/review session; bounds launch-time recovery and Stop/idle budget refills because an accepted nudge does not guarantee a wake · 0 = stall on first grace expiry | | `workflow_stall_nudges_cap` | integer | `3` | ≥ 0 | total (never-restored) stall nudges for an injected plugin-workflow session before it is called stalled; bounds a session that finished its work but never wrote its completion marker · 0 = stall on first grace expiry | diff --git a/src/bmad_loop/adapters/generic.py b/src/bmad_loop/adapters/generic.py index 65a0de6d5..0893e273c 100644 --- a/src/bmad_loop/adapters/generic.py +++ b/src/bmad_loop/adapters/generic.py @@ -958,6 +958,10 @@ def __init__( # and belongs to every pane-driven session the same. Same knob, no new # policy field; 0 disables the notice along with the stall timer. self._idle_threshold_s = float(policy.limits.dev_stall_grace_s) + # Grace for the one-shot `session-start-missing` journal notice + # (`_note_session_start_missing`). Observation only, like the idle notice, + # so every adapter reads it, the plain one included; 0 disables it. + self._session_start_grace_s = float(policy.limits.session_start_grace_s) # Wake-nudges to spend on grace expiry before stalling. 0 here is moot for # the base adapter (grace 0 never opens the window); the dev adapter sets # it from policy so an idle wait is re-invoked rather than killed outright. @@ -1328,6 +1332,10 @@ def sample_frame() -> None: # stalls or kills (#680 item 2 stays open), and `stall_deadline` is # neither consulted nor touched. idle = _IdleTracker() + # An admitted SessionStart arrived, and the one-shot `session-start-missing` + # notice was journaled; see `_note_session_start_missing`. Observes only. + saw_session_start = False + session_start_noted = False # A transcript can grow for an initial user prompt or the loop's own # wake nudge, neither of which proves model work. Latch model-side @@ -1501,6 +1509,10 @@ def produced_work() -> bool: # has named the transcript. if transcript_path: sample_transcript(transcript_path, now) + if not saw_session_start and not session_start_noted: + session_start_noted = self._note_session_start_missing( + handle, now - loop_started + ) self._write_heartbeat( handle.task_id, { @@ -1949,6 +1961,7 @@ def produced_work() -> bool: try_resolve() if event.event == "SessionStart": + saw_session_start = True continue if event.event == "Notification" or event.event in PARKED_EVENTS: # Parked-session signal (DW-348): latch it, never act on it here. @@ -2290,6 +2303,51 @@ def flush_active() -> None: else: idle.open_since = idle.last_change_wall + def _note_session_start_missing(self, handle: SessionHandle, waited_s: float) -> bool: + """Journal one `session-start-missing` (`task_id`, `waited_s`, `grace_s`, + `cli`) for a session whose profile maps SessionStart but that has run + `limits.session_start_grace_s` without one; True once written, so the + caller latches it. + + A CLI whose hooks run without the window's BMAD_LOOP_* env (a Codex + session attached to a shared app-server daemon) fires its hooks into a + relay that exits silently, so the session would otherwise look healthy + until `session_timeout_min`. A `codex-hooks-json` profile adds `hint`, + naming that cause. Observation only: no deadline, nudge or verdict reads + it, and it fires on the plain adapter too, which arms no stall timer. No + journal, grace 0, a window that is not alive (or a liveness probe that + raised) or an unwritable journal writes nothing and leaves it unlatched, + and the session runs on.""" + if ( + self.journal is None + or "SessionStart" not in self.profile.hooks.events.values() + or self._session_start_grace_s <= 0 + or waited_s < self._session_start_grace_s + ): + return False + try: + if not self._window_alive(handle): + return False + except MultiplexerError: + return False + extra: dict[str, str] = {} + if self.profile.hooks.dialect == "codex-hooks-json": + from ..codex_trust import DAEMON_ENV_HINT + + extra["hint"] = DAEMON_ENV_HINT + try: + self.journal.append( + "session-start-missing", + task_id=handle.task_id, + waited_s=int(waited_s), + grace_s=self._session_start_grace_s, + cli=self.profile.name, + **extra, + ) + except OSError: + return False + return True + def _window_alive(self, handle: SessionHandle) -> bool: return self.mux.window_alive(self.session_name, handle.native_id) diff --git a/src/bmad_loop/codex_trust.py b/src/bmad_loop/codex_trust.py index 4d1564cdd..2eb933d34 100644 --- a/src/bmad_loop/codex_trust.py +++ b/src/bmad_loop/codex_trust.py @@ -31,6 +31,13 @@ NO_DAEMON_ARG = "--no-daemon" # The first Codex that accepts NO_DAEMON_ARG; older ones exit at launch on it. CODEX_MIN_VERSION: tuple[int, int, int] = (0, 156, 0) +# Why a Codex session can report no hook events: the sweep's failure text and +# the adapter's session-start-missing journal notice both carry it. +DAEMON_ENV_HINT = ( + "a Codex session attached to a shared app-server daemon runs tools and hooks" + f" without the BMAD_LOOP_* environment; keep {NO_DAEMON_ARG} in the profile's" + f" launch_args (Codex >= {CODEX_MIN_VERSION[0]}.{CODEX_MIN_VERSION[1]})" +) # Launch args that leave hook discovery unchanged: the bypass switch changes # approvals, and NO_DAEMON_ARG changes where the app-server is hosted. _HOOK_DISCOVERY_SAFE_ARGS = frozenset({"--dangerously-bypass-approvals-and-sandbox", NO_DAEMON_ARG}) diff --git a/src/bmad_loop/data/settings/core.toml b/src/bmad_loop/data/settings/core.toml index d8451d867..8d1ca1cbb 100644 --- a/src/bmad_loop/data/settings/core.toml +++ b/src/bmad_loop/data/settings/core.toml @@ -162,6 +162,12 @@ minimum = 0 default_ref = "LimitsPolicy.dev_stall_grace_s" description = "silence grace for a dev/review session, armed at launch and re-armed by transport activity or fresh Stop/idle evidence, before bounded nudges/stalled classification · also the transcript-idle notice threshold (journal session-idle/session-active, TUI idle age) · 0 = no launch timer (and no idle notice), but a result-less turn end still fails fast" [[section.field]] +key = "session_start_grace_s" +kind = "int" +minimum = 0 +default_ref = "LimitsPolicy.session_start_grace_s" +description = "seconds a session whose CLI maps SessionStart may run without one before a journal-only session-start-missing notice; 0 = off" +[[section.field]] key = "dev_stall_nudges" kind = "int" minimum = 0 diff --git a/src/bmad_loop/policy.py b/src/bmad_loop/policy.py index 7a582c22c..525aa23fb 100644 --- a/src/bmad_loop/policy.py +++ b/src/bmad_loop/policy.py @@ -127,6 +127,10 @@ class LimitsPolicy: # the idle notice) while retaining fail-fast handling when a turn ends without # a terminal spec/result. dev_stall_grace_s: int = 600 + # seconds a session whose CLI maps SessionStart may run without one before + # the journal gets a one-shot session-start-missing notice. Observation only: + # no deadline, nudge or verdict depends on it. 0 = off. + session_start_grace_s: int = 120 # how many best-effort wake nudges a silent dev/review session receives on # dev_stall_grace_s expiry before it is called stalled. Transport activity # re-arms the grace without spending a nudge; fresh Stop/idle evidence also @@ -1002,6 +1006,9 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy: dev_stall_grace_s=_typed_int( limits_d, "limits", "dev_stall_grace_s", LimitsPolicy.dev_stall_grace_s ), + session_start_grace_s=_typed_int( + limits_d, "limits", "session_start_grace_s", LimitsPolicy.session_start_grace_s + ), dev_stall_nudges=_typed_int( limits_d, "limits", "dev_stall_nudges", LimitsPolicy.dev_stall_nudges ), @@ -1063,6 +1070,10 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy: ) if limits.dev_stall_grace_s < 0: raise PolicyError(f"limits.dev_stall_grace_s must be >= 0: got {limits.dev_stall_grace_s}") + if limits.session_start_grace_s < 0: + raise PolicyError( + f"limits.session_start_grace_s must be >= 0: got {limits.session_start_grace_s}" + ) if limits.dev_stall_nudges < 0: raise PolicyError(f"limits.dev_stall_nudges must be >= 0: got {limits.dev_stall_nudges}") if limits.dev_stall_nudges_cap < 0: @@ -1498,6 +1509,7 @@ def _fold_deprecated_engine( teardown_grace_s = 20 # verified teardown: poll a killed session window up to this long, then force-kill its pane pids and re-kill (#157). 0 = single unverified best-effort kill stop_without_result_nudges = 1 dev_stall_grace_s = 600 # silence grace armed at dev/review launch and re-armed by transport activity or fresh Stop/idle evidence before bounded recovery; also the transcript-idle notice threshold (journal session-idle/session-active, TUI idle age). 0 = no launch timer (and no idle notice), but a result-less turn end still fails fast +session_start_grace_s = 120 # seconds a session whose CLI maps SessionStart may run without one before a journal-only session-start-missing notice; 0 = off dev_stall_nudges = 2 # best-effort wake nudges per silent grace before stalling; fresh Stop/idle evidence restores this budget. 0 = stall on grace expiry dev_stall_nudges_cap = 6 # total (never-restored) nudge bound per dev/review session; bounds launch-time recovery and Stop/idle budget refills because an accepted nudge does not guarantee a wake. 0 = stall on first grace expiry workflow_stall_nudges_cap = 3 # total (never-restored) stall nudges for an injected plugin-workflow session before it is called stalled; bounds a session that finished its work but never wrote its completion marker. 0 = stall on first grace expiry diff --git a/src/bmad_loop/sweep.py b/src/bmad_loop/sweep.py index df2310a81..d4384eaca 100644 --- a/src/bmad_loop/sweep.py +++ b/src/bmad_loop/sweep.py @@ -26,6 +26,7 @@ from . import deferredwork, gates, sprintstatus, ticketstore, verify from .adapters.generic import load_result_document from .bmadconfig import ProjectPaths +from .codex_trust import DAEMON_ENV_HINT from .engine import ( Engine, RunPaused, @@ -1400,7 +1401,8 @@ def _diagnostic_suffix(diagnostic: dict[str, Any] | None) -> str: what was on disk and which hook events arrived, so an operator whose event channel is miswired debugs the channel, not the agent's output. Nested-CLI events attribution dropped are named too: ``foreign ignored`` counts foreign - ids (#767), ``foreign id-less events ignored`` the id-less events lineage dropped (DW-507).""" + ids (#767), ``foreign id-less events ignored`` the id-less events lineage dropped (DW-507). + A ``hint`` naming the likely cause closes the tail.""" if diagnostic is None: return "" artifact = str(diagnostic["artifact"]) @@ -1411,6 +1413,8 @@ def _diagnostic_suffix(diagnostic: dict[str, Any] | None) -> str: hooks += f"; foreign ignored: {diagnostic['hook_foreign_ids']}" if diagnostic.get("hook_foreign_idless"): hooks += f"; foreign id-less events ignored: {diagnostic['hook_foreign_idless']}" + if diagnostic.get("hint"): + hooks += f"; hint: {diagnostic['hint']}" return f" [result.json: {artifact}; hook events: {hooks}]" @@ -5083,6 +5087,11 @@ def _session_failure_diagnostic( ``hook_foreign_idless`` the id-less events it dropped — only a trusted relay-side lineage ``mismatch`` drops those (DW-507). + ``hint``, present only when ``hook_events`` is ``none`` on a Codex + (``codex-hooks-json``) profile: a session attached to a shared app-server + daemon runs its hooks without the window's BMAD_LOOP_* env, so the relay + drops every event and the skill finds no sweep mode. + Observation degrades, never raises: this runs on a failure path that must still reach its retry/escalation decision.""" # The id `_run_session` just minted for this attempt: the sweep dispatches @@ -5136,6 +5145,9 @@ def _session_failure_diagnostic( admitted, foreign = attribute_events(events) kinds = {event.event for event in admitted} diagnostic["hook_events"] = _hook_verdict(kinds) + hooks = getattr(getattr(self.adapters["triage"], "profile", None), "hooks", None) + if not kinds and getattr(hooks, "dialect", None) == "codex-hooks-json": + diagnostic["hint"] = DAEMON_ENV_HINT diagnostic["hook_event_kinds"] = sorted(kinds) diagnostic["hook_event_count"] = len(admitted) diagnostic["hook_foreign_ids"] = len(foreign) diff --git a/tests/test_generic_tmux.py b/tests/test_generic_tmux.py index ff8bed4e4..6d53ea04e 100644 --- a/tests/test_generic_tmux.py +++ b/tests/test_generic_tmux.py @@ -12421,3 +12421,184 @@ def make_spec(halt: bool) -> SessionSpec: assert ( adapter._synth_result(_dev_handle(), make_spec(False), wait=False, dead_window=True) is None ) + + +# ------------------------------- session-start-missing notice +# +# Observation only. A session whose profile maps SessionStart but that has run +# `limits.session_start_grace_s` without one gets one latched journal entry; a +# Codex profile's entry names the shared-daemon cause. Nothing here ends, nudges +# or reclassifies the session. + + +def _start_missing_adapter(tmp_path, monkeypatch, *, hooks_from="claude", grace=None): + """The plain adapter (the sweep's triage role, `_stall_grace_s` 0) with a fake + journal and the steerable clock; `hooks_from` swaps in that profile's name + and hook spec, `grace` overrides the policy's default.""" + adapter, clock, sent = _budget_adapter(tmp_path, monkeypatch) + assert adapter._stall_grace_s == 0.0 + if hooks_from != "claude": + donor = get_profile(hooks_from) + adapter.profile = dataclasses.replace(adapter.profile, name=donor.name, hooks=donor.hooks) + if grace is not None: + adapter._session_start_grace_s = grace + adapter.journal = _FakeJournal() + return adapter, clock, sent + + +def _start_missing_script(clock, *, end_at=10): + """on_call hook: one heartbeat per watcher call from the second on (waited 30, + 60, 90 ...), then past `spec.timeout_s` at `end_at`.""" + + def script(call_n): + if call_n >= 2: + clock["t"] += generic.HEARTBEAT_INTERVAL_S + if call_n == end_at: + clock["t"] += 100_000.0 + + return script + + +def _start_missing_entries(adapter): + return [e for e in adapter.journal.entries if e["kind"] == "session-start-missing"] + + +def _run_start_missing(adapter, clock, tmp_path, events=()): + adapter.watcher = _ScriptedWatcher(list(events), on_call=_start_missing_script(clock)) + return adapter.wait_for_completion( + _budget_handle(), _budget_spec(tmp_path, mode="off", timeout_s=5000.0) + ) + + +def test_session_start_missing_fires_once_after_grace(tmp_path, monkeypatch): + """No SessionStart by the policy's default grace (120 s): one entry at the first + heartbeat at or past it, latched for every later tick. + + ABLATION: drop the `session_start_noted` latch and every later heartbeat + appends another entry.""" + adapter, clock, sent = _start_missing_adapter(tmp_path, monkeypatch) + assert adapter._session_start_grace_s == 120.0 # read from policy + result = _run_start_missing(adapter, clock, tmp_path) + assert result.status == "timeout" + assert sent == [] + assert _start_missing_entries(adapter) == [ + { + "kind": "session-start-missing", + "task_id": "b-1", + "waited_s": 120, + "grace_s": 120.0, + "cli": "claude", + } + ] + + +def test_session_start_missing_absent_when_session_start_arrives(tmp_path, monkeypatch): + """An admitted SessionStart before the grace crossing means no entry. + + ABLATION: drop `saw_session_start = True` and the entry fires at 120 s.""" + adapter, clock, _ = _start_missing_adapter(tmp_path, monkeypatch) + transcript = tmp_path / "t.jsonl" + transcript.write_bytes(b"{}\n") + result = _run_start_missing(adapter, clock, tmp_path, events=[_start_event(transcript)]) + assert result.status == "timeout" + assert _start_missing_entries(adapter) == [] + + +def test_session_start_missing_disabled_at_zero_grace(tmp_path, monkeypatch): + """`limits.session_start_grace_s = 0` turns the notice off. + + ABLATION: drop the `grace <= 0` gate and the entry fires on the first + heartbeat (waited 0 >= grace 0).""" + adapter, clock, _ = _start_missing_adapter(tmp_path, monkeypatch, grace=0.0) + result = _run_start_missing(adapter, clock, tmp_path) + assert result.status == "timeout" + assert _start_missing_entries(adapter) == [] + + +def test_session_start_missing_skipped_for_profile_without_session_start(tmp_path, monkeypatch): + """A profile whose hooks never map SessionStart (antigravity maps only Stop) + cannot be missing one. Copilot maps it from a lower-case native name, so the + gate reads the canonical side of the table. + + ABLATION: drop the SessionStart gate and the antigravity row fires; test the + native keys instead of the canonical values and the copilot row stays silent.""" + adapter, clock, _ = _start_missing_adapter(tmp_path, monkeypatch, hooks_from="antigravity") + assert "SessionStart" not in adapter.profile.hooks.events.values() + _run_start_missing(adapter, clock, tmp_path) + assert _start_missing_entries(adapter) == [] + + other = tmp_path / "copilot" + other.mkdir() + adapter, clock, _ = _start_missing_adapter(other, monkeypatch, hooks_from="copilot") + _run_start_missing(adapter, clock, other) + assert [e["cli"] for e in _start_missing_entries(adapter)] == ["copilot"] + + +def test_session_start_missing_carries_codex_hint_only_for_codex_dialect(tmp_path, monkeypatch): + """A `codex-hooks-json` profile's entry names the shared app-server daemon and + the `--no-daemon` flag; other dialects carry no `hint`. + + ABLATION: drop the dialect gate and the claude entry carries the hint too.""" + from bmad_loop.codex_trust import DAEMON_ENV_HINT, NO_DAEMON_ARG + + adapter, clock, _ = _start_missing_adapter(tmp_path, monkeypatch, hooks_from="codex") + _run_start_missing(adapter, clock, tmp_path) + (entry,) = _start_missing_entries(adapter) + assert entry["cli"] == "codex" + assert entry["hint"] == DAEMON_ENV_HINT + assert NO_DAEMON_ARG in entry["hint"] and "BMAD_LOOP_*" in entry["hint"] + + other = tmp_path / "claude" + other.mkdir() + adapter, clock, _ = _start_missing_adapter(other, monkeypatch) + _run_start_missing(adapter, clock, other) + (entry,) = _start_missing_entries(adapter) + assert "hint" not in entry + + +def test_session_start_missing_never_ends_or_nudges_the_session(tmp_path, monkeypatch): + """The notice fires mid-session, and the session still completes on its later + Stop with the verdict a grace-0 run reaches: no nudge, no stall, no early end. + + ABLATION: return a "stalled" verdict from the notice site and the first run + ends before its Stop.""" + + def run(root, grace): + root.mkdir() + adapter, clock, sent = _start_missing_adapter(root, monkeypatch, grace=grace) + transcript = root / "t.jsonl" + transcript.write_bytes(b"{}\n") + (adapter.tasks_dir / "b-1" / "result.json").write_text('{"ok": true}') + events = [None] * 6 + [_stop_event("b-1", "sess", str(transcript))] + result = _run_start_missing(adapter, clock, root, events=events) + return adapter, result, sent + + adapter, result, sent = run(tmp_path / "on", 120.0) + assert len(_start_missing_entries(adapter)) == 1 + _, control, control_sent = run(tmp_path / "off", 0.0) + assert result.status == control.status == "completed" + assert result.result_json == control.result_json == {"ok": True} + assert result.stop_seen is control.stop_seen is True + assert sent == control_sent == [] + + +def test_session_start_missing_journal_write_failure_is_swallowed(tmp_path, monkeypatch): + """An unwritable journal must not end the session: the write is retried on the + next heartbeat and the session runs to its own end. + + ABLATION: drop the `except OSError` and the session raises out of the loop.""" + adapter, clock, _ = _start_missing_adapter(tmp_path, monkeypatch) + attempts: list[str] = [] + + class _Failing(_FakeJournal): + def append(self, kind, **fields): + attempts.append(kind) + if len(attempts) == 1: + raise OSError("disk full") + super().append(kind, **fields) + + adapter.journal = _Failing() + result = _run_start_missing(adapter, clock, tmp_path) + assert result.status == "timeout" + assert attempts == ["session-start-missing", "session-start-missing"] + assert [e["waited_s"] for e in _start_missing_entries(adapter)] == [150] diff --git a/tests/test_policy.py b/tests/test_policy.py index 41c6ef97f..60f6733b4 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -636,6 +636,24 @@ def test_teardown_grace_must_be_nonnegative(): policy.loads("[limits]\nteardown_grace_s = -1\n") +def test_session_start_grace_default_parse_and_template(): + import tomllib + + assert policy.loads("").limits.session_start_grace_s == 120 + loaded = policy.loads("[limits]\nsession_start_grace_s = 45\n") + assert loaded.limits.session_start_grace_s == 45 + # 0 is legal: no session-start-missing notice + assert policy.loads("[limits]\nsession_start_grace_s = 0\n").limits.session_start_grace_s == 0 + # the emitted template documents the knob at its dataclass default + doc = tomllib.loads(policy.POLICY_TEMPLATE) + assert doc["limits"]["session_start_grace_s"] == policy.LimitsPolicy.session_start_grace_s + + +def test_session_start_grace_must_be_nonnegative(): + with pytest.raises(policy.PolicyError, match=r"limits\.session_start_grace_s must be >= 0"): + policy.loads("[limits]\nsession_start_grace_s = -1\n") + + def test_workflow_stall_nudges_cap_default_parse_and_template(): import tomllib @@ -697,6 +715,7 @@ def test_dev_contract_nudge_rejects_non_boolean(): "teardown_grace_s", "stop_without_result_nudges", "dev_stall_grace_s", + "session_start_grace_s", "dev_stall_nudges", "dev_stall_nudges_cap", "workflow_stall_nudges_cap", diff --git a/tests/test_portability_guard.py b/tests/test_portability_guard.py index 76a031264..c546f50d4 100644 --- a/tests/test_portability_guard.py +++ b/tests/test_portability_guard.py @@ -542,6 +542,11 @@ # DW-446: the recorded root's `(st_dev, st_ino)` — two integers naming no story, # branch, commit or path; left as-is so a maintainer can compare the record with # a later refusal. Kind-scoped because `dev` is generic. + # `session-start-missing`: `cli` is the session's CLI profile name, the config + # name `profile` carries on `bypass-dropped`; `hint` is a code constant + # (`codex_trust.DAEMON_ENV_HINT`), never session or operator text. Kind-scoped + # because both names are generic. + "session-start-missing": frozenset({"cli", "hint"}), "root-identity-recorded": frozenset({"dev", "ino"}), # DW-446: the `st_dev` a locked resume/re-arm re-bound (`old_dev` -> `dev`) under # a still-matching `ino` — integers only, kept for the same comparison. @@ -757,6 +762,9 @@ "followup_review_recommended", "frm", "generation", + # `session-start-missing`: `limits.session_start_grace_s` as a float, so + # the record reads without the policy snapshot in hand. + "grace_s", "graceful", "harvest_attempt", # `head` is NOT here any more: a HEAD sha (`attempt-preserve-failed`, @@ -1003,6 +1011,9 @@ "verification_sequence", "verification_stage", "via", + # `session-start-missing`: whole seconds the session had run without a + # SessionStart when the notice fired — a measured integer. + "waited_s", "weighted", "workflow", "worktree", @@ -1324,6 +1335,11 @@ # `in_tool`, a closed slug the adapter chose (declared benign). "session-active", "session-idle", + # One `session-start-missing` when a CLI whose profile maps SessionStart + # has sent none by `limits.session_start_grace_s`: `task_id` (routed), + # `waited_s`/`grace_s` (declared benign), `cli` and, on a Codex profile, + # the constant `hint` (declared benign on this kind). + "session-start-missing", # checkpoints.py "checkpoint-pause", "checkpoint-resume", diff --git a/tests/test_sweep.py b/tests/test_sweep.py index a516553d7..39f388c1a 100644 --- a/tests/test_sweep.py +++ b/tests/test_sweep.py @@ -5551,6 +5551,54 @@ def test_non_completed_triage_reports_hook_evidence_distinctly(project, primary, assert diag["hook_event_count"] == len(primary) +def test_triage_diagnostic_names_codex_daemon_when_no_hook_events(project): + """A Codex triage session that sent no hook event at all (its hooks ran in a + shared app-server daemon without the BMAD_LOOP_* env, so the relay dropped + them) gets a `hint` naming that cause, closing the escalation text's tail. + + ABLATION: drop the hint assignment in `_session_failure_diagnostic` and this + fails on `hint`; drop it from `_diagnostic_suffix` and it fails on the text.""" + from bmad_loop.adapters.profile import get_profile + from bmad_loop.codex_trust import DAEMON_ENV_HINT + + write_ledger(project, {"DW-1": "open"}) + engine, adapter = make_sweep(project, [failed_session_effect()] * 2) + adapter.profile = get_profile("codex") # the triage role falls back to this adapter + engine.run() + + diag = _decisions(engine, "triage-decision")[-1]["diagnostic"] + assert diag["hook_events"] == "none" + assert diag["hint"] == DAEMON_ENV_HINT + assert f"hook events: none; hint: {DAEMON_ENV_HINT}]" in engine.state.paused_reason + + +@pytest.mark.parametrize( + ("profile", "primary"), + [ + (None, ()), # the mock adapter carries no profile at all + ("claude", ()), # no events, but not a Codex profile + ("codex", ("SessionStart",)), # a Codex relay that did deliver + ], +) +def test_triage_diagnostic_has_no_hint_for_non_codex_or_with_events(project, profile, primary): + """The daemon hint is additive and narrow: absent for a profile that is not + `codex-hooks-json`, and absent once any admitted hook event arrived. + + ABLATION: drop the dialect test and the claude row carries the hint; drop the + no-events test and the codex row does.""" + from bmad_loop.adapters.profile import get_profile + + write_ledger(project, {"DW-1": "open"}) + engine, adapter = make_sweep(project, [failed_session_effect(primary_events=primary)] * 2) + if profile is not None: + adapter.profile = get_profile(profile) + engine.run() + + diag = _decisions(engine, "triage-decision")[-1]["diagnostic"] + assert "hint" not in diag + assert "; hint:" not in engine.state.paused_reason + + def test_non_completed_triage_diagnostic_replays_session_attribution(project): """A nested CLI launched from inside the triage session writes into its event stream (#767). The diagnostic replays the attribution wait_for_completion From 0ed7c479d11d67cbe947d2a06fcd139a5843c320 Mon Sep 17 00:00:00 2001 From: t Date: Sun, 25 Oct 2026 10:40:11 -0700 Subject: [PATCH 3/4] docs: document Codex --no-daemon, adapter.codex-version and session-start-missing --- CHANGELOG.md | 15 ++++++++------- docs/FEATURES.md | 7 ++++--- docs/adapter-authoring-guide.md | 8 ++++++++ docs/setup-guide.md | 10 +++++++++- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6875ab66..67ffb8016 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -107,10 +107,11 @@ breaking changes may land in a minor release. `in_tool`, and the TUI shows `· idle · in ` (#858). - Add `[verify] command_timeout_min` (default 30): per-command verify timeout, previously fixed at 30 minutes (#857). -- `validate` reports `adapter.codex-version` (#901). -- Journal `session-start-missing` when a session's CLI reports no SessionStart within - `limits.session_start_grace_s` (default 120 s); Codex failures with no hook events - name the shared-daemon cause in sweep escalation text (#901). +- Add `adapter.codex-version` to `validate`: warn when Codex is older than 0.156 or the + profile's `launch_args` drop `--no-daemon` (#901). +- Journal `session-start-missing` when a session's CLI sends no SessionStart within + `[limits] session_start_grace_s` (default 120 s); a Codex sweep session with no hook + events names the shared daemon in its escalation text (#901). ### Changed @@ -315,9 +316,9 @@ breaking changes may land in a minor release. - Refuse to type a nudge into a pane in copy or clock mode, which swallowed it while it counted as sent; the nudge is now recorded as not delivered (#949). - Run Codex sessions with `--no-daemon` so tools and hooks see the session's - `BMAD_LOOP_*` environment instead of a shared app-server daemon's; Codex >= 0.156 is - now required. A Codex run paused across this upgrade prints the config-changed - advisory once on resume (#901). + `BMAD_LOOP_*` environment, not a shared app-server daemon's; require Codex >= 0.156. + A Codex run paused across the upgrade prints the config-changed advisory once on + resume (#901). ### Security diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 7a190e280..217fdd61c 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -313,11 +313,12 @@ bmad-loop drives a BMAD sprint unattended: a deterministic Python loop launches - Bundled profiles: claude, codex, gemini, copilot, antigravity and opencode-http. The names `opencode` and `claude-code-tmux` resolve as aliases. - A `.bmad-loop/profiles/*.toml` file whose `name` matches a bundled profile overrides it; a new `name` adds a CLI. Project files win over packaged and entry-point profiles. - opencode runs as a headless `opencode serve` over HTTP/SSE. It needs no tmux window and registers no hooks. It speaks both OpenCode server APIs, 1.x and 2.x, picked per session from `opencode --version`; an unsupported major fails the session start with a clear error, and `validate` reports it as `adapter.opencode-version`. + - codex runs with `--no-daemon` (Codex ≥ 0.156), so shell tools and hooks run in the session's own process and see its `BMAD_LOOP_*` environment instead of a shared app-server daemon's. `validate` reports the version floor and a profile that drops the flag as `adapter.codex-version`; a session whose CLI never sends `SessionStart` is journaled as `session-start-missing` after `[limits] session_start_grace_s`. - `bmad-loop probe-adapter ` does a zero-launch scan by default. `--probe` opts in to one live, content-free turn. Every report is sanitized before it is printed. - Antigravity (experimental) never reports token usage. Under worktree isolation, it trusts each unit worktree only when the project root is already trusted. -- **Config:** `[adapter] name`; `bmad-loop probe-adapter --probe`; `bmad-loop probe-adapter --workspace `; `bmad-loop adapters` -- **Tested by:** `tests/test_profile.py`, `tests/test_probe.py`, `tests/test_adapter_registry.py`, `tests/test_workspace_trust.py` -- **Docs:** [Choosing which CLIs to drive](setup-guide.md#choosing-which-clis-to-drive), [Adapter authoring guide: finalizing a profile](adapter-authoring-guide.md#walkthrough-finalizing-a-profile), [CLI reference: probe-adapter](reference/cli.md#probe-adapter), [CLI reference: adapters](reference/cli.md#adapters) +- **Config:** `[adapter] name`; `[limits] session_start_grace_s`; `bmad-loop probe-adapter --probe`; `bmad-loop probe-adapter --workspace `; `bmad-loop adapters` +- **Tested by:** `tests/test_profile.py`, `tests/test_probe.py`, `tests/test_adapter_registry.py`, `tests/test_workspace_trust.py`, `tests/test_codex_trust.py`, `tests/test_generic_tmux.py` +- **Docs:** [Choosing which CLIs to drive](setup-guide.md#choosing-which-clis-to-drive), [Codex shared app-server daemon](setup-guide.md#codex-shared-app-server-daemon), [Adapter authoring guide: finalizing a profile](adapter-authoring-guide.md#walkthrough-finalizing-a-profile), [CLI reference: probe-adapter](reference/cli.md#probe-adapter), [CLI reference: adapters](reference/cli.md#adapters) ### Per-stage adapters diff --git a/docs/adapter-authoring-guide.md b/docs/adapter-authoring-guide.md index 26a516b65..ac5a3da67 100644 --- a/docs/adapter-authoring-guide.md +++ b/docs/adapter-authoring-guide.md @@ -367,6 +367,14 @@ parent's stream. The generic adapter tells them apart by `session_id` `probe-adapter --probe` shows the payload's field names; confirm the `session_id` and `source` casing before relying on them. +The engine passes its `BMAD_LOOP_*` contract on the session window's environment, so only +processes descended from the launched CLI inherit it. A CLI that runs tool shells or hooks +in a long-lived server process it shares across sessions gives them the server's +environment instead: hooks relay nothing and skills cannot tell they run under bmad-loop. +Host that server per session. The `codex` profile passes `--no-daemon` in `launch_args`, +and the [opencode adapter](#worked-example-the-opencode-adapter) spawns one +`opencode serve` per session. + ### `WorkspaceTrustSpec` (the `[workspace_trust]` table) Some CLIs refuse to start in a workspace that is not listed in a home-level settings file, diff --git a/docs/setup-guide.md b/docs/setup-guide.md index d06907c5e..f680da9c0 100644 --- a/docs/setup-guide.md +++ b/docs/setup-guide.md @@ -361,7 +361,7 @@ them to whoever owns the machine: dialogs. - **codex** — run `codex` once in the project and accept **both** prompts: workspace trust, then "Hooks need review → Trust all and continue" (untrusted hooks silently never fire). - Requires Codex ≥ 0.139. + Requires Codex ≥ 0.156. - **gemini** — authenticate once (browser OAuth or `GEMINI_API_KEY`). Requires Gemini CLI ≥ 0.46. - **copilot** — run `copilot` once in the project and authenticate (`gh` / a Copilot @@ -458,6 +458,14 @@ Under `[scm] isolation = "worktree"`, every unit runs in a new worktree that Cod Expect one pause per unit: each worktree path needs its own grant. The re-armed story restarts from its baseline. bmad-loop never writes Codex's trust state itself. +#### Codex shared app-server daemon + +By default, Codex attaches to a shared background app-server daemon, which may have been started by an unrelated Codex session. Shell tools and hooks run inside that daemon, with the daemon's environment rather than the session window's, so they never see the session's `BMAD_LOOP_*` variables: a skill that checks `BMAD_LOOP_MODE` quits early, and the hook relay drops every event. The bundled `codex` profile therefore passes `--no-daemon` (Codex ≥ 0.156), which hosts the app-server inside the session's own client process. + +- **Overlay profiles:** a `.bmad-loop/profiles/codex.toml` that sets `launch_args` must keep `--no-daemon` in them. It goes in `launch_args`, not `bypass_args`: a stage's `extra_args` replace `bypass_args`. +- **Check:** `bmad-loop validate` reports `adapter.codex-version`. It warns when the installed Codex is older than 0.156 (it rejects `--no-daemon`, so every session exits at launch), when the profile's `launch_args` lack `--no-daemon`, or when the version can't be read. It never fails validation. +- **Symptom:** when a CLI that maps `SessionStart` sends none within `[limits] session_start_grace_s` (default 120 s), the run journals one `session-start-missing` entry (`task_id`, `waited_s`, `grace_s`, `cli`, plus a `hint` naming the daemon for Codex). The notice is observation only and never ends the session. A sweep triage session on Codex that reports no hook events adds the same hint to its escalation text (`; hint: …`). + ### Skill location `claude` reads skills from `.claude/skills/`; `codex`, `gemini`, `copilot`, and `antigravity` From 2b699ed2556d31ae3b88f16cf935e555c9bb4bd5 Mon Sep 17 00:00:00 2001 From: t Date: Sun, 25 Oct 2026 10:54:32 -0700 Subject: [PATCH 4/4] docs: list tests/test_cli.py under Multi-CLI profiles for adapter.codex-version --- docs/FEATURES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 217fdd61c..c162da3f3 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -317,7 +317,7 @@ bmad-loop drives a BMAD sprint unattended: a deterministic Python loop launches - `bmad-loop probe-adapter ` does a zero-launch scan by default. `--probe` opts in to one live, content-free turn. Every report is sanitized before it is printed. - Antigravity (experimental) never reports token usage. Under worktree isolation, it trusts each unit worktree only when the project root is already trusted. - **Config:** `[adapter] name`; `[limits] session_start_grace_s`; `bmad-loop probe-adapter --probe`; `bmad-loop probe-adapter --workspace `; `bmad-loop adapters` -- **Tested by:** `tests/test_profile.py`, `tests/test_probe.py`, `tests/test_adapter_registry.py`, `tests/test_workspace_trust.py`, `tests/test_codex_trust.py`, `tests/test_generic_tmux.py` +- **Tested by:** `tests/test_profile.py`, `tests/test_probe.py`, `tests/test_adapter_registry.py`, `tests/test_workspace_trust.py`, `tests/test_codex_trust.py`, `tests/test_generic_tmux.py`, `tests/test_cli.py` - **Docs:** [Choosing which CLIs to drive](setup-guide.md#choosing-which-clis-to-drive), [Codex shared app-server daemon](setup-guide.md#codex-shared-app-server-daemon), [Adapter authoring guide: finalizing a profile](adapter-authoring-guide.md#walkthrough-finalizing-a-profile), [CLI reference: probe-adapter](reference/cli.md#probe-adapter), [CLI reference: adapters](reference/cli.md#adapters) ### Per-stage adapters