diff --git a/SKILL_DESIGN_PRINCIPLES.md b/SKILL_DESIGN_PRINCIPLES.md index 032de4dd..8c4d5a46 100644 --- a/SKILL_DESIGN_PRINCIPLES.md +++ b/SKILL_DESIGN_PRINCIPLES.md @@ -190,6 +190,11 @@ Every skill MUST include a **Dependencies** section listing: - Pack-level `/references/` or skill-level `skills//references/` are the only allowed reference locations. - Pack-level `references/INDEX.md` and `references/SOURCES.md` may exist for repository navigation/source attribution, but skills must not depend on them at execution time. +**Skill-local scripts rule (required):** +- Scripts the agent runs must live under `skills//scripts/` (often as symlinks into `/scripts//`). +- If a skill symlinks any file from `scripts//`, symlink **every** non-test file in that group (YAML, JSON, and config files included — not only `.py`). +- In skill markdown, document commands with skill-local paths (`python3 scripts/foo.py`, `oc apply -f scripts/manifest.yaml`). Do not use `/scripts/...` paths meant for the authoring repo. + **Rationale**: Makes dependencies explicit for debugging and troubleshooting. ## 6. Human-in-the-Loop Requirements diff --git a/ocp-admin/README.md b/ocp-admin/README.md index acc3a197..54354c97 100644 --- a/ocp-admin/README.md +++ b/ocp-admin/README.md @@ -452,13 +452,13 @@ For running `cluster-report` across many clusters (10–100+), use service accou ```bash # 1. One-time setup (requires cluster-admin): apply RBAC and extract tokens -python3 ocp-admin/scripts/cluster-report/build-kubeconfig.py setup --all-contexts +python3 scripts/cluster-report/build-kubeconfig.py setup --all-contexts # If RBAC is already configured, skip the apply step -python3 ocp-admin/scripts/cluster-report/build-kubeconfig.py setup --all-contexts --skip-rbac +python3 scripts/cluster-report/build-kubeconfig.py setup --all-contexts --skip-rbac # 2. Build merged kubeconfig from saved tokens -python3 ocp-admin/scripts/cluster-report/build-kubeconfig.py \ +python3 scripts/cluster-report/build-kubeconfig.py \ build --clusters ~/.ocp-clusters/clusters.json --verify # 3. Export and run diff --git a/ocp-admin/skills/cluster-report/references/multi-cluster-auth.md b/ocp-admin/skills/cluster-report/references/multi-cluster-auth.md index 99d8f620..d15ca47e 100644 --- a/ocp-admin/skills/cluster-report/references/multi-cluster-auth.md +++ b/ocp-admin/skills/cluster-report/references/multi-cluster-auth.md @@ -40,10 +40,10 @@ If you're currently logged into all the clusters you would like to get a report ```bash # Step 1: Setup — applies RBAC to each cluster, extracts SA tokens -python3 ocp-admin/scripts/cluster-report/build-kubeconfig.py setup --all-contexts +python3 scripts/build-kubeconfig.py setup --all-contexts # Step 2: Build — assembles a merged kubeconfig from the inventory -python3 ocp-admin/scripts/cluster-report/build-kubeconfig.py \ +python3 scripts/build-kubeconfig.py \ build --clusters ~/.ocp-clusters/clusters.json --verify # Step 3: Use — export and run the skill @@ -63,7 +63,7 @@ If you prefer to set up each cluster individually: ```bash oc login -oc apply -f ocp-admin/scripts/cluster-report/cluster-reporter-rbac.yaml +oc apply -f scripts/cluster-reporter-rbac.yaml ``` This creates: @@ -105,7 +105,7 @@ Set permissions: `chmod 600 ~/.ocp-clusters/clusters.json` ### 4. Build Kubeconfig ```bash -python3 ocp-admin/scripts/cluster-report/build-kubeconfig.py \ +python3 scripts/build-kubeconfig.py \ build --clusters ~/.ocp-clusters/clusters.json --output ~/.kube/cluster-report-kubeconfig ``` @@ -182,7 +182,7 @@ If `ca_cert` is omitted, TLS verification is skipped (`--insecure-skip-tls-verif ### `setup` Subcommand ```bash -python3 build-kubeconfig.py setup [OPTIONS] +python3 scripts/build-kubeconfig.py setup [OPTIONS] ``` @@ -204,7 +204,7 @@ Behavior: ### `build` Subcommand ```bash -python3 build-kubeconfig.py build --clusters [OPTIONS] +python3 scripts/build-kubeconfig.py build --clusters [OPTIONS] ``` @@ -229,18 +229,18 @@ SA token Secrets do not expire, but you may want to rotate them periodically: ```bash oc delete secret cluster-reporter-token -n cluster-reporter-system -oc apply -f ocp-admin/scripts/cluster-report/cluster-reporter-rbac.yaml +oc apply -f scripts/cluster-reporter-rbac.yaml oc get secret cluster-reporter-token -n cluster-reporter-system \ -o jsonpath='{.data.token}' | base64 -d -python3 build-kubeconfig.py build --clusters ~/.ocp-clusters/clusters.json --verify +python3 scripts/build-kubeconfig.py build --clusters ~/.ocp-clusters/clusters.json --verify ``` To detect expired or invalid tokens: ```bash -python3 build-kubeconfig.py build --clusters ~/.ocp-clusters/clusters.json --verify +python3 scripts/build-kubeconfig.py build --clusters ~/.ocp-clusters/clusters.json --verify ``` ## Security Best Practices diff --git a/ocp-admin/skills/cluster-report/scripts/cluster-reporter-rbac.yaml b/ocp-admin/skills/cluster-report/scripts/cluster-reporter-rbac.yaml new file mode 120000 index 00000000..cd07edcd --- /dev/null +++ b/ocp-admin/skills/cluster-report/scripts/cluster-reporter-rbac.yaml @@ -0,0 +1 @@ +../../../scripts/cluster-report/cluster-reporter-rbac.yaml \ No newline at end of file diff --git a/scripts/test_validate_compass_layout.py b/scripts/test_validate_compass_layout.py index d72502a1..cecd47c2 100644 --- a/scripts/test_validate_compass_layout.py +++ b/scripts/test_validate_compass_layout.py @@ -96,6 +96,82 @@ def test_nested_references_references_flagged(self) -> None: self.assertTrue(errors) self.assertIn("references/references/", errors[0]) + def test_missing_shared_script_symlink_flagged(self) -> None: + pack_dir = self.fixture_root + group_dir = pack_dir / "scripts" / "demo-group" + group_dir.mkdir(parents=True) + (group_dir / "run.py").write_text("# run\n", encoding="utf-8") + (group_dir / "config.yaml").write_text("key: value\n", encoding="utf-8") + + scripts_dir = pack_dir / "skills" / "demo-skill" / "scripts" + scripts_dir.mkdir(parents=True) + os.symlink("../../../scripts/demo-group/run.py", scripts_dir / "run.py") + + errors: list[str] = [] + compass._check_skill_scripts_layout( + pack_dir.name, pack_dir / "skills" / "demo-skill", errors + ) + + self.assertTrue(errors) + self.assertIn("config.yaml", errors[0]) + + def test_forbidden_pack_scripts_path_in_skill_markdown_flagged(self) -> None: + pack_dir = self.fixture_root + pack = pack_dir.name + skill_dir = pack_dir / "skills" / "demo-skill" + group_dir = pack_dir / "scripts" / "demo-group" + group_dir.mkdir(parents=True) + (group_dir / "run.py").write_text("# run\n", encoding="utf-8") + + scripts_dir = skill_dir / "scripts" + scripts_dir.mkdir(parents=True) + os.symlink("../../../scripts/demo-group/run.py", scripts_dir / "run.py") + + refs = skill_dir / "references" + refs.mkdir(parents=True) + (refs / "guide.md").write_text( + f"Run python3 {pack}/scripts/demo-group/run.py\n", + encoding="utf-8", + ) + + errors: list[str] = [] + compass._check_skill_scripts_layout(pack, skill_dir, errors) + + self.assertTrue(errors) + self.assertTrue( + any(f"{pack}/scripts/" in err and "authoring-repo" in err for err in errors), + msg=f"expected pack scripts path error, got: {errors}", + ) + + def test_shared_script_symlinks_complete_passes(self) -> None: + pack_dir = self.fixture_root + pack = pack_dir.name + skill_dir = pack_dir / "skills" / "demo-skill" + group_dir = pack_dir / "scripts" / "demo-group" + group_dir.mkdir(parents=True) + (group_dir / "run.py").write_text("# run\n", encoding="utf-8") + (group_dir / "config.yaml").write_text("key: value\n", encoding="utf-8") + (group_dir / "test_extra.py").write_text("# test\n", encoding="utf-8") + + scripts_dir = skill_dir / "scripts" + scripts_dir.mkdir(parents=True) + os.symlink("../../../scripts/demo-group/run.py", scripts_dir / "run.py") + os.symlink( + "../../../scripts/demo-group/config.yaml", scripts_dir / "config.yaml" + ) + + refs = skill_dir / "references" + refs.mkdir(parents=True) + (refs / "guide.md").write_text( + "Run `python3 scripts/run.py` and `oc apply -f scripts/config.yaml`.\n", + encoding="utf-8", + ) + + errors: list[str] = [] + compass._check_skill_scripts_layout(pack, skill_dir, errors) + + self.assertEqual(errors, []) + def test_forbidden_docs_markdown_link_flagged(self) -> None: skill_dir = self.fixture_root / "skills" / "demo-skill" refs = skill_dir / "references" diff --git a/scripts/validate_compass_manifests.py b/scripts/validate_compass_manifests.py index ba8ef85f..b5316570 100644 --- a/scripts/validate_compass_manifests.py +++ b/scripts/validate_compass_manifests.py @@ -13,6 +13,8 @@ - Skill documentation layout (all packs with skills/): no skills//docs/; no references/references/ nesting; no internal docs/ links; symlinks under references/ must not target docs/ + - Skill scripts layout: symlinks into pack scripts// must fan out all + non-test files in that group; skill docs must not use /scripts/ paths """ from __future__ import annotations @@ -34,6 +36,10 @@ _EXPECTED_NAMESPACE = "ai5-marketplace" _MD_LINK_RE = re.compile(r"\[[^\]]+\]\(([^)]+)\)") _SKILL_DOCS_STANDARD = "https://agent-plugins.org/specification" +_SHARED_SCRIPT_SYMLINK_RE = re.compile( + r"^(?:\.\./)+scripts/([^/]+)/(.+)$" +) +_TEST_SCRIPT_RE = re.compile(r"^test_.*\.py$") def _load_yaml(path: Path) -> dict: @@ -193,6 +199,72 @@ def _check_skill_docs_layout(skill_dir: Path, errors: list[str]) -> None: ) +def _parse_shared_script_symlink_target(raw: str) -> tuple[str, str] | None: + normalized = raw.replace("\\", "/") + match = _SHARED_SCRIPT_SYMLINK_RE.match(normalized) + if not match: + return None + return match.group(1), match.group(2) + + +def _check_skill_scripts_layout(pack: str, skill_dir: Path, errors: list[str]) -> None: + scripts_dir = skill_dir / "scripts" + if not scripts_dir.is_dir(): + return + + pack_dir = skill_dir.parent.parent + skill_rel = skill_dir.relative_to(_REPO_ROOT) + linked_by_group: dict[str, set[str]] = {} + + for entry in sorted(scripts_dir.iterdir()): + if not entry.is_symlink(): + continue + parsed = _parse_shared_script_symlink_target(os.readlink(entry)) + if not parsed: + continue + group, filename = parsed + linked_by_group.setdefault(group, set()).add(filename) + + if not linked_by_group: + return + + for group, linked_files in sorted(linked_by_group.items()): + group_dir = pack_dir / "scripts" / group + if not group_dir.is_dir(): + errors.append( + f"{skill_rel}/scripts: symlinks reference scripts/{group}/ but " + f"{pack}/scripts/{group}/ is missing" + ) + continue + for path in sorted(group_dir.iterdir()): + if not path.is_file(): + continue + if _TEST_SCRIPT_RE.match(path.name): + continue + if path.name not in linked_files: + errors.append( + f"{skill_rel}/scripts: missing symlink for " + f"{pack}/scripts/{group}/{path.name} — fan out all non-test " + f"files when linking from scripts/{group}/" + ) + + forbidden = f"{pack}/scripts/" + for md_file in sorted(skill_dir.rglob("*.md")): + if md_file.is_symlink() and not md_file.exists(): + continue + try: + text = md_file.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + continue + rel_md = md_file.relative_to(_REPO_ROOT) + for line_no, line in enumerate(text.splitlines(), start=1): + if forbidden in line: + errors.append( + f"{rel_md}:{line_no}: use skill-local scripts/ paths, not " + f"'{forbidden}' (authoring-repo layout only)" + ) + + def _check_skill_conventions(path: Path, data: dict, errors: list[str]) -> None: meta = data.get("metadata", {}) spec = data.get("spec", {}) @@ -214,7 +286,9 @@ def _check_skill_conventions(path: Path, data: dict, errors: list[str]) -> None: def validate_pack_layout(pack: str, errors: list[str]) -> None: pack_dir = _REPO_ROOT / pack for skill in sorted(skills_on_disk(pack_dir)): - _check_skill_docs_layout(pack_dir / "skills" / skill, errors) + skill_dir = pack_dir / "skills" / skill + _check_skill_docs_layout(skill_dir, errors) + _check_skill_scripts_layout(pack, skill_dir, errors) def validate_pack(pack: str, owned_mcps: dict[str, Path], errors: list[str]) -> None: @@ -276,6 +350,7 @@ def validate_pack(pack: str, owned_mcps: dict[str, Path], errors: list[str]) -> for skill in sorted(disk): skill_dir = pack_dir / "skills" / skill _check_skill_docs_layout(skill_dir, errors) + _check_skill_scripts_layout(pack, skill_dir, errors) manifest = skill_dir / "catalog-info.yaml" if not manifest.is_file():