ci: open PR to homebrew-private-tap and auto-merge with scoped bypass - #24
Conversation
homebrew-private-tap main is protected; direct pushes fail unless the SAWMILLS_REPO_APP token bypasses protection on every change. Use the sawmills-agent pattern instead: create a branch, open a PR, verify the diff only touches the expected formula, verify HEAD hasn't moved, then gh pr merge --admin. Humans still need PR review for any manual change. Requires the SAWMILLS_REPO_APP installation (or configured app) to be in the bypass actors list for homebrew-private-tap main protection. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughWorkflows now push Homebrew updates to dedicated per-release branches, open or reuse PRs targeting Changes
Sequence Diagram(s)sequenceDiagram
participant CI as rgba(70,130,180,0.5)
participant Git as rgba(34,139,34,0.5)
participant GH as rgba(255,165,0,0.5)
participant Tap as rgba(128,0,128,0.5)
CI->>Git: fetch remote branch (if exists)
CI->>Git: create local branch (clerk.../v${version} or clerk/${tag})
CI->>Git: commit recipe update
CI->>Git: push --force-with-lease to branch
CI->>GH: gh pr list (find open PR from branch)
alt PR exists
GH-->>CI: return PR number
else no PR
CI->>GH: gh pr create -> returns PR URL/number
end
CI->>GH: gh pr view --json baseRefName,isCrossRepository,files,headRefOid
GH-->>CI: PR metadata snapshot
CI->>CI: validate base == "main" && not cross-repo && files == expected
alt validation passes
CI->>GH: gh pr merge --squash --match-head-commit --delete-branch
GH->>Tap: merge changes into `main`
GH->>Git: delete source branch
else validation fails
CI-->>GH: exit non-zero (leave PR open)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 2/5
- There is a high-confidence, high-severity workflow risk in
.github/workflows/release-raycast.yaml: the job can auto-merge without verifying that the PR head still matches the commit it pushed. - This can cause an unexpected but "stable" branch head to be merged, creating a concrete regression risk in release automation rather than a purely cosmetic CI issue.
- Pay close attention to
.github/workflows/release-raycast.yaml- ensure the merge step validates the current PR head SHA against the pushed commit before auto-merging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/release-raycast.yaml">
<violation number="1" location=".github/workflows/release-raycast.yaml:200">
P1: The PR head is never checked against the commit this job pushed, so a stable-but-unexpected branch head can be auto-merged.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Address review feedback on the PR-based tap flow: - Reuse an existing open PR for the same branch instead of erroring on retry (gh pr list --head BRANCH first; only create if missing). - Force-with-lease push so a stale branch from a prior failed attempt doesn't block the push. - Drop the double-read + sleep SHA check; use gh pr merge --match-head-commit to atomically abort if HEAD moves between read and merge. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![sawmills-architect-review[bot]](https://avatars.githubusercontent.com/in/2910947?s=60&v=4){kind=small}
There was a problem hiding this comment.
[ARCH-REVIEW] REQUEST_CHANGES
This switches both release workflows to PR-based updates against homebrew-private-tap, and the file-scope guard is fine. The release still depends on gh pr merge --admin, though, which makes completion require repo-admin rights instead of the scoped bypass you described.
If the app token only has write + bypass permissions, the job will open the PR and then fail at merge time.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release-raycast.yaml:
- Around line 188-204: Replace the separate validation and merge steps with a
single atomic snapshot: call gh pr view once to retrieve baseRefName,
isCrossRepository, files, and headRefOid for PR_NUMBER (or the created PR),
validate baseRefName == "main", isCrossRepository == false, and files ==
"Casks/clerk-raycast.rb", then immediately pass that headRefOid into gh pr merge
via --match-head-commit (using the same HEAD_OID variable) so validation and
merge use the exact same PR snapshot; keep the PR creation logic for PR_NUMBER
but ensure you re-run the single gh pr view after creating a PR to obtain its
headRefOid before validating/merging.
- Around line 183-186: The push uses --force-with-lease without a
remote-tracking ref for BRANCH, so fetch the remote branch into
refs/remotes/origin/$BRANCH before pushing and then push to origin; e.g., after
creating BRANCH run a fetch like git fetch origin
"$BRANCH":"refs/remotes/origin/$BRANCH" (or git fetch --no-tags --depth=1 origin
"+refs/heads/$BRANCH:refs/remotes/origin/$BRANCH") to create the tracking ref,
then use git push --force-with-lease origin "$BRANCH" instead of pushing
directly to the URL; update the commands that reference BRANCH and the git push
invocation accordingly.
In @.github/workflows/release.yaml:
- Around line 177-193: Add --base main to the gh pr list invocation to ensure
the PR selected matches base main, and replace the two separate gh pr view calls
with a single gh pr view that requests baseRefName, isCrossRepository, files,
and headRefOid (use these fields to validate the base is main, confirm
repository ownership via isCrossRepository, and that files only contains
"Formula/clerk.rb"); capture headRefOid into HEAD_SHA and then call gh pr merge
"$PR_NUMBER" with --match-head-commit "$HEAD_SHA" as before. Ensure you still
set PR_NUMBER from gh pr list/gh pr create logic and use the consolidated gh pr
view results to perform the file/base validations before merging.
- Around line 172-175: The push uses --force-with-lease against a raw URL so it
cannot validate remote-tracking refs; fetch the branch into
refs/remotes/origin/$BRANCH first and push to the configured remote instead of
the URL. Specifically, before creating/pushing BRANCH (variable BRANCH), run a
fetch to populate refs/remotes/origin/$BRANCH (e.g., fetch origin
"$BRANCH":"refs/remotes/origin/$BRANCH"), then replace the git push that targets
the raw URL with a push to origin using --force-with-lease "origin" "$BRANCH".
This keeps the existing BRANCH creation/commit flow (git checkout -b "$BRANCH",
git commit -m ...) but ensures --force-with-lease has an origin/$BRANCH to
validate against.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8816ae65-3188-4a60-8fe3-9b4012116264
📒 Files selected for processing (2)
.github/workflows/release-raycast.yaml.github/workflows/release.yaml
Review feedback: - Drop `--admin`: with scoped bypass the app can merge via standard endpoint; `--admin` would require admin rights. - Fetch the remote branch before --force-with-lease so retries in shallow clones have a tracking ref to compare against. - Consolidate validation and merge: single `gh pr view` reads baseRefName, isCrossRepository, files, and headRefOid; merge passes that snapshot's head SHA to --match-head-commit. - Also filter `gh pr list` by --base main to avoid matching a stale PR against a different base. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@sawmills-architect-review Addressed since your last review — requesting re-review.
|
There was a problem hiding this comment.
[ARCH-REVIEW] REQUEST_CHANGES
1 blocking issue: both release workflows now open PRs in Sawmills/homebrew-private-tap, but the branch is still pushed to the cloned homebrew-tap origin, so the target repo never gets the branch. gh pr create/merge against the private tap cannot work until this remote mismatch is fixed. Same issue exists in release-raycast.yaml.
The cloned workdir's 'origin' is homebrew-private-tap, but a diff hunk without the clone/cd context made automated reviewers repeatedly read it as the source repo. Use the full tap URL inline on fetch/push so the target is unambiguous regardless of surrounding context. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Stale — all flagged threads have been addressed and resolved; see updated diff.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release-raycast.yaml (1)
61-71:⚠️ Potential issue | 🟠 MajorDon’t skip tap reconciliation once the release exists.
If the initial run creates the GitHub release and then stops before auto-merging the tap PR—for example because the bypass actor has not been configured yet—
check_release.outputs.skipflips totrueand every rerun skips this entire block. The PR is then left open permanently unless someone merges it by hand..github/workflows/release.yamlavoids that retry hole. The tap update needs to run independently of release creation, withsha256sourced from the published asset on reruns.Also applies to: 109-110, 190-215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-raycast.yaml around lines 61 - 71, The current "Check existing release" step (id: check_release) sets outputs.skip by running `gh release view "${tag}"` and later gating the tap update on that value, which causes reruns to skip tap reconciliation once the release exists; change the workflow so the tap update runs unconditionally (or at least when the tap PR is open) regardless of check_release.outputs.skip — remove or change the conditional that prevents the tap update job/steps from executing when check_release reports the release exists. Keep the check_release step only for deciding whether to create a release, and for reruns make the tap update step source sha256 from the already-published asset (use `gh release download` or `gh api` to fetch the built asset and compute its sha256) so that tap reconciliation can complete independently of release creation.
♻️ Duplicate comments (2)
.github/workflows/release.yaml (1)
173-177:⚠️ Potential issue | 🟠 MajorPush through
originso the fetched lease actually protects the branch.This fetch populates
refs/remotes/origin/$BRANCH, but the subsequentgit push --force-with-leasestill targets a raw URL. That may disconnect the lease from the tracking ref and let concurrent retries overwrite the branch instead of failing cleanly. The clone already pointsoriginathomebrew-private-tap, so pushing viaoriginkeeps the target the same while preserving the lease.Suggested change
- git fetch "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" "refs/heads/$BRANCH:refs/remotes/origin/$BRANCH" 2>/dev/null || true + git fetch origin "refs/heads/$BRANCH:refs/remotes/origin/$BRANCH" 2>/dev/null || true git checkout -b "$BRANCH" git commit -m "feat(clerk): update to ${tag}" - git push --force-with-lease "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" "$BRANCH" + git push --force-with-lease origin "$BRANCH"Does `git push --force-with-lease <raw-url> <branch>` honor an existing `refs/remotes/origin/<branch>` tracking ref, or must the push target be the named remote (for example `origin`) or an explicit `<ref>:<expect>` lease for concurrent-update protection to work?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yaml around lines 173 - 177, The push uses a raw URL while the fetch populated refs/remotes/origin/$BRANCH, which can break the --force-with-lease protection; change the push to target the named remote (origin) instead of the raw URL so the lease against refs/remotes/origin/$BRANCH is honored—i.e., replace the git push --force-with-lease "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" "$BRANCH" invocation with a git push --force-with-lease origin "$BRANCH" (or an explicit <localref>:refs/heads/<branch> lease syntax) to preserve concurrent-update protection..github/workflows/release-raycast.yaml (1)
184-188:⚠️ Potential issue | 🟠 MajorPush through
originhere as well so the lease check has a real tracking ref.This fetches
refs/remotes/origin/$BRANCH, but the--force-with-leasepush still goes to a raw URL. If Git treats that as an anonymous remote, concurrent reruns can still force-update the branch instead of tripping the lease. Usingoriginor an explicit<ref>:<expect>lease keeps the retry protection intact.Suggested change
- git fetch "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" "refs/heads/$BRANCH:refs/remotes/origin/$BRANCH" 2>/dev/null || true + git fetch origin "refs/heads/$BRANCH:refs/remotes/origin/$BRANCH" 2>/dev/null || true git checkout -b "$BRANCH" git commit -m "feat(clerk-raycast): update to v${version}" - git push --force-with-lease "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" "$BRANCH" + git push --force-with-lease origin "$BRANCH"Does `git push --force-with-lease <raw-url> <branch>` honor an existing `refs/remotes/origin/<branch>` tracking ref, or must the push target be the named remote (for example `origin`) or an explicit `<ref>:<expect>` lease for concurrent-update protection to work?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-raycast.yaml around lines 184 - 188, The push uses a raw URL which can bypass the local tracking ref and defeat --force-with-lease; update the push so it targets the named remote or an explicit ref-to-ref lease instead of the raw URL — e.g. ensure the earlier git fetch that populates refs/remotes/origin/$BRANCH is matched by using git push --force-with-lease origin "$BRANCH" or git push --force-with-lease "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" "refs/heads/$BRANCH:refs/heads/$BRANCH" so the lease check will consult the tracking ref; adjust the git push invocation that references $BRANCH accordingly.
🧹 Nitpick comments (1)
.github/workflows/release.yaml (1)
172-204: Consider extracting the tap PR flow into shared workflow logic.This block is now almost identical to
.github/workflows/release-raycast.yaml:183-215. Keeping the branch push, PR snapshot, and merge safety checks duplicated makes future fixes easy to miss in one workflow. A reusable workflow or composite action would let these safeguards live in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/release-raycast.yaml:
- Around line 61-71: The current "Check existing release" step (id:
check_release) sets outputs.skip by running `gh release view "${tag}"` and later
gating the tap update on that value, which causes reruns to skip tap
reconciliation once the release exists; change the workflow so the tap update
runs unconditionally (or at least when the tap PR is open) regardless of
check_release.outputs.skip — remove or change the conditional that prevents the
tap update job/steps from executing when check_release reports the release
exists. Keep the check_release step only for deciding whether to create a
release, and for reruns make the tap update step source sha256 from the
already-published asset (use `gh release download` or `gh api` to fetch the
built asset and compute its sha256) so that tap reconciliation can complete
independently of release creation.
---
Duplicate comments:
In @.github/workflows/release-raycast.yaml:
- Around line 184-188: The push uses a raw URL which can bypass the local
tracking ref and defeat --force-with-lease; update the push so it targets the
named remote or an explicit ref-to-ref lease instead of the raw URL — e.g.
ensure the earlier git fetch that populates refs/remotes/origin/$BRANCH is
matched by using git push --force-with-lease origin "$BRANCH" or git push
--force-with-lease
"https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git"
"refs/heads/$BRANCH:refs/heads/$BRANCH" so the lease check will consult the
tracking ref; adjust the git push invocation that references $BRANCH
accordingly.
In @.github/workflows/release.yaml:
- Around line 173-177: The push uses a raw URL while the fetch populated
refs/remotes/origin/$BRANCH, which can break the --force-with-lease protection;
change the push to target the named remote (origin) instead of the raw URL so
the lease against refs/remotes/origin/$BRANCH is honored—i.e., replace the git
push --force-with-lease
"https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git"
"$BRANCH" invocation with a git push --force-with-lease origin "$BRANCH" (or an
explicit <localref>:refs/heads/<branch> lease syntax) to preserve
concurrent-update protection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1e55da64-e42b-4fd2-b73b-6d99f4c4fc1e
📒 Files selected for processing (2)
.github/workflows/release-raycast.yaml.github/workflows/release.yaml
There was a problem hiding this comment.
[ARCH-REVIEW] REQUEST_CHANGES
The PR rewrite is directionally good, but both release workflows still reuse a bare head-branch lookup to find an existing PR. That selection is ambiguous across forks, so a same-named fork PR can be picked up and make the job validate the wrong PR or abort. Please disambiguate the lookup before merging.
--force-with-lease with an explicit URL can reject valid pushes because git cannot resolve a remote-tracking ref for the URL. This is an automated workflow with a single writer (the release pipeline), so the concurrent-update protection --with-lease adds is unnecessary. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/release-raycast.yaml">
<violation number="1" location=".github/workflows/release-raycast.yaml:188">
P1: Using `git push --force` here can overwrite concurrent updates to the release branch; keep `--force-with-lease` to preserve the expected-head safety check.</violation>
</file>
<file name=".github/workflows/release.yaml">
<violation number="1" location=".github/workflows/release.yaml:177">
P1: Using `--force` here can clobber newer remote branch commits; keep `--force-with-lease` to preserve concurrency safety during retries.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
.github/workflows/release.yaml (1)
179-186:⚠️ Potential issue | 🟠 MajorResolve the correct PR, not just the first matching head branch.
This is still vulnerable to selecting the wrong open PR.
gh pr list --head "$BRANCH" --base maincan return a fork PR that happens to use the same deterministic branch name; the laterisCrossRepositorycheck then aborts instead of finding the repo-owned PR you actually pushed. Filter the candidates by owner / cross-repo status before choosingPR_NUMBER.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yaml around lines 179 - 186, The current logic grabs the first open PR for head "$BRANCH" which may be from a fork; modify the selection to list PRs with machine-readable fields (use gh pr list --json number,headRefName,isCrossRepository,headRepository.owner.login) and filter the results for a non-cross-repository PR owned by Sawmills (or where isCrossRepository is false) before setting PR_NUMBER; only if no matching repo-owned PR exists, fall back to creating a new PR and deriving PR_NUMBER from PR_URL as before. Ensure you update the parsing that sets PR_NUMBER (replacing the current single-item jq use) and keep the later isCrossRepository check consistent with the new selection logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release-raycast.yaml:
- Around line 190-197: The gh CLI query that sets PR_NUMBER can return a forked
PR; update the gh pr list invocation that assigns PR_NUMBER (the command using
gh pr list --head "$BRANCH" --base main and jq) to filter out cross-repository
PRs by selecting only items with isCrossRepository == false before taking the
first element, so PR_NUMBER points to an in-repo PR; keep the fallback PR
creation logic that sets PR_URL and derives PR_NUMBER from PR_URL unchanged.
- Around line 183-188: The git push currently uses an unconditional --force
which can overwrite reviewer commits; change the push in this job to use
--force-with-lease instead so it respects remote changes; specifically update
the git push invocation that targets the BRANCH variable (the line using git
push "...${GH_TOKEN}...\" \"$BRANCH\"") to push with --force-with-lease, keeping
the preceding git fetch and git checkout -b steps as-is to ensure the lease has
a tracking ref for retries.
In @.github/workflows/release.yaml:
- Around line 172-177: Replace the unconditional force push with a lease-aware
push to avoid clobbering reviewer commits: in the block that sets
BRANCH="clerk/${tag}", keeps the git fetch that populates
refs/remotes/origin/$BRANCH, and then change the git push invocation that
currently uses "git push --force
\"https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git\"
\"$BRANCH\"" to use --force-with-lease against that same explicit URL so the
push honors the fetched tracking ref and fails if someone else updated the
branch.
---
Duplicate comments:
In @.github/workflows/release.yaml:
- Around line 179-186: The current logic grabs the first open PR for head
"$BRANCH" which may be from a fork; modify the selection to list PRs with
machine-readable fields (use gh pr list --json
number,headRefName,isCrossRepository,headRepository.owner.login) and filter the
results for a non-cross-repository PR owned by Sawmills (or where
isCrossRepository is false) before setting PR_NUMBER; only if no matching
repo-owned PR exists, fall back to creating a new PR and deriving PR_NUMBER from
PR_URL as before. Ensure you update the parsing that sets PR_NUMBER (replacing
the current single-item jq use) and keep the later isCrossRepository check
consistent with the new selection logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 82bc2e6a-110d-4576-987f-76c409b80360
📒 Files selected for processing (2)
.github/workflows/release-raycast.yaml.github/workflows/release.yaml
gh pr list --head filters only by branch name, so a fork PR sharing the same branch name could be picked up and validated/merged in place of the internal release PR. Filter on headRepositoryOwner = Sawmills to ensure we only reuse a PR from the intended repo. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Architect concern: plain --force can clobber concurrent updates to the release branch. Cubic concern: --force-with-lease with an inline URL rejects valid pushes because there's no auto-discovered tracking ref. Both are addressed by --force-with-lease=$BRANCH:$EXPECT_SHA where EXPECT_SHA is read from the tracking ref populated by the earlier fetch (empty string when the remote branch doesn't exist, which means 'expect no remote branch' and allows first-time push). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Stale — feedback addressed in latest commits; baz-reviewer acknowledged.
There was a problem hiding this comment.
[ARCH-REVIEW] REQUEST_CHANGES
The branch lookup is still ambiguous. In both release workflows, a bare --head "$BRANCH" can select a fork PR with the same deterministic head branch, so the job can validate the wrong PR and abort before merging the repo-owned one.
gh pr list --head is a bare-branch filter and doesn't accept the owner:branch syntax, so architect flagged the lookup as ambiguous against fork PRs even with a jq post-filter. Call the REST API directly with head=Sawmills:$BRANCH, which the pulls endpoint honors natively — no cross-owner branch can match. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Stale — all flagged concerns addressed in later commits.
There was a problem hiding this comment.
[ARCH-REVIEW] REQUEST_CHANGES
Prior blocking issues are fixed. One blocking issue remains: the branch lease is refreshed from the live remote tip on every rerun, so a manual commit on the open PR branch can still be overwritten on the next push. Preserve the original head SHA, or stop force-pushing once the PR exists.
| exit 0 | ||
| fi | ||
|
|
||
| BRANCH="clerk/${tag}" |
There was a problem hiding this comment.
🔴 [blocking] EXPECT_SHA is refreshed from the current remote tip immediately before the push, so this only protects the fetch→push race. If someone updates the open PR branch before a rerun starts, the workflow will fetch that new commit and then overwrite it anyway. Preserve the original PR head SHA (or stop force-pushing once the PR exists) if this branch can be edited manually.
There was a problem hiding this comment.
[ARCH-REVIEW] Re-review: REQUEST_CHANGES
Prior blocking issue remains open on .github/workflows/release.yaml. EXPECT_SHA is still sourced from the fetched remote tip immediately before push, so the lease only protects fetch→push, not edits made before the rerun starts.
Unresolved threads: 1
| # Fetch existing remote branch so --force-with-lease has a tracking ref on retries. | ||
| git fetch "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" "refs/heads/$BRANCH:refs/remotes/origin/$BRANCH" 2>/dev/null || true | ||
| git checkout -b "$BRANCH" | ||
| git commit -m "feat(clerk): update to ${tag}" | ||
| git push "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" HEAD:main | ||
| # Use explicit-lease form: local ref must match remote. Empty expect = no remote branch yet. |
There was a problem hiding this comment.
🔴 [blocking] Previously flagged, still unaddressed: this lease is still derived from refs/remotes/origin/$BRANCH fetched at job start. If the release PR branch was updated before the rerun, this code accepts the newer remote tip and then overwrites it. Preserve the original PR head SHA (or stop force-pushing once the PR exists) so retries cannot clobber manual updates.
| # Fetch existing remote branch so --force-with-lease has a tracking ref on retries. | ||
| git fetch "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" "refs/heads/$BRANCH:refs/remotes/origin/$BRANCH" 2>/dev/null || true | ||
| git checkout -b "$BRANCH" | ||
| git commit -m "feat(clerk): update to ${tag}" | ||
| git push "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" HEAD:main | ||
| # Use explicit-lease form: local ref must match remote. Empty expect = no remote branch yet. | ||
| EXPECT_SHA=$(git rev-parse -q --verify refs/remotes/origin/$BRANCH || true) | ||
| git push --force-with-lease="$BRANCH:$EXPECT_SHA" "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" "$BRANCH" |
There was a problem hiding this comment.
🔴 [blocking] This lease is still rebuilt from the current remote tip on every rerun. If the open tap PR branch was edited after the previous job, the fetch here makes this run adopt that newer tip and will overwrite it instead of rejecting the rerun. Preserve the original PR head SHA for the lease, or stop force-pushing once the PR exists. The same bug is present in release-raycast.yaml.
There was a problem hiding this comment.
[ARCH-REVIEW] REQUEST_CHANGES
Both release workflows still rebuild the lease from the current remote tip before pushing, so a rerun can overwrite pre-existing edits on the open tap PR branch. That is still a merge-safety regression in the PR-based release flow.
Unresolved threads: 2.
| git commit -m "feat(clerk): update to ${tag}" | ||
| git push "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" HEAD:main | ||
| # Use explicit-lease form: local ref must match remote. Empty expect = no remote branch yet. | ||
| EXPECT_SHA=$(git rev-parse -q --verify refs/remotes/origin/$BRANCH || true) |
There was a problem hiding this comment.
🔴 [blocking] This lease is reconstructed from the current remote tip on every rerun, so any manual update that lands on the open tap PR branch before the job starts will be fetched and then overwritten by the next push. If this branch is meant to survive human edits, keep the originally observed head SHA (or stop force-pushing once the PR exists) instead of refreshing EXPECT_SHA from refs/remotes/origin/$BRANCH.
There was a problem hiding this comment.
[ARCH-REVIEW] REQUEST_CHANGES
The PR selection and atomic snapshot work looks fine. One blocking issue remains: the force-with-lease expectation is still rebuilt from the current remote tip on each rerun, so an open release branch that was edited by a human can be fetched and then overwritten instead of rejected. Preserve the original PR head SHA, or stop force-pushing once the PR exists, so retries cannot clobber manual updates.
| git commit -m "feat(clerk): update to ${tag}" | ||
| git push "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" HEAD:main | ||
| # Use explicit-lease form: local ref must match remote. Empty expect = no remote branch yet. | ||
| EXPECT_SHA=$(git rev-parse -q --verify refs/remotes/origin/$BRANCH || true) |
There was a problem hiding this comment.
🔴 [blocking] Previously flagged, still unaddressed: EXPECT_SHA is re-derived from refs/remotes/origin/$BRANCH on every rerun, so if someone updates the open PR branch before this job starts, the fetch adopts that newer tip and the subsequent force-push overwrites it. Preserve the original PR head SHA for the lease, or stop force-pushing once the PR exists.
There was a problem hiding this comment.
[ARCH-REVIEW] Code still has 5 unresolved review threads, so I cannot approve yet. The remaining blockers are all on the release workflow lease handling in .github/workflows/release.yaml. Resolve those threads, then I can re-check the branch.
Unresolved threads: 5
| git checkout -b "$BRANCH" | ||
| git commit -m "feat(clerk): update to ${tag}" | ||
| git push "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" HEAD:main | ||
| git push --force "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" "$BRANCH" |
There was a problem hiding this comment.
🔴 [blocking] Plain --force bypasses the lease entirely, so a rerun can overwrite commits on this open PR branch. Since this job leaves the PR open for human review, switch back to --force-with-lease (or stop force-pushing after PR creation). Same regression exists in release-raycast.yaml.
| git commit -m "feat(clerk): update to ${tag}" | ||
| git push "https://x-access-token:${GH_TOKEN}@github.com/Sawmills/homebrew-private-tap.git" HEAD:main | ||
| # Use explicit-lease form: local ref must match remote. Empty expect = no remote branch yet. | ||
| EXPECT_SHA=$(git rev-parse -q --verify refs/remotes/origin/$BRANCH || true) |
There was a problem hiding this comment.
🔴 [blocking] EXPECT_SHA is rebuilt from the current remote tip on every rerun, so the lease only protects fetch→push. If this open PR branch was edited before the job starts, this run will adopt that newer commit and then force-push over it. Preserve the original PR head SHA, or stop force-pushing once the PR exists. Same pattern exists in release-raycast.yaml.
1 participant
Summary
Releases of clerk-cli currently push formula updates directly to
mainofSawmills/homebrew-private-tap. With branch protection now enforced, theSAWMILLS_REPO_APPtoken reportspush:falseand the next tagged release would fail.Change
Switch to the
sawmills-agent/ci.yamlpattern:git checkout -b <tool>/<tag>→ push the branch.gh pr createagainstSawmills/homebrew-private-tap.head.shais stable across two reads.gh pr merge --squash --delete-branch --admin.If any validation step fails, the PR is left open for human review — no direct
mainpush.Required admin step (one-time)
Add the
SAWMILLS_REPO_APPinstallation to the bypass actors onSawmills/homebrew-private-tapmainbranch protection / ruleset, with bypass modepull_request. Until then, the release will succeed up to the auto-merge step and leave the PR open.🤖 Generated with Claude Code
Summary by cubic
Switch release workflows (formula and Raycast cask) to PR-based updates in
Sawmills/homebrew-private-tapwith strict validation and scoped-bypass auto-merge. Uses explicit-lease pushes and match-head merges for safe, idempotent releases; leaves PRs open if checks fail.Refactors
clerk/<tag>orclerk-raycast/v<version>), fetch remote, push with--force-with-lease="$BRANCH:$EXPECT_SHA"to the explicit tap URL, then open or reuse a PR tomainvia the REST pulls API (head=Sawmills:$BRANCH) to avoid fork collisions.gh pr view(base ismain, same-repo, onlyFormula/clerk.rborCasks/clerk-raycast.rbchanged); merge with--squash --delete-branch --match-head-commit <sha>(no--admin).Migration
SAWMILLS_REPO_APPinstallation to bypass actors forpull_requestonmaininSawmills/homebrew-private-tapso the bot can auto-merge without admin rights.Written for commit 5b65b29. Summary will update on new commits.
Summary by CodeRabbit