From f601c572f89e4cff0bf620e834543b3b3105e6e4 Mon Sep 17 00:00:00 2001 From: j7an Date: Thu, 14 May 2026 22:30:46 -0700 Subject: [PATCH 01/10] test(bump-version-files): add /tmp/bump.modified manifest contract tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These tests are intentionally failing — they pin the contract that the Git Data API rewrite of tag-release.yml will consume. The manifest must be a unique path set so POST /git/trees does not receive duplicate tree[] entries. Refs cross-agent-reviews#12. --- tests/bump-version-files.bats | 46 +++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/bump-version-files.bats b/tests/bump-version-files.bats index 1aa2593..4bebe67 100644 --- a/tests/bump-version-files.bats +++ b/tests/bump-version-files.bats @@ -395,3 +395,49 @@ JSON [ "$(cat server.json)" = "$ORIG" ] [[ "$output" =~ "skipped (invalid path_expr)" ]] } + +# === Manifest emission: /tmp/bump.modified contract (#issue-cross-agent-12) === +# Manifest must be a UNIQUE PATH SET — see spec §3.1 step 2. The Git Data API +# rewrite of tag-release.yml consumes this file to build POST /git/trees; +# duplicates would produce duplicate tree[] entries with undefined behavior. + +@test "manifest: /tmp/bump.modified is unique path set after multi-entry-same-file run" { + run_bumper "valid/multi-entry-same-file.json" "1.2.3" + [ "$status" -eq 0 ] + [ -f /tmp/bump.modified ] + # Exactly one line: server.json (despite TWO entries targeting it) + [ "$(wc -l < /tmp/bump.modified)" -eq 1 ] + [ "$(cat /tmp/bump.modified)" = "server.json" ] +} + +@test "manifest: /tmp/bump.modified contains all unique modified paths in mixed run" { + run_bumper "valid/mixed-old-and-new.json" "1.2.3" + [ "$status" -eq 0 ] + # mixed-old-and-new.json bumps package.json (legacy field) AND server.json (path_expr) + expected=$(printf 'package.json\nserver.json\n') + actual=$(sort /tmp/bump.modified) + [ "$actual" = "$expected" ] +} + +@test "manifest: /tmp/bump.modified is empty when no entries are updated (idempotent rerun)" { + cp "$REPO_ROOT/tests/fixtures/bump-version-files/valid/legacy-field.json" .version-bump.json + bash "$REPO_ROOT/scripts/bump-version-files.sh" .version-bump.json 1.2.3 # First run: bumps + run bash "$REPO_ROOT/scripts/bump-version-files.sh" .version-bump.json 1.2.3 # Second run: no-op + [ "$status" -eq 2 ] + [ -f /tmp/bump.modified ] + [ ! -s /tmp/bump.modified ] # File exists but is empty +} + +@test "manifest: /tmp/bump.modified is truncated on each invocation (no cross-run pollution)" { + # Run 1: bump multi-entry-same-file (writes "server.json") + run_bumper "valid/multi-entry-same-file.json" "1.2.3" + [ "$status" -eq 0 ] + # Run 2: no-op (legacy-field already at 0.0.0, bump to same version - actually different) + # Use idempotent rerun: run legacy-field twice + cp "$REPO_ROOT/tests/fixtures/bump-version-files/valid/legacy-field.json" .version-bump.json + bash "$REPO_ROOT/scripts/bump-version-files.sh" .version-bump.json 9.9.9 # First bump + run bash "$REPO_ROOT/scripts/bump-version-files.sh" .version-bump.json 9.9.9 # Idempotent rerun + [ "$status" -eq 2 ] + # /tmp/bump.modified must NOT contain server.json from the earlier multi-entry run + ! grep -q server.json /tmp/bump.modified +} From 09b0cc6079709dbd2cb92f46e40784936b96f0dc Mon Sep 17 00:00:00 2001 From: j7an Date: Thu, 14 May 2026 22:36:28 -0700 Subject: [PATCH 02/10] test(bump-version-files): tighten manifest tests against vacuous red-phase pass Code-quality review found that Test 4 (no cross-run pollution) and Test 2 (unique-paths in mixed run) could pass for the wrong reason during the red phase if /tmp/bump.modified didn't exist: - Test 4: `! grep -q server.json /tmp/bump.modified` returned true on a missing file (grep fails, ! flips), masking that the script wasn't creating the manifest at all. Add explicit `[ -f /tmp/bump.modified ]` guard before the negated grep. - Test 2: `actual=$(sort /tmp/bump.modified)` swallowed the missing-file error into an empty `actual`, producing a content-mismatch message that hid the real problem. Switch to `< /tmp/bump.modified` so the shell-level open fails loudly with No such file or directory. Also clarify Test 3's confusing inline comment about idempotent rerun. Refs cross-agent-reviews#12. --- tests/bump-version-files.bats | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/bump-version-files.bats b/tests/bump-version-files.bats index 4bebe67..47d75b1 100644 --- a/tests/bump-version-files.bats +++ b/tests/bump-version-files.bats @@ -413,10 +413,11 @@ JSON @test "manifest: /tmp/bump.modified contains all unique modified paths in mixed run" { run_bumper "valid/mixed-old-and-new.json" "1.2.3" [ "$status" -eq 0 ] - # mixed-old-and-new.json bumps package.json (legacy field) AND server.json (path_expr) + # mixed-old-and-new.json bumps package.json (legacy field) AND server.json (path_expr). + # `< file` redirect makes the assertion fail loudly if the manifest is missing, + # rather than passing a stray empty string into the comparison. expected=$(printf 'package.json\nserver.json\n') - actual=$(sort /tmp/bump.modified) - [ "$actual" = "$expected" ] + [ "$(sort < /tmp/bump.modified)" = "$expected" ] } @test "manifest: /tmp/bump.modified is empty when no entries are updated (idempotent rerun)" { @@ -432,12 +433,16 @@ JSON # Run 1: bump multi-entry-same-file (writes "server.json") run_bumper "valid/multi-entry-same-file.json" "1.2.3" [ "$status" -eq 0 ] - # Run 2: no-op (legacy-field already at 0.0.0, bump to same version - actually different) - # Use idempotent rerun: run legacy-field twice + # Run 2: idempotent rerun of legacy-field. First bump 0.0.0 -> 9.9.9 succeeds; + # second bump 9.9.9 -> 9.9.9 is a no-op (exit 2). The manifest after run 2 + # must still be truncated (empty) AND must NOT carry server.json across. cp "$REPO_ROOT/tests/fixtures/bump-version-files/valid/legacy-field.json" .version-bump.json bash "$REPO_ROOT/scripts/bump-version-files.sh" .version-bump.json 9.9.9 # First bump run bash "$REPO_ROOT/scripts/bump-version-files.sh" .version-bump.json 9.9.9 # Idempotent rerun [ "$status" -eq 2 ] - # /tmp/bump.modified must NOT contain server.json from the earlier multi-entry run + # File-existence guard prevents this from passing vacuously when the + # script doesn't yet create the manifest (red phase): grep on a missing + # file returns 1, and `! grep` would silently succeed without it. + [ -f /tmp/bump.modified ] ! grep -q server.json /tmp/bump.modified } From 4ffece1ad27a249b76f95947683975e1dc373fd7 Mon Sep 17 00:00:00 2001 From: j7an Date: Thu, 14 May 2026 22:38:29 -0700 Subject: [PATCH 03/10] fix(bump-version-files): emit /tmp/bump.modified unique path set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manifest of modified paths consumed by tag-release.yml's Git Data API rewrite. De-duped via sort -u so multi-entry-same-file configs (two path_expr entries against one file) produce one path, not two — which would create duplicate tree[] entries in POST /git/trees. Refs cross-agent-reviews#12. --- scripts/bump-version-files.sh | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/scripts/bump-version-files.sh b/scripts/bump-version-files.sh index 0b3a824..c2a66fa 100755 --- a/scripts/bump-version-files.sh +++ b/scripts/bump-version-files.sh @@ -105,6 +105,7 @@ bump_entry() { fi write_value "$file_path" "$field" "$path_expr" "$version" + printf '%s\n' "$file_path" >> /tmp/bump.modified printf '| `%s` | `%s` | `%s` -> `%s` | updated |\n' \ "$file_path" "$disp" "$current" "$version" return 0 @@ -116,6 +117,12 @@ config="${1:-.version-bump.json}" version="${2:-${VERSION:-}}" : "${version:?usage: $0 | VERSION=X $0 [config]}" +# Manifest of unique modified paths (consumed by tag-release.yml's +# Git Data API path to build POST /git/trees). Truncate-or-create at +# start so the file is always a fresh, well-defined snapshot of THIS +# invocation's modifications. De-dup happens at end of apply pass. +: > /tmp/bump.modified + if [ ! -f "$config" ]; then echo "No $config found, skipping version file bump" exit 2 @@ -157,6 +164,14 @@ while IFS= read -r row; do fi done < <(jq -c '.files[]' "$config") +# De-dup manifest in place: a single file may appear N times if +# .version-bump.json has multiple entries against it (see test +# `multi-entry: two path_expr entries against same file write both`). +# sort -u keeps the contract that /tmp/bump.modified is a unique set. +if [ -s /tmp/bump.modified ]; then + sort -u /tmp/bump.modified -o /tmp/bump.modified +fi + if [ "$changed" -eq 1 ]; then exit 0 else From 2437b57922ec2278729f55ad99c2e07faa06cfe7 Mon Sep 17 00:00:00 2001 From: j7an Date: Thu, 14 May 2026 22:52:04 -0700 Subject: [PATCH 04/10] fix(tag-release): sync inline copy of bump-version-files.sh Mirror the manifest-emission changes from scripts/bump-version-files.sh into the inline copy embedded in tag-release.yml, satisfying the inline-sync invariant validated by scripts/check-inline-sync.sh. Refs cross-agent-reviews#12. --- .github/workflows/tag-release.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/tag-release.yml b/.github/workflows/tag-release.yml index 9ae14ed..5e57e69 100644 --- a/.github/workflows/tag-release.yml +++ b/.github/workflows/tag-release.yml @@ -332,6 +332,7 @@ jobs: fi write_value "$file_path" "$field" "$path_expr" "$version" + printf '%s\n' "$file_path" >> /tmp/bump.modified printf '| `%s` | `%s` | `%s` -> `%s` | updated |\n' \ "$file_path" "$disp" "$current" "$version" return 0 @@ -343,6 +344,12 @@ jobs: version="${2:-${VERSION:-}}" : "${version:?usage: $0 | VERSION=X $0 [config]}" + # Manifest of unique modified paths (consumed by tag-release.yml's + # Git Data API path to build POST /git/trees). Truncate-or-create at + # start so the file is always a fresh, well-defined snapshot of THIS + # invocation's modifications. De-dup happens at end of apply pass. + : > /tmp/bump.modified + if [ ! -f "$config" ]; then echo "No $config found, skipping version file bump" exit 2 @@ -384,6 +391,14 @@ jobs: fi done < <(jq -c '.files[]' "$config") + # De-dup manifest in place: a single file may appear N times if + # .version-bump.json has multiple entries against it (see test + # `multi-entry: two path_expr entries against same file write both`). + # sort -u keeps the contract that /tmp/bump.modified is a unique set. + if [ -s /tmp/bump.modified ]; then + sort -u /tmp/bump.modified -o /tmp/bump.modified + fi + if [ "$changed" -eq 1 ]; then exit 0 else From 3a3383102cb1b7f3a17b9d1333982f3aa83bae06 Mon Sep 17 00:00:00 2001 From: j7an Date: Thu, 14 May 2026 22:57:17 -0700 Subject: [PATCH 05/10] =?UTF-8?q?test(tag-release):=20add=20=C2=A72=20inva?= =?UTF-8?q?riant=20guards=20for=20Phase=20B=20rewrite?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Static checks that the rewritten Bump and Tag steps do not regress to local-state reads (which would reintroduce the silent-overwrite race the API rewrite is designed to prevent) and that bot-signing-disabling identity fields stay out of API payloads. Phase B-only checks (GITHUB_SHA usage, no identity fields) skip on the current Phase A baseline and activate after the workflow rewrite lands. Refs cross-agent-reviews#12. --- tests/tag-release-invariants.bats | 98 +++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 tests/tag-release-invariants.bats diff --git a/tests/tag-release-invariants.bats b/tests/tag-release-invariants.bats new file mode 100644 index 0000000..effbad2 --- /dev/null +++ b/tests/tag-release-invariants.bats @@ -0,0 +1,98 @@ +#!/usr/bin/env bats + +# tag-release-invariants.bats — static checks on the rewritten Git Data API +# blocks in .github/workflows/tag-release.yml. +# +# Spec §2 invariant: the rewritten "Bump version files" and "Create and push +# tag" steps must never depend on a post-checkout local-state read for +# deciding what SHA to use as a commit parent, what SHA to update main to, +# or what SHA to tag. The single load-bearing SHA value is $GITHUB_SHA, +# captured once. + +setup() { + REPO_ROOT="$BATS_TEST_DIRNAME/.." + WORKFLOW="$REPO_ROOT/.github/workflows/tag-release.yml" +} + +# Extract the YAML body of a step by name, up to the next " - name:" boundary. +extract_step_body() { + local name="$1" + awk -v marker=" - name: $name" ' + index($0, marker) == 1 { inside=1; next } + inside && /^ - name: / { inside=0 } + inside { print } + ' "$WORKFLOW" +} + +@test "invariant: 'Bump version files' step contains no post-checkout local-state git reads" { + body=$(extract_step_body "Bump version files") + [ -n "$body" ] + # Spec §2 forbids ALL post-checkout local-state git reads — the same set + # in both the bump step and the tag step. `git log` is included even though + # the bump step doesn't currently call it, to prevent future regressions + # (e.g., "look up the parent commit's date" via local git instead of API). + for forbidden in 'git rev-parse' 'git symbolic-ref' 'git for-each-ref' 'git describe' 'git log'; do + if printf '%s' "$body" | grep -qF "$forbidden"; then + printf 'VIOLATION: "%s" appears in Bump version files step (spec §2)\n' "$forbidden" + printf '%s\n' "$body" | grep -nF "$forbidden" + false + fi + done +} + +@test "invariant: 'Create and push tag' step contains no post-checkout local-state git reads" { + body=$(extract_step_body "Create and push tag") + [ -n "$body" ] + for forbidden in 'git rev-parse' 'git symbolic-ref' 'git for-each-ref' 'git describe' 'git log'; do + if printf '%s' "$body" | grep -qF "$forbidden"; then + printf 'VIOLATION: "%s" appears in Create and push tag step (spec §2)\n' "$forbidden" + printf '%s\n' "$body" | grep -nF "$forbidden" + false + fi + done +} + +@test "invariant: 'Bump version files' step uses GITHUB_SHA as commit parent" { + body=$(extract_step_body "Bump version files") + # After Phase B rewrite, BASE_SHA must be derived from GITHUB_SHA, not from + # GET /git/ref/heads/main (which would race per spec §2). Skip this check + # if the step still uses git push (Phase A only — pre-Phase-B baseline). + if printf '%s' "$body" | grep -qF 'git push origin HEAD:main'; then + skip "Phase A baseline — bump step still uses git push; check applies after Phase B" + fi + printf '%s' "$body" | grep -qF 'GITHUB_SHA' || { + echo "VIOLATION: rewritten Bump step does not reference GITHUB_SHA (spec §2 / §3.1 step 1)" + false + } +} + +@test "invariant: bump-commit and tag payloads omit author/committer/tagger fields" { + # Strip comment-only lines first — the workflow code's own commentary + # legitimately mentions these field names (e.g., "NO author/committer + # fields — auto-signs"). The test enforces that the words appear ONLY + # in comments, never in code positions where they could become real + # identity overrides via a jq object literal, gh api -f/-F/--field flag, + # or YAML key. + body_bump=$(extract_step_body "Bump version files" | grep -v '^[[:space:]]*#') + body_tag=$(extract_step_body "Create and push tag" | grep -v '^[[:space:]]*#') + if printf '%s' "$body_bump" | grep -qF 'git push origin HEAD:main'; then + skip "Phase A baseline — checks apply after Phase B" + fi + # Word-boundary match catches every escape hatch: "author", 'author', + # author: (jq literal), -f author=, -F author=, --field author=. + # If a real workflow change needs one of these field names in code + # for a non-identity-override reason, the test must be updated + # deliberately, with a comment explaining why. + for field in 'author' 'committer'; do + if printf '%s' "$body_bump" | grep -qE "\\b${field}\\b"; then + echo "VIOLATION: '$field' appears in Bump step (non-comment context) — disables auto-signing if used as a payload field name (spec §3.3)" + printf '%s' "$body_bump" | grep -nE "\\b${field}\\b" + false + fi + done + if printf '%s' "$body_tag" | grep -qE '\btagger\b'; then + echo "VIOLATION: 'tagger' appears in Tag step (non-comment context) — disables auto-signing if used as a payload field name (spec §3.3)" + printf '%s' "$body_tag" | grep -nE '\btagger\b' + false + fi +} From 600b5d5a1c51b41ef82dc40b4d4f3f431d3dc021 Mon Sep 17 00:00:00 2001 From: j7an Date: Thu, 14 May 2026 23:04:11 -0700 Subject: [PATCH 06/10] fix(tag-release): rewrite bump commit using Git Data API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace `git commit && git push origin HEAD:main` with a sequence of gh api calls: blobs → tree → commit → PATCH ref. The commit's parent is BASE_SHA=$GITHUB_SHA (the runner's checkout SHA), not a fresh read of main, to avoid the silent-overwrite race documented in spec §2. The commit payload omits author/committer fields so GitHub auto-signs with its bot key (spec §3.3) — satisfying the required_signatures branch ruleset on consumer repos like cross-agent-reviews without needing bypass-actor entries. The exit-2 branch (no bump created) reads main HEAD via API and exports TAG_TARGET_SHA for the tag step. Fixes cross-agent-reviews#12. --- .github/workflows/tag-release.yml | 80 +++++++++++++++++++++++++++---- 1 file changed, 71 insertions(+), 9 deletions(-) diff --git a/.github/workflows/tag-release.yml b/.github/workflows/tag-release.yml index 5e57e69..504396f 100644 --- a/.github/workflows/tag-release.yml +++ b/.github/workflows/tag-release.yml @@ -220,6 +220,7 @@ jobs: env: NEXT_TAG: ${{ steps.compute.outputs.next_tag }} TAG_PREFIX: ${{ inputs.tag-prefix }} + GH_TOKEN: ${{ steps.app-token.outputs.token }} run: | PREFIX="${TAG_PREFIX:-v}" VERSION="${NEXT_TAG#"${PREFIX}"}" @@ -414,21 +415,82 @@ jobs: case "$BUMP_EXIT" in 0) - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - for entry in $(jq -r '.files[] | .path' .version-bump.json); do - git add "$entry" - done - git commit -m "chore(release): bump version files to $VERSION" - git push origin HEAD:main - RESULT="Committed and pushed version file updates" + # --- Git Data API rewrite (spec §3.1) --- + # BASE_SHA is the workflow's runtime view of the repo HEAD — + # the SHA actions/checkout checked out. Reading refs/heads/main + # fresh here would race with concurrent pushes (spec §2). + REPO="${GITHUB_REPOSITORY}" + BASE_SHA="${GITHUB_SHA}" + + # Step 4 (spec): read parent tree SHA from BASE_SHA's commit. + PARENT_TREE_SHA=$(gh api "repos/${REPO}/git/commits/${BASE_SHA}" --jq '.tree.sha') + + # Steps 3+5: create blobs for each modified file, accumulate + # tree[] entries, then create the tree. + TREE_ENTRIES='[]' + while IFS= read -r modified_path; do + [ -z "$modified_path" ] && continue + blob_content_b64=$(base64 < "$modified_path" | tr -d '\n') + blob_sha=$(jq -nc \ + --arg c "$blob_content_b64" \ + '{content: $c, encoding: "base64"}' \ + | gh api -X POST "repos/${REPO}/git/blobs" --input - --jq '.sha') + TREE_ENTRIES=$(jq --arg p "$modified_path" --arg s "$blob_sha" \ + '. + [{path: $p, mode: "100644", type: "blob", sha: $s}]' \ + <<<"$TREE_ENTRIES") + done < /tmp/bump.modified + + TREE_SHA=$(jq -nc \ + --arg base "$PARENT_TREE_SHA" \ + --argjson entries "$TREE_ENTRIES" \ + '{base_tree: $base, tree: $entries}' \ + | gh api -X POST "repos/${REPO}/git/trees" --input - --jq '.sha') + + # Step 6: create commit. NO author/committer fields — GitHub + # auto-signs with the bot/web-flow key only when the request is + # authenticated as the App AND the body has no identity overrides + # (spec §3.3). + NEW_COMMIT_SHA=$(jq -nc \ + --arg msg "chore(release): bump version files to $VERSION" \ + --arg tree "$TREE_SHA" \ + --arg parent "$BASE_SHA" \ + '{message: $msg, tree: $tree, parents: [$parent]}' \ + | gh api -X POST "repos/${REPO}/git/commits" --input - --jq '.sha') + + # Step 7: fast-forward main to the new commit. force=false (default). + # 422 = non-fast-forward = main moved past BASE_SHA after checkout + # → fail the run loudly (spec §3.1 step 7, race-detection backstop). + gh api -X PATCH "repos/${REPO}/git/refs/heads/main" \ + -f sha="$NEW_COMMIT_SHA" \ + -F force=false + + # Acceptance criterion #2 (spec §6): bump commit MUST verify. + VERIFIED=$(gh api "repos/${REPO}/git/commits/${NEW_COMMIT_SHA}" \ + --jq '.commit.verification.verified') + REASON=$(gh api "repos/${REPO}/git/commits/${NEW_COMMIT_SHA}" \ + --jq '.commit.verification.reason') + if [ "$VERIFIED" != "true" ]; then + echo "::error::Bump commit ${NEW_COMMIT_SHA} did not auto-sign (verified=${VERIFIED} reason=${REASON})" + echo "::error::This means the API rewrite is not satisfying the required_signatures rule — investigate before proceeding." + exit 1 + fi + + # Export TAG_TARGET_SHA for the Create-and-push-tag step. + echo "TAG_TARGET_SHA=${NEW_COMMIT_SHA}" >> "$GITHUB_ENV" + RESULT="Committed and pushed version file updates via Git Data API (sha: ${NEW_COMMIT_SHA}, verified: true, reason: ${REASON})" ;; 1) echo "::error::Bump version files step failed due to .version-bump.json error" exit 1 ;; 2) - RESULT="No changes — config absent, all up to date, or all entries skipped" + # No bump created — set TAG_TARGET_SHA from main HEAD via API + # (spec §3.2 conditional). Covers fresh-no-bump-needed AND + # rerun-after-partial-failure. NEVER read local checkout state. + REPO="${GITHUB_REPOSITORY}" + MAIN_SHA=$(gh api "repos/${REPO}/git/ref/heads/main" --jq '.object.sha') + echo "TAG_TARGET_SHA=${MAIN_SHA}" >> "$GITHUB_ENV" + RESULT="No changes — config absent, all up to date, or all entries skipped (tag will point at current main: ${MAIN_SHA})" ;; *) echo "::error::bump-version-files.sh exited with unexpected code $BUMP_EXIT" From 8c4c84640f7e84e5f4f5dc42e6cab6240f370976 Mon Sep 17 00:00:00 2001 From: j7an Date: Thu, 14 May 2026 23:31:14 -0700 Subject: [PATCH 07/10] fix(tag-release): correct verification jq path for /git/commits endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-quality review against GitHub's REST API docs revealed the plan and spec had inherited a wrong jq path: GET /repos/.../git/commits/{sha} returns the raw Git commit object with `verification` at the TOP level, not under `.commit.verification` (that nesting only exists on the higher- level GET /repos/.../commits/{sha} endpoint). With the wrong path, $VERIFIED would always be the literal string "null" and the acceptance-criterion-#2 check would always fail with exit 1 AFTER the bump commit had already been PATCH'd onto main — leaving a dangling commit and no tag. Fix: keep the low-level /git/commits/{sha} endpoint (consistent with the PARENT_TREE_SHA call earlier in the same step) but use .verification.* jq paths instead. Add a comment documenting the gotcha so future maintainers don't reintroduce it. Refs cross-agent-reviews#12. --- .github/workflows/tag-release.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tag-release.yml b/.github/workflows/tag-release.yml index 504396f..231cd8e 100644 --- a/.github/workflows/tag-release.yml +++ b/.github/workflows/tag-release.yml @@ -465,10 +465,14 @@ jobs: -F force=false # Acceptance criterion #2 (spec §6): bump commit MUST verify. + # Note jq path: GET /git/commits/{sha} returns the raw Git commit + # object with `verification` at the TOP level. The higher-level + # /commits/{sha} endpoint (no `git/` prefix) wraps it as + # `.commit.verification` — do NOT confuse the two. VERIFIED=$(gh api "repos/${REPO}/git/commits/${NEW_COMMIT_SHA}" \ - --jq '.commit.verification.verified') + --jq '.verification.verified') REASON=$(gh api "repos/${REPO}/git/commits/${NEW_COMMIT_SHA}" \ - --jq '.commit.verification.reason') + --jq '.verification.reason') if [ "$VERIFIED" != "true" ]; then echo "::error::Bump commit ${NEW_COMMIT_SHA} did not auto-sign (verified=${VERIFIED} reason=${REASON})" echo "::error::This means the API rewrite is not satisfying the required_signatures rule — investigate before proceeding." From e6eb161e13110c692df59411822c256af4ce6760 Mon Sep 17 00:00:00 2001 From: j7an Date: Thu, 14 May 2026 23:34:35 -0700 Subject: [PATCH 08/10] fix(tag-release): create annotated tag via Git Data API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace `git tag -a && git push origin ` with POST /git/tags + POST /git/refs. Tag points at TAG_TARGET_SHA exported by the prior Bump step (new commit SHA in the happy path; current main HEAD via API in the no-bump case). Tag payload omits the tagger field so GitHub auto-signs the tag object with its bot key when possible. The verification.verified status is reported in the workflow step summary so the canary run can record acceptance criterion #3 (signed) vs #4 (unsigned-fallback) per spec §6. Fixes cross-agent-reviews#12. --- .github/workflows/tag-release.yml | 53 ++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 5 deletions(-) diff --git a/.github/workflows/tag-release.yml b/.github/workflows/tag-release.yml index 231cd8e..e3d4db9 100644 --- a/.github/workflows/tag-release.yml +++ b/.github/workflows/tag-release.yml @@ -517,11 +517,54 @@ jobs: - name: Create and push tag env: NEXT_TAG: ${{ steps.compute.outputs.next_tag }} + GH_TOKEN: ${{ steps.app-token.outputs.token }} run: | - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + # --- Git Data API rewrite (spec §3.2) --- + # TAG_TARGET_SHA is set by the prior "Bump version files" step: + # - case 0: the new bump commit's SHA (returned by POST /git/commits) + # - case 2: current refs/heads/main SHA (read via GET /git/ref/heads/main) + # Either way, NEVER from local checkout state (spec §2). + REPO="${GITHUB_REPOSITORY}" + + if [ -z "${TAG_TARGET_SHA:-}" ]; then + echo "::error::TAG_TARGET_SHA not set by Bump version files step — workflow contract violated" + exit 1 + fi + + # Create tag OBJECT. NO tagger field — GitHub auto-signs with the + # bot/web-flow key only when authenticated as the App AND the body + # has no identity overrides (spec §3.3). + TAG_OBJECT_SHA=$(jq -nc \ + --arg tag "$NEXT_TAG" \ + --arg msg "Release $NEXT_TAG" \ + --arg sha "$TAG_TARGET_SHA" \ + '{tag: $tag, message: $msg, object: $sha, type: "commit"}' \ + | gh api -X POST "repos/${REPO}/git/tags" --input - --jq '.sha') + + # Create tag REF. 422 = ref already exists → fail loudly (spec §4 + # row 3): re-creating an existing tag is not a workflow-level + # decision; operator must intentionally delete/replace before rerun. + gh api -X POST "repos/${REPO}/git/refs" \ + -f ref="refs/tags/${NEXT_TAG}" \ + -f sha="$TAG_OBJECT_SHA" + + # Report tag verification status (acceptance criterion #3 OR #4 + # per spec §6 — the canary run records which path locks in for v2.5.3). + # Note jq path: GET /git/tags/{sha} returns the raw Git tag object + # with `verification` at the TOP level (same convention as + # /git/commits/{sha} — see Bump step's verification check). + TAG_VERIFIED=$(gh api "repos/${REPO}/git/tags/${TAG_OBJECT_SHA}" --jq '.verification.verified') + TAG_REASON=$(gh api "repos/${REPO}/git/tags/${TAG_OBJECT_SHA}" --jq '.verification.reason') - git tag -a "$NEXT_TAG" -m "Release $NEXT_TAG" - git push origin "$NEXT_TAG" + { + echo "### Tag created" + echo "" + echo "**Tag:** \`$NEXT_TAG\`" + echo "**Tag object SHA:** \`$TAG_OBJECT_SHA\`" + echo "**Target commit SHA:** \`$TAG_TARGET_SHA\`" + echo "**Tag object verification:** \`verified=$TAG_VERIFIED\` \`reason=$TAG_REASON\`" + echo "" + echo "Tag created via Git Data API. If the caller workflow chains a publish step, it will pick up from here." + } >> "$GITHUB_STEP_SUMMARY" - echo "Tagged and pushed $NEXT_TAG - release.yml will pick up from here." + echo "Tag $NEXT_TAG created via Git Data API (verified=$TAG_VERIFIED reason=$TAG_REASON)." From a525233cd449d1112b19c9b9486733ca0f1a97bd Mon Sep 17 00:00:00 2001 From: j7an Date: Fri, 15 May 2026 07:25:27 -0700 Subject: [PATCH 09/10] fix(tag-release): verify bump commit before advancing main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings on the signed-commit release path: - Finding 2: the case-0 PATCH to refs/heads/main now runs *after* the verification check, not before. A commit object is addressable by SHA the instant POST /git/commits returns, so verification needs no ref update first. Verifying first means a verification failure leaves main untouched — no dangling unverified commit for a later rerun to pick up. - Finding 1: the no-bump (exit 2) path now tags GITHUB_SHA — the commit "Compute release" semver-analyzed — instead of a live GET /git/ref/heads/main read. The live read raced with concurrent pushes (spec §2) and could tag an unanalyzed commit. GITHUB_SHA also covers rerun-after-partial-failure: a fresh dispatch resolves it to current main HEAD anyway. - Finding 4: manifest path is now configurable via BUMP_MODIFIED_FILE (default unchanged) so concurrent script/test runs don't clobber a shared /tmp file. The bats suite pins a per-test path; the inline copy stays in sync. --- .github/workflows/tag-release.yml | 59 ++++++++++++++++++++----------- scripts/bump-version-files.sh | 15 +++++--- tests/bump-version-files.bats | 32 ++++++++++------- 3 files changed, 69 insertions(+), 37 deletions(-) diff --git a/.github/workflows/tag-release.yml b/.github/workflows/tag-release.yml index e3d4db9..a4417a0 100644 --- a/.github/workflows/tag-release.yml +++ b/.github/workflows/tag-release.yml @@ -249,6 +249,13 @@ jobs: set -euo pipefail + # Manifest file path — the unique set of modified paths, consumed by + # tag-release.yml's Git Data API tree builder. Overridable so parallel + # test runs and concurrent script invocations don't clobber one + # another through a shared /tmp file (the default preserves prior + # behavior). + BUMP_MODIFIED_FILE="${BUMP_MODIFIED_FILE:-/tmp/bump.modified}" + # Strict allowlist: identifiers, integer indices, bracket-quoted string keys # (npm/composer style: kebab-case, @scope/pkg, dotted paths), and the [] # iterator (applies-to-every). The jq [*] form is NOT valid jq grammar and @@ -333,7 +340,7 @@ jobs: fi write_value "$file_path" "$field" "$path_expr" "$version" - printf '%s\n' "$file_path" >> /tmp/bump.modified + printf '%s\n' "$file_path" >> "$BUMP_MODIFIED_FILE" printf '| `%s` | `%s` | `%s` -> `%s` | updated |\n' \ "$file_path" "$disp" "$current" "$version" return 0 @@ -349,7 +356,7 @@ jobs: # Git Data API path to build POST /git/trees). Truncate-or-create at # start so the file is always a fresh, well-defined snapshot of THIS # invocation's modifications. De-dup happens at end of apply pass. - : > /tmp/bump.modified + : > "$BUMP_MODIFIED_FILE" if [ ! -f "$config" ]; then echo "No $config found, skipping version file bump" @@ -396,8 +403,8 @@ jobs: # .version-bump.json has multiple entries against it (see test # `multi-entry: two path_expr entries against same file write both`). # sort -u keeps the contract that /tmp/bump.modified is a unique set. - if [ -s /tmp/bump.modified ]; then - sort -u /tmp/bump.modified -o /tmp/bump.modified + if [ -s "$BUMP_MODIFIED_FILE" ]; then + sort -u "$BUMP_MODIFIED_FILE" -o "$BUMP_MODIFIED_FILE" fi if [ "$changed" -eq 1 ]; then @@ -426,7 +433,10 @@ jobs: PARENT_TREE_SHA=$(gh api "repos/${REPO}/git/commits/${BASE_SHA}" --jq '.tree.sha') # Steps 3+5: create blobs for each modified file, accumulate - # tree[] entries, then create the tree. + # tree[] entries, then create the tree. The manifest path must + # match the one bump_version_files wrote — same default, same + # BUMP_MODIFIED_FILE override if the step env sets it. + MANIFEST="${BUMP_MODIFIED_FILE:-/tmp/bump.modified}" TREE_ENTRIES='[]' while IFS= read -r modified_path; do [ -z "$modified_path" ] && continue @@ -438,7 +448,7 @@ jobs: TREE_ENTRIES=$(jq --arg p "$modified_path" --arg s "$blob_sha" \ '. + [{path: $p, mode: "100644", type: "blob", sha: $s}]' \ <<<"$TREE_ENTRIES") - done < /tmp/bump.modified + done < "$MANIFEST" TREE_SHA=$(jq -nc \ --arg base "$PARENT_TREE_SHA" \ @@ -457,14 +467,13 @@ jobs: '{message: $msg, tree: $tree, parents: [$parent]}' \ | gh api -X POST "repos/${REPO}/git/commits" --input - --jq '.sha') - # Step 7: fast-forward main to the new commit. force=false (default). - # 422 = non-fast-forward = main moved past BASE_SHA after checkout - # → fail the run loudly (spec §3.1 step 7, race-detection backstop). - gh api -X PATCH "repos/${REPO}/git/refs/heads/main" \ - -f sha="$NEW_COMMIT_SHA" \ - -F force=false - # Acceptance criterion #2 (spec §6): bump commit MUST verify. + # Verify the commit OBJECT *before* advancing main. A commit is + # addressable by SHA the instant POST /git/commits returns — + # independent of any ref — so this check needs no branch update + # first. Verifying first means a verification failure (or a + # failed lookup) leaves main untouched: no dangling unverified + # commit for a later rerun to pick up via the no-bump path. # Note jq path: GET /git/commits/{sha} returns the raw Git commit # object with `verification` at the TOP level. The higher-level # /commits/{sha} endpoint (no `git/` prefix) wraps it as @@ -479,6 +488,14 @@ jobs: exit 1 fi + # Step 7: fast-forward main to the verified commit. force=false + # (default). 422 = non-fast-forward = main moved past BASE_SHA + # after checkout → fail the run loudly (spec §3.1 step 7, + # race-detection backstop). + gh api -X PATCH "repos/${REPO}/git/refs/heads/main" \ + -f sha="$NEW_COMMIT_SHA" \ + -F force=false + # Export TAG_TARGET_SHA for the Create-and-push-tag step. echo "TAG_TARGET_SHA=${NEW_COMMIT_SHA}" >> "$GITHUB_ENV" RESULT="Committed and pushed version file updates via Git Data API (sha: ${NEW_COMMIT_SHA}, verified: true, reason: ${REASON})" @@ -488,13 +505,15 @@ jobs: exit 1 ;; 2) - # No bump created — set TAG_TARGET_SHA from main HEAD via API - # (spec §3.2 conditional). Covers fresh-no-bump-needed AND - # rerun-after-partial-failure. NEVER read local checkout state. - REPO="${GITHUB_REPOSITORY}" - MAIN_SHA=$(gh api "repos/${REPO}/git/ref/heads/main" --jq '.object.sha') - echo "TAG_TARGET_SHA=${MAIN_SHA}" >> "$GITHUB_ENV" - RESULT="No changes — config absent, all up to date, or all entries skipped (tag will point at current main: ${MAIN_SHA})" + # No bump created. The commit to tag is exactly the commit that + # "Compute release" semver-analyzed — i.e. GITHUB_SHA. Reading + # live refs/heads/main here instead would race with concurrent + # pushes (spec §2) and could tag a commit that was never + # analyzed, publishing it under a bump computed from a different + # tree. GITHUB_SHA also covers rerun-after-partial-failure: a + # fresh dispatch resolves GITHUB_SHA to current main HEAD anyway. + echo "TAG_TARGET_SHA=${GITHUB_SHA}" >> "$GITHUB_ENV" + RESULT="No changes — config absent, all up to date, or all entries skipped (tag will point at analyzed commit: ${GITHUB_SHA})" ;; *) echo "::error::bump-version-files.sh exited with unexpected code $BUMP_EXIT" diff --git a/scripts/bump-version-files.sh b/scripts/bump-version-files.sh index c2a66fa..a2da211 100755 --- a/scripts/bump-version-files.sh +++ b/scripts/bump-version-files.sh @@ -21,6 +21,13 @@ set -euo pipefail +# Manifest file path — the unique set of modified paths, consumed by +# tag-release.yml's Git Data API tree builder. Overridable so parallel +# test runs and concurrent script invocations don't clobber one +# another through a shared /tmp file (the default preserves prior +# behavior). +BUMP_MODIFIED_FILE="${BUMP_MODIFIED_FILE:-/tmp/bump.modified}" + # Strict allowlist: identifiers, integer indices, bracket-quoted string keys # (npm/composer style: kebab-case, @scope/pkg, dotted paths), and the [] # iterator (applies-to-every). The jq [*] form is NOT valid jq grammar and @@ -105,7 +112,7 @@ bump_entry() { fi write_value "$file_path" "$field" "$path_expr" "$version" - printf '%s\n' "$file_path" >> /tmp/bump.modified + printf '%s\n' "$file_path" >> "$BUMP_MODIFIED_FILE" printf '| `%s` | `%s` | `%s` -> `%s` | updated |\n' \ "$file_path" "$disp" "$current" "$version" return 0 @@ -121,7 +128,7 @@ version="${2:-${VERSION:-}}" # Git Data API path to build POST /git/trees). Truncate-or-create at # start so the file is always a fresh, well-defined snapshot of THIS # invocation's modifications. De-dup happens at end of apply pass. -: > /tmp/bump.modified +: > "$BUMP_MODIFIED_FILE" if [ ! -f "$config" ]; then echo "No $config found, skipping version file bump" @@ -168,8 +175,8 @@ done < <(jq -c '.files[]' "$config") # .version-bump.json has multiple entries against it (see test # `multi-entry: two path_expr entries against same file write both`). # sort -u keeps the contract that /tmp/bump.modified is a unique set. -if [ -s /tmp/bump.modified ]; then - sort -u /tmp/bump.modified -o /tmp/bump.modified +if [ -s "$BUMP_MODIFIED_FILE" ]; then + sort -u "$BUMP_MODIFIED_FILE" -o "$BUMP_MODIFIED_FILE" fi if [ "$changed" -eq 1 ]; then diff --git a/tests/bump-version-files.bats b/tests/bump-version-files.bats index 47d75b1..515afaf 100644 --- a/tests/bump-version-files.bats +++ b/tests/bump-version-files.bats @@ -11,6 +11,10 @@ setup() { REPO_ROOT="$BATS_TEST_DIRNAME/.." cp "$REPO_ROOT"/tests/fixtures/bump-version-files/targets/*.json "$TMPDIR/" cd "$TMPDIR" + # Per-test manifest path. The script defaults to a shared /tmp/bump.modified; + # pinning it inside this test's unique TMPDIR keeps parallel/concurrent runs + # from clobbering one another (teardown's rm -rf cleans it up). + export BUMP_MODIFIED_FILE="$TMPDIR/bump.modified" } teardown() { @@ -396,40 +400,42 @@ JSON [[ "$output" =~ "skipped (invalid path_expr)" ]] } -# === Manifest emission: /tmp/bump.modified contract (#issue-cross-agent-12) === +# === Manifest emission: bump.modified contract (#issue-cross-agent-12) === # Manifest must be a UNIQUE PATH SET — see spec §3.1 step 2. The Git Data API # rewrite of tag-release.yml consumes this file to build POST /git/trees; # duplicates would produce duplicate tree[] entries with undefined behavior. +# Each test reads $BUMP_MODIFIED_FILE (a per-test path exported in setup), +# not the shared /tmp default, so concurrent runs stay isolated. -@test "manifest: /tmp/bump.modified is unique path set after multi-entry-same-file run" { +@test "manifest: bump.modified is unique path set after multi-entry-same-file run" { run_bumper "valid/multi-entry-same-file.json" "1.2.3" [ "$status" -eq 0 ] - [ -f /tmp/bump.modified ] + [ -f "$BUMP_MODIFIED_FILE" ] # Exactly one line: server.json (despite TWO entries targeting it) - [ "$(wc -l < /tmp/bump.modified)" -eq 1 ] - [ "$(cat /tmp/bump.modified)" = "server.json" ] + [ "$(wc -l < "$BUMP_MODIFIED_FILE")" -eq 1 ] + [ "$(cat "$BUMP_MODIFIED_FILE")" = "server.json" ] } -@test "manifest: /tmp/bump.modified contains all unique modified paths in mixed run" { +@test "manifest: bump.modified contains all unique modified paths in mixed run" { run_bumper "valid/mixed-old-and-new.json" "1.2.3" [ "$status" -eq 0 ] # mixed-old-and-new.json bumps package.json (legacy field) AND server.json (path_expr). # `< file` redirect makes the assertion fail loudly if the manifest is missing, # rather than passing a stray empty string into the comparison. expected=$(printf 'package.json\nserver.json\n') - [ "$(sort < /tmp/bump.modified)" = "$expected" ] + [ "$(sort < "$BUMP_MODIFIED_FILE")" = "$expected" ] } -@test "manifest: /tmp/bump.modified is empty when no entries are updated (idempotent rerun)" { +@test "manifest: bump.modified is empty when no entries are updated (idempotent rerun)" { cp "$REPO_ROOT/tests/fixtures/bump-version-files/valid/legacy-field.json" .version-bump.json bash "$REPO_ROOT/scripts/bump-version-files.sh" .version-bump.json 1.2.3 # First run: bumps run bash "$REPO_ROOT/scripts/bump-version-files.sh" .version-bump.json 1.2.3 # Second run: no-op [ "$status" -eq 2 ] - [ -f /tmp/bump.modified ] - [ ! -s /tmp/bump.modified ] # File exists but is empty + [ -f "$BUMP_MODIFIED_FILE" ] + [ ! -s "$BUMP_MODIFIED_FILE" ] # File exists but is empty } -@test "manifest: /tmp/bump.modified is truncated on each invocation (no cross-run pollution)" { +@test "manifest: bump.modified is truncated on each invocation (no cross-run pollution)" { # Run 1: bump multi-entry-same-file (writes "server.json") run_bumper "valid/multi-entry-same-file.json" "1.2.3" [ "$status" -eq 0 ] @@ -443,6 +449,6 @@ JSON # File-existence guard prevents this from passing vacuously when the # script doesn't yet create the manifest (red phase): grep on a missing # file returns 1, and `! grep` would silently succeed without it. - [ -f /tmp/bump.modified ] - ! grep -q server.json /tmp/bump.modified + [ -f "$BUMP_MODIFIED_FILE" ] + ! grep -q server.json "$BUMP_MODIFIED_FILE" } From d4fb52bbcd8749bacd78ddcf0c5ea603b2118321 Mon Sep 17 00:00:00 2001 From: j7an Date: Fri, 15 May 2026 07:25:40 -0700 Subject: [PATCH 10/10] test(tag-release): strengthen GITHUB_SHA-parent invariant guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 3: the parent-SHA invariant test previously only checked that GITHUB_SHA appeared somewhere in the bump step — a stray echo would satisfy it. It now asserts the full load-bearing chain: GITHUB_SHA -> BASE_SHA -> jq --arg parent -> commit payload. Adds a companion guard (Finding 1 regression): the bump step must not perform a live `GET .../git/ref/heads/` read. The singular-`ref` GET form is forbidden; the legitimate fast-forward PATCH uses the plural `git/refs/heads/main` and is unaffected. --- tests/tag-release-invariants.bats | 33 +++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/tests/tag-release-invariants.bats b/tests/tag-release-invariants.bats index effbad2..1ce6487 100644 --- a/tests/tag-release-invariants.bats +++ b/tests/tag-release-invariants.bats @@ -60,10 +60,39 @@ extract_step_body() { if printf '%s' "$body" | grep -qF 'git push origin HEAD:main'; then skip "Phase A baseline — bump step still uses git push; check applies after Phase B" fi - printf '%s' "$body" | grep -qF 'GITHUB_SHA' || { - echo "VIOLATION: rewritten Bump step does not reference GITHUB_SHA (spec §2 / §3.1 step 1)" + # Assert the full load-bearing chain, not merely that GITHUB_SHA appears + # somewhere — a stray `echo "$GITHUB_SHA"` must NOT satisfy this test: + # GITHUB_SHA -> BASE_SHA -> jq --arg parent -> commit payload + printf '%s' "$body" | grep -qF 'BASE_SHA="${GITHUB_SHA}"' || { + echo "VIOLATION: bump step does not bind BASE_SHA to GITHUB_SHA (spec §2 / §3.1 step 1)" false } + # `--` ends grep option parsing so the leading `--arg` is treated as a + # pattern, not a flag. + printf '%s' "$body" | grep -qF -- '--arg parent "$BASE_SHA"' || { + echo "VIOLATION: commit-create jq call does not wire its parent arg from BASE_SHA" + false + } + printf '%s' "$body" | grep -qF 'parents: [$parent]' || { + echo "VIOLATION: commit payload does not build parents from the \$parent arg" + false + } +} + +@test "invariant: 'Bump version files' step never reads a live ref for the tag target" { + body=$(extract_step_body "Bump version files") + [ -n "$body" ] + # Spec §2 / review Finding 1: the no-bump (exit 2) path must tag the + # semver-analyzed commit (GITHUB_SHA), never a fresh `GET /git/ref/heads/...` + # read — that races with concurrent pushes and can tag an unanalyzed commit. + # The discriminator is the singular/plural ref endpoint form: the legitimate + # fast-forward PATCH uses the PLURAL `git/refs/heads/main`; only the + # SINGULAR-`ref` GET form (`git/ref/heads/`) is forbidden here. + if printf '%s' "$body" | grep -qF 'git/ref/heads/'; then + echo "VIOLATION: Bump step performs a live 'GET .../git/ref/heads/' read (spec §2 / Finding 1)" + printf '%s' "$body" | grep -nF 'git/ref/heads/' + false + fi } @test "invariant: bump-commit and tag payloads omit author/committer/tagger fields" {