From 8ddae751827511b342c2f77ac45fc7f096a95ca7 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Wed, 21 Oct 2026 20:01:37 +0530 Subject: [PATCH 1/2] fix(agent-guard): discover contributed skill guards under plugins/*/skills/*/guards (#1756) --- tools/agent-guard/src/agent_guard/__init__.py | 19 ++++--- tools/agent-guard/tests/test_discovery.py | 49 ++++++++++++++++--- tools/agent-guard/tests/test_skill_guards.py | 24 ++++++--- 3 files changed, 72 insertions(+), 20 deletions(-) diff --git a/tools/agent-guard/src/agent_guard/__init__.py b/tools/agent-guard/src/agent_guard/__init__.py index 9a7419f6b..bb94debac 100644 --- a/tools/agent-guard/src/agent_guard/__init__.py +++ b/tools/agent-guard/src/agent_guard/__init__.py @@ -1610,35 +1610,40 @@ def framework_root() -> Path | None: """The framework tree this engine is running out of, or ``None``. Recognised by the two directories the engine always sits beside: a - ``skills/`` tree and the ``tools/agent-guard`` package holding this file. + ``skills/`` or ``plugins/`` tree and the ``tools/agent-guard`` package holding this file. That holds whether the hook runs from an installed plugin, a framework checkout, or an adopter's snapshot — and does not hold for a standalone copy of this one file, which is why the result is optional rather than a path computed by counting ``parents``. """ for parent in Path(__file__).resolve().parents: - if (parent / "skills").is_dir() and (parent / "tools" / "agent-guard").is_dir(): + if ((parent / "skills").is_dir() or (parent / "plugins").is_dir()) and ( + parent / "tools" / "agent-guard" + ).is_dir(): return parent return None def guard_dirs() -> list[Path]: """Directories scanned for contributed guards: ``$MAGPIE_GUARD_DIRS`` entries, - then every ``skills/*/guards`` in the framework tree this file runs from, then - the ``guards.d`` sibling of this file.""" + then every ``plugins/*/skills/*/guards`` and ``skills/*/guards`` in the framework + tree this file runs from, then the ``guards.d`` sibling of this file.""" dirs: list[Path] = [] env = os.environ.get(GUARD_DIRS_ENV) if env: dirs.extend(Path(p) for p in env.split(os.pathsep) if p) if (root := framework_root()) is not None: + dirs.extend(sorted((root / "plugins").glob("*/skills/*/guards"))) dirs.extend(sorted((root / "skills").glob("*/guards"))) dirs.append(Path(__file__).resolve().parent / "guards.d") seen: set[Path] = set() out: list[Path] = [] for d in dirs: - if d not in seen and d.is_dir(): - seen.add(d) - out.append(d) + if d.is_dir(): + real = d.resolve() + if real not in seen: + seen.add(real) + out.append(d) return out diff --git a/tools/agent-guard/tests/test_discovery.py b/tools/agent-guard/tests/test_discovery.py index 46f39c147..30dd7c6ef 100644 --- a/tools/agent-guard/tests/test_discovery.py +++ b/tools/agent-guard/tests/test_discovery.py @@ -67,11 +67,14 @@ def test_skill_guard_dirs_are_discovered_without_configuration(): """Every ``skills/*/guards`` in the framework tree is scanned as-is — this is what removes the "collect skill guards into a local guards.d" install step.""" dirs = agent_guard.guard_dirs() - on_disk = sorted((REPO_ROOT / "skills").glob("*/guards")) + resolved_dirs = {d.resolve() for d in dirs} + on_disk = sorted((REPO_ROOT / "plugins").glob("*/skills/*/guards")) + sorted( + (REPO_ROOT / "skills").glob("*/guards") + ) assert on_disk, "expected at least one skill-owned guards dir in the repo" for d in on_disk: - assert d in dirs - assert dirs[-1] == Path(agent_guard.__file__).resolve().parent / "guards.d" + assert d.resolve() in resolved_dirs + assert dirs[-1].resolve() == (Path(agent_guard.__file__).resolve().parent / "guards.d").resolve() def test_env_dirs_are_searched_before_discovered_ones(monkeypatch, tmp_path): @@ -100,7 +103,10 @@ def test_discovery_survives_being_reached_through_a_symlinked_plugin_root(tmp_pa framework tree until it is resolved.""" link_parent = tmp_path / "plugin" / "tools" link_parent.mkdir(parents=True) - (link_parent / "agent-guard").symlink_to(REPO_ROOT / "tools" / "agent-guard") + try: + (link_parent / "agent-guard").symlink_to(REPO_ROOT / "tools" / "agent-guard") + except OSError as exc: + pytest.skip(f"symlinks not supported on this platform: {exc}") entry = link_parent / "agent-guard" / "src" / "agent_guard" / "__init__.py" spec = importlib.util.spec_from_file_location("_linked_agent_guard", entry) @@ -109,12 +115,43 @@ def test_discovery_survives_being_reached_through_a_symlinked_plugin_root(tmp_pa spec.loader.exec_module(module) assert module.framework_root() == REPO_ROOT - assert sorted((REPO_ROOT / "skills").glob("*/guards"))[0] in module.guard_dirs() + expected = ( + sorted((REPO_ROOT / "plugins").glob("*/skills/*/guards")) + + sorted((REPO_ROOT / "skills").glob("*/guards")) + )[0] + assert expected in module.guard_dirs() def test_guard_dirs_are_deduplicated(monkeypatch): """A skill guards dir named in the environment as well as discovered is scanned once — otherwise its guards would run twice per command.""" - skill_guards = sorted((REPO_ROOT / "skills").glob("*/guards"))[0] + skill_guards = ( + sorted((REPO_ROOT / "plugins").glob("*/skills/*/guards")) + + sorted((REPO_ROOT / "skills").glob("*/guards")) + )[0] monkeypatch.setenv(agent_guard.GUARD_DIRS_ENV, f"{skill_guards}{os.pathsep}{skill_guards}") assert agent_guard.guard_dirs().count(skill_guards) == 1 + + +def test_plugin_skill_guards_are_discovered(): + """Contributed guards under plugins/*/skills/*/guards are discovered.""" + dirs = agent_guard.guard_dirs() + plugin_guards = sorted((REPO_ROOT / "plugins").glob("*/skills/*/guards")) + assert plugin_guards, "expected plugin skill guards to exist" + for d in plugin_guards: + assert d in dirs + + +def test_guard_dirs_deduplicates_symlinks(tmp_path, monkeypatch): + """A directory reached through a symlink and its canonical path are deduplicated.""" + real_dir = tmp_path / "real_guards" + real_dir.mkdir() + sym_dir = tmp_path / "sym_guards" + try: + sym_dir.symlink_to(real_dir, target_is_directory=True) + except OSError as exc: + pytest.skip(f"symlinks not supported on this platform: {exc}") + + monkeypatch.setenv(agent_guard.GUARD_DIRS_ENV, f"{real_dir}{os.pathsep}{sym_dir}") + resolved_dirs = [d.resolve() for d in agent_guard.guard_dirs()] + assert resolved_dirs.count(real_dir.resolve()) == 1 diff --git a/tools/agent-guard/tests/test_skill_guards.py b/tools/agent-guard/tests/test_skill_guards.py index e4c2de2de..99cdb3193 100644 --- a/tools/agent-guard/tests/test_skill_guards.py +++ b/tools/agent-guard/tests/test_skill_guards.py @@ -17,7 +17,7 @@ """Tests for the skill-owned guards, exercised end-to-end through the agent-guard discovery path — exactly how an adopter runs them. ``MAGPIE_GUARD_DIRS`` points -the dispatcher at the real ``skills//guards`` directories in this repo, so +the dispatcher at the canonical plugin or skill guard directories in this repo, so these tests fail if a guard file is moved, renamed, or broken.""" import os @@ -28,8 +28,16 @@ import agent_guard REPO_ROOT = Path(__file__).resolve().parents[3] -TRIAGE_GUARDS = REPO_ROOT / "skills" / "pr-management-triage" / "guards" -SECURITY_GUARDS = REPO_ROOT / "skills" / "security-issue-fix" / "guards" +TRIAGE_GUARDS = ( + REPO_ROOT / "plugins" / "magpie-pr-management" / "skills" / "pr-triage" / "guards" + if (REPO_ROOT / "plugins" / "magpie-pr-management" / "skills" / "pr-triage" / "guards").is_dir() + else REPO_ROOT / "skills" / "pr-management-triage" / "guards" +) +SECURITY_GUARDS = ( + REPO_ROOT / "plugins" / "magpie-security" / "skills" / "issue-fix" / "guards" + if (REPO_ROOT / "plugins" / "magpie-security" / "skills" / "issue-fix" / "guards").is_dir() + else REPO_ROOT / "skills" / "security-issue-fix" / "guards" +) @pytest.fixture(autouse=True) @@ -163,7 +171,7 @@ def test_graphql_review_payload_is_scanned(tmp_path): ), encoding="utf-8", ) - reason = dispatch(f"gh api graphql --input {payload}") + reason = dispatch(f"gh api graphql --input {payload.as_posix()}") assert reason and "dave" in reason and "carol" not in reason @@ -175,7 +183,7 @@ def test_any_graphql_write_is_scanned_not_just_reviews(tmp_path): payload.write_text( json.dumps({"query": "mutation { somethingNew(input: {}) { ok } }", "variables": {"note": "@x"}}) ) - assert dispatch(f"gh api graphql --input {payload}") + assert dispatch(f"gh api graphql --input {payload.as_posix()}") def _git(repo, *args): @@ -220,7 +228,9 @@ def _allow(*handles): def _handoff(tmp_path, body, repo_flag="--repo acme/product"): f = tmp_path / "b.md" f.write_text(body, encoding="utf-8") - return agent_guard.dispatch(f"gh issue comment 7 {repo_flag} --body-file {f}", cwd=str(tmp_path)) + return agent_guard.dispatch( + f"gh issue comment 7 {repo_flag} --body-file {f.as_posix()}", cwd=str(tmp_path) + ) def test_mentoring_handoff_may_mention_the_configured_team(monkeypatch, tmp_path): @@ -394,7 +404,7 @@ def test_review_body_on_stdin_is_refused(): def test_rest_comment_through_gh_api_is_scanned(tmp_path): body = tmp_path / "c.md" body.write_text("ping @bob", encoding="utf-8") - reason = dispatch(f"gh api repos/acme/product/issues/5/comments -F body=@{body}") + reason = dispatch(f"gh api repos/acme/product/issues/5/comments -F body=@{body.as_posix()}") assert reason and "bob" in reason From 88d986ea5feee5a04fa57383242a05fe1e5d45b9 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Wed, 21 Oct 2026 20:10:24 +0530 Subject: [PATCH 2/2] chore: sync plugin agent-guard copy and update isolated fingerprint --- .../agent-guard/src/agent_guard/__init__.py | 19 ++++++++++++------- .../setup_preflight/isolated_fingerprint.py | 2 +- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py b/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py index 9a7419f6b..bb94debac 100644 --- a/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py +++ b/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py @@ -1610,35 +1610,40 @@ def framework_root() -> Path | None: """The framework tree this engine is running out of, or ``None``. Recognised by the two directories the engine always sits beside: a - ``skills/`` tree and the ``tools/agent-guard`` package holding this file. + ``skills/`` or ``plugins/`` tree and the ``tools/agent-guard`` package holding this file. That holds whether the hook runs from an installed plugin, a framework checkout, or an adopter's snapshot — and does not hold for a standalone copy of this one file, which is why the result is optional rather than a path computed by counting ``parents``. """ for parent in Path(__file__).resolve().parents: - if (parent / "skills").is_dir() and (parent / "tools" / "agent-guard").is_dir(): + if ((parent / "skills").is_dir() or (parent / "plugins").is_dir()) and ( + parent / "tools" / "agent-guard" + ).is_dir(): return parent return None def guard_dirs() -> list[Path]: """Directories scanned for contributed guards: ``$MAGPIE_GUARD_DIRS`` entries, - then every ``skills/*/guards`` in the framework tree this file runs from, then - the ``guards.d`` sibling of this file.""" + then every ``plugins/*/skills/*/guards`` and ``skills/*/guards`` in the framework + tree this file runs from, then the ``guards.d`` sibling of this file.""" dirs: list[Path] = [] env = os.environ.get(GUARD_DIRS_ENV) if env: dirs.extend(Path(p) for p in env.split(os.pathsep) if p) if (root := framework_root()) is not None: + dirs.extend(sorted((root / "plugins").glob("*/skills/*/guards"))) dirs.extend(sorted((root / "skills").glob("*/guards"))) dirs.append(Path(__file__).resolve().parent / "guards.d") seen: set[Path] = set() out: list[Path] = [] for d in dirs: - if d not in seen and d.is_dir(): - seen.add(d) - out.append(d) + if d.is_dir(): + real = d.resolve() + if real not in seen: + seen.add(real) + out.append(d) return out diff --git a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py index 587fd7153..32190d8c6 100644 --- a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py +++ b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py @@ -21,4 +21,4 @@ for installs that do not carry the framework source (see `isolated.py`). """ -FRAMEWORK_FINGERPRINT = "sha256:a4b5967225871a91" +FRAMEWORK_FINGERPRINT = "sha256:56c99611e46a3af6"