[0.15] fix(submit): push should run from worktree (#726)

'git push' should be run from the worktree where the submit was run
instead of the .git directory of the repository
even if the push operation can be done without a worktree
as pre-push hooks may depend on the worktree state.

Resolves #725

Author: abhinav

.changes/v0.15.2.md

@@ -0,0 +1,3 @@
+## <a name="v0.15.2">v0.15.2</a> - 2025-07-07
+### Fixed
+- submit: Pre-push hooks should run in the same worktree as the submit command.

CHANGELOG.md

@@ -5,6 +5,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html),
 and is generated by [Changie](https://github.com/miniscruff/changie).
 
+## <a name="v0.15.2">v0.15.2</a> - 2025-07-07
+### Fixed
+- submit: Pre-push hooks should run in the same worktree as the submit command.
+
 ## <a name="v0.15.1">v0.15.1</a> - 2025-06-25
 ### Fixed
 - Fix several operations using the incorrect Git worktree when invoked from the non-primary worktree.

branch_submit.go

@@ -440,7 +440,7 @@ func (cmd *branchSubmitCmd) run(
 			}
 		}
 
-		err = repo.Push(ctx, pushOpts)
+		err = wt.Push(ctx, pushOpts)
 		if err != nil {
 			return fmt.Errorf("push branch: %w", err)
 		}
@@ -549,7 +549,7 @@ func (cmd *branchSubmitCmd) run(
 				}
 			}
 
