From eb1967761489d019d7fa0338259ed8a434e4dd63 Mon Sep 17 00:00:00 2001 From: Justin McLean Date: Tue, 30 Jun 2026 10:25:06 +1000 Subject: [PATCH 1/2] feat(validator): add adapter authoring smoke check for contract:* tools Add a SOFT advisory check (aspect #11) to skill-and-tool-validator that verifies every contract:* adapter tool README declares the three authoring fields the adapter contract requires: credential/privacy handling, supported operations, and adopter config keys. All three checks are SOFT so legacy adapters can be brought into compliance deliberately without blocking unrelated changes. Twelve advisories fire on the current tree; exit code stays 0. New TestValidateAdapterAuthoring class adds 13 targeted tests. Generated-by: Claude (Opus 4.7) --- .../src/skill_and_tool_validator/__init__.py | 130 +++++++++++++- .../tests/test_validator.py | 167 ++++++++++++++++++ 2 files changed, 296 insertions(+), 1 deletion(-) diff --git a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py index 58233d46..fc700585 100644 --- a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py +++ b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py @@ -17,7 +17,7 @@ """Validate framework skill definitions. -This module validates ten aspects of every skill under +This module validates eleven aspects of every skill under skills/: 1. YAML frontmatter — every SKILL.md must have a valid frontmatter @@ -60,6 +60,12 @@ satisfy without editing the skill. Each hit is tagged with a remedy class (placeholder / adapter / capability-flag). Never fails the run — advisory only. +11. Adapter authoring smoke (SOFT) — every ``contract:*`` adapter + tool README must declare the three authoring fields that make an + adapter self-contained for adopters: credential / privacy + handling, supported operations, and adopter config keys. + Missing fields are advisories so legacy adapters can be brought + into compliance deliberately without blocking unrelated changes. SOFT categories surface as advisory warnings (stderr) without failing the run unless ``--strict`` is passed. @@ -104,6 +110,40 @@ # stated up front rather than discovered at first run. TOOL_PREREQUISITES_RE = re.compile(r"^##[ \t]+Prerequisites[ \t]*$", re.MULTILINE) +# --------------------------------------------------------------------------- +# Adapter authoring contract patterns (aspect #11, SOFT advisory) +# --------------------------------------------------------------------------- + +# Capability prefix that identifies an adapter (as opposed to a substrate tool). +_ADAPTER_CONTRACT_PREFIX = "contract:" + +# Credential / privacy handling — matches the canonical **Credentials / auth:** +# bullet used in all adapter Prerequisites sections. +_ADAPTER_CREDENTIALS_RE = re.compile(r"\*\*Credentials\s*/\s*auth:\*\*", re.IGNORECASE) + +# Operations documentation — any of: a named operations section heading, a +# tool.md or operations.md reference in the intro text. +_ADAPTER_OPERATIONS_RE = re.compile( + r"(?:" + r"^##\s+(?:Operations|Interface|How\s+to\s+use|Invocation" + r"|Read\s+subcommands|Write\s+subcommands|Subcommands)\s*$" + r"|\btool\.md\b" + r"|\boperations\.md\b" + r")", + re.MULTILINE | re.IGNORECASE, +) + +# Config keys documentation — a Configuration section or a project-config +# / *-config.md reference that points adopters to the adopter-visible knobs. +_ADAPTER_CONFIG_RE = re.compile( + r"(?:" + r"^##\s+(?:Configuration|Config(?:uration)?\s+[Kk]eys?)\s*$" + r"|\bproject-config\b" + r"|-config\.md\b" + r")", + re.MULTILINE, +) + # Optional `**Organization:** ` line in a tool README — declares that # the tool belongs to / is the adapter for a specific organization (e.g. # the ASF backends cve-tool-vulnogram, ponymail, apache-projects). Absent = @@ -314,6 +354,8 @@ def _read_mode_table() -> dict[str, str]: # editing the skill body. Each hit is tagged with a remedy class so maintainers # know how to generalise it. Never fails the run. ASF_COUPLING_CATEGORY = "asf_coupling" +# SOFT advisory: adapter authoring fields for contract:* tools. +ADAPTER_AUTHORING_CATEGORY = "adapter-authoring" # The `magpie-` namespace prefix every installed framework skill carries. SKILL_NAME_PREFIX = "magpie-" @@ -328,6 +370,7 @@ def _read_mode_table() -> dict[str, str]: LOWERCASE_F_FIELD_CATEGORY, EVAL_COVERAGE_CATEGORY, ASF_COUPLING_CATEGORY, + ADAPTER_AUTHORING_CATEGORY, } ) HARD_CATEGORIES: frozenset[str] = frozenset( @@ -1466,6 +1509,86 @@ def validate_tools(root: Path | None = None) -> Iterable[Violation]: ) +def validate_adapter_authoring(root: Path | None = None) -> Iterable[Violation]: + """Advisory (SOFT) checks for ``contract:*`` adapter tool READMEs. + + Adapter READMEs are the primary documentation surface for adopters + choosing and configuring an adapter. Three authoring fields make + an adapter self-contained: + + 1. **Credential / privacy handling** — ``**Credentials / auth:**`` + in the README so adopters know what credentials the adapter needs + and what privacy boundaries it respects. + 2. **Operations documentation** — at least one of: an ``## Operations`` + / ``## Interface`` / ``## Invocation`` / ``## How to use`` section, + or a ``tool.md`` / ``operations.md`` reference so adopters can + discover what the adapter actually does. + 3. **Config keys** — a ``## Configuration`` section or a + ``project-config`` / ``*-config.md`` reference so adopters know + which knobs they control. + + All are SOFT advisories — legacy adapters can be brought into + compliance deliberately without blocking unrelated changes. + ``substrate:*`` tools are excluded; the contract applies only to + ``contract:*`` adapter tools. + """ + for tool_dir in collect_tool_dirs(root): + readme = tool_dir / "README.md" + if not readme.exists(): + continue # validate_tools already reported the missing README + try: + text = readme.read_text(encoding="utf-8") + except OSError: + continue + + # Only check contract:* adapter tools + cap_match = TOOL_CAPABILITY_RE.search(text) + if cap_match is None: + continue + raw_cap = cap_match.group(1).strip() + entries = [e.strip() for e in raw_cap.split("+") if e.strip()] + if not any(e.startswith(_ADAPTER_CONTRACT_PREFIX) for e in entries): + continue + + # Check 1: credential / privacy handling + if _ADAPTER_CREDENTIALS_RE.search(text) is None: + yield Violation( + readme, + 1, + f"adapter-authoring [credential-handling] adapter '{tool_dir.name}' " + f"README missing '**Credentials / auth:**' — adapter READMEs must " + f"declare credential and privacy handling requirements so adopters " + f"know what the adapter needs before wiring it in " + f"(see docs/adapters.md § Adapter READMEs are contracts)", + category=ADAPTER_AUTHORING_CATEGORY, + ) + + # Check 2: operations documentation + if _ADAPTER_OPERATIONS_RE.search(text) is None: + yield Violation( + readme, + 1, + f"adapter-authoring [operations] adapter '{tool_dir.name}' " + f"README has no operations section (## Operations / ## Interface / " + f"## Invocation / ## How to use) or tool.md reference — " + f"document supported operations so adopters know what the adapter provides " + f"(see docs/adapters.md § Adapter READMEs are contracts)", + category=ADAPTER_AUTHORING_CATEGORY, + ) + + # Check 3: config keys documentation + if _ADAPTER_CONFIG_RE.search(text) is None: + yield Violation( + readme, + 1, + f"adapter-authoring [config-keys] adapter '{tool_dir.name}' " + f"README has no ## Configuration section or project-config reference — " + f"document adopter config keys so the adapter is self-contained " + f"(see docs/adapters.md § Adapter READMEs are contracts)", + category=ADAPTER_AUTHORING_CATEGORY, + ) + + def _parse_capability_doc_table(text: str, header: str) -> dict[str, set[str]]: """Parse a markdown table rooted at *header* in labels-and-capabilities.md. @@ -2042,6 +2165,10 @@ def run_validation(root: Path | None = None) -> list[Violation]: # Tool-level checks: every tools// has a README that declares its capability. violations.extend(validate_tools(repo_root)) + # Adapter authoring smoke: contract:* tool READMEs declare credentials, + # operations, and config keys (SOFT advisory). + violations.extend(validate_adapter_authoring(repo_root)) + # Capability-sync check: the doc tables and the source must agree. violations.extend(validate_capability_sync(repo_root)) @@ -2113,6 +2240,7 @@ def main(argv: list[str] | None = None) -> int: _SOFT_RULE_PREFIXES: tuple[str, ...] = ( "action-inventory", + "adapter-authoring", "asf-coupling", "chain-handoff", "criteria-source", diff --git a/tools/skill-and-tool-validator/tests/test_validator.py b/tools/skill-and-tool-validator/tests/test_validator.py index 9dc53f26..e7e34025 100644 --- a/tools/skill-and-tool-validator/tests/test_validator.py +++ b/tools/skill-and-tool-validator/tests/test_validator.py @@ -28,6 +28,7 @@ _MODE_TAXONOMY, _OFF_MODES, _PRIVACY_EXTERNAL_CONTENT_MODES, + ADAPTER_AUTHORING_CATEGORY, ALL_CATEGORIES, ALLOWED_MODES, ASF_COUPLING_CATEGORY, @@ -63,6 +64,7 @@ resolve_link, run_validation, slugify, + validate_adapter_authoring, validate_asf_coupling, validate_capability_sync, validate_eval_coverage, @@ -2453,6 +2455,171 @@ def test_tool_organization_unknown(self, tmp_path: Path) -> None: assert "'**Organization:** Nope'" in violations[0].message +class TestValidateAdapterAuthoring: + """Tests for the SOFT adapter authoring smoke check (aspect #11).""" + + def _contract_readme( + self, + *, + credentials: bool = True, + operations: bool = True, + config: bool = True, + contract: str = "contract:tracker", + ) -> str: + """Build a minimal contract:* adapter README with selectable fields.""" + lines = [ + "# tools/my-adapter", + "", + f"**Capability:** {contract}", + "", + "An adapter description.", + "", + ] + if operations: + lines += ["## Operations", "", "- `search` — find issues.", ""] + lines += ["## Prerequisites", ""] + if credentials: + lines.append("- **Credentials / auth:** API token required.") + else: + lines.append("- **Runtime:** curl.") + lines.append("") + if config: + lines += ["## Configuration", "", "Set `MY_TOKEN` in the environment.", ""] + return "\n".join(lines) + + def test_complete_contract_adapter_no_violations(self, tmp_path: Path) -> None: + root = _make_tools_root(tmp_path) + tool = root / "tools" / "my-adapter" + tool.mkdir() + (tool / "README.md").write_text(self._contract_readme()) + violations = [v for v in validate_adapter_authoring(root) if v.category == ADAPTER_AUTHORING_CATEGORY] + assert violations == [] + + def test_substrate_tool_not_checked(self, tmp_path: Path) -> None: + root = _make_tools_root(tmp_path) + tool = root / "tools" / "substrate-tool" + tool.mkdir() + # substrate:* tool with no credentials / operations / config + (tool / "README.md").write_text( + "# substrate-tool\n\n**Capability:** substrate:analytics\n\n" + "A substrate tool.\n\n## Prerequisites\n\n- Python 3.11+.\n" + ) + violations = [v for v in validate_adapter_authoring(root) if v.category == ADAPTER_AUTHORING_CATEGORY] + assert violations == [] + + def test_missing_credentials_fires_advisory(self, tmp_path: Path) -> None: + root = _make_tools_root(tmp_path) + tool = root / "tools" / "no-creds" + tool.mkdir() + (tool / "README.md").write_text(self._contract_readme(credentials=False)) + violations = [v for v in validate_adapter_authoring(root) if v.category == ADAPTER_AUTHORING_CATEGORY] + assert len(violations) == 1 + assert "credential-handling" in violations[0].message + assert "no-creds" in violations[0].message + + def test_missing_operations_fires_advisory(self, tmp_path: Path) -> None: + root = _make_tools_root(tmp_path) + tool = root / "tools" / "no-ops" + tool.mkdir() + (tool / "README.md").write_text(self._contract_readme(operations=False)) + violations = [v for v in validate_adapter_authoring(root) if v.category == ADAPTER_AUTHORING_CATEGORY] + assert len(violations) == 1 + assert "operations" in violations[0].message + assert "no-ops" in violations[0].message + + def test_missing_config_fires_advisory(self, tmp_path: Path) -> None: + root = _make_tools_root(tmp_path) + tool = root / "tools" / "no-config" + tool.mkdir() + (tool / "README.md").write_text(self._contract_readme(config=False)) + violations = [v for v in validate_adapter_authoring(root) if v.category == ADAPTER_AUTHORING_CATEGORY] + assert len(violations) == 1 + assert "config-keys" in violations[0].message + assert "no-config" in violations[0].message + + def test_all_three_missing_fires_three_advisories(self, tmp_path: Path) -> None: + root = _make_tools_root(tmp_path) + tool = root / "tools" / "bare-adapter" + tool.mkdir() + (tool / "README.md").write_text( + "# bare-adapter\n\n**Capability:** contract:mail-source\n\n" + "A bare adapter.\n\n## Prerequisites\n\n- Something.\n" + ) + violations = [v for v in validate_adapter_authoring(root) if v.category == ADAPTER_AUTHORING_CATEGORY] + assert len(violations) == 3 + tags = {v.message.split("[")[1].split("]")[0] for v in violations} + assert tags == {"credential-handling", "operations", "config-keys"} + + def test_tool_md_reference_satisfies_operations(self, tmp_path: Path) -> None: + root = _make_tools_root(tmp_path) + tool = root / "tools" / "tool-md-ops" + tool.mkdir() + readme = ( + "# tool-md-ops\n\n**Capability:** contract:tracker\n\n" + "See [tool.md](tool.md) for the operation catalogue.\n\n" + "## Prerequisites\n\n- **Credentials / auth:** API token.\n\n" + "## Configuration\n\nSet `TRACKER_URL`.\n" + ) + (tool / "README.md").write_text(readme) + violations = [v for v in validate_adapter_authoring(root) if v.category == ADAPTER_AUTHORING_CATEGORY] + assert violations == [] + + def test_interface_section_satisfies_operations(self, tmp_path: Path) -> None: + root = _make_tools_root(tmp_path) + tool = root / "tools" / "iface-ops" + tool.mkdir() + readme = ( + "# iface-ops\n\n**Capability:** contract:mail-archive\n\n" + "A mail archive adapter.\n\n" + "## Prerequisites\n\n- **Credentials / auth:** None.\n\n" + "## Interface\n\nList of operations.\n\n" + "## Configuration\n\nSet `ARCHIVE_URL`.\n" + ) + (tool / "README.md").write_text(readme) + violations = [v for v in validate_adapter_authoring(root) if v.category == ADAPTER_AUTHORING_CATEGORY] + assert violations == [] + + def test_project_config_reference_satisfies_config(self, tmp_path: Path) -> None: + root = _make_tools_root(tmp_path) + tool = root / "tools" / "proj-config-adapter" + tool.mkdir() + readme = ( + "# proj-config-adapter\n\n**Capability:** contract:source-control\n\n" + "Config lives in `/vcs-config.md`.\n\n" + "## Prerequisites\n\n- **Credentials / auth:** OAuth token.\n\n" + "## How to use\n\nInvoke via `vcs clone`.\n" + ) + (tool / "README.md").write_text(readme) + violations = [v for v in validate_adapter_authoring(root) if v.category == ADAPTER_AUTHORING_CATEGORY] + assert violations == [] + + def test_multi_capability_with_contract_is_checked(self, tmp_path: Path) -> None: + root = _make_tools_root(tmp_path) + tool = root / "tools" / "multi-cap" + tool.mkdir() + # contract:mail-source + contract:mail-draft — should be checked + (tool / "README.md").write_text( + "# multi-cap\n\n**Capability:** contract:mail-source + contract:mail-draft\n\n" + "A dual-contract adapter.\n\n## Prerequisites\n\n- Something.\n" + ) + violations = [v for v in validate_adapter_authoring(root) if v.category == ADAPTER_AUTHORING_CATEGORY] + assert len(violations) == 3 + + def test_adapter_authoring_is_soft_category(self) -> None: + assert ADAPTER_AUTHORING_CATEGORY in SOFT_CATEGORIES + + def test_adapter_authoring_in_all_categories(self) -> None: + assert ADAPTER_AUTHORING_CATEGORY in ALL_CATEGORIES + + def test_no_readme_skipped_gracefully(self, tmp_path: Path) -> None: + root = _make_tools_root(tmp_path) + (root / "tools" / "no-readme-adapter").mkdir() + # No README — validate_tools reports this; adapter_authoring must not crash + violations = list(validate_adapter_authoring(root)) + assert all(v.category == ADAPTER_AUTHORING_CATEGORY for v in violations) + assert violations == [] + + class TestOrganizationMembership: def test_known_organizations_excludes_template(self, tmp_path: Path) -> None: for name in ("ASF", "independent", "_template"): From 7a2ec502003036af192f3ebc8b25d2ea7dbbb992 Mon Sep 17 00:00:00 2001 From: Justin McLean Date: Tue, 30 Jun 2026 12:07:05 +1000 Subject: [PATCH 2/2] fix two false positives and add to implmentation plan --- .../src/skill_and_tool_validator/__init__.py | 14 +++++-- .../tests/test_validator.py | 40 +++++++++++++++++++ tools/spec-loop/IMPLEMENTATION_PLAN.md | 26 ++++++++++++ 3 files changed, 76 insertions(+), 4 deletions(-) diff --git a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py index fc700585..f2fbcefd 100644 --- a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py +++ b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py @@ -118,8 +118,12 @@ _ADAPTER_CONTRACT_PREFIX = "contract:" # Credential / privacy handling — matches the canonical **Credentials / auth:** -# bullet used in all adapter Prerequisites sections. -_ADAPTER_CREDENTIALS_RE = re.compile(r"\*\*Credentials\s*/\s*auth:\*\*", re.IGNORECASE) +# bullet used in most adapter Prerequisites sections, plus equivalent bolded +# labels that declare credential handling under different wording (e.g. a +# contract README that delegates to a backend with +# **CLIs / credentials / network:**). The shape is a bolded label ending in a +# colon that mentions "credential(s)" — narrow enough to avoid matching prose. +_ADAPTER_CREDENTIALS_RE = re.compile(r"\*\*[^*]*\bcredentials?\b[^*]*:\*\*", re.IGNORECASE) # Operations documentation — any of: a named operations section heading, a # tool.md or operations.md reference in the intro text. @@ -133,13 +137,15 @@ re.MULTILINE | re.IGNORECASE, ) -# Config keys documentation — a Configuration section or a project-config -# / *-config.md reference that points adopters to the adopter-visible knobs. +# Config keys documentation — a Configuration section, a project-config +# / *-config.md reference, or an inline dotted project-config key +# (`tools..`) that points adopters to the adopter-visible knobs. _ADAPTER_CONFIG_RE = re.compile( r"(?:" r"^##\s+(?:Configuration|Config(?:uration)?\s+[Kk]eys?)\s*$" r"|\bproject-config\b" r"|-config\.md\b" + r"|\btools\.[a-z0-9_-]+\.[a-z0-9_]+\b" r")", re.MULTILINE, ) diff --git a/tools/skill-and-tool-validator/tests/test_validator.py b/tools/skill-and-tool-validator/tests/test_validator.py index e7e34025..4e5110c8 100644 --- a/tools/skill-and-tool-validator/tests/test_validator.py +++ b/tools/skill-and-tool-validator/tests/test_validator.py @@ -2593,6 +2593,46 @@ def test_project_config_reference_satisfies_config(self, tmp_path: Path) -> None violations = [v for v in validate_adapter_authoring(root) if v.category == ADAPTER_AUTHORING_CATEGORY] assert violations == [] + def test_alt_credentials_label_satisfies_credentials(self, tmp_path: Path) -> None: + # Regression: a contract README that declares credential handling under + # a non-canonical bolded label (here, delegating to a backend adapter + # like the real tools/cve-tool) must NOT be flagged as missing creds. + root = _make_tools_root(tmp_path) + tool = root / "tools" / "delegating-contract" + tool.mkdir() + readme = ( + "# delegating-contract\n\n**Capability:** contract:cve-authority\n\n" + "A contract that delegates to a resolved backend adapter.\n\n" + "## Prerequisites\n\n" + "- **CLIs / credentials / network:** Provided entirely by the " + "resolved adapter — see that adapter for its concrete prerequisites.\n\n" + "## Interface\n\nThe contract methods.\n\n" + "## Configuration\n\nSet via `project.md`.\n" + ) + (tool / "README.md").write_text(readme) + violations = [v for v in validate_adapter_authoring(root) if v.category == ADAPTER_AUTHORING_CATEGORY] + assert violations == [] + + def test_inline_dotted_config_key_satisfies_config(self, tmp_path: Path) -> None: + # Regression: a contract README that documents an adopter knob inline as + # a dotted project-config key (here, like the real tools/gmail + # `tools.gmail.oauth_credentials_path`) must NOT be flagged as missing + # config, even without a dedicated ## Configuration heading. + root = _make_tools_root(tmp_path) + tool = root / "tools" / "inline-config" + tool.mkdir() + readme = ( + "# inline-config\n\n**Capability:** contract:mail-source\n\n" + "A mail-source adapter.\n\n" + "## Operations\n\n- `fetch` — read mail.\n\n" + "## Prerequisites\n\n" + "- **Credentials / auth:** OAuth refresh-token file, overridable " + "via `tools.inline-config.oauth_credentials_path`.\n" + ) + (tool / "README.md").write_text(readme) + violations = [v for v in validate_adapter_authoring(root) if v.category == ADAPTER_AUTHORING_CATEGORY] + assert violations == [] + def test_multi_capability_with_contract_is_checked(self, tmp_path: Path) -> None: root = _make_tools_root(tmp_path) tool = root / "tools" / "multi-cap" diff --git a/tools/spec-loop/IMPLEMENTATION_PLAN.md b/tools/spec-loop/IMPLEMENTATION_PLAN.md index 77929679..3ea42c3e 100644 --- a/tools/spec-loop/IMPLEMENTATION_PLAN.md +++ b/tools/spec-loop/IMPLEMENTATION_PLAN.md @@ -464,6 +464,32 @@ slugs, not numbers (numbering implies an order the specs don't carry). Spec: [`specs/skill-reconciler.md`](specs/skill-reconciler.md). Branch `skill-reconciler-source-pairing`. +21. **Bring legacy adapter READMEs into adapter-authoring compliance.** + The adapter-authoring smoke check (work item 9, shipped on + `adapter-authoring-smoke-validation`) now flags 10 SOFT advisories across 9 + `contract:*` adapter READMEs that are missing a required authoring field. + Bring each into compliance by adding the missing section/reference (or + documenting why the field legitimately does not apply, once an opt-out + convention exists). Missing **config-keys** (a `## Configuration` section, + a `project-config` / `*-config.md` reference, or an inline + `tools..` knob): `apache-projects`, `cve-org`, `github`, + `github-body-field`, `github-rollup`, `ponymail`, `vcs`, and + `cve-tool-vulnogram`. Missing **operations** (an `## Operations` / + `## Interface` / `## Invocation` / `## How to use` section or a `tool.md` + reference): `cve-tool-vulnogram` and `mail-source`. Several are thin + backend adapters whose real docs live in their contract README, so prefer a + one-line pointer to the contract over duplicating prose. The two former + false positives (`cve-tool` credential delegation, `gmail` inline config + key) are already resolved by broadening the validator matchers and are not + part of this item. + Validation: + ```bash + uv run --directory tools/skill-and-tool-validator skill-and-tool-validate + uv run --directory tools/skill-and-tool-validator --group dev pytest + ``` + Spec: [`specs/adapters.md`](specs/adapters.md). + Branch `adapter-readme-authoring-compliance`. + --- ## Notes & discoveries