chore(template-sync): sync to v1.6.1

.claude/README.md

@@ -58,7 +58,10 @@ this directory).
   Base lists each of these skills one by one in its sync manifest, so an
   instance adds its own as `skills/<name>/` and lists that directory
   (as `.claude/skills/<name>/`, or `.claude/agents/<name>.md` for an
-  agent) under `ignore` in `.github/template-sync-manifest.local.yml`. That
+  agent) under `ignore` in `.github/template-sync-manifest.local.yml`
+  (base's own entries are `replace` tier, one line per skill and agent; a
+  deleted one keeps its line as a tombstone so sync removes it from
+  instances, see `../docs/TEMPLATE.md`, "Instance extension points"). That
   entry is what keeps the completeness check passing; sync never reads
   the local file. Don't reuse a base (or removed base) skill or agent
   name, since sync would overwrite or remove it.

.github/scripts/README.md

@@ -21,8 +21,13 @@
   `../template-sync-state.json`. Its `check-apply` subcommand cross-checks
   `apply`'s exit status against the summary it wrote, so a crash that
   exits 1 is not read as merge conflicts. A merge-tier file the instance
-  never changed takes the new version outright; a binary one the instance
-  did change is reported as a conflict and left as it is. It catches the broad `apply` failure with
+  never changed takes the new version outright (content and executable bit,
+  keeping its other mode bits); a merged file keeps its own mode. A binary
+  one the instance did change is reported as a conflict and left as it is;
+  since it has no conflict markers, it is also listed in the summary's
+  `merge.binary`, labelled in the PR body and annotated with `::warning::` in
+  the job log. The workflow's apply step writes only `check-apply`'s validated
+  `conflicts=0|1` line to `$GITHUB_OUTPUT`. `apply` catches its broad failure with
   `logging.exception` rather than a `noqa`, so instances that don't
   select ruff's `BLE` rules see no unused-`noqa` finding.
   `apply` is not atomic: on exit 2, files written before the error stay on
@@ -51,7 +56,9 @@
   `.` member cannot matter: the `data` extraction filter sets a directory's
   mode to `None`, so no member restricts the staging directory mid-run (a test
   pins this). A failed run, including a collision found in a later leg,
-  discards the staging directory and leaves `dist/` untouched.
+  discards the staging directory and leaves `dist/` untouched. A corrupt or
+  truncated tarball (including a cut-off compressed one) is an `::error::`
+  naming the file, not a traceback.
   `has_make_target.sh` runs no recipe, but make still evaluates
   `$(shell ...)` and `include` while parsing the Makefile.
   `test_release_scripts.py` tests both, `verify_release_tag.sh`, and that
@@ -74,20 +81,31 @@
   supersedes the instance-level `select ALL` remarks above for these files.
   `test_script_syntax.py` parses every script with
   `ast.parse(feature_version=(3, 12))` and checks `ruff.toml` names the same
-  version. Bump both together if the runner's `python3` changes.
+  version. The runner image's `python3` is not read at test time (that would
+  need the network), so `test_script_syntax.py` also maps each runner label
+  the workflows default to (`ubuntu-24.04`) to its Python: when Renovate bumps
+  that label the test fails until you look up the new image's `python3`
+  (actions/runner-images) and update `RUNNER_IMAGE_PYTHON`, `RUNNER_PYTHON` and
+  `ruff.toml` together.
 - `verify_release_tag.sh` — the `release` job's check that an already
   published release's tag points at the commit the run built (fails closed;
   retries only transient lookup errors, six attempts over about two
   minutes, and never a missing tag or a missing token, which it recognises by
   `gh`'s own "set/populate the GH_TOKEN environment variable" message, not
-  by any stderr that names `GH_TOKEN`). Tested against a fake `gh` in
+  by any stderr that names `GH_TOKEN`). `TAG_LOOKUP_ATTEMPTS` (1-99) and `TAG_LOOKUP_DELAY` (0-9999, no leading zeros) are validated up
+  front, every attempt's stderr is logged labelled with its attempt number,
+  and a 404 is not retried even on a just-created tag: GitHub documents no
+  read-after-write lag for refs, so the error says to re-run once the tag is
+  visible. Tested against a fake `gh` in
   `test_release_scripts.py`.
 - `find_moderation_result.sh` / `test_moderation_scripts.py` — the check
   step shared by `../workflows/moderate-bug-fix-apply.yml` and
   `../workflows/moderate-feature-build-apply.yml`: asks the API for the
   `moderation-result` artifact by name (so it is found however many
   artifacts the run has) and writes `found=true|false`. It fails on an
-  expired artifact (a produced result that can no longer be applied), an
+  expired artifact (a produced result that can no longer be applied; the
+  message says to re-run the worker run, since re-running the apply run
+  cannot bring the artifact back), an
   API error, and an empty or unrecognised response. The tests run it
   against a fake `gh`.
 - `run_claude.sh` — invokes `claude -p` for one issue-moderation stage

