starred/onyx
1
0 Fork 0
mirror of https://github.com/onyx-dot-app/onyx.git synced 2026-10-19 01:50:51 -05:00
Code Issues Packages Projects Releases 10 Wiki Activity

fix(ods): pin fork CI promotion to the confirmed commit (#14865)

Browse Source
This commit is contained in:
Justin Tahara authored and GitHub committed 2026-09-23 23:19:27 +00:00
1 parent 3416606f3e
commit 07eef23f83
2 files changed
+79 -8
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

tools/ods/cmd/run-ci.go
+24 -6
View File
@@ -109,6 +109,7 @@ type PRInfo struct {
Title string `json:"title"`
Body string `json:"body"`
HeadRefName string `json:"headRefName"`
HeadRefOid string `json:"headRefOid"`
HeadRepository struct {
Name string `json:"name"`
} `json:"headRepository"`
@@ -293,7 +294,7 @@ func runCI(prNumber string, opts *RunCIOptions) error {
if existingPRURL != "" {
action = "Update existing CI branch"
}
if !prompt.Confirm(fmt.Sprintf("%s for PR #%s? (yes/no): ", action, prNumber)) {
if !prompt.Confirm(fmt.Sprintf("%s for PR #%s (%s:%s @ %s)? (yes/no): ", action, prNumber, forkRepo, prInfo.HeadRefName, prInfo.HeadRefOid)) {
log.Info("Exiting...")
return nil
}
@@ -309,7 +310,15 @@ func runCI(prNumber string, opts *RunCIOptions) error {
return fatalErrorf("Failed to fetch fork branch: %w", err)
}
// Create or update the CI branch from FETCH_HEAD
fetchedSHA, err := git.ResolveCommit("FETCH_HEAD")
if err != nil {
return fatalErrorf("Failed to resolve FETCH_HEAD: %w", err)
}
if fetchedSHA != prInfo.HeadRefOid {
return fatalErrorf("Fork branch moved since PR #%s was read: fetched %s, expected %s. Re-run to review the new head", prNumber, fetchedSHA, prInfo.HeadRefOid)
}
// Create or update the CI branch from the confirmed commit
if originalBranch == ciBranch {
// Already on the CI branch - stash any uncommitted changes before resetting
stashResult, err := git.StashChanges()
@@ -317,7 +326,7 @@ func runCI(prNumber string, opts *RunCIOptions) error {
return fatalErrorf("Failed to stash changes: %w", err)
}
log.Infof("Already on %s, resetting to fork's HEAD", ciBranch)
if err := git.RunCommand("reset", "--hard", "FETCH_HEAD"); err != nil {
if err := git.RunCommand("reset", "--hard", fetchedSHA); err != nil {
return fatalErrorf("Failed to reset branch to fork's HEAD: %w", err)
}
git.RestoreStash(stashResult)
@@ -330,7 +339,7 @@ func runCI(prNumber string, opts *RunCIOptions) error {
}
}
log.Infof("Creating CI branch: %s", ciBranch)
if err := git.RunCommand("checkout", "--quiet", "-b", ciBranch, "FETCH_HEAD"); err != nil {
if err := git.RunCommand("checkout", "--quiet", "-b", ciBranch, fetchedSHA); err != nil {
return fatalErrorf("Failed to create CI branch: %w", err)
}
}
@@ -360,7 +369,7 @@ func runCI(prNumber string, opts *RunCIOptions) error {
} else {
pushArgs = append(pushArgs, "-f")
}
pushArgs = append(pushArgs, "--quiet", "-u", "origin", ciBranch)
pushArgs = append(pushArgs, "--quiet", "origin", fetchedSHA+":refs/heads/"+ciBranch)
if err := pushWithHookHint(opts.NoVerify, func() error { return git.RunCommand(pushArgs...) }); err != nil {
// Switch back to original branch before exiting
if switchErr := git.RunCommand("switch", "--quiet", originalBranch); switchErr != nil {
@@ -369,6 +378,15 @@ func runCI(prNumber string, opts *RunCIOptions) error {
return fatalErrorf("Failed to push CI branch: %w", err)
}
for _, setting := range [][2]string{{"remote", "origin"}, {"merge", "refs/heads/" + ciBranch}} {
if err := git.RunCommand("config", "branch."+ciBranch+"."+setting[0], setting[1]); err != nil {
if switchErr := git.RunCommand("switch", "--quiet", originalBranch); switchErr != nil {
log.Warnf("Failed to switch back to original branch: %v", switchErr)
}
return fatalErrorf("Failed to set CI branch upstream %s: %w", setting[0], err)
}
}
if existingPRURL != "" {
// PR already exists - force push is enough to re-trigger CI
log.Infof("Switching back to original branch: %s", originalBranch)
@@ -409,7 +427,7 @@ func runCI(prNumber string, opts *RunCIOptions) error {
// getPRInfo fetches PR information using the GitHub CLI
func getPRInfo(prNumber string) (*PRInfo, error) {
cmd := exec.Command("gh", "pr", "view", prNumber,
"--json", "number,title,body,headRefName,headRepository,headRepositoryOwner,baseRefName,isCrossRepository")
"--json", "number,title,body,headRefName,headRefOid,headRepository,headRepositoryOwner,baseRefName,isCrossRepository")
output, err := cmd.Output()
if err != nil {
if exitErr, ok := err.(*exec.ExitError); ok {
tools/ods/cmd/run-ci_test.go
+55 -2
View File
@@ -14,6 +14,7 @@ package cmd
// R6 gh, fork, or push failures -> errors, back on the original TestRunCI_failures
import (
"fmt"
"os"
"path/filepath"
"strings"
@@ -24,8 +25,8 @@ import (
const (
gitrelEditableBranch = "chore/feature-pr-7353-edits"
gitrelForkPRJSON = `{"number":7353,"title":"feat: from a fork","body":"PR body","headRefName":"feature","headRepository":{"name":"onyx"},"headRepositoryOwner":{"login":"alice"},"baseRefName":"main","isCrossRepository":true}`
gitrelPRViewArm = `"pr view 7353 --json number,title,body,headRefName,headRepository,headRepositoryOwner,baseRefName,isCrossRepository") echo '` + gitrelForkPRJSON + `' ;;`
gitrelForkPRJSON = `{"number":7353,"title":"feat: from a fork","body":"PR body","headRefName":"feature","headRefOid":"'"$GITREL_FORK_SHA"'","headRepository":{"name":"onyx"},"headRepositoryOwner":{"login":"alice"},"baseRefName":"main","isCrossRepository":true}`
gitrelPRViewArm = `"pr view 7353 --json number,title,body,headRefName,headRefOid,headRepository,headRepositoryOwner,baseRefName,isCrossRepository") echo '` + gitrelForkPRJSON + `' ;;`
gitrelNoCIPRArm = `"pr list --head run-ci/7353 --state open --limit 1 --json url") echo '[]' ;;`
gitrelCIPRArm = `"pr list --head run-ci/7353 --state open --limit 1 --json url") echo '[{"url":"https://github.com/onyx-dot-app/onyx/pull/8000"}]' ;;`
gitrelNoEditablePRArm = `"pr list --head ` + gitrelEditableBranch + ` --state all --limit 1 --json url") echo '[]' ;;`
@@ -60,6 +61,7 @@ func gitrelSetupRunCIRepo(t *testing.T) gitrelRunCIRepo {
// Refuse any transport but local paths, should the rewrite ever miss.
t.Setenv("GIT_ALLOW_PROTOCOL", "file")
t.Chdir(work)
t.Setenv("GITREL_FORK_SHA", forkSHA)
return gitrelRunCIRepo{Origin: origin, Work: work, ForkSHA: forkSHA}
}
@@ -78,6 +80,12 @@ func TestRunCI_createsBranchAndPR(t *testing.T) {
if tip := gittest.Git(t, repo.Origin, "rev-parse", "refs/heads/run-ci/7353"); tip != repo.ForkSHA {
t.Errorf("expected origin run-ci/7353 at the fork head %s, got %s", repo.ForkSHA, tip)
}
if remote := gittest.Git(t, repo.Work, "config", "branch.run-ci/7353.remote"); remote != "origin" {
t.Errorf("expected CI branch remote origin, got %q", remote)
}
if merge := gittest.Git(t, repo.Work, "config", "branch.run-ci/7353.merge"); merge != "refs/heads/run-ci/7353" {
t.Errorf("expected CI branch merge ref, got %q", merge)
}
creates := gitrelCallsWithPrefix(calls(), "pr", "create")
if len(creates) != 1 {
t.Fatalf("expected one gh pr create call, got %q", creates)
@@ -316,6 +324,15 @@ func TestRunCI_failures(t *testing.T) {
},
wantErr: "Failed to push CI branch: exit status 1",
},
{
name: "upstream config write fails",
ghArms: gitrelPRViewArm + "\n" + gitrelNoCIPRArm + "\n" + gitrelCIPRCreate,
setup: func(t *testing.T, repo gitrelRunCIRepo) {
// A held config lock makes the post-push tracking write fail.
gitrelHooks(t, repo.Work, map[string]string{"pre-push": "#!/bin/sh\ntouch \"$(git rev-parse --git-dir)/config.lock\"\n"})
},
wantErr: "Failed to set CI branch upstream remote: ",
},
{
name: "PR creation fails",
ghArms: gitrelPRViewArm + "\n" + gitrelNoCIPRArm + "\n" + `"pr create "*) echo 'base branch not found' >&2; exit 1 ;;`,
@@ -344,3 +361,39 @@ func TestRunCI_failures(t *testing.T) {
})
}
}
func TestRunCI_rejectsChangedForkHead(t *testing.T) {
for _, onCI := range []bool{false, true} {
t.Run(fmt.Sprintf("onCI=%v", onCI), func(t *testing.T) {
repo := gitrelSetupRunCIRepo(t)
if onCI {
gittest.Git(t, repo.Work, "checkout", "--quiet", "-b", "run-ci/7353")
}
before := gittest.Git(t, repo.Work, "rev-parse", "HEAD")
t.Setenv("GITREL_FORK_SHA", before)
gitrelFakeGH(t, gitrelPRViewArm+"\n"+gitrelNoCIPRArm+"\n"+gitrelCIPRCreate)
err := runCI("7353", &RunCIOptions{Yes: true})
if err == nil || !strings.Contains(err.Error(), "Fork branch moved") {
t.Fatalf("expected changed-head rejection, got %v", err)
}
if head := gittest.Git(t, repo.Work, "rev-parse", "HEAD"); head != before {
t.Fatalf("local head changed: %s", head)
}
if gitrelRefExists(repo.Origin, "refs/heads/run-ci/7353") {
t.Fatal("unconfirmed commit was pushed")
}
})
}
}
func TestRunCI_pushesConfirmedCommitAfterLocalBranchMoves(t *testing.T) {
repo := gitrelSetupRunCIRepo(t)
gitrelFakeGH(t, gitrelPRViewArm+"\n"+gitrelNoCIPRArm+"\n"+gitrelCIPRCreate)
gitrelHooks(t, repo.Work, map[string]string{"post-checkout": "#!/bin/sh\nif [ \"$(git branch --show-current)\" = run-ci/7353 ]; then git reset --hard main; fi\n"})
if err := runCI("7353", &RunCIOptions{Yes: true}); err != nil {
t.Fatal(err)
}
if tip := gittest.Git(t, repo.Origin, "rev-parse", "refs/heads/run-ci/7353"); tip != repo.ForkSHA {
t.Fatalf("pushed mutable branch instead of confirmed commit: got %s, want %s", tip, repo.ForkSHA)
}
}