-			if err := repo.Push(ctx, pushOpts); err != nil {
+			if err := wt.Push(ctx, pushOpts); err != nil {
 				log.Error("Push failed. Branch may have been updated by someone else. Try with --force.")
 				return fmt.Errorf("push branch: %w", err)
 			}

internal/forge/github/integration_test.go

@@ -293,15 +293,15 @@ func TestIntegration_Repository_SubmitEditChange(t *testing.T) {
 
 		t.Logf("Pushing to origin")
 		require.NoError(t,
-			gitRepo.Push(ctx, git.PushOptions{
+			gitWork.Push(ctx, git.PushOptions{
 				Remote:  "origin",
 				Refspec: git.Refspec(branchName),
 			}), "error pushing branch")
 
 		t.Cleanup(func() {
 			t.Logf("Deleting remote branch: %s", branchName)
 			assert.NoError(t,
-				gitRepo.Push(context.WithoutCancel(ctx), git.PushOptions{
+				gitWork.Push(context.WithoutCancel(ctx), git.PushOptions{
 					Remote:  "origin",
 					Refspec: git.Refspec(":" + branchName),
 				}), "error deleting branch")
@@ -333,7 +333,7 @@ func TestIntegration_Repository_SubmitEditChange(t *testing.T) {
 		t.Logf("Pushing new base: %s", newBase)
 		if *_update {
 			require.NoError(t,
-				gitRepo.Push(t.Context(), git.PushOptions{
+				gitWork.Push(t.Context(), git.PushOptions{
 					Remote:  "origin",
 					Refspec: git.Refspec("main:" + newBase),
 				}), "could not push base branch")
@@ -342,7 +342,7 @@ func TestIntegration_Repository_SubmitEditChange(t *testing.T) {
 				t.Logf("Deleting remote branch: %s", newBase)
 				ctx := context.WithoutCancel(t.Context())
 				require.NoError(t,
-					gitRepo.Push(ctx, git.PushOptions{
+					gitWork.Push(ctx, git.PushOptions{
 						Remote:  "origin",
 						Refspec: git.Refspec(":" + newBase),
 					}), "error deleting branch")
@@ -455,15 +455,15 @@ func TestIntegration_Repository_SubmitChange_baseBranchDoesNotExist(t *testing.T
 
 		t.Logf("Pushing to origin")
 		require.NoError(t,
-			gitRepo.Push(ctx, git.PushOptions{
+			gitWork.Push(ctx, git.PushOptions{
 				Remote:  "origin",
 				Refspec: git.Refspec(branchName),
 			}), "error pushing branch")
 
 		t.Cleanup(func() {
 			t.Logf("Deleting remote branch: %s", branchName)
 			assert.NoError(t,
-				gitRepo.Push(context.WithoutCancel(ctx), git.PushOptions{
+				gitWork.Push(context.WithoutCancel(ctx), git.PushOptions{
 					Remote:  "origin",
 					Refspec: git.Refspec(":" + branchName),
 				}), "error deleting branch")

internal/forge/gitlab/integration_test.go

@@ -305,7 +305,7 @@ func TestIntegration_Repository_SubmitEditChange(t *testing.T) {
 
 		t.Logf("Pushing to origin")
 		require.NoError(t,
-			gitRepo.Push(ctx, git.PushOptions{
+			gitWork.Push(ctx, git.PushOptions{
 				Remote:  "origin",
 				Refspec: git.Refspec(branchName),
 			}), "error pushing branch")
@@ -314,7 +314,7 @@ func TestIntegration_Repository_SubmitEditChange(t *testing.T) {
 			ctx := context.WithoutCancel(t.Context())
 			t.Logf("Deleting remote branch: %s", branchName)
 			assert.NoError(t,
-				gitRepo.Push(ctx, git.PushOptions{
+				gitWork.Push(ctx, git.PushOptions{
 					Remote:  "origin",
 					Refspec: git.Refspec(":" + branchName),
 				}), "error deleting branch")
@@ -347,7 +347,7 @@ func TestIntegration_Repository_SubmitEditChange(t *testing.T) {
 		t.Logf("Pushing new base: %s", newBase)
 		if *_update {
 			require.NoError(t,
-				gitRepo.Push(ctx, git.PushOptions{
+				gitWork.Push(ctx, git.PushOptions{
 					Remote:  "origin",
 					Refspec: git.Refspec("main:" + newBase),
 				}), "could not push base branch")
@@ -356,7 +356,7 @@ func TestIntegration_Repository_SubmitEditChange(t *testing.T) {
 				ctx := context.WithoutCancel(t.Context())
 				t.Logf("Deleting remote branch: %s", newBase)
 				require.NoError(t,
-					gitRepo.Push(ctx, git.PushOptions{
+					gitWork.Push(ctx, git.PushOptions{
 						Remote:  "origin",
 						Refspec: git.Refspec(":" + newBase),
 					}), "error deleting branch")

internal/git/push_test.go

@@ -69,11 +69,11 @@ func TestPushOptions_NoVerify(t *testing.T) {
 					return nil
 				})
 
-			repo := &Repository{
+			wt := &Worktree{
 				exec: mockExecer,
 			}
 
-			err := repo.Push(t.Context(), tt.opts)
+			err := wt.Push(t.Context(), tt.opts)
 			require.NoError(t, err)
 			assert.Equal(t, tt.wantCmd, gotCmd)
 		})

internal/git/push.go renamed to internal/git/push_wt.go

@@ -36,12 +36,12 @@ type PushOptions struct {
 }
 
 // Push pushes objects and refs to a remote repository.
-func (r *Repository) Push(ctx context.Context, opts PushOptions) error {
+func (w *Worktree) Push(ctx context.Context, opts PushOptions) error {
 	if opts.Remote == "" && opts.Refspec == "" {
 		return errors.New("push: no remote or refspec specified")
 	}
 
-	r.log.Debug("Pushing to remote",
+	w.log.Debug("Pushing to remote",
 		silog.NonZero("name", opts.Remote),
 		silog.NonZero("force", opts.Force),
 		silog.NonZero("lease", forceWithLease(opts.ForceWithLease)))
@@ -63,7 +63,7 @@ func (r *Repository) Push(ctx context.Context, opts PushOptions) error {
 		args = append(args, opts.Refspec.String())
 	}
 
-	if err := r.gitCmd(ctx, args...).Run(r.exec); err != nil {
+	if err := w.gitCmd(ctx, args...).Run(w.exec); err != nil {
 		return fmt.Errorf("push: %w", err)
 	}
 

testdata/script/issue725_pre_push_hook_worktree.txt

@@ -0,0 +1,53 @@
+# Test for issue #725: pre-push hooks should work correctly with worktrees
+# The hook checks for file existence in the worktree, not just the main checkout
+
+as 'Test User <[email protected]>'
+at 2025-01-15T10:00:00Z
+
+mkdir repo
+cd repo
+git init
+git add required.txt
+git commit -m 'initial commit'
+
+shamhub init
+shamhub register alice
+shamhub new origin alice/example.git
+git push origin main
+env SHAMHUB_USERNAME=alice
+gs auth login
+
+# Create a pre-push hook that checks for file existence in worktree
+cp $WORK/extra/pre-push .git/hooks/pre-push
+exec chmod +x .git/hooks/pre-push
+
+# Create a new branch and commit
+git add feature.txt
+gs branch create feature1 -m 'Add feature1'
+
+# Remove the required file to simulate the issue
+git rm required.txt
+gs commit create -m 'remove required file'
+
+# submit should fail because the required file is missing
+! gs branch submit --fill
+stderr 'required.txt not found in worktree'
+
+# Restore the required file and submit again
+git revert HEAD --no-edit
+gs branch submit --fill
+
+-- repo/required.txt --
+This file is required by the pre-push hook.
+-- repo/feature.txt --
+This is a feature file.
+-- extra/pre-push --
+#!/bin/bash
+
+# Pre-push hook that checks for file existence in worktree.
+# This hook will always fail if the required file is not found.
+
+[ -f "required.txt" ] || {
+    echo "Error: required.txt not found in worktree" >&2
+    exit 1
+}