From 94c1e3847687e8c3126b524672d63d134f87fdcb Mon Sep 17 00:00:00 2001 From: Arnav Date: Tue, 1 Sep 2026 21:13:33 +0530 Subject: [PATCH] fix(symlink-lint): enforce relay completeness and sync missing self-adoption symlinks The self-adoption relay directories drifted out of sync with skills/: - dependency-license-audit was missing from .agents/skills/ (canonical), .claude/skills/, .github/skills/, and .kiro/skills/ (relays). - report-framework-issue was missing from .kiro/skills/. Add Rule 3 (completeness) to symlink-lint to verify that every skill under skills/ has a canonical entry in .agents/skills/ and that every wired agent directory relays the full canonical set (scoped to framework checkouts). Also wire the missing symlinks, update the pre-commit hook filter, and add corresponding test coverage. Fixes #1137 Generated-by: Antigravity (Gemini 3.7 Flash) --- .../skills/magpie-dependency-license-audit | 1 + .../skills/magpie-dependency-license-audit | 1 + .../skills/magpie-dependency-license-audit | 1 + .kiro/skills/magpie-dependency-license-audit | 1 + .kiro/skills/magpie-report-framework-issue | 1 + .pre-commit-config.yaml | 10 +- docs/adapters/add-a-harness.md | 2 +- docs/labels-and-capabilities.md | 2 +- .../specs/meta-and-quality-tooling.md | 4 +- tools/symlink-lint/README.md | 22 +++-- .../symlink-lint/src/symlink_lint/__init__.py | 74 ++++++++++++++- tools/symlink-lint/tests/test_symlink_lint.py | 94 ++++++++++++++++++- 12 files changed, 192 insertions(+), 21 deletions(-) create mode 120000 .agents/skills/magpie-dependency-license-audit create mode 120000 .claude/skills/magpie-dependency-license-audit create mode 120000 .github/skills/magpie-dependency-license-audit create mode 120000 .kiro/skills/magpie-dependency-license-audit create mode 120000 .kiro/skills/magpie-report-framework-issue diff --git a/.agents/skills/magpie-dependency-license-audit b/.agents/skills/magpie-dependency-license-audit new file mode 120000 index 000000000..26c73177e --- /dev/null +++ b/.agents/skills/magpie-dependency-license-audit @@ -0,0 +1 @@ +../../skills/dependency-license-audit \ No newline at end of file diff --git a/.claude/skills/magpie-dependency-license-audit b/.claude/skills/magpie-dependency-license-audit new file mode 120000 index 000000000..7104fd031 --- /dev/null +++ b/.claude/skills/magpie-dependency-license-audit @@ -0,0 +1 @@ +../../.agents/skills/magpie-dependency-license-audit \ No newline at end of file diff --git a/.github/skills/magpie-dependency-license-audit b/.github/skills/magpie-dependency-license-audit new file mode 120000 index 000000000..7104fd031 --- /dev/null +++ b/.github/skills/magpie-dependency-license-audit @@ -0,0 +1 @@ +../../.agents/skills/magpie-dependency-license-audit \ No newline at end of file diff --git a/.kiro/skills/magpie-dependency-license-audit b/.kiro/skills/magpie-dependency-license-audit new file mode 120000 index 000000000..7104fd031 --- /dev/null +++ b/.kiro/skills/magpie-dependency-license-audit @@ -0,0 +1 @@ +../../.agents/skills/magpie-dependency-license-audit \ No newline at end of file diff --git a/.kiro/skills/magpie-report-framework-issue b/.kiro/skills/magpie-report-framework-issue new file mode 120000 index 000000000..447049552 --- /dev/null +++ b/.kiro/skills/magpie-report-framework-issue @@ -0,0 +1 @@ +../../.agents/skills/magpie-report-framework-issue \ No newline at end of file diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d0db23595..499737029 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -221,9 +221,10 @@ repos: entry: python3 ai-tutors/inject-knowledge-base.py --check files: ^(ai-tutors/|docs/education/) pass_filenames: false - # Self-adoption symlink hygiene: reject cyclic symlinks and misdirected - # skill relays. `types: [symlink]` fires it on any staged symlink (always - # on `prek run --all-files`). Rules, rationale, and the pytest suite live + # Self-adoption symlink hygiene: reject cyclic symlinks, misdirected + # skill relays, and incomplete self-adoption symlink sets. Fires on + # changes under `skills/` or any agent-skills tree (always on + # `prek run --all-files`). Rules, rationale, and the pytest suite live # in tools/symlink-lint (README + tests). - repo: local hooks: @@ -231,7 +232,8 @@ repos: name: symlink-lint language: system entry: python3 tools/symlink-lint/src/symlink_lint/__init__.py - types: [symlink] + files: ^(\.(agents|claude|github|kiro)/skills/.*|skills/.*)$ + types_or: [symlink, file] pass_filenames: false # Release-archive guard: build the source archive exactly as the release # does (git archive of the export tree, honouring .gitattributes diff --git a/docs/adapters/add-a-harness.md b/docs/adapters/add-a-harness.md index 2abf0d700..2c8cc20cb 100644 --- a/docs/adapters/add-a-harness.md +++ b/docs/adapters/add-a-harness.md @@ -73,7 +73,7 @@ relay symlinks automatically. The rules are: `../../.agents/skills/magpie-` — **through the canonical, not straight at source**. -`symlink-lint` enforces both rules and is wired into `prek`; run it to +`symlink-lint` enforces the working-tree rules and is wired into `prek`; run it to verify after any manual link creation: ```bash diff --git a/docs/labels-and-capabilities.md b/docs/labels-and-capabilities.md index e0be266bf..e4a41b0eb 100644 --- a/docs/labels-and-capabilities.md +++ b/docs/labels-and-capabilities.md @@ -302,7 +302,7 @@ it implements multiple contracts (e.g. `tools/gmail` provides both | [`tools/spec-status-index`](../tools/spec-status-index/) | `substrate:framework-dev` + `substrate:analytics` | Index of spec / RFC implementation status — framework-dev substrate that also doubles as a governance/stats view (`analytics`) | | [`tools/vendor-neutrality-score`](../tools/vendor-neutrality-score/) | `substrate:framework-dev` + `substrate:analytics` | Deterministic vendor-neutrality score — reads each contract tool's `**Kind:**` / `**Vendor:**` metadata and scores per-contract + per-skill neutrality (`analytics`); backs the score block in [`docs/vendor-neutrality.md`](vendor-neutrality.md) | | [`tools/spec-validator`](../tools/spec-validator/) | `substrate:framework-dev` | Spec-frontmatter and body-section validator — counterpart to `skill-and-tool-validator` for `tools/spec-loop/specs/` | -| [`tools/symlink-lint`](../tools/symlink-lint/) | `substrate:framework-dev` | Self-adoption symlink hygiene — rejects cyclic symlinks and misdirected skill relays (canonical/relay target-correctness) | +| [`tools/symlink-lint`](../tools/symlink-lint/) | `substrate:framework-dev` | Self-adoption symlink hygiene — rejects cyclic symlinks, misdirected skill relays (canonical/relay target-correctness), and incomplete self-adoption symlink sets | | [`tools/pilot-report-validator`](../tools/pilot-report-validator/) | `substrate:framework-dev` | Adopter pilot-report validator — required frontmatter keys, no unfilled placeholders, valid profile, and required body sections; counterpart to `spec-validator` for `docs/pilot-report-template.md` | | [`tools/skill-reconciler-diff`](../tools/skill-reconciler-diff/) | `substrate:framework-dev` | Deterministic structural diff between two skill trees — parses frontmatter, section headings, step inventory, placeholders, support files, and safety-baseline clauses into a JSON diff object for the `skill-reconciler` skill | | [`tools/vcs`](../tools/vcs/) | `contract:source-control` | Backend-dispatching implementation of the source-control (VCS) capability ([`tools/github/source-control.md`](../tools/github/source-control.md)); complete Git and Mercurial (Hg) backends, plus detected extension point for SVN (#602) | diff --git a/tools/spec-loop/specs/meta-and-quality-tooling.md b/tools/spec-loop/specs/meta-and-quality-tooling.md index cdbdf07e1..54cd8fd82 100644 --- a/tools/spec-loop/specs/meta-and-quality-tooling.md +++ b/tools/spec-loop/specs/meta-and-quality-tooling.md @@ -40,8 +40,8 @@ trustworthy as it grows. - `tools/skill-evals/` — harness for measuring skill behaviour. - `tools/sandbox-lint/` — lints the sandbox/permissions configuration. - `tools/symlink-lint/` — lints the framework's self-adoption skill - symlinks: rejects cyclic symlinks and misdirected relays (canonical/ - relay target-correctness). + symlinks: rejects cyclic symlinks, misdirected relays (canonical/ + relay target-correctness), and incomplete self-adoption symlink sets. - `tools/dashboard-generator/` — read-only HTML dashboards over campaign artefacts. - `tools/probe-templates/` — reusable probes. diff --git a/tools/symlink-lint/README.md b/tools/symlink-lint/README.md index bc07fe05c..0ae8c45d1 100644 --- a/tools/symlink-lint/README.md +++ b/tools/symlink-lint/README.md @@ -23,7 +23,7 @@ **Harness:** agnostic Lints the framework's **self-adoption skill symlinks** — the canonical -`.agents/skills/` links and their relays — against two invariants. This is +`.agents/skills/` links and their relays — against three invariants. This is the single place the rationale lives; the module, the prek hook, and the error output all point back here. @@ -47,13 +47,19 @@ every other agent dir (`.claude/`, `.github/`, `.windsurf/`, `.goose/`, must point at `../../.agents/skills/magpie-` (through the canonical, not straight at source). Catches relays that bypass `.agents/` — acyclic, so rule 1 alone would miss them. +3. **Completeness.** In the framework checkout (where `skills/` lives), every + skill directory under `skills/` must have its canonical + `.agents/skills/magpie-` entry, and every wired agent directory + must relay the full canonical set. Catches new skills that landed + without their self-adoption symlinks. Adopter checkouts (where + `skills/setup/SKILL.md` is absent) are exempt. **Dangling / unresolvable links are skipped** by rules 1–2, never flagged: an adopter's canonical links legitimately dangle until the gitignored `.apache-magpie/` snapshot is installed. Broken-target detection is `setup verify`'s job. -3. **Release archive is extractor-safe** (run with `--archive`). Builds the +4. **Release archive is extractor-safe** (run with `--archive`). Builds the source archive exactly as the release does — `git archive --worktree-attributes` of the *staged* tree (`git write-tree`), honouring `.gitattributes` `export-ignore` — then rejects any symlink in it that a @@ -69,7 +75,7 @@ an adopter's canonical links legitimately dangle until the gitignored - **dangling** — the link's target is absent from the archive (its real file got `export-ignore`d), orphaning the link. - Unlike rules 1–2 this inspects the archive, not the working tree, so a + Unlike rules 1–3 this inspects the archive, not the working tree, so a dangling link here *is* a defect — it means the RC would ship a broken link. Runs as the `symlink-lint-archive` hook (below); `release-verify-rc` re-checks the same property against the unpacked tarball before the @@ -85,8 +91,8 @@ an adopter's canonical links legitimately dangle until the gitignored ## How to use ```bash -python3 tools/symlink-lint/src/symlink_lint/__init__.py # rules 1–2 (working tree) -python3 tools/symlink-lint/src/symlink_lint/__init__.py --archive # rule 3 (release archive) +python3 tools/symlink-lint/src/symlink_lint/__init__.py # rules 1–3 (working tree) +python3 tools/symlink-lint/src/symlink_lint/__init__.py --archive # rule 4 (release archive) # or, once the workspace is synced: uv run --project tools/symlink-lint symlink-lint ``` @@ -97,9 +103,9 @@ Exit `0` if clean; `1` otherwise, with each offender printed to stderr. Two [prek](https://github.com/j178/prek) hooks (`.pre-commit-config.yaml`): -- `symlink-lint` — rules 1–2, fired on any staged symlink and always on - `prek run --all-files` (CI). -- `symlink-lint-archive` — rule 3 (`--archive`), fired whenever +- `symlink-lint` — rules 1–3, fired on changes under `skills/` or any + agent-skills tree, and always on `prek run --all-files` (CI). +- `symlink-lint-archive` — rule 4 (`--archive`), fired whenever `.gitattributes`, any agent-view `skills/` tree, `skills/`, or `projects/_template/` changes (the inputs that shape the release archive), and always on `prek run --all-files`. diff --git a/tools/symlink-lint/src/symlink_lint/__init__.py b/tools/symlink-lint/src/symlink_lint/__init__.py index 3939595ed..6aea309ca 100644 --- a/tools/symlink-lint/src/symlink_lint/__init__.py +++ b/tools/symlink-lint/src/symlink_lint/__init__.py @@ -15,17 +15,21 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -"""Lint the framework's self-adoption skill symlinks. Two working-tree rules: +"""Lint the framework's self-adoption skill symlinks. Three working-tree rules: 1. **No cycles** — a symlink must not resolve to its own directory or an ancestor (that traps recursive `**/SKILL.md` scanners in looped paths). 2. **Relay correctness** — a `magpie-` link under `.agents/skills/` (canonical) points into `../../skills/`; the same link under any other agent dir relays through `../../.agents/skills/magpie-`. +3. **Completeness** — in the framework checkout (where `skills/` lives), every + skill directory under `skills/` must have its canonical + `.agents/skills/magpie-` entry, and every wired agent directory + must relay the full canonical set. Plus one **release-archive** rule, run with `--archive`: -3. **Archive is extractor-safe** — build the source archive exactly as the +4. **Archive is extractor-safe** — build the source archive exactly as the release does (`git archive --worktree-attributes` of the staged tree, honouring `.gitattributes` `export-ignore`), then reject any symlink in it that is *dangling* (target absent — orphaned by an `export-ignore`) @@ -136,6 +140,65 @@ def find_misdirected_relays( return sorted(problems) +def find_missing_relays(root: Path, prune: frozenset[str] = PRUNE_DIR_NAMES) -> list[tuple[Path, str, str]]: + """Return `(link, expected_target, kind)` for every missing canonical or + relay symlink. + + Scoped to the framework checkout: if ``skills/setup/SKILL.md`` does not + exist under ``root``, returns an empty list (adopters keep their skills in + the snapshot at ``.apache-magpie/skills/`` and are exempt). + + When in the framework checkout: + 1. Every skill directory under ``skills/`` (containing a ``SKILL.md``) + must have its canonical symlink under ``.agents/skills/magpie-``. + 2. Every wired agent directory (any direct subdirectory of ``root`` + whose name starts with ``.`` carrying a ``skills/`` folder, such as + ``.claude/``, ``.github/``, ``.kiro/``) must relay the full canonical + set. + """ + skills_dir = root / "skills" + if not (skills_dir / "setup" / "SKILL.md").is_file(): + return [] + + skills: list[str] = sorted( + d.name + for d in skills_dir.iterdir() + if d.is_dir() and not d.name.startswith(".") and d.name not in prune and (d / "SKILL.md").is_file() + ) + if not skills: + return [] + + missing: list[tuple[Path, str, str]] = [] + + # 1. Canonical entries under .agents/skills/ + canonical_dir = root / ".agents" / "skills" + for skill in skills: + link = canonical_dir / f"magpie-{skill}" + if not link.is_symlink(): + expected = f"../../skills/{skill}" + missing.append((link, expected, "canonical")) + + # 2. Relay entries under every wired agent directory + for entry in sorted(root.iterdir()): + if ( + not entry.is_dir() + or not entry.name.startswith(".") + or entry.name in prune + or entry.name == ".agents" + ): + continue + agent_skills = entry / "skills" + if not agent_skills.is_dir(): + continue + for skill in skills: + link = agent_skills / f"magpie-{skill}" + if not link.is_symlink(): + expected = f"../../.agents/skills/magpie-{skill}" + missing.append((link, expected, "relay")) + + return sorted(missing) + + def find_archive_symlink_problems(root: Path) -> list[tuple[str, str, str]]: """Build the source archive the way the release does and return `(archive_path, target, kind)` for every symlink in it that a safe @@ -232,7 +295,8 @@ def main(argv: list[str] | None = None) -> int: cycles = find_cyclic_symlinks(root) relays = find_misdirected_relays(root) - if not cycles and not relays: + missing = find_missing_relays(root) + if not cycles and not relays and not missing: return 0 out = sys.stderr.write @@ -244,6 +308,10 @@ def main(argv: list[str] | None = None) -> int: out("error: misdirected skill relay symlink(s):\n") for link, actual, expected in relays: out(f" {_rel(link, root)} -> {actual} (expected {expected})\n") + if missing: + out("error: missing skill symlink(s):\n") + for link, expected, kind in missing: + out(f" {_rel(link, root)} (expected {kind} link -> {expected})\n") out("\nSee tools/symlink-lint/README.md and skills/setup/agents.md.\n") return 1 diff --git a/tools/symlink-lint/tests/test_symlink_lint.py b/tools/symlink-lint/tests/test_symlink_lint.py index 668a94b5f..03ac172a3 100644 --- a/tools/symlink-lint/tests/test_symlink_lint.py +++ b/tools/symlink-lint/tests/test_symlink_lint.py @@ -14,7 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -"""Behavioural fixtures for symlink-lint's two rules. +"""Behavioural fixtures for symlink-lint's rules. Rule 1 (cycles): parity with the earlier bash draft — cyclic -> flagged, dangling -> skipped, canonical+relay -> allowed, plus pruned / symlink-to- @@ -22,6 +22,9 @@ Rule 2 (relay correctness): canonical links point into ../../skills/; relays point at ../../.agents/skills/magpie-. + +Rule 3 (completeness): every skill under skills/ has a canonical entry in +.agents/skills/, and every wired agent directory relays the full set. """ from __future__ import annotations @@ -144,10 +147,90 @@ def test_non_magpie_symlink_ignored_by_relay_rule(tmp_path: Path) -> None: assert symlink_lint.find_misdirected_relays(tmp_path) == [] +# ---- Rule 3: completeness ------------------------------------------------ + + +def test_completeness_clean_when_all_canonical_and_relays_wired(tmp_path: Path) -> None: + (tmp_path / "skills" / "setup").mkdir(parents=True) + (tmp_path / "skills" / "setup" / "SKILL.md").write_text("setup\n") + _symlink(tmp_path, ".agents/skills/magpie-setup", "../../skills/setup") + _symlink(tmp_path, ".claude/skills/magpie-setup", "../../.agents/skills/magpie-setup") + _wire_skill(tmp_path, relay_target="../../.agents/skills/magpie-x") + assert symlink_lint.find_missing_relays(tmp_path) == [] + + +def test_completeness_flags_missing_canonical_link(tmp_path: Path) -> None: + (tmp_path / "skills" / "setup").mkdir(parents=True) + (tmp_path / "skills" / "setup" / "SKILL.md").write_text("setup\n") + (tmp_path / "skills" / "x").mkdir(parents=True) + (tmp_path / "skills" / "x" / "SKILL.md").write_text("x\n") + _symlink(tmp_path, ".agents/skills/magpie-setup", "../../skills/setup") + assert offending_paths(symlink_lint.find_missing_relays(tmp_path), tmp_path) == { + ".agents/skills/magpie-x" + } + + +def test_completeness_flags_missing_relay_in_wired_agent_dir(tmp_path: Path) -> None: + (tmp_path / "skills" / "setup").mkdir(parents=True) + (tmp_path / "skills" / "setup" / "SKILL.md").write_text("setup\n") + (tmp_path / "skills" / "x").mkdir(parents=True) + (tmp_path / "skills" / "x" / "SKILL.md").write_text("x\n") + _symlink(tmp_path, ".agents/skills/magpie-setup", "../../skills/setup") + _symlink(tmp_path, ".agents/skills/magpie-x", "../../skills/x") + _symlink(tmp_path, ".claude/skills/magpie-setup", "../../.agents/skills/magpie-setup") + assert offending_paths(symlink_lint.find_missing_relays(tmp_path), tmp_path) == { + ".claude/skills/magpie-x" + } + + +def test_completeness_unwired_agent_dir_not_required(tmp_path: Path) -> None: + (tmp_path / "skills" / "setup").mkdir(parents=True) + (tmp_path / "skills" / "setup" / "SKILL.md").write_text("setup\n") + _symlink(tmp_path, ".agents/skills/magpie-setup", "../../skills/setup") + _symlink(tmp_path, ".claude/skills/magpie-setup", "../../.agents/skills/magpie-setup") + _wire_skill(tmp_path, relay_target="../../.agents/skills/magpie-x") + (tmp_path / ".github").mkdir() + assert symlink_lint.find_missing_relays(tmp_path) == [] + + +def test_completeness_skipped_when_no_skills_dir(tmp_path: Path) -> None: + (tmp_path / ".agents" / "skills").mkdir(parents=True) + assert symlink_lint.find_missing_relays(tmp_path) == [] + + +def test_completeness_skipped_when_not_framework_checkout(tmp_path: Path) -> None: + # Adopter repo without skills/setup/SKILL.md is exempt + (tmp_path / "skills" / "custom").mkdir(parents=True) + (tmp_path / "skills" / "custom" / "SKILL.md").write_text("custom\n") + (tmp_path / ".agents" / "skills").mkdir(parents=True) + assert symlink_lint.find_missing_relays(tmp_path) == [] + + +def test_completeness_ignores_source_pointer_and_non_skill_files(tmp_path: Path) -> None: + (tmp_path / "skills" / "setup").mkdir(parents=True) + (tmp_path / "skills" / "setup" / "SKILL.md").write_text("setup\n") + _symlink(tmp_path, ".agents/skills/magpie-setup", "../../skills/setup") + _symlink(tmp_path, ".claude/skills/magpie-setup", "../../.agents/skills/magpie-setup") + _wire_skill(tmp_path, relay_target="../../.agents/skills/magpie-x") + # Trusted-source pointer directory (source.md, no SKILL.md) -> ignored + (tmp_path / "skills" / "pointer").mkdir(parents=True) + (tmp_path / "skills" / "pointer" / "source.md").write_text("source: foo\n") + # Non-skill files and cache directories -> ignored + (tmp_path / "skills" / "pyproject.toml").write_text("[project]\n") + (tmp_path / "skills" / ".mypy_cache").mkdir(parents=True) + (tmp_path / "skills" / ".ruff_cache").mkdir(parents=True) + (tmp_path / "skills" / "node_modules").mkdir(parents=True) + assert symlink_lint.find_missing_relays(tmp_path) == [] + + # ---- main() --------------------------------------------------------------- def test_main_returns_zero_when_clean(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + (tmp_path / "skills" / "setup").mkdir(parents=True) + (tmp_path / "skills" / "setup" / "SKILL.md").write_text("setup\n") + _symlink(tmp_path, ".agents/skills/magpie-setup", "../../skills/setup") + _symlink(tmp_path, ".claude/skills/magpie-setup", "../../.agents/skills/magpie-setup") _wire_skill(tmp_path, relay_target="../../.agents/skills/magpie-x") monkeypatch.setattr(symlink_lint, "repo_root", lambda: tmp_path) assert symlink_lint.main() == 0 @@ -165,7 +248,14 @@ def test_main_returns_one_on_misdirected_relay(tmp_path: Path, monkeypatch: pyte assert symlink_lint.main() == 1 -# ---- rule 3: release-archive symlink safety ------------------------------- +def test_main_returns_one_on_missing_symlink(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + (tmp_path / "skills" / "setup").mkdir(parents=True) + (tmp_path / "skills" / "setup" / "SKILL.md").write_text("setup\n") + monkeypatch.setattr(symlink_lint, "repo_root", lambda: tmp_path) + assert symlink_lint.main() == 1 + + +# ---- rule 4: release-archive symlink safety ------------------------------- def _git_repo(root: Path, gitattributes: str = "") -> None: