From e18bc8f422d0923b8bceb9a4dd5182c076c904fa Mon Sep 17 00:00:00 2001 From: t Date: Wed, 28 Oct 2026 13:59:12 -0700 Subject: [PATCH 1/9] feat(adapters): grant a linked worktree's gitdir to sandboxed Codex sessions (DW-599) Under scm.isolation = "worktree" + adapter.permission_mode = "sandboxed", a Codex session could not git add/commit: the linked worktree's gitdir (
/.git/worktrees/) and the common dir (
/.git) are read-only to the sandbox. The sandboxed mode now declares `linked_worktree_args`, which the generic adapter appends after the mode args, with both exact paths filled in, when the session's cwd is a linked worktree. Probe record (codex-cli 0.165.0, Linux; scratch repos: main repo + one commit, `git worktree add ../wt`). Repos first under /tmp, then moved to ~/.cache/dw599-probe: /tmp is writable under `:workspace`, which made the common dir writable and masked (b). Zero-token matrix via `codex sandbox -P bmad_loop -c 'permissions.bmad_loop.extends=":workspace"' -C -- sh -c 'git add a.txt && git commit -qm a'`: - (c) baseline, shipped table only, in wt: FAIL "Unable to create .../.git/worktrees/wt/index.lock: Read-only file system" - dotted per-key `-c 'permissions.bmad_loop.filesystem."/abs/.git/..."="write"'`: REFUSED at config load ("filesystem path ... must be absolute"); Codex splits -c keys on every `.`, quotes included, so no path holding `.git` can be a key - (b) 2nd `filesystem={="write"}` only: FAIL ("unable to index file", objects in the common dir); common dir only: FAIL (gitdir index.lock); both: COMMITTED - (a) main checkout, shipped table then a 2nd `filesystem={...}` table: FAIL (main .git/index.lock read-only) => a second table REPLACES the first - child profile: `-c permissions.bmad_loop_worktree.extends="bmad_loop" -c permissions.bmad_loop_worktree.filesystem={="write",="write"}` run with -P bmad_loop_worktree: COMMITTED in wt and in main => extends MERGES the parent's filesystem map (the :workspace_roots .git grant kept) - the adapter's actual rendered tokens, re-run through `codex sandbox`: COMMITTED Real runs (`codex --no-daemon [grant] exec --ephemeral "echo hi > b.txt && git add b.txt && git commit -m probe-b"`): - with the grant (a later -c default_permissions="bmad_loop_worktree" after the shipped default_permissions="bmad_loop"): COMMITTED (8,242 tokens), so the last -c wins - without it: FAIL, wt/index.lock Read-only file system (7,446 tokens) DW-599 decisions: - Override shape: a child permission profile `bmad_loop_worktree` that extends `bmad_loop` with exact-path write grants for the gitdir and common dir, selected by a trailing `-c default_permissions=...`. It does not restate the shipped table (a second table on bmad_loop would replace it), and dotted per-path keys are impossible. Each path is rendered as a TOML basic string (quote/backslash/control escaped; a lone-surrogate path raises). - Mode-detection seam: declarative. `[permission_modes.]` gains an optional `linked_worktree_args` list ({gitdir}/{common_dir} placeholders, validated: non-empty strings, at least one placeholder, on both the TOML and provider routes), and `for_permission_mode` copies it onto a new appended `CLIProfile.linked_worktree_args` beside `bypass_args`. Bypass and every profile that declares none (all non-Codex) carry (); the adapter never names a mode or a CLI. `config_digest` hashes the template only when non-empty, so every other digest stays byte-identical. - extra_args: the grant rides with the inherited mode args, so a policy `extra_args` override (which replaces them) drops the grant too, as it must: the grant extends the `bmad_loop` profile those args define. - Fault behaviour: raise. Git is asked (one `rev-parse --path-format=absolute --git-dir --git-common-dir` through verify.git_bytes) only when the mode declares a grant and extra_args is unset. A GitError, a non-zero rc, an undecodable or malformed answer, a resolve fault or an unrenderable path raises LinkedWorktreeGrantError out of interactive_argv. In start_session that is before the window exists, and the engine records a resumable run-crash (crash.txt + journal). Launching without the grant was rejected: that session cannot commit and would fail its unit only after spending its turn. - The validate warning (`_report_permission_mode`) is left for the follow-up. --- src/bmad_loop/adapters/generic.py | 61 +++++++++++++- src/bmad_loop/adapters/profile.py | 81 +++++++++++++++++-- src/bmad_loop/data/profiles/codex.toml | 21 ++++- src/bmad_loop/runsetup.py | 9 ++- tests/test_generic_tmux.py | 106 ++++++++++++++++++++++++- tests/test_profile.py | 53 ++++++++++++- tests/test_runsetup.py | 18 +++++ 7 files changed, 332 insertions(+), 17 deletions(-) diff --git a/src/bmad_loop/adapters/generic.py b/src/bmad_loop/adapters/generic.py index 5d7559189..df51dc4fc 100644 --- a/src/bmad_loop/adapters/generic.py +++ b/src/bmad_loop/adapters/generic.py @@ -53,7 +53,7 @@ from ..signals import REBIND_SOURCES, SessionAttribution, SignalWatcher from ..tokens import read_usage as tally_usage from ..usagereset import UsageReset, parse_reset -from ..verify import read_frontmatter, status_of +from ..verify import GitError, git_bytes, read_frontmatter, status_of from .base import ( USAGE_LIMIT_BLOCKED, USAGE_LIMIT_WAIT, @@ -510,6 +510,43 @@ def _idle_tool_slug(name: str) -> str: return "other" +class LinkedWorktreeGrantError(RuntimeError): + """A sandboxed launch that owes a linked worktree's gitdir grant (DW-599) + could not resolve the paths to grant. Raised out of ``start_session`` + rather than launching without the grant: such a session could not + `git add`/`commit`, and would fail its unit only after spending its turn.""" + + +def _linked_worktree_git_dirs(cwd: Path) -> tuple[Path, Path] | None: + """``(gitdir, common dir)`` of the linked worktree ``cwd`` is in, both + resolved; None in a main checkout, whose gitdir IS its common dir. Every + fault raises :class:`LinkedWorktreeGrantError`: the caller has already + decided a grant is owed, and a session run without it cannot commit.""" + try: + answer = git_bytes( + cwd, "rev-parse", "--path-format=absolute", "--git-dir", "--git-common-dir" + ) + except GitError as e: + raise LinkedWorktreeGrantError(f"could not ask git for the git dirs of {cwd}: {e}") from e + try: + # fsdecode can raise on Windows (utf-8/surrogatepass refuses a lone byte). + stdout, stderr = os.fsdecode(answer.stdout), os.fsdecode(answer.stderr) + except UnicodeError as e: + raise LinkedWorktreeGrantError(f"could not decode the git dirs of {cwd}: {e}") from e + if answer.returncode != 0: + detail = stderr.strip() or f"git exited {answer.returncode}" + raise LinkedWorktreeGrantError(f"could not read the git dirs of {cwd}: {detail}") + # One path per line; removesuffix, not strip: every other byte is the path's. + lines = stdout.removesuffix("\n").split("\n") + if len(lines) != 2 or not all(lines): + raise LinkedWorktreeGrantError(f"unexpected git dirs for {cwd}: {lines!r}") + try: + git_dir, common_dir = Path(lines[0]).resolve(), Path(lines[1]).resolve() + except (OSError, RuntimeError) as e: + raise LinkedWorktreeGrantError(f"could not resolve the git dirs of {cwd}: {e}") from e + return None if git_dir == common_dir else (git_dir, common_dir) + + class _ResultFileMixin: """Result-file read-back and verdict finalization: acquire the skill-written result dict and fold it into the session's final @@ -1145,7 +1182,9 @@ def _ensure_session(self) -> None: def interactive_argv(self, spec: SessionSpec) -> list[str]: extra = self.extra_args if extra is None: - extra = self.profile.bypass_args + # The linked-worktree grant rides with the mode args it extends, so an + # `extra_args` override (which replaces them) drops it too. + extra = (*self.profile.bypass_args, *self._linked_worktree_grant(spec.cwd)) binary = self.binary if self.profile.hooks.dialect == "codex-hooks-json": from ..codex_trust import resolved_codex_binary @@ -1161,6 +1200,24 @@ def interactive_argv(self, spec: SessionSpec) -> list[str]: argv += [self.profile.model_flag, spec.model] return argv + def _linked_worktree_grant(self, cwd: Path) -> tuple[str, ...]: + """The permission mode's ``linked_worktree_args`` filled in with the + gitdir and common dir of the linked worktree ``cwd`` is in (DW-599); () + when the mode declares none (bypass, or a CLI whose sandbox needs no + grant) or ``cwd`` is a main checkout. Git is asked only when a grant + could be owed; a fault raises :class:`LinkedWorktreeGrantError`.""" + if not self.profile.linked_worktree_args: + return () + dirs = _linked_worktree_git_dirs(cwd) + if dirs is None: + return () + try: + return self.profile.render_linked_worktree_args(*dirs) + except ValueError as e: + raise LinkedWorktreeGrantError( + f"cannot write the git dirs of {cwd} into {self.profile.name}'s grant: {e}" + ) from e + def interactive_env(self, spec: SessionSpec) -> dict[str, str]: # The pin chokepoint (runs.pin_state_root): the profile's [env] table # must not be able to move a session off this process's state root — diff --git a/src/bmad_loop/adapters/profile.py b/src/bmad_loop/adapters/profile.py index 463d20230..a740d253f 100644 --- a/src/bmad_loop/adapters/profile.py +++ b/src/bmad_loop/adapters/profile.py @@ -36,10 +36,11 @@ import string import sys import tomllib +from collections.abc import Callable from dataclasses import dataclass, field, replace from importlib import resources from importlib.resources.abc import Traversable -from pathlib import Path +from pathlib import Path, PurePath import regex @@ -161,6 +162,13 @@ class ParkedAnswersSpec: # The `sys.platform` values a `[permission_modes.]` table may name. PERMISSION_MODE_PLATFORMS = frozenset({"linux", "darwin", "win32"}) +# The placeholders a `linked_worktree_args` entry may carry (DW-599), each +# rendered as a TOML basic string, quotes included, of the resolved path. +LINKED_WORKTREE_PLACEHOLDERS = ("{gitdir}", "{common_dir}") +_LINKED_WORKTREE_PLACEHOLDER_RE = regex.compile( + "|".join(regex.escape(p) for p in LINKED_WORKTREE_PLACEHOLDERS) +) + @dataclass(frozen=True) class PermissionModeSpec: @@ -170,11 +178,31 @@ class PermissionModeSpec: (:meth:`CLIProfile.for_permission_mode`), so ``extra_args`` replaces them exactly as it replaces ``bypass_args``. ``platforms`` names the ``sys.platform`` values the mapping is known to work on; empty = every - platform. Mode names, args and platforms are validated in + platform. ``linked_worktree_args`` (DW-599) are appended after ``args`` when + the session's cwd is a linked worktree, with :data:`LINKED_WORKTREE_PLACEHOLDERS` + filled in, for a sandbox that must be granted that worktree's gitdir and + common dir explicitly. Mode names, args and platforms are validated in :func:`_validate_profile`.""" args: tuple[str, ...] platforms: tuple[str, ...] = () + linked_worktree_args: tuple[str, ...] = () + + +def toml_basic_string(value: str) -> str: + """``value`` as a TOML basic string, quotes included. Raises ValueError for a + value TOML cannot carry: a lone surrogate (a non-UTF-8 path byte under + ``os.fsdecode``) has no TOML spelling.""" + value.encode("utf-8") # UnicodeEncodeError (a ValueError) on a lone surrogate + out: list[str] = [] + for ch in value: + if ch in '"\\': + out.append("\\" + ch) + elif ord(ch) < 0x20 or ord(ch) == 0x7F: + out.append(f"\\u{ord(ch):04X}") + else: + out.append(ch) + return '"' + "".join(out) + '"' @dataclass(frozen=True) @@ -348,6 +376,10 @@ class CLIProfile: # a mode absent here is refused for this CLI. APPENDED after parked_answers # for the same positional-compatibility reason. permission_modes: dict[str, PermissionModeSpec] = field(default_factory=dict) + # The selected mode's `linked_worktree_args` (DW-599), set beside + # `bypass_args` by `for_permission_mode`; empty under bypass. APPENDED after + # permission_modes for the same positional-compatibility reason. + linked_worktree_args: tuple[str, ...] = () @property def hookless(self) -> bool: @@ -384,7 +416,8 @@ def for_permission_mode(self, mode: str, platform: str = sys.platform) -> CLIPro (the launched argv, the config digest, :meth:`missing_bypass_tokens`) sees the effective permission args with no second code path. Raises :class:`ProfileError` when the profile has no mapping for ``mode`` or - the mapping excludes ``platform``.""" + the mapping excludes ``platform``. The mode's ``linked_worktree_args`` + ride along the same way, for the adapter to fill in per session.""" if mode == "bypass": return self spec = self.permission_modes.get(mode) @@ -399,7 +432,22 @@ def for_permission_mode(self, mode: str, platform: str = sys.platform) -> CLIPro f"profile {self.name!r} maps permission_mode {mode!r} only on " f"{', '.join(spec.platforms)}, not {platform}" ) - return replace(self, bypass_args=spec.args) + return replace(self, bypass_args=spec.args, linked_worktree_args=spec.linked_worktree_args) + + def render_linked_worktree_args( + self, gitdir: PurePath, common_dir: PurePath + ) -> tuple[str, ...]: + """``linked_worktree_args`` with each placeholder replaced by its path as a + TOML basic string. Raises ValueError for a path TOML cannot carry.""" + values = { + "{gitdir}": toml_basic_string(str(gitdir)), + "{common_dir}": toml_basic_string(str(common_dir)), + } + # One pass, so a path that itself spells a placeholder is not rewritten. + return tuple( + _LINKED_WORKTREE_PLACEHOLDER_RE.sub(lambda m: values[m.group()], arg) + for arg in self.linked_worktree_args + ) def missing_bypass_tokens(self, extra_args: tuple[str, ...] | None) -> tuple[str, ...]: """The permission args (``bypass_args``) a policy ``extra_args`` override @@ -418,6 +466,19 @@ def missing_bypass_tokens(self, extra_args: tuple[str, ...] | None) -> tuple[str return tuple(t for t in dict.fromkeys(self.bypass_args) if t not in present) +def _check_linked_worktree_args( + args: tuple[str, ...], where: str, fail: Callable[[str], ProfileError] +) -> None: + """Non-empty strings, and at least one placeholder across the list: an arg + with no path in it belongs in the mode's ``args``.""" + if not args: + return + if not all(isinstance(a, str) and a for a in args): + raise fail(f"{where} must list non-empty strings") + if not any(p in a for a in args for p in LINKED_WORKTREE_PLACEHOLDERS): + raise fail(f"{where} must use at least one of {', '.join(LINKED_WORKTREE_PLACEHOLDERS)}") + + def _validate_profile(profile: CLIProfile, source: str) -> None: """Enforce every value-level invariant a ``CLIProfile`` must satisfy, whatever built it. @@ -726,6 +787,11 @@ def fail(msg: str) -> ProfileError: f"permission_modes.{mode}.platforms must name values from " f"{sorted(PERMISSION_MODE_PLATFORMS)}: got {bad!r}" ) + _check_linked_worktree_args( + spec.linked_worktree_args, f"permission_modes.{mode}.linked_worktree_args", fail + ) + # A provider-built profile may set the resolved field directly. + _check_linked_worktree_args(profile.linked_worktree_args, "linked_worktree_args", fail) answers = profile.parked_answers if answers is not None: @@ -844,7 +910,7 @@ def str_list(key: str) -> tuple[str, ...]: lists[key] = tuple(raw) parked_answers = ParkedAnswersSpec(**lists) - # `[permission_modes.]` tables of two string lists; mode names and + # `[permission_modes.]` tables of string lists; mode names and # values are checked in `_validate_profile`. modes_d = doc.get("permission_modes", {}) if not isinstance(modes_d, dict): @@ -853,11 +919,12 @@ def str_list(key: str) -> tuple[str, ...]: for mode, spec_d in modes_d.items(): if not isinstance(spec_d, dict): raise fail(f"permission_modes.{mode} must be a table") - unknown = sorted(set(spec_d) - {"args", "platforms"}) + mode_keys = ("args", "platforms", "linked_worktree_args") + unknown = sorted(set(spec_d) - set(mode_keys)) if unknown: raise fail(f"permission_modes.{mode} has unknown keys: {unknown}") mode_lists: dict[str, tuple[str, ...]] = {} - for key in ("args", "platforms"): + for key in mode_keys: raw = spec_d.get(key, []) if not isinstance(raw, list) or not all(isinstance(x, str) for x in raw): raise fail(f"permission_modes.{mode}.{key} must be a list of strings") diff --git a/src/bmad_loop/data/profiles/codex.toml b/src/bmad_loop/data/profiles/codex.toml index ea70a8c92..9b307ec18 100644 --- a/src/bmad_loop/data/profiles/codex.toml +++ b/src/bmad_loop/data/profiles/codex.toml @@ -34,9 +34,16 @@ events = { SessionStart = "SessionStart", Stop = "Stop" } # grant instead (it replaces `--sandbox`: Codex refuses `sandbox_mode` beside # `default_permissions`). Network stays off. Per Codex's agent-approvals-security # and permissions docs; live-checked 2026-10-27 on codex-cli 0.164.0. -# KNOWN GAP: in a linked worktree (`scm.isolation = "worktree"`) the gitdir lives -# under the main repo's `.git/worktrees/` and stays read-only (Codex grants -# only exact paths there, no globs), so git add/commit fail in that checkout. +# In a linked worktree (`scm.isolation = "worktree"`) `.git` is a file, and git +# writes the worktree's gitdir (`
/.git/worktrees/`, its index) and the +# common dir (`
/.git`, objects and refs), both read-only to the sandbox. +# `linked_worktree_args` grants the two exact paths (Codex grants no globs): the +# adapter appends them after `args` only when the session's cwd is a linked +# worktree, each placeholder rendered as a TOML string. A child profile extends +# `bmad_loop` (its filesystem map merges) and a later `default_permissions` selects +# it: a second `filesystem` table on `bmad_loop` would REPLACE the one above, and a +# dotted per-path key cannot work since Codex splits `-c` keys on every `.`. Both +# grants are needed. Live-checked 2026-10-28 on codex-cli 0.165.0. # Windows is unverified, hence the platform list. [permission_modes.sandboxed] args = [ @@ -50,3 +57,11 @@ args = [ "never", ] platforms = ["linux", "darwin"] +linked_worktree_args = [ + "-c", + 'permissions.bmad_loop_worktree.extends="bmad_loop"', + "-c", + 'permissions.bmad_loop_worktree.filesystem={{gitdir}="write",{common_dir}="write"}', + "-c", + 'default_permissions="bmad_loop_worktree"', +] diff --git a/src/bmad_loop/runsetup.py b/src/bmad_loop/runsetup.py index 7e17c21fc..49a46dfe8 100644 --- a/src/bmad_loop/runsetup.py +++ b/src/bmad_loop/runsetup.py @@ -168,8 +168,9 @@ def config_digest( what flags and environment**. That rule, not a hand-picked list, is what keeps this complete: walk ``GenericAdapter.interactive_argv`` and ``interactive_env`` and every token there traces back to one of - ``binary`` / ``launch_args`` / ``bypass_args`` / ``model_flag`` / - ``prompt_template`` / ``env`` on the *resolved* profile, or to + ``binary`` / ``launch_args`` / ``bypass_args`` / ``linked_worktree_args`` + (its paths filled in) / ``model_flag`` / ``prompt_template`` / ``env`` on + the *resolved* profile, or to ``extra_args`` on the resolved adapter — plus ``session_id_flag``, which ``build_command`` appends (with a minted id) to the launched argv only. The opencode-http builder reads a strict SUBSET of those — ``_serve_argv`` @@ -440,6 +441,10 @@ def config_digest( # pin also records which mapping those args came from. if cfg.permission_mode != "bypass": launch[role]["permission_mode"] = cfg.permission_mode + # Argv tokens `interactive_argv` appends after the mode's args in a linked + # worktree (DW-599), placeholders unfilled; same byte-identity rule. + if prof.linked_worktree_args: + launch[role]["linked_worktree_args"] = list(prof.linked_worktree_args) payload = { "verify_commands": list(policy.verify.commands), "plugins_enabled": sorted(policy.plugins.enabled), diff --git a/tests/test_generic_tmux.py b/tests/test_generic_tmux.py index c4c687c6b..1954fbdda 100644 --- a/tests/test_generic_tmux.py +++ b/tests/test_generic_tmux.py @@ -24,7 +24,7 @@ import pytest import regex -from conftest import REAL_MUX_HANG_CEILING_S, json_recursion_payload, real_mux_e2e +from conftest import REAL_MUX_HANG_CEILING_S, git, json_recursion_payload, real_mux_e2e from bmad_loop import devcontract, runs from bmad_loop.adapters import base as adapter_base @@ -41,12 +41,18 @@ ) from bmad_loop.adapters.generic import GenericDevAdapter, GenericTmuxAdapter from bmad_loop.adapters.multiplexer import MultiplexerError -from bmad_loop.adapters.profile import PARKED_EVENTS, PermissionModeSpec, get_profile +from bmad_loop.adapters.profile import ( + PARKED_EVENTS, + PermissionModeSpec, + get_profile, + toml_basic_string, +) from bmad_loop.bmadconfig import ProjectPaths from bmad_loop.journal import TASK_CYCLE_ARTIFACTS, Journal from bmad_loop.model import TokenUsage from bmad_loop.policy import LimitsPolicy, NotifyPolicy, Policy from bmad_loop.signals import HookEvent +from bmad_loop.verify import GitTimeoutError # A bump the filesystem can actually record. NTFS stores ~100ns ticks (FAT, 2s), # so a 1ns increment rounds away on Windows and the rewritten marker never reads @@ -469,6 +475,102 @@ def test_a_sandboxed_profile_launches_its_mode_args_in_the_bypass_slot(tmp_path) assert shlex.split(cmd)[prompt_at + 2] == settings +def _grant_adapter(tmp_path, profile_name="codex", mode="sandboxed", extra_args=None): + adapter = make_adapter(tmp_path, profile_name=profile_name, extra_args=extra_args) + adapter.profile = get_profile(profile_name).for_permission_mode(mode, platform="linux") + return adapter + + +def _linked_worktree(project) -> Path: + wt = project.project.parent / "wt" + git(project.project, "worktree", "add", "-q", "--detach", str(wt), "HEAD") + (wt / "nested").mkdir() # a nested project's sessions run below the worktree root + return wt + + +def test_sandboxed_codex_in_a_linked_worktree_is_granted_its_gitdir_and_common_dir( + tmp_path, project +): + """DW-599: Codex's sandbox keeps a linked worktree's gitdir and the shared + common dir read-only, so the sandboxed launch appends the mode's + `linked_worktree_args` with both exact paths, after the mode args (its later + `default_permissions` must win) and before the model flag. + + ABLATION: return () from `_linked_worktree_grant` and this reddens.""" + wt = _linked_worktree(project) + adapter = _grant_adapter(tmp_path) + mode_args = list(adapter.profile.bypass_args) + gitdir = toml_basic_string(str((project.project / ".git" / "worktrees" / "wt").resolve())) + common = toml_basic_string(str((project.project / ".git").resolve())) + grant = [ + "-c", + 'permissions.bmad_loop_worktree.extends="bmad_loop"', + "-c", + f'permissions.bmad_loop_worktree.filesystem={{{gitdir}="write",{common}="write"}}', + "-c", + 'default_permissions="bmad_loop_worktree"', + ] + for cwd in (wt, wt / "nested"): + argv = adapter.interactive_argv(dataclasses.replace(make_spec(tmp_path), cwd=cwd)) + start = argv.index(mode_args[0]) + assert argv[start:] == [*mode_args, *grant, "--model", "sonnet"] + + +def test_no_linked_worktree_grant_in_a_main_checkout(tmp_path, project): + adapter = _grant_adapter(tmp_path) + argv = adapter.interactive_argv(dataclasses.replace(make_spec(tmp_path), cwd=project.project)) + assert argv[-4:-2] == ["--ask-for-approval", "never"] # the mode args end the slot + assert not any("bmad_loop_worktree" in a for a in argv) + + +@pytest.mark.parametrize( + ("profile_name", "mode", "extra_args"), + [ + ("codex", "bypass", None), + ("claude", "sandboxed", None), # a sandbox that needs no grant declares none + ("codex", "sandboxed", ("--custom",)), # extra_args replaces the mode args, grant too + ], + ids=["bypass", "non-codex", "extra-args"], +) +def test_no_linked_worktree_grant_without_a_declaring_mode( + tmp_path, project, monkeypatch, profile_name, mode, extra_args +): + """No grant, and git is not even asked: a broken git must not fail a launch + that owes no grant.""" + wt = _linked_worktree(project) + + def no_git(*_a, **_k): + raise AssertionError("git asked for a launch that owes no grant") + + monkeypatch.setattr(generic, "git_bytes", no_git) + adapter = _grant_adapter(tmp_path, profile_name, mode, extra_args) + argv = adapter.interactive_argv(dataclasses.replace(make_spec(tmp_path), cwd=wt)) + assert not any("bmad_loop_worktree" in a for a in argv) + + +def test_a_linked_worktree_grant_fault_refuses_the_launch(tmp_path, monkeypatch): + """A git fault while resolving the paths raises rather than launching a + session that could not commit. Covers a cwd git disowns (rc != 0) and a git + that raises (timeout / spawn failure). + + ABLATION: return None from `_linked_worktree_git_dirs` on a fault and both + cases launch.""" + adapter = _grant_adapter(tmp_path) + plain = tmp_path / "not-a-repo" + plain.mkdir() + spec = dataclasses.replace(make_spec(tmp_path), cwd=plain) + monkeypatch.setenv("GIT_CEILING_DIRECTORIES", str(tmp_path)) + with pytest.raises(generic.LinkedWorktreeGrantError, match="could not read the git dirs"): + adapter.build_command(spec) + + def hung(*_a, **_k): + raise GitTimeoutError("git timed out") + + monkeypatch.setattr(generic, "git_bytes", hung) + with pytest.raises(generic.LinkedWorktreeGrantError, match="git timed out"): + adapter.interactive_argv(spec) + + def test_read_result_variants(tmp_path): adapter = make_adapter(tmp_path) task_dir = adapter.tasks_dir / "t1" diff --git a/tests/test_profile.py b/tests/test_profile.py index 1a2ef9efb..88dae42e4 100644 --- a/tests/test_profile.py +++ b/tests/test_profile.py @@ -1,8 +1,9 @@ import dataclasses import json import re +import tomllib from importlib import resources -from pathlib import Path +from pathlib import Path, PurePosixPath import pytest from conftest import fault_read_text @@ -597,6 +598,14 @@ def test_permission_modes_table_parses_into_its_spec(tmp_path): ), ('\n[permission_modes.sandboxed]\nargs = "--x"\n', "args must be a list"), ("\n[permission_modes]\nsandboxed = 1\n", "sandboxed must be a table"), + ( + _PERMISSION_MODES_TABLE + 'linked_worktree_args = "{gitdir}"\n', + "linked_worktree_args must be a list", + ), + ( + _PERMISSION_MODES_TABLE + 'linked_worktree_args = ["--grant", "/fixed"]\n', + "must use at least one of", + ), ], ) def test_malformed_permission_modes_is_a_profile_error(tmp_path, table, match): @@ -616,6 +625,38 @@ def test_for_permission_mode_resolves_the_mode_into_bypass_args(tmp_path): assert sandboxed.missing_bypass_tokens(("--yes",)) == ("--sandbox", '{"a": 1}') +def test_for_permission_mode_carries_the_linked_worktree_args(tmp_path): + """DW-599: the mode's grant template rides onto the resolved profile beside + its args; bypass (and a mode that declares none) carries none.""" + body = MINIMAL_PROFILE + _PERMISSION_MODES_TABLE + 'linked_worktree_args = ["-g={gitdir}"]\n' + prof = _mode_profiles(tmp_path, body)["mycli"] + assert prof.permission_modes["sandboxed"].linked_worktree_args == ("-g={gitdir}",) + assert prof.linked_worktree_args == () + assert prof.for_permission_mode("sandboxed", platform="linux").linked_worktree_args == ( + "-g={gitdir}", + ) + assert ( + get_profile("claude").for_permission_mode("sandboxed", "linux").linked_worktree_args == () + ) + + +def test_render_linked_worktree_args_writes_each_path_as_one_toml_string(): + """Each placeholder becomes a TOML basic string, so a path holding a quote, + a backslash or a control character cannot break out of the value Codex + parses; one substitution pass, so a path spelling a placeholder stays put; a + path TOML cannot carry (a lone surrogate) raises.""" + prof = dataclasses.replace( + get_profile("claude"), linked_worktree_args=("{gitdir}={common_dir}", "plain") + ) + gitdir = PurePosixPath('/a"b\\c\td/{common_dir}') # Posix: the bytes are the point + rendered = prof.render_linked_worktree_args(gitdir, PurePosixPath("/r")) + assert rendered == ('"/a\\"b\\\\c\\u0009d/{common_dir}"="/r"', "plain") + key = rendered[0].partition("=")[0] + assert tomllib.loads(f"k = {key}")["k"] == str(gitdir) + with pytest.raises(ValueError): + prof.render_linked_worktree_args(PurePosixPath("/a\udcff"), PurePosixPath("/r")) + + def test_for_permission_mode_refuses_an_excluded_platform(tmp_path): prof = _mode_profiles(tmp_path, MINIMAL_PROFILE + _PERMISSION_MODES_TABLE)["mycli"] with pytest.raises(ProfileError, match="only on linux, darwin, not win32"): @@ -641,6 +682,10 @@ def test_shipped_sandboxed_mappings_parse_and_resolve(): } codex = get_profile("codex").for_permission_mode("sandboxed", platform="darwin") assert codex.bypass_args[-2:] == ("--ask-for-approval", "never") + # DW-599: a child of the mode's permission profile with the two exact paths, + # selected by a `default_permissions` placed after the mode's own + assert codex.linked_worktree_args[-1] == 'default_permissions="bmad_loop_worktree"' + assert any("{gitdir}" in a and "{common_dir}" in a for a in codex.linked_worktree_args) assert "--dangerously-bypass-approvals-and-sandbox" not in codex.bypass_args for name in ("claude", "codex"): with pytest.raises(ProfileError, match="not win32"): @@ -1754,6 +1799,12 @@ def test_profile_scan_failure_degrades(profile_scan): {"permission_modes": {"sandboxed": PermissionModeSpec(("--x",), ("haiku",))}}, "platforms must name", ), + # the DW-599 linked-worktree grant: per mode and on the resolved field + ( + {"permission_modes": {"sandboxed": PermissionModeSpec(("--x",), (), ("--y",))}}, + "must use at least one of", + ), + ({"linked_worktree_args": ("{gitdir}", "")}, "non-empty strings"), # the remaining value-level knobs ({"usage_parser": "magic"}, "usage_parser"), ({"usage_grace_s": -1.0}, "usage_grace_s"), diff --git a/tests/test_runsetup.py b/tests/test_runsetup.py index f71df7ede..bd3a11a63 100644 --- a/tests/test_runsetup.py +++ b/tests/test_runsetup.py @@ -426,6 +426,24 @@ def test_sandboxed_permission_mode_moves_the_digest_and_hashes_its_args(pinned): assert _digest(pinned, sandboxed) != before +def test_linked_worktree_args_are_hashed_only_when_declared(pinned): + """DW-599: the grant template adds argv tokens in a linked worktree, so it is + pinned; a mode that declares none hashes no key (its digest is unchanged).""" + sandboxed = _with_adapter_key('permission_mode = "sandboxed"') + (pinned / PROFILE_REL).write_text(PROFILE + _SANDBOX_TABLE, encoding="utf-8") + without = _digest(pinned, sandboxed) + grant = 'linked_worktree_args = ["-g", "{gitdir}"]\n' + (pinned / PROFILE_REL).write_text(PROFILE + _SANDBOX_TABLE + grant, encoding="utf-8") + declared = _digest(pinned, sandboxed) + assert declared != without + (pinned / PROFILE_REL).write_text( + PROFILE + _SANDBOX_TABLE + grant.replace('"-g"', '"-G"'), encoding="utf-8" + ) + assert _digest(pinned, sandboxed) not in (without, declared) + # bypass never resolves the mode, so the declared template is not launched + assert _digest(pinned, POLICY) == _PRE_PERMISSION_MODE_DIGEST + + def test_digest_raises_on_an_unmapped_permission_mode(pinned): """The same rule as an unknown profile: the launch refuses the mode, so the digest must not hash a launch surface that never runs.""" From 4d6ee0953ce0d3b36a21d1eb55b7fffb9a2a9545 Mon Sep 17 00:00:00 2001 From: t Date: Wed, 28 Oct 2026 14:13:43 -0700 Subject: [PATCH 2/9] fix(validate): sandboxed Codex under worktree isolation is ok when the gitdir grant rides (DW-599) `_report_permission_mode` warned on every Codex-family role sandboxed under scm.isolation=worktree. Since the sandboxed mode's `linked_worktree_args` grant the linked worktree's gitdir and the common dir, that role is now ok, with a message naming the grant. It stays a warning when the grant cannot ride: the resolved profile's mode declares no `linked_worktree_args` (a user overlay, say), or a non-None `extra_args` (an empty one included) replaces the mode's args and the grant with them, as `interactive_argv` does. Check id and detail dict unchanged. Docs: FEATURES, the cli.md check row, the adapter-authoring guide's `linked_worktree_args` key (placeholders, validation, when it is appended, fault refusal, unfilled-template digest) and the common-dir exposure note; core.toml's `extra_args` description (configuration.md regenerated) says it drops the grant; the config_digest docstring now says the template is hashed unfilled. --- CHANGELOG.md | 3 + docs/FEATURES.md | 2 +- docs/adapter-authoring-guide.md | 40 ++++++++++--- docs/reference/cli.md | 2 +- docs/reference/configuration.md | 2 +- src/bmad_loop/cli.py | 41 ++++++++++--- src/bmad_loop/data/settings/core.toml | 2 +- src/bmad_loop/runsetup.py | 5 +- tests/test_cli.py | 85 +++++++++++++++++++++++---- 9 files changed, 147 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f151cdb4..39e9c15d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -218,6 +218,9 @@ breaking changes may land in a minor release. ### Fixed +- Let sandboxed Codex sessions commit under worktree isolation: grant them write access to + the linked worktree's gitdir and the main repo's `.git`. `validate` no longer warns + unless `extra_args` drops the grant. - Pause a tickets resume at `ticket-tree` (`binding-moved`) when an in-flight unit's ticket was moved with `tickets.py move` or its plan is gone, naming the old and new ref and plan, instead of re-dispatching a ref the tree no longer holds (DW-575). diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 334e615a8..29987e829 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -358,7 +358,7 @@ bmad-loop drives a BMAD sprint unattended: a deterministic Python loop launches - A stage inherits the base mode even when it switches client; `[adapter.] permission_mode` overrides it. - `bmad-loop resolve` sessions, where a human is present, keep the bypass flags whatever the mode. - `validate` reports each role's permission posture as `adapter.permission-mode`, suggesting `sandboxed` for an unsandboxed role without worktree isolation when the profile maps it. The note is advisory: a worktree is not a security boundary. - - Sandboxed Codex under `scm.isolation = "worktree"` is a warning: the sandbox keeps a linked worktree's gitdir read-only, so git add and commit fail. + - A sandboxed Codex session in a linked worktree (`scm.isolation = "worktree"`) gets exact-path write grants for that worktree's gitdir and the main repo's shared `.git`, so git add and commit work. `validate` warns only when the grant cannot ride: an `extra_args` override replaces the mode's args and drops it, or the profile's mode declares no `linked_worktree_args`. - `tickets.store-sandbox` also warns when a sandboxed role writes a ticket store outside its cwd. - **Config:** `[adapter] permission_mode`, `[adapter.dev]` / `[adapter.review]` / `[adapter.triage]` / `[adapter.retro]` `permission_mode`; profile table `[permission_modes.]` - **Tested by:** `tests/test_policy.py`, `tests/test_profile.py`, `tests/test_runsetup.py`, `tests/test_cli.py`, `tests/test_codex_trust.py`, `tests/test_install.py` diff --git a/docs/adapter-authoring-guide.md b/docs/adapter-authoring-guide.md index 00e3cbc47..ad651b5ae 100644 --- a/docs/adapter-authoring-guide.md +++ b/docs/adapter-authoring-guide.md @@ -519,10 +519,25 @@ deny_guard = ['Do you want to procee[d]\?'] default) launches `bypass_args`. Any other mode launches the args of the profile's `[permission_modes.]` table in their place: -| Field | Required | Meaning | -| ----------- | -------- | -------------------------------------------------------------------------------------------------------- | -| `args` | ✅ | Launch args that stand in for `bypass_args`. A policy `extra_args` still replaces them. | -| `platforms` | | `sys.platform` values the mapping works on (`linux`, `darwin`, `win32`). Empty or unset: every platform. | +| Field | Required | Meaning | +| ---------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------ | +| `args` | ✅ | Launch args that stand in for `bypass_args`. A policy `extra_args` still replaces them. | +| `platforms` | | `sys.platform` values the mapping works on (`linux`, `darwin`, `win32`). Empty or unset: every platform. | +| `linked_worktree_args` | | Args appended after `args` when the session runs in a linked worktree, with `{gitdir}` and `{common_dir}` filled in. Empty or unset: none. | + +`linked_worktree_args` is for a sandbox that must be granted a linked worktree's git +directories explicitly. When the session's cwd is in a linked worktree (as under +`scm.isolation = "worktree"`), the generic adapter asks git for that worktree's gitdir +(`
/.git/worktrees/`) and the common dir (`
/.git`), resolves both, and +appends the list after the mode's `args` with each `{gitdir}` / `{common_dir}` replaced by +that path written as a TOML basic string, quotes included. In a main checkout nothing is +appended. Under `bypass`, or when a policy `extra_args` replaces the mode's args, nothing is +appended and git is not asked. + +If git cannot answer, or a path cannot be written as TOML, the session refuses to launch +rather than start without the grant. Each entry must be a non-empty string, and at least one +entry must use a placeholder: an arg with no path in it belongs in `args`. The config digest +hashes the unfilled list, so a resumed run's digest does not depend on the worktree path. A profile without the table, or on an excluded platform, refuses the mode at run start and in `validate` (`adapter.permission-mode-unsupported`). `opencode-http` never launches @@ -537,14 +552,21 @@ return to the agent as an error. The shipped mappings, both for `linux` and `dar sandbox with no unsandboxed retry. Sandboxed Bash can run git and write `.git`; network is denied. On Linux the Bash sandbox needs bubblewrap and socat installed, or the session refuses to start. -- **codex:** a `-c` permission profile extending `:workspace` with a `.git` write grant, - plus `--ask-for-approval never`. Network is off. In a linked worktree the gitdir under - the main repo's `.git/worktrees/` stays read-only, so git add and commit fail there; - `validate` warns about sandboxed Codex under `scm.isolation = "worktree"`. +- **codex:** a `-c` permission profile `bmad_loop` extending `:workspace` with a `.git` + write grant, plus `--ask-for-approval never`. Network is off. Its `linked_worktree_args` + define a child profile `bmad_loop_worktree` that extends `bmad_loop` with write grants + for the linked worktree's gitdir and the common dir, and select it with a later + `default_permissions`, so git add and commit work in a worktree checkout. `validate` + warns about sandboxed Codex under `scm.isolation = "worktree"` only when that grant + cannot ride: an `extra_args` override, or a profile whose mode declares no + `linked_worktree_args`. The sandbox confines the session, not what bmad-loop later runs from the checkout. Both mappings leave `.git` writable, hooks and config included, and the orchestrator runs -`[verify] commands` and its own git commits there unsandboxed. +`[verify] commands` and its own git commits there unsandboxed. In a linked worktree, +Codex's common-dir grant opens the main repo's `.git` (hooks, config, the main index and +refs) to the session, the same exposure a sandboxed session in the main checkout already +has through the `.git` grant. Both CLIs show a one-time workspace-trust dialog in a directory they have not seen. `validate` reports each role's posture as `adapter.permission-mode`. diff --git a/docs/reference/cli.md b/docs/reference/cli.md index cdb5ca782..800a20e6c 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -631,7 +631,7 @@ Every finding's `check` is one of these ids. Severity is `ok`, `warning` or `pro | `policy.effort-unsupported` | warning | `effort` set on a profile whose adapter ignores it | | `policy.bypass-dropped` | warning | an explicit `extra_args` drops the profile's `bypass_args` | | `adapter.permission-mode-unsupported` | problem | a role's profile cannot run its `permission_mode` on this platform | -| `adapter.permission-mode` | ok / warning | each role's permission posture; a warning for sandboxed Codex under worktree isolation | +| `adapter.permission-mode` | ok / warning | each role's permission posture; a warning for sandboxed Codex under worktree isolation when no linked-worktree gitdir grant rides | | `adapter.launch-probe` | ok / warning / problem | `--probes`: each distinct `generic`-profile launch; a startup dialog is a problem, an unseen screen a warning; ok with nothing launched if no patterns | | `adapter.profile` | problem | a role's adapter name does not resolve to a loadable profile | | `adapter.binary` | ok / problem | the profile binary is on `PATH` | diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index cce0bab1e..c38aa118b 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -143,7 +143,7 @@ CLI client, model & permission flags (base for all stages). | `name` | string | unset — CLI profile — default: claude | | | | `model` | string | unset — CLI default model | | | | `effort` | string | unset — provider default | | reasoning effort, free-form (provider/model-specific, e.g. high, max); sent by opencode-http as the model variant (on 2.x it must be one of the model's variants), ignored by the tmux CLIs | -| `extra_args` | array of strings | unset | | when set, replaces the CLI profile's permission-bypass flags (its bypass_args), so repeat them (e.g. claude: --permission-mode bypassPermissions) or unattended sessions may stall on permission prompts | +| `extra_args` | array of strings | unset | | when set, replaces the CLI profile's permission-bypass flags (its bypass_args), so repeat them (e.g. claude: --permission-mode bypassPermissions) or unattended sessions may stall on permission prompts · under a sandboxed permission_mode it also drops the mode's linked-worktree gitdir grant, so a sandboxed codex session under worktree isolation cannot commit | | `permission_mode` | string | `"bypass"` | `bypass` / `sandboxed` | bypass (default): launch the CLI profile's permission-bypass flags (its bypass_args) · sandboxed: launch the profile's non-blocking sandboxed flags instead — a denied action returns to the agent as an error rather than a prompt (claude and codex on linux/macOS; other profiles refuse it) · extra_args, when set, still replaces either · stages inherit it even when they switch client | | `cleanup_session_on_finish` | boolean | `true` | | | | `usage_grace_s` | float | unset — inherit from CLI profile (copilot ships 8s) | ≥ 0 | seconds to poll the transcript for token usage after a session ends; CLIs that flush totals on shutdown (copilot) need a grace | diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index b00ded1d0..4047997a8 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -2268,10 +2268,15 @@ def _report_permission_mode( ``runsetup.make_adapters``. Otherwise an ok posture line (``adapter.permission-mode``) whose isolation coupling is advisory only: a bypass role with no worktree isolation is pointed at the sandboxed mode when - its profile maps one here. The one warning is a Codex-family role sandboxed - under worktree isolation, where git cannot write the linked worktree's gitdir - (it lives under the main repo's ``.git/worktrees/``, outside the sandbox's - writable roots), so the dev skill's preflight and commit fail.""" + its profile maps one here. A Codex-family role sandboxed under worktree + isolation needs the mode's ``linked_worktree_args`` grant: its sandbox keeps the + linked worktree's gitdir (under the main repo's ``.git/worktrees/``) and the + common dir read-only otherwise, so the dev skill's preflight and commit fail. + The role is ok when its launch carries that grant (the resolved profile + declares it and ``cfg.extra_args`` is None). It warns when the grant cannot + ride: the resolved profile declares none, or a non-None ``extra_args`` (``()`` + included) replaces the mode's args, and the grant with them, as + ``GenericAdapter.interactive_argv`` does.""" from .adapters.profile import ProfileError mode = cfg.permission_mode @@ -2283,7 +2288,7 @@ def _report_permission_mode( "platform": sys.platform, } try: - profile.for_permission_mode(mode, platform=sys.platform) + resolved = profile.for_permission_mode(mode, platform=sys.platform) except ProfileError as e: report.fail( "adapter.permission-mode-unsupported", @@ -2303,12 +2308,30 @@ def _report_permission_mode( message += '; consider adapter.permission_mode = "sandboxed"' report.ok("adapter.permission-mode", message, detail) elif isolation == "worktree" and profile.hooks.dialect == "codex-hooks-json": + fallback = 'use scm.isolation = "none" or permission_mode = "bypass"' + if not resolved.linked_worktree_args: + why = f"its [permission_modes.{mode}] declares no linked_worktree_args grant" + fix = fallback + elif cfg.extra_args is not None: + why = ( + "adapter.extra_args replaces the mode's args, dropping their " + "linked_worktree_args grant" + ) + fix = f"drop extra_args, or {fallback}" + else: + report.ok( + "adapter.permission-mode", + f"{profile.name} {role}: {mode} under scm.isolation=worktree, with write " + "grants for the linked worktree's gitdir and the main repo's .git", + detail, + ) + return report.warn( "adapter.permission-mode", - f"{profile.name} {role}: {mode} under scm.isolation=worktree — the sandbox " - "keeps a linked worktree's gitdir (under the main repo's .git/worktrees/) " - "read-only, so git add/commit fail in isolated sessions; use " - 'scm.isolation = "none" or permission_mode = "bypass" for this role', + f"{profile.name} {role}: {mode} under scm.isolation=worktree — {why}, so the " + "sandbox keeps the linked worktree's gitdir (under the main repo's " + f".git/worktrees/) and the main repo's .git read-only and git add/commit fail " + f"in isolated sessions; {fix} for this role", detail, ) else: diff --git a/src/bmad_loop/data/settings/core.toml b/src/bmad_loop/data/settings/core.toml index 2db998cdc..75c411da8 100644 --- a/src/bmad_loop/data/settings/core.toml +++ b/src/bmad_loop/data/settings/core.toml @@ -306,7 +306,7 @@ description = "reasoning effort, free-form (provider/model-specific, e.g. high, [[section.field]] key = "extra_args" kind = "args" -description = "when set, replaces the CLI profile's permission-bypass flags (its bypass_args), so repeat them (e.g. claude: --permission-mode bypassPermissions) or unattended sessions may stall on permission prompts" +description = "when set, replaces the CLI profile's permission-bypass flags (its bypass_args), so repeat them (e.g. claude: --permission-mode bypassPermissions) or unattended sessions may stall on permission prompts · under a sandboxed permission_mode it also drops the mode's linked-worktree gitdir grant, so a sandboxed codex session under worktree isolation cannot commit" [[section.field]] key = "permission_mode" kind = "select" diff --git a/src/bmad_loop/runsetup.py b/src/bmad_loop/runsetup.py index 49a46dfe8..3dc3e8d5f 100644 --- a/src/bmad_loop/runsetup.py +++ b/src/bmad_loop/runsetup.py @@ -169,8 +169,9 @@ def config_digest( keeps this complete: walk ``GenericAdapter.interactive_argv`` and ``interactive_env`` and every token there traces back to one of ``binary`` / ``launch_args`` / ``bypass_args`` / ``linked_worktree_args`` - (its paths filled in) / ``model_flag`` / ``prompt_template`` / ``env`` on - the *resolved* profile, or to + (hashed as the UNFILLED template, placeholders and all, so a resumed run's + digest does not depend on its worktree's path) / ``model_flag`` / + ``prompt_template`` / ``env`` on the *resolved* profile, or to ``extra_args`` on the resolved adapter — plus ``session_id_flag``, which ``build_command`` appends (with a minted id) to the launched argv only. The opencode-http builder reads a strict SUBSET of those — ``_serve_argv`` diff --git a/tests/test_cli.py b/tests/test_cli.py index 78d12926c..2854bf2d1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -73,6 +73,7 @@ from bmad_loop import probe as probe_mod from bmad_loop import runs, runsetup, verify from bmad_loop.adapters import multiplexer as mux_mod +from bmad_loop.adapters.profile import CLIProfile from bmad_loop.adapters.psmux_backend import PsmuxMultiplexer from bmad_loop.plugins import GrantSet from bmad_loop.plugins.store import load_grants @@ -13409,7 +13410,14 @@ def test_validate_text_mode_prints_the_permission_posture(project, capsys, monke assert "claude dev: bypass, unsandboxed (scm.isolation=none)" in capsys.readouterr().out -def _posture(monkeypatch, platform: str, profile: str, mode: str, isolation: str = "none"): +def _posture( + monkeypatch, + platform: str, + profile: str | CLIProfile, + mode: str, + isolation: str = "none", + extra_args: tuple[str, ...] | None = None, +): from bmad_loop.adapters.profile import get_profile from bmad_loop.checks import ValidationReport @@ -13418,8 +13426,8 @@ def _posture(monkeypatch, platform: str, profile: str, mode: str, isolation: str cli._report_permission_mode( report, "dev", - types.SimpleNamespace(permission_mode=mode), - get_profile(profile), + types.SimpleNamespace(permission_mode=mode, extra_args=extra_args), + get_profile(profile) if isinstance(profile, str) else profile, isolation, ) monkeypatch.undo() @@ -13469,17 +13477,72 @@ def test_store_sandboxed_roles_names_the_non_bypass_launch_roles(tmp_path): assert cli._store_sandboxed_roles(policy_mod.load(policy), cli.worksource.SOURCE_TICKETS) == () -def test_permission_posture_warns_sandboxed_codex_under_worktree_isolation(monkeypatch): - """Codex's sandbox keeps a linked worktree's gitdir read-only, so git - add/commit fail there: a warning, not the ok line. Claude's sandbox passes in - a linked worktree and keeps the ok line.""" +def test_permission_posture_sandboxed_codex_under_worktree_isolation_carries_the_grant( + monkeypatch, +): + """Codex's sandboxed mode declares `linked_worktree_args`, which grant a linked + worktree's gitdir and the main repo's .git, so a sandboxed Codex role under + worktree isolation can commit: the ok line, naming the grant. Claude's sandbox + needs no grant and keeps its plain ok line. + + ABLATION: warn unconditionally in the codex-hooks-json arm and this reddens.""" (codex,) = _posture(monkeypatch, "linux", "codex", "sandboxed", isolation="worktree") - assert (codex.check, codex.severity) == ("adapter.permission-mode", "warning") - assert ".git/worktrees/" in codex.message + assert (codex.check, codex.severity) == ("adapter.permission-mode", "ok") + assert codex.message == ( + "codex dev: sandboxed under scm.isolation=worktree, with write grants for the " + "linked worktree's gitdir and the main repo's .git" + ) + assert codex.detail == { + "role": "dev", + "profile": "codex", + "mode": "sandboxed", + "isolation": "worktree", + "platform": "linux", + } (claude,) = _posture(monkeypatch, "linux", "claude", "sandboxed", isolation="worktree") - assert claude.severity == "ok" + assert (claude.severity, claude.message) == ("ok", "claude dev: sandboxed") (codex_main,) = _posture(monkeypatch, "linux", "codex", "sandboxed") - assert codex_main.severity == "ok" + assert (codex_main.severity, codex_main.message) == ("ok", "codex dev: sandboxed") + + +@pytest.mark.parametrize("extra_args", [("--custom",), ()], ids=["override", "empty"]) +def test_permission_posture_warns_when_extra_args_drops_the_worktree_grant(monkeypatch, extra_args): + """A non-None `extra_args` (an empty one included) replaces the mode's args, + and the linked-worktree grant rides with them: git add/commit fail again.""" + (finding,) = _posture( + monkeypatch, "linux", "codex", "sandboxed", isolation="worktree", extra_args=extra_args + ) + assert (finding.check, finding.severity) == ("adapter.permission-mode", "warning") + assert "adapter.extra_args replaces the mode's args" in finding.message + assert ".git/worktrees/" in finding.message + assert finding.message.endswith( + 'drop extra_args, or use scm.isolation = "none" or permission_mode = "bypass" ' + "for this role" + ) + # Without worktree isolation the same override needs no grant. + (main,) = _posture(monkeypatch, "linux", "codex", "sandboxed", extra_args=extra_args) + assert main.severity == "ok" + + +def test_permission_posture_warns_a_codex_mode_without_a_worktree_grant(monkeypatch): + """A Codex-family profile (a user overlay, say) whose sandboxed mode declares no + `linked_worktree_args` cannot open the linked worktree's gitdir: a warning.""" + from bmad_loop.adapters.profile import get_profile + + codex = get_profile("codex") + bare = dataclasses.replace( + codex, + permission_modes={ + "sandboxed": dataclasses.replace( + codex.permission_modes["sandboxed"], linked_worktree_args=() + ) + }, + ) + (finding,) = _posture(monkeypatch, "linux", bare, "sandboxed", isolation="worktree") + assert (finding.check, finding.severity) == ("adapter.permission-mode", "warning") + assert "declares no linked_worktree_args grant" in finding.message + assert ".git/worktrees/" in finding.message + assert "drop extra_args" not in finding.message def _stub_launch_probe(monkeypatch, *results): From e5bf57b6ea348d301660ce0932dd3f7fc7d9fea6 Mon Sep 17 00:00:00 2001 From: t Date: Wed, 28 Oct 2026 14:33:44 -0700 Subject: [PATCH 3/9] fix(opencode): classify captured 2.x provider outages (5xx, auth, no-route) as environment faults Extend the opencode profile's V2 provider-error env-fault alternation from rate-limit|transport to rate-limit|transport|internal|auth|no-route, seeded only from lines captured live. Probe: opencode v2.0.32, 2026-10-28, zero real tokens. Each scenario ran one session through OpencodeHttpAdapter.run() (scrubbed env, mock-only opencode.json with the hosted provider denied, as in test_opencode_live_turn) against a local stdlib OpenAI-compatible fake; lines are the adapter's own .provider-errors.log output. HTTP 500/503 (JSON server_error) provider.internal, retry.scheduled x10 (attempts 2..11), then step.failed + execution.failed HTTP 502 (HTML body) provider.internal, same; message "Provider request failed with HTTP 502" HTTP 401 invalid_api_key provider.auth, step.failed + execution.failed, not retried HTTP 403 permission_error provider.auth, not retried session model nosuch/mock-1 provider.no-route "Model unavailable: nosuch/mock-1", execution.failed only, no provider request unknown variant (adapter's provider.no-route "Variant unavailable pre-check bypassed in the probe) for mock/mock-1: bogus", same HTTP 400 invalid_request_error provider.invalid-request, not retried malformed SSE chunk provider.invalid-output "Invalid mock/openai-compatible-chat stream event", not retried stream cut before finish_reason provider.invalid-output "OpenAI Chat stream ended without finish_reason", retried hung request / hung stream nothing for 300 s; adapter timeout, then type=aborted "Step interrupted" hung, timeout/headerTimeout 3000 provider.transport "Timed out waiting for response headers", retried hung stream, chunkTimeout 3000 provider.transport "Timed out waiting for response data" (status 200) Lines as written (seeded types): provider-error event=session.retry.scheduled type=provider.internal status=500 attempt=2 message="The server had an error while processing your request. Sorry about that!" provider-error event=session.retry.scheduled type=provider.internal status=503 attempt=2 message="The engine is currently overloaded, please try again later." provider-error event=session.execution.failed type=provider.internal status=502 attempt=- message="Provider request failed with HTTP 502" provider-error event=session.step.failed type=provider.auth status=401 attempt=- message="Incorrect API key provided: x. You can find your API key at https://platform.openai.com/account/api-keys." provider-error event=session.execution.failed type=provider.auth status=403 attempt=- message="Your organization does not have access to this model." provider-error event=session.execution.failed type=provider.no-route status=- attempt=- message="Model unavailable: nosuch/mock-1" provider-error event=session.execution.failed type=provider.no-route status=- attempt=- message="Variant unavailable for mock/mock-1: bogus" Not seeded: invalid-request and invalid-output (the session's own request can cause them); provider.timeout was not provoked (the 2.0.32 binary raises its Timeout reason only from media-generation polling; chat timeouts read provider.transport), nor content-filter, unsupported-operation, unknown, error. The server log's "Failed to drain Session" ERROR line appeared for every failed execution (auth, internal, invalid-request, invalid-output, no-route) with only `cause="AI.Error: "` and a stack, no error type, so it cannot tell an outage from a refusal: not seeded. Tests: the captured lines join OPENCODE_REAL with their frames (provenance test rebuilds them via format_provider_error); captured invalid-request / invalid-output lines must match no opencode field; the bait's auth/no-route lines move to still-unseeded types. Ablation: reverting the alternation reddens 16 tests; widening it to invalid-request|invalid-output reddens 4. --- CHANGELOG.md | 3 + docs/FEATURES.md | 2 +- docs/adapter-authoring-guide.md | 5 +- docs/setup-guide.md | 3 +- src/bmad_loop/adapters/opencode_protocol.py | 13 +- src/bmad_loop/data/profiles/opencode.toml | 30 ++- tests/test_env_fault_patterns.py | 252 +++++++++++++++++++- 7 files changed, 293 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39e9c15d5..9f6b5e2ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -218,6 +218,9 @@ breaking changes may land in a minor release. ### Fixed +- Classify OpenCode 2.x provider 5xx errors (`provider.internal`), rejected credentials + (`provider.auth`) and unroutable models (`provider.no-route`) as environment faults + instead of charging the session as a code failure. - Let sandboxed Codex sessions commit under worktree isolation: grant them write access to the linked worktree's gitdir and the main repo's `.git`. `validate` no longer warns unless `extra_args` drops the grant. diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 29987e829..640e06855 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -169,7 +169,7 @@ bmad-loop drives a BMAD sprint unattended: a deterministic Python loop launches - `bmad-loop resume` re-runs the probes; if they pass, the withheld session launches with no rollback, and otherwise the run pauses again unchanged. - A verify command exiting 126, 127 or `[verify] env_fault_rc` is an environment fault, not a code failure. - A non-completed session whose log tail matches its profile's `env_fault_patterns` pauses instead of spending a dev attempt. - - opencode is classified from model-free files only: the server's stdout, plus on OpenCode 2.x the adapter-written `provider-errors.log` (one line per provider-error event). Its 2.x environment-fault patterns cover the `provider.rate-limit` and `provider.transport` error types; `provider.quota` is a usage limit. + - opencode is classified from model-free files only: the server's stdout, plus on OpenCode 2.x the adapter-written `provider-errors.log` (one line per provider-error event). Its 2.x environment-fault patterns cover the `provider.rate-limit`, `provider.transport`, `provider.internal` (a provider 5xx), `provider.auth` and `provider.no-route` error types; `provider.quota` is a usage limit, and error types the session's own request can cause (`provider.invalid-request`, `provider.invalid-output`) stay unclassified. - A pattern match is overruled when the session's tracked token usage is nonzero; that session is charged like any failure. - A session that times out, stalls or crashes after a live usage-limit sighting pauses with a `usage limit:` reason and is never charged, whatever its token usage. A live sighting is a match for the profile's `usage_limit_patterns` or `usage_wait_patterns` from one of two sources: the visible pane of a tmux-hosted CLI such as claude, on an idle session at stall-grace expiry or when the timeout fires; or an opencode 2.x provider-error line, matched as the adapter writes it while the session runs (opencode's shipped profile lists `provider.quota` there as a blocking limit). The same line found only in the log tail stays subject to the overrule. - When the usage-limit line names a reset time, the pause reason ends its evidence with `; resets