fix: use --force-with-lease to abort stale dev branch pushes#401
fix: use --force-with-lease to abort stale dev branch pushes#4010x46616c6b merged 5 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the AutoDev action against stale overwrites of the dev branch when multiple workflow runs push concurrently, by switching from unconditional force-push to a lease-guarded force-push that fails if the remote tip moved.
Changes:
- Replace
git push -fwithgit push --force-with-lease=refs/heads/<branch>:<observed-sha>and explicitly fail the action if the push is rejected. - Add Vitest coverage to assert the lease-pinned push behavior and the failure path on push rejection.
- Update the compiled
dist/index.jsbundle to reflect the new push logic.
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/autodev.ts |
Uses --force-with-lease and fails the run when the remote tip changes during execution. |
src/autodev.test.ts |
Adds tests validating the new lease-pinned push and the rejected-push failure behavior. |
dist/index.js |
Updates the distributed build output to match the source changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
flaxel
left a comment
There was a problem hiding this comment.
I guess you tested it with a simple repo to see if the new implementation works as expected. 🎉
merge() previously posted success comments and labels at the end of
its body, before autoDev() ran the push step. With force-with-lease
now able to abort the push when origin/<branch> moved during the run,
we would still leave "merged successfully" comments and the
successful label on PRs whose merges never landed on the branch.
Move the comment/label calls out of merge() into autoDev() and run
them only after the push step has completed without rejection.
merge() now returns {message, success} so the caller still has the
information it needs.
Addresses Copilot review feedback on PR #401.
Co-Authored-By: Claude <claude@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent stale AutoDev workflow runs from overwriting a newer dev branch tip when multiple runs execute concurrently, by replacing an unconditional force-push with a force-with-lease guarded push and by failing the action on push rejection.
Changes:
- Replace
git push -fwithgit push --force-with-lease=...and fail the action when the push is rejected. - Defer PR comments/labels updates until after the push step completes successfully.
- Add/extend Vitest coverage for the new push behavior and the “do not comment/label on rejected push” behavior.
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/autodev.ts | Switches push strategy to force-with-lease, adds failure handling, and defers comment/label updates until after push success. |
| src/autodev.test.ts | Adds tests around the new push command, rejection handling, and ordering of push vs. comment/label updates. |
| dist/index.js | Updates the built distribution output to reflect the source changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Multiple AutoDev workflow runs can execute in parallel — each captures origin/<base> at its own start time, builds the dev tip from base + labeled PRs, and ends with `git push -f`. If a slower run finishes after a newer one, the slower run's stale snapshot overwrites a fresher dev tip. Concrete observation in a downstream repository: a Kubernetes deployment was rolled forward, then back, then forward again within ~3 minutes, although git history showed only a single forward step — the "back" was a transient state caused by an older AutoDev push landing after a newer one. This commit replaces the unconditional force-push with a force-with-lease pinned to the SHA of origin/<branch> observed right before the push. If a concurrent run has updated the branch tip in the meantime, the push fails and the action calls setFailed instead of silently overwriting; a subsequent trigger then rebuilds the branch from a fresh state. Two new tests cover the lease argument and the setFailed path. The prebuilt dist/ is rebuilt via `pnpm run package`. Co-Authored-By: Claude <claude@anthropic.com>
merge() previously posted success comments and labels at the end of
its body, before autoDev() ran the push step. With force-with-lease
now able to abort the push when origin/<branch> moved during the run,
we would still leave "merged successfully" comments and the
successful label on PRs whose merges never landed on the branch.
Move the comment/label calls out of merge() into autoDev() and run
them only after the push step has completed without rejection.
merge() now returns {message, success} so the caller still has the
information it needs.
Addresses Copilot review feedback on PR #401.
Co-Authored-By: Claude <claude@anthropic.com>
bd7d5de to
9211c74
Compare
Co-Authored-By: Claude <claude@anthropic.com>
The previous --force-with-lease guard captured origin/${branch} immediately
before the push, after a fresh `git fetch`. That allowed a stale run to adopt
the newer remote tip as its lease expectation and overwrite it anyway, which
defeats the guard. The lease SHA is now snapshotted right after the initial
fetch (or set to the SHA we pushed when the branch is created during the run),
so a remote that moves while we work causes the lease to fail.
The push failure path now distinguishes lease rejection from other failures
(auth, network, protected branch, …) by inspecting captured stderr, instead of
always claiming origin/${branch} moved.
Co-Authored-By: Claude <claude@anthropic.com>
…nt run
Lease rejection means another AutoDev run won the race and produced a fresh
${branch} tip — the workflow that pushed last has already done the work, and a
subsequent AutoDev run will reconcile anything that landed afterwards. That is
expected behavior, not a failure, so a red ✗ on the run is misleading and
generates avoidable failure notifications.
The lease-rejected path now uses warning() so the run stays green while still
surfacing the skip in the job summary. Genuine push failures (auth, network,
protected branch, …) keep using setFailed().
Co-Authored-By: Claude <claude@anthropic.com>
Type of Change
Description
Multiple AutoDev runs can race: each one captures
origin/<base>, builds a dev tip from base + labeled PRs, and ends withgit push -f. When a slower run finishes after a newer one, its stale build silently overwrites the fresher tip. Observed downstream as a Flux-driven deployment rolling forward → back → forward within minutes, while git log showed only a single forward step.This PR replaces the unconditional
git push -fwithgit push --force-with-lease=refs/heads/<branch>:<sha>, where<sha>isorigin/<branch>snapshotted at the start of the run (capturing it right before the push would defeat the guard). Behavior on push:setFailedwith the captured stderr, instead of being swallowed byignoreReturnCode: truelike before.Recommended complementary mitigation in consumer workflows: a
concurrencygroup withcancel-in-progress: true. The lease check here is defense in depth for consumers that forget it.Checklist
The changes and the PR were generated by Claude.