diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c7c5e44..31a5b36 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -39,7 +39,7 @@ jobs: # tests/test-repoman-setup.sh is deliberately NOT listed here because it # carries intentional test idioms (SC2088 literal-tilde, SC2034 var read # by a sourced function) that are load-bearing for what it asserts. - run: shellcheck --severity=warning tests/test-repoman-requirements.sh tests/test-github-api.sh tests/test-repoman-check.sh tests/test-repoman-run.sh tests/test-repo-sync.sh tests/test-repoman-index.sh tests/test-repoman-match-name.sh tests/test-automation-health-dashboard.sh tests/test-link-health-index-wiring.sh tests/test-dep-bump-index-wiring.sh tests/test-skill-meta.sh tests/test-pr-review-integration.sh tests/test-pr-review-impact.sh tests/test-pr-review-state.sh tests/test-weekly-report.sh tests/test-link-health-subset-carryforward.sh tests/test-dep-bump-subset-carryforward.sh tests/test-pr-review-subset-carryforward.sh tests/test-link-health-dryrun-baseline.sh + run: shellcheck --severity=warning tests/test-repoman-requirements.sh tests/test-github-api.sh tests/test-repoman-check.sh tests/test-repoman-run.sh tests/test-repo-sync.sh tests/test-repoman-index.sh tests/test-repoman-match-name.sh tests/test-automation-health-dashboard.sh tests/test-link-health-index-wiring.sh tests/test-dep-bump-index-wiring.sh tests/test-skill-meta.sh tests/test-attribution-consumers.sh tests/test-pr-review-integration.sh tests/test-pr-review-impact.sh tests/test-pr-review-state.sh tests/test-weekly-report.sh tests/test-link-health-subset-carryforward.sh tests/test-dep-bump-subset-carryforward.sh tests/test-pr-review-subset-carryforward.sh tests/test-link-health-dryrun-baseline.sh - name: Run test suite # NOTE: this list and the shellcheck lists above are EXPLICIT, not globs. # A new test file must be added here (and to the relevant shellcheck @@ -62,6 +62,7 @@ jobs: tests/test-repoman-index.sh tests/test-automation-health-dashboard.sh \ tests/test-link-health-index-wiring.sh tests/test-dep-bump-index-wiring.sh \ tests/test-github-api.sh tests/test-skill-meta.sh \ + tests/test-attribution-consumers.sh \ tests/test-pr-review-integration.sh tests/test-pr-review-impact.sh \ tests/test-pr-review-state.sh tests/test-weekly-report.sh \ tests/test-repoman-run.sh tests/test-repoman-check.sh \ diff --git a/scripts/github-api.sh b/scripts/github-api.sh index 9514e6a..fc855eb 100644 --- a/scripts/github-api.sh +++ b/scripts/github-api.sh @@ -297,7 +297,8 @@ resolve_skill_meta() { # Prints: a one-line "Generated by RepoMan ..." attribution footer. skill_attribution() { local skill_name="$1" - local skill_dir="${SCRIPT_DIR:-.}" + # REPOMAN_SKILL_DIR lets tests pin _meta outside the shared scripts dir. + local skill_dir="${REPOMAN_SKILL_DIR:-${SCRIPT_DIR:-.}}" local sha sha=$(resolve_skill_meta "$skill_dir" "$skill_name") diff --git a/scripts/pr-review-fixer.sh b/scripts/pr-review-fixer.sh index 3568623..c0a248d 100755 --- a/scripts/pr-review-fixer.sh +++ b/scripts/pr-review-fixer.sh @@ -25,7 +25,7 @@ COMMAND="" while [[ $# -gt 0 ]]; do case $1 in - begin|finalize) COMMAND="$1"; shift ;; + begin|finalize|footer) COMMAND="$1"; shift ;; --reports-dir) REPORTS_DIR="$2"; shift 2 ;; --verbose) VERBOSE=true; shift ;; --dry-run) DRY_RUN=true; shift ;; @@ -45,6 +45,7 @@ USAGE: COMMANDS: begin Read latest.json, mark eligible PRs as in_progress, output queue finalize Check reviews API, move reviewed PRs to reviewed state + footer Print the pinned-SHA line for the review body and its Slack post OPTIONS: --reports-dir DIR Reports directory (default: $REPORTS_DIR or ~/reports/pr-review) @@ -67,10 +68,19 @@ USAGE fi if [ -z "$COMMAND" ]; then - echo "ERROR: command required (begin or finalize). Use --help for usage." >&2 + echo "ERROR: command required (begin, finalize, or footer). Use --help for usage." >&2 exit 1 fi +# Pin provenance: write _meta only after the SHA resolves, never overwrite a +# valid file, and fall back to blob/main when resolution fails. `footer` is the +# line for the review body and its Slack post. Inline comments do not get it. +SKILL_FOOTER=$(skill_attribution "github-pr-review") +if [ "$COMMAND" = "footer" ]; then + printf '%s\n' "$SKILL_FOOTER" + exit 0 +fi + # --- Setup --- setup_workspace "pr-review-fixer" TMPDIR="$PROGRAM_TMPDIR" diff --git a/scripts/pr-review-scanner.sh b/scripts/pr-review-scanner.sh index 9d5e2a1..5ded18b 100755 --- a/scripts/pr-review-scanner.sh +++ b/scripts/pr-review-scanner.sh @@ -58,6 +58,10 @@ USAGE exit 0 fi +# Pin provenance on startup: write _meta only after the SHA resolves, and never +# overwrite a valid file. Resolution failure must not fail the scan. +skill_attribution "github-pr-review" >/dev/null + # Resolve the enrolled repo set from ~/.repoman/repos.json (owner/name tuples). # repoman_config is not needed here -- the scanner queries GitHub directly and # writes no fork, so it needs no REPOS_DIR/FORK_OWNER. diff --git a/scripts/weekly-report.sh b/scripts/weekly-report.sh index 512a3a0..f721303 100755 --- a/scripts/weekly-report.sh +++ b/scripts/weekly-report.sh @@ -83,6 +83,10 @@ if [ ! -f "$REPORT_PY" ]; then exit 1 fi +# Pin provenance and append that exact line to the report that gets posted. +# Write _meta only after the SHA resolves; never overwrite a valid file. +SKILL_FOOTER=$(skill_attribution "github-weekly-report") + # Build args. $repos is intentionally unquoted so each owner/name line becomes a # separate --repos value; repo slugs never contain whitespace. No --org: the # enrolled set can span owners, so each entry is owner-qualified and report.py @@ -94,4 +98,11 @@ set -- --repos $repos [ -n "$OUTPUT" ] && set -- "$@" --output "$OUTPUT" [ -n "$JSON_OUTPUT" ] && set -- "$@" --json-output "$JSON_OUTPUT" -exec python3 "$REPORT_PY" "$@" +python3 "$REPORT_PY" "$@" + +# --output: the posted artifact is the file. Otherwise the report is stdout. +if [ -n "$OUTPUT" ]; then + printf '\n%s\n' "$SKILL_FOOTER" >> "$OUTPUT" +else + printf '\n%s\n' "$SKILL_FOOTER" +fi diff --git a/tests/test-attribution-consumers.sh b/tests/test-attribution-consumers.sh new file mode 100644 index 0000000..ba55c8a --- /dev/null +++ b/tests/test-attribution-consumers.sh @@ -0,0 +1,127 @@ +#!/usr/bin/env bash + +# Pinned-SHA footer for pr-review (pr-review-fixer.sh footer) and +# weekly-report (appended to the posted report). +# The rules are: write _meta only after the SHA resolves, never overwrite a valid +# file, and fall back to blob/main when resolution fails. +set -uo pipefail +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +SCRIPTS="$ROOT/scripts" +fail=0 +TMP=$(mktemp -d) +trap 'rm -rf "$TMP"' EXIT + +FAKE_SHA="85449beabc1234567890abcdef1234567890abcd" +PINNED="deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" + +mkdir -p "$TMP/bin" +cat > "$TMP/bin/gh" << 'EOF' +#!/bin/sh +if [ -n "${GH_LOG:-}" ]; then + echo "$@" >> "$GH_LOG" +fi +if [ "${GH_FAIL:-}" = "1" ]; then + echo "rate limited" >&2 + exit 1 +fi +printf '%s\n' "$GH_SHA" +EOF +chmod +x "$TMP/bin/gh" +export PATH="$TMP/bin:$PATH" +export GH_SHA="$FAKE_SHA" +export GH_LOG="$TMP/gh.log" +export SKILL_SOURCE_REPO="rossoctl/agent-skills" +: > "$GH_LOG" + +check() { + if eval "$1"; then + echo "PASS: $2" + else + echo "FAIL: $2" + fail=1 + fi +} + +export REPOMAN_SKILL_DIR="$TMP/fixer" +mkdir -p "$REPOMAN_SKILL_DIR" +foot=$(bash "$SCRIPTS/pr-review-fixer.sh" footer) +case "$foot" in + *"Generated by RepoMan github-pr-review@${FAKE_SHA:0:7}"*"blob/$FAKE_SHA/skills/github-pr-review/SKILL.md"*) + echo "PASS: fixer footer line" ;; + *) echo "FAIL: fixer footer line: $foot"; fail=1 ;; +esac +check "[ -f \"$REPOMAN_SKILL_DIR/_meta.github-pr-review.json\" ]" "fixer writes _meta" +: > "$GH_LOG" +bash "$SCRIPTS/pr-review-fixer.sh" footer >/dev/null +check "[ ! -s \"$GH_LOG\" ]" "fixer does not refresh a valid _meta" + +export GH_FAIL=1 +export REPOMAN_SKILL_DIR="$TMP/fixer-fail" +mkdir -p "$REPOMAN_SKILL_DIR" +footf=$(bash "$SCRIPTS/pr-review-fixer.sh" footer 2>/dev/null) +case "$footf" in + *"blob/main/skills/github-pr-review/SKILL.md"*) + echo "PASS: fixer failure uses blob/main" ;; + *) echo "FAIL: fixer failure uses blob/main: $footf"; fail=1 ;; +esac +case "$footf" in + *@*) echo "FAIL: fixer fallback has no @sha: $footf"; fail=1 ;; + *) echo "PASS: fixer fallback has no @sha" ;; +esac +check "[ ! -f \"$REPOMAN_SKILL_DIR/_meta.github-pr-review.json\" ]" "fixer failure writes no _meta" + +export GH_FAIL= +export REPOMAN_REPOS_FILE="$TMP/repos.json" +printf '[{"owner":"acme","name":"widgets"}]\n' > "$REPOMAN_REPOS_FILE" +export REPORT_PY="$TMP/report.py" +cat > "$REPORT_PY" << 'PY' +import sys +args = sys.argv[1:] +if "--output" in args: + open(args[args.index("--output") + 1], "w").write("# Weekly\nbody\n") +else: + sys.stdout.write("# Weekly\nbody\n") +PY +export REPOMAN_SKILL_DIR="$TMP/weekly" +mkdir -p "$REPOMAN_SKILL_DIR" +: > "$GH_LOG" +out="$TMP/report.md" +bash "$SCRIPTS/weekly-report.sh" --output "$out" >/dev/null +last=$(tail -n 1 "$out") +check "grep -q '# Weekly' \"$out\"" "weekly report keeps its body" +case "$last" in + *"Generated by RepoMan github-weekly-report@${FAKE_SHA:0:7}"*"blob/$FAKE_SHA/skills/github-weekly-report/SKILL.md"*) + echo "PASS: weekly report ends with pinned footer" ;; + *) echo "FAIL: weekly report ends with pinned footer: $last"; fail=1 ;; +esac +check "[ -f \"$REPOMAN_SKILL_DIR/_meta.github-weekly-report.json\" ]" "weekly writes its own _meta" + +# A pre-pinned file is kept, and a different gh answer is ignored. +printf '{"version":"%s","source":"rossoctl/agent-skills","installed_at":"2026-01-01"}\n' \ + "$PINNED" > "$REPOMAN_SKILL_DIR/_meta.github-weekly-report.json" +export GH_SHA="ffffffffffffffffffffffffffffffffffffffff" +: > "$GH_LOG" +out2="$TMP/report-kept.md" +bash "$SCRIPTS/weekly-report.sh" --output "$out2" >/dev/null +kept=$(tail -n 1 "$out2") +case "$kept" in + *"github-weekly-report@${PINNED:0:7}"*) + echo "PASS: weekly keeps an existing _meta sha" ;; + *) echo "FAIL: weekly keeps an existing _meta sha: $kept"; fail=1 ;; +esac +check "[ ! -s \"$GH_LOG\" ]" "weekly does not call gh when _meta is valid" + +export GH_FAIL=1 +export REPOMAN_SKILL_DIR="$TMP/weekly-fail" +mkdir -p "$REPOMAN_SKILL_DIR" +outf="$TMP/report-fail.md" +bash "$SCRIPTS/weekly-report.sh" --output "$outf" >/dev/null 2>&1 +lastf=$(tail -n 1 "$outf") +case "$lastf" in + *"blob/main/skills/github-weekly-report/SKILL.md"*) + echo "PASS: weekly failure uses blob/main" ;; + *) echo "FAIL: weekly failure uses blob/main: $lastf"; fail=1 ;; +esac +check "[ ! -f \"$REPOMAN_SKILL_DIR/_meta.github-weekly-report.json\" ]" "weekly failure writes no _meta" + +[ "$fail" -eq 0 ] && echo "PASS: pr-review and weekly-report attribution" || exit 1 diff --git a/tests/test-weekly-report.sh b/tests/test-weekly-report.sh index b0cea61..453b097 100755 --- a/tests/test-weekly-report.sh +++ b/tests/test-weekly-report.sh @@ -2,10 +2,10 @@ set -euo pipefail # Verifies scripts/weekly-report.sh builds the generator invocation from the -# RepoMan enrolled set. Hermetic: a fixture enrolled set via $REPOMAN_REPOS_FILE -# and a stub report.py via $REPORT_PY, so no gh / network / real config is -# touched. The enrolled set can span owners, so each repo is passed -# owner-qualified and there is no --org. +# RepoMan enrolled set. Hermetic: a fixture enrolled set via $REPOMAN_REPOS_FILE, +# a stub report.py via $REPORT_PY, and a failing gh so provenance falls back +# to blob/main without network. The enrolled set can span owners, so each repo +# is passed owner-qualified and there is no --org. SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" WRAPPER="$SCRIPT_DIR/../scripts/weekly-report.sh" @@ -23,25 +23,45 @@ cat > "$TEST_TMPDIR/repos.json" <<'EOF' ] EOF -# Stub generator: echo the args it was called with. +# Stub generator: echo the args it was called with. With --output, also write a body. cat > "$TEST_TMPDIR/report.py" <<'PY' import sys -print(" ".join(sys.argv[1:])) +args = sys.argv[1:] +print(" ".join(args)) +if "--output" in args: + open(args[args.index("--output") + 1], "w").write("BODY\n") PY -# Owner-qualified --repos, no --org. +# Provenance must not call a real gh or write _meta into the scripts dir. +mkdir -p "$TEST_TMPDIR/bin" "$TEST_TMPDIR/meta" +cat > "$TEST_TMPDIR/bin/gh" << 'EOF' +#!/bin/sh +exit 1 +EOF +chmod +x "$TEST_TMPDIR/bin/gh" +export PATH="$TEST_TMPDIR/bin:$PATH" +export REPOMAN_SKILL_DIR="$TEST_TMPDIR/meta" +FALLBACK='Generated by RepoMan github-weekly-report (https://github.com/rossoctl/agent-skills/blob/main/skills/github-weekly-report/SKILL.md).' + +# Owner-qualified --repos, no --org. The footer is appended to the file, not stdout. +out="$TEST_TMPDIR/out.md" got=$(REPOMAN_REPOS_FILE="$TEST_TMPDIR/repos.json" \ REPORT_PY="$TEST_TMPDIR/report.py" \ - bash "$WRAPPER" --output /tmp/ignored.md) -want="--repos rossoctl/alpha alice/beta --output /tmp/ignored.md" + bash "$WRAPPER" --output "$out" 2>/dev/null) +want="--repos rossoctl/alpha alice/beta --output $out" [ "$got" = "$want" ] || { echo "FAIL wrapper args: got [$got] want [$want]"; fail=1; } +[ "$(tail -n 1 "$out")" = "$FALLBACK" ] || { echo "FAIL output file missing footer: [$(tail -n 1 "$out")]"; fail=1; } +grep -q '^BODY$' "$out" || { echo "FAIL output file lost the report body"; fail=1; } -# --since / --until pass through. +# --since / --until pass through. No --output, so the footer is the last stdout line. got2=$(REPOMAN_REPOS_FILE="$TEST_TMPDIR/repos.json" \ REPORT_PY="$TEST_TMPDIR/report.py" \ - bash "$WRAPPER" --since 2026-08-10 --until 2026-08-17) + bash "$WRAPPER" --since 2026-08-10 --until 2026-08-17 2>/dev/null) want2="--repos rossoctl/alpha alice/beta --since 2026-08-10 --until 2026-08-17" -[ "$got2" = "$want2" ] || { echo "FAIL wrapper window args: got [$got2] want [$want2]"; fail=1; } +first2=$(printf '%s\n' "$got2" | head -n 1) +last2=$(printf '%s\n' "$got2" | tail -n 1) +[ "$first2" = "$want2" ] || { echo "FAIL wrapper window args: got [$first2] want [$want2]"; fail=1; } +[ "$last2" = "$FALLBACK" ] || { echo "FAIL stdout missing footer: [$last2]"; fail=1; } # Missing generator fails loud. if REPOMAN_REPOS_FILE="$TEST_TMPDIR/repos.json" \