From 01960d77e84ce8424f401ab7ec819acadee0c57c Mon Sep 17 00:00:00 2001 From: Luke Marsden Date: Mon, 2 Mar 2026 09:27:10 +0000 Subject: [PATCH 1/3] Detect and propagate force pushes to upstream When an agent force-pushes to their feature branch (e.g., after rebasing), detect this by checking if the old commit is an ancestor of the new commit. If not (force push), propagate the force flag to PushBranchToRemote so the upstream push uses --force. Changes: - detectChangedBranches now returns map[string]bool (branch -> isForce) - Added repoPath parameter for running git merge-base --is-ancestor - Log force push detection with old/new commit hashes - Pass isForce flag to PushBranchToRemote call This allows agents to recover from stuck states by force-pushing after rebasing onto the latest base branch. Spec-Ref: helix-specs@497a41c80:001289_force-pushing-on-a-spec --- api/pkg/services/git_http_server.go | 57 +++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/api/pkg/services/git_http_server.go b/api/pkg/services/git_http_server.go index 49ac2b8f27..236a20eb1a 100644 --- a/api/pkg/services/git_http_server.go +++ b/api/pkg/services/git_http_server.go @@ -579,8 +579,15 @@ func (s *GitHTTPServer) handleReceivePack(w http.ResponseWriter, r *http.Request } // Detect pushed branches by comparing before/after + // Returns map[branch]isForce where isForce=true means force push detected branchesAfter := s.getBranchHashes(repoPath) - pushedBranches := s.detectChangedBranches(branchesBefore, branchesAfter) + pushedBranchesMap := s.detectChangedBranches(repoPath, branchesBefore, branchesAfter) + + // Extract branch names for logging + pushedBranches := make([]string, 0, len(pushedBranchesMap)) + for branch := range pushedBranchesMap { + pushedBranches = append(pushedBranches, branch) + } log.Info().Str("repo_id", repoID).Strs("pushed_branches", pushedBranches).Msg("Receive-pack completed") @@ -594,24 +601,24 @@ func (s *GitHTTPServer) handleReceivePack(w http.ResponseWriter, r *http.Request // headers have already been sent, so we cannot signal upstream push failures to the // client - they will see success regardless. If upstream push fails, we rollback // locally but the agent won't know. This is a known architectural limitation. - if len(pushedBranches) > 0 && repo != nil && repo.ExternalURL != "" { - log.Debug().Str("repo_id", repoID).Int("branch_count", len(pushedBranches)).Msg("Starting external push with detached context") + if len(pushedBranchesMap) > 0 && repo != nil && repo.ExternalURL != "" { + log.Debug().Str("repo_id", repoID).Int("branch_count", len(pushedBranchesMap)).Msg("Starting external push with detached context") upstreamPushFailed := false - for _, branch := range pushedBranches { + for branch, isForce := range pushedBranchesMap { // Create per-branch timeout so later branches don't get starved branchCtx, branchCancel := context.WithTimeout(context.Background(), 90*time.Second) - log.Info().Str("repo_id", repoID).Str("branch", branch).Msg("Pushing branch to upstream") - err := s.gitRepoService.PushBranchToRemote(branchCtx, repoID, branch, false) + log.Info().Str("repo_id", repoID).Str("branch", branch).Bool("force", isForce).Msg("Pushing branch to upstream") + err := s.gitRepoService.PushBranchToRemote(branchCtx, repoID, branch, isForce) branchCancel() if err != nil { - log.Error().Err(err).Str("repo_id", repoID).Str("branch", branch).Msg("Failed to push branch to upstream - rolling back") + log.Error().Err(err).Str("repo_id", repoID).Str("branch", branch).Bool("force", isForce).Msg("Failed to push branch to upstream - rolling back") upstreamPushFailed = true break } - log.Info().Str("repo_id", repoID).Str("branch", branch).Msg("Successfully pushed branch to upstream") + log.Info().Str("repo_id", repoID).Str("branch", branch).Bool("force", isForce).Msg("Successfully pushed branch to upstream") } if upstreamPushFailed { @@ -670,15 +677,35 @@ func (s *GitHTTPServer) getBranchHashes(repoPath string) map[string]string { return result } -// detectChangedBranches compares before/after branch hashes to find changed branches -func (s *GitHTTPServer) detectChangedBranches(before, after map[string]string) []string { - var changed []string - for branch, hash := range after { - if beforeHash, exists := before[branch]; !exists || beforeHash != hash { - changed = append(changed, branch) +// detectChangedBranches compares before/after branch hashes to find changed branches. +// Returns a map of branch name -> isForce (true if force push detected). +// A force push is detected when the old commit is NOT an ancestor of the new commit. +func (s *GitHTTPServer) detectChangedBranches(repoPath string, before, after map[string]string) map[string]bool { + result := make(map[string]bool) + for branch, newHash := range after { + oldHash, existed := before[branch] + if !existed || oldHash != newHash { + isForce := false + if existed && oldHash != "" { + // Check if old commit is ancestor of new commit (fast-forward) + // If NOT ancestor, this is a force push + _, _, err := gitcmd.NewCommand("merge-base", "--is-ancestor"). + AddDynamicArguments(oldHash, newHash). + RunStdString(context.Background(), &gitcmd.RunOpts{Dir: repoPath}) + if err != nil { + // merge-base --is-ancestor returns non-zero if not ancestor + isForce = true + log.Info(). + Str("branch", branch). + Str("old_hash", oldHash). + Str("new_hash", newHash). + Msg("Force push detected: old commit is not ancestor of new commit") + } + } + result[branch] = isForce } } - return changed + return result } // rollbackBranchRefs restores branch refs to their previous state using native git From b2c78922d726f26febd42cc4a75ba2afc514288f Mon Sep 17 00:00:00 2001 From: Luke Marsden Date: Mon, 2 Mar 2026 11:04:51 +0000 Subject: [PATCH 2/3] Extract IsAncestor helper function for commit ancestry checks Reuse pattern from existing codebase - adds IsAncestor() to gitea_git_helpers.go and uses it in detectChangedBranches instead of inline git command. Spec-Ref: helix-specs@d70fa2e18:001289_force-pushing-on-a-spec --- api/pkg/services/git_http_server.go | 6 +----- api/pkg/services/gitea_git_helpers.go | 31 ++++++++++++++++++--------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/api/pkg/services/git_http_server.go b/api/pkg/services/git_http_server.go index 236a20eb1a..4aedeb3e98 100644 --- a/api/pkg/services/git_http_server.go +++ b/api/pkg/services/git_http_server.go @@ -689,11 +689,7 @@ func (s *GitHTTPServer) detectChangedBranches(repoPath string, before, after map if existed && oldHash != "" { // Check if old commit is ancestor of new commit (fast-forward) // If NOT ancestor, this is a force push - _, _, err := gitcmd.NewCommand("merge-base", "--is-ancestor"). - AddDynamicArguments(oldHash, newHash). - RunStdString(context.Background(), &gitcmd.RunOpts{Dir: repoPath}) - if err != nil { - // merge-base --is-ancestor returns non-zero if not ancestor + if !IsAncestor(context.Background(), repoPath, oldHash, newHash) { isForce = true log.Info(). Str("branch", branch). diff --git a/api/pkg/services/gitea_git_helpers.go b/api/pkg/services/gitea_git_helpers.go index 5c410fa577..3a01b91601 100644 --- a/api/pkg/services/gitea_git_helpers.go +++ b/api/pkg/services/gitea_git_helpers.go @@ -27,8 +27,8 @@ func IsErrEmptyRepository(err error) bool { // Note: Git error messages can vary by version and locale. These patterns // cover common English git outputs from versions 2.x+. var gitErrorPatterns = struct { - AlreadyUpToDate []string // Not really an error, just informational - EmptyRepository []string // Remote has no refs (empty repo) + AlreadyUpToDate []string // Not really an error, just informational + EmptyRepository []string // Remote has no refs (empty repo) RemoteAlreadyExists []string }{ AlreadyUpToDate: []string{ @@ -57,14 +57,14 @@ func matchesAnyPattern(text string, patterns []string) bool { // FetchOptions contains options for git fetch operations type FetchOptions struct { - Remote string // Remote name (e.g., "origin") - Branch string // Specific branch to fetch (empty for all) - Force bool // Force fetch (overwrite local refs) - Prune bool // Remove remote-tracking refs that no longer exist - Depth int // Shallow fetch with depth limit (0 for full) - Env []string // Environment variables (for auth) - Timeout time.Duration // Command timeout - RefSpecs []string // Explicit refspecs (optional) + Remote string // Remote name (e.g., "origin") + Branch string // Specific branch to fetch (empty for all) + Force bool // Force fetch (overwrite local refs) + Prune bool // Remove remote-tracking refs that no longer exist + Depth int // Shallow fetch with depth limit (0 for full) + Env []string // Environment variables (for auth) + Timeout time.Duration // Command timeout + RefSpecs []string // Explicit refspecs (optional) } // Fetch fetches from a remote repository using native git @@ -365,6 +365,17 @@ func ShortHash(hash string) string { return hash[:8] } +// IsAncestor checks if ancestorCommit is an ancestor of descendantCommit. +// Returns true if ancestorCommit is reachable from descendantCommit's history. +// This is useful for detecting force pushes (old commit not ancestor of new = force push). +func IsAncestor(ctx context.Context, repoPath, ancestorCommit, descendantCommit string) bool { + _, _, err := gitcmd.NewCommand("merge-base", "--is-ancestor"). + AddDynamicArguments(ancestorCommit, descendantCommit). + RunStdString(ctx, &gitcmd.RunOpts{Dir: repoPath}) + // merge-base --is-ancestor returns 0 if ancestor, non-zero otherwise + return err == nil +} + // PreReceiveHookVersion is incremented when the hook logic changes. // The hook script contains this version and will be updated if it differs. const PreReceiveHookVersion = "4" From 42d07ec957f335342a436a629d093fead23187a8 Mon Sep 17 00:00:00 2001 From: Luke Marsden Date: Mon, 2 Mar 2026 11:05:40 +0000 Subject: [PATCH 3/3] Revert "Extract IsAncestor helper function for commit ancestry checks" This reverts commit b2c78922d726f26febd42cc4a75ba2afc514288f. --- api/pkg/services/git_http_server.go | 6 +++++- api/pkg/services/gitea_git_helpers.go | 31 +++++++++------------------ 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/api/pkg/services/git_http_server.go b/api/pkg/services/git_http_server.go index 4aedeb3e98..236a20eb1a 100644 --- a/api/pkg/services/git_http_server.go +++ b/api/pkg/services/git_http_server.go @@ -689,7 +689,11 @@ func (s *GitHTTPServer) detectChangedBranches(repoPath string, before, after map if existed && oldHash != "" { // Check if old commit is ancestor of new commit (fast-forward) // If NOT ancestor, this is a force push - if !IsAncestor(context.Background(), repoPath, oldHash, newHash) { + _, _, err := gitcmd.NewCommand("merge-base", "--is-ancestor"). + AddDynamicArguments(oldHash, newHash). + RunStdString(context.Background(), &gitcmd.RunOpts{Dir: repoPath}) + if err != nil { + // merge-base --is-ancestor returns non-zero if not ancestor isForce = true log.Info(). Str("branch", branch). diff --git a/api/pkg/services/gitea_git_helpers.go b/api/pkg/services/gitea_git_helpers.go index 3a01b91601..5c410fa577 100644 --- a/api/pkg/services/gitea_git_helpers.go +++ b/api/pkg/services/gitea_git_helpers.go @@ -27,8 +27,8 @@ func IsErrEmptyRepository(err error) bool { // Note: Git error messages can vary by version and locale. These patterns // cover common English git outputs from versions 2.x+. var gitErrorPatterns = struct { - AlreadyUpToDate []string // Not really an error, just informational - EmptyRepository []string // Remote has no refs (empty repo) + AlreadyUpToDate []string // Not really an error, just informational + EmptyRepository []string // Remote has no refs (empty repo) RemoteAlreadyExists []string }{ AlreadyUpToDate: []string{ @@ -57,14 +57,14 @@ func matchesAnyPattern(text string, patterns []string) bool { // FetchOptions contains options for git fetch operations type FetchOptions struct { - Remote string // Remote name (e.g., "origin") - Branch string // Specific branch to fetch (empty for all) - Force bool // Force fetch (overwrite local refs) - Prune bool // Remove remote-tracking refs that no longer exist - Depth int // Shallow fetch with depth limit (0 for full) - Env []string // Environment variables (for auth) - Timeout time.Duration // Command timeout - RefSpecs []string // Explicit refspecs (optional) + Remote string // Remote name (e.g., "origin") + Branch string // Specific branch to fetch (empty for all) + Force bool // Force fetch (overwrite local refs) + Prune bool // Remove remote-tracking refs that no longer exist + Depth int // Shallow fetch with depth limit (0 for full) + Env []string // Environment variables (for auth) + Timeout time.Duration // Command timeout + RefSpecs []string // Explicit refspecs (optional) } // Fetch fetches from a remote repository using native git @@ -365,17 +365,6 @@ func ShortHash(hash string) string { return hash[:8] } -// IsAncestor checks if ancestorCommit is an ancestor of descendantCommit. -// Returns true if ancestorCommit is reachable from descendantCommit's history. -// This is useful for detecting force pushes (old commit not ancestor of new = force push). -func IsAncestor(ctx context.Context, repoPath, ancestorCommit, descendantCommit string) bool { - _, _, err := gitcmd.NewCommand("merge-base", "--is-ancestor"). - AddDynamicArguments(ancestorCommit, descendantCommit). - RunStdString(ctx, &gitcmd.RunOpts{Dir: repoPath}) - // merge-base --is-ancestor returns 0 if ancestor, non-zero otherwise - return err == nil -} - // PreReceiveHookVersion is incremented when the hook logic changes. // The hook script contains this version and will be updated if it differs. const PreReceiveHookVersion = "4"