From 6174ed6ee8a2a17ef8e2a9953f936805fdeec09f Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Thu, 27 Aug 2026 06:50:01 +0000 Subject: [PATCH 1/9] fix: verify remote release promotion before success (#211) --- commands/do/release.md | 164 +++++++++++++++++++++++++++++++--- test/release-contract.test.js | 65 ++++++++++++++ 2 files changed, 216 insertions(+), 13 deletions(-) create mode 100644 test/release-contract.test.js diff --git a/commands/do/release.md b/commands/do/release.md index 235a439..b85c08b 100644 --- a/commands/do/release.md +++ b/commands/do/release.md @@ -160,10 +160,57 @@ Verification — self-check before proceeding (no user prompt needed): ## Open the Release PR -- Push the source branch to remote (it should already be up to date with the release commit) -- Create a PR from `{source}` → `{target}` (e.g., `main` → `release`) +- **Checkpoint 1 — source push.** Push the prepared source commit and verify the + forge reports the exact same commit before creating or reusing a PR. A successful + `git push` by itself is not proof that the remote ref was updated; empty, + malformed, or mismatched output is an incomplete release and must name + `Source push` as the first unverified checkpoint: ```bash - gh pr create --title "Release v{version}" --base {target} --head {source} --body "..." + git push -u origin "HEAD:refs/heads/{source}" + SOURCE_SHA="$(git rev-parse HEAD)" + REMOTE_SOURCE_SHA="$(git ls-remote --heads origin "refs/heads/{source}" | awk 'NF { print $1; exit }')" + if ! printf '%s\n' "$REMOTE_SOURCE_SHA" | grep -Eq '^[0-9a-f]{40}$' || [ "$REMOTE_SOURCE_SHA" != "$SOURCE_SHA" ]; then + echo "INCOMPLETE — Source push is unverified; expected $SOURCE_SHA, got ${REMOTE_SOURCE_SHA:-empty}. Preserve the prepared release state and retry." + exit 1 + fi + ``` +- **Checkpoint 2 — release PR.** Query all matching PRs for the current source SHA + before creating one. Reuse an open PR, or a merged PR whose head is still this + source SHA when an interrupted rerun already completed it; never create a + duplicate. Missing, empty, malformed, or ambiguous forge output is incomplete + and must name `Release PR` as the first unverified checkpoint. A closed, + unmerged PR is not reusable, so a later run may create a new PR for the newly + pushed source SHA: + ```bash + RELEASE_PRS_JSON="$(gh pr list --state all --base "{target}" --head "{source}" --limit 100 \ + --json number,state,headRefOid,baseRefName,headRefName,url,createdAt)" || { + echo "INCOMPLETE — Release PR is unverified; the forge query failed. Preserve the prepared release state and retry." + exit 1 + } + if ! printf '%s\n' "$RELEASE_PRS_JSON" | jq -e 'type == "array"' >/dev/null; then + echo "INCOMPLETE — Release PR is unverified; the forge returned empty or malformed data. Preserve the prepared release state and retry." + exit 1 + fi + MATCHING_RELEASE_PRS="$(printf '%s\n' "$RELEASE_PRS_JSON" | jq -c --arg sha "$SOURCE_SHA" \ + '[.[] | select(.headRefOid == $sha and (.state == "OPEN" or .state == "MERGED"))]')" + MATCHING_COUNT="$(printf '%s\n' "$MATCHING_RELEASE_PRS" | jq 'length')" + if [ "$MATCHING_COUNT" -gt 1 ]; then + echo "INCOMPLETE — Release PR is ambiguous; more than one open or merged PR matches $SOURCE_SHA. Preserve the prepared release state and investigate." + exit 1 + elif [ "$MATCHING_COUNT" -eq 1 ]; then + PR_NUMBER="$(printf '%s\n' "$MATCHING_RELEASE_PRS" | jq -r '.[0].number')" + PR_URL="$(printf '%s\n' "$MATCHING_RELEASE_PRS" | jq -r '.[0].url')" + else + PR_URL="$(gh pr create --title "Release v{version}" --base "{target}" --head "{source}" --body "...")" || { + echo "INCOMPLETE — Release PR is unverified; creation failed. Preserve the prepared release state and retry without creating another PR." + exit 1 + } + PR_NUMBER="${PR_URL##*/}" + if ! printf '%s\n' "$PR_NUMBER" | grep -Eq '^[0-9]+$'; then + echo "INCOMPLETE — Release PR is unverified; creation returned empty or malformed data. Preserve the prepared release state and retry." + exit 1 + fi + fi ``` - Title: `Release v{version}` (read version from package.json or equivalent) - Body: include the changelog content for this version if available, otherwise summarize commits since last release @@ -248,20 +295,111 @@ For `dirty` or `inconclusive`: ```bash gh pr merge --merge ``` -- Verify the merge succeeded: `gh pr view --json state,mergedAt` +- **Checkpoint 3 — merged release PR.** Do not infer completion from the merge + command's exit status. Read back all three remote fields and require a merged + state, a non-empty merge timestamp, and a non-empty merge commit. Empty, + malformed, timed-out, queued, or otherwise inconclusive output is incomplete; + name `Merged release PR` as the first unverified checkpoint and preserve the + prepared state: + ```bash + MERGE_JSON="$(gh pr view "$PR_NUMBER" --json state,mergedAt,mergeCommit)" || { + echo "INCOMPLETE — Merged release PR is unverified; the forge query failed. Preserve the prepared release state and retry." + exit 1 + } + if ! printf '%s\n' "$MERGE_JSON" | jq -e \ + 'type == "object" and .state == "MERGED" and (.mergedAt | type == "string") and (.mergedAt | length > 0) and (.mergeCommit.oid | type == "string") and (.mergeCommit.oid | length > 0)' >/dev/null; then + echo "INCOMPLETE — Merged release PR is unverified; state, mergedAt, or mergeCommit is missing or not MERGED. Preserve the prepared release state and retry." + exit 1 + fi + MERGE_COMMIT="$(printf '%s\n' "$MERGE_JSON" | jq -r '.mergeCommit.oid')" + ``` ## Post-Merge -1. **Tag the release** on the target branch to trigger the publish workflow. Refuse to overwrite an existing tag — a colliding `v{version}` usually means the version bump heuristic picked an already-released value or a prior partial release left state behind, both of which need human attention before force-tagging would be safe: +1. **Checkpoint 4 — target-branch tree.** Fetch the target and verify its remote + ref is a real commit with a tree that contains the merged release commit. This + proves the source-to-target promotion landed; checking only PR state would miss + a queued or otherwise incomplete target update. If any command is empty, + malformed, timed out, or fails, report `Target branch tree` as the first + unverified checkpoint and do not create or reuse a tag: + ```bash + git fetch origin "{target}" || { + echo "INCOMPLETE — Target branch tree is unverified; fetching {target} failed. Preserve the prepared release state and retry." + exit 1 + } + TARGET_SHA="$(git ls-remote --heads origin "refs/heads/{target}" | awk 'NF { print $1; exit }')" + if ! printf '%s\n' "$TARGET_SHA" | grep -Eq '^[0-9a-f]{40}$' \ + || ! git cat-file -e "$TARGET_SHA^{tree}" 2>/dev/null \ + || ! git merge-base --is-ancestor "$MERGE_COMMIT" "$TARGET_SHA"; then + echo "INCOMPLETE — Target branch tree is unverified; expected {target} to contain $MERGE_COMMIT, got ${TARGET_SHA:-empty}. Preserve the prepared release state and retry." + exit 1 + fi + ``` +2. **Checkpoint 5 — version tag.** Publish `v{version}` only when it is absent; + on a rerun, reuse it only if its remotely resolved commit is exactly + `TARGET_SHA`. Never force-push or overwrite a conflicting tag. A failed push + may have raced with another successful publisher, so re-read the tag before + reporting failure; empty, malformed, mismatched, or inconclusive output names + `Version tag` as the first unverified checkpoint: + ```bash + TAG_SHA="$(git ls-remote origin "refs/tags/v{version}^{}" | awk 'NF { print $1; exit }')" + if ! printf '%s\n' "$TAG_SHA" | grep -Eq '^[0-9a-f]{40}$'; then + TAG_SHA="$(git ls-remote origin "refs/tags/v{version}" | awk 'NF { print $1; exit }')" + fi + if printf '%s\n' "$TAG_SHA" | grep -Eq '^[0-9a-f]{40}$'; then + if [ "$TAG_SHA" != "$TARGET_SHA" ]; then + echo "INCOMPLETE — Version tag v{version} points to $TAG_SHA, not target commit $TARGET_SHA; refusing to overwrite it." + exit 1 + fi + else + if git rev-parse -q --verify "refs/tags/v{version}" >/dev/null 2>&1; then + LOCAL_TAG_SHA="$(git rev-parse "v{version}^{commit}")" + if [ "$LOCAL_TAG_SHA" != "$TARGET_SHA" ]; then + echo "INCOMPLETE — Local version tag v{version} points to $LOCAL_TAG_SHA, not target commit $TARGET_SHA; refusing to overwrite it." + exit 1 + fi + else + git tag "v{version}" "$TARGET_SHA" || { + echo "INCOMPLETE — Version tag is unverified; local tag creation failed. Preserve the prepared release state and retry." + exit 1 + } + fi + git push origin "refs/tags/v{version}" || true + TAG_SHA="$(git ls-remote origin "refs/tags/v{version}^{}" | awk 'NF { print $1; exit }')" + if ! printf '%s\n' "$TAG_SHA" | grep -Eq '^[0-9a-f]{40}$'; then + TAG_SHA="$(git ls-remote origin "refs/tags/v{version}" | awk 'NF { print $1; exit }')" + fi + if [ "$TAG_SHA" != "$TARGET_SHA" ]; then + echo "INCOMPLETE — Version tag is unverified; expected $TARGET_SHA, got ${TAG_SHA:-empty}. Preserve the prepared release state and retry." + exit 1 + fi + fi + ``` +3. **Checkpoint 6 — GitHub Release.** The release workflow may need time to + publish after the tag. Poll for a published, non-draft, non-prerelease release + whose tag is `v{version}` for a bounded period. Missing, empty, malformed, or + timed-out output is incomplete, not success; name `GitHub Release` as the first + unverified checkpoint and preserve the prepared release state: ```bash - git fetch origin {target} 'refs/tags/*:refs/tags/*' - if git rev-parse -q --verify "refs/tags/v{version}" >/dev/null; then - echo "Tag v{version} already exists. Aborting tag step. Investigate (rerun version bump? force-tag manually?) before retrying." + RELEASE_JSON="" + for ATTEMPT in $(seq 1 30); do + RELEASE_JSON="$(gh release view "v{version}" --json tagName,isDraft,isPrerelease,publishedAt 2>/dev/null || true)" + if printf '%s\n' "$RELEASE_JSON" | jq -e \ + 'type == "object" and .tagName == "v{version}" and .isDraft == false and .isPrerelease == false and (.publishedAt | type == "string") and (.publishedAt | length > 0)' >/dev/null 2>&1; then + break + fi + RELEASE_JSON="" + sleep 10 + done + if ! printf '%s\n' "$RELEASE_JSON" | jq -e \ + 'type == "object" and .tagName == "v{version}" and .isDraft == false and .isPrerelease == false and (.publishedAt | type == "string") and (.publishedAt | length > 0)' >/dev/null 2>&1; then + echo "INCOMPLETE — GitHub Release is unverified after the bounded wait; preserve the prepared release state and retry." exit 1 fi - git tag v{version} origin/{target} - git push origin v{version} ``` -2. **Switch back to the source branch** locally: `git checkout {source} && git pull --rebase --autostash` -3. **Report the final status** including version, PR URL, tag, and merge state -4. Remind the user to check for the GitHub release once CI completes (if the project uses automated releases) +4. **Only after all six checkpoints pass** report the release as complete, including + the source SHA, PR URL and merged state, target SHA, tag SHA, and published + GitHub Release. A local prepared commit, a successful PR merge command, or a + pushed tag is never sufficient on its own. Switch back to the source branch + locally only after the remote verification succeeds: + `git checkout {source} && git pull --rebase --autostash`. diff --git a/test/release-contract.test.js b/test/release-contract.test.js new file mode 100644 index 0000000..520278d --- /dev/null +++ b/test/release-contract.test.js @@ -0,0 +1,65 @@ +'use strict'; + +const { describe, it } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const body = fs.readFileSync(path.join(__dirname, '..', 'commands', 'do', 'release.md'), 'utf8'); + +describe('/do:release remote promotion contracts', () => { + it('requires ordered remote checkpoints before reporting completion', () => { + const checkpoints = [ + 'Checkpoint 1 — source push', + 'Checkpoint 2 — release PR', + 'Checkpoint 3 — merged release PR', + 'Checkpoint 4 — target-branch tree', + 'Checkpoint 5 — version tag', + 'Checkpoint 6 — GitHub Release', + ]; + let previous = -1; + for (const checkpoint of checkpoints) { + const index = body.indexOf(checkpoint); + assert.ok(index > previous, `${checkpoint} must follow the previous checkpoint`); + previous = index; + } + assert.match(body, /Only after all six checkpoints pass.*report the release as complete/s); + assert.match(body, /[Ee]mpty,\s+malformed, timed-out, queued, or otherwise inconclusive output is incomplete/); + }); + + it('verifies the source push and avoids duplicate PRs on reruns', () => { + assert.match(body, /git push -u origin "HEAD:refs\/heads\/\{source\}"/); + assert.match(body, /REMOTE_SOURCE_SHA="\$\(git ls-remote --heads origin "refs\/heads\/\{source\}"/); + assert.match(body, /REMOTE_SOURCE_SHA.*\[ "\$REMOTE_SOURCE_SHA" != "\$SOURCE_SHA" \]/s); + assert.match(body, /gh pr list --state all --base "\{target\}" --head "\{source\}"/); + assert.match(body, /select\(\.headRefOid == \$sha and \(\.state == "OPEN" or \.state == "MERGED"\)\)/); + assert.match(body, /never create a\s+duplicate/); + }); + + it('requires mergedAt and mergeCommit instead of trusting merge exit status', () => { + assert.match(body, /gh pr view "\$PR_NUMBER" --json state,mergedAt,mergeCommit/); + assert.match(body, /\.state == "MERGED"/); + assert.match(body, /\.mergedAt \| type == "string"/); + assert.match(body, /\.mergeCommit\.oid \| type == "string"/); + assert.match(body, /MERGE_COMMIT="\$\(.*\.mergeCommit\.oid/s); + }); + + it('verifies target ancestry and makes tag publication idempotent', () => { + assert.match(body, /git ls-remote --heads origin "refs\/heads\/\{target\}"/); + assert.match(body, /git cat-file -e "\$TARGET_SHA\^\{tree\}"/); + assert.match(body, /git merge-base --is-ancestor "\$MERGE_COMMIT" "\$TARGET_SHA"/); + assert.match(body, /refs\/tags\/v\{version\}\^\{/); + assert.match(body, /refusing to overwrite it/); + assert.match(body, /git push origin "refs\/tags\/v\{version\}" \|\| true/); + assert.match(body, /expected \$TARGET_SHA, got \$\{TAG_SHA:-empty\}/); + }); + + it('polls for a published GitHub Release and fails closed on timeout', () => { + assert.match(body, /for ATTEMPT in \$\(seq 1 30\)/); + assert.match(body, /gh release view "v\{version\}" --json tagName,isDraft,isPrerelease,publishedAt/); + assert.match(body, /\.tagName == "v\{version\}"/); + assert.match(body, /\.isDraft == false/); + assert.match(body, /\.isPrerelease == false/); + assert.match(body, /GitHub Release is unverified after the bounded wait/); + }); +}); From 2e3c84d6827d519b72f771953ae084801b86d614 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Thu, 27 Aug 2026 07:05:09 +0000 Subject: [PATCH 2/9] fix: make release recovery idempotent (#211) --- commands/do/release.md | 67 +++++++++++++++++++++++++++++------ test/release-contract.test.js | 15 +++++++- 2 files changed, 71 insertions(+), 11 deletions(-) diff --git a/commands/do/release.md b/commands/do/release.md index b85c08b..9922ff6 100644 --- a/commands/do/release.md +++ b/commands/do/release.md @@ -91,8 +91,39 @@ Print the detected workflow: `Detected release flow: {source} → {target}` 4. **Run tests** — execute the project's test suite (per project conventions already in context, or check package.json) 5. **Run build** — execute the project's build command if one exists +## Recover Prepared Release State + +Before determining a new version, look for an existing release-preparation commit +on the current source history. An interrupted run must resume the prepared version, +not bump it again: + +```bash +PREVIOUS_TAG="$(git describe --tags --abbrev=0 2>/dev/null || true)" +if [ -n "$PREVIOUS_TAG" ]; then + PREPARED_RELEASE="$(git log --format='%H%x09%s' "$PREVIOUS_TAG..HEAD" | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print; exit }')" +else + PREPARED_RELEASE="$(git log --format='%H%x09%s' | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print; exit }')" +fi +if [ -n "$PREPARED_RELEASE" ]; then + PREPARED_RELEASE_SHA="$(printf '%s\n' "$PREPARED_RELEASE" | cut -f1)" + VERSION="$(printf '%s\n' "$PREPARED_RELEASE" | sed -E 's/.*release v//')" + echo "Resuming prepared release v${VERSION} at ${PREPARED_RELEASE_SHA}; skipping version bump and changelog generation." +else + echo "No prepared release commit found; determine a new version and finalize its changelog below." +fi +``` + +When `PREPARED_RELEASE` is non-empty, verify that the checked-out package version +is `{version}` and continue directly to **Local Code Review**. Do not determine a +new bump, rewrite release notes, or create another `chore: release` commit. If the +package version does not match the prepared commit's version, fail closed and +preserve the prepared state for investigation. + ## Determine Version and Finalize Changelog +Skip this entire section when `PREPARED_RELEASE` is non-empty; it is only for a +release with no prepared release commit. + 1. **Determine version bump** from commits since the last git tag: - Scan commit messages for conventional commit prefixes (also check each commit's body/footer for `BREAKING CHANGE:` — a recognized way to signal a breaking change without the prefix): - `breaking:`, any prefix with a `!` (e.g. `feat!:`, `fix!:`, `refactor!:`), or a `BREAKING CHANGE:` footer → **major** bump @@ -200,6 +231,7 @@ Verification — self-check before proceeding (no user prompt needed): elif [ "$MATCHING_COUNT" -eq 1 ]; then PR_NUMBER="$(printf '%s\n' "$MATCHING_RELEASE_PRS" | jq -r '.[0].number')" PR_URL="$(printf '%s\n' "$MATCHING_RELEASE_PRS" | jq -r '.[0].url')" + PR_STATE="$(printf '%s\n' "$MATCHING_RELEASE_PRS" | jq -r '.[0].state')" else PR_URL="$(gh pr create --title "Release v{version}" --base "{target}" --head "{source}" --body "...")" || { echo "INCOMPLETE — Release PR is unverified; creation failed. Preserve the prepared release state and retry without creating another PR." @@ -210,6 +242,7 @@ Verification — self-check before proceeding (no user prompt needed): echo "INCOMPLETE — Release PR is unverified; creation returned empty or malformed data. Preserve the prepared release state and retry." exit 1 fi + PR_STATE="OPEN" fi ``` - Title: `Release v{version}` (read version from package.json or equivalent) @@ -220,6 +253,10 @@ Verification — self-check before proceeding (no user prompt needed): ## Run the Review Loop +If the selected PR already has `PR_STATE=MERGED`, skip this section entirely. +Do not request another review or treat an already-merged PR as an open merge +candidate; set `OVERALL_STATUS=clean` for the post-merge verification path. + **If `REVIEW_AGENTS` is empty** (no `--review-with` was passed), skip this entire section — no external review loop runs. The Local Code Review gate above plus the passing build/tests are the merge gate; set `OVERALL_STATUS=clean` (no-review path) and proceed to the merge section. The Copilot-specific and local-agent-specific merge checks below do not apply when no reviewer ran. Otherwise, hand off to the **multi-reviewer loop** with the parsed inputs: @@ -289,6 +326,11 @@ For `dirty` or `inconclusive`: ### Merging (after all checks above pass) +If `PR_STATE=MERGED`, skip the CI gate and merge command below and continue +directly to **Checkpoint 3**. Otherwise, run the gate and merge command. This +conditional is required for an interrupted rerun to recover from a merge that +already succeeded remotely. + - **Gate on required CI first.** If the repo has required checks on the target branch, watch them in-session before merging: `gh pr checks --required --watch --fail-fast`. (If `gh` reports no required checks, this gate is vacuously satisfied — merge directly.) - On a required-check **failure**, apply the **CI flake handling** routine — one conservative re-run on the same commit (see `~/.claude/lib/ci-flake-handling.md` and the inlined copy above). If the same SHA passes on the single re-run, treat it as a flake and proceed (logging which check flaked); if it fails again, **abort the release merge** and report which check failed. A release must never merge over a real red. - Once confirmed clean, merge: @@ -312,6 +354,7 @@ For `dirty` or `inconclusive`: exit 1 fi MERGE_COMMIT="$(printf '%s\n' "$MERGE_JSON" | jq -r '.mergeCommit.oid')" + RELEASE_TREE="$(git rev-parse "$MERGE_COMMIT^{tree}")" ``` ## Post-Merge @@ -336,8 +379,10 @@ For `dirty` or `inconclusive`: fi ``` 2. **Checkpoint 5 — version tag.** Publish `v{version}` only when it is absent; - on a rerun, reuse it only if its remotely resolved commit is exactly - `TARGET_SHA`. Never force-push or overwrite a conflicting tag. A failed push + on a rerun, reuse it only if its remotely resolved commit has the exact + `RELEASE_TREE` from the merged release commit. The target branch may advance + with workflow housekeeping after the merge, so do not use its moving tip as + the tag identity. Never force-push or overwrite a conflicting tag. A failed push may have raced with another successful publisher, so re-read the tag before reporting failure; empty, malformed, mismatched, or inconclusive output names `Version tag` as the first unverified checkpoint: @@ -347,19 +392,20 @@ For `dirty` or `inconclusive`: TAG_SHA="$(git ls-remote origin "refs/tags/v{version}" | awk 'NF { print $1; exit }')" fi if printf '%s\n' "$TAG_SHA" | grep -Eq '^[0-9a-f]{40}$'; then - if [ "$TAG_SHA" != "$TARGET_SHA" ]; then - echo "INCOMPLETE — Version tag v{version} points to $TAG_SHA, not target commit $TARGET_SHA; refusing to overwrite it." + TAG_TREE="$(git rev-parse "$TAG_SHA^{tree}" 2>/dev/null || true)" + if [ "$TAG_TREE" != "$RELEASE_TREE" ]; then + echo "INCOMPLETE — Version tag v{version} points to tree ${TAG_TREE:-empty}, not merged release tree $RELEASE_TREE; refusing to overwrite it." exit 1 fi else if git rev-parse -q --verify "refs/tags/v{version}" >/dev/null 2>&1; then - LOCAL_TAG_SHA="$(git rev-parse "v{version}^{commit}")" - if [ "$LOCAL_TAG_SHA" != "$TARGET_SHA" ]; then - echo "INCOMPLETE — Local version tag v{version} points to $LOCAL_TAG_SHA, not target commit $TARGET_SHA; refusing to overwrite it." + LOCAL_TAG_TREE="$(git rev-parse "v{version}^{tree}" 2>/dev/null || true)" + if [ "$LOCAL_TAG_TREE" != "$RELEASE_TREE" ]; then + echo "INCOMPLETE — Local version tag v{version} points to tree ${LOCAL_TAG_TREE:-empty}, not merged release tree $RELEASE_TREE; refusing to overwrite it." exit 1 fi else - git tag "v{version}" "$TARGET_SHA" || { + git tag "v{version}" "$MERGE_COMMIT" || { echo "INCOMPLETE — Version tag is unverified; local tag creation failed. Preserve the prepared release state and retry." exit 1 } @@ -369,8 +415,9 @@ For `dirty` or `inconclusive`: if ! printf '%s\n' "$TAG_SHA" | grep -Eq '^[0-9a-f]{40}$'; then TAG_SHA="$(git ls-remote origin "refs/tags/v{version}" | awk 'NF { print $1; exit }')" fi - if [ "$TAG_SHA" != "$TARGET_SHA" ]; then - echo "INCOMPLETE — Version tag is unverified; expected $TARGET_SHA, got ${TAG_SHA:-empty}. Preserve the prepared release state and retry." + TAG_TREE="$(git rev-parse "$TAG_SHA^{tree}" 2>/dev/null || true)" + if [ "$TAG_TREE" != "$RELEASE_TREE" ]; then + echo "INCOMPLETE — Version tag is unverified; expected merged release tree $RELEASE_TREE, got ${TAG_TREE:-empty}. Preserve the prepared release state and retry." exit 1 fi fi diff --git a/test/release-contract.test.js b/test/release-contract.test.js index 520278d..9285872 100644 --- a/test/release-contract.test.js +++ b/test/release-contract.test.js @@ -36,6 +36,16 @@ describe('/do:release remote promotion contracts', () => { assert.match(body, /never create a\s+duplicate/); }); + it('resumes prepared releases before version bumping and merged PRs after review', () => { + const recovery = body.indexOf('## Recover Prepared Release State'); + const determineVersion = body.indexOf('## Determine Version and Finalize Changelog'); + assert.ok(recovery >= 0 && recovery < determineVersion, 'prepared recovery must precede version determination'); + assert.match(body, /Skip this entire section when `PREPARED_RELEASE` is non-empty/); + assert.match(body, /PR_STATE="\$\(printf '[^\n]+' \"\$MATCHING_RELEASE_PRS\" \| jq -r '\.\[0\]\.state'/); + assert.match(body, /If the selected PR already has `PR_STATE=MERGED`, skip this section entirely[\s\S]*?Do not request another review/); + assert.match(body, /If `PR_STATE=MERGED`, skip the CI gate and merge command below/); + }); + it('requires mergedAt and mergeCommit instead of trusting merge exit status', () => { assert.match(body, /gh pr view "\$PR_NUMBER" --json state,mergedAt,mergeCommit/); assert.match(body, /\.state == "MERGED"/); @@ -51,7 +61,10 @@ describe('/do:release remote promotion contracts', () => { assert.match(body, /refs\/tags\/v\{version\}\^\{/); assert.match(body, /refusing to overwrite it/); assert.match(body, /git push origin "refs\/tags\/v\{version\}" \|\| true/); - assert.match(body, /expected \$TARGET_SHA, got \$\{TAG_SHA:-empty\}/); + assert.match(body, /RELEASE_TREE="\$\(git rev-parse "\$MERGE_COMMIT\^\{tree\}"\)"/); + assert.match(body, /TAG_TREE.*RELEASE_TREE/); + assert.match(body, /git tag "v\{version\}" "\$MERGE_COMMIT"/); + assert.match(body, /expected merged release tree \$RELEASE_TREE, got \$\{TAG_TREE:-empty\}/); }); it('polls for a published GitHub Release and fails closed on timeout', () => { From f66516084671ca9a1fd4634a83a29129d4487b48 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Thu, 27 Aug 2026 07:14:35 +0000 Subject: [PATCH 3/9] fix: harden release checkpoint handoff (#211) --- commands/do/release.md | 109 +++++++++++++++++++++++----------- test/release-contract.test.js | 13 ++-- 2 files changed, 82 insertions(+), 40 deletions(-) diff --git a/commands/do/release.md b/commands/do/release.md index 9922ff6..3cc8de6 100644 --- a/commands/do/release.md +++ b/commands/do/release.md @@ -73,6 +73,13 @@ Before doing anything, determine the project's source and target branches for re ``` This ensures the PR diff shows ALL changes since the last release, not just the version bump. +4. **Detect GitHub Release publication** — set `{publishes_github_release}` to true + only when the documented workflow or release instructions publish a GitHub + Release (for example, they use `gh release`, `softprops/action-gh-release`, or + an equivalent release action). Projects that publish only packages or tags do + not have a GitHub Release checkpoint; their successful completion ends after + the version-tag checkpoint. + Print the detected workflow: `Detected release flow: {source} → {target}` **Default mode**: If ambiguous, use the most likely branch (prefer `release` if it exists). If the target branch does not exist, create it from the last release tag (see step 3 above). If detection still yields `target == source`, abort with an error — a release PR cannot merge a branch into itself. **Interactive mode (`--interactive`)**: Ask the user to confirm before proceeding. @@ -98,9 +105,9 @@ on the current source history. An interrupted run must resume the prepared versi not bump it again: ```bash -PREVIOUS_TAG="$(git describe --tags --abbrev=0 2>/dev/null || true)" -if [ -n "$PREVIOUS_TAG" ]; then - PREPARED_RELEASE="$(git log --format='%H%x09%s' "$PREVIOUS_TAG..HEAD" | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print; exit }')" +git fetch origin "refs/heads/{target}:refs/remotes/origin/{target}" >/dev/null 2>&1 || true +if git show-ref --verify --quiet "refs/remotes/origin/{target}"; then + PREPARED_RELEASE="$(git log --format='%H%x09%s' "origin/{target}..HEAD" | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print; exit }')" else PREPARED_RELEASE="$(git log --format='%H%x09%s' | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print; exit }')" fi @@ -213,6 +220,12 @@ Verification — self-check before proceeding (no user prompt needed): unmerged PR is not reusable, so a later run may create a new PR for the newly pushed source SHA: ```bash + SOURCE_SHA="$(git rev-parse HEAD)" + REMOTE_SOURCE_SHA="$(git ls-remote --heads origin "refs/heads/{source}" | awk 'NF { print $1; exit }')" + if ! printf '%s\n' "$REMOTE_SOURCE_SHA" | grep -Eq '^[0-9a-f]{40}$' || [ "$REMOTE_SOURCE_SHA" != "$SOURCE_SHA" ]; then + echo "INCOMPLETE — Source push is unverified; expected $SOURCE_SHA, got ${REMOTE_SOURCE_SHA:-empty}. Preserve the prepared release state and retry." + exit 1 + fi RELEASE_PRS_JSON="$(gh pr list --state all --base "{target}" --head "{source}" --limit 100 \ --json number,state,headRefOid,baseRefName,headRefName,url,createdAt)" || { echo "INCOMPLETE — Release PR is unverified; the forge query failed. Preserve the prepared release state and retry." @@ -335,7 +348,14 @@ already succeeded remotely. - On a required-check **failure**, apply the **CI flake handling** routine — one conservative re-run on the same commit (see `~/.claude/lib/ci-flake-handling.md` and the inlined copy above). If the same SHA passes on the single re-run, treat it as a flake and proceed (logging which check flaked); if it fails again, **abort the release merge** and report which check failed. A release must never merge over a real red. - Once confirmed clean, merge: ```bash - gh pr merge --merge + PR_NUMBER="" + CURRENT_PR_STATE="$(gh pr view "$PR_NUMBER" --json state -q .state)" || { + echo "INCOMPLETE — Merged release PR is unverified; the forge state query failed. Preserve the prepared release state and retry." + exit 1 + } + if [ "$CURRENT_PR_STATE" != "MERGED" ]; then + gh pr merge "$PR_NUMBER" --merge + fi ``` - **Checkpoint 3 — merged release PR.** Do not infer completion from the merge command's exit status. Read back all three remote fields and require a merged @@ -343,7 +363,12 @@ already succeeded remotely. malformed, timed-out, queued, or otherwise inconclusive output is incomplete; name `Merged release PR` as the first unverified checkpoint and preserve the prepared state: + Run the Checkpoint 3 through Checkpoint 6 blocks below as one shell invocation; + this keeps their verified values together. Substitute the selected PR number + for `` in the invocation rather than relying on a variable from an + earlier shell call. ```bash + PR_NUMBER="" MERGE_JSON="$(gh pr view "$PR_NUMBER" --json state,mergedAt,mergeCommit)" || { echo "INCOMPLETE — Merged release PR is unverified; the forge query failed. Preserve the prepared release state and retry." exit 1 @@ -354,7 +379,10 @@ already succeeded remotely. exit 1 fi MERGE_COMMIT="$(printf '%s\n' "$MERGE_JSON" | jq -r '.mergeCommit.oid')" - RELEASE_TREE="$(git rev-parse "$MERGE_COMMIT^{tree}")" + RELEASE_TREE="$(git rev-parse --verify --quiet "$MERGE_COMMIT^{tree}")" || { + echo "INCOMPLETE — Merged release PR is unverified; the merge commit tree could not be read locally. Preserve the prepared release state and retry." + exit 1 + } ``` ## Post-Merge @@ -364,44 +392,48 @@ already succeeded remotely. proves the source-to-target promotion landed; checking only PR state would miss a queued or otherwise incomplete target update. If any command is empty, malformed, timed out, or fails, report `Target branch tree` as the first - unverified checkpoint and do not create or reuse a tag: + unverified checkpoint and do not create or reuse a tag. The Checkpoint 3 through + Checkpoint 6 commands below must run as one shell invocation so verified values + survive between checkpoints: ```bash - git fetch origin "{target}" || { + # Checkpoint 4 — FETCH_HEAD pins the exact target ref fetched; do not resolve + # a second moving tip with ls-remote. + git fetch origin "refs/heads/{target}" || { echo "INCOMPLETE — Target branch tree is unverified; fetching {target} failed. Preserve the prepared release state and retry." exit 1 } - TARGET_SHA="$(git ls-remote --heads origin "refs/heads/{target}" | awk 'NF { print $1; exit }')" + TARGET_SHA="$(git rev-parse --verify --quiet FETCH_HEAD^{commit} || true)" if ! printf '%s\n' "$TARGET_SHA" | grep -Eq '^[0-9a-f]{40}$' \ || ! git cat-file -e "$TARGET_SHA^{tree}" 2>/dev/null \ || ! git merge-base --is-ancestor "$MERGE_COMMIT" "$TARGET_SHA"; then echo "INCOMPLETE — Target branch tree is unverified; expected {target} to contain $MERGE_COMMIT, got ${TARGET_SHA:-empty}. Preserve the prepared release state and retry." exit 1 fi - ``` -2. **Checkpoint 5 — version tag.** Publish `v{version}` only when it is absent; - on a rerun, reuse it only if its remotely resolved commit has the exact - `RELEASE_TREE` from the merged release commit. The target branch may advance - with workflow housekeeping after the merge, so do not use its moving tip as - the tag identity. Never force-push or overwrite a conflicting tag. A failed push - may have raced with another successful publisher, so re-read the tag before - reporting failure; empty, malformed, mismatched, or inconclusive output names - `Version tag` as the first unverified checkpoint: - ```bash + # Checkpoint 5 — version tag. A workflow may add housekeeping commits after + # the merge, so accept only a tag on the merged-release lineage, never an + # unrelated or stale tag, and never overwrite an existing tag. A failed push + # may have raced with another successful publisher, so re-read the tag before + # reporting failure. TAG_SHA="$(git ls-remote origin "refs/tags/v{version}^{}" | awk 'NF { print $1; exit }')" if ! printf '%s\n' "$TAG_SHA" | grep -Eq '^[0-9a-f]{40}$'; then TAG_SHA="$(git ls-remote origin "refs/tags/v{version}" | awk 'NF { print $1; exit }')" fi if printf '%s\n' "$TAG_SHA" | grep -Eq '^[0-9a-f]{40}$'; then - TAG_TREE="$(git rev-parse "$TAG_SHA^{tree}" 2>/dev/null || true)" - if [ "$TAG_TREE" != "$RELEASE_TREE" ]; then - echo "INCOMPLETE — Version tag v{version} points to tree ${TAG_TREE:-empty}, not merged release tree $RELEASE_TREE; refusing to overwrite it." + TAG_COMMIT="$TAG_SHA" + if ! git merge-base --is-ancestor "$MERGE_COMMIT" "$TAG_COMMIT" \ + || ! git merge-base --is-ancestor "$TAG_COMMIT" "$TARGET_SHA"; then + echo "INCOMPLETE — Version tag v{version} is not on the merged release lineage; refusing to overwrite it." exit 1 fi else - if git rev-parse -q --verify "refs/tags/v{version}" >/dev/null 2>&1; then - LOCAL_TAG_TREE="$(git rev-parse "v{version}^{tree}" 2>/dev/null || true)" - if [ "$LOCAL_TAG_TREE" != "$RELEASE_TREE" ]; then - echo "INCOMPLETE — Local version tag v{version} points to tree ${LOCAL_TAG_TREE:-empty}, not merged release tree $RELEASE_TREE; refusing to overwrite it." + if git rev-parse --verify --quiet "refs/tags/v{version}^{commit}" >/dev/null; then + LOCAL_TAG_COMMIT="$(git rev-parse --verify --quiet "refs/tags/v{version}^{commit}")" || { + echo "INCOMPLETE — Version tag is unverified; the local tag could not be read. Preserve the prepared release state and retry." + exit 1 + } + if ! git merge-base --is-ancestor "$MERGE_COMMIT" "$LOCAL_TAG_COMMIT" \ + || ! git merge-base --is-ancestor "$LOCAL_TAG_COMMIT" "$TARGET_SHA"; then + echo "INCOMPLETE — Local version tag v{version} is not on the merged release lineage; refusing to overwrite it." exit 1 fi else @@ -415,19 +447,19 @@ already succeeded remotely. if ! printf '%s\n' "$TAG_SHA" | grep -Eq '^[0-9a-f]{40}$'; then TAG_SHA="$(git ls-remote origin "refs/tags/v{version}" | awk 'NF { print $1; exit }')" fi - TAG_TREE="$(git rev-parse "$TAG_SHA^{tree}" 2>/dev/null || true)" - if [ "$TAG_TREE" != "$RELEASE_TREE" ]; then - echo "INCOMPLETE — Version tag is unverified; expected merged release tree $RELEASE_TREE, got ${TAG_TREE:-empty}. Preserve the prepared release state and retry." + TAG_COMMIT="$TAG_SHA" + if ! printf '%s\n' "$TAG_COMMIT" | grep -Eq '^[0-9a-f]{40}$' \ + || ! git merge-base --is-ancestor "$MERGE_COMMIT" "$TAG_COMMIT" \ + || ! git merge-base --is-ancestor "$TAG_COMMIT" "$TARGET_SHA"; then + echo "INCOMPLETE — Version tag is unverified; expected a tag on the merged release lineage, got ${TAG_COMMIT:-empty}. Preserve the prepared release state and retry." exit 1 fi fi - ``` -3. **Checkpoint 6 — GitHub Release.** The release workflow may need time to - publish after the tag. Poll for a published, non-draft, non-prerelease release - whose tag is `v{version}` for a bounded period. Missing, empty, malformed, or - timed-out output is incomplete, not success; name `GitHub Release` as the first - unverified checkpoint and preserve the prepared release state: - ```bash + + # Checkpoint 6 — GitHub Release. The release workflow may need time to publish + # after the tag. Missing, empty, malformed, or timed-out output is incomplete + # when GitHub Release publication is part of the documented workflow. + if [ "{publishes_github_release}" = "true" ]; then RELEASE_JSON="" for ATTEMPT in $(seq 1 30); do RELEASE_JSON="$(gh release view "v{version}" --json tagName,isDraft,isPrerelease,publishedAt 2>/dev/null || true)" @@ -436,13 +468,18 @@ already succeeded remotely. break fi RELEASE_JSON="" - sleep 10 + [ "$ATTEMPT" -lt 30 ] && sleep 10 done if ! printf '%s\n' "$RELEASE_JSON" | jq -e \ 'type == "object" and .tagName == "v{version}" and .isDraft == false and .isPrerelease == false and (.publishedAt | type == "string") and (.publishedAt | length > 0)' >/dev/null 2>&1; then echo "INCOMPLETE — GitHub Release is unverified after the bounded wait; preserve the prepared release state and retry." exit 1 fi + else + echo "Checkpoint 6 — GitHub Release: skipped because the documented workflow does not publish one." + fi + + echo "COMPLETE — source $SOURCE_SHA; PR $PR_NUMBER merged at $MERGE_COMMIT; target $TARGET_SHA; tag $TAG_COMMIT." ``` 4. **Only after all six checkpoints pass** report the release as complete, including the source SHA, PR URL and merged state, target SHA, tag SHA, and published diff --git a/test/release-contract.test.js b/test/release-contract.test.js index 9285872..8a9dbe9 100644 --- a/test/release-contract.test.js +++ b/test/release-contract.test.js @@ -41,6 +41,8 @@ describe('/do:release remote promotion contracts', () => { const determineVersion = body.indexOf('## Determine Version and Finalize Changelog'); assert.ok(recovery >= 0 && recovery < determineVersion, 'prepared recovery must precede version determination'); assert.match(body, /Skip this entire section when `PREPARED_RELEASE` is non-empty/); + assert.match(body, /PREPARED_RELEASE="\$\(git log[^\n]+origin\/\{target\}\.\.HEAD/); + assert.doesNotMatch(body, /PREVIOUS_TAG=.*git describe/); assert.match(body, /PR_STATE="\$\(printf '[^\n]+' \"\$MATCHING_RELEASE_PRS\" \| jq -r '\.\[0\]\.state'/); assert.match(body, /If the selected PR already has `PR_STATE=MERGED`, skip this section entirely[\s\S]*?Do not request another review/); assert.match(body, /If `PR_STATE=MERGED`, skip the CI gate and merge command below/); @@ -55,16 +57,19 @@ describe('/do:release remote promotion contracts', () => { }); it('verifies target ancestry and makes tag publication idempotent', () => { - assert.match(body, /git ls-remote --heads origin "refs\/heads\/\{target\}"/); + assert.match(body, /git fetch origin "refs\/heads\/\{target\}"/); assert.match(body, /git cat-file -e "\$TARGET_SHA\^\{tree\}"/); assert.match(body, /git merge-base --is-ancestor "\$MERGE_COMMIT" "\$TARGET_SHA"/); assert.match(body, /refs\/tags\/v\{version\}\^\{/); assert.match(body, /refusing to overwrite it/); assert.match(body, /git push origin "refs\/tags\/v\{version\}" \|\| true/); - assert.match(body, /RELEASE_TREE="\$\(git rev-parse "\$MERGE_COMMIT\^\{tree\}"\)"/); - assert.match(body, /TAG_TREE.*RELEASE_TREE/); + assert.match(body, /RELEASE_TREE="\$\(git rev-parse --verify --quiet "\$MERGE_COMMIT\^\{tree\}"\)/); + assert.match(body, /git rev-parse --verify --quiet FETCH_HEAD\^\{commit\}/); + assert.match(body, /git merge-base --is-ancestor "\$MERGE_COMMIT" "\$TAG_COMMIT"/); + assert.match(body, /git merge-base --is-ancestor "\$TAG_COMMIT" "\$TARGET_SHA"/); assert.match(body, /git tag "v\{version\}" "\$MERGE_COMMIT"/); - assert.match(body, /expected merged release tree \$RELEASE_TREE, got \$\{TAG_TREE:-empty\}/); + assert.match(body, /publishes_github_release/); + assert.match(body, /\[ "\$ATTEMPT" -lt 30 \] && sleep 10/); }); it('polls for a published GitHub Release and fails closed on timeout', () => { From 088b555b77cc38b34fb22d22ec67abf40ad794e9 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Thu, 27 Aug 2026 07:25:18 +0000 Subject: [PATCH 4/9] fix: preserve release state across retries (#211) --- commands/do/release.md | 43 +++++++++++++++++++++++++++++++---- test/release-contract.test.js | 3 ++- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/commands/do/release.md b/commands/do/release.md index 3cc8de6..8ac2d40 100644 --- a/commands/do/release.md +++ b/commands/do/release.md @@ -108,6 +108,17 @@ not bump it again: git fetch origin "refs/heads/{target}:refs/remotes/origin/{target}" >/dev/null 2>&1 || true if git show-ref --verify --quiet "refs/remotes/origin/{target}"; then PREPARED_RELEASE="$(git log --format='%H%x09%s' "origin/{target}..HEAD" | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print; exit }')" + TARGET_PREPARED_RELEASE="$(git log -1 --format='%H%x09%s' "origin/{target}" | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print }')" + if [ -z "$PREPARED_RELEASE" ] && [ -n "$TARGET_PREPARED_RELEASE" ]; then + TARGET_VERSION="$(printf '%s\n' "$TARGET_PREPARED_RELEASE" | sed -E 's/.*release v//')" + TARGET_TAG="$(git ls-remote origin "refs/tags/v${TARGET_VERSION}^{}" | awk 'NF { print $1; exit }')" + if ! printf '%s\n' "$TARGET_TAG" | grep -Eq '^[0-9a-f]{40}$'; then + TARGET_TAG="$(git ls-remote origin "refs/tags/v${TARGET_VERSION}" | awk 'NF { print $1; exit }')" + fi + if [ -z "$TARGET_TAG" ] || { [ "{publishes_github_release}" = "true" ] && ! gh release view "v${TARGET_VERSION}" >/dev/null 2>&1; }; then + PREPARED_RELEASE="$TARGET_PREPARED_RELEASE" + fi + fi else PREPARED_RELEASE="$(git log --format='%H%x09%s' | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print; exit }')" fi @@ -257,6 +268,7 @@ Verification — self-check before proceeding (no user prompt needed): fi PR_STATE="OPEN" fi + printf 'RELEASE_PR_HANDOFF\tPR_NUMBER=%s\tPR_URL=%s\tPR_STATE=%s\n' "$PR_NUMBER" "$PR_URL" "$PR_STATE" ``` - Title: `Release v{version}` (read version from package.json or equivalent) - Body: include the changelog content for this version if available, otherwise summarize commits since last release @@ -264,6 +276,11 @@ Verification — self-check before proceeding (no user prompt needed): **Note**: Do NOT bump the version for review fixes — the version was already set during the release preparation. +Record the printed `RELEASE_PR_HANDOFF` line and carry its literal `PR_NUMBER`, +`PR_URL`, and `PR_STATE` values into the review and merge steps. Shell variables do +not survive separate tool calls; do not re-expand them later expecting them to be +populated. + ## Run the Review Loop If the selected PR already has `PR_STATE=MERGED`, skip this section entirely. @@ -353,8 +370,11 @@ already succeeded remotely. echo "INCOMPLETE — Merged release PR is unverified; the forge state query failed. Preserve the prepared release state and retry." exit 1 } - if [ "$CURRENT_PR_STATE" != "MERGED" ]; then + if [ "$CURRENT_PR_STATE" = "OPEN" ]; then gh pr merge "$PR_NUMBER" --merge + elif [ "$CURRENT_PR_STATE" != "MERGED" ]; then + echo "INCOMPLETE — Merged release PR is unverified; expected OPEN or MERGED, got ${CURRENT_PR_STATE:-empty}. Preserve the prepared release state and retry." + exit 1 fi ``` - **Checkpoint 3 — merged release PR.** Do not infer completion from the merge @@ -379,10 +399,7 @@ already succeeded remotely. exit 1 fi MERGE_COMMIT="$(printf '%s\n' "$MERGE_JSON" | jq -r '.mergeCommit.oid')" - RELEASE_TREE="$(git rev-parse --verify --quiet "$MERGE_COMMIT^{tree}")" || { - echo "INCOMPLETE — Merged release PR is unverified; the merge commit tree could not be read locally. Preserve the prepared release state and retry." - exit 1 - } + printf 'RELEASE_PR_HANDOFF\tPR_NUMBER=%s\tPR_URL=%s\tPR_STATE=MERGED\tMERGE_COMMIT=%s\n' "$PR_NUMBER" "$(gh pr view "$PR_NUMBER" --json url -q .url)" "$MERGE_COMMIT" ``` ## Post-Merge @@ -396,6 +413,22 @@ already succeeded remotely. Checkpoint 6 commands below must run as one shell invocation so verified values survive between checkpoints: ```bash + PR_NUMBER="" + SOURCE_SHA="$(git rev-parse HEAD)" + MERGE_JSON="$(gh pr view "$PR_NUMBER" --json state,mergedAt,mergeCommit)" || { + echo "INCOMPLETE — Merged release PR is unverified; the forge query failed. Preserve the prepared release state and retry." + exit 1 + } + if ! printf '%s\n' "$MERGE_JSON" | jq -e \ + 'type == "object" and .state == "MERGED" and (.mergedAt | type == "string") and (.mergeCommit.oid | type == "string") and (.mergeCommit.oid | length > 0)' >/dev/null; then + echo "INCOMPLETE — Merged release PR is unverified; the remote merge state is incomplete. Preserve the prepared release state and retry." + exit 1 + fi + MERGE_COMMIT="$(printf '%s\n' "$MERGE_JSON" | jq -r '.mergeCommit.oid')" + PR_URL="$(gh pr view "$PR_NUMBER" --json url -q .url)" || { + echo "INCOMPLETE — Merged release PR is unverified; the PR URL could not be read. Preserve the prepared release state and retry." + exit 1 + } # Checkpoint 4 — FETCH_HEAD pins the exact target ref fetched; do not resolve # a second moving tip with ls-remote. git fetch origin "refs/heads/{target}" || { diff --git a/test/release-contract.test.js b/test/release-contract.test.js index 8a9dbe9..bf743ae 100644 --- a/test/release-contract.test.js +++ b/test/release-contract.test.js @@ -63,13 +63,14 @@ describe('/do:release remote promotion contracts', () => { assert.match(body, /refs\/tags\/v\{version\}\^\{/); assert.match(body, /refusing to overwrite it/); assert.match(body, /git push origin "refs\/tags\/v\{version\}" \|\| true/); - assert.match(body, /RELEASE_TREE="\$\(git rev-parse --verify --quiet "\$MERGE_COMMIT\^\{tree\}"\)/); assert.match(body, /git rev-parse --verify --quiet FETCH_HEAD\^\{commit\}/); assert.match(body, /git merge-base --is-ancestor "\$MERGE_COMMIT" "\$TAG_COMMIT"/); assert.match(body, /git merge-base --is-ancestor "\$TAG_COMMIT" "\$TARGET_SHA"/); assert.match(body, /git tag "v\{version\}" "\$MERGE_COMMIT"/); assert.match(body, /publishes_github_release/); assert.match(body, /\[ "\$ATTEMPT" -lt 30 \] && sleep 10/); + assert.match(body, /TARGET_PREPARED_RELEASE=.*git log -1 .*origin\/\{target\}/); + assert.match(body, /RELEASE_PR_HANDOFF/); }); it('polls for a published GitHub Release and fails closed on timeout', () => { From 570301b8e811551c656027b8f3b7880001657c40 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Thu, 27 Aug 2026 07:34:50 +0000 Subject: [PATCH 5/9] fix: close release retry edge cases (#211) --- commands/do/release.md | 77 ++++++++++++++++++++++------------- test/release-contract.test.js | 5 ++- 2 files changed, 52 insertions(+), 30 deletions(-) diff --git a/commands/do/release.md b/commands/do/release.md index 8ac2d40..8b0cc53 100644 --- a/commands/do/release.md +++ b/commands/do/release.md @@ -105,27 +105,29 @@ on the current source history. An interrupted run must resume the prepared versi not bump it again: ```bash -git fetch origin "refs/heads/{target}:refs/remotes/origin/{target}" >/dev/null 2>&1 || true -if git show-ref --verify --quiet "refs/remotes/origin/{target}"; then - PREPARED_RELEASE="$(git log --format='%H%x09%s' "origin/{target}..HEAD" | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print; exit }')" - TARGET_PREPARED_RELEASE="$(git log -1 --format='%H%x09%s' "origin/{target}" | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print }')" - if [ -z "$PREPARED_RELEASE" ] && [ -n "$TARGET_PREPARED_RELEASE" ]; then - TARGET_VERSION="$(printf '%s\n' "$TARGET_PREPARED_RELEASE" | sed -E 's/.*release v//')" - TARGET_TAG="$(git ls-remote origin "refs/tags/v${TARGET_VERSION}^{}" | awk 'NF { print $1; exit }')" - if ! printf '%s\n' "$TARGET_TAG" | grep -Eq '^[0-9a-f]{40}$'; then - TARGET_TAG="$(git ls-remote origin "refs/tags/v${TARGET_VERSION}" | awk 'NF { print $1; exit }')" - fi - if [ -z "$TARGET_TAG" ] || { [ "{publishes_github_release}" = "true" ] && ! gh release view "v${TARGET_VERSION}" >/dev/null 2>&1; }; then - PREPARED_RELEASE="$TARGET_PREPARED_RELEASE" - fi +if ! git fetch origin "refs/heads/{target}:refs/remotes/origin/{target}" >/dev/null 2>&1 \ + || ! git show-ref --verify --quiet "refs/remotes/origin/{target}"; then + echo "INCOMPLETE — Prepared release state is unverified; origin/{target} could not be resolved. Preserve the prepared state and retry." + exit 1 +fi + PREPARED_RELEASE="$(git log --extended-regexp --format='%H%x09%s' "origin/{target}..HEAD" | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print; exit }')" + TARGET_PREPARED_RELEASE="$(git log --extended-regexp --format='%H%x09%s' "origin/{target}" | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print; exit }')" +if [ -z "$PREPARED_RELEASE" ] && [ -n "$TARGET_PREPARED_RELEASE" ]; then + TARGET_VERSION="$(printf '%s\n' "$TARGET_PREPARED_RELEASE" | sed -E 's/.*release v//')" + TARGET_TAG="$(git ls-remote origin "refs/tags/v${TARGET_VERSION}^{}" | awk 'NF { print $1; exit }')" + if ! printf '%s\n' "$TARGET_TAG" | grep -Eq '^[0-9a-f]{40}$'; then + TARGET_TAG="$(git ls-remote origin "refs/tags/v${TARGET_VERSION}" | awk 'NF { print $1; exit }')" + fi + TARGET_RELEASE_JSON="$(gh release view "v${TARGET_VERSION}" --json isDraft,isPrerelease,publishedAt 2>/dev/null || true)" + if [ -z "$TARGET_TAG" ] || { [ "{publishes_github_release}" = "true" ] && ! printf '%s\n' "$TARGET_RELEASE_JSON" | jq -e 'type == "object" and .isDraft == false and .isPrerelease == false and (.publishedAt | type == "string") and (.publishedAt | length > 0)' >/dev/null 2>&1; }; then + PREPARED_RELEASE="$TARGET_PREPARED_RELEASE" fi -else - PREPARED_RELEASE="$(git log --format='%H%x09%s' | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print; exit }')" fi if [ -n "$PREPARED_RELEASE" ]; then PREPARED_RELEASE_SHA="$(printf '%s\n' "$PREPARED_RELEASE" | cut -f1)" VERSION="$(printf '%s\n' "$PREPARED_RELEASE" | sed -E 's/.*release v//')" echo "Resuming prepared release v${VERSION} at ${PREPARED_RELEASE_SHA}; skipping version bump and changelog generation." + printf 'RELEASE_PREPARED_HANDOFF\tPREPARED_RELEASE_SHA=%s\tVERSION=%s\n' "$PREPARED_RELEASE_SHA" "$VERSION" else echo "No prepared release commit found; determine a new version and finalize its changelog below." fi @@ -217,6 +219,11 @@ Verification — self-check before proceeding (no user prompt needed): ```bash git push -u origin "HEAD:refs/heads/{source}" SOURCE_SHA="$(git rev-parse HEAD)" + PREPARED_RELEASE_SHA="$(git log --extended-regexp --format='%H' --grep='^chore: release v[0-9]+\.[0-9]+\.[0-9]+$' -n 1)" + if ! printf '%s\n' "$PREPARED_RELEASE_SHA" | grep -Eq '^[0-9a-f]{40}$'; then + echo "INCOMPLETE — Prepared release state is unverified; the release preparation commit could not be identified. Preserve the prepared state and retry." + exit 1 + fi REMOTE_SOURCE_SHA="$(git ls-remote --heads origin "refs/heads/{source}" | awk 'NF { print $1; exit }')" if ! printf '%s\n' "$REMOTE_SOURCE_SHA" | grep -Eq '^[0-9a-f]{40}$' || [ "$REMOTE_SOURCE_SHA" != "$SOURCE_SHA" ]; then echo "INCOMPLETE — Source push is unverified; expected $SOURCE_SHA, got ${REMOTE_SOURCE_SHA:-empty}. Preserve the prepared release state and retry." @@ -232,6 +239,7 @@ Verification — self-check before proceeding (no user prompt needed): pushed source SHA: ```bash SOURCE_SHA="$(git rev-parse HEAD)" + PREPARED_RELEASE_SHA="$(git log --extended-regexp --format='%H' --grep='^chore: release v[0-9]+\.[0-9]+\.[0-9]+$' -n 1)" REMOTE_SOURCE_SHA="$(git ls-remote --heads origin "refs/heads/{source}" | awk 'NF { print $1; exit }')" if ! printf '%s\n' "$REMOTE_SOURCE_SHA" | grep -Eq '^[0-9a-f]{40}$' || [ "$REMOTE_SOURCE_SHA" != "$SOURCE_SHA" ]; then echo "INCOMPLETE — Source push is unverified; expected $SOURCE_SHA, got ${REMOTE_SOURCE_SHA:-empty}. Preserve the prepared release state and retry." @@ -268,7 +276,7 @@ Verification — self-check before proceeding (no user prompt needed): fi PR_STATE="OPEN" fi - printf 'RELEASE_PR_HANDOFF\tPR_NUMBER=%s\tPR_URL=%s\tPR_STATE=%s\n' "$PR_NUMBER" "$PR_URL" "$PR_STATE" + printf 'RELEASE_PR_HANDOFF\tPREPARED_RELEASE_SHA=%s\tPR_NUMBER=%s\tPR_URL=%s\tPR_STATE=%s\n' "$PREPARED_RELEASE_SHA" "$PR_NUMBER" "$PR_URL" "$PR_STATE" ``` - Title: `Release v{version}` (read version from package.json or equivalent) - Body: include the changelog content for this version if available, otherwise summarize commits since last release @@ -276,10 +284,10 @@ Verification — self-check before proceeding (no user prompt needed): **Note**: Do NOT bump the version for review fixes — the version was already set during the release preparation. -Record the printed `RELEASE_PR_HANDOFF` line and carry its literal `PR_NUMBER`, -`PR_URL`, and `PR_STATE` values into the review and merge steps. Shell variables do -not survive separate tool calls; do not re-expand them later expecting them to be -populated. +Record the printed `RELEASE_PREPARED_HANDOFF` and `RELEASE_PR_HANDOFF` lines and +carry their literal `PREPARED_RELEASE_SHA`, `PR_NUMBER`, `PR_URL`, and `PR_STATE` +values into the review and merge steps. Shell variables do not survive separate +tool calls; do not re-expand them later expecting them to be populated. ## Run the Review Loop @@ -325,6 +333,10 @@ Each pass uses the matching single-reviewer loop: ## Merge the PR (only after a CLEAN multi-reviewer result) +If `PR_STATE=MERGED`, skip all review-verdict and CI/merge gates in this section +and continue directly to Checkpoint 3's remote read-back. An already-merged PR +does not need another reviewer verdict to recover its post-merge checkpoints. + The merge gate consumes the **wrapper's `{OVERALL_STATUS}`** plus, for any copilot pass that ran, the standard copilot post-pass checks. ### Wrapper status @@ -383,10 +395,11 @@ already succeeded remotely. malformed, timed-out, queued, or otherwise inconclusive output is incomplete; name `Merged release PR` as the first unverified checkpoint and preserve the prepared state: - Run the Checkpoint 3 through Checkpoint 6 blocks below as one shell invocation; - this keeps their verified values together. Substitute the selected PR number - for `` in the invocation rather than relying on a variable from an - earlier shell call. + Run the Checkpoint 3 through Checkpoint 6 blocks below with a command timeout of + at least 600 seconds and as one shell invocation; + this keeps their verified values together. Substitute the carried preparation + SHA and selected PR number for `` and `` rather + than relying on variables from earlier shell calls. ```bash PR_NUMBER="" MERGE_JSON="$(gh pr view "$PR_NUMBER" --json state,mergedAt,mergeCommit)" || { @@ -413,6 +426,7 @@ already succeeded remotely. Checkpoint 6 commands below must run as one shell invocation so verified values survive between checkpoints: ```bash + PREPARED_RELEASE_SHA="" PR_NUMBER="" SOURCE_SHA="$(git rev-parse HEAD)" MERGE_JSON="$(gh pr view "$PR_NUMBER" --json state,mergedAt,mergeCommit)" || { @@ -453,7 +467,7 @@ already succeeded remotely. fi if printf '%s\n' "$TAG_SHA" | grep -Eq '^[0-9a-f]{40}$'; then TAG_COMMIT="$TAG_SHA" - if ! git merge-base --is-ancestor "$MERGE_COMMIT" "$TAG_COMMIT" \ + if ! git merge-base --is-ancestor "$PREPARED_RELEASE_SHA" "$TAG_COMMIT" \ || ! git merge-base --is-ancestor "$TAG_COMMIT" "$TARGET_SHA"; then echo "INCOMPLETE — Version tag v{version} is not on the merged release lineage; refusing to overwrite it." exit 1 @@ -464,7 +478,7 @@ already succeeded remotely. echo "INCOMPLETE — Version tag is unverified; the local tag could not be read. Preserve the prepared release state and retry." exit 1 } - if ! git merge-base --is-ancestor "$MERGE_COMMIT" "$LOCAL_TAG_COMMIT" \ + if ! git merge-base --is-ancestor "$PREPARED_RELEASE_SHA" "$LOCAL_TAG_COMMIT" \ || ! git merge-base --is-ancestor "$LOCAL_TAG_COMMIT" "$TARGET_SHA"; then echo "INCOMPLETE — Local version tag v{version} is not on the merged release lineage; refusing to overwrite it." exit 1 @@ -482,7 +496,7 @@ already succeeded remotely. fi TAG_COMMIT="$TAG_SHA" if ! printf '%s\n' "$TAG_COMMIT" | grep -Eq '^[0-9a-f]{40}$' \ - || ! git merge-base --is-ancestor "$MERGE_COMMIT" "$TAG_COMMIT" \ + || ! git merge-base --is-ancestor "$PREPARED_RELEASE_SHA" "$TAG_COMMIT" \ || ! git merge-base --is-ancestor "$TAG_COMMIT" "$TARGET_SHA"; then echo "INCOMPLETE — Version tag is unverified; expected a tag on the merged release lineage, got ${TAG_COMMIT:-empty}. Preserve the prepared release state and retry." exit 1 @@ -492,6 +506,13 @@ already succeeded remotely. # Checkpoint 6 — GitHub Release. The release workflow may need time to publish # after the tag. Missing, empty, malformed, or timed-out output is incomplete # when GitHub Release publication is part of the documented workflow. + case "{publishes_github_release}" in + true|false) ;; + *) + echo "INCOMPLETE — GitHub Release publication flag is unresolved; preserve the prepared release state." + exit 1 + ;; + esac if [ "{publishes_github_release}" = "true" ]; then RELEASE_JSON="" for ATTEMPT in $(seq 1 30); do @@ -514,7 +535,7 @@ already succeeded remotely. echo "COMPLETE — source $SOURCE_SHA; PR $PR_NUMBER merged at $MERGE_COMMIT; target $TARGET_SHA; tag $TAG_COMMIT." ``` -4. **Only after all six checkpoints pass** report the release as complete, including +2. **Only after all six checkpoints pass** report the release as complete, including the source SHA, PR URL and merged state, target SHA, tag SHA, and published GitHub Release. A local prepared commit, a successful PR merge command, or a pushed tag is never sufficient on its own. Switch back to the source branch diff --git a/test/release-contract.test.js b/test/release-contract.test.js index bf743ae..4416281 100644 --- a/test/release-contract.test.js +++ b/test/release-contract.test.js @@ -64,13 +64,14 @@ describe('/do:release remote promotion contracts', () => { assert.match(body, /refusing to overwrite it/); assert.match(body, /git push origin "refs\/tags\/v\{version\}" \|\| true/); assert.match(body, /git rev-parse --verify --quiet FETCH_HEAD\^\{commit\}/); - assert.match(body, /git merge-base --is-ancestor "\$MERGE_COMMIT" "\$TAG_COMMIT"/); + assert.match(body, /git merge-base --is-ancestor "\$PREPARED_RELEASE_SHA" "\$TAG_COMMIT"/); assert.match(body, /git merge-base --is-ancestor "\$TAG_COMMIT" "\$TARGET_SHA"/); assert.match(body, /git tag "v\{version\}" "\$MERGE_COMMIT"/); assert.match(body, /publishes_github_release/); assert.match(body, /\[ "\$ATTEMPT" -lt 30 \] && sleep 10/); - assert.match(body, /TARGET_PREPARED_RELEASE=.*git log -1 .*origin\/\{target\}/); + assert.match(body, /TARGET_PREPARED_RELEASE=.*git log --format=.*origin\/\{target\}/); assert.match(body, /RELEASE_PR_HANDOFF/); + assert.match(body, /case "\{publishes_github_release\}" in[\s\S]*true\|false/); }); it('polls for a published GitHub Release and fails closed on timeout', () => { From bbce545be4768c3b957a5d459b1a575e81528386 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Thu, 27 Aug 2026 07:36:05 +0000 Subject: [PATCH 6/9] test: match multiline release recovery contract (#211) --- test/release-contract.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/release-contract.test.js b/test/release-contract.test.js index 4416281..8ee288d 100644 --- a/test/release-contract.test.js +++ b/test/release-contract.test.js @@ -69,7 +69,7 @@ describe('/do:release remote promotion contracts', () => { assert.match(body, /git tag "v\{version\}" "\$MERGE_COMMIT"/); assert.match(body, /publishes_github_release/); assert.match(body, /\[ "\$ATTEMPT" -lt 30 \] && sleep 10/); - assert.match(body, /TARGET_PREPARED_RELEASE=.*git log --format=.*origin\/\{target\}/); + assert.match(body, /TARGET_PREPARED_RELEASE="\$\(git log[\s\S]*?origin\/\{target\}/); assert.match(body, /RELEASE_PR_HANDOFF/); assert.match(body, /case "\{publishes_github_release\}" in[\s\S]*true\|false/); }); From e3b5fc7b82184969c352e1b0168cbdacc8fde08e Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Thu, 27 Aug 2026 07:45:54 +0000 Subject: [PATCH 7/9] fix: harden target release recovery (#211) --- commands/do/release.md | 68 +++++++++++++++++++++++++++++++---- test/release-contract.test.js | 6 ++++ 2 files changed, 67 insertions(+), 7 deletions(-) diff --git a/commands/do/release.md b/commands/do/release.md index 8b0cc53..9f4ce0e 100644 --- a/commands/do/release.md +++ b/commands/do/release.md @@ -105,6 +105,14 @@ on the current source history. An interrupted run must resume the prepared versi not bump it again: ```bash +case "{publishes_github_release}" in + true|false) ;; + *) + echo "INCOMPLETE — GitHub Release publication flag is unresolved; preserve the prepared release state." + exit 1 + ;; +esac +RECOVERED_TARGET_RELEASE=false if ! git fetch origin "refs/heads/{target}:refs/remotes/origin/{target}" >/dev/null 2>&1 \ || ! git show-ref --verify --quiet "refs/remotes/origin/{target}"; then echo "INCOMPLETE — Prepared release state is unverified; origin/{target} could not be resolved. Preserve the prepared state and retry." @@ -118,16 +126,50 @@ if [ -z "$PREPARED_RELEASE" ] && [ -n "$TARGET_PREPARED_RELEASE" ]; then if ! printf '%s\n' "$TARGET_TAG" | grep -Eq '^[0-9a-f]{40}$'; then TARGET_TAG="$(git ls-remote origin "refs/tags/v${TARGET_VERSION}" | awk 'NF { print $1; exit }')" fi - TARGET_RELEASE_JSON="$(gh release view "v${TARGET_VERSION}" --json isDraft,isPrerelease,publishedAt 2>/dev/null || true)" + TARGET_RELEASE_STATUS="$(gh api --include "repos/{owner}/{repo}/releases/tags/v${TARGET_VERSION}" 2>/dev/null | awk '$1 ~ /^HTTP\// { print $2; exit }' || true)" + case "$TARGET_RELEASE_STATUS" in + 200) + TARGET_RELEASE_JSON="$(gh release view "v${TARGET_VERSION}" --json isDraft,isPrerelease,publishedAt 2>/dev/null)" || { + echo "INCOMPLETE — Prepared release state is unverified; GitHub Release metadata could not be read. Preserve the prepared state and retry." + exit 1 + } + ;; + 404) TARGET_RELEASE_JSON="" ;; + *) + echo "INCOMPLETE — Prepared release state is unverified; GitHub Release lookup returned ${TARGET_RELEASE_STATUS:-empty}. Preserve the prepared state and retry." + exit 1 + ;; + esac if [ -z "$TARGET_TAG" ] || { [ "{publishes_github_release}" = "true" ] && ! printf '%s\n' "$TARGET_RELEASE_JSON" | jq -e 'type == "object" and .isDraft == false and .isPrerelease == false and (.publishedAt | type == "string") and (.publishedAt | length > 0)' >/dev/null 2>&1; }; then PREPARED_RELEASE="$TARGET_PREPARED_RELEASE" + RECOVERED_TARGET_RELEASE=true fi fi if [ -n "$PREPARED_RELEASE" ]; then PREPARED_RELEASE_SHA="$(printf '%s\n' "$PREPARED_RELEASE" | cut -f1)" VERSION="$(printf '%s\n' "$PREPARED_RELEASE" | sed -E 's/.*release v//')" echo "Resuming prepared release v${VERSION} at ${PREPARED_RELEASE_SHA}; skipping version bump and changelog generation." - printf 'RELEASE_PREPARED_HANDOFF\tPREPARED_RELEASE_SHA=%s\tVERSION=%s\n' "$PREPARED_RELEASE_SHA" "$VERSION" + if [ "$RECOVERED_TARGET_RELEASE" = "true" ]; then + TARGET_RELEASE_PRS_JSON="$(gh pr list --state merged --base "{target}" --limit 100 --json number,state,headRefOid,baseRefName,headRefName,url,mergedAt,mergeCommit)" || { + echo "INCOMPLETE — Merged release PR is unverified; the forge query failed. Preserve the prepared state and retry." + exit 1 + } + if ! printf '%s\n' "$TARGET_RELEASE_PRS_JSON" | jq -e 'type == "array"' >/dev/null; then + echo "INCOMPLETE — Merged release PR is unverified; the forge returned empty or malformed data. Preserve the prepared state and retry." + exit 1 + fi + MATCHING_TARGET_RELEASE_PRS="$(printf '%s\n' "$TARGET_RELEASE_PRS_JSON" | jq -c --arg sha "$PREPARED_RELEASE_SHA" --arg source "{source}" '[.[] | select(.headRefOid == $sha and .baseRefName == "{target}" and .headRefName == $source)]')" + MATCHING_TARGET_RELEASE_COUNT="$(printf '%s\n' "$MATCHING_TARGET_RELEASE_PRS" | jq 'length')" + if [ "$MATCHING_TARGET_RELEASE_COUNT" -ne 1 ]; then + echo "INCOMPLETE — Merged release PR is unverified; expected exactly one merged PR for prepared SHA $PREPARED_RELEASE_SHA, found $MATCHING_TARGET_RELEASE_COUNT. Preserve the prepared state and retry." + exit 1 + fi + PR_NUMBER="$(printf '%s\n' "$MATCHING_TARGET_RELEASE_PRS" | jq -r '.[0].number')" + PR_URL="$(printf '%s\n' "$MATCHING_TARGET_RELEASE_PRS" | jq -r '.[0].url')" + PR_STATE="MERGED" + printf 'RELEASE_TARGET_HANDOFF\tPREPARED_RELEASE_SHA=%s\tPR_NUMBER=%s\tPR_URL=%s\tPR_STATE=%s\n' "$PREPARED_RELEASE_SHA" "$PR_NUMBER" "$PR_URL" "$PR_STATE" + fi + printf 'RELEASE_PREPARED_HANDOFF\tPREPARED_RELEASE_SHA=%s\tVERSION=%s\tTARGET_RECOVERY=%s\n' "$PREPARED_RELEASE_SHA" "$VERSION" "$RECOVERED_TARGET_RELEASE" else echo "No prepared release commit found; determine a new version and finalize its changelog below." fi @@ -137,7 +179,11 @@ When `PREPARED_RELEASE` is non-empty, verify that the checked-out package versio is `{version}` and continue directly to **Local Code Review**. Do not determine a new bump, rewrite release notes, or create another `chore: release` commit. If the package version does not match the prepared commit's version, fail closed and -preserve the prepared state for investigation. +preserve the prepared state for investigation. When `TARGET_RECOVERY=true`, the +prepared release is already merged into `{target}`: carry the +`RELEASE_TARGET_HANDOFF` values and skip Local Code Review, Checkpoints 1–2, and +the open-PR review/CI/merge gates; continue directly to Checkpoint 3 and then +verify the target tree, tag, and GitHub Release. ## Determine Version and Finalize Changelog @@ -211,6 +257,10 @@ Verification — self-check before proceeding (no user prompt needed): ## Open the Release PR +When `TARGET_RECOVERY=true`, use the carried `RELEASE_TARGET_HANDOFF` instead of +running Checkpoints 1–2; the already-merged PR is the release PR for this retry. +Continue with Checkpoint 3 and the post-merge verification blocks below. + - **Checkpoint 1 — source push.** Push the prepared source commit and verify the forge reports the exact same commit before creating or reusing a PR. A successful `git push` by itself is not proof that the remote ref was updated; empty, @@ -219,7 +269,7 @@ Verification — self-check before proceeding (no user prompt needed): ```bash git push -u origin "HEAD:refs/heads/{source}" SOURCE_SHA="$(git rev-parse HEAD)" - PREPARED_RELEASE_SHA="$(git log --extended-regexp --format='%H' --grep='^chore: release v[0-9]+\.[0-9]+\.[0-9]+$' -n 1)" + PREPARED_RELEASE_SHA="$(git log --extended-regexp --format='%H%x09%s' | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print $1; exit }')" if ! printf '%s\n' "$PREPARED_RELEASE_SHA" | grep -Eq '^[0-9a-f]{40}$'; then echo "INCOMPLETE — Prepared release state is unverified; the release preparation commit could not be identified. Preserve the prepared state and retry." exit 1 @@ -239,7 +289,7 @@ Verification — self-check before proceeding (no user prompt needed): pushed source SHA: ```bash SOURCE_SHA="$(git rev-parse HEAD)" - PREPARED_RELEASE_SHA="$(git log --extended-regexp --format='%H' --grep='^chore: release v[0-9]+\.[0-9]+\.[0-9]+$' -n 1)" + PREPARED_RELEASE_SHA="$(git log --extended-regexp --format='%H%x09%s' | awk -F '\t' '$2 ~ /^chore: release v[0-9]+\.[0-9]+\.[0-9]+$/ { print $1; exit }')" REMOTE_SOURCE_SHA="$(git ls-remote --heads origin "refs/heads/{source}" | awk 'NF { print $1; exit }')" if ! printf '%s\n' "$REMOTE_SOURCE_SHA" | grep -Eq '^[0-9a-f]{40}$' || [ "$REMOTE_SOURCE_SHA" != "$SOURCE_SHA" ]; then echo "INCOMPLETE — Source push is unverified; expected $SOURCE_SHA, got ${REMOTE_SOURCE_SHA:-empty}. Preserve the prepared release state and retry." @@ -428,13 +478,17 @@ already succeeded remotely. ```bash PREPARED_RELEASE_SHA="" PR_NUMBER="" - SOURCE_SHA="$(git rev-parse HEAD)" + if [ "" = "true" ]; then + SOURCE_SHA="$PREPARED_RELEASE_SHA" + else + SOURCE_SHA="$(git rev-parse HEAD)" + fi MERGE_JSON="$(gh pr view "$PR_NUMBER" --json state,mergedAt,mergeCommit)" || { echo "INCOMPLETE — Merged release PR is unverified; the forge query failed. Preserve the prepared release state and retry." exit 1 } if ! printf '%s\n' "$MERGE_JSON" | jq -e \ - 'type == "object" and .state == "MERGED" and (.mergedAt | type == "string") and (.mergeCommit.oid | type == "string") and (.mergeCommit.oid | length > 0)' >/dev/null; then + 'type == "object" and .state == "MERGED" and (.mergedAt | type == "string") and (.mergedAt | length > 0) and (.mergeCommit.oid | type == "string") and (.mergeCommit.oid | length > 0)' >/dev/null; then echo "INCOMPLETE — Merged release PR is unverified; the remote merge state is incomplete. Preserve the prepared release state and retry." exit 1 fi diff --git a/test/release-contract.test.js b/test/release-contract.test.js index 8ee288d..f6dc819 100644 --- a/test/release-contract.test.js +++ b/test/release-contract.test.js @@ -46,6 +46,9 @@ describe('/do:release remote promotion contracts', () => { assert.match(body, /PR_STATE="\$\(printf '[^\n]+' \"\$MATCHING_RELEASE_PRS\" \| jq -r '\.\[0\]\.state'/); assert.match(body, /If the selected PR already has `PR_STATE=MERGED`, skip this section entirely[\s\S]*?Do not request another review/); assert.match(body, /If `PR_STATE=MERGED`, skip the CI gate and merge command below/); + assert.match(body, /TARGET_RECOVERY=true/); + assert.match(body, /RELEASE_TARGET_HANDOFF/); + assert.match(body, /skip Local Code Review, Checkpoints 1–2/); }); it('requires mergedAt and mergeCommit instead of trusting merge exit status', () => { @@ -72,6 +75,9 @@ describe('/do:release remote promotion contracts', () => { assert.match(body, /TARGET_PREPARED_RELEASE="\$\(git log[\s\S]*?origin\/\{target\}/); assert.match(body, /RELEASE_PR_HANDOFF/); assert.match(body, /case "\{publishes_github_release\}" in[\s\S]*true\|false/); + assert.match(body, /TARGET_RELEASE_STATUS=.*gh api --include/); + assert.match(body, /404\) TARGET_RELEASE_JSON=""/); + assert.match(body, /SOURCE_SHA="\$PREPARED_RELEASE_SHA"/); }); it('polls for a published GitHub Release and fails closed on timeout', () => { From 9d2bc104eeaf5e774b1d36a815cb779b176ceb61 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Wed, 26 Aug 2026 23:32:09 -0700 Subject: [PATCH 8/9] fix reasoning-effort carriers for codex and agy, which both reject --effort The local-agent review loop built one blanket EFFORT_FLAG=(--effort ) for every reviewer CLI. That flag is only valid for claude and grok: codex (0.149.1) has no --effort at any level -- not top-level, not on `review`, not on `exec` -- and exits 2 with "unexpected argument '--effort' found" before the review starts. Its reasoning effort is a config value, set via the top-level override -c model_reasoning_effort=. agy (1.1.22) treats --effort as mutually exclusive with --model, and this loop always pins --model for agy. Every combination is rejected, at every level including ones agy itself offers. agy encodes effort as a model variant, so it is resolved by picking from the `agy models` roster at run time -- the roster and level names change between releases, so nothing is hardcoded. Either way the reviewer never ran, and its slot in the merge gate was filled by a launch failure rather than a verdict. The deeper fix is the dispatch shape: effort was assigned generically and then clobbered per agent, so the fallthrough for any CLI nobody wrote a branch for was --effort. That fallthrough is what broke both. It is now a case over REVIEW_AGENT that fails closed -- an unrecognized agent gets no flag and keeps prompt-advisory effort, because a wrong flag is not a weaker review, it is a non-zero exit. A new effort-carrier table replaces the same rule restated across six places. --- README.md | 2 +- commands/do/rpr.md | 4 +- lib/local-agent-review-loop.md | 84 +++++++++++++++++++++---------- test/review-loop-contract.test.js | 72 +++++++++++++++++++++++--- 4 files changed, 126 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 53c5e21..6315a2d 100644 --- a/README.md +++ b/README.md @@ -270,7 +270,7 @@ Reviewers run **in the order listed**, and whatever you list is exactly what run **Per-reviewer iteration caps** (`~max=` suffix): caps how many **review → fix → re-review cycles** that one reviewer runs. It is the per-entry form of `--review-iterations`, and unlike that flag it reaches every reviewer type — including `codex`/`agy`/`claude`/`grok`/`cursor` and `ollama`, whose caps are otherwise fixed at 3 — so a single run can budget each reviewer separately: `--review-with claude~max=2,ollama~max=1,codex~max=3`. `` is a non-negative integer; `0` means "loop until clean", bounded by a 10-iteration safety guardrail. A reviewer that stops because it spent a cap *you* set reports `capped`, which counts as clean for the merge gate — as opposed to `guardrail`, which is what a *built-in* cap reports when it cuts off a reviewer that was still finding real problems, and which blocks the merge. -**Per-reviewer reasoning effort** (`~effort=` suffix): specifies the reasoning effort level (`low`, `medium`, `high`, `xhigh`, `max`) for that reviewer: `--review-with codex[gpt-5.6-luna]~effort=max~opt`, `--review-with claude~effort=high~max=2`, `--review-with cursor[gpt-5]~effort=max`. For Cursor the suffix is folded into `--model` as `[effort=]` (the CLI has no `--effort` flag); pair it with a `cursor[]` bracket or a saved `--review-models cursor=…` default so there is a model to attach the variant to. +**Per-reviewer reasoning effort** (`~effort=` suffix): specifies the reasoning effort level (`low`, `medium`, `high`, `xhigh`, `max`) for that reviewer: `--review-with codex[gpt-5.6-luna]~effort=max~opt`, `--review-with claude~effort=high~max=2`, `--review-with cursor[gpt-5]~effort=max`. Each reviewer receives it in the form its own CLI accepts — `--effort` is **not** universal. `claude` and `grok` take the flag; **codex** takes `-c model_reasoning_effort=`; **Cursor** folds it into `--model` as `[effort=]`, so pair that one with a `cursor[]` bracket or a saved `--review-models cursor=…` default; and **agy** picks the matching model variant from whatever `agy models` lists. Where a reviewer offers no such control — or no level matching what you asked — the effort falls back to prompt guidance rather than failing the review. `~max` applies in `series` mode (the default). In `--review-mode parallel` each reviewer runs a single review-only pass and the orchestrator applies the union once, so there are no per-reviewer cycles to cap — `~max` is ignored there with a warning. diff --git a/commands/do/rpr.md b/commands/do/rpr.md index 7bd33c2..028182b 100644 --- a/commands/do/rpr.md +++ b/commands/do/rpr.md @@ -41,7 +41,7 @@ Parse `$ARGUMENTS` for `--issues` / `--no-issues` / `--issues-label `: whe Note whether **any** completed review exists (from a copilot bot, a human, or another bot) — call this `HAS_EXISTING_REVIEW` — and specifically whether a **completed** `copilot-pull-request-reviewer` review exists (a node in `reviews.nodes`, NOT merely a pending review request) — call this `HAS_COPILOT_REVIEW`. Track a Copilot review that is only **pending** (Copilot present in `reviewRequests.nodes[].requestedReviewer` with no completed Copilot review yet) separately as `COPILOT_REVIEW_PENDING` — a pending-only review must NOT set `HAS_COPILOT_REVIEW`, or the "completed review exists" branch below would fire and resolve threads before Copilot has posted anything. Then dispatch on `REVIEW_AGENTS`: - **If `REVIEW_AGENTS` is empty** (no `--review-with`, no saved default, or an explicit `none`): request **no** review — not Copilot, not anything else. Proceed straight to step 3 and resolve whatever unresolved threads the PR already carries. - - **If `REVIEW_AGENTS` contains a local CLI (`codex`/`agy`/`claude`/`grok`/`cursor`):** run the **local-agent review loop** (`lib/local-agent-review-loop.md`, referenced below) for each such agent against the PR branch, forwarding `REVIEWER_APPLIES`, that entry's resolved `{REVIEW_MODEL}` (bracket, else saved `review-models[slug]` default, else empty → built-in default — see the loop reference below), that entry's `{REVIEW_EFFORT}` (from its `~effort=`; empty when it carried none — Cursor folds this into `--model`, the other CLIs take it as `--effort`), and that entry's `{MAX_ITERATIONS}` / `{MAX_EXPLICIT}` (from its `~max=`; the built-in `3` / `false` when it carried none). This produces findings (and, in reviewer-applies mode, fixes) locally — it does **not** request a Copilot cloud review for those slugs. Then proceed to step 3 to fetch and resolve any pre-existing unresolved threads as well. (If `REVIEW_AGENTS` also contains `copilot`, additionally run the Copilot path below.) + - **If `REVIEW_AGENTS` contains a local CLI (`codex`/`agy`/`claude`/`grok`/`cursor`):** run the **local-agent review loop** (`lib/local-agent-review-loop.md`, referenced below) for each such agent against the PR branch, forwarding `REVIEWER_APPLIES`, that entry's resolved `{REVIEW_MODEL}` (bracket, else saved `review-models[slug]` default, else empty → built-in default — see the loop reference below), that entry's `{REVIEW_EFFORT}` (from its `~effort=`; empty when it carried none — the loop maps it to each CLI's accepted carrier), and that entry's `{MAX_ITERATIONS}` / `{MAX_EXPLICIT}` (from its `~max=`; the built-in `3` / `false` when it carried none). This produces findings (and, in reviewer-applies mode, fixes) locally — it does **not** request a Copilot cloud review for those slugs. Then proceed to step 3 to fetch and resolve any pre-existing unresolved threads as well. (If `REVIEW_AGENTS` also contains `copilot`, additionally run the Copilot path below.) - **If `REVIEW_AGENTS` contains `ollama`:** run the **Ollama review loop** (`lib/ollama-review-loop.md`, referenced below) for each `ollama` entry against the PR branch (which must be checked out locally — the loop reviews a local `git diff`), forwarding that entry's `{OLLAMA_MODEL}` (empty = auto-select), its `{OLLAMA_EFFORT}` (from its `~effort=`; empty when it carried none), and its `{MAX_ITERATIONS}` / `{MAX_EXPLICIT}` (from its `~max=`; the built-in `3` / `false` when it carried none). Like the local-CLI path it produces findings the orchestrator applies locally (Ollama is always review-only) and does **not** request a Copilot review. Then proceed to step 3 to resolve any pre-existing threads. (If `REVIEW_AGENTS` also contains `copilot`, additionally run the Copilot path below.) - **If `REVIEW_AGENTS` contains `copilot`** (only ever because you asked for it — typed or saved): a `copilot~max=` cap applies to this path too — see the cap-accounting rule in step 8. rpr accepts no `--review-iterations`, so `~max` is the only budget signal here, and each Copilot round below counts against it. - **A completed Copilot review exists** (`HAS_COPILOT_REVIEW`): skip requesting a new one — proceed to step 3 to address its threads. @@ -128,7 +128,7 @@ Parse `$ARGUMENTS` for `--issues` / `--no-issues` / `--issues-label `: whe ## Local-Agent Review Loop (for `--review-with codex|agy|claude|grok|cursor`) -When `REVIEW_AGENTS` names a local CLI, step 2 (and the step-8 re-request) runs that agent's review against the PR branch via the shared local-agent loop. Pass `{REVIEW_AGENT}`, `{REVIEWER_APPLIES}`, that entry's resolved `{REVIEW_MODEL}` (the `[]` bracket if the token carried one, else the saved `review-models[slug]` default resolved above — project over global, else empty → the reviewer's built-in default), that entry's `{REVIEW_EFFORT}` (from its `~effort=`; empty when unset — without it Cursor cannot fold effort into `--model` and the other CLIs cannot pass `--effort`), the PR branch (`headRefName`), the base branch (`baseRefName`), and the project `{BUILD_CMD}`. Forwarding `{REVIEW_MODEL}` is what makes `--review-with=codex[o3]` and a saved `review-models` default actually pin the model on rpr's local passes — without it those passes would silently run the CLI's default model. The loop verifies build + tests in the main thread before pushing; afterward, continue to step 3 to resolve any pre-existing threads. +When `REVIEW_AGENTS` names a local CLI, step 2 (and the step-8 re-request) runs that agent's review against the PR branch via the shared local-agent loop. Pass `{REVIEW_AGENT}`, `{REVIEWER_APPLIES}`, that entry's resolved `{REVIEW_MODEL}` (the `[]` bracket if the token carried one, else the saved `review-models[slug]` default resolved above — project over global, else empty → the reviewer's built-in default), that entry's `{REVIEW_EFFORT}` (from its `~effort=`; empty when unset — the loop's effort-carrier table maps it to each CLI's accepted form), the PR branch (`headRefName`), the base branch (`baseRefName`), and the project `{BUILD_CMD}`. Forwarding `{REVIEW_MODEL}` is what makes `--review-with=codex[o3]` and a saved `review-models` default actually pin the model on rpr's local passes — without it those passes would silently run the CLI's default model. The loop verifies build + tests in the main thread before pushing; afterward, continue to step 3 to resolve any pre-existing threads. !`cat ~/.claude/lib/local-agent-review-loop.md` diff --git a/lib/local-agent-review-loop.md b/lib/local-agent-review-loop.md index bd83ae1..5a95ea5 100644 --- a/lib/local-agent-review-loop.md +++ b/lib/local-agent-review-loop.md @@ -47,7 +47,7 @@ When to use this: 5. Record `{REVIEWER_APPLIES}` — boolean, defaults to `false`. Set to `true` when the orchestrating command was invoked with `--reviewer-applies`. This flag selects which side of the loop holds the editor: when `false` (default), the orchestrator applies fixes from the CLI's findings log; when `true`, the headless CLI applies fixes directly in the working tree and the orchestrator only verifies. 6. Record `{REVIEW_MODEL}` — the model to run this reviewer on, resolved by the caller (the multi-reviewer loop: explicit `[]` bracket → saved `review-models[slug]` default → empty). **May be empty**, which means "use the reviewer's built-in default" — for `codex`/`claude`/`grok`/`cursor` that is the CLI's own default model (no `--model` flag passed); for `agy` it is the pinned `AGY_REVIEW_MODEL` default resolved below. When set, it is passed through to the reviewer's invocation (`codex --model`, `claude --model` / the in-process `Agent` tool's `model`, `agy --model`, `grok --model`, or `cursor --model`) so a run/config can pin which model reviews. The value is free-form (model names churn and may contain spaces/parens, e.g. `Gemini 3.5 Flash (High)`) — do not validate it against an allowlist; pass it verbatim. 7. Record `{MAX_ITERATIONS}` — how many review → fix → re-review cycles this reviewer may run, resolved by the caller (the multi-reviewer loop: a per-entry `~max=` suffix on the `--review-with` token → this loop's built-in default of `3`). **Defaults to `3`** when the caller passes nothing, which is the historical behavior. `0` means **unlimited** — loop until the reviewer is clean or the convergence gate converges, bounded by the 10-iteration safety guardrail in Step 6. Also record `{MAX_EXPLICIT}` — boolean, `true` only when the cap came from a `~max=` the user typed (or saved), `false` when it is this loop's built-in `3`. Step 6 uses it to decide whether exhausting the cap is `capped` (a budget the user chose — clean-equivalent for the merge gate) or `guardrail` (a built-in ceiling nobody vouched for — inconclusive). Note the `--review-iterations` flag never reaches this loop; `~max` is the only way to move this cap. -8. Record `{REVIEW_EFFORT}` — optional reasoning effort string for this reviewer (`low`, `medium`, `high`, `xhigh`, `max`), resolved by the caller (the multi-reviewer loop: explicit `~effort=` suffix on the `--review-with` token → empty). **Defaults to empty** when unset. When set, it is appended as advisory reasoning effort to the prompt preamble and passed as `--effort` where supported — which is a CLI flag on the subprocess paths, folded into `--model` for `cursor`, and prompt-only for the in-process Claude sub-agent, which has no effort parameter to pass (Step 2). +8. Record `{REVIEW_EFFORT}` — optional reasoning effort string for this reviewer (`low`, `medium`, `high`, `xhigh`, `max`), resolved by the caller (the multi-reviewer loop: explicit `~effort=` suffix on the `--review-with` token → empty). **Defaults to empty** when unset. When set, it is appended as advisory reasoning effort to the prompt preamble and *also* passed to the CLI in whatever form that CLI accepts. The carriers differ per agent — see the effort-carrier table below, which the pre-flight `case` implements. Never assume `--effort` is universal. ### Editing mode @@ -131,30 +131,36 @@ elif command -v gtimeout >/dev/null 2>&1; then TIMEOUT_CMD=(gtimeout 1800); fi # (built-in default), so its flag is never empty. MODEL_FLAG=() [ -n "$REVIEW_MODEL" ] && MODEL_FLAG=(--model "$REVIEW_MODEL") +# Reasoning effort carrier. Each reviewer CLI takes effort in a DIFFERENT form, +# so build it per agent -- and default to NO flag, not to `--effort`. That +# default matters: `--effort` is correct for only two of these CLIs, and the +# unknown-agent arm must degrade to prompt-advisory effort (the "Target +# reasoning effort level" sentence $LOCAL_PROMPT already carries) rather than +# guess a flag. A wrong guess is not a weaker review -- it is a non-zero exit +# before the review runs, which fills that reviewer's merge-gate slot with a +# launch failure. See the effort-carrier table below for the per-agent forms. EFFORT_FLAG=() -[ -n "$REVIEW_EFFORT" ] && EFFORT_FLAG=(--effort "$REVIEW_EFFORT") - -# Cursor has no --effort flag (passing it exits non-zero). Its native effort -# control is a model-variant parameter — `gpt-5[effort=max]`, or -# `claude-opus-4-7[thinking=true,effort=high]`. Fold {REVIEW_EFFORT} into -# --model so `cursor[gpt-5]~effort=max` and a saved -# `--review-models cursor=gpt-5` plus `cursor~effort=max` actually change -# inference, matching the other reviewers' ~effort behavior. A model string -# that already carries `effort=` (typed in the bracket or saved in -# review-models) is left alone. Effort with no model stays prompt-advisory -# only — there is nothing to attach the variant to. -if [ "$REVIEW_AGENT" = cursor ]; then - EFFORT_FLAG=() - if [ -n "$REVIEW_EFFORT" ] && [ -n "$REVIEW_MODEL" ]; then - case "$REVIEW_MODEL" in - *effort=*) CURSOR_MODEL="$REVIEW_MODEL" ;; - *\[*\]) CURSOR_MODEL="${REVIEW_MODEL%]},effort=${REVIEW_EFFORT}]" ;; - *) CURSOR_MODEL="${REVIEW_MODEL}[effort=${REVIEW_EFFORT}]" ;; - esac - MODEL_FLAG=(--model "$CURSOR_MODEL") - fi +if [ -n "$REVIEW_EFFORT" ]; then + case "$REVIEW_AGENT" in + claude|grok) EFFORT_FLAG=(--effort "$REVIEW_EFFORT") ;; + codex) EFFORT_FLAG=(-c "model_reasoning_effort=$REVIEW_EFFORT") ;; + cursor) + # Effort is a model-variant parameter; fold it into --model. A model + # string that already carries `effort=` is left alone, and effort with no + # model stays prompt-advisory (nothing to attach the variant to). + if [ -n "$REVIEW_MODEL" ]; then + case "$REVIEW_MODEL" in + *effort=*) CURSOR_MODEL="$REVIEW_MODEL" ;; + *\[*\]) CURSOR_MODEL="${REVIEW_MODEL%]},effort=${REVIEW_EFFORT}]" ;; + *) CURSOR_MODEL="${REVIEW_MODEL}[effort=${REVIEW_EFFORT}]" ;; + esac + MODEL_FLAG=(--model "$CURSOR_MODEL") + fi + ;; + agy) : ;; # effort is a model variant, resolved from `agy models` below + *) : ;; # unknown agent: prompt-advisory only -- never guess a flag + esac fi - # agy only: pin the review model. A per-run/config model wins via {REVIEW_MODEL} # (the `agy[]` bracket or a saved `review-models` default), then the # AGY_REVIEW_MODEL env var, then the built-in default below. agy's DEFAULT can be a heavy "Thinking" model @@ -174,10 +180,35 @@ fi # (e.g. `agy models`) — a nested agy invocation inside a print session can stall. # Precedence: bracket/config-resolved {REVIEW_MODEL} > AGY_REVIEW_MODEL env > built-in default. AGY_REVIEW_MODEL="${REVIEW_MODEL:-${AGY_REVIEW_MODEL:-Gemini 3.5 Flash (High)}}" +# agy effort: resolved as a model variant (see the effort-carrier table), so +# print agy's own roster for the selection step below. Do not hardcode a level +# vocabulary or a name shape -- both change between agy releases. This runs in +# the ORCHESTRATOR's shell; the NOTE above bans `agy models` from the reviewer +# PROMPT (a nested agy call inside a print session stalls), not from here. +if [ "$REVIEW_AGENT" = agy ] && [ -n "$REVIEW_EFFORT" ] && [ -z "$AGY_MODEL_RESOLVED" ]; then + agy models 2>/dev/null +fi ``` Run the pre-flight block above verbatim. The `TIMEOUT_CMD` resolution is deterministic — do NOT think out loud about whether `timeout`/`gtimeout` is installed or about falling back; just execute it and move on. +**Effort carriers.** `{REVIEW_EFFORT}` reaches each reviewer in the one form its CLI accepts. The pre-flight `case` above builds it; this table is the rule, and the per-agent bullets under "Flag rationale" below record the verified failures behind it. **Never assume `--effort` is universal** — three of these reject it outright, and a rejected flag is a non-zero exit before the review runs, not a weaker review. + +| Agent | Effort carrier | +|-------|----------------| +| `claude` (subprocess) / `grok` | `--effort ` | +| `claude` (in-process sub-agent) | prompt-advisory only — the `Agent` tool has no effort parameter | +| `codex` | `-c model_reasoning_effort=` (top-level config override; **no** `--effort` flag exists) | +| `cursor` | folded into `--model` as `[effort=]` | +| `agy` | a model **variant** picked from `agy models` (see below) | +| anything else | prompt-advisory only — never guess a flag | + +**Selecting agy's effort variant** (only when `{REVIEW_AGENT}` is `agy` and `{REVIEW_EFFORT}` is set). agy encodes effort as a model variant and rejects `--effort` whenever `--model` is pinned, which this loop always does. The pre-flight printed `agy models` — one entry per line, id and display name. Choose from **that listing**, not from a remembered table (the roster and level names change between releases): + +- Take the entry that is the same base model as the resolved `AGY_REVIEW_MODEL` at the requested level. If agy offers no exact match, take the **closest level it does offer** and say which you took — `~effort=max` against a base topping out at "High" means High, since the intent is "as much reasoning as this reviewer has," not "abort because the ceiling is lower than asked." +- If the base has no variants, or `agy models` printed nothing (offline, not signed in), keep `AGY_REVIEW_MODEL` as-is — effort stays prompt-advisory. Never invent a variant that wasn't listed: a base that merely *looks* like it has variants becomes a model agy rejects, trading a degraded review for a launch failure. +- **Record the choice.** Set `AGY_REVIEW_MODEL` to the chosen entry and reuse that literal string in every Step 2 invocation for the rest of this loop, and set `AGY_MODEL_RESOLVED=1`. Pre-flight is re-materialized on each review → fix → re-review iteration (shell variables do not survive between Bash calls), so without this the roster is re-fetched and the choice re-derived every cycle. + Pick the invocation based on `{REVIEW_AGENT}` and `{REVIEWER_APPLIES}`: | Agent | Review-only (`REVIEWER_APPLIES=false`, default) | Reviewer-applies (`REVIEWER_APPLIES=true`) | @@ -188,7 +219,7 @@ Pick the invocation based on `{REVIEW_AGENT}` and `{REVIEWER_APPLIES}`: | `claude` | `claude -p "$LOCAL_PROMPT" ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} --dangerously-skip-permissions` | `claude -p "$LOCAL_PROMPT" ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} --dangerously-skip-permissions` | | `codex` | `codex ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} --sandbox read-only review --base "$BASE_BRANCH" --title "$REVIEW_TITLE"` | `codex ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} --sandbox danger-full-access -a never exec "$CODEX_APPLY_PROMPT"` | -| `agy` | `agy --dangerously-skip-permissions --model "$AGY_REVIEW_MODEL" ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} --print-timeout 30m -p "$LOCAL_PROMPT"` | `agy --dangerously-skip-permissions --model "$AGY_REVIEW_MODEL" ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} --print-timeout 30m -p "$LOCAL_PROMPT"` | +| `agy` | `agy --dangerously-skip-permissions --model "$AGY_REVIEW_MODEL" --print-timeout 30m -p "$LOCAL_PROMPT"` | `agy --dangerously-skip-permissions --model "$AGY_REVIEW_MODEL" --print-timeout 30m -p "$LOCAL_PROMPT"` | | `grok` | `grok --permission-mode bypassPermissions ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} -p "$LOCAL_PROMPT"` | `grok --permission-mode bypassPermissions ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} -p "$LOCAL_PROMPT"` | | `cursor` | `"$REVIEW_BIN" -p --trust --mode=ask --output-format text ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} "$LOCAL_PROMPT"` | `"$REVIEW_BIN" -p --force --trust --output-format text --sandbox disabled ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} "$LOCAL_PROMPT"` | @@ -196,7 +227,7 @@ For `claude`, `agy`, `grok`, and `cursor`, the same `$LOCAL_PROMPT` drives both > **Pass the prompt as a positional argument — never via stdin.** `claude -p`, `agy -p` (`--print`), `grok -p` (`--single`), and `cursor-agent -p` (`--print`) all take the prompt as the argument directly after the flag: `agy --dangerously-skip-permissions -p "$LOCAL_PROMPT"`, `grok --permission-mode bypassPermissions -p "$LOCAL_PROMPT"`, `"$REVIEW_BIN" -p --trust --mode=ask "$LOCAL_PROMPT"`. They do **not** read the prompt from stdin. Do NOT write `echo "$LOCAL_PROMPT" | agy --dangerously-skip-permissions -p`, `agy -p < prompt.txt`, or `printf … | agy -p` — agy ignores piped stdin and exits with `agy --print takes the prompt as an argument, not stdin`, forcing a wasted second invocation. The `> "$LOG_FILE" 2> "$ERR_FILE"` redirect in Step 2 captures the reviewer's *output*; it is unrelated to how the prompt goes in. Keep `"$LOCAL_PROMPT"` as the quoted argument to `-p` exactly as shown in the invocation table. -**Pinning the reviewer's model (`${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"}` / `--model`) and reasoning effort (`${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"}` / `--effort`).** When `{REVIEW_MODEL}` / `{REVIEW_EFFORT}` is set (from an `[]` bracket, a `~effort=` suffix, or a saved default — resolved by the caller), the reviewer runs on that model and effort level; when empty, `MODEL_FLAG` / `EFFORT_FLAG` are empty arrays so `codex`/`claude`/`agy`/`grok`/`cursor` fall back to their default or session values. **Do not pass `EFFORT_FLAG` to `cursor`** — the Cursor CLI has no `--effort` flag and would exit non-zero. Instead the pre-flight block above **folds `{REVIEW_EFFORT}` into the `--model` value** as Cursor's native variant parameter (`gpt-5` + `~effort=max` → `--model gpt-5[effort=max]`; a bracket that already has params gets `,effort=` appended; a model string that already contains `effort=` is left alone). That is what makes `cursor[gpt-5]~effort=max` and `/do:config --review-models cursor=gpt-5` plus `cursor~effort=max` actually change inference, the same way `--effort` does for the other CLIs. The advisory "Target reasoning effort level" sentence in `$REVIEW_TASK` still fires (covers the no-model case, where there is nothing to fold into). For **codex**, `-m`/`--model` and `--effort` are **top-level** Codex options (like `--sandbox` and `-a`), so they MUST precede the `review`/`exec` subcommand — that is why `${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"}` and `${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"}` sit before `--sandbox` in both codex invocations; passing them after the subcommand would exit 2 with an unexpected-argument error, exactly as `-a` does. (The two paths pass *different* sandbox policies — `read-only` for review-only, `danger-full-access` for reviewer-applies — see below.) For **claude**, `--model` is a session flag valid alongside `-p`. For **grok**, `-m`/`--model` is a session flag valid alongside `-p`, so `${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"}` sits inline in the invocation (empty array → grok's own default). For **agy**, the model is always pinned via `--model "$AGY_REVIEW_MODEL"` (resolved above with `{REVIEW_MODEL}` taking precedence over the `AGY_REVIEW_MODEL` env and the built-in default) — agy's own default may be a slow "Thinking" tier, so it is never left unpinned. Because the model string may contain spaces/parens, `MODEL_FLAG` is a shell array (see the pre-flight block) — never a bare string. +**Pinning the reviewer's model (`${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"}` / `--model`) and reasoning effort (`${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"}` / `--effort`).** When `{REVIEW_MODEL}` / `{REVIEW_EFFORT}` is set (from an `[]` bracket, a `~effort=` suffix, or a saved default — resolved by the caller), the reviewer runs on that model and effort level; when empty, `MODEL_FLAG` / `EFFORT_FLAG` are empty arrays so `codex`/`claude`/`agy`/`grok`/`cursor` fall back to their default or session values. Effort is **not** a universal `--effort` — the pre-flight `case` builds the per-agent carrier listed in the effort-carrier table above (`cursor` and `agy` fold it into `--model`; `codex` takes `-c model_reasoning_effort=`), and the "Flag rationale" bullets below record why each. The advisory "Target reasoning effort level" sentence in `$REVIEW_TASK` still fires wherever a carrier can't be built. For **codex**, `-m`/`--model` and `-c` are **top-level** Codex options (like `--sandbox` and `-a`), so they MUST precede the `review`/`exec` subcommand — that is why `${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"}` and `${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"}` sit before `--sandbox` in both codex invocations; `-c` happens to be accepted by the subcommands too, but `-m`/`--model` after the subcommand would exit 2 with an unexpected-argument error, exactly as `-a` does. (The two paths pass *different* sandbox policies — `read-only` for review-only, `danger-full-access` for reviewer-applies — see below.) For **claude**, `--model` is a session flag valid alongside `-p`. For **grok**, `-m`/`--model` is a session flag valid alongside `-p`, so `${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"}` sits inline in the invocation (empty array → grok's own default). For **agy**, the model is always pinned via `--model "$AGY_REVIEW_MODEL"` (resolved above with `{REVIEW_MODEL}` taking precedence over the `AGY_REVIEW_MODEL` env and the built-in default) — agy's own default may be a slow "Thinking" tier, so it is never left unpinned — and that pinned model is also where its effort lives. Because the model string may contain spaces/parens, `MODEL_FLAG` is a shell array (see the pre-flight block) — never a bare string. Notes on each invocation: - **claude / agy / grok / cursor** run the self-contained `$LOCAL_PROMPT` (a single-agent inline review), **not** slashdo's `/do-review` skill — the skill's sub-agent fan-out never re-syncs into a print-mode/headless response, so it would hang and emit zero findings (see the `$LOCAL_PROMPT` rationale above). Under Claude Code the `claude` reviewer is an in-process sub-agent (via the `Agent` tool) that runs `$LOCAL_PROMPT` directly, rather than a `claude -p` subprocess — and because the prompt is a single-agent inline review, it does not recursively spawn the skill's own sub-agents. In `REVIEWER_APPLIES=true` mode, `$LOCAL_PROMPT` tells the CLI to apply each fix, verify with build+tests, commit as `address review (): ` (`` = the reviewing CLI's slug, `claude`, `agy`, `grok`, or `cursor`), and NOT push (the orchestrating agent verifies and pushes). The parenthesized agent name records which reviewer surfaced the finding, useful when scanning the log of a release that ran multiple reviewers. In `REVIEWER_APPLIES=false` mode, `$LOCAL_PROMPT` tells the CLI to emit `FINDING :` blocks (or `NO FINDINGS`) to stdout for the orchestrator to parse — the orchestrator then commits the fixes using the same `address review (): ` form to preserve attribution. @@ -211,7 +242,8 @@ Flag rationale (reckless / unattended mode): - **review-only → `read-only`.** Verified: `codex --sandbox read-only review --base ` reads the diff, tracked-file list, commit graph and base tree and returns normal severity-tagged findings, while `printf … > file` inside the repo fails with `zsh:1: operation not permitted`. Review quality is unaffected and the contract becomes unbypassable. This matches `lib/enhance-loop.md`, which already runs codex `--sandbox read-only` for the same reason. - **reviewer-applies → `danger-full-access`.** This path must write fixes, run build/tests, and reach the network unattended, so full access is the intended posture on a trusted single-user machine (mirrors `claude --dangerously-skip-permissions` / `agy --dangerously-skip-permissions`). - `--sandbox` and `-a` are independent top-level flags and may be combined (`codex --sandbox danger-full-access -a never exec …`). -- `agy --dangerously-skip-permissions --model "$AGY_REVIEW_MODEL" --print-timeout 30m` — `--dangerously-skip-permissions` auto-approves all tool permission requests so the Antigravity CLI runs unattended (the headless equivalent of confirming every prompt). `--model "$AGY_REVIEW_MODEL"` pins the reviewing model (resolved in pre-flight, default `Gemini 3.5 Flash (High)`, override via `AGY_REVIEW_MODEL`): without it agy picks its own default, which may be a heavy "Thinking" tier that spends many minutes in hidden reasoning and — depending on the model, emits little or no visible output meanwhile — makes a review look hung for 20-30 minutes; a fast capable model returns in well under a minute on a small diff. This is the agy successor to the Gemini CLI's `gemini --yolo` + `env GEMINI_SANDBOX=false`: agy folds both "auto-approve tools" and "no sandbox gate" into the single flag, and runs the prompt non-interactively via `-p` — which takes the prompt as its positional argument (`agy … -p "$LOCAL_PROMPT"`), **not** from stdin. Piping into `agy -p` (e.g. `echo … | agy -p`) fails with `agy --print takes the prompt as an argument, not stdin` and wastes an invocation; always pass the quoted prompt as the argument. `--print-timeout 30m` raises the print-mode wait above agy's 5-minute default so a real multi-file review isn't cut off, and — since stock macOS has no `timeout`/`gtimeout` and `TIMEOUT_CMD` is empty — is the effective bound on the invocation; it bounds the wait for the next response chunk, not the total runtime, so an actively-streaming review is never truncated. Unlike the old gemini invocation, no `env VAR=…` prefix is needed, so it composes cleanly with the `${TIMEOUT_CMD[@]+"${TIMEOUT_CMD[@]}"} {INVOCATION}` wrapper at step 2 of the loop when one is present. +- `codex -c model_reasoning_effort=` — codex's **only** reasoning-effort control, and the reason the carrier table exists. codex has no `--effort` flag anywhere (not top-level, not on `review`, not on `exec`): passing one exits 2 with `error: unexpected argument '--effort' found` *before* any review runs, so the loop gets a launch failure where it expected a verdict. `-c key=value` is codex's config override, and `model_reasoning_effort` is the key. Pass it once and never pair it with a `--effort` "for good measure" — the pairing is the failure, not a safety net. +- `agy --dangerously-skip-permissions --model "$AGY_REVIEW_MODEL" --print-timeout 30m` — `--dangerously-skip-permissions` auto-approves all tool permission requests so the Antigravity CLI runs unattended (the headless equivalent of confirming every prompt). **Never pass `--effort` to agy**: agy treats it as mutually exclusive with `--model`, and this invocation always sets `--model`, so it is an unconditional launch failure rather than a degraded review — `--effort is not supported for model "…"`, at every level, including levels agy itself offers. Effort rides in the model name instead (carrier table above). `--model "$AGY_REVIEW_MODEL"` pins the reviewing model (resolved in pre-flight, default `Gemini 3.5 Flash (High)`, override via `AGY_REVIEW_MODEL`): without it agy picks its own default, which may be a heavy "Thinking" tier that spends many minutes in hidden reasoning and — depending on the model, emits little or no visible output meanwhile — makes a review look hung for 20-30 minutes; a fast capable model returns in well under a minute on a small diff. This is the agy successor to the Gemini CLI's `gemini --yolo` + `env GEMINI_SANDBOX=false`: agy folds both "auto-approve tools" and "no sandbox gate" into the single flag, and runs the prompt non-interactively via `-p` — which takes the prompt as its positional argument (`agy … -p "$LOCAL_PROMPT"`), **not** from stdin. Piping into `agy -p` (e.g. `echo … | agy -p`) fails with `agy --print takes the prompt as an argument, not stdin` and wastes an invocation; always pass the quoted prompt as the argument. `--print-timeout 30m` raises the print-mode wait above agy's 5-minute default so a real multi-file review isn't cut off, and — since stock macOS has no `timeout`/`gtimeout` and `TIMEOUT_CMD` is empty — is the effective bound on the invocation; it bounds the wait for the next response chunk, not the total runtime, so an actively-streaming review is never truncated. Unlike the old gemini invocation, no `env VAR=…` prefix is needed, so it composes cleanly with the `${TIMEOUT_CMD[@]+"${TIMEOUT_CMD[@]}"} {INVOCATION}` wrapper at step 2 of the loop when one is present. - `grok --permission-mode bypassPermissions ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} -p` — `-p`/`--single` runs a single-turn headless prompt, prints the response to stdout, and exits (the grok analog of `claude -p` / `agy -p`). `--permission-mode bypassPermissions` auto-approves every tool execution so grok runs unattended (grok's equivalent of `--dangerously-skip-permissions`); it folds "auto-approve tools" into one flag, so no separate sandbox/`env VAR=…` prefix is needed and it composes cleanly with the `${TIMEOUT_CMD[@]+"${TIMEOUT_CMD[@]}"} {INVOCATION}` wrapper. `${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"}` pins the reviewing model for a `grok[]` bracket (empty array → grok's own default; grok accepts the long `--model` form alongside `-p`). Like `agy -p`, `grok -p` takes the prompt as its positional argument — **not** from stdin (`grok … -p "$LOCAL_PROMPT"`); do not pipe into it. Grok has no `--print-timeout` equivalent, so the run is bounded by `TIMEOUT_CMD` (when present) and grok's own internal limits — the same background-launch + poll in Step 2 keeps it off the host's ~10-minute foreground cap. - `"$REVIEW_BIN" -p --trust …` (cursor) — `-p`/`--print` is Cursor Agent's headless mode (the analog of `claude -p` / `agy -p` / `grok -p`). `{REVIEW_BIN}` is `cursor-agent` when that name is on `$PATH`, else a probed `agent` that identified as Cursor — never a bare `agent` that is actually Grok (see the Cursor binary probe). `--trust` is required for headless runs in an untrusted workspace (Cursor fails those with guidance unless `--trust` or `--force` is passed). **Review-only** adds `--mode=ask` (Cursor's read-only exploration mode) and **omits `--force`**, so print mode only proposes changes — the same contract as the prompt, plus a mode flag that actually refuses writes. **Reviewer-applies** adds `--force` (alias `--yolo`) and `--sandbox disabled` so the agent can write fixes, run build/tests, and reach the network unattended. `--output-format text` is the default but is passed explicitly so stdout is a clean verdict document for Step 3. `${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"}` pins a `cursor[]` bracket or a saved `review-models` default (empty array → Cursor's own default). A `~effort=` is folded into that same `--model` value as `[effort=]` (see the pre-flight block) — **do not pass `--effort`**, Cursor has no such flag and would exit non-zero. So `cursor[gpt-5]~effort=max` and `/do:config --review-models cursor=gpt-5` plus `--review-with cursor~effort=max` both become `--model gpt-5[effort=max]`. A model string that already encodes effort (`cursor[claude-opus-4-7[thinking=true,effort=high]]`, or a saved `review-models` value that already has `effort=`) is passed through unchanged. Effort with no model is prompt-advisory only. Like the other `-p` CLIs, the prompt is a positional argument (`… -p "$LOCAL_PROMPT"`), not stdin. Cursor has no `--print-timeout` equivalent, so the run is bounded by `TIMEOUT_CMD` (when present) and Cursor's own limits — the same background-launch + poll in Step 2 keeps it off the host's ~10-minute foreground cap. diff --git a/test/review-loop-contract.test.js b/test/review-loop-contract.test.js index 71332c1..bda14e9 100644 --- a/test/review-loop-contract.test.js +++ b/test/review-loop-contract.test.js @@ -5,8 +5,14 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const readLib = (name) => fs.readFileSync(path.join(__dirname, '..', 'lib', name), 'utf8'); -const readCommand = (name) => fs.readFileSync(path.join(__dirname, '..', 'commands', 'do', name), 'utf8'); +const _readCache = new Map(); +const _read = (...parts) => { + const f = path.join(__dirname, "..", ...parts); + if (!_readCache.has(f)) _readCache.set(f, fs.readFileSync(f, "utf8")); + return _readCache.get(f); +}; +const readLib = (name) => _read("lib", name); +const readCommand = (name) => _read("commands", "do", name); // The loop partials whose invocations carry arrays that can legitimately be empty // (TIMEOUT_CMD when no timeout/gtimeout is installed, MODEL_FLAG when no model is @@ -132,6 +138,60 @@ describe('review-loop parse contracts', () => { assert.match(ollama, /OLLAMA_EFFORT/); assert.match(ollama, /PROMPT="\$PROMPT Target reasoning effort level: \$OLLAMA_EFFORT\."/); }); + it('builds each reviewer a carrier its CLI actually accepts, defaulting to none', () => { + // `--effort` is correct for only claude/grok. Passing it to a CLI that + // rejects it is a non-zero exit BEFORE the review runs, so that reviewer's + // merge-gate slot holds a launch failure rather than a verdict: + // codex-cli 0.149.1: no --effort at any level (top-level, `review`, `exec`) + // -> error: unexpected argument '--effort' found + // agy 1.1.22: --effort is mutually exclusive with --model, which this loop + // always pins -> --effort is not supported for model "..." + // The pre-flight therefore dispatches per agent and defaults to NO flag; an + // agent nobody wrote an arm for must degrade to prompt-advisory effort, not + // inherit `--effort`. That inheritance is what broke codex and agy. + const loop = readLib('local-agent-review-loop.md'); + const preflight = loop.slice( + loop.indexOf('# Reasoning effort carrier.'), + loop.indexOf('# agy only: pin the review model'), + ); + assert.ok(preflight, 'the effort-carrier pre-flight block must exist'); + + // Per-agent carrier, asserted as a table so a new reviewer adds a row. + const CARRIERS = [ + ['claude|grok', /claude\|grok\) EFFORT_FLAG=\(--effort "\$REVIEW_EFFORT"\)/], + ['codex', /codex\)\s+EFFORT_FLAG=\(-c "model_reasoning_effort=\$REVIEW_EFFORT"\)/], + ['cursor', /CURSOR_MODEL="\$\{REVIEW_MODEL\}\[effort=\$\{REVIEW_EFFORT\}\]"/], + ['agy', /agy\) : ;;/], + ]; + for (const [agent, re] of CARRIERS) { + assert.match(preflight, re, `${agent} must get the carrier its CLI accepts`); + } + + // Fail closed: the default is no flag, and the unknown-agent arm guesses nothing. + assert.match(preflight, /^EFFORT_FLAG=\(\)$/m); + assert.match(preflight, /\*\)\s+: ;;/, 'unknown agents must not inherit a flag'); + assert.ok( + !/^\[ -n "\$REVIEW_EFFORT" \] && EFFORT_FLAG=\(--effort/m.test(preflight), + 'no unconditional --effort assignment may precede the per-agent dispatch', + ); + + // No invocation may pass a carrier its CLI rejects. + for (const agent of ['codex', 'agy', 'cursor']) { + const row = loop.split('\n').find((l) => l.startsWith(`| \`${agent}\` |`)); + assert.ok(row, `${agent} invocation row must exist`); + assert.ok( + agent === 'codex' || !row.includes('EFFORT_FLAG'), + `the ${agent} invocation must not pass EFFORT_FLAG`, + ); + } + + // The carrier table is the documented rule, and agy's variant is discovered + // at run time rather than baked into a level table that would go stale. + assert.match(loop, /\*\*Effort carriers\.\*\*/); + assert.match(loop, /\| `agy` \| a model \*\*variant\*\* picked from `agy models`/); + assert.match(loop, /not from a remembered table/); + assert.match(loop, /AGY_MODEL_RESOLVED/, 'the agy choice must persist across loop iterations'); + }); it('tells the in-process claude reviewer what to do with ~effort, and what not to reach for', () => { // The Agent tool takes a model but no reasoning effort, so a dispatching agent @@ -432,12 +492,11 @@ describe('review-loop parse contracts', () => { assert.match(loop, /Grok Build also installs an `agent` binary/); assert.match(loop, /--mode=ask/); assert.match(loop, /--force --trust/); - assert.match(loop, /Do not pass `EFFORT_FLAG` to `cursor`/); + assert.match(loop, /\| `cursor` \| folded into `--model` as `\[effort=\]`/); // ~effort must actually change Cursor inference: fold into --model as // [effort=], matching cursor[gpt-5]~effort=max and a saved // review-models cursor=gpt-5 plus cursor~effort=max. Never pass --effort. assert.match(loop, /CURSOR_MODEL="\$\{REVIEW_MODEL\}\[effort=\$\{REVIEW_EFFORT\}\]"/); - assert.match(loop, /folds `\{REVIEW_EFFORT\}` into the `--model` value/); assert.match(loop, /gpt-5\[effort=max\]/); // Review-only must not grant --force; reviewer-applies must. assert.match(loop, /omits `--force`/); @@ -446,9 +505,8 @@ describe('review-loop parse contracts', () => { // other reviewers — a saved review-models entry and a ~effort suffix. assert.match(readCommand('config.md'), /--review-models codex=o3,claude=claude-opus-4-8,cursor=gpt-5/); assert.match(readCommand('config.md'), /cursor\[gpt-5\]~effort=max/); - const readme = fs.readFileSync(path.join(__dirname, '..', 'README.md'), 'utf8'); - assert.match(readme, /cursor\[gpt-5\]~effort=max/); - assert.match(readme, /--review-models cursor=/); + assert.match(_read('README.md'), /cursor\[gpt-5\]~effort=max/); + assert.match(_read("README.md"), /--review-models cursor=/); assert.match(wrapper, /`cursor` \(alias `cursor-agent`\)/); assert.match(wrapper, /`codex` \| `agy` \| `claude` \| `grok` \| `cursor`/); From abcc21bad9a8f9f25b5011155430367325981e35 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Thu, 27 Aug 2026 07:43:18 -0700 Subject: [PATCH 9/9] chore: release v3.35.0 --- .changelogs/v3.35.0.md | 25 +++++++++++++++++++++++++ package.json | 2 +- 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 .changelogs/v3.35.0.md diff --git a/.changelogs/v3.35.0.md b/.changelogs/v3.35.0.md new file mode 100644 index 0000000..11b1e03 --- /dev/null +++ b/.changelogs/v3.35.0.md @@ -0,0 +1,25 @@ +# Release v3.35.0 + +Released: 2026-08-27 + +## Highlights + +- **`~effort=` now actually reaches codex and agy — before this, it silently killed them.** Reasoning effort was passed to every reviewer as `--effort `, a flag only `claude` and `grok` accept. codex and agy both reject it and exit non-zero *before the review starts*, so the reviewer's slot in the merge gate held a launch failure rather than a verdict. Each reviewer now gets the carrier its own CLI accepts. +- **The effort dispatch fails closed.** The old shape assigned `--effort` generically and then overrode it per agent, so any CLI without an explicit branch inherited the wrong flag — which is exactly how codex and agy broke. An unrecognized reviewer now gets no flag at all and keeps prompt-advisory effort, because a wrong flag isn't a weaker review, it's a non-zero exit. +- **`/do:release` verifies that main actually reached release before reporting success**, and recovers cleanly when a release is interrupted partway. + +## Reasoning-effort carriers + +- **codex** has no `--effort` flag at any level — not top-level, not on `review`, not on `exec`. It exits 2 with `unexpected argument '--effort' found`. Effort is a config value there, so it now goes through the top-level `-c model_reasoning_effort=` override. +- **agy** treats `--effort` as mutually exclusive with `--model`, and the loop always pins `--model` for agy. Since the built-in default model is itself an effort variant, `agy~effort=` failed 100% of the time — at every level, including ones agy offers. agy encodes effort as a *model variant*, so it is now resolved by picking from the `agy models` roster at run time. Nothing is hardcoded: the roster and level names change between agy releases, and a level agy doesn't offer resolves to the closest one it does. +- A **new effort-carrier table** in the local-agent loop is now the single statement of the rule, replacing the same fact restated across six places. The per-agent "Flag rationale" bullets keep the verified evidence behind each. +- A reviewer with no effort control — or no matching level — degrades to prompt-advisory effort instead of failing the pass. + +## Release promotion + +- `/do:release` now confirms the source branch actually promoted to the target on the remote before it reports success, instead of trusting an exit status. +- Release preparation is recoverable: interrupted runs preserve their state across retries, checkpoint handoff is idempotent, and the retry edge cases around an already-prepared release are closed. + +## Full Changelog + +**Full Diff**: https://github.com/atomantic/slashdo/compare/v3.34.0...v3.35.0 diff --git a/package.json b/package.json index efd2dfb..a5cc913 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "slash-do", - "version": "3.34.0", + "version": "3.35.0", "description": "Curated slash commands for AI coding assistants — Claude Code, OpenCode, Antigravity CLI, Codex, and Grok Build", "author": "Adam Eivy ", "license": "MIT",