.github/scripts/assemble_dist.py

@@ -23,12 +23,15 @@
 
 from __future__ import annotations
 
+import gzip
 import json
+import lzma
 import re
 import shutil
 import sys
 import tarfile
 import tempfile
+import zlib
 from pathlib import Path, PurePosixPath
 from typing import TYPE_CHECKING
 
@@ -39,6 +42,10 @@
 _ARCH = re.compile(r"[A-Za-z0-9][A-Za-z0-9._-]*")
 
 
+# What reading a corrupt or truncated (compressed) tarball raises .
+_CORRUPT = (tarfile.TarError, EOFError, zlib.error, lzma.LZMAError, gzip.BadGzipFile)
+
+
 class AssembleError(Exception):
     """ARCHES_JSON is invalid, a tarball is missing, or two legs collide on a path."""
 
@@ -150,18 +157,32 @@ def assemble(arches: list[str], parts: Path, dist: Path) -> None:
     stage = Path(tempfile.mkdtemp(prefix=f".{dist.name}.", dir=dist.parent))
     try:
         for arch, tarball in zip(names, tarballs, strict=True):
-            with tarfile.open(tarball) as tar:
-                # The "data" filter refuses unsafe links and absolute paths.
-                try:
-                    tar.extractall(
-                        stage, members=_checked_members(tar, arch, leaves, dirs), filter="data"
-                    )
-                except tarfile.FilterError as exc:
-                    msg = (
-                        f"{tarball.name}: {exc}; release assets may only contain regular "
-                        "files, directories and links that stay inside dist/"
-                    )
-                    raise AssembleError(msg) from exc
+            try:
+                with tarfile.open(tarball) as tar:
+                    # The "data" filter refuses unsafe links and absolute paths.
+                    try:
+                        tar.extractall(
+                            stage, members=_checked_members(tar, arch, leaves, dirs), filter="data"
+                        )
+                    except tarfile.FilterError as exc:
+                        msg = (
+                            f"{tarball.name}: {exc}; release assets may only contain regular "
+                            "files, directories and links that stay inside dist/"
+                        )
+                        raise AssembleError(msg) from exc
+            except _CORRUPT as exc:
+                # The collision check reads the tarball lazily, so a corrupt or
+                # truncated one surfaces here, deep inside extraction.
+                msg = (
+                    f"could not read {tarball.name} ({type(exc).__name__}: {exc}); "
+                    "it may be corrupt or truncated"
+                )
+                raise AssembleError(msg) from exc
+            except OSError as exc:
+                # bz2 corruption is a bare OSError; so is a full disk. Name the file
+                # without claiming which.
+                msg = f"{tarball.name}: {exc}"
+                raise AssembleError(msg) from exc
         # mkdtemp creates 0700, and dist/ must be readable to the other user the
         # devcontainer runs as. The "data" filter sets every directory's mode to
         # None, so a "." member (`tar -C dist -cf x.tar .`) never restricts the

.github/scripts/find_moderation_result.sh

@@ -24,7 +24,8 @@ state=$(gh api "repos/${repo}/actions/runs/${run_id}/artifacts?name=moderation-r
 case "$state" in
   found) found=true ;;
   expired)
-    echo "::error::The moderation-result artifact of run ${run_id} has expired before it was applied; re-run the worker workflow." >&2
+    run_url="${GITHUB_SERVER_URL:-https://github.com}/${repo}/actions/runs/${run_id}"
+    echo "::error::The moderation-result artifact of run ${run_id} expired before it was applied, so this result is lost and nothing was changed. Re-running this apply run cannot bring it back. To redo it, re-run the worker run (${run_url}) with Re-run all jobs; its fresh artifact triggers a new apply run." >&2
     exit 1
     ;;
   missing)

.github/scripts/ruff.toml

@@ -15,7 +15,8 @@
 # `.github/scripts/` entry in ../template-sync-manifest.yml).
 #
 # Keep `target-version` equal to the runner's `python3` minor version.
-# test_script_syntax.py parses every script against that version.
+# test_script_syntax.py parses every script against that version and fails
+# when the workflows' default runner label changes, so this is not forgotten.
 
 target-version = "py312"
 line-length = 100

.github/scripts/template_sync.py

