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..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 @@ -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,46 @@ # 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 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. +_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, 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, +) + # 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 +360,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 +376,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 +1515,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 +2171,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 +2246,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..4e5110c8 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,211 @@ 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_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" + 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"): 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