lint-repo now fails when a workflow asks repomatic metadata for a key that no longer exists

Author: kdeldycke

changelog.md

@@ -5,6 +5,7 @@
 > [!WARNING]
 > This version is **not released yet** and is under active development.
 
+- `lint-repo` now fails when a workflow asks `repomatic metadata` for a key that no longer exists. `init` syncs a header-only workflow's header and pins but never its job bodies, so a retired key sits in a `run:` line nothing sweeps until the command rejects it and every job gated on it through `needs:` dies with it. `coverage_cells` took a downstream test workflow down this way.
 - `lint-repo` now warns when a Sphinx project's GitHub website field differs from the documentation URL declared in `[project.urls]`.
 - New `ci-status` command reporting each workflow's latest run and which of its failing jobs actually gate a merge. Reads jobs rather than runs, so an allowed-failure probe cannot hide inside a green run conclusion.
 - New `repomatic run <tool> --verify` reporting which files a formatter would rewrite, without touching the working tree.

docs/workflows.md

@@ -418,6 +418,7 @@ None of these jobs read a label config committed to the repository. `labels.toml
 - Validates repository metadata (package name, Sphinx docs, project description) and Dependabot configuration using [`repomatic lint-repo`](https://github.com/kdeldycke/repomatic/blob/main/repomatic/cli.py). Reads `pyproject.toml` directly. When `REPOMATIC_PAT` is configured, also validates PAT capabilities (contents, issues, pull requests, Dependabot alerts, workflows permissions). Warns when the fork PR workflow approval policy is weaker than `first_time_contributors`. Warns about missing `VIRUSTOTAL_API_KEY` when Nuitka binary compilation is active. Warns about missing `REPOMATIC_NOTIFICATIONS_PAT` when the unsubscribe workflow is enabled.
 - Warns when a Sphinx project's GitHub website field does not name the documentation URL it declares in `[project.urls]` (`Documentation`, then `Docs`). A trailing slash and the case of the scheme and host are ignored, since GitHub stores the website field with the slash a browser appends. Moving a documentation site to a new domain is what this catches: Sphinx renders `<link rel="canonical">` from `html_baseurl`, so every published page names the new origin while the repository sidebar keeps sending visitors to the old one. A project declaring no documentation URL keeps the presence-only check
 - Warns when a release download URL in `docs/install.md` names a file its release does not carry. The release freeze pins those URLs before the binaries exist, so a failed build lane leaves the guide advertising 404s until the next release moves past it: this is the check that surfaces the gap instead of leaving it for a user to hit. Versionless `releases/latest/download` URLs are checked against the latest published release too, and rot longer: nothing rewrites them at release time, so a renamed asset leaves one pointing at a 404 indefinitely
+- Fails when a workflow's `run:` line asks `repomatic metadata` for a key that no longer exists, reading the invocation the way Click does so an option's value is never mistaken for a positional key. `repomatic init` syncs a header-only workflow's header and its `uses:` pins and leaves the job bodies to the repository, so a key retired upstream stays in a `run:` line nothing sweeps. The command answers an unknown key with a `UsageError`, and every job reaching the metadata job through `needs:` dies with it, which turns a retired key into a whole workflow failing at its first job on the next push. Fatal, like the inline-pin check beside it: both describe a workflow that is already broken rather than one that might age badly
 - **Requires**:
   - Python package (with a `pyproject.toml` file)
 

repomatic/lint_repo.py

@@ -28,6 +28,7 @@
 import json
 import logging
 import re
+import shlex
 from dataclasses import dataclass
 from functools import cached_property
 from pathlib import Path
@@ -48,7 +49,12 @@
     TEST_RUNNERS_PR,
     UNSTABLE_PYTHON_VERSIONS,
 )
-from .metadata import Dialect, Metadata
+from .metadata import (
+    METADATA_VALUE_OPTIONS,
+    Dialect,
+    Metadata,
+    all_metadata_keys,
+)
 from .pypi import (
     PYPI_TRUSTED_PUBLISHER_WORKFLOW,
     get_latest_release_file,
@@ -1775,6 +1781,151 @@ def check_inline_pins_match_upstream(
     )
 
 
+_METADATA_KEY_TOKEN = re.compile(r"^[a-z][a-z0-9_]*$")
+"""Shape of a metadata key, used to tell one from a neighbouring shell token.
+
+Every key is a Python identifier, so a token carrying a hyphen (`github-json`),
+a dollar (`$GITHUB_OUTPUT`) or a dot is something else on the command line and
+is passed over rather than reported as unknown.
+"""
+
+_SHELL_OPERATORS = frozenset(("&&", "||", ";", "|", ">", ">>", "<", "&"))
+"""Tokens that end the invocation and start unrelated words.
+
+`repomatic metadata a b && echo done` must not report `echo` and `done` as
+metadata keys.
+"""
+
+
+def _requested_metadata_keys(command: str, package: str) -> list[str]:
+    """Positional keys a shell command passes to `<package> metadata`.
+
+    Reads the tail of the invocation the way Click would: options are dropped
+    along with the value each one consumes, and what remains are the positional
+    key arguments. Handles both spellings in use, the upstream
+    `uv run -- repomatic metadata …` and the downstream
+    `uvx 'repomatic==1.2.3' metadata …`, by looking for the subcommand after
+    any token naming the package.
+
+    :param command: The step's `run:` script, folded or literal.
+    :param package: Upstream package name (like `repomatic`).
+    :return: The key names requested, in the order written.
+    """
+    keys: list[str] = []
+    # A literal block scalar holds a whole script: read it a line at a time so
+    # a later command's words cannot be mistaken for this one's arguments.
+    # Backslash continuations are rejoined first, being one command still.
+    for line in command.replace("\\\n", " ").splitlines():
+        try:
+            tokens = shlex.split(line, comments=True)
+        except ValueError:
+            # An unbalanced quote is a line this cannot read. A shell would
+            # reject it too, so leave it to the shell to complain.
+            continue
+        seen_package = False
+        tail: list[str] | None = None
+        for token in tokens:
+            if tail is not None:
+                if token in _SHELL_OPERATORS:
+                    break
+                tail.append(token)
+            elif token == "metadata" and seen_package:
+                tail = []
+            elif token == package or token.startswith(f"{package}=="):
+                seen_package = True
+        if not tail:
+            continue
+
+        skip_next = False
+        for token in tail:
+            if skip_next:
+                skip_next = False
+                continue
+            if token.startswith("-"):
+                # `--format=json` carries its value inline, consuming nothing.
+                skip_next = "=" not in token and token in METADATA_VALUE_OPTIONS
+                continue
+            if _METADATA_KEY_TOKEN.match(token):
+                keys.append(token)
+    return keys
+
+
+def check_metadata_keys(
+    workflow_dir: Path = WORKFLOW_DIR,
+    upstream_repo: str = DEFAULT_REPO,
+) -> list[CheckResult]:
+    """Check the metadata keys workflows request still exist.
+
+    A downstream repository owns the job bodies of its header-only workflows:
+    `repomatic init` syncs their `name`, `on` and `concurrency` blocks and the
+    `uses:` pins, and never touches the steps below. So a key retired upstream
+    keeps being asked for by a `run:` line nothing sweeps, and the `metadata`
+    command answers a retired key with a `UsageError`. Since every other job in
+    a test workflow reaches it through `needs:`, the whole run dies at the first
+    job, on the next push, from a workflow file that looks freshly synced.
+
+    That is not hypothetical: `coverage_cells` went away with the Codecov
+    integration and took a downstream test workflow down with it. Failing here
+    instead moves the report to lint time, where it names the file and the job.
+
+    :param workflow_dir: Directory holding the workflow YAML files.
+    :param upstream_repo: Upstream `owner/repo`; its name is the package whose
+        `metadata` invocations are read (like `repomatic`).
+    :return: A list of `CheckResult`.
+    """
+    package = upstream_repo.rsplit("/", 1)[-1]
+    workflows = _load_workflows(workflow_dir)
+    if not workflows:
+        return [CheckResult(None, "Metadata keys check: skipped (no workflows).")]
+
+    valid = all_metadata_keys()
+    results: list[CheckResult] = []
+    for path, data in workflows.items():
+        for job_id, job in data["jobs"].items():
+            if not isinstance(job, dict):
+                continue
+            for step in job.get("steps", []) or ():
+                if not isinstance(step, dict):
+                    continue
+                command = step.get("run")
+                if not isinstance(command, str):
+                    continue
+                requested = _requested_metadata_keys(command, package)
+                if not requested:
+                    continue
+                where = f"{path.name}:{job_id}"
+                unknown = sorted(set(requested) - valid)
+                if unknown:
+                    names = ", ".join(f"`{key}`" for key in unknown)
+                    results.append(
+                        CheckResult(
+                            False,
+                            f"{where} asks `{package} metadata` for {names}, which"
+                            f" no longer exists. The command rejects an unknown"
+                            f" key outright, so this job fails on its next run,"
+                            f" and every job gated on it through `needs:` with"
+                            f" it. Run `{package} metadata --list-keys` for the"
+                            f" current set.",
+                        )
+                    )
+                else:
+                    count = len(requested)
+                    plural = "" if count == 1 else "s"
+                    results.append(
+                        CheckResult(
+                            True,
+                            f"{where}: {count} requested metadata key{plural}"
+                            f" exist{'s' if count == 1 else ''}.",
+                        )
+                    )
+
+    if not results:
+        return [
+            CheckResult(None, f"Metadata keys check: no `{package} metadata` calls.")
+        ]
+    return results
+
+
 def check_pr_templates(
     workflow_dir: Path = WORKFLOW_DIR,
     template_dir: Path = PR_TEMPLATE_DIR,
@@ -2148,6 +2299,9 @@ def _pat_permissions(ctx: LintContext) -> Iterator[CheckResult]:
         lambda ctx: check_inline_pins_match_upstream(),
         fatal=True,
     ),
+    # Fatal for the same reason as the pin check above: both describe a
+    # workflow that is already broken, not one that might age badly.
+    RepoCheck("metadata-keys", lambda ctx: check_metadata_keys(), fatal=True),
     RepoCheck("pr-templates", lambda ctx: check_pr_templates()),
     RepoCheck(
         "virustotal-secret",

repomatic/metadata.py

@@ -357,6 +357,25 @@ def all_metadata_keys() -> frozenset[str]:
     return frozenset(_METADATA_KEY_DESCRIPTIONS) | frozenset(_metadata_config_fields())
 
 
+METADATA_VALUE_OPTIONS: frozenset[str] = frozenset((
+    "--format",
+    "-o",
+    "--output",
+    "--sort-by",
+))
+"""Options on the `metadata` command consuming the token that follows them.
+
+Needed by {func}`repomatic.lint_repo.check_metadata_keys` to tell a positional
+key from an option's value while reading a workflow's `run:` line. The command
+itself is not importable from there: {mod}`repomatic.cli` reads `sys.stdout.name`
+at import time, so importing it under a test that has replaced stdout raises.
+
+Listed here rather than derived, and pinned against the real command by
+repomatic's own test suite, so an option added later cannot quietly turn its
+value into a token the lint reports as an unknown key.
+"""
+
+
 # Silence overly verbose debug messages from py-walk logger.
 logging.getLogger("py_walk").setLevel(logging.WARNING)
 

tests/test_lint_repo.py

@@ -25,6 +25,7 @@
 import pytest
 
 from repomatic import lint_repo
+from repomatic.cli import metadata as metadata_command
 from repomatic.github.token import PAT_PERMISSION_PROBES, probe_pat_permission
 from repomatic.lint_repo import (
     REPO_CHECKS,
@@ -34,6 +35,7 @@
     check_immutable_releases,
     check_inline_pins_match_upstream,
     check_install_guide_downloads,
+    check_metadata_keys,
     check_package_name_vs_repo,
     check_pat_stale_statuses_permission,
     check_pr_templates,
@@ -51,6 +53,7 @@
     run_repo_lint,
 )
 from repomatic.matrix_axes import UNSTABLE_PYTHON_VERSIONS
+from repomatic.metadata import METADATA_VALUE_OPTIONS
 from repomatic.pypi import TrustedPublisher
 from repomatic.registry import INSTALL_GUIDE_PATH
 from tests.conftest import metadata_from_pyproject, pat_results
@@ -1478,6 +1481,123 @@ def test_inline_pins_match_upstream_skips_without_refs(tmp_path):
     assert "nothing to compare" in msg
 
 
+# ---------------------------------------------------------------------------
+# Metadata key check tests
+# ---------------------------------------------------------------------------
+
+
+def _metadata_step(command):
+    """A one-job workflow whose single step runs *command*."""
+    return f"on: push\njobs:\n  metadata:\n    steps:\n      - run: {command}\n"
+
+
+@pytest.mark.parametrize(
+    ("command", "expected"),
+    [
+        # Upstream spelling: the CLI comes from the project's own lockfile.
+        pytest.param(
+            "uv --no-progress run --frozen -- repomatic metadata --format github-json"
+            ' --output "$GITHUB_OUTPUT" cli_scripts package_name',
+            ["cli_scripts", "package_name"],
+            id="uv-run",
+        ),
+        # Downstream spelling: the CLI comes from a pinned uvx environment.
+        pytest.param(
+            "uvx --no-progress 'repomatic==7.11.0' metadata --format github-json"
+            ' --output "$GITHUB_OUTPUT" cli_scripts package_name',
+            ["cli_scripts", "package_name"],
+            id="uvx-pinned",
+        ),
+        # An option's value is never a key, whether attached or separate.
+        pytest.param(
+            "repomatic metadata --format=json current_version",
+            ["current_version"],
+            id="inline-option-value",
+        ),
+        pytest.param(
+            "repomatic metadata -o out current_version",
+            ["current_version"],
+            id="short-option-value",
+        ),
+        # Words belonging to another command are not arguments to this one.
+        pytest.param(
+            "repomatic metadata current_version && echo done",
+            ["current_version"],
+            id="shell-operator",
+        ),
+        pytest.param(
+            "repomatic metadata current_version\necho done",
+            ["current_version"],
+            id="second-line",
+        ),
+        pytest.param("uv run pytest -m once", [], id="unrelated-command"),
+        # `metadata` also names a step id, an output and a job. Only the token
+        # following the package invocation is the subcommand.
+        pytest.param("echo metadata cli_scripts", [], id="bare-word"),
+    ],
+)
+def test_requested_metadata_keys(command, expected):
+    """Positional keys are read off the command line the way Click reads them."""
+    assert lint_repo._requested_metadata_keys(command, "repomatic") == expected
+
+
+def test_metadata_keys_flags_a_retired_key(tmp_path, monkeypatch):
+    """A key removed upstream fails the lint instead of the next workflow run."""
+    monkeypatch.chdir(tmp_path)
+    _write_ci_workflow(
+        tmp_path,
+        _metadata_step(
+            "uvx --no-progress 'repomatic==7.11.0' metadata cli_scripts coverage_cells"
+        ),
+    )
+    failures = [r for r in check_metadata_keys() if r.passed is False]
+    assert failures
+    assert "coverage_cells" in failures[0].message
+    assert "--list-keys" in failures[0].message
+
+
+def test_metadata_keys_accepts_current_keys(tmp_path, monkeypatch):
+    """Keys the command still answers raise nothing."""
+    monkeypatch.chdir(tmp_path)
+    _write_ci_workflow(
+        tmp_path, _metadata_step("repomatic metadata cli_scripts package_name")
+    )
+    results = check_metadata_keys()
+    assert all(r.passed is not False for r in results)
+
+
+def test_metadata_keys_skips_a_repo_that_never_calls_it(tmp_path, monkeypatch):
+    """No invocation to read is a skip, not a pass."""
+    monkeypatch.chdir(tmp_path)
+    _write_ci_workflow(tmp_path, _metadata_step("echo apricot"))
+    results = check_metadata_keys()
+    assert [r.passed for r in results] == [None]
+
+
+def test_metadata_value_options_match_the_command():
+    """The hand-listed value options are the ones the command really declares.
+
+    `check_metadata_keys` cannot import the CLI to ask (see
+    `METADATA_VALUE_OPTIONS`), so the list is pinned here instead: an option
+    gaining a value would otherwise make the lint read that value as a key.
+    """
+    declared = {
+        opt
+        for param in metadata_command.params
+        if not getattr(param, "is_flag", False)
+        for opt in (*param.opts, *param.secondary_opts)
+        if opt.startswith("-")
+    }
+    assert declared == set(METADATA_VALUE_OPTIONS)
+
+
+def test_metadata_keys_covers_every_workflow_of_this_repo():
+    """This repository's own workflows only ask for keys that exist."""
+    results = check_metadata_keys(Path(".github/workflows"))
+    assert results
+    assert all(r.passed is not False for r in results)
+
+
 # ---------------------------------------------------------------------------
 # Branch ruleset check tests
 # ---------------------------------------------------------------------------