@@ -304,9 +304,15 @@ def apply_merge(
     ``merged`` lists only files whose content actually changed, so running the
     same step again (an instance already at the new version) reports nothing.
     A path the instance links elsewhere is left alone, as in the replace tier.
+    ``binary`` lists the conflicts that are binary files (a subset of
+    ``conflicts``): they get no conflict markers, so nothing in the tree shows
+    them. A merged existing file keeps its own mode; one the instance never
+    changed takes the new version's content and executable bit, as in the
+    replace tier, and keeps its other mode bits.
     """
     merged: list[str] = []
     conflicts: list[str] = []
+    binary: list[str] = []
     for path in sorted(paths):
         new_file = new_root / path
         old_file = old_root / path
@@ -320,6 +326,8 @@ def apply_merge(
             shutil.copy2(new_file, instance_file)
             merged.append(path)
             continue
+        # Probed up front, the same way merge_file decides to decline a binary file.
+        binary_input = any(is_binary(p) for p in (instance_file, old_file, new_file))
         base_content = old_file.read_bytes() if old_file.exists() else b""
         new_content = new_file.read_bytes()
         if new_content == base_content:
@@ -330,7 +338,11 @@ def apply_merge(
         if old_file.exists() and before == base_content:
             # Untouched by the instance: take the new version outright. Git
             # would do the same for text; a binary file can only be merged this way.
-            shutil.copy2(new_file, instance_file)
+            # Content and the executable bit, as in the replace tier, but not the
+            # template's other mode bits (copy2 would overwrite them).
+            shutil.copyfile(new_file, instance_file)
+            if is_executable(new_file) != is_executable(instance_file):
+                sync_executable_bit(new_file, instance_file)
             merged.append(path)
             continue
         if old_file.exists():
@@ -342,9 +354,17 @@ def apply_merge(
                 conflicted = merge_file(instance_file, Path(empty_base.name), new_file)
         if conflicted:
             conflicts.append(path)
+            if binary_input:
+                binary.append(path)
         elif instance_file.read_bytes() != before:
             merged.append(path)
-    return {"merged": merged, "conflicts": conflicts}
+    return {"merged": merged, "conflicts": conflicts, "binary": binary}
+
+
+def _annotation_escape(value: str, *, prop: bool = False) -> str:
+    """Percent-encode ``value`` for a GitHub workflow command (property or message)."""
+    value = value.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A")
+    return value.replace(":", "%3A").replace(",", "%2C") if prop else value
 
 
 def cmd_apply(args: argparse.Namespace) -> int:
@@ -377,6 +397,13 @@ def cmd_apply(args: argparse.Namespace) -> int:
         log.exception("apply failed")
         print(f"error: {type(err).__name__}: {err}", file=sys.stderr)
         return EXIT_ERROR
+    for path in summary["merge"].get("binary", []):
+        # No markers in a binary file, so say it where the job log is read.
+        print(
+            f"::warning file={_annotation_escape(path, prop=True)}::binary merge conflict, "
+            "left at the instance's version; take the template's new version by hand",
+            file=sys.stderr,
+        )
     print(json.dumps(summary, indent=2))
     return EXIT_CONFLICTS if summary["merge"]["conflicts"] else 0
 
@@ -400,7 +427,8 @@ def cmd_check_apply(args: argparse.Namespace) -> int:
         agrees = isinstance(conflicts, list) and bool(conflicts) == (
             args.exit_code == EXIT_CONFLICTS
         )
-    except (OSError, ValueError, KeyError, TypeError):
+    except (OSError, ValueError, KeyError, TypeError, RecursionError):
+        # RecursionError: json.loads on a pathologically nested file.
         agrees = False
     if not agrees:
         print(
@@ -422,14 +450,20 @@ def pr_body(summary: dict[str, dict[str, list[str]]], old_tag: str, new_tag: str
         lines.append(f"- replace (removed upstream): `{path}`")
     for path in summary["merge"]["merged"]:
         lines.append(f"- merge (clean): `{path}`")
+    binary = set(summary["merge"].get("binary", []))
     for path in summary["merge"]["conflicts"]:
-        lines.append(f"- merge (**conflict, needs manual resolution**): `{path}`")
+        if path in binary:
+            lines.append(
+                f"- merge (**binary conflict, no markers; kept at this repo's version, "
+                f"take the new one from the template's new tag by hand**): `{path}`"
+            )
+        else:
+            lines.append(f"- merge (**conflict, needs manual resolution**): `{path}`")
     if summary["merge"]["conflicts"]:
         lines += [
             "",
-            "One or more files above have `<<<<<<<` conflict markers to resolve before merging "
-            "(a binary file has none: it is left at the instance's version, so take the new "
-            "one from the template's new tag).",
+            "Resolve every conflict above before merging: text files have `<<<<<<<` markers; "
+            "a binary file has none (it is left at the instance's version).",
         ]
     if "pyproject.toml" in summary["merge"]["merged"] + summary["merge"]["conflicts"]:
         lines += [

.github/scripts/test_moderation_scripts.py

@@ -89,6 +89,10 @@ def test_expired_artifact_fails_the_script(self) -> None:
         self.assertIn("::error::", result.stderr)
         self.assertIn("expired", result.stderr)
         self.assertFalse((self.tmp / "output.txt").exists())
+        # Actionable: names what to re-run, and that re-running the apply run won't help.
+        self.assertIn("Re-run all jobs", result.stderr)
+        self.assertIn("/actions/runs/", result.stderr)
+        self.assertIn("cannot bring it back", result.stderr)
 
     def test_live_artifact_wins_over_an_expired_one(self) -> None:
         """A re-run's fresh artifact is found even if an older one expired."""