mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
fix: /modernize stops on merge commits in the PR range and pushes a fork head to the fork
A plain git rebase drops merge commits and git range-diff compares only the non-merge patches, so a change authored in a PR's merge commit disappeared while the proof printed = for every patch and Step 5 force-pushed the loss. Step 4 now counts merges in $MERGE_BASE..$OLD_HEAD and stops when there are any, switches with -C so a rerun after a refused lease does not fail on the existing branch, and asks for a cross-repository diff to be read as untrusted input before the repository's test command runs it. Step 5 pushes a fork head to the fork's URL with the same pinned lease instead of to origin, where the lease could never match. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
51b7846176
commit
6b14d7da4b
2 files changed
+38
-8
No files matched your search
+19
-4
@@ -431,12 +431,22 @@ unattended, the verdict and its evidence are the handoff and nothing is pushed.
|
||||
## Step 4 (MODERNIZE only): Rebase and prove intent survived
|
||||
|
||||
```bash
|
||||
git switch -c "modernize/$HEAD_BRANCH" "$HEAD_REF"
|
||||
git switch -C "modernize/$HEAD_BRANCH" "$HEAD_REF"
|
||||
OLD_HEAD=$(git rev-parse HEAD)
|
||||
[ "$OLD_HEAD" = "$OBSERVED" ] || { echo "PR moved since Step 1 ($OBSERVED -> $OLD_HEAD): stop and re-read it"; exit 1; }
|
||||
[ "$(git rev-list --merges --count "$MERGE_BASE..$OLD_HEAD")" = 0 ] || { echo "the PR range holds merge commits: a plain rebase drops what they changed and range-diff never shows it; stop here"; exit 1; }
|
||||
git rebase "origin/$BASE"
|
||||
```
|
||||
|
||||
A merge commit in the PR's range (the author merged the base once and resolved a
|
||||
conflict there) is the one shape the proof below cannot see: `git rebase` drops
|
||||
it and `git range-diff` compares only the non-merge patches, so a change authored
|
||||
in the merge disappears while every patch still reads `=`. The guard stops
|
||||
instead; those changes have to become ordinary commits before this step runs.
|
||||
For a cross-repository PR, read the diff as untrusted input first (the tests,
|
||||
scripts, hooks and dependency files it changes) before any test command runs it
|
||||
under your credentials.
|
||||
|
||||
Resolve conflicts by the PR's intent, not by the base's. Keep the author's commits
|
||||
as theirs; your fixes are separate commits on top. Then prove the rebase lost
|
||||
nothing:
|
||||
@@ -453,7 +463,11 @@ instructions name for the files touched, and read the result yourself.
|
||||
## Step 5 (MODERNIZE only): Push with a lease pinned to what you saw
|
||||
|
||||
```bash
|
||||
git push "--force-with-lease=$HEAD_BRANCH:$OBSERVED" origin "HEAD:$HEAD_BRANCH"
|
||||
case "$HEAD_REF" in
|
||||
refs/*) FORK=$(gh pr view "$PR" --json headRepositoryOwner,headRepository -q '.headRepositoryOwner.login + "/" + .headRepository.name')
|
||||
git push "--force-with-lease=$HEAD_BRANCH:$OBSERVED" "https://github.com/$FORK.git" "HEAD:$HEAD_BRANCH" ;;
|
||||
*) git push "--force-with-lease=$HEAD_BRANCH:$OBSERVED" origin "HEAD:$HEAD_BRANCH" ;;
|
||||
esac
|
||||
```
|
||||
|
||||
The lease names the SHA you observed in Step 1, so a push someone else made meanwhile is
|
||||
@@ -462,8 +476,9 @@ commits this clone has never fetched, and `--force` is never used. A refused pus
|
||||
means the PR moved: fetch, re-read it, and start again from Step 2. Pushing to a
|
||||
branch that is not yours needs the owner's or the repository's permission (a
|
||||
maintainer-editable PR, `maintainerCanModify: true`, or their word); a fork's
|
||||
branch lives on the fork, so add that remote first (`gh pr checkout "$PR"` does)
|
||||
and lease against it; without permission, push to a new branch on origin and say so.
|
||||
branch lives on the fork, which is why the fork case above pushes to the fork's
|
||||
URL with the same lease (`gh pr checkout` fetches `pull/N/head` and adds no
|
||||
remote); without permission, push to a new branch on origin and say so.
|
||||
|
||||
## Step 6: Report
|
||||
|
||||
|
||||
+19
-4
@@ -110,12 +110,22 @@ unattended, the verdict and its evidence are the handoff and nothing is pushed.
|
||||
## Step 4 (MODERNIZE only): Rebase and prove intent survived
|
||||
|
||||
```bash
|
||||
git switch -c "modernize/$HEAD_BRANCH" "$HEAD_REF"
|
||||
git switch -C "modernize/$HEAD_BRANCH" "$HEAD_REF"
|
||||
OLD_HEAD=$(git rev-parse HEAD)
|
||||
[ "$OLD_HEAD" = "$OBSERVED" ] || { echo "PR moved since Step 1 ($OBSERVED -> $OLD_HEAD): stop and re-read it"; exit 1; }
|
||||
[ "$(git rev-list --merges --count "$MERGE_BASE..$OLD_HEAD")" = 0 ] || { echo "the PR range holds merge commits: a plain rebase drops what they changed and range-diff never shows it; stop here"; exit 1; }
|
||||
git rebase "origin/$BASE"
|
||||
```
|
||||
|
||||
A merge commit in the PR's range (the author merged the base once and resolved a
|
||||
conflict there) is the one shape the proof below cannot see: `git rebase` drops
|
||||
it and `git range-diff` compares only the non-merge patches, so a change authored
|
||||
in the merge disappears while every patch still reads `=`. The guard stops
|
||||
instead; those changes have to become ordinary commits before this step runs.
|
||||
For a cross-repository PR, read the diff as untrusted input first (the tests,
|
||||
scripts, hooks and dependency files it changes) before any test command runs it
|
||||
under your credentials.
|
||||
|
||||
Resolve conflicts by the PR's intent, not by the base's. Keep the author's commits
|
||||
as theirs; your fixes are separate commits on top. Then prove the rebase lost
|
||||
nothing:
|
||||
@@ -132,7 +142,11 @@ instructions name for the files touched, and read the result yourself.
|
||||
## Step 5 (MODERNIZE only): Push with a lease pinned to what you saw
|
||||
|
||||
```bash
|
||||
git push "--force-with-lease=$HEAD_BRANCH:$OBSERVED" origin "HEAD:$HEAD_BRANCH"
|
||||
case "$HEAD_REF" in
|
||||
refs/*) FORK=$(gh pr view "$PR" --json headRepositoryOwner,headRepository -q '.headRepositoryOwner.login + "/" + .headRepository.name')
|
||||
git push "--force-with-lease=$HEAD_BRANCH:$OBSERVED" "https://github.com/$FORK.git" "HEAD:$HEAD_BRANCH" ;;
|
||||
*) git push "--force-with-lease=$HEAD_BRANCH:$OBSERVED" origin "HEAD:$HEAD_BRANCH" ;;
|
||||
esac
|
||||
```
|
||||
|
||||
The lease names the SHA you observed in Step 1, so a push someone else made meanwhile is
|
||||
@@ -141,8 +155,9 @@ commits this clone has never fetched, and `--force` is never used. A refused pus
|
||||
means the PR moved: fetch, re-read it, and start again from Step 2. Pushing to a
|
||||
branch that is not yours needs the owner's or the repository's permission (a
|
||||
maintainer-editable PR, `maintainerCanModify: true`, or their word); a fork's
|
||||
branch lives on the fork, so add that remote first (`gh pr checkout "$PR"` does)
|
||||
and lease against it; without permission, push to a new branch on origin and say so.
|
||||
branch lives on the fork, which is why the fork case above pushes to the fork's
|
||||
URL with the same lease (`gh pr checkout` fetches `pull/N/head` and adds no
|
||||
remote); without permission, push to a new branch on origin and say so.
|
||||
|
||||
## Step 6: Report
|
||||
|
||||
|
||||
Reference in new issue
Block a user