From 26b842c5a3b0a0fc2af1afff6af55c84961c5965 Mon Sep 17 00:00:00 2001 From: AJ Kerrigan Date: Thu, 10 Sep 2026 10:30:38 -0400 Subject: [PATCH 1/6] feat: add a justfile recipe to bump the sinistral-cli version - Default to latest - Allow & resolve other target gitrefs - Update the default input value and README at the same time --- justfile | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/justfile b/justfile index ed3be35..6606c94 100644 --- a/justfile +++ b/justfile @@ -13,3 +13,59 @@ format: # Run script unit tests test: uv run pytest -v + +# Bump the default sinistral_cli_version (ref: tag, branch, or SHA; default latest release) +bump-cli-version ref="latest": + #!/usr/bin/env bash + set -euo pipefail + repo=stacklet/sinistral-cli + ref="{{ref}}" + + if [ "$ref" = "latest" ]; then + ref="$(gh api "repos/${repo}/releases/latest" --jq .tag_name)" + fi + + # One call resolves any ref (tag, branch, full or short SHA) to its commit. + read -r sha date < <(gh api "repos/${repo}/commits/${ref}" \ + --jq '[.sha, (.commit.committer.date | split("T")[0])] | @tsv') + + # Prefer a tag pointing at that exact commit for the human-readable comment; + # otherwise fall back to what was asked for, dated so it stays meaningful. + tags="$(gh api --paginate "repos/${repo}/tags?per_page=100" \ + --jq ".[] | select(.commit.sha == \"${sha}\") | .name")" + name="${tags%%$'\n'*}" + if [ -z "$name" ]; then + case "$sha" in + "$ref"*) name="${sha:0:7} (${date})" ;; + *) name="${ref} (${date})" ;; + esac + fi + + # Ref names allow characters that are special to sed's replacement side. + esc="${name//\\/\\\\}"; esc="${esc//&/\\&}"; esc="${esc//@/\\@}" + + # action.yml and README.md are checked independently: either can be stale on + # its own, so neither is skipped just because the other is already correct. + changed=() + + cur_sha=""; cur_name="" + read -r cur_sha cur_name < <(sed -n -E \ + "/^ sinistral_cli_version:/,/^ [a-z_]+:/ s@^ +default: '([^']*)'( *# *(.*))?\$@\1\t\3@p" \ + action.yml) || true + if [ "$cur_sha" != "$sha" ] || [ "$cur_name" != "$name" ]; then + sed -i -E "/^ sinistral_cli_version:/,/^ [a-z_]+:/ s@^( +default: ).*\$@\1'${sha}' # ${esc}@" action.yml + changed+=(action.yml) + fi + + cur_doc="$(sed -n -E 's@^\| `sinistral_cli_version` \| No \| `([^`]*)`.*@\1@p' README.md)" + if [ "$cur_doc" != "$name" ]; then + sed -i -E "s@(\| \`sinistral_cli_version\` \| No \| \`)[^\`]*(\`)@\1${esc}\2@" README.md + changed+=(README.md) + fi + + if [ ${#changed[@]} -eq 0 ]; then + echo "Already pinned to ${sha} # ${name}; action.yml and README.md both current" + else + echo "Pinned to ${sha} # ${name}" + printf 'updated: %s\n' "${changed[*]}" + fi From 3c5dc3ab5bf8a0458593a798597fcd1c2009724c Mon Sep 17 00:00:00 2001 From: AJ Kerrigan Date: Thu, 10 Sep 2026 10:40:16 -0400 Subject: [PATCH 2/6] chore: bump default sinistral_cli_version --- README.md | 2 +- action.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4c6c01a..472d7ac 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ Set `skip_unchanged: 'false'` to always scan: | `sinistral_project` | Yes | — | Sinistral project name to scan against. | | `iac_directories` | Yes | — | Path(s) to IaC folders relative to the repo root. Newline-separated. | | `recurse` | No | `false` | Recursively discover subdirectories containing `.tf` files. | -| `sinistral_cli_version` | No | `v0.5.34` | Git ref (tag, branch, SHA) of [sinistral-cli](https://github.com/stacklet/sinistral-cli). | +| `sinistral_cli_version` | No | `v0.5.38` | Git ref (tag, branch, SHA) of [sinistral-cli](https://github.com/stacklet/sinistral-cli). | | `post_pr_comment` | No | `true` | Whether to post scan results as a PR comment. | | `skip_unchanged` | No | `true` | Skip the scan when no Terraform files under `iac_directories` changed. Fails open. See [Skipping unchanged runs](#skipping-unchanged-runs). | diff --git a/action.yml b/action.yml index 8380fd0..7726c50 100644 --- a/action.yml +++ b/action.yml @@ -34,7 +34,7 @@ inputs: description: > sinistral-cli version to use. Accepts any git ref (branch, tag, SHA) from https://github.com/stacklet/sinistral-cli. - default: '8552c68f72b9df5bb4985f722fec4063ebb35218' # v0.5.36 + default: 'dbefcee8c50ad4300be1236e10dc613bd9c2f3dc' # v0.5.38 post_pr_comment: required: false description: Whether to post scan results as a PR comment. From cfbc2cbb619bef7e8e17e9a6ddcc6d8d30671ec2 Mon Sep 17 00:00:00 2001 From: AJ Kerrigan Date: Thu, 10 Sep 2026 11:06:49 -0400 Subject: [PATCH 3/6] fix: make bump-cli-version portable and quote the ref argument Addresses cubic review feedback on #11: - `sed -i` is not portable: BSD sed reads the following argument as a backup suffix, so `sed -i -E` silently loses ERE mode on macOS. Edits now go through a temp file that is copied back, preserving the target's inode and permissions. - Drop the `\t` separator from the extraction expression, since BSD sed emits a literal `t` there rather than a tab. - Quote the just parameter with `{{quote(ref)}}`; refs may legitimately contain `$()`, backticks or quotes, which were previously interpolated straight into the recipe's bash source. - Track updated files in a string rather than an array, which bash 3.2 on macOS treats as unset under `set -u`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VBgEPS6veg5TYY4D48QgXb --- justfile | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/justfile b/justfile index 6606c94..28931f4 100644 --- a/justfile +++ b/justfile @@ -19,7 +19,17 @@ bump-cli-version ref="latest": #!/usr/bin/env bash set -euo pipefail repo=stacklet/sinistral-cli - ref="{{ref}}" + ref={{quote(ref)}} + + # `sed -i` is not portable: BSD sed reads the next argument as a backup + # suffix. Write through a temp file, then copy back so the target keeps its + # own inode and permissions. + edit() { + local expr="$1" file="$2" tmp + tmp="$(mktemp "${file}.XXXXXX")" + sed -E "$expr" "$file" > "$tmp" && cat "$tmp" > "$file" + rm -f "$tmp" + } if [ "$ref" = "latest" ]; then ref="$(gh api "repos/${repo}/releases/latest" --jq .tag_name)" @@ -46,26 +56,26 @@ bump-cli-version ref="latest": # action.yml and README.md are checked independently: either can be stale on # its own, so neither is skipped just because the other is already correct. - changed=() + changed="" cur_sha=""; cur_name="" read -r cur_sha cur_name < <(sed -n -E \ - "/^ sinistral_cli_version:/,/^ [a-z_]+:/ s@^ +default: '([^']*)'( *# *(.*))?\$@\1\t\3@p" \ + "/^ sinistral_cli_version:/,/^ [a-z_]+:/ s@^ +default: '([^']*)'( *# *(.*))?\$@\1 \3@p" \ action.yml) || true if [ "$cur_sha" != "$sha" ] || [ "$cur_name" != "$name" ]; then - sed -i -E "/^ sinistral_cli_version:/,/^ [a-z_]+:/ s@^( +default: ).*\$@\1'${sha}' # ${esc}@" action.yml - changed+=(action.yml) + edit "/^ sinistral_cli_version:/,/^ [a-z_]+:/ s@^( +default: ).*\$@\1'${sha}' # ${esc}@" action.yml + changed="${changed}action.yml " fi cur_doc="$(sed -n -E 's@^\| `sinistral_cli_version` \| No \| `([^`]*)`.*@\1@p' README.md)" if [ "$cur_doc" != "$name" ]; then - sed -i -E "s@(\| \`sinistral_cli_version\` \| No \| \`)[^\`]*(\`)@\1${esc}\2@" README.md - changed+=(README.md) + edit "s@(\| \`sinistral_cli_version\` \| No \| \`)[^\`]*(\`)@\1${esc}\2@" README.md + changed="${changed}README.md " fi - if [ ${#changed[@]} -eq 0 ]; then + if [ -z "$changed" ]; then echo "Already pinned to ${sha} # ${name}; action.yml and README.md both current" else echo "Pinned to ${sha} # ${name}" - printf 'updated: %s\n' "${changed[*]}" + echo "updated: ${changed% }" fi From 629318656a7f773bd0faf2fc64d42deb018119d0 Mon Sep 17 00:00:00 2001 From: AJ Kerrigan Date: Thu, 10 Sep 2026 11:33:54 -0400 Subject: [PATCH 4/6] refactor: rewrite the cli version bump as a python script The recipe had grown past what shell comfortably carries: the pin lives inside one input's block in action.yml and is repeated in the README table, so locating and rewriting it meant range-scoped sed expressions whose replacement text had to be escaped for sed's own metacharacters, with a separate portability layer because BSD and GNU sed disagree about in-place editing. Moving it to scripts/ follows what this repo already does with Python, and the parts worth trusting are now pure text transforms with tests around them: which line the pin lives on, what a commit is named when no tag points at it, and which files are actually out of step. Behaviour is unchanged, including per-file staleness checks and `latest` defaulting. Co-Authored-By: Claude Opus 5 (1M context) --- justfile | 64 +------- scripts/bump_cli_version.py | 232 ++++++++++++++++++++++++++ tests/test_bump_cli_version.py | 289 +++++++++++++++++++++++++++++++++ 3 files changed, 522 insertions(+), 63 deletions(-) create mode 100644 scripts/bump_cli_version.py create mode 100644 tests/test_bump_cli_version.py diff --git a/justfile b/justfile index 28931f4..c48af84 100644 --- a/justfile +++ b/justfile @@ -16,66 +16,4 @@ test: # Bump the default sinistral_cli_version (ref: tag, branch, or SHA; default latest release) bump-cli-version ref="latest": - #!/usr/bin/env bash - set -euo pipefail - repo=stacklet/sinistral-cli - ref={{quote(ref)}} - - # `sed -i` is not portable: BSD sed reads the next argument as a backup - # suffix. Write through a temp file, then copy back so the target keeps its - # own inode and permissions. - edit() { - local expr="$1" file="$2" tmp - tmp="$(mktemp "${file}.XXXXXX")" - sed -E "$expr" "$file" > "$tmp" && cat "$tmp" > "$file" - rm -f "$tmp" - } - - if [ "$ref" = "latest" ]; then - ref="$(gh api "repos/${repo}/releases/latest" --jq .tag_name)" - fi - - # One call resolves any ref (tag, branch, full or short SHA) to its commit. - read -r sha date < <(gh api "repos/${repo}/commits/${ref}" \ - --jq '[.sha, (.commit.committer.date | split("T")[0])] | @tsv') - - # Prefer a tag pointing at that exact commit for the human-readable comment; - # otherwise fall back to what was asked for, dated so it stays meaningful. - tags="$(gh api --paginate "repos/${repo}/tags?per_page=100" \ - --jq ".[] | select(.commit.sha == \"${sha}\") | .name")" - name="${tags%%$'\n'*}" - if [ -z "$name" ]; then - case "$sha" in - "$ref"*) name="${sha:0:7} (${date})" ;; - *) name="${ref} (${date})" ;; - esac - fi - - # Ref names allow characters that are special to sed's replacement side. - esc="${name//\\/\\\\}"; esc="${esc//&/\\&}"; esc="${esc//@/\\@}" - - # action.yml and README.md are checked independently: either can be stale on - # its own, so neither is skipped just because the other is already correct. - changed="" - - cur_sha=""; cur_name="" - read -r cur_sha cur_name < <(sed -n -E \ - "/^ sinistral_cli_version:/,/^ [a-z_]+:/ s@^ +default: '([^']*)'( *# *(.*))?\$@\1 \3@p" \ - action.yml) || true - if [ "$cur_sha" != "$sha" ] || [ "$cur_name" != "$name" ]; then - edit "/^ sinistral_cli_version:/,/^ [a-z_]+:/ s@^( +default: ).*\$@\1'${sha}' # ${esc}@" action.yml - changed="${changed}action.yml " - fi - - cur_doc="$(sed -n -E 's@^\| `sinistral_cli_version` \| No \| `([^`]*)`.*@\1@p' README.md)" - if [ "$cur_doc" != "$name" ]; then - edit "s@(\| \`sinistral_cli_version\` \| No \| \`)[^\`]*(\`)@\1${esc}\2@" README.md - changed="${changed}README.md " - fi - - if [ -z "$changed" ]; then - echo "Already pinned to ${sha} # ${name}; action.yml and README.md both current" - else - echo "Pinned to ${sha} # ${name}" - echo "updated: ${changed% }" - fi + uv run scripts/bump_cli_version.py {{quote(ref)}} diff --git a/scripts/bump_cli_version.py b/scripts/bump_cli_version.py new file mode 100644 index 0000000..ba53349 --- /dev/null +++ b/scripts/bump_cli_version.py @@ -0,0 +1,232 @@ +"""Repoint the action's default sinistral-cli version at a resolved commit. + +`action.yml` pins `sinistral_cli_version` to a full commit SHA so a run can't be +retargeted by moving a tag, with a trailing comment carrying the human-readable +version so the pin stays reviewable. This keeps both halves of that pattern — +and the copy of the default documented in the README table — in step. + +Any git ref works: a tag, a branch, or a full or short SHA. The comment prefers +a tag pointing at the resolved commit; failing that it records what was asked +for, dated, so it still says something the SHA doesn't. + +A maintenance script, not part of the action itself: it runs on a developer's +machine via `just bump-cli-version`, and reaches GitHub through the `gh` CLI so +it inherits an existing login rather than needing a token of its own. +""" +# Copyright (c) 2026 - Stacklet, Inc. + +import argparse +import json +import re +import subprocess +import sys +from pathlib import Path +from typing import Any, NamedTuple + +DESCRIPTION = "Repoint the action's default sinistral-cli version at a resolved commit." +DEFAULT_REPO = "stacklet/sinistral-cli" +INPUT_NAME = "sinistral_cli_version" + +ROOT = Path(__file__).resolve().parent.parent +ACTION_FILE = ROOT / "action.yml" +README_FILE = ROOT / "README.md" + +# The pinned line inside the input's block, e.g. +# default: 'dbefcee...' # v0.5.38 +# The comment is optional: an older pin may be a bare tag with nothing after it. +PIN_LINE = re.compile(r"^(?P\s*)default: '(?P[^']*)'(?:\s*#\s*(?P.*?))?\s*$") + +# The input's row in the README table, whose Default column repeats the version. +README_ROW = re.compile(rf"^(\| `{INPUT_NAME}` \| No \| `)(?P[^`]*)(`)", re.MULTILINE) + +# Guard against paging forever if the tags endpoint ever misbehaves. +TAG_PAGE_SIZE = 100 +MAX_TAG_PAGES = 20 + + +class Pin(NamedTuple): + """A resolved commit and the human-readable name recorded alongside it.""" + + sha: str + name: str + + +def fail(message: str) -> Any: + """Report an error and exit non-zero.""" + print(f"error: {message}", file=sys.stderr) + raise SystemExit(1) + + +def gh_api(path: str) -> Any: + """Fetch a GitHub API path through the `gh` CLI and parse the JSON response.""" + try: + result = subprocess.run( + ["gh", "api", path], + capture_output=True, + text=True, + check=True, + ) + except FileNotFoundError: + return fail("'gh' not found. Install the GitHub CLI and run `gh auth login`.") + except subprocess.CalledProcessError as error: + return fail(error.stderr.strip() or f"gh api {path} failed") + return json.loads(result.stdout) + + +def find_tag(repo: str, sha: str, *, api: Any = gh_api) -> str | None: + """Return the first tag pointing at the given commit, or None if it has none.""" + for page in range(1, MAX_TAG_PAGES + 1): + tags = api(f"repos/{repo}/tags?per_page={TAG_PAGE_SIZE}&page={page}") + for tag in tags: + if tag["commit"]["sha"] == sha: + return str(tag["name"]) + if len(tags) < TAG_PAGE_SIZE: + break + return None + + +def describe(ref: str, sha: str, date: str, tag: str | None) -> str: + """Name a commit for the pin comment. + + A tag on the commit is the most useful label. Without one, the ref that was + asked for plus the commit date beats repeating the SHA — except when the ref + *is* the SHA, where a short SHA and the date are all there is to say. + """ + if tag: + return tag + label = sha[:7] if sha.startswith(ref) else ref + return f"{label} ({date})" + + +def resolve(repo: str, ref: str, *, api: Any = gh_api) -> Pin: + """Resolve any git ref (or "latest") to a commit SHA and a name for it.""" + if ref == "latest": + ref = api(f"repos/{repo}/releases/latest")["tag_name"] + commit = api(f"repos/{repo}/commits/{ref}") + sha = commit["sha"] + date = commit["commit"]["committer"]["date"].split("T")[0] + return Pin(sha, describe(ref, sha, date, find_tag(repo, sha, api=api))) + + +def pin_line_index(lines: list[str]) -> int | None: + """Index of the `default:` line within the version input's block.""" + in_block = False + for index, line in enumerate(lines): + if line.startswith(f" {INPUT_NAME}:"): + in_block = True + elif in_block: + if line.startswith(" default:"): + return index + # A sibling key at the inputs level, or any dedent past it, means + # the block ended without a default. + if line.strip() and not line.startswith(" "): + break + return None + + +def read_pin(text: str) -> Pin | None: + """Read the commit and comment currently pinned in action.yml.""" + lines = text.splitlines() + index = pin_line_index(lines) + if index is None: + return None + match = PIN_LINE.match(lines[index]) + if match is None: + return None + return Pin(match["sha"], match["name"] or "") + + +def write_pin(text: str, pin: Pin) -> str: + """Return action.yml text with the input's default repointed at the pin.""" + lines = text.splitlines() + index = pin_line_index(lines) + if index is None: + return fail(f"no `default:` found for `{INPUT_NAME}` in {ACTION_FILE.name}") + match = PIN_LINE.match(lines[index]) + indent = match["indent"] if match else " " + lines[index] = f"{indent}default: '{pin.sha}' # {pin.name}" + return "".join(f"{line}\n" for line in lines) + + +def read_doc(text: str) -> str | None: + """Read the version documented in the README's inputs table.""" + match = README_ROW.search(text) + return match["name"] if match else None + + +def write_doc(text: str, name: str) -> str: + """Return README text with the input's documented default set to name.""" + return README_ROW.sub(lambda match: f"{match[1]}{name}{match[3]}", text, count=1) + + +def read_file(path: Path) -> str: + """Read a file, reporting a clean error rather than a traceback.""" + try: + return path.read_text(encoding="utf-8") + except OSError as error: + return fail(f"could not read {path.name}: {error.strerror}") + + +def write_file(path: Path, text: str) -> None: + """Write a file, reporting a clean error rather than a traceback. + + A failed write must never be mistaken for a successful pin, so this exits + instead of returning and letting the caller report what it meant to do. + """ + try: + path.write_text(text, encoding="utf-8") + except OSError as error: + fail(f"could not write {path.name}: {error.strerror}") + + +def apply(pin: Pin) -> list[str]: + """Write the pin to every file that is out of step, returning their names. + + Each file is checked on its own: either can be stale while the other is + current, so neither is skipped on account of the other. + """ + changed = [] + + action_text = read_file(ACTION_FILE) + if read_pin(action_text) != pin: + write_file(ACTION_FILE, write_pin(action_text, pin)) + changed.append(ACTION_FILE.name) + + readme_text = read_file(README_FILE) + if read_doc(readme_text) != pin.name: + write_file(README_FILE, write_doc(readme_text, pin.name)) + changed.append(README_FILE.name) + + return changed + + +def main() -> None: + """Resolve the requested ref and update the pin wherever it is recorded.""" + parser = argparse.ArgumentParser(description=DESCRIPTION) + parser.add_argument( + "ref", + nargs="?", + default="latest", + help="git ref to pin: a tag, branch, or SHA (default: the latest release)", + ) + parser.add_argument( + "--repo", + default=DEFAULT_REPO, + help=f"repository to resolve the ref against (default: {DEFAULT_REPO})", + ) + args = parser.parse_args() + + pin = resolve(args.repo, args.ref) + changed = apply(pin) + if changed: + print(f"Pinned to {pin.sha} # {pin.name}") + print(f"updated: {' '.join(changed)}") + else: + print( + f"Already pinned to {pin.sha} # {pin.name}; " + f"{ACTION_FILE.name} and {README_FILE.name} both current" + ) + + +if __name__ == "__main__": + main() diff --git a/tests/test_bump_cli_version.py b/tests/test_bump_cli_version.py new file mode 100644 index 0000000..0f9aa72 --- /dev/null +++ b/tests/test_bump_cli_version.py @@ -0,0 +1,289 @@ +"""Tests for the sinistral-cli version bump script.""" +# Copyright (c) 2026 - Stacklet, Inc. + +import pytest +from bump_cli_version import ( + Pin, + apply, + describe, + find_tag, + pin_line_index, + read_doc, + read_pin, + resolve, + write_doc, + write_pin, +) + +SHA = "dbefcee8c50ad4300be1236e10dc613bd9c2f3dc" +OLD_SHA = "8552c68f72b9df5bb4985f722fec4063ebb35218" + +ACTION_YML = """\ +inputs: + sinistral_project: + required: true + description: Sinistral project name to scan against + sinistral_cli_version: + required: false + description: > + sinistral-cli version to use. Accepts any git ref (branch, tag, SHA) from + https://github.com/stacklet/sinistral-cli. + default: '{value}'{comment} + post_pr_comment: + required: false + default: 'true' +""" + +README = """\ +| Input | Required | Default | Description | +| :--- | :---: | :---: | :--- | +| `recurse` | No | `false` | Recursively discover subdirectories. | +| `sinistral_cli_version` | No | `{value}` | Git ref (tag, branch, SHA) of sinistral-cli. | +| `post_pr_comment` | No | `true` | Whether to post scan results as a PR comment. | +""" + + +def action_yml(value: str, comment: str | None = None) -> str: + return ACTION_YML.format(value=value, comment=f" # {comment}" if comment else "") + + +def readme(value: str) -> str: + return README.format(value=value) + + +class TestPinLineIndex: + def test_finds_default_within_the_block(self): + lines = action_yml(SHA, "v0.5.38").splitlines() + index = pin_line_index(lines) + assert index is not None + assert lines[index].strip().startswith("default:") + + def test_skips_defaults_of_other_inputs(self): + # The first `default:` in the file belongs to a later input; picking it + # would rewrite post_pr_comment instead. + lines = action_yml(SHA, "v0.5.38").splitlines() + index = pin_line_index(lines) + assert index is not None + assert lines[index].endswith("' # v0.5.38") + + def test_missing_input_returns_none(self): + assert pin_line_index(["inputs:", " other:", " default: 'x'"]) is None + + def test_block_without_default_returns_none(self): + lines = [ + "inputs:", + " sinistral_cli_version:", + " required: false", + " post_pr_comment:", + " default: 'true'", + ] + assert pin_line_index(lines) is None + + +class TestReadPin: + def test_reads_sha_and_comment(self): + assert read_pin(action_yml(SHA, "v0.5.38")) == Pin(SHA, "v0.5.38") + + def test_reads_pin_without_a_comment(self): + assert read_pin(action_yml("v0.5.34")) == Pin("v0.5.34", "") + + def test_reads_a_comment_containing_spaces(self): + pinned = action_yml(SHA, "demo-fixes (2023-03-13)") + assert read_pin(pinned) == Pin(SHA, "demo-fixes (2023-03-13)") + + def test_missing_input_returns_none(self): + assert read_pin("inputs:\n other:\n default: 'x'\n") is None + + +class TestWritePin: + def test_replaces_the_sha_and_comment(self): + result = write_pin(action_yml(OLD_SHA, "v0.5.36"), Pin(SHA, "v0.5.38")) + assert f" default: '{SHA}' # v0.5.38\n" in result + assert OLD_SHA not in result + + def test_upgrades_a_bare_tag_pin(self): + result = write_pin(action_yml("v0.5.34"), Pin(SHA, "v0.5.38")) + assert f" default: '{SHA}' # v0.5.38\n" in result + + def test_leaves_other_inputs_alone(self): + result = write_pin(action_yml(OLD_SHA, "v0.5.36"), Pin(SHA, "v0.5.38")) + assert " default: 'true'\n" in result + assert result.count("default:") == action_yml("x", "c").count("default:") + + def test_round_trips_through_read_pin(self): + pin = Pin(SHA, "demo-fixes (2023-03-13)") + assert read_pin(write_pin(action_yml("v0.5.34"), pin)) == pin + + def test_preserves_surrounding_content(self): + result = write_pin(action_yml(OLD_SHA, "v0.5.36"), Pin(SHA, "v0.5.38")) + assert result.startswith("inputs:\n sinistral_project:\n") + assert result.endswith(" default: 'true'\n") + + @pytest.mark.parametrize("name", ["v1.0&2", r"back\slash", "at@sign", "semi;colon"]) + def test_names_with_replacement_metacharacters(self, name): + # These characters are special to sed's replacement side; the point of + # the Python rewrite is that they need no escaping. + assert read_pin(write_pin(action_yml("v0.5.34"), Pin(SHA, name))) == Pin(SHA, name) + + def test_missing_input_exits(self): + with pytest.raises(SystemExit): + write_pin("inputs:\n other:\n default: 'x'\n", Pin(SHA, "v0.5.38")) + + +class TestReadme: + def test_reads_the_documented_default(self): + assert read_doc(readme("v0.5.34")) == "v0.5.34" + + def test_writes_the_documented_default(self): + assert read_doc(write_doc(readme("v0.5.34"), "v0.5.38")) == "v0.5.38" + + def test_leaves_other_rows_alone(self): + result = write_doc(readme("v0.5.34"), "v0.5.38") + assert "| `recurse` | No | `false` |" in result + assert "| `post_pr_comment` | No | `true` |" in result + + @pytest.mark.parametrize("name", ["v1.0&2", r"back\slash", "at@sign"]) + def test_names_with_replacement_metacharacters(self, name): + assert read_doc(write_doc(readme("v0.5.34"), name)) == name + + def test_missing_row_returns_none(self): + assert read_doc("| Input | Required |\n| `other` | No | `x` |\n") is None + + +class TestDescribe: + def test_prefers_a_tag_on_the_commit(self): + assert describe("main", SHA, "2026-09-01", "v0.5.38") == "v0.5.38" + + def test_untagged_ref_gets_the_ref_and_date(self): + assert describe("demo-fixes", SHA, "2023-03-13", None) == "demo-fixes (2023-03-13)" + + def test_untagged_full_sha_gets_a_short_sha(self): + assert describe(SHA, SHA, "2023-03-13", None) == "dbefcee (2023-03-13)" + + def test_untagged_short_sha_gets_a_short_sha(self): + assert describe(SHA[:8], SHA, "2023-03-13", None) == "dbefcee (2023-03-13)" + + +class FakeApi: + """Stand in for gh_api, recording the paths requested.""" + + def __init__( + self, *, tag_pages=None, latest="v0.5.38", commit_sha=SHA, date="2026-09-01T12:00:00Z" + ): + self.pages = tag_pages if tag_pages is not None else [[]] + self.latest = latest + self.commit_sha = commit_sha + self.date = date + self.calls = [] + + def __call__(self, path): + self.calls.append(path) + if path.endswith("/releases/latest"): + return {"tag_name": self.latest} + if "/commits/" in path: + return {"sha": self.commit_sha, "commit": {"committer": {"date": self.date}}} + page = int(path.rsplit("page=", 1)[1]) + return self.pages[page - 1] if page <= len(self.pages) else [] + + +def tag_page(*entries): + return [{"name": name, "commit": {"sha": sha}} for name, sha in entries] + + +class TestFindTag: + def test_finds_a_tag_on_the_first_page(self): + api = FakeApi(tag_pages=[tag_page(("v0.5.37", OLD_SHA), ("v0.5.38", SHA))]) + assert find_tag("owner/repo", SHA, api=api) == "v0.5.38" + + def test_returns_none_when_no_tag_matches(self): + api = FakeApi(tag_pages=[tag_page(("v0.5.37", OLD_SHA))]) + assert find_tag("owner/repo", SHA, api=api) is None + + def test_stops_paging_on_a_short_page(self): + api = FakeApi(tag_pages=[tag_page(("v0.5.37", OLD_SHA))]) + find_tag("owner/repo", SHA, api=api) + assert len(api.calls) == 1 + + def test_pages_past_a_full_page(self): + full = tag_page(*[(f"v0.0.{n}", f"{n:040d}") for n in range(100)]) + api = FakeApi(tag_pages=[full, tag_page(("v0.5.38", SHA))]) + assert find_tag("owner/repo", SHA, api=api) == "v0.5.38" + assert len(api.calls) == 2 + + +class TestResolve: + def test_latest_resolves_through_the_release(self): + api = FakeApi(tag_pages=[tag_page(("v0.5.38", SHA))]) + assert resolve("owner/repo", "latest", api=api) == Pin(SHA, "v0.5.38") + assert api.calls[0].endswith("/releases/latest") + assert "/commits/v0.5.38" in api.calls[1] + + def test_explicit_ref_skips_the_release_lookup(self): + api = FakeApi(tag_pages=[tag_page(("v0.5.36", SHA))]) + assert resolve("owner/repo", "v0.5.36", api=api) == Pin(SHA, "v0.5.36") + assert not any("releases" in call for call in api.calls) + + def test_untagged_branch_falls_back_to_ref_and_date(self): + api = FakeApi(tag_pages=[[]], date="2023-03-13T09:00:00Z") + assert resolve("owner/repo", "demo-fixes", api=api) == Pin(SHA, "demo-fixes (2023-03-13)") + + +class TestApply: + @pytest.fixture(autouse=True) + def _files(self, tmp_path, monkeypatch): + action = tmp_path / "action.yml" + readme_file = tmp_path / "README.md" + monkeypatch.setattr("bump_cli_version.ACTION_FILE", action) + monkeypatch.setattr("bump_cli_version.README_FILE", readme_file) + self.action = action + self.readme = readme_file + + def write(self, sha, comment, readme_value): + self.action.write_text(action_yml(sha, comment), encoding="utf-8") + self.readme.write_text(readme(readme_value), encoding="utf-8") + + def test_both_stale(self): + self.write(OLD_SHA, "v0.5.36", "v0.5.34") + assert apply(Pin(SHA, "v0.5.38")) == ["action.yml", "README.md"] + + def test_both_current(self): + self.write(SHA, "v0.5.38", "v0.5.38") + assert apply(Pin(SHA, "v0.5.38")) == [] + + def test_only_readme_stale(self): + self.write(SHA, "v0.5.38", "v0.5.34") + assert apply(Pin(SHA, "v0.5.38")) == ["README.md"] + assert read_doc(self.readme.read_text(encoding="utf-8")) == "v0.5.38" + + def test_only_action_stale(self): + self.write(OLD_SHA, "v0.5.36", "v0.5.38") + assert apply(Pin(SHA, "v0.5.38")) == ["action.yml"] + assert read_pin(self.action.read_text(encoding="utf-8")) == Pin(SHA, "v0.5.38") + + def test_same_commit_with_a_new_name_refreshes_the_comment(self): + self.write(SHA, "demo-fixes (2023-03-13)", "demo-fixes (2023-03-13)") + assert apply(Pin(SHA, "dbefcee (2023-03-13)")) == ["action.yml", "README.md"] + + def test_unwritable_file_exits_without_claiming_success(self): + # A failed write must never be reported as a completed pin. + self.write(OLD_SHA, "v0.5.36", "v0.5.34") + self.action.chmod(0o444) + try: + with pytest.raises(SystemExit): + apply(Pin(SHA, "v0.5.38")) + finally: + self.action.chmod(0o644) + assert read_pin(self.action.read_text(encoding="utf-8")) == Pin(OLD_SHA, "v0.5.36") + + def test_missing_file_exits(self): + self.write(OLD_SHA, "v0.5.36", "v0.5.34") + self.action.unlink() + with pytest.raises(SystemExit): + apply(Pin(SHA, "v0.5.38")) + + def test_preserves_file_permissions(self): + self.write(OLD_SHA, "v0.5.36", "v0.5.34") + self.action.chmod(0o644) + mode = self.action.stat().st_mode + apply(Pin(SHA, "v0.5.38")) + assert self.action.stat().st_mode == mode From 55f288f8003921ef1d188fea55db4eec7e7a76bd Mon Sep 17 00:00:00 2001 From: AJ Kerrigan Date: Thu, 10 Sep 2026 12:03:01 -0400 Subject: [PATCH 5/6] fix: satisfy ruff's Any and executable-path rules `just lint` was failing on five ruff errors the formatter doesn't touch: - ANN401 on four `Any` annotations. `fail` never returns, so it is `NoReturn`, which also lets its callers drop the `return fail(...)` dance they used to convince the type checker. The gh helpers really do hand back arbitrary decoded JSON, so that is now a named `Json` alias and the injectable seams are `Callable[[str], Json]`. - S607 on the literal `gh` in the subprocess argv. Resolving it through `shutil.which` fixes the partial path and gives a better home for the not-installed message than catching FileNotFoundError after the fact. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/bump_cli_version.py | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/scripts/bump_cli_version.py b/scripts/bump_cli_version.py index ba53349..2f12e38 100644 --- a/scripts/bump_cli_version.py +++ b/scripts/bump_cli_version.py @@ -18,10 +18,16 @@ import argparse import json import re +import shutil import subprocess import sys +from collections.abc import Callable from pathlib import Path -from typing import Any, NamedTuple +from typing import Any, NamedTuple, NoReturn + +# Decoded JSON from the GitHub API: an object or an array, shaped by whichever +# endpoint produced it. +type Json = Any DESCRIPTION = "Repoint the action's default sinistral-cli version at a resolved commit." DEFAULT_REPO = "stacklet/sinistral-cli" @@ -51,29 +57,35 @@ class Pin(NamedTuple): name: str -def fail(message: str) -> Any: +def fail(message: str) -> NoReturn: """Report an error and exit non-zero.""" print(f"error: {message}", file=sys.stderr) raise SystemExit(1) -def gh_api(path: str) -> Any: +def gh_path() -> str: + """Locate the `gh` CLI, with install guidance if it isn't on PATH.""" + found = shutil.which("gh") + if found is None: + fail("'gh' not found. Install the GitHub CLI and run `gh auth login`.") + return found + + +def gh_api(path: str) -> Json: """Fetch a GitHub API path through the `gh` CLI and parse the JSON response.""" try: result = subprocess.run( - ["gh", "api", path], + [gh_path(), "api", path], capture_output=True, text=True, check=True, ) - except FileNotFoundError: - return fail("'gh' not found. Install the GitHub CLI and run `gh auth login`.") except subprocess.CalledProcessError as error: - return fail(error.stderr.strip() or f"gh api {path} failed") + fail(error.stderr.strip() or f"gh api {path} failed") return json.loads(result.stdout) -def find_tag(repo: str, sha: str, *, api: Any = gh_api) -> str | None: +def find_tag(repo: str, sha: str, *, api: Callable[[str], Json] = gh_api) -> str | None: """Return the first tag pointing at the given commit, or None if it has none.""" for page in range(1, MAX_TAG_PAGES + 1): tags = api(f"repos/{repo}/tags?per_page={TAG_PAGE_SIZE}&page={page}") @@ -98,7 +110,7 @@ def describe(ref: str, sha: str, date: str, tag: str | None) -> str: return f"{label} ({date})" -def resolve(repo: str, ref: str, *, api: Any = gh_api) -> Pin: +def resolve(repo: str, ref: str, *, api: Callable[[str], Json] = gh_api) -> Pin: """Resolve any git ref (or "latest") to a commit SHA and a name for it.""" if ref == "latest": ref = api(f"repos/{repo}/releases/latest")["tag_name"] @@ -141,7 +153,7 @@ def write_pin(text: str, pin: Pin) -> str: lines = text.splitlines() index = pin_line_index(lines) if index is None: - return fail(f"no `default:` found for `{INPUT_NAME}` in {ACTION_FILE.name}") + fail(f"no `default:` found for `{INPUT_NAME}` in {ACTION_FILE.name}") match = PIN_LINE.match(lines[index]) indent = match["indent"] if match else " " lines[index] = f"{indent}default: '{pin.sha}' # {pin.name}" @@ -164,7 +176,7 @@ def read_file(path: Path) -> str: try: return path.read_text(encoding="utf-8") except OSError as error: - return fail(f"could not read {path.name}: {error.strerror}") + fail(f"could not read {path.name}: {error.strerror}") def write_file(path: Path, text: str) -> None: From ff4d9ad343c3a9bbd10dfc0bc9c86a737e540cae Mon Sep 17 00:00:00 2001 From: AJ Kerrigan Date: Thu, 10 Sep 2026 12:08:51 -0400 Subject: [PATCH 6/6] fix: fail when the README pin row has nowhere to go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses cubic review feedback on #11: - A README whose input row no longer matches substituted nothing, yet `apply` still counted the file as updated — the same false success the shell helper was faulted for, and inconsistent with `write_pin`, which already fails when the input block is missing. Fail on a zero-count substitution instead. - The unwritable-file test leaned on `chmod(0o444)`, which does not stop root and so would fail inside a root dev container. Deny the write directly instead, which tests the same code path for any user. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VBgEPS6veg5TYY4D48QgXb --- scripts/bump_cli_version.py | 12 ++++++++++-- tests/test_bump_cli_version.py | 35 ++++++++++++++++++++++++++-------- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/scripts/bump_cli_version.py b/scripts/bump_cli_version.py index 2f12e38..1308cb4 100644 --- a/scripts/bump_cli_version.py +++ b/scripts/bump_cli_version.py @@ -167,8 +167,16 @@ def read_doc(text: str) -> str | None: def write_doc(text: str, name: str) -> str: - """Return README text with the input's documented default set to name.""" - return README_ROW.sub(lambda match: f"{match[1]}{name}{match[3]}", text, count=1) + """Return README text with the input's documented default set to name. + + A row that no longer matches would substitute nothing, and a silent no-op + here would be reported as an updated file, so this fails like write_pin + does when the pin has nowhere to go. + """ + updated, count = README_ROW.subn(lambda match: f"{match[1]}{name}{match[3]}", text, count=1) + if not count: + fail(f"no `{INPUT_NAME}` row found in {README_FILE.name}") + return updated def read_file(path: Path) -> str: diff --git a/tests/test_bump_cli_version.py b/tests/test_bump_cli_version.py index 0f9aa72..874ff5b 100644 --- a/tests/test_bump_cli_version.py +++ b/tests/test_bump_cli_version.py @@ -149,6 +149,16 @@ def test_names_with_replacement_metacharacters(self, name): def test_missing_row_returns_none(self): assert read_doc("| Input | Required |\n| `other` | No | `x` |\n") is None + def test_missing_row_exits_rather_than_substituting_nothing(self): + with pytest.raises(SystemExit): + write_doc("| Input | Required |\n| `other` | No | `x` |\n", "v0.5.38") + + def test_reformatted_row_exits(self): + # An extra space in the Required column stops the row from matching; + # reporting it as updated would leave the documented default stale. + with pytest.raises(SystemExit): + write_doc("| `sinistral_cli_version` | No | `v0.5.34` | desc |\n", "v0.5.38") + class TestDescribe: def test_prefers_a_tag_on_the_commit(self): @@ -264,17 +274,26 @@ def test_same_commit_with_a_new_name_refreshes_the_comment(self): self.write(SHA, "demo-fixes (2023-03-13)", "demo-fixes (2023-03-13)") assert apply(Pin(SHA, "dbefcee (2023-03-13)")) == ["action.yml", "README.md"] - def test_unwritable_file_exits_without_claiming_success(self): - # A failed write must never be reported as a completed pin. + def test_unwritable_file_exits_without_claiming_success(self, monkeypatch): + # A failed write must never be reported as a completed pin. Denying the + # write directly rather than via chmod keeps this meaningful as root, + # who is not stopped by a read-only mode bit. self.write(OLD_SHA, "v0.5.36", "v0.5.34") - self.action.chmod(0o444) - try: - with pytest.raises(SystemExit): - apply(Pin(SHA, "v0.5.38")) - finally: - self.action.chmod(0o644) + + def deny(*args, **kwargs): + raise OSError(13, "Permission denied") + + monkeypatch.setattr("pathlib.Path.write_text", deny) + with pytest.raises(SystemExit): + apply(Pin(SHA, "v0.5.38")) assert read_pin(self.action.read_text(encoding="utf-8")) == Pin(OLD_SHA, "v0.5.36") + def test_missing_readme_row_exits_without_claiming_an_update(self): + self.write(OLD_SHA, "v0.5.36", "v0.5.34") + self.readme.write_text("| Input | Required |\n| `other` | No | `x` |\n", encoding="utf-8") + with pytest.raises(SystemExit): + apply(Pin(SHA, "v0.5.38")) + def test_missing_file_exits(self): self.write(OLD_SHA, "v0.5.36", "v0.5.34") self.action.unlink()