diff --git a/plugins/magpie-setup/skills/setup/setup_preflight/cli.py b/plugins/magpie-setup/skills/setup/setup_preflight/cli.py index b1123c341..31ade8803 100644 --- a/plugins/magpie-setup/skills/setup/setup_preflight/cli.py +++ b/plugins/magpie-setup/skills/setup/setup_preflight/cli.py @@ -33,8 +33,10 @@ import argparse import json +import os import subprocess import sys +from collections.abc import Mapping from pathlib import Path from .core import ( @@ -47,50 +49,110 @@ ) from .lockfile import MalformedLock +#: How to notice which harness is running, and that harness's own +#: plugin-listing command (`install.md` Step M2/M5). The markers mirror +#: the canonical self-detection order in `tools/adversarial-review`: a +#: harness started from inside another inherits the outer one's variables, +#: so the innermost candidates are checked first and Claude Code's widely +#: inherited `CLAUDECODE` comes last. Only the running harness's listing +#: runs — a machine may have several installed, and whichever comes first +#: in a fixed probe order is not the one whose versions this session runs. +#: Copilot installs from a repo URL or catalogue and has no listing CLI, +#: so its entry has none: the floor stays unknown there rather than +#: guessing what a listing would print. +HARNESS_LISTINGS: tuple[ + tuple[str, tuple[tuple[str, str | None], ...], list[str] | None], ... +] = ( + ("codex", (("CODEX_SANDBOX", None), ("CODEX_THREAD_ID", None)), ["codex", "plugin", "list", "--json"]), + ("copilot", (("COPILOT_CLI", None),), None), + ("gemini", (("GEMINI_CLI", "1"),), ["gemini", "extensions", "list", "-o", "json"]), + ("claude", (("CLAUDECODE", "1"),), ["claude", "plugin", "list", "--json"]), +) + + +def _running_harness(env: Mapping[str, str]) -> tuple[str | None, list[str] | None]: + """The harness these environment variables are running, and its listing.""" + for harness, markers, argv in HARNESS_LISTINGS: + for var, want in markers: + value = env.get(var) + if value and (want is None or value == want): + return harness, argv + return None, None -def read_installed(explicit: str | None) -> dict[str, str] | None: - """The installed plugin map, or `None` for *unknown* — never `{}` for it. - `--plugin-list` lets the caller supply what it already read; otherwise - the harness CLI is asked directly. Every failure mode collapses to - `None`: the command missing, a non-zero exit, unparsable output, or a - sandbox that denies the plugin cache. The one case that is *not* - unknown is a listing that parsed and was genuinely empty, which only a - harness with no plugins installed produces. +def _parse_listing(text: str) -> dict[str, str] | None: + """One harness listing, parsed into the shared `{name: version}` map. + + Shared by every harness's listing command: `None` for unknown (the + output did not parse, or an empty parsed listing leaves the sandbox + read-denial ambiguity standing), never an empty map. """ - if explicit is not None: - text = sys.stdin.read() if explicit == "-" else Path(explicit).read_text(encoding="utf-8") - else: - try: - completed = subprocess.run( - ["claude", "plugin", "list", "--json"], - capture_output=True, - text=True, - timeout=20, - check=False, - ) - except (OSError, subprocess.SubprocessError): - return None - if completed.returncode != 0: - return None - text = completed.stdout try: - parsed = json.loads(text) + listing = json.loads(text) except json.JSONDecodeError: return None - if not isinstance(parsed, list): + if not isinstance(listing, list): return None - installed: dict[str, str] = {} - for entry in parsed: - if not isinstance(entry, dict): + out: dict[str, str] = {} + for item in listing: + if not isinstance(item, dict): return None - name, version = entry.get("name"), entry.get("version") - if isinstance(name, str) and isinstance(version, str): - installed[name] = version - # An empty *parsed* list inside a sandbox is the read-denied case, and - # is indistinguishable from a genuinely plugin-free harness. Unknown is - # the safe reading of the ambiguity: it proposes nothing. - return installed or None + version = item.get("version") + if isinstance(item.get("name"), str) and isinstance(version, str): + out[item["name"]] = version + # A listing that parses but is empty leaves the sandbox read-denial + # ambiguity standing, so we report unknown rather than an empty map. + return out or None + + +def read_installed( + explicit: str | None, env: Mapping[str, str] | None = None +) -> tuple[str | None, dict[str, str] | None]: + """The running harness and its installed plugin map, or `None` for + *unknown* — never `{}` for it. The pair is a coherent answer: both + are `None` unless the running harness's own listing was read and + parsed. + + `--plugin-list` lets the caller supply what it already read; the + harness is then not known, and the map stands alone. Otherwise the + running harness is detected from its own environment markers and only + its listing command runs — no other harness's listing may answer for + it. Every failure mode collapses to `(None, None)`: the harness + undetectable, a listing CLI the running harness does not have + (Copilot), the command missing, a non-zero exit, a timeout or a + refusal from the sandbox, unparsable output, or a sandbox that denies + the plugin cache. The one case that is *not* unknown is a listing + that parsed and was genuinely empty, which only a harness with no + plugins installed produces. + """ + if explicit is not None: + text = sys.stdin.read() if explicit == "-" else Path(explicit).read_text(encoding="utf-8") + return None, _parse_listing(text) + harness, argv = _running_harness(os.environ if env is None else env) + if argv is None: + # No harness is running here, or the one running has no listing + # CLI (Copilot installs from a repo URL): unknown, and no other + # harness's listing may be probed instead. + return None, None + try: + completed = subprocess.run( + argv, + check=False, + capture_output=True, + text=True, + timeout=20, + ) + except (OSError, subprocess.SubprocessError): + # The listing cannot be produced: the binary is missing from the + # harness it belongs to, it times out, or the sandbox refuses to + # execute it. Unknown — never another harness's answer. + return None, None + if completed.returncode != 0: + return None, None + parsed = _parse_listing(completed.stdout) + if parsed is None: + return None, None + return harness, parsed def build_parser() -> argparse.ArgumentParser: @@ -111,7 +173,7 @@ def build_parser() -> argparse.ArgumentParser: parser.add_argument( "--plugin-list", metavar="PATH", - help="`claude plugin list --json` output to use instead of running it ('-' for stdin)", + help="harness plugin listing JSON to use instead of running it ('-' for stdin)", ) parser.add_argument( "--verify-interval-days", @@ -144,13 +206,13 @@ def main(argv: list[str] | None = None) -> int: root: Path = args.project_root try: - installed = read_installed(args.plugin_list) + harness, installed = read_installed(args.plugin_list) if args.no_cache: from .core import project_findings - project, cached = project_findings(root, installed), False + project, cached = project_findings(root, installed, harness), False else: - project, cached = cached_project_findings(root, installed) + project, cached = cached_project_findings(root, installed, harness=harness) findings = [ *project, *skill_findings(root, args.skill, args.surface_hash, args.requires), diff --git a/plugins/magpie-setup/skills/setup/setup_preflight/core.py b/plugins/magpie-setup/skills/setup/setup_preflight/core.py index 412e0aaad..407dbaca3 100644 --- a/plugins/magpie-setup/skills/setup/setup_preflight/core.py +++ b/plugins/magpie-setup/skills/setup/setup_preflight/core.py @@ -152,7 +152,9 @@ def _snapshot_findings(lock: Lock, root: Path) -> list[Finding]: return [] -def _floor_findings(lock: Lock, installed: dict[str, str] | None) -> list[Finding]: +def _floor_findings( + lock: Lock, installed: dict[str, str] | None, harness: str | None +) -> list[Finding]: if lock.url and lock.url != TRUSTED_MARKETPLACE: return [Finding("project", "untrusted-marketplace", "step-3", {"url": lock.url})] if installed is None: @@ -177,7 +179,7 @@ def _floor_findings(lock: Lock, installed: dict[str, str] | None) -> list[Findin "project", "below-floor", "step-3", - {"missing": missing, "outdated": outdated, "restart_required": True}, + {"missing": missing, "outdated": outdated, "restart_required": True, "harness": harness}, ) ] @@ -223,8 +225,16 @@ def legacy_layout_findings(root: Path) -> list[Finding]: ] -def project_findings(root: Path, installed: dict[str, str] | None) -> list[Finding]: - """Everything true of the checkout rather than of one skill.""" +def project_findings( + root: Path, installed: dict[str, str] | None, harness: str | None = None +) -> list[Finding]: + """Everything true of the checkout rather than of one skill. + + `harness` is the agent the session is running under, as the floor + check measured it — `None` when the listing came from `--plugin-list` + or could not be read. It travels with the findings so the rules can + name that harness's own install and update commands. + """ lock = load(root / LOCK_NAME) if lock is None: # A supported end state, not a fault: the marketplace install @@ -235,7 +245,7 @@ def project_findings(root: Path, installed: dict[str, str] | None) -> list[Findi if lock.method in SNAPSHOT_METHODS: return _snapshot_findings(lock, root) if lock.method == "marketplace": - return _floor_findings(lock, installed) + return _floor_findings(lock, installed, harness) if lock.method == LOCAL_METHOD: return [] return [Finding("project", "unknown-method", "step-2", {"method": lock.method})] @@ -445,7 +455,7 @@ def isolated_setup_findings( # --- memoising the project scope ---------------------------------------------------- -def _cache_key(root: Path, installed: dict[str, str] | None) -> str: +def _cache_key(root: Path, installed: dict[str, str] | None, harness: str | None) -> str: # Linked worktrees of an adopted repository can share their main # checkout's personal layer, and so this cache file: key it on the # checkout as well, so one worktree never serves another's verdict. @@ -458,6 +468,9 @@ def _cache_key(root: Path, installed: dict[str, str] | None) -> str: # The legacy-layout finding depends on the in-tree directory existing. parts.append("legacy:" + ("1" if legacy_local_dir(root) is not None else "-")) parts.append("installed:" + ("?" if installed is None else json.dumps(installed, sort_keys=True))) + # The below-floor finding names the harness the listing came from, so a + # verdict computed under one harness must not be served to another. + parts.append("harness:" + (harness or "-")) # The verdict is also a function of this code: an upgrade that refreshes # the checker must not keep serving the previous checker's answer. here = Path(__file__).parent @@ -474,6 +487,7 @@ def _cache_key(root: Path, installed: dict[str, str] | None) -> str: def cached_project_findings( root: Path, installed: dict[str, str] | None, + harness: str | None = None, *, now: float | None = None, ) -> tuple[list[Finding], bool]: @@ -481,19 +495,19 @@ def cached_project_findings( Returns `(findings, was_cached)`. The cache lives in the personal layer (`layers.personal_dir`, which in a linked worktree of an adopted - repository may be its main checkout's), is keyed on the checkout, the lock files' identity and - the plugin listing, and expires so that a plugin installed mid-session - is picked up by the next skill rather than at the end of the day. A - project whose personal layer does not exist is not cached at all — - creating it would create the very state whose absence the skill scope - reads as "never configured". + repository may be its main checkout's), is keyed on the checkout, the lock files' identity, + the plugin listing and the harness the listing came from, and expires + so that a plugin installed mid-session is picked up by the next skill + rather than at the end of the day. A project whose personal layer + does not exist is not cached at all — creating it would create the + very state whose absence the skill scope reads as "never configured". """ local_dir = personal_dir(root) if local_dir is None or not local_dir.is_dir(): - return project_findings(root, installed), False + return project_findings(root, installed, harness), False stamp_now = now if now is not None else time.time() - key = _cache_key(root, installed) + key = _cache_key(root, installed, harness) cache_path = local_dir / CACHE_NAME if cache_path.exists(): try: @@ -510,7 +524,7 @@ def cached_project_findings( if isinstance(entries, list): return [Finding(**entry) for entry in entries], True - findings = project_findings(root, installed) + findings = project_findings(root, installed, harness) payload = {"key": key, "at": stamp_now, "findings": [asdict(f) for f in findings]} tmp = cache_path.with_suffix(".tmp") tmp.write_text(json.dumps(payload), encoding="utf-8") diff --git a/plugins/magpie-setup/skills/setup/setup_preflight/sections/step-3.md b/plugins/magpie-setup/skills/setup/setup_preflight/sections/step-3.md index a673d10b8..5d46fc83e 100644 --- a/plugins/magpie-setup/skills/setup/setup_preflight/sections/step-3.md +++ b/plugins/magpie-setup/skills/setup/setup_preflight/sections/step-3.md @@ -16,16 +16,28 @@ the release segment. The reconciliation check in step 4 is gated on the fingerprint, never on this version delta. **Why an unreadable result is *unknown* rather than *absent*.** Inside a -sandboxed session the plugin cache is read-denied and `claude plugin list ---json` returns `[]` there — that reads exactly like "nothing installed" +sandboxed session the plugin cache is read-denied and the running harness's +own listing returns `[]` there — that reads exactly like "nothing installed" but is not. Acting on it would propose installing a project's entire floor on every sandboxed run. -**The actions.** +**The actions.** The `below-floor` finding names the harness its listing +came from (`facts.harness`); the commands differ per client, and naming one +the operator cannot run in this session wastes their turn: -- a floor plugin absent → `claude plugin install @apache-magpie`; -- a floor plugin below `min_version` → `claude plugin update - @apache-magpie`. +| Harness | a floor plugin absent | a floor plugin below `min_version` | +|---|---|---| +| Claude Code | `claude plugin install @apache-magpie` | `claude plugin update @apache-magpie` | +| Codex | `codex plugin add @apache-magpie` | `codex plugin update ` | +| Gemini CLI | `gemini extensions install https://github.com/apache/magpie` | `gemini extensions update magpie` | + +Gemini installs the framework as one extension, so on it the floor is +carried by the `magpie` extension as a whole: update that one extension +rather than naming individual plugins. + +Copilot CLI installs from a repo URL or catalogue and has no plugin CLI to +read a listing or run an action with. There the floor stays unknown: run +nothing, and leave it to the user. Where there is no such CLI, run nothing and print the commands instead. diff --git a/tools/setup-preflight/README.md b/tools/setup-preflight/README.md index df0daeba3..5fdc23775 100644 --- a/tools/setup-preflight/README.md +++ b/tools/setup-preflight/README.md @@ -44,9 +44,10 @@ how to word it, and the prohibitions. Those live in each skill's - **Runtime** — Python 3.11+. Standard library only, deliberately: the module is copied into the user's personal configuration layer and run with bare `python3`, where nothing else is available. -- **CLIs** — none required. The harness CLI (`claude plugin list --json`) - is consulted when present and its absence is a supported state, not an - error. +- **CLIs** — none required. The running harness's own listing command + (detected from its environment markers, e.g. `claude plugin list + --json` under Claude Code) is consulted when present; its absence or + the absence of any harness marker is a supported state, not an error. - **Credentials / auth** — none. - **Network** — none. Every input is a file in the project. - **Optional** — `--plugin-list` accepts a listing the caller already @@ -147,11 +148,11 @@ is settled when the run finishes rather than before it starts. **Unknown is never absent.** A plugin listing that could not be read is `None`, not `{}`. Inside a sandboxed session the plugin cache is -read-denied and `claude plugin list --json` prints `[]`, which reads -exactly like "nothing installed"; acting on it would propose installing a -project's entire floor on every sandboxed run. An empty *parsed* listing -is treated as unknown for the same reason — it is indistinguishable from -the denied case. +read-denied and the running harness's own listing prints `[]`, which +reads exactly like "nothing installed"; acting on it would propose +installing a project's entire floor on every sandboxed run. An empty +*parsed* listing is treated as unknown for the same reason — it is +indistinguishable from the denied case. **A dev build is a version like any other.** Nothing strips or rounds a `.devN` segment. `0.2.0.dev202609110041` is below `0.2.0`, and `0.10.0` diff --git a/tools/setup-preflight/tests/test_cli.py b/tools/setup-preflight/tests/test_cli.py index 76c788cb1..2e8e92816 100644 --- a/tools/setup-preflight/tests/test_cli.py +++ b/tools/setup-preflight/tests/test_cli.py @@ -21,6 +21,7 @@ from __future__ import annotations import json +import subprocess from pathlib import Path import pytest @@ -108,7 +109,7 @@ def test_every_section_named_is_one_a_reader_can_find(project: Path, capsys) -> def test_unparsable_output_is_unknown(tmp_path: Path) -> None: bad = tmp_path / "bad.json" bad.write_text("not json at all") - assert read_installed(str(bad)) is None + assert read_installed(str(bad)) == (None, None) def test_an_empty_listing_is_unknown_not_an_empty_install(tmp_path: Path) -> None: @@ -117,24 +118,229 @@ def test_an_empty_listing_is_unknown_not_an_empty_install(tmp_path: Path) -> Non harness. Unknown is the safe reading: it proposes nothing.""" empty = tmp_path / "empty.json" empty.write_text("[]") - assert read_installed(str(empty)) is None + assert read_installed(str(empty)) == (None, None) def test_a_real_listing_parses_to_a_name_version_map(tmp_path: Path) -> None: listing = tmp_path / "list.json" listing.write_text('[{"name": "magpie-setup", "version": "0.2.0"}]') - assert read_installed(str(listing)) == {"magpie-setup": "0.2.0"} + assert read_installed(str(listing)) == (None, {"magpie-setup": "0.2.0"}) def test_a_listing_that_is_not_a_list_is_unknown(tmp_path: Path) -> None: listing = tmp_path / "list.json" listing.write_text('{"plugins": []}') - assert read_installed(str(listing)) is None + assert read_installed(str(listing)) == (None, None) -def test_a_missing_harness_cli_is_unknown(monkeypatch: pytest.MonkeyPatch) -> None: - def explode(*_args: object, **_kwargs: object) -> None: +# --- read_installed: the running harness answers with its own listing ---------------- + + +def _listing_ok(argv: list[str], body: str) -> subprocess.CompletedProcess: + completed = subprocess.CompletedProcess[str](argv, 0) + completed.stdout = body + return completed + + +def test_the_claude_marker_runs_only_claudes_listing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The harness running this session is detected from its own marker, + and only its listing command runs.""" + seen: list[list[str]] = [] + + def run(argv: list[str], **_kwargs: object) -> subprocess.CompletedProcess: + seen.append(list(argv)) + return _listing_ok(argv, '[{"name": "magpie-setup", "version": "0.2.0"}]') + + monkeypatch.setattr("setup_preflight.cli.subprocess.run", run) + assert read_installed(None, {"CLAUDECODE": "1"}) == ("claude", {"magpie-setup": "0.2.0"}) + assert seen == [["claude", "plugin", "list", "--json"]] + + +def test_the_codex_marker_runs_codex_listing_with_its_json_flag( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`codex plugin list` prints a terminal summary by default; only + `--json` makes it emit the array the shared parser reads.""" + seen: list[list[str]] = [] + + def run(argv: list[str], **_kwargs: object) -> subprocess.CompletedProcess: + seen.append(list(argv)) + return _listing_ok(argv, '[{"name": "magpie-setup", "version": "0.3.1"}]') + + monkeypatch.setattr("setup_preflight.cli.subprocess.run", run) + assert read_installed(None, {"CODEX_SANDBOX": "1"}) == ("codex", {"magpie-setup": "0.3.1"}) + assert seen == [["codex", "plugin", "list", "--json"]] + + +def test_the_gemini_marker_runs_gemini_listing_with_json_output( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`gemini extensions list` prints plain text by default; only + `-o json` makes it emit the array the shared parser reads.""" + seen: list[list[str]] = [] + + def run(argv: list[str], **_kwargs: object) -> subprocess.CompletedProcess: + seen.append(list(argv)) + return _listing_ok(argv, '[{"name": "magpie", "version": "0.2.0"}]') + + monkeypatch.setattr("setup_preflight.cli.subprocess.run", run) + assert read_installed(None, {"GEMINI_CLI": "1"}) == ("gemini", {"magpie": "0.2.0"}) + assert seen == [["gemini", "extensions", "list", "-o", "json"]] + + +def test_the_innermost_marker_wins_when_two_harnesses_inherit( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A harness started from inside another inherits the outer one's + variables; the innermost candidate answers, and `CLAUDECODE` — which + every child inherits — is checked last.""" + seen: list[list[str]] = [] + + def run(argv: list[str], **_kwargs: object) -> subprocess.CompletedProcess: + seen.append(list(argv)) + return _listing_ok(argv, '[{"name": "magpie", "version": "0.2.0"}]') + + monkeypatch.setattr("setup_preflight.cli.subprocess.run", run) + harness, _ = read_installed(None, {"CLAUDECODE": "1", "GEMINI_CLI": "1"}) + assert harness == "gemini" + assert seen == [["gemini", "extensions", "list", "-o", "json"]] + + +def test_no_marker_is_unknown_and_no_listing_runs( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A session under no detected harness stays unknown: nothing is + probed in a fixed order, so no harness can answer for another.""" + + def explode(*_args: object, **_kwargs: object) -> subprocess.CompletedProcess: + raise AssertionError("no listing may run without a detected harness") + + monkeypatch.setattr("setup_preflight.cli.subprocess.run", explode) + assert read_installed(None, {}) == (None, None) + + +def test_the_copilot_marker_is_unknown_without_a_listing_cli( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Copilot installs from a repo URL and has no listing CLI: the floor + stays unknown rather than guessing what a listing would print.""" + + def explode(*_args: object, **_kwargs: object) -> subprocess.CompletedProcess: + raise AssertionError("copilot has no listing command to run") + + monkeypatch.setattr("setup_preflight.cli.subprocess.run", explode) + assert read_installed(None, {"COPILOT_CLI": "1"}) == (None, None) + + +def test_a_timeout_of_the_running_harness_is_unknown( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A `claude` that exists and times out must not borrow another + harness's listing: unknown, and no other harness is ever probed.""" + + def explode(*_args: object, **_kwargs: object) -> subprocess.CompletedProcess: + raise subprocess.TimeoutExpired(cmd=["claude"], timeout=20) + + monkeypatch.setattr("setup_preflight.cli.subprocess.run", explode) + assert read_installed(None, {"CLAUDECODE": "1"}) == (None, None) + + +def test_a_refused_binary_of_the_running_harness_is_unknown( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A sandbox that refuses to execute the harness's own binary reads as + unknown, not as a missing binary to probe around.""" + + def explode(*_args: object, **_kwargs: object) -> subprocess.CompletedProcess: + raise PermissionError("sandbox denies execution") + + monkeypatch.setattr("setup_preflight.cli.subprocess.run", explode) + assert read_installed(None, {"CLAUDECODE": "1"}) == (None, None) + + +def test_a_missing_binary_of_the_running_harness_is_unknown( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The marker says Claude Code but `claude` is not on PATH: unknown — + the checker never asks a different harness to answer for it.""" + + def explode(*_args: object, **_kwargs: object) -> subprocess.CompletedProcess: raise FileNotFoundError("claude") monkeypatch.setattr("setup_preflight.cli.subprocess.run", explode) - assert read_installed(None) is None + assert read_installed(None, {"CLAUDECODE": "1"}) == (None, None) + + +def test_a_failed_listing_of_the_running_harness_is_unknown( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A detected harness whose listing exits non-zero stays unknown.""" + + def run(argv: list[str], **_kwargs: object) -> subprocess.CompletedProcess: + completed = subprocess.CompletedProcess[str](argv, 1) + completed.stdout = "" + return completed + + monkeypatch.setattr("setup_preflight.cli.subprocess.run", run) + assert read_installed(None, {"CLAUDECODE": "1"}) == (None, None) + + +def test_an_unparsable_listing_of_the_running_harness_is_unknown( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A detected harness whose listing is not the shared JSON shape reads + as unknown — the checker proposes nothing it cannot read.""" + + def run(argv: list[str], **_kwargs: object) -> subprocess.CompletedProcess: + return _listing_ok(argv, "magpie-setup 0.3.1 (marketplace)") + + monkeypatch.setattr("setup_preflight.cli.subprocess.run", run) + assert read_installed(None, {"CLAUDECODE": "1"}) == (None, None) + + +def test_the_finding_names_the_running_harness( + project: Path, + capsys: pytest.CaptureFixture[str], + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The `below-floor` finding carries the harness its listing came + from, so the rules can name that harness's own commands.""" + write_lock(project, MARKETPLACE) + monkeypatch.setenv("CLAUDECODE", "1") + monkeypatch.setattr( + "setup_preflight.cli.subprocess.run", + lambda argv, **_kwargs: _listing_ok(argv, '[{"name": "magpie-setup", "version": "0.1.0"}]'), + ) + code, payload = run( + capsys, + "--skill", + "magpie-x", + "--project-root", + str(project), + "--verify-interval-days", + "0", + ) + assert code == 0 + finding = next(f for f in payload["findings"] if f["code"] == "below-floor") + assert finding["facts"]["harness"] == "claude" + + +def test_a_supplied_listing_reports_no_harness(project: Path, capsys) -> None: + """`--plugin-list` is what the caller already read; the map stands + alone and the finding names no harness.""" + write_lock(project, MARKETPLACE) + listing = project / "plugins.json" + listing.write_text('[{"name": "other", "version": "1.0.0"}]') + _, payload = run( + capsys, + "--skill", + "magpie-x", + "--project-root", + str(project), + "--plugin-list", + str(listing), + ) + finding = next(f for f in payload["findings"] if f["code"] == "below-floor") + assert finding["facts"]["harness"] is None diff --git a/tools/setup-preflight/tests/test_core.py b/tools/setup-preflight/tests/test_core.py index a47807f93..d08341b46 100644 --- a/tools/setup-preflight/tests/test_core.py +++ b/tools/setup-preflight/tests/test_core.py @@ -88,6 +88,17 @@ def test_a_genuinely_missing_plugin_is_a_finding(project: Path) -> None: found = project_findings(project, {"something-else": "9.9.9"}) assert codes(found) == ["below-floor"] assert found[0].facts["missing"] == ["magpie-setup"] + # The listing came from `--plugin-list` here, so no harness is named. + assert found[0].facts["harness"] is None + + +def test_the_floor_finding_names_the_harness_it_measured(project: Path) -> None: + """The finding carries the harness the listing came from, so the rules + can name that harness's own install and update commands.""" + write_lock(project, MARKETPLACE) + found = project_findings(project, {"something-else": "9.9.9"}, "gemini") + assert codes(found) == ["below-floor"] + assert found[0].facts["harness"] == "gemini" def test_being_ahead_of_the_floor_is_never_a_finding(project: Path) -> None: @@ -338,6 +349,19 @@ def test_a_changed_plugin_listing_invalidates_the_cache(project: Path) -> None: assert cached is False and found == [] +def test_a_changed_harness_invalidates_the_cache(project: Path) -> None: + """The below-floor finding names the harness its listing came from, so + a verdict computed under one harness must not be served to another.""" + write_lock(project, MARKETPLACE) + (project / ".apache-magpie-local").mkdir() + first, cached = cached_project_findings(project, {"magpie-setup": "0.1.0"}, "claude") + assert cached is False and codes(first) == ["below-floor"] + assert first[0].facts["harness"] == "claude" + second, cached = cached_project_findings(project, {"magpie-setup": "0.1.0"}, "codex") + assert cached is False and codes(second) == ["below-floor"] + assert second[0].facts["harness"] == "codex" + + def test_a_refreshed_checker_invalidates_the_cache(project: Path) -> None: """`upgrade` replaces the checker's copy; a verdict the old code computed must not outlive it.""" diff --git a/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-5-no-claude-cli/expected.json b/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-5-no-claude-cli/expected.json index 56cd059af..86b5931c6 100644 --- a/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-5-no-claude-cli/expected.json +++ b/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-5-no-claude-cli/expected.json @@ -1 +1 @@ -{"action": "print-command", "commands": [], "blocks": true} +{"action": "update", "commands": ["gemini extensions update magpie"], "blocks": true, "mention_restart": true} diff --git a/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-5-no-claude-cli/report.md b/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-5-no-claude-cli/report.md index aee4b3389..038747fd8 100644 --- a/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-5-no-claude-cli/report.md +++ b/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-5-no-claude-cli/report.md @@ -7,5 +7,8 @@ magpie-agent-guard. `.apache-magpie-overrides/` exists and contains `project.md`. -The running agent is Google Gemini CLI. There is no `claude` executable on -PATH. The installed Magpie extension reports version 0.2.0. +The running agent is Google Gemini CLI (`GEMINI_CLI=1`). There is no +`claude` executable on PATH and no Codex session markers in the +environment. The installed Magpie extension reports version 0.2.0: +`gemini extensions list -o json` prints +`[{"name": "magpie", "version": "0.2.0"}]`. diff --git a/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-9-below-floor-codex/case-meta.json b/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-9-below-floor-codex/case-meta.json new file mode 100644 index 000000000..03128a3ee --- /dev/null +++ b/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-9-below-floor-codex/case-meta.json @@ -0,0 +1 @@ +{"tags":["local-smoke","smoke"]} diff --git a/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-9-below-floor-codex/expected.json b/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-9-below-floor-codex/expected.json new file mode 100644 index 000000000..d2a0200ed --- /dev/null +++ b/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-9-below-floor-codex/expected.json @@ -0,0 +1 @@ +{"action": "update", "commands": ["codex plugin update magpie-setup", "codex plugin update magpie-agent-guard"], "blocks": true, "mention_restart": true} diff --git a/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-9-below-floor-codex/report.md b/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-9-below-floor-codex/report.md new file mode 100644 index 000000000..ec6c04446 --- /dev/null +++ b/tools/skill-evals/evals/setup/preflight-floor/fixtures/case-9-below-floor-codex/report.md @@ -0,0 +1,13 @@ + + +`.apache-magpie.lock` contains `method: marketplace`, `url: apache/magpie`, +`min_version: 0.3.0`, and a `plugins` list of magpie-setup and +magpie-agent-guard. + +`.apache-magpie-overrides/` exists and contains `project.md`. + +The running agent is OpenAI Codex CLI (`CODEX_SANDBOX=1`). There is no +`claude` executable on PATH and no Gemini session markers in the +environment. `codex plugin list --json` reports magpie-setup 0.2.0 and +magpie-agent-guard 0.2.0, both from the `apache-magpie` marketplace.