Improve stiff from downstream repo

Author: kdeldycke

.github/workflows/autofix.yaml

@@ -187,10 +187,12 @@ jobs:
           version: "0.12.2"
       - name: Run pyproject-fmt
         # pyproject-fmt returns exit code 1 when it reformats the file, which is
-        # the expected outcome in an autofix workflow. No file list: the tool
-        # runner resolves it, which also drops the xargs that used to translate
-        # that 1 into a 123. See `ToolSpec.default_paths` in
-        # repomatic/tool_registry.py.
+        # the expected outcome in an autofix workflow. Tolerating it is only safe
+        # because the tool runner checks the files too: a crash that exits 1
+        # without rewriting anything arrives here as 70 and fails the job. See
+        # `ToolSpec.rewrite_exit_code`. No file list: the tool runner resolves it,
+        # which also drops the xargs that used to translate that 1 into a 123.
+        # See `ToolSpec.default_paths` in repomatic/tool_registry.py.
         run: |
           rc=0
           uv --no-progress run --frozen -- repomatic run pyproject-fmt || rc=$?

changelog.md

@@ -15,6 +15,9 @@
 - Fix `gh` re-downloading on every single command instead of once per version: a registry binary whose archive nests its executable in a subdirectory was stored under one cache key and looked up under another, so the cache never hit. `7.11.0` routed every GitHub call through the pinned binary, making it a 13 MB fetch per invocation.
 - Fix `repomatic init` realigning a workflow's inline `repomatic==X.Y.Z` pin without the cooldown exemption beside it, leaving a command that cannot resolve the version just written. `--no-cooldown` hit this on every run.
 - `repomatic run actionlint` now ships a bundled config declaring the `ubuntu-26.04` runner labels actionlint `1.7.12` predates, so the Lint job no longer fails on the `runs-on:` values repomatic itself generates.
+- `repomatic init` now warns when a workflow asks `metadata` for a key the adopted version no longer emits, instead of leaving the mismatch to fail the job every other job hangs off.
+- New `ToolSpec.rewrite_exit_code` naming the status a formatter returns after rewriting a file. A run exiting with it while leaving every target unchanged is reported as a crash (exit code `70`) instead of passing for a successful reformat: pyproject-fmt uses `1` for both, so its panics reached the autofix job as green runs that formatted nothing.
+- `repomatic run --verify` now surfaces a tool that failed on the throwaway copies, instead of reading the unformatted result as an absence of drift.
 - Fix the `[tool.repomatic.workflow]` key names the `repomatic-audit` skill recommends: they are `extra-paths` and `ignore-paths`, not the snake_case attribute names.
 - Every job now caps its runtime with `timeout-minutes`, so a hung job frees its runner in minutes instead of holding it for the platform's 6-hour ceiling. Downstream callers inherit the caps.
 - The bundled `lychee.toml` now excludes `bitdefender.com`, `npmjs.com`, `star-history.com` and `githubstatus.com`, which answer bots with 403, 405 or JavaScript rather than a link.

repomatic/init_project.py

@@ -35,6 +35,7 @@
 import hashlib
 import logging
 import re
+import shlex
 import sys
 from dataclasses import dataclass, field
 from datetime import date, datetime, timedelta, timezone
@@ -44,6 +45,8 @@
 from urllib.request import Request, urlopen
 
 import tomlrt
+import yaml
+from packaging.version import InvalidVersion, Version
 
 from . import __git_tag_sha__, __version__
 from .bundle import get_data_content
@@ -57,7 +60,7 @@
     render_thin_caller_for_target,
 )
 from .http import DEFAULT_TIMEOUT
-from .metadata import Metadata
+from .metadata import Metadata, all_metadata_keys
 from .plugin import merge_plugin_settings
 from .prepare_release import SELF_PIN_COOLDOWN_EXEMPTION
 from .pyproject import is_python_package, is_python_project, resolve_source_paths
@@ -1320,6 +1323,8 @@ def _init_workflows(
         )
 
     _realign_inline_pins(workflows_dir, version, result, output_dir, repo=repo)
+    # Runs after the realignment, so it reads the pin the workflows now carry.
+    _check_metadata_keys(workflows_dir, result, output_dir, version, repo=repo)
 
 
 def _realign_inline_pins(
@@ -1396,6 +1401,154 @@ def _realign_inline_pins(
         )
 
 
+def _run_commands(workflow: Path) -> list[str]:
+    """Collect every `run:` script in a workflow file.
+
+    :param workflow: Path to a workflow YAML file.
+    :return: One entry per step carrying a `run:`. Empty when the file does not
+        parse as a workflow, which is not this function's problem to report.
+    """
+    try:
+        data = yaml.safe_load(workflow.read_text(encoding="UTF-8"))
+    except (yaml.YAMLError, OSError):
+        return []
+    if not isinstance(data, dict):
+        return []
+    commands = []
+    for job in data.get("jobs", {}).values():
+        if not isinstance(job, dict):
+            continue
+        for step in job.get("steps") or []:
+            if isinstance(step, dict) and isinstance(step.get("run"), str):
+                commands.append(step["run"])
+    return commands
+
+
+def _requested_metadata_keys(command: str, package: str) -> list[str]:
+    """Extract the metadata keys a shell command asks for.
+
+    ```{note}
+    Deliberately conservative: a token is only read as a key when it is
+    lowercase snake_case and does not follow a bare option, which could be that
+    option's value. A boolean option ahead of the first key therefore hides it.
+    Under-reporting costs a missed warning; over-reporting would blame a
+    perfectly good workflow, and this runs on files the user did not write.
+    ```
+
+    :param command: One step's `run:` script.
+    :param package: Upstream package name, as it appears in the invocation.
+    :return: The key arguments, in the order they appear.
+    """
+    try:
+        tokens = shlex.split(command)
+    except ValueError:
+        # Unbalanced quotes: a shell script this function has no business
+        # second-guessing.
+        return []
+
+    keys = []
+    seen_package = False
+    after_subcommand = False
+    for index, token in enumerate(tokens):
+        previous = tokens[index - 1] if index else ""
+        if package in token:
+            seen_package = True
+        elif token == "metadata" and seen_package:
+            after_subcommand = True
+        # Outside the subcommand's arguments, or looking at an option or at the
+        # value of one written apart from it: none of these can be a key.
+        elif (
+            not after_subcommand
+            or token.startswith("-")
+            or (previous.startswith("-") and "=" not in previous)
+        ):
+            continue
+        elif re.fullmatch(r"[a-z][a-z0-9_]*", token):
+            keys.append(token)
+        else:
+            # A value that cannot be a key: the invocation moved on to
+            # something else (a shell operator, a path, another command).
+            after_subcommand = False
+    return keys
+
+
+def _check_metadata_keys(
+    workflows_dir: Path,
+    result: InitResult,
+    output_dir: Path,
+    version: str,
+    *,
+    repo: str = DEFAULT_REPO,
+) -> None:
+    """Warn when a workflow asks `metadata` for a key this version dropped.
+
+    A workflow reaches the toolkit's metadata through a `run:` command naming
+    the keys it wants, and an unknown key is a hard `UsageError`. So retiring a
+    key breaks every downstream workflow still asking for it, at the first push
+    after the adoption, in the job every other job hangs off: click-extra's
+    whole test workflow went dark that way when `coverage_cells` left with the
+    Codecov integration.
+
+    `init` is where the version changes, and the job bodies holding these
+    commands belong to the downstream repository, so header-only sync never
+    reads them. That makes this the one moment the mismatch is both introduced
+    and fixable, ahead of the commit rather than after the red run.
+
+    Scope is every workflow file, matching {func}`_realign_inline_pins`: the
+    invocation hides in a job body wherever the repository chose to put it.
+
+    ```{note}
+    The key set is the running version's, read in-process, so the check applies
+    only when the workflows end up pinned to that same version. A cooldown
+    holding the pin back leaves the answer unknowable from here: the older
+    release's key set is not importable, and judging it by this one's would
+    blame a workflow that works.
+    ```
+
+    :param workflows_dir: The repository's `.github/workflows/` directory.
+    :param result: {class}`InitResult` accumulator, mutated in place.
+    :param output_dir: Repository root, for the reported relative path.
+    :param version: Version just pinned into the workflows, `vX.Y.Z` spelling.
+    :param repo: Upstream `owner/repo`; its name is the package to match.
+    """
+    # Compared on the release tuple, so a development build checks the release
+    # it is heading for: the pin drops the `.devN` suffix the running version
+    # carries, and string equality would skip the check on every dev checkout.
+    try:
+        same_release = (
+            Version(version.removeprefix("v")).release == Version(__version__).release
+        )
+    except InvalidVersion:
+        same_release = False
+    if not same_release:
+        logging.debug(
+            "Workflows pin %s, not the running %s: skipping the metadata-key "
+            "check, whose key set is this version's.",
+            version,
+            __version__,
+        )
+        return
+
+    package = repo.rsplit("/", 1)[-1]
+    valid_keys = all_metadata_keys()
+    for target in sorted(workflows_dir.glob("*.y*ml")):
+        unknown = sorted({
+            key
+            for command in _run_commands(target)
+            for key in _requested_metadata_keys(command, package)
+            if key not in valid_keys
+        })
+        if not unknown:
+            continue
+        result.warnings.append(
+            f"{_relative_label(target, output_dir)} asks `{package} metadata` "
+            f"for {', '.join(repr(key) for key in unknown)}, which "
+            f"{package} {__version__} does not emit. The job will fail; drop "
+            "the key or pin an older version."
+        )
+        logging.warning(result.warnings[-1])
+
+
 def is_source_repo(output_dir: Path) -> bool:
     """Detect whether `output_dir` is the repomatic source repository root.
 

repomatic/tool_registry.py

@@ -781,6 +781,26 @@ class ToolSpec:
     ```
     """
 
+    rewrite_exit_code: int | None = None
+    """Exit code the tool returns when it rewrote at least one file.
+
+    Formatters that signal "I reformatted something" with a non-zero status
+    force every caller to tolerate that code, which is what lets a crash pass
+    for a success: pyproject-fmt exits `1` both when it reformats a file and
+    when it dies on a `PanicException`, and the autofix job cannot tell the two
+    apart from the status alone.
+
+    Declaring the code here gives `run_tool` the second signal it needs: the
+    files themselves. A run exiting with this code and leaving every target
+    byte-identical contradicts what the code claims, so it is reported as a
+    failure instead of being waved through. See
+    {data}`repomatic.tool_runner.TOOL_CRASH_EXIT_CODE`.
+
+    `None` for tools with no such convention, which is most of them: a
+    formatter that exits `0` whether or not it wrote anything needs no
+    disambiguation.
+    """
+
     binary: BinarySpec | None = None
     """Platform-specific binary download spec. When set, the tool is downloaded
     as a binary instead of installed via `uvx` or `uv run`.
@@ -1822,6 +1842,7 @@ def _fix_myst_directives(extra_args: Sequence[str]) -> None:
         config_flag="--config",
         native_format=NativeFormat.TOML,
         reads_pyproject=True,
+        rewrite_exit_code=1,
         docs_notes=cleandoc(r"""
             **Try it:**
 

repomatic/tool_runner.py

@@ -1077,6 +1077,41 @@ def resolve_default_args(spec: ToolSpec) -> list[list[str]] | None:
     return [[*base, *paths]]
 
 
+TOOL_CRASH_EXIT_CODE = 70
+"""Exit code reported when a tool contradicts its own rewrite status.
+
+`EX_SOFTWARE` from `sysexits.h`: an internal error in the tool being run.
+Deliberately outside the set a formatter's caller tolerates, so a crash cannot
+land on the code that means "I reformatted a file". See
+{attr}`~repomatic.tool_registry.ToolSpec.rewrite_exit_code`.
+"""
+
+
+def _digest_targets(args: Sequence[str]) -> dict[str, str]:
+    """Digest every existing file among a batch's arguments.
+
+    Directories are walked, so a tool pointed at a tree is covered too.
+    Non-path arguments (flags, their values) simply do not exist on disk and
+    are skipped, which needs no flag parsing: a flag that happens to name a
+    real file is digested harmlessly.
+
+    :param args: One invocation's arguments.
+    :return: Path string mapped to the SHA-256 of its content.
+    """
+    digests: dict[str, str] = {}
+    for arg in args:
+        path = Path(arg)
+        if path.is_file():
+            candidates = [path]
+        elif path.is_dir():
+            candidates = [child for child in path.rglob("*") if child.is_file()]
+        else:
+            continue
+        for candidate in candidates:
+            digests[str(candidate)] = compute_file_sha256(candidate)
+    return digests
+
+
 def run_tool(
     name: str,
     extra_args: Sequence[str] = (),
@@ -1101,7 +1136,10 @@ def run_tool(
     :param skip_checksum: Skip SHA-256 verification entirely.
     :param no_cache: Bypass the binary cache when `True`.
     :return: The tool's exit code; the first non-zero one when the defaults
-        resolved to several invocations.
+        resolved to several invocations, or {data}`TOOL_CRASH_EXIT_CODE` when a
+        tool declaring
+        {attr}`~repomatic.tool_registry.ToolSpec.rewrite_exit_code` reports a
+        rewrite it did not perform.
     """
     if name not in TOOL_REGISTRY:
         msg = (
@@ -1209,11 +1247,36 @@ def run_tool(
                         continue
                     destination.parent.mkdir(parents=True, exist_ok=True)
 
+            # Snapshot the targets when the tool reports rewrites through its
+            # exit code, so the claim can be checked against the files below.
+            before = (
+                _digest_targets(batch) if spec.rewrite_exit_code is not None else {}
+            )
+
             logging.info("Running: %s", " ".join(cmd))
             result = subprocess.run(cmd, check=False, env=env)
 
             logging.info("%s exited with code %d.", spec.name, result.returncode)
 
+            # A rewrite status that rewrote nothing is a contradiction, and the
+            # shape a crash takes in a tool whose caller has to tolerate that
+            # status: pyproject-fmt exits 1 on a PanicException exactly as it
+            # does on a successful reformat. Trust the files over the code.
+            crashed = (
+                spec.rewrite_exit_code is not None
+                and result.returncode == spec.rewrite_exit_code
+                and _digest_targets(batch) == before
+            )
+            if crashed:
+                logging.error(
+                    "%s exited with code %d, which it uses to report rewritten "
+                    "files, but left every target unchanged. Treating it as a "
+                    "crash and reporting %d; re-run it directly to see why.",
+                    spec.name,
+                    result.returncode,
+                    TOOL_CRASH_EXIT_CODE,
+                )
+
             # post_process is a write-mode-only fixup: it rewrites files on
             # disk, so it runs only after a successful write (return code 0),
             # never in check/dry-run mode, which writes nothing. The warning
@@ -1240,8 +1303,9 @@ def run_tool(
 
             # Keep going after a failure, the way `xargs` does, but never let a
             # later success overwrite an earlier failure.
-            if result.returncode != 0 and exit_code == 0:
-                exit_code = result.returncode
+            batch_code = TOOL_CRASH_EXIT_CODE if crashed else result.returncode
+            if batch_code != 0 and exit_code == 0:
+                exit_code = batch_code
 
         return exit_code
 
@@ -1290,7 +1354,8 @@ def verify_via_write_path(
     :param run_kwargs: Forwarded verbatim to {func}`run_tool`.
     :return: `(exit_code, drifted)`, where `exit_code` is `0` when every target
         is already formatted and `1` otherwise, and `drifted` names the paths
-        the write path would have changed.
+        the write path would have changed. A tool that fails on the copies
+        yields its own exit code and no drift, since it measured nothing.
     """
     spec = TOOL_REGISTRY[name]
     if not extra_args:
@@ -1346,7 +1411,20 @@ def verify_via_write_path(
             )
             return 0, []
 
-        run_tool(name, extra_args=rewritten, **run_kwargs)
+        # A tool that died on the copies formatted nothing, so the comparison
+        # below would find no difference and report the tree as clean. Surface
+        # the failure instead of laundering it into a passing verdict. The
+        # rewrite status is the one non-zero code that means success here: it
+        # is what a formatter returns after reformatting a copy.
+        run_code = run_tool(name, extra_args=rewritten, **run_kwargs)
+        if run_code not in {0, spec.rewrite_exit_code}:
+            logging.error(
+                "%s failed with code %d on the throwaway copies; its formatting "
+                "cannot be verified.",
+                name,
+                run_code,
+            )
+            return run_code, []
 
         drifted = sorted(
             str(source_file)