feat(agentctl): push to a caller-named remote and branch, not just origin (#3558)

Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
Henry
2026-09-13 06:01:44 +08:00
committed by GitHub
parent 2dc1596667
commit df3c9142ab
26 changed files with 3883 additions and 146 deletions
@@ -184,6 +184,10 @@ type GitPushRequest struct {
Force bool `json:"force"`
SetUpstream bool `json:"set_upstream"`
Repo string `json:"repo,omitempty"`
// Remote and ExpectedBranch are optional and forwarded uninterpreted; the
// workspace git operator owns every destination decision.
Remote string `json:"remote,omitempty"`
ExpectedBranch string `json:"expected_branch,omitempty"`
}
// GitContributionRequest carries the provider-head lease for a managed
@@ -331,13 +335,18 @@ func (h *GitHandlers) wsPush(ctx context.Context, msg *ws.Message) (*ws.Message,
return nil, fmt.Errorf("session_id is required")
}
client, releaseClient, err := h.getAgentCtlClient(ctx, req.SessionID)
agentClient, releaseClient, err := h.getAgentCtlClient(ctx, req.SessionID)
defer releaseClient()
if err != nil {
return nil, err
}
result, err := client.GitPush(ctx, req.Force, req.SetUpstream, req.Repo)
result, err := agentClient.GitPush(ctx, req.Repo, client.PushOptions{
Force: req.Force,
SetUpstream: req.SetUpstream,
Remote: req.Remote,
ExpectedBranch: req.ExpectedBranch,
})
if err != nil {
return nil, fmt.Errorf("push failed: %w", err)
}
@@ -121,6 +121,7 @@ func TestGitMutationHandlersForwardRequestsAndReturnResults(t *testing.T) {
}{
{name: "pull", path: "/api/v1/git/pull", request: GitPullRequest{SessionID: "s", Rebase: true, Repo: "repo"}, invoke: (*GitHandlers).wsPull, assertBody: expectGitBody("rebase", true)},
{name: "push", path: "/api/v1/git/push", request: GitPushRequest{SessionID: "s", Force: true, SetUpstream: true, Repo: "repo"}, invoke: (*GitHandlers).wsPush, assertBody: expectGitBody("force", true)},
{name: "push with explicit target", path: "/api/v1/git/push", request: GitPushRequest{SessionID: "s", Repo: "repo", Remote: "backup", ExpectedBranch: "feature/work"}, invoke: (*GitHandlers).wsPush, assertBody: expectGitBody("remote", "backup")},
{name: "replace contribution", path: "/api/v1/git/contribution/replace", request: GitContributionRequest{SessionID: "s", ExpectedRemoteHead: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Repo: "repo"}, invoke: (*GitHandlers).wsReplaceContribution, assertBody: expectGitBody("expected_remote_head", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")},
{name: "use contribution", path: "/api/v1/git/contribution/use", request: GitContributionRequest{SessionID: "s", ExpectedRemoteHead: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", Repo: "repo"}, invoke: (*GitHandlers).wsUseContribution, assertBody: expectGitBody("expected_remote_head", "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")},
{name: "commit", path: "/api/v1/git/commit", request: GitCommitRequest{SessionID: "s", Message: "message", StageAll: true, Repo: "repo"}, invoke: (*GitHandlers).wsCommit, assertBody: expectGitBody("message", "message")},
@@ -0,0 +1,52 @@
package handlers
import (
"context"
"encoding/json"
"net/http"
"strings"
"testing"
ws "github.com/kandev/kandev/pkg/websocket"
)
// TestWsPushForwardsPushOptions proves the ws payload's two optional inputs
// reach the agentctl request body together, rather than one being dropped.
func TestWsPushForwardsPushOptions(t *testing.T) {
var body map[string]any
h, server := gitHandlerServer(t, func(w http.ResponseWriter, r *http.Request) {
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
t.Errorf("decode request: %v", err)
}
_, _ = w.Write([]byte(`{"success":true,"operation":"push","output":"done","pushed_remote":"backup","pushed_branch":"feature/work"}`))
})
defer server.Close()
msg, err := ws.NewRequest("id", "action", GitPushRequest{
SessionID: "s",
Repo: "repo",
Remote: "backup",
ExpectedBranch: "feature/work",
})
if err != nil {
t.Fatal(err)
}
response, err := h.wsPush(context.Background(), msg)
if err != nil {
t.Fatalf("wsPush: %v", err)
}
if body["remote"] != "backup" {
t.Errorf("body[remote] = %#v, want backup", body["remote"])
}
if body["expected_branch"] != "feature/work" {
t.Errorf("body[expected_branch] = %#v, want feature/work", body["expected_branch"])
}
// The destination fields must survive the round trip back to the caller.
payload := string(response.Payload)
if !strings.Contains(payload, `"pushed_remote":"backup"`) {
t.Errorf("response payload lost pushed_remote: %s", payload)
}
if !strings.Contains(payload, `"pushed_branch":"feature/work"`) {
t.Errorf("response payload lost pushed_branch: %s", payload)
}
}
@@ -22,6 +22,26 @@ type GitOperationResult struct {
PreflightReason string `json:"preflight_reason,omitempty"`
ConflictFiles []string `json:"conflict_files,omitempty"`
RecoveryBranch string `json:"recovery_branch,omitempty"`
// Destination a push or preflight validated. A push populates these only
// when the request carried an explicit push target.
PushedRemote string `json:"pushed_remote,omitempty"`
PushedBranch string `json:"pushed_branch,omitempty"`
// Branches accompanying a mismatch refusal. CurrentBranch is empty for a
// detached HEAD.
ExpectedBranch string `json:"expected_branch,omitempty"`
CurrentBranch string `json:"current_branch,omitempty"`
// BaselinePublished marks a mismatch refusal after empty-remote first
// publication already published the baseline in this request.
BaselinePublished bool `json:"baseline_published,omitempty"`
}
// PushOptions carries the optional inputs of a push or push-preflight call.
// The zero value reproduces the behavior of a request that names neither.
type PushOptions struct {
Force bool
SetUpstream bool
Remote string
ExpectedBranch string
}
// PRCreateResult represents the result of a PR creation operation.
@@ -51,28 +71,40 @@ func (c *Client) GitPull(ctx context.Context, rebase bool, repo string) (*GitOpe
}
// GitPush performs a git push operation on the worktree.
// If force is true, uses --force-with-lease.
// If setUpstream is true, uses --set-upstream.
// If opts.Force is true, uses --force-with-lease.
// If opts.SetUpstream is true, uses --set-upstream.
// opts.Remote and opts.ExpectedBranch are optional and passed through.
// repo is the multi-repo subpath (e.g. "kandev"); empty for single-repo workspaces.
func (c *Client) GitPush(ctx context.Context, force, setUpstream bool, repo string) (*GitOperationResult, error) {
func (c *Client) GitPush(ctx context.Context, repo string, opts PushOptions) (*GitOperationResult, error) {
payload := struct {
Force bool `json:"force"`
SetUpstream bool `json:"set_upstream"`
Repo string `json:"repo,omitempty"`
Force bool `json:"force"`
SetUpstream bool `json:"set_upstream"`
Repo string `json:"repo,omitempty"`
Remote string `json:"remote,omitempty"`
ExpectedBranch string `json:"expected_branch,omitempty"`
}{
Force: force,
SetUpstream: setUpstream,
Repo: repo,
Force: opts.Force,
SetUpstream: opts.SetUpstream,
Repo: repo,
Remote: opts.Remote,
ExpectedBranch: opts.ExpectedBranch,
}
return c.gitOperation(ctx, "/api/v1/git/push", payload)
}
// GitPushPreflight validates the configured contribution push target without
// mutating the remote. repo is the multi-repo workspace subpath.
func (c *Client) GitPushPreflight(ctx context.Context, repo string) (*GitOperationResult, error) {
// GitPushPreflight validates the push destination without mutating the remote.
// repo is the multi-repo workspace subpath; opts carries the optional explicit
// push target and expected branch.
func (c *Client) GitPushPreflight(ctx context.Context, repo string, opts PushOptions) (*GitOperationResult, error) {
payload := struct {
Repo string `json:"repo,omitempty"`
}{Repo: repo}
Repo string `json:"repo,omitempty"`
Remote string `json:"remote,omitempty"`
ExpectedBranch string `json:"expected_branch,omitempty"`
}{
Repo: repo,
Remote: opts.Remote,
ExpectedBranch: opts.ExpectedBranch,
}
return c.gitOperation(ctx, "/api/v1/git/push-preflight", payload)
}
@@ -11,6 +11,27 @@ import (
// gitOperationOK is the canonical success body every /api/v1/git/* POST returns.
const gitOperationOK = `{"success":true,"operation":"pull","output":"Already up to date."}`
func TestGitPushSendsPushOptions(t *testing.T) {
srv, got := captureServer(t, jsonResponder(http.StatusOK, gitOperationOK))
_, err := newHTTPOnlyClient(srv.URL).GitPush(context.Background(), "svc", PushOptions{
Remote: "backup", ExpectedBranch: "feature/work",
})
if err != nil {
t.Fatalf("GitPush() error = %v", err)
}
var body map[string]any
if err := json.Unmarshal(got.Body, &body); err != nil {
t.Fatalf("decode request body: %v", err)
}
for key, want := range map[string]any{
"repo": "svc", "remote": "backup", "expected_branch": "feature/work",
} {
if gotValue := body[key]; gotValue != want {
t.Errorf("body[%q] = %#v, want %#v", key, gotValue, want)
}
}
}
// TestGitOperations_PostExpectedPathAndPayload pins the endpoint and JSON body
// of every thin wrapper over gitOperation. A wrapper that posts to the wrong
// path or drops a field is the whole failure mode for this layer.
@@ -36,7 +57,7 @@ func TestGitOperations_PostExpectedPathAndPayload(t *testing.T) {
{
name: "push force with upstream",
call: func(c *Client) (*GitOperationResult, error) {
return c.GitPush(context.Background(), true, true, "svc")
return c.GitPush(context.Background(), "svc", PushOptions{Force: true, SetUpstream: true})
},
wantPath: "/api/v1/git/push",
wantBody: map[string]any{"force": true, "set_upstream": true, "repo": "svc"},
@@ -44,17 +65,44 @@ func TestGitOperations_PostExpectedPathAndPayload(t *testing.T) {
{
name: "push plain",
call: func(c *Client) (*GitOperationResult, error) {
return c.GitPush(context.Background(), false, false, "")
return c.GitPush(context.Background(), "", PushOptions{})
},
wantPath: "/api/v1/git/push",
wantBody: map[string]any{"force": false, "set_upstream": false},
},
{
name: "push preflight",
call: func(c *Client) (*GitOperationResult, error) { return c.GitPushPreflight(context.Background(), "svc") },
name: "push preflight",
call: func(c *Client) (*GitOperationResult, error) {
return c.GitPushPreflight(context.Background(), "svc", PushOptions{})
},
wantPath: "/api/v1/git/push-preflight",
wantBody: map[string]any{"repo": "svc"},
},
{
name: "push with explicit target and expected branch",
call: func(c *Client) (*GitOperationResult, error) {
return c.GitPush(context.Background(), "svc", PushOptions{
Remote: "backup", ExpectedBranch: "feature/work",
})
},
wantPath: "/api/v1/git/push",
wantBody: map[string]any{
"force": false, "set_upstream": false, "repo": "svc",
"remote": "backup", "expected_branch": "feature/work",
},
},
{
name: "push preflight with explicit target and expected branch",
call: func(c *Client) (*GitOperationResult, error) {
return c.GitPushPreflight(context.Background(), "svc", PushOptions{
Remote: "backup", ExpectedBranch: "feature/work",
})
},
wantPath: "/api/v1/git/push-preflight",
wantBody: map[string]any{
"repo": "svc", "remote": "backup", "expected_branch": "feature/work",
},
},
{
name: "replace remote contribution",
call: func(c *Client) (*GitOperationResult, error) {
@@ -331,7 +379,7 @@ func TestGitOperation_HonoursContextCancellation(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
cancel()
_, err := newHTTPOnlyClient(srv.URL).GitPush(ctx, false, false, "")
_, err := newHTTPOnlyClient(srv.URL).GitPush(ctx, "", PushOptions{})
if err == nil || !strings.Contains(err.Error(), "context canceled") {
t.Fatalf("error = %v, want context canceled", err)
}
@@ -242,7 +242,7 @@ func (m *Manager) preflightRemoteContributionPushes(ctx context.Context, executi
}
sort.Strings(keys)
for _, key := range keys {
result, err := client.GitPushPreflight(ctx, key)
result, err := client.GitPushPreflight(ctx, key, agentctl.PushOptions{})
if err != nil {
return &BootstrapFailure{
Operation: bootstrapOperation(execution),
@@ -53,15 +53,24 @@ type GitPullRequest struct {
}
// GitPushRequest for POST /api/v1/git/push
//
// Remote and ExpectedBranch are optional. Remote is a configured remote name or
// a remote URL that must resolve to one; ExpectedBranch is the branch the
// caller believes it is publishing. Both are passed through untouched: this
// surface performs no destination logic of its own.
type GitPushRequest struct {
Force bool `json:"force"`
SetUpstream bool `json:"set_upstream"`
Repo string `json:"repo,omitempty"`
Force bool `json:"force"`
SetUpstream bool `json:"set_upstream"`
Repo string `json:"repo,omitempty"`
Remote string `json:"remote,omitempty"`
ExpectedBranch string `json:"expected_branch,omitempty"`
}
// GitPushPreflightRequest for POST /api/v1/git/push-preflight.
type GitPushPreflightRequest struct {
Repo string `json:"repo,omitempty"`
Repo string `json:"repo,omitempty"`
Remote string `json:"remote,omitempty"`
ExpectedBranch string `json:"expected_branch,omitempty"`
}
// GitContributionRequest is shared by the managed contribution replacement
@@ -220,7 +229,12 @@ func (s *Server) handleGitPush(c *gin.Context) {
if gitOp == nil {
return
}
result, err := gitOp.Push(c.Request.Context(), req.Force, req.SetUpstream)
result, err := gitOp.Push(c.Request.Context(), process.PushOptions{
Force: req.Force,
SetUpstream: req.SetUpstream,
Remote: req.Remote,
ExpectedBranch: req.ExpectedBranch,
})
if err != nil {
s.handleGitError(c, "push", err)
return
@@ -241,7 +255,10 @@ func (s *Server) handleGitPushPreflight(c *gin.Context) {
if gitOp == nil {
return
}
result, err := gitOp.PushPreflight(c.Request.Context())
result, err := gitOp.PushPreflight(c.Request.Context(), process.PushOptions{
Remote: req.Remote,
ExpectedBranch: req.ExpectedBranch,
})
if err != nil {
s.handleGitError(c, "push preflight", err)
return
@@ -0,0 +1,139 @@
package api
import (
"net/http"
"os"
"path/filepath"
"strings"
"testing"
"github.com/kandev/kandev/internal/agentctl/server/config"
"github.com/kandev/kandev/internal/agentctl/server/process"
"github.com/kandev/kandev/internal/common/logger"
)
// TestHandleGitPushBindsRemoteAndExpectedBranch proves the HTTP surface carries
// both optional inputs into the operator rather than dropping them.
func TestHandleGitPushBindsRemoteAndExpectedBranch(t *testing.T) {
fixture := newGitAPIFixture(t)
backup := filepath.Join(t.TempDir(), "backup.git")
runGitAPI(t, fixture.repo, "init", "--bare", "--initial-branch=main", backup)
runGitAPI(t, fixture.repo, "remote", "add", "backup", backup)
rec := postGitAPI(t, fixture.server, "/api/v1/git/push", GitPushRequest{
Remote: "backup",
ExpectedBranch: "feature/work",
})
if rec.Code != http.StatusOK {
t.Fatalf("status = %d (body %s)", rec.Code, rec.Body.String())
}
result := decodeGitOperationResult(t, rec)
if !result.Success {
t.Fatalf("push failed: %+v", result)
}
if result.PushedRemote != "backup" || result.PushedBranch != "feature/work" {
t.Errorf("destination = (%q, %q), want (backup, feature/work)", result.PushedRemote, result.PushedBranch)
}
head := fixture.head(t)
if got := strings.TrimSpace(runGitAPI(t, fixture.repo, "--git-dir="+backup, "rev-parse", "refs/heads/feature/work")); got != head {
t.Errorf("backup feature/work = %q, want %q", got, head)
}
// origin must be untouched by a push that named another destination.
runGitAPIExpectFailure(t, fixture.repo, "--git-dir="+fixture.bare, "rev-parse", "--verify", "refs/heads/feature/work")
}
// TestHandleGitPushRejectsMismatchedExpectedBranch proves the expected branch is
// enforced through the HTTP surface, not merely accepted.
func TestHandleGitPushRejectsMismatchedExpectedBranch(t *testing.T) {
fixture := newGitAPIFixture(t)
rec := postGitAPI(t, fixture.server, "/api/v1/git/push", GitPushRequest{ExpectedBranch: "feature/other"})
if rec.Code != http.StatusOK {
t.Fatalf("status = %d (body %s)", rec.Code, rec.Body.String())
}
result := decodeGitOperationResult(t, rec)
if result.Success {
t.Fatalf("push succeeded on a mismatch: %+v", result)
}
if result.ErrorCode != "push_branch_mismatch" {
t.Errorf("error_code = %q, want push_branch_mismatch", result.ErrorCode)
}
if result.ExpectedBranch != "feature/other" || result.CurrentBranch != "feature/work" {
t.Errorf("branches = (%q, %q), want (feature/other, feature/work)", result.ExpectedBranch, result.CurrentBranch)
}
}
// TestHandleGitPushPreflightBindsRemoteAndExpectedBranch covers the preflight
// endpoint's binding and its reporting of the destination it validated.
func TestHandleGitPushPreflightBindsRemoteAndExpectedBranch(t *testing.T) {
fixture := newGitAPIFixture(t)
backup := filepath.Join(t.TempDir(), "backup.git")
runGitAPI(t, fixture.repo, "init", "--bare", "--initial-branch=main", backup)
runGitAPI(t, fixture.repo, "remote", "add", "backup", backup)
rec := postGitAPI(t, fixture.server, "/api/v1/git/push-preflight", GitPushPreflightRequest{
Remote: "backup",
ExpectedBranch: "feature/work",
})
if rec.Code != http.StatusOK {
t.Fatalf("status = %d (body %s)", rec.Code, rec.Body.String())
}
result := decodeGitOperationResult(t, rec)
if !result.Success {
t.Fatalf("preflight failed: %+v", result)
}
if result.PushedRemote != "backup" || result.PushedBranch != "feature/work" {
t.Errorf("validated (%q, %q), want (backup, feature/work)", result.PushedRemote, result.PushedBranch)
}
runGitAPIExpectFailure(t, fixture.repo, "--git-dir="+backup, "rev-parse", "--verify", "refs/heads/feature/work")
}
// TestGitPushRequestScopesTargetToSelectedRepo proves both optional inputs act
// on the repository the request selects and leave every sibling untouched.
func TestGitPushRequestScopesTargetToSelectedRepo(t *testing.T) {
root := t.TempDir()
backups := map[string]string{}
for _, name := range []string{"alpha", "beta"} {
repo := filepath.Join(root, name)
if err := os.Mkdir(repo, 0o755); err != nil {
t.Fatalf("mkdir %s: %v", name, err)
}
backup := filepath.Join(root, name+"-backup.git")
runGitAPI(t, root, "init", "--bare", "--initial-branch=main", backup)
backups[name] = backup
runGitAPI(t, repo, "init", "--initial-branch=main")
runGitAPI(t, repo, "config", "user.email", "test@test.com")
runGitAPI(t, repo, "config", "user.name", "Test User")
writeFileAPI(t, repo, "README.md", name+"\n")
runGitAPI(t, repo, "add", ".")
runGitAPI(t, repo, "commit", "-m", "initial")
runGitAPI(t, repo, "remote", "add", "backup", backup)
runGitAPI(t, repo, "checkout", "-b", "feature/work")
writeFileAPI(t, repo, "one.txt", "one\n")
runGitAPI(t, repo, "add", ".")
runGitAPI(t, repo, "commit", "-m", "work")
}
log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error"})
cfg := &config.InstanceConfig{WorkDir: root}
server := NewServer(cfg, process.NewManager(cfg, log), nil, nil, log)
rec := postGitAPI(t, server, "/api/v1/git/push", GitPushRequest{
Repo: "alpha",
Remote: "backup",
ExpectedBranch: "feature/work",
})
if rec.Code != http.StatusOK {
t.Fatalf("status = %d (body %s)", rec.Code, rec.Body.String())
}
if result := decodeGitOperationResult(t, rec); !result.Success {
t.Fatalf("push failed: %+v", result)
}
alphaHead := strings.TrimSpace(runGitAPI(t, filepath.Join(root, "alpha"), "rev-parse", "HEAD"))
if got := strings.TrimSpace(runGitAPI(t, root, "--git-dir="+backups["alpha"], "rev-parse", "refs/heads/feature/work")); got != alphaHead {
t.Errorf("alpha backup = %q, want %q", got, alphaHead)
}
runGitAPIExpectFailure(t, root, "--git-dir="+backups["beta"], "rev-parse", "--verify", "refs/heads/feature/work")
}
@@ -28,6 +28,12 @@ var ErrInvalidBranchName = errors.New("invalid branch name")
const preflightReasonHistoryUpdateRequired = "history_update_required"
var contributionPreflightEnvironment = map[string]string{
"GIT_TERMINAL_PROMPT": "0",
"LANG": "C",
"LC_ALL": "C",
}
// GitOperationResult represents the result of a git operation.
type GitOperationResult struct {
Success bool `json:"success"`
@@ -38,6 +44,18 @@ type GitOperationResult struct {
PreflightReason string `json:"preflight_reason,omitempty"`
ConflictFiles []string `json:"conflict_files,omitempty"`
RecoveryBranch string `json:"recovery_branch,omitempty"`
// PushedRemote and PushedBranch name the destination a push or preflight
// validated. A push reports them only when the request carried an explicit
// push target, so a request that named none keeps its existing shape.
PushedRemote string `json:"pushed_remote,omitempty"`
PushedBranch string `json:"pushed_branch,omitempty"`
// ExpectedBranch and CurrentBranch accompany a branch-mismatch refusal.
// CurrentBranch is empty for a detached HEAD.
ExpectedBranch string `json:"expected_branch,omitempty"`
CurrentBranch string `json:"current_branch,omitempty"`
// BaselinePublished marks a mismatch refused after empty-remote first
// publication already published the baseline in this request.
BaselinePublished bool `json:"baseline_published,omitempty"`
}
// GitOperator executes git operations in a workspace directory.
@@ -458,7 +476,8 @@ func (g *GitOperator) Pull(ctx context.Context, rebase bool) (*GitOperationResul
}
// Push performs a git push operation.
func (g *GitOperator) Push(ctx context.Context, force bool, setUpstream bool) (*GitOperationResult, error) {
func (g *GitOperator) Push(ctx context.Context, opts PushOptions) (*GitOperationResult, error) {
opts = opts.normalized()
if !g.tryLock("push") {
return nil, ErrOperationInProgress
}
@@ -467,36 +486,21 @@ func (g *GitOperator) Push(ctx context.Context, force bool, setUpstream bool) (*
result := &GitOperationResult{
Operation: "push",
}
if g.remoteContributionErr != nil {
result.Error = g.remoteContributionErr.Error()
return result, nil
}
if g.contributionDestinationErr != nil {
result.Error = g.contributionDestinationErr.Error()
return result, nil
}
if g.contributionDestination != nil {
if err := g.validateContributionDestinationRemote(ctx); err != nil {
result.Error = err.Error()
return result, nil
}
}
if (g.remoteContribution != nil || g.contributionDestination != nil) && force {
result.Error = "force push is not allowed for a remote contribution"
return result, nil
}
if err := g.validateContributionRemote(ctx); err != nil {
result.Error = err.Error()
if refusal := g.validateContributionState(ctx, opts.Force); refusal != nil {
refusal.apply(result)
return result, nil
}
branch, err := g.getCurrentBranch(ctx)
if err != nil {
result.Error = err.Error()
// Every refusal below is raised before any remote is contacted, so a
// refused request is a no-op.
plan, refusal := g.resolvePushPlan(ctx, opts, false)
if refusal != nil {
refusal.apply(result)
return result, nil
}
basePublication := emptyRemotePublication{}
if g.remoteContribution == nil && g.contributionDestination == nil {
if plan.baselineEligible {
basePublication = g.prepareEmptyRemotePublication(ctx, "")
if basePublication.err != nil {
result.Error = basePublication.err.Error()
@@ -506,29 +510,24 @@ func (g *GitOperator) Push(ctx context.Context, force bool, setUpstream bool) (*
}
}
args := []string{"push"}
shouldSetUpstream := setUpstream || g.getUpstreamRef(ctx) == ""
remote := "origin"
refspec := branch
if g.contributionDestination != nil {
remote = g.contributionDestination.ContributionRemoteName()
refspec = branch
shouldSetUpstream = setUpstream
} else if g.remoteContribution != nil {
remote = g.remoteContribution.ContributionRemoteName()
refspec = "HEAD:refs/heads/" + g.remoteContribution.HeadBranch
shouldSetUpstream = setUpstream
// Every remaining read happens before the second verification, so that the
// branch read is the last git command before the push.
shouldSetUpstream := g.resolveSetUpstream(ctx, opts, plan)
if refusal := g.verifyExpectedBranch(ctx, opts.ExpectedBranch, basePublication.published); refusal != nil {
refusal.apply(result)
result.Output = basePublication.output
return result, nil
}
args := []string{"push"}
if shouldSetUpstream {
args = append(args, "--set-upstream")
}
if force {
if opts.Force {
// Use --force-with-lease for safer force push
args = append(args, "--force-with-lease")
}
args = append(args, remote, refspec)
args = append(args, plan.remote, plan.refspec)
output, err := g.runGitCommand(ctx, args...)
result.Output = output
@@ -544,107 +543,148 @@ func (g *GitOperator) Push(ctx context.Context, force bool, setUpstream bool) (*
result.Output = combineGitOutputs(basePublication.output, output)
result.Success = true
plan.reportDestination(result)
g.logger.Info("push completed",
zap.String("branch", branch),
zap.String("remote", remote),
zap.Bool("force", force),
zap.Bool("set_upstream", shouldSetUpstream))
zap.String("branch", plan.branch),
zap.String("remote", plan.remote),
zap.Bool("force", opts.Force),
zap.Bool("set_upstream", shouldSetUpstream),
zap.Bool("expected_branch_supplied", opts.ExpectedBranch != ""),
zap.Bool("explicit_target", plan.explicit))
return result, nil
}
// PushPreflight verifies that the configured contribution remote and final
// head-branch refspec are writable without mutating the remote or local refs.
func (g *GitOperator) PushPreflight(ctx context.Context) (*GitOperationResult, error) {
if !g.tryLock("push-preflight") {
return nil, ErrOperationInProgress
}
defer g.unlock()
result := &GitOperationResult{Operation: "push_preflight"}
// validateContributionState runs the existing contribution binding checks,
// which precede every refusal this capability adds.
func (g *GitOperator) validateContributionState(ctx context.Context, force bool) *pushRefusal {
if g.remoteContributionErr != nil {
setPushPreflightError(result, g.remoteContributionErr)
return result, nil
return pushRefusalFromError(g.remoteContributionErr)
}
if g.contributionDestinationErr != nil {
setPushPreflightError(result, g.contributionDestinationErr)
return result, nil
return pushRefusalFromError(g.contributionDestinationErr)
}
if g.contributionDestination != nil {
if err := g.validateContributionDestinationRemote(ctx); err != nil {
setPushPreflightError(result, err)
return result, nil
return pushRefusalFromError(err)
}
branch, err := g.getCurrentBranch(ctx)
if err != nil {
setPushPreflightError(result, err)
return result, nil
}
output, err := g.runGitCommand(ctx, "push", "--dry-run", g.contributionDestination.ContributionRemoteName(), branch)
result.Output = output
if err != nil {
setPushPreflightError(result, err)
return result, nil
}
result.Success = true
g.logger.Info("contribution destination push preflight completed", zap.String("branch", branch))
return result, nil
}
if g.remoteContribution == nil {
result.Success = true
return result, nil
if g.contributionRouted() && force {
return &pushRefusal{message: "force push is not allowed for a remote contribution"}
}
if err := g.validateContributionRemote(ctx); err != nil {
setPushPreflightError(result, err)
return result, nil
return pushRefusalFromError(err)
}
branch, err := g.getCurrentBranch(ctx)
if err != nil {
setPushPreflightError(result, err)
return result, nil
return nil
}
func pushRefusalFromError(err error) *pushRefusal {
return &pushRefusal{
code: classifyPushPreflightError(err),
message: err.Error(),
}
environment := map[string]string{
"GIT_TERMINAL_PROMPT": "0",
"LANG": "C",
"LC_ALL": "C",
}
func (g *GitOperator) validateContributionSource(
ctx context.Context,
environmentOverrides map[string]string,
) *pushRefusal {
if g.remoteContribution == nil {
return nil
}
remote := g.remoteContribution.ContributionRemoteName()
destinationRef := "refs/heads/" + g.remoteContribution.HeadBranch
sourceOutput, sourceErr := g.runGitCommandWithEnvironment(
output, err := g.runGitCommandWithEnvironment(
ctx,
environment,
environmentOverrides,
"ls-remote",
"--refs",
remote,
destinationRef,
)
if sourceErr != nil {
setPushPreflightError(result, sourceErr)
if err != nil {
return pushRefusalFromError(err)
}
if strings.TrimSpace(output) == "" {
return &pushRefusal{
code: models.AgentErrorCauseCodeSourceBranchMissing,
message: "contribution source branch is missing",
}
}
return nil
}
// resolveSetUpstream reads the upstream tracking ref only on the path that can
// use it. The explicit-target path never sets upstream and never reads it,
// because that flag combination is refused before the push.
func (g *GitOperator) resolveSetUpstream(ctx context.Context, opts PushOptions, plan *pushPlan) bool {
if plan.explicit {
return false
}
if plan.routed {
return opts.SetUpstream
}
return opts.SetUpstream || g.getUpstreamRef(ctx) == ""
}
// PushPreflight verifies that the configured contribution remote and final
// head-branch refspec are writable without mutating the remote or local refs.
func (g *GitOperator) PushPreflight(ctx context.Context, opts PushOptions) (*GitOperationResult, error) {
opts = opts.normalized()
if !g.tryLock("push-preflight") {
return nil, ErrOperationInProgress
}
defer g.unlock()
result := &GitOperationResult{Operation: "push_preflight"}
if refusal := g.validateContributionState(ctx, false); refusal != nil {
refusal.apply(result)
return result, nil
}
if strings.TrimSpace(sourceOutput) == "" {
result.Error = "contribution source branch is missing"
result.ErrorCode = models.AgentErrorCauseCodeSourceBranchMissing
// Preflight verifies the expected branch once. It publishes no baseline, so
// there is no window for a second read to close.
plan, refusal := g.resolvePushPlan(ctx, opts, true)
if refusal != nil {
refusal.apply(result)
return result, nil
}
refspec := "HEAD:refs/heads/" + g.remoteContribution.HeadBranch
if refusal := g.validateContributionSource(ctx, contributionPreflightEnvironment); refusal != nil {
refusal.apply(result)
return result, nil
}
// --no-verify: a dry-run push still invokes the local pre-push hook, which
// can mutate the worktree or perform arbitrary side effects. Preflight must
// not mutate anything, so the hook must not run.
output, err := g.runGitCommandWithEnvironment(
ctx,
environment,
contributionPreflightEnvironment,
"push",
"--dry-run",
"--no-verify",
"--porcelain",
remote,
refspec,
plan.remote,
plan.refspec,
)
result.Output = output
if err != nil {
if classifyPushPreflightHistoryUpdate(output, destinationRef) {
if destinationRef, ok := strings.CutPrefix(plan.refspec, "HEAD:"); ok &&
classifyPushPreflightHistoryUpdate(output, destinationRef) {
result.PreflightReason = preflightReasonHistoryUpdateRequired
}
setPushPreflightError(result, err)
return result, nil
}
result.Success = true
g.logger.Info("contribution push preflight completed", zap.String("branch", branch))
// A preflight under contribution routing keeps the result shape it has
// today; every other preflight reports what it validated.
if !plan.routed {
result.PushedRemote = plan.remote
result.PushedBranch = plan.branch
}
g.logger.Info("push preflight completed",
zap.String("branch", plan.branch),
zap.String("remote", plan.remote),
zap.Bool("contribution_routed", plan.routed),
zap.Bool("explicit_target", plan.explicit))
return result, nil
}
@@ -19,6 +19,7 @@ const (
type emptyRemotePublication struct {
active bool
published bool
output string
errorCode string
err error
@@ -135,7 +136,7 @@ func (g *GitOperator) publishEmptyRemoteBase(
err: fmt.Errorf("empty-remote base was published but its local marker could not be retired: %s", g.sanitizePRFailure(retireErr.Error())),
}
}
return emptyRemotePublication{active: true, output: g.sanitizeGitPushOutput(output)}
return emptyRemotePublication{active: true, published: true, output: g.sanitizeGitPushOutput(output)}
}
func (g *GitOperator) validatePublishedEmptyRemoteBase(
@@ -16,7 +16,7 @@ import (
func TestGitOperatorPushPublishesEmptyRemoteBaseBeforeTaskBranch(t *testing.T) {
repoDir, originDir, operator := setupEmptyRemoteTaskRepo(t)
result, err := operator.Push(context.Background(), false, false)
result, err := operator.Push(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
@@ -43,7 +43,7 @@ func TestGitOperatorPushPublishesEmptyRemoteBaseBeforeTaskBranch(t *testing.T) {
func TestGitOperatorPushPublishesAnotherTaskBranchAfterBaseInitialization(t *testing.T) {
repoDir, originDir, operator := setupEmptyRemoteTaskRepo(t)
first, err := operator.Push(context.Background(), false, false)
first, err := operator.Push(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("first Push() error = %v", err)
}
@@ -58,7 +58,7 @@ func TestGitOperatorPushPublishesAnotherTaskBranchAfterBaseInitialization(t *tes
runGit(t, repoDir, "add", "second.txt")
runGit(t, repoDir, "commit", "-m", "second task change")
second, err := operator.Push(context.Background(), false, false)
second, err := operator.Push(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("second Push() error = %v", err)
}
@@ -83,7 +83,7 @@ func TestGitOperatorPushAcceptsMarkedBaseWithUnrelatedRemoteRefs(t *testing.T) {
runGit(t, repoDir, "push", "origin", "external")
runGit(t, repoDir, "checkout", "feature/empty")
result, err := operator.Push(context.Background(), false, false)
result, err := operator.Push(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
@@ -116,7 +116,7 @@ func TestGitOperatorPushStopsWhenEmptyRemoteGainsHistory(t *testing.T) {
runGit(t, seedDir, "commit", "-m", "external history")
runGit(t, seedDir, "push", "origin", "external")
result, err := operator.Push(context.Background(), false, false)
result, err := operator.Push(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
@@ -135,7 +135,7 @@ func TestGitOperatorPushReportsEmptyRemoteBasePublicationFailure(t *testing.T) {
_, originDir, operator := setupEmptyRemoteTaskRepo(t)
writeExecutable(t, filepath.Join(originDir, "hooks", "pre-receive"), "#!/bin/sh\necho reject >&2\nexit 1\n")
result, err := operator.Push(context.Background(), false, false)
result, err := operator.Push(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
@@ -152,7 +152,7 @@ func TestGitOperatorPushReportsTaskBranchFailureAfterBasePublication(t *testing.
hook := "#!/bin/sh\nwhile read old new ref; do\n case \"$ref\" in\n refs/heads/main) exit 0 ;;\n refs/heads/feature/empty) echo reject >&2; exit 1 ;;\n esac\ndone\nexit 0\n"
writeExecutable(t, filepath.Join(originDir, "hooks", "pre-receive"), hook)
result, err := operator.Push(context.Background(), false, false)
result, err := operator.Push(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
@@ -184,7 +184,7 @@ func TestGitOperatorPushRetainsMarkerWhenUnrelatedRefAppearsDuringBasePublicatio
"exit 0\n"
writeExecutable(t, filepath.Join(originDir, "hooks", "pre-receive"), hook)
result, err := operator.Push(context.Background(), false, false)
result, err := operator.Push(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
@@ -229,7 +229,7 @@ func TestGitOperatorPushPublishesValidatedBaselineCommit(t *testing.T) {
writeExecutable(t, filepath.Join(scriptDir, "git"), shim)
t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH"))
result, err := operator.Push(context.Background(), false, false)
result, err := operator.Push(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
@@ -321,7 +321,7 @@ func TestGitOperatorPushRedactsEmptyRemoteProbeFailure(t *testing.T) {
writeExecutable(t, filepath.Join(scriptDir, "git"), fmt.Sprintf("#!/bin/sh\nif [ \"$1\" = \"ls-remote\" ]; then\n printf 'fatal: unable to access https://user:%s@example.com/empty.git: denied\\n' >&2\n exit 1\nfi\nexec %q \"$@\"\n", secret, realGit))
t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH"))
result, err := operator.Push(context.Background(), false, false)
result, err := operator.Push(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
@@ -0,0 +1,838 @@
package process
import (
"context"
"fmt"
"os"
osExec "os/exec"
"path/filepath"
"runtime"
"strings"
"testing"
taskmodels "github.com/kandev/kandev/internal/task/models"
)
// setupPushRemotesRepo builds a checkout on "feature/work" with two bare
// remotes, "origin" and "backup". origin already carries main, so empty-remote
// first publication does not apply unless a test arranges for it.
func setupPushRemotesRepo(t *testing.T) (repoDir, originDir, backupDir string, operator *GitOperator) {
t.Helper()
isolateTestGitEnv(t)
root := t.TempDir()
originDir = filepath.Join(root, "origin.git")
backupDir = filepath.Join(root, "backup.git")
repoDir = filepath.Join(root, "repo")
runGit(t, root, "init", "--bare", "--initial-branch=main", originDir)
runGit(t, root, "init", "--bare", "--initial-branch=main", backupDir)
runGit(t, root, "init", "-b", "main", repoDir)
runGit(t, repoDir, "config", "user.name", "Task User")
runGit(t, repoDir, "config", "user.email", "task@example.com")
runGit(t, repoDir, "remote", "add", "origin", originDir)
runGit(t, repoDir, "remote", "add", "backup", backupDir)
writeFile(t, repoDir, "seed.txt", "seed\n")
runGit(t, repoDir, "add", ".")
runGit(t, repoDir, "commit", "-m", "seed")
runGit(t, repoDir, "push", "origin", "main")
runGit(t, repoDir, "checkout", "-b", "feature/work")
writeFile(t, repoDir, "change.txt", "change\n")
runGit(t, repoDir, "add", ".")
runGit(t, repoDir, "commit", "-m", "task change")
tracker := NewWorkspaceTracker(repoDir, newTestLogger(t))
tracker.SetBaseBranch("main")
return repoDir, originDir, backupDir, NewGitOperator(repoDir, newTestLogger(t), tracker)
}
func remoteBranchSHA(t *testing.T, remoteDir, branch string) string {
t.Helper()
cmd := osExec.Command("git", "-C", remoteDir, "rev-parse", "--verify", "--quiet", "refs/heads/"+branch)
out, err := cmd.Output()
if err != nil {
return ""
}
return strings.TrimSpace(string(out))
}
// recordGitCommands installs a PATH shim that logs every git invocation before
// delegating to the real binary, so a test can assert command ordering.
func recordGitCommands(t *testing.T) func() []string {
t.Helper()
if runtime.GOOS == "windows" {
t.Skip("git PATH shim is a POSIX shell script")
}
realGit, err := osExec.LookPath("git")
if err != nil {
t.Fatalf("LookPath(git) = %v", err)
}
dir := t.TempDir()
logPath := filepath.Join(dir, "git.log")
script := "#!/bin/sh\nprintf '%s\\n' \"$*\" >> " + logPath + "\nexec " + realGit + " \"$@\"\n"
if err := os.WriteFile(filepath.Join(dir, "git"), []byte(script), 0o755); err != nil {
t.Fatal(err)
}
t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH"))
return func() []string {
data, err := os.ReadFile(logPath)
if err != nil {
return nil
}
var lines []string
for _, line := range strings.Split(string(data), "\n") {
if trimmed := strings.TrimSpace(line); trimmed != "" {
lines = append(lines, trimmed)
}
}
return lines
}
}
func TestGitOperatorPushWithoutOptionsIsUnchanged(t *testing.T) {
repoDir, originDir, backupDir, operator := setupPushRemotesRepo(t)
result, err := operator.Push(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if !result.Success {
t.Fatalf("Push() result = %+v, want success", result)
}
local := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD"))
if got := remoteBranchSHA(t, originDir, "feature/work"); got != local {
t.Errorf("origin feature/work = %q, want %q", got, local)
}
if got := remoteBranchSHA(t, backupDir, "feature/work"); got != "" {
t.Errorf("backup gained %q, want untouched", got)
}
// A request naming no target keeps the result shape it has today.
if result.PushedRemote != "" || result.PushedBranch != "" {
t.Errorf("destination fields = (%q, %q), want both empty", result.PushedRemote, result.PushedBranch)
}
// The default path still sets upstream on first publication.
if upstream := operator.getUpstreamRef(context.Background()); upstream != "origin/feature/work" {
t.Errorf("upstream = %q, want origin/feature/work", upstream)
}
}
func TestGitOperatorPushOmitsTargetFieldsWithoutExplicitTarget(t *testing.T) {
_, _, _, operator := setupPushRemotesRepo(t)
result, err := operator.Push(context.Background(), PushOptions{ExpectedBranch: "feature/work"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if !result.Success {
t.Fatalf("Push() result = %+v, want success", result)
}
if result.PushedRemote != "" || result.PushedBranch != "" {
t.Errorf("destination fields = (%q, %q), want both empty", result.PushedRemote, result.PushedBranch)
}
}
func TestGitOperatorPushPublishesHeadToExpectedBranch(t *testing.T) {
repoDir, originDir, backupDir, operator := setupPushRemotesRepo(t)
result, err := operator.Push(context.Background(), PushOptions{
Remote: "backup",
ExpectedBranch: "feature/work",
})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if !result.Success {
t.Fatalf("Push() result = %+v, want success", result)
}
local := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD"))
if got := remoteBranchSHA(t, backupDir, "feature/work"); got != local {
t.Errorf("backup feature/work = %q, want %q", got, local)
}
if got := remoteBranchSHA(t, originDir, "feature/work"); got != "" {
t.Errorf("origin gained %q, want untouched", got)
}
if result.PushedRemote != "backup" || result.PushedBranch != "feature/work" {
t.Errorf("destination fields = (%q, %q), want (backup, feature/work)", result.PushedRemote, result.PushedBranch)
}
}
func TestGitOperatorPushCreatesMissingDestinationBranch(t *testing.T) {
repoDir, _, backupDir, operator := setupPushRemotesRepo(t)
result, err := operator.Push(context.Background(), PushOptions{Remote: "backup"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if !result.Success {
t.Fatalf("Push() result = %+v, want success", result)
}
local := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD"))
if got := remoteBranchSHA(t, backupDir, "feature/work"); got != local {
t.Errorf("backup feature/work = %q, want the branch created at %q", got, local)
}
}
func TestGitOperatorPushToNamedRemoteLeavesUpstreamUnset(t *testing.T) {
_, _, _, operator := setupPushRemotesRepo(t)
before := operator.getUpstreamRef(context.Background())
result, err := operator.Push(context.Background(), PushOptions{Remote: "backup"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if !result.Success {
t.Fatalf("Push() result = %+v, want success", result)
}
if after := operator.getUpstreamRef(context.Background()); after != before {
t.Errorf("upstream changed from %q to %q", before, after)
}
}
func TestGitOperatorPushDoesNotEscalateRejectedNonForcePush(t *testing.T) {
repoDir, _, backupDir, operator := setupPushRemotesRepo(t)
// Give backup a history the local branch does not contain.
runGit(t, repoDir, "push", "backup", "HEAD:refs/heads/feature/work")
runGit(t, repoDir, "checkout", "-b", "diverged")
writeFile(t, repoDir, "diverged.txt", "diverged\n")
runGit(t, repoDir, "add", ".")
runGit(t, repoDir, "commit", "-m", "diverged")
runGit(t, repoDir, "push", "--force", "backup", "HEAD:refs/heads/feature/work")
runGit(t, repoDir, "checkout", "feature/work")
remoteBefore := remoteBranchSHA(t, backupDir, "feature/work")
result, err := operator.Push(context.Background(), PushOptions{Remote: "backup"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if result.Success {
t.Fatalf("non-force push to a diverged branch succeeded: %+v", result)
}
if got := remoteBranchSHA(t, backupDir, "feature/work"); got != remoteBefore {
t.Errorf("backup moved to %q, want %q", got, remoteBefore)
}
}
func TestGitOperatorPushNamedFanoutRemotePublishesToEveryPushURL(t *testing.T) {
repoDir, originDir, backupDir, operator := setupPushRemotesRepo(t)
runGit(t, repoDir, "remote", "add", "mirror", originDir)
runGit(t, repoDir, "remote", "set-url", "--push", "mirror", originDir)
runGit(t, repoDir, "remote", "set-url", "--add", "--push", "mirror", backupDir)
result, err := operator.Push(context.Background(), PushOptions{
Remote: "mirror", ExpectedBranch: "feature/work",
})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if !result.Success {
t.Fatalf("Push() result = %+v, want success", result)
}
want := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD"))
for name, remoteDir := range map[string]string{"origin": originDir, "backup": backupDir} {
if got := remoteBranchSHA(t, remoteDir, "feature/work"); got != want {
t.Errorf("%s feature/work = %q, want %q", name, got, want)
}
}
}
func TestGitOperatorPushToNamedRemoteSkipsBaselinePublication(t *testing.T) {
root := t.TempDir()
isolateTestGitEnv(t)
originDir := filepath.Join(root, "origin.git")
backupDir := filepath.Join(root, "backup.git")
repoDir := filepath.Join(root, "repo")
runGit(t, root, "init", "--bare", "--initial-branch=main", originDir)
runGit(t, root, "init", "--bare", "--initial-branch=main", backupDir)
runGit(t, root, "init", "-b", "main", repoDir)
runGit(t, repoDir, "config", "user.name", "Task User")
runGit(t, repoDir, "config", "user.email", "task@example.com")
runGit(t, repoDir, "remote", "add", "origin", originDir)
runGit(t, repoDir, "remote", "add", "backup", backupDir)
writeFile(t, repoDir, "seed.txt", "seed\n")
runGit(t, repoDir, "add", ".")
runGit(t, repoDir, "commit", "-m", "seed")
runGit(t, repoDir, "checkout", "-b", "feature/work")
tracker := NewWorkspaceTracker(repoDir, newTestLogger(t))
tracker.SetBaseBranch("main")
operator := NewGitOperator(repoDir, newTestLogger(t), tracker)
// origin is empty; a push to backup must not publish a baseline anywhere.
result, err := operator.Push(context.Background(), PushOptions{Remote: "backup"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if !result.Success {
t.Fatalf("Push() result = %+v, want success", result)
}
if got := remoteBranchSHA(t, originDir, "main"); got != "" {
t.Errorf("origin gained a baseline %q, want empty", got)
}
if result.BaselinePublished {
t.Error("BaselinePublished = true, want false")
}
}
func TestGitOperatorPushRefusesTargetWithSetUpstream(t *testing.T) {
_, _, backupDir, operator := setupPushRemotesRepo(t)
result, err := operator.Push(context.Background(), PushOptions{Remote: "backup", SetUpstream: true})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if result.ErrorCode != pushRemoteUpstreamUnsupportedErrorCode {
t.Fatalf("ErrorCode = %q, want %q", result.ErrorCode, pushRemoteUpstreamUnsupportedErrorCode)
}
if got := remoteBranchSHA(t, backupDir, "feature/work"); got != "" {
t.Errorf("backup gained %q, want untouched", got)
}
}
func TestGitOperatorPushRefusesExpectedBranchMismatch(t *testing.T) {
_, originDir, _, operator := setupPushRemotesRepo(t)
result, err := operator.Push(context.Background(), PushOptions{ExpectedBranch: "feature/other"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if result.ErrorCode != pushBranchMismatchErrorCode {
t.Fatalf("ErrorCode = %q, want %q", result.ErrorCode, pushBranchMismatchErrorCode)
}
if result.ExpectedBranch != "feature/other" || result.CurrentBranch != "feature/work" {
t.Errorf("branches = (%q, %q), want (feature/other, feature/work)", result.ExpectedBranch, result.CurrentBranch)
}
if result.BaselinePublished {
t.Error("BaselinePublished = true, want false")
}
if got := remoteBranchSHA(t, originDir, "feature/work"); got != "" {
t.Errorf("origin gained %q, want no remote contact", got)
}
}
func TestGitOperatorPushRefusesDetachedHeadAsMismatch(t *testing.T) {
repoDir, originDir, _, operator := setupPushRemotesRepo(t)
runGit(t, repoDir, "checkout", "--detach", "HEAD")
result, err := operator.Push(context.Background(), PushOptions{ExpectedBranch: "feature/work"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if result.ErrorCode != pushBranchMismatchErrorCode {
t.Fatalf("ErrorCode = %q, want %q", result.ErrorCode, pushBranchMismatchErrorCode)
}
// A detached HEAD is an empty current branch, never the literal "HEAD".
if result.CurrentBranch != "" {
t.Errorf("CurrentBranch = %q, want empty", result.CurrentBranch)
}
if got := remoteBranchSHA(t, originDir, "feature/work"); got != "" {
t.Errorf("origin gained %q, want no remote contact", got)
}
}
func TestGitOperatorPushRefusesDetachedHeadWithTargetOnly(t *testing.T) {
repoDir, _, backupDir, operator := setupPushRemotesRepo(t)
runGit(t, repoDir, "checkout", "--detach", "HEAD")
result, err := operator.Push(context.Background(), PushOptions{Remote: "backup"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if result.ErrorCode != pushBranchDetachedErrorCode {
t.Fatalf("ErrorCode = %q, want %q", result.ErrorCode, pushBranchDetachedErrorCode)
}
if got := remoteBranchSHA(t, backupDir, "feature/work"); got != "" {
t.Errorf("backup gained %q, want untouched", got)
}
}
func TestGitOperatorPushExpectedBranchAloneOnlyGates(t *testing.T) {
repoDir, originDir, _, operator := setupPushRemotesRepo(t)
result, err := operator.Push(context.Background(), PushOptions{ExpectedBranch: "feature/work"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if !result.Success {
t.Fatalf("Push() result = %+v, want success", result)
}
// The expected branch is a precondition only: remote, refspec and upstream
// behavior are the ones the request would have had without it.
local := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD"))
if got := remoteBranchSHA(t, originDir, "feature/work"); got != local {
t.Errorf("origin feature/work = %q, want %q", got, local)
}
if upstream := operator.getUpstreamRef(context.Background()); upstream != "origin/feature/work" {
t.Errorf("upstream = %q, want origin/feature/work", upstream)
}
}
func TestGitOperatorPushRefusalOrdering(t *testing.T) {
_, _, _, operator := setupPushRemotesRepo(t)
for _, tc := range []struct {
name string
opts PushOptions
want string
}{
{
name: "upstream conflict outranks an invalid branch",
opts: PushOptions{Remote: "backup", SetUpstream: true, ExpectedBranch: "bad..branch"},
want: pushRemoteUpstreamUnsupportedErrorCode,
},
{
name: "invalid branch outranks target resolution",
opts: PushOptions{Remote: "missing", ExpectedBranch: "bad..branch"},
want: pushBranchInvalidErrorCode,
},
{
name: "target resolution outranks a branch mismatch",
opts: PushOptions{Remote: "missing", ExpectedBranch: "feature/other"},
want: pushRemoteNotFoundErrorCode,
},
{
name: "detached outranks nothing but is reported for a target alone",
opts: PushOptions{Remote: "backup", ExpectedBranch: "HEAD"},
want: pushBranchInvalidErrorCode,
},
} {
t.Run(tc.name, func(t *testing.T) {
result, err := operator.Push(context.Background(), tc.opts)
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if result.ErrorCode != tc.want {
t.Errorf("ErrorCode = %q, want %q", result.ErrorCode, tc.want)
}
})
}
}
func TestGitOperatorPushRefusalsAreNoOps(t *testing.T) {
repoDir, originDir, backupDir, operator := setupPushRemotesRepo(t)
upstreamBefore := operator.getUpstreamRef(context.Background())
headBefore := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD"))
for _, opts := range []PushOptions{
{Remote: "missing"},
{Remote: "https://example.test/nope.git"},
{Remote: "backup", SetUpstream: true},
{ExpectedBranch: "bad..branch"},
{ExpectedBranch: "feature/other"},
} {
result, err := operator.Push(context.Background(), opts)
if err != nil {
t.Fatalf("Push(%+v) error = %v", opts, err)
}
if result.Success {
t.Fatalf("Push(%+v) succeeded, want refusal", opts)
}
}
if got := remoteBranchSHA(t, originDir, "feature/work"); got != "" {
t.Errorf("origin gained %q", got)
}
if got := remoteBranchSHA(t, backupDir, "feature/work"); got != "" {
t.Errorf("backup gained %q", got)
}
if got := operator.getUpstreamRef(context.Background()); got != upstreamBefore {
t.Errorf("upstream changed from %q to %q", upstreamBefore, got)
}
if got := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD")); got != headBefore {
t.Errorf("HEAD moved from %q to %q", headBefore, got)
}
}
func TestGitOperatorPushReportsOperationInProgress(t *testing.T) {
_, _, _, operator := setupPushRemotesRepo(t)
if !operator.tryLock("test") {
t.Fatal("tryLock() = false, want the lock")
}
defer operator.unlock()
_, err := operator.Push(context.Background(), PushOptions{Remote: "backup"})
if err != ErrOperationInProgress {
t.Fatalf("Push() error = %v, want ErrOperationInProgress", err)
}
}
func TestGitOperatorPushRepeatedIdenticalRequestSucceeds(t *testing.T) {
repoDir, _, backupDir, operator := setupPushRemotesRepo(t)
opts := PushOptions{Remote: "backup", ExpectedBranch: "feature/work"}
first, err := operator.Push(context.Background(), opts)
if err != nil || !first.Success {
t.Fatalf("first Push() = %+v, err = %v", first, err)
}
after := remoteBranchSHA(t, backupDir, "feature/work")
second, err := operator.Push(context.Background(), opts)
if err != nil {
t.Fatalf("second Push() error = %v", err)
}
if !second.Success {
t.Fatalf("second Push() = %+v, want success", second)
}
if got := remoteBranchSHA(t, backupDir, "feature/work"); got != after {
t.Errorf("destination moved from %q to %q", after, got)
}
local := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD"))
if after != local {
t.Errorf("destination = %q, want %q", after, local)
}
}
func TestGitOperatorPushNeverCreatesRemote(t *testing.T) {
repoDir, _, _, operator := setupPushRemotesRepo(t)
before := strings.TrimSpace(runGit(t, repoDir, "remote"))
if _, err := operator.Push(context.Background(), PushOptions{Remote: "https://example.test/new.git"}); err != nil {
t.Fatalf("Push() error = %v", err)
}
if _, err := operator.Push(context.Background(), PushOptions{Remote: "brand-new"}); err != nil {
t.Fatalf("Push() error = %v", err)
}
if after := strings.TrimSpace(runGit(t, repoDir, "remote")); after != before {
t.Errorf("remotes changed from %q to %q", before, after)
}
}
func TestGitOperatorPushResultAndLogsCarryNoRemoteURL(t *testing.T) {
repoDir, _, backupDir, _ := setupPushRemotesRepo(t)
log, observed := newObservedTestLogger(t)
operator := NewGitOperator(repoDir, log, nil)
result, err := operator.Push(context.Background(), PushOptions{Remote: backupDir})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if !result.Success {
t.Fatalf("Push() result = %+v, want success", result)
}
// The URL form resolves to a name, and only the name is reported back.
if result.PushedRemote != "backup" {
t.Errorf("PushedRemote = %q, want backup", result.PushedRemote)
}
if strings.Contains(result.Error, backupDir) {
t.Errorf("error carries the remote URL: %q", result.Error)
}
for _, entry := range observed.All() {
if strings.Contains(entry.Message, backupDir) || strings.Contains(fmt.Sprint(entry.ContextMap()), backupDir) {
t.Errorf("log entry carries the remote URL: message=%q context=%v", entry.Message, entry.ContextMap())
}
}
}
func TestGitOperatorPushRedactsCredentialsOnExplicitTargetPath(t *testing.T) {
repoDir, _, _, operator := setupPushRemotesRepo(t)
const secret = "s3cr3t-token"
runGit(t, repoDir, "remote", "add", "secured", "https://user:"+secret+"@example.invalid/repo.git")
result, err := operator.Push(context.Background(), PushOptions{Remote: "secured"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if result.Success {
t.Fatal("push to an unreachable host succeeded")
}
if strings.Contains(result.Output, secret) || strings.Contains(result.Error, secret) {
t.Errorf("credential leaked: output=%q error=%q", result.Output, result.Error)
}
}
func TestGitOperatorPushVerifiesBranchAsLastReadBeforePush(t *testing.T) {
for _, tc := range []struct {
name string
opts PushOptions
}{
{name: "explicit target", opts: PushOptions{Remote: "backup", ExpectedBranch: "feature/work"}},
{name: "default path", opts: PushOptions{ExpectedBranch: "feature/work"}},
} {
t.Run(tc.name, func(t *testing.T) {
_, _, _, operator := setupPushRemotesRepo(t)
readLog := recordGitCommands(t)
result, err := operator.Push(context.Background(), tc.opts)
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if !result.Success {
t.Fatalf("Push() result = %+v, want success", result)
}
lines := readLog()
pushIndex := -1
for i, line := range lines {
if strings.HasPrefix(line, "push ") {
pushIndex = i
}
}
if pushIndex <= 0 {
t.Fatalf("no push command recorded in %v", lines)
}
if prev := lines[pushIndex-1]; prev != "symbolic-ref HEAD" {
t.Errorf("command before push = %q, want the expected-branch read %q (full log: %v)",
prev, "symbolic-ref HEAD", lines)
}
})
}
}
func TestGitOperatorPushRefusesTargetUnderContributionRouting(t *testing.T) {
repoDir, _, backupDir, operator := setupPushRemotesRepo(t)
head := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD"))
const sourceURL = "https://github.com/contributor/widget.git"
binding := &taskmodels.RemoteContribution{
Version: taskmodels.RemoteContributionVersion,
Provider: taskmodels.RemoteContributionProviderGitHub,
Kind: taskmodels.RemoteContributionKindPullRequest,
CanonicalURL: "https://github.com/acme/widget/pull/7",
Number: 7,
State: taskmodels.RemoteContributionStateOpen,
BaseBranch: "main",
HeadBranch: "feature/work",
HeadSHA: head,
SourceRepository: taskmodels.RemoteContributionRepository{
Host: "github.com", Path: "contributor/widget", RemoteURL: sourceURL,
},
CollaborationAllowed: true,
}
if err := binding.Validate(); err != nil {
t.Fatalf("binding.Validate() = %v", err)
}
runGit(t, repoDir, "remote", "add", binding.ContributionRemoteName(), sourceURL)
operator.setRemoteContribution(binding)
result, err := operator.Push(context.Background(), PushOptions{Remote: "backup"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if result.ErrorCode != pushRemoteContributionConflictErrorCode {
t.Fatalf("ErrorCode = %q, want %q", result.ErrorCode, pushRemoteContributionConflictErrorCode)
}
if got := remoteBranchSHA(t, backupDir, "feature/work"); got != "" {
t.Errorf("backup gained %q, want untouched", got)
}
}
// TestGitOperatorPushReportsMismatchAfterBaselinePublication drives the only
// refusal in this capability that is not side-effect-free: HEAD moves after
// empty-remote first publication published the baseline, so the second
// verification refuses a push whose baseline is already on the remote.
func TestGitOperatorPushReportsMismatchAfterBaselinePublication(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("git PATH shim is a POSIX shell script")
}
repoDir, originDir, operator := setupEmptyRemoteTaskRepo(t)
realGit, err := osExec.LookPath("git")
if err != nil {
t.Fatalf("LookPath(git) = %v", err)
}
// Move HEAD immediately after the baseline push, which is the window the
// second verification exists to close.
shimDir := t.TempDir()
script := "#!/bin/sh\n" +
"case \"$1 $2\" in\n" +
" 'push --force-with-lease=refs/heads/main:')\n" +
" " + realGit + " \"$@\"; rc=$?\n" +
" " + realGit + " -C " + repoDir + " symbolic-ref HEAD refs/heads/switched\n" +
" exit $rc ;;\n" +
"esac\n" +
"exec " + realGit + " \"$@\"\n"
if err := os.WriteFile(filepath.Join(shimDir, "git"), []byte(script), 0o755); err != nil {
t.Fatal(err)
}
t.Setenv("PATH", shimDir+string(os.PathListSeparator)+os.Getenv("PATH"))
result, err := operator.Push(context.Background(), PushOptions{ExpectedBranch: "feature/empty"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if result.Success {
t.Fatalf("Push() succeeded, want a post-baseline mismatch refusal: %+v", result)
}
if result.ErrorCode != pushBranchMismatchAfterBaselineErrorCode {
t.Fatalf("ErrorCode = %q, want %q", result.ErrorCode, pushBranchMismatchAfterBaselineErrorCode)
}
if !result.BaselinePublished {
t.Error("BaselinePublished = false, want true")
}
if result.ExpectedBranch != "feature/empty" || result.CurrentBranch != "switched" {
t.Errorf("branches = (%q, %q), want (feature/empty, switched)", result.ExpectedBranch, result.CurrentBranch)
}
// The baseline landed; the task branch must not have.
if got := remoteBranchSHA(t, originDir, "main"); got == "" {
t.Error("baseline was not published, so this test did not exercise its window")
}
if got := remoteBranchSHA(t, originDir, "feature/empty"); got != "" {
t.Errorf("task branch published as %q, want unpublished", got)
}
}
// TestGitOperatorPushReportsPlainMismatchAtSecondVerification drives the other
// outcome of the second verification: a mismatch that did not follow a
// baseline publication. An explicit push target to a remote other than
// "origin" is never baseline-eligible, so moving HEAD in the window between
// the first and second verification must still be caught, and must carry
// plain push_branch_mismatch rather than the after-baseline code.
func TestGitOperatorPushReportsPlainMismatchAtSecondVerification(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("git PATH shim is a POSIX shell script")
}
repoDir, _, backupDir, operator := setupPushRemotesRepo(t)
realGit, err := osExec.LookPath("git")
if err != nil {
t.Fatalf("LookPath(git) = %v", err)
}
// The explicit-target, no-baseline path issues exactly two "symbolic-ref
// HEAD" reads: the first verification inside resolvePushPlan, and the
// second verification immediately before the push. Move HEAD right after
// answering the first one.
shimDir := t.TempDir()
counterPath := filepath.Join(shimDir, "count")
script := "#!/bin/sh\n" +
"if [ \"$1 $2\" = 'symbolic-ref HEAD' ]; then\n" +
" n=$(cat " + counterPath + " 2>/dev/null || echo 0)\n" +
" n=$((n+1))\n" +
" echo $n > " + counterPath + "\n" +
" " + realGit + " \"$@\"; rc=$?\n" +
" if [ \"$n\" = \"1\" ]; then\n" +
" " + realGit + " -C " + repoDir + " symbolic-ref HEAD refs/heads/switched\n" +
" fi\n" +
" exit $rc\n" +
"fi\n" +
"exec " + realGit + " \"$@\"\n"
if err := os.WriteFile(filepath.Join(shimDir, "git"), []byte(script), 0o755); err != nil {
t.Fatal(err)
}
t.Setenv("PATH", shimDir+string(os.PathListSeparator)+os.Getenv("PATH"))
result, err := operator.Push(context.Background(), PushOptions{Remote: "backup", ExpectedBranch: "feature/work"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if result.Success {
t.Fatalf("Push() succeeded, want a second-verification mismatch refusal: %+v", result)
}
if result.ErrorCode != pushBranchMismatchErrorCode {
t.Fatalf("ErrorCode = %q, want %q", result.ErrorCode, pushBranchMismatchErrorCode)
}
if result.BaselinePublished {
t.Error("BaselinePublished = true, want false: backup is never baseline-eligible")
}
if result.ExpectedBranch != "feature/work" || result.CurrentBranch != "switched" {
t.Errorf("branches = (%q, %q), want (feature/work, switched)", result.ExpectedBranch, result.CurrentBranch)
}
if got := remoteBranchSHA(t, backupDir, "feature/work"); got != "" {
t.Errorf("backup gained %q, want untouched", got)
}
}
// TestGitOperatorPushReportsPlainMismatchWhenBaselineWasAlreadyPublished
// covers the empty-remote path where the baseline was published by an earlier
// request: this request's prepareEmptyRemotePublication only retires the
// local marker and pushes nothing itself. A HEAD race at the second
// verification must still report plain push_branch_mismatch, not the
// after-baseline variant, because no baseline publication happened in this
// request.
func TestGitOperatorPushReportsPlainMismatchWhenBaselineWasAlreadyPublished(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("git PATH shim is a POSIX shell script")
}
repoDir, originDir, operator := setupEmptyRemoteTaskRepo(t)
// Publish the baseline directly, bypassing gitbootstrap, to simulate an
// earlier request having already published it. The local marker is left
// in place, so this request's empty-remote path takes the
// already-published, retire-only branch.
runGit(t, repoDir, "push", originDir, "main:refs/heads/main")
realGit, err := osExec.LookPath("git")
if err != nil {
t.Fatalf("LookPath(git) = %v", err)
}
// Move HEAD right after the remote-ref probe that decides the baseline is
// already published, which is before this request issues any push.
shimDir := t.TempDir()
script := "#!/bin/sh\n" +
"if [ \"$1 $2\" = 'ls-remote --refs' ]; then\n" +
" " + realGit + " \"$@\"; rc=$?\n" +
" " + realGit + " -C " + repoDir + " symbolic-ref HEAD refs/heads/switched\n" +
" exit $rc\n" +
"fi\n" +
"exec " + realGit + " \"$@\"\n"
if err := os.WriteFile(filepath.Join(shimDir, "git"), []byte(script), 0o755); err != nil {
t.Fatal(err)
}
t.Setenv("PATH", shimDir+string(os.PathListSeparator)+os.Getenv("PATH"))
result, err := operator.Push(context.Background(), PushOptions{ExpectedBranch: "feature/empty"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if result.Success {
t.Fatalf("Push() succeeded, want a second-verification mismatch refusal: %+v", result)
}
if result.ErrorCode != pushBranchMismatchErrorCode {
t.Fatalf("ErrorCode = %q, want %q (not the after-baseline variant: no publication happened this request)", result.ErrorCode, pushBranchMismatchErrorCode)
}
if result.BaselinePublished {
t.Error("BaselinePublished = true, want false: the baseline was published by an earlier request, not this one")
}
if result.ExpectedBranch != "feature/empty" || result.CurrentBranch != "switched" {
t.Errorf("branches = (%q, %q), want (feature/empty, switched)", result.ExpectedBranch, result.CurrentBranch)
}
if got := remoteBranchSHA(t, originDir, "feature/empty"); got != "" {
t.Errorf("task branch published as %q, want unpublished", got)
}
}
// TestGitOperatorPushUsesExpectedBranchNotARaceableRereadForNoTargetRefspec
// covers the no-explicit-target path (the default-remote and
// contribution-destination cases in buildPushPlan both build a bare-name
// refspec off a branch value). That value must come from the same identity
// verifyExpectedBranch confirms immediately before the push, not from a
// separate read: a separate read can observe a different branch if HEAD
// moves and moves back between the first verification and this one, letting
// the second verification pass while the push publishes unrelated content
// under an unverified name and leaves the intended branch unpublished.
func TestGitOperatorPushUsesExpectedBranchNotARaceableRereadForNoTargetRefspec(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("git PATH shim is a POSIX shell script")
}
repoDir, originDir, _, operator := setupPushRemotesRepo(t)
runGit(t, repoDir, "branch", "feature/other", "main")
realGit, err := osExec.LookPath("git")
if err != nil {
t.Fatalf("LookPath(git) = %v", err)
}
// Move HEAD to feature/other around the rev-parse call that would decide
// the push's branch/refspec independently of the first verification, then
// move it back before the push runs, so only a re-read would observe the
// switch.
shimDir := t.TempDir()
script := "#!/bin/sh\n" +
"if [ \"$1 $2 $3\" = 'rev-parse --abbrev-ref HEAD' ]; then\n" +
" " + realGit + " -C " + repoDir + " checkout feature/other >/dev/null 2>&1\n" +
" " + realGit + " \"$@\"; rc=$?\n" +
" " + realGit + " -C " + repoDir + " checkout feature/work >/dev/null 2>&1\n" +
" exit $rc\n" +
"fi\n" +
"exec " + realGit + " \"$@\"\n"
if err := os.WriteFile(filepath.Join(shimDir, "git"), []byte(script), 0o755); err != nil {
t.Fatal(err)
}
t.Setenv("PATH", shimDir+string(os.PathListSeparator)+os.Getenv("PATH"))
wantSHA := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "feature/work"))
result, err := operator.Push(context.Background(), PushOptions{ExpectedBranch: "feature/work"})
if err != nil {
t.Fatalf("Push() error = %v", err)
}
if !result.Success {
t.Fatalf("Push() = %+v, want success", result)
}
if got := remoteBranchSHA(t, originDir, "feature/work"); got != wantSHA {
t.Errorf("origin feature/work = %q, want %q: the verified branch was not what got pushed", got, wantSHA)
}
if got := remoteBranchSHA(t, originDir, "feature/other"); got != "" {
t.Errorf("origin gained feature/other (%q): the push targeted a branch never verified", got)
}
}
@@ -3,8 +3,15 @@ package process
import (
"context"
"encoding/json"
"fmt"
"os"
osExec "os/exec"
"path/filepath"
"runtime"
"strings"
"testing"
taskmodels "github.com/kandev/kandev/internal/task/models"
)
func TestPushPreflightHistoryClassification(t *testing.T) {
@@ -12,7 +19,7 @@ func TestPushPreflightHistoryClassification(t *testing.T) {
operator := NewGitOperator(repoDir, newTestLogger(t), nil)
operator.setRemoteContribution(binding)
result, err := operator.PushPreflight(context.Background())
result, err := operator.PushPreflight(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("PushPreflight returned error: %v", err)
}
@@ -40,6 +47,83 @@ func TestPushPreflightHistoryClassification(t *testing.T) {
}
}
func TestPushPreflightRejectsMissingContributionSource(t *testing.T) {
repoDir, originDir, _, binding, _, _, _ := setupDivergedContributionRepo(t)
runGit(t, originDir, "update-ref", "-d", "refs/heads/"+binding.HeadBranch)
operator := NewGitOperator(repoDir, newTestLogger(t), nil)
operator.setRemoteContribution(binding)
result, err := operator.PushPreflight(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("PushPreflight returned error: %v", err)
}
if result.Success {
t.Fatalf("PushPreflight = %+v, want missing-source refusal", result)
}
if result.ErrorCode != taskmodels.AgentErrorCauseCodeSourceBranchMissing {
t.Fatalf("ErrorCode = %q, want %q", result.ErrorCode, taskmodels.AgentErrorCauseCodeSourceBranchMissing)
}
if got := remoteBranchSHA(t, originDir, binding.HeadBranch); got != "" {
t.Fatalf("missing contribution source was recreated at %q", got)
}
}
func TestPushPreflightUsesStableGitEnvironment(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("git PATH shim is a POSIX shell script")
}
repoDir, _, _, binding, _, _, _ := setupDivergedContributionRepo(t)
operator := NewGitOperator(repoDir, newTestLogger(t), nil)
operator.setRemoteContribution(binding)
realGit, err := osExec.LookPath("git")
if err != nil {
t.Fatalf("LookPath(git) = %v", err)
}
shimDir := t.TempDir()
envPath := filepath.Join(shimDir, "push-env")
script := fmt.Sprintf("#!/bin/sh\ncase \"$1\" in\nls-remote|push) printf 'LANG=%%s\\nLC_ALL=%%s\\nGIT_TERMINAL_PROMPT=%%s\\n' \"$LANG\" \"$LC_ALL\" \"$GIT_TERMINAL_PROMPT\" > \"$KANDEV_TEST_PUSH_ENV\";;\nesac\nexec %s \"$@\"\n", realGit)
if err := os.WriteFile(filepath.Join(shimDir, "git"), []byte(script), 0o755); err != nil {
t.Fatal(err)
}
t.Setenv("PATH", shimDir+string(os.PathListSeparator)+os.Getenv("PATH"))
t.Setenv("KANDEV_TEST_PUSH_ENV", envPath)
t.Setenv("LANG", "pt_PT.UTF-8")
t.Setenv("LC_ALL", "pt_PT.UTF-8")
t.Setenv("GIT_TERMINAL_PROMPT", "1")
_, err = operator.PushPreflight(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("PushPreflight returned error: %v", err)
}
env, err := os.ReadFile(envPath)
if err != nil {
t.Fatalf("read recorded Git environment: %v", err)
}
want := "LANG=C\nLC_ALL=C\nGIT_TERMINAL_PROMPT=0\n"
if got := string(env); got != want {
t.Fatalf("preflight Git environment = %q, want %q", got, want)
}
}
func TestPushPreflightPreservesContributionValidationErrorCode(t *testing.T) {
repoDir, _, remoteName, binding, _, _, _ := setupDivergedContributionRepo(t)
runGit(t, repoDir, "remote", "set-url", remoteName, "https://example.test/wrong-source.git")
operator := NewGitOperator(repoDir, newTestLogger(t), nil)
operator.setRemoteContribution(binding)
result, err := operator.PushPreflight(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("PushPreflight returned error: %v", err)
}
if result.Success {
t.Fatalf("PushPreflight = %+v, want validation refusal", result)
}
if result.ErrorCode != taskmodels.AgentErrorCauseCodeDestinationInvalid {
t.Fatalf("ErrorCode = %q, want %q", result.ErrorCode, taskmodels.AgentErrorCauseCodeDestinationInvalid)
}
}
func TestClassifyPushPreflightHistoryUpdate(t *testing.T) {
const destination = "refs/heads/feature/remote"
tests := []struct {
@@ -86,3 +170,233 @@ func TestClassifyPushPreflightHistoryUpdate(t *testing.T) {
})
}
}
func TestPushPreflightValidatesOriginWithoutTarget(t *testing.T) {
_, originDir, _, operator := setupPushRemotesRepo(t)
result, err := operator.PushPreflight(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("PushPreflight() error = %v", err)
}
if !result.Success {
t.Fatalf("PushPreflight() = %+v, want success", result)
}
// It reports what it validated rather than a blanket success.
if result.PushedRemote != "origin" || result.PushedBranch != "feature/work" {
t.Errorf("validated (%q, %q), want (origin, feature/work)", result.PushedRemote, result.PushedBranch)
}
if got := remoteBranchSHA(t, originDir, "feature/work"); got != "" {
t.Errorf("preflight published %q, want a dry run", got)
}
}
func TestPushPreflightValidatesNamedRemote(t *testing.T) {
_, originDir, backupDir, operator := setupPushRemotesRepo(t)
result, err := operator.PushPreflight(context.Background(), PushOptions{
Remote: "backup",
ExpectedBranch: "feature/work",
})
if err != nil {
t.Fatalf("PushPreflight() error = %v", err)
}
if !result.Success {
t.Fatalf("PushPreflight() = %+v, want success", result)
}
if result.PushedRemote != "backup" || result.PushedBranch != "feature/work" {
t.Errorf("validated (%q, %q), want (backup, feature/work)", result.PushedRemote, result.PushedBranch)
}
if got := remoteBranchSHA(t, backupDir, "feature/work"); got != "" {
t.Errorf("preflight published %q to backup, want a dry run", got)
}
if got := remoteBranchSHA(t, originDir, "feature/work"); got != "" {
t.Errorf("preflight touched origin (%q)", got)
}
}
func TestPushPreflightReportsValidatedRemoteAndBranch(t *testing.T) {
_, _, _, operator := setupPushRemotesRepo(t)
// Both with and without an explicit target, a non-routed preflight reports
// the destination it checked.
for _, opts := range []PushOptions{{}, {Remote: "backup"}} {
result, err := operator.PushPreflight(context.Background(), opts)
if err != nil {
t.Fatalf("PushPreflight(%+v) error = %v", opts, err)
}
if !result.Success {
t.Fatalf("PushPreflight(%+v) = %+v, want success", opts, result)
}
if result.PushedRemote == "" || result.PushedBranch == "" {
t.Errorf("PushPreflight(%+v) reported (%q, %q), want both populated",
opts, result.PushedRemote, result.PushedBranch)
}
}
}
func TestPushPreflightReportsNoRemoteConfigured(t *testing.T) {
root := t.TempDir()
isolateTestGitEnv(t)
repoDir := filepath.Join(root, "repo")
runGit(t, root, "init", "-b", "main", repoDir)
runGit(t, repoDir, "config", "user.name", "Task User")
runGit(t, repoDir, "config", "user.email", "task@example.com")
writeFile(t, repoDir, "seed.txt", "seed\n")
runGit(t, repoDir, "add", ".")
runGit(t, repoDir, "commit", "-m", "seed")
operator := NewGitOperator(repoDir, newTestLogger(t), nil)
result, err := operator.PushPreflight(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("PushPreflight() error = %v", err)
}
if result.Success {
t.Fatalf("PushPreflight() = %+v, want a refusal", result)
}
if result.ErrorCode != pushNoRemoteConfiguredErrorCode {
t.Errorf("ErrorCode = %q, want %q", result.ErrorCode, pushNoRemoteConfiguredErrorCode)
}
}
func TestPushPreflightMutatesNothing(t *testing.T) {
repoDir, originDir, backupDir, operator := setupPushRemotesRepo(t)
headBefore := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD"))
branchesBefore := runGit(t, repoDir, "branch", "--all")
upstreamBefore := operator.getUpstreamRef(context.Background())
originMain := remoteBranchSHA(t, originDir, "main")
if _, err := operator.PushPreflight(context.Background(), PushOptions{Remote: "backup"}); err != nil {
t.Fatalf("PushPreflight() error = %v", err)
}
if got := strings.TrimSpace(runGit(t, repoDir, "rev-parse", "HEAD")); got != headBefore {
t.Errorf("HEAD moved from %q to %q", headBefore, got)
}
if got := runGit(t, repoDir, "branch", "--all"); got != branchesBefore {
t.Errorf("local refs changed:\n%s\nwant:\n%s", got, branchesBefore)
}
if got := operator.getUpstreamRef(context.Background()); got != upstreamBefore {
t.Errorf("upstream changed from %q to %q", upstreamBefore, got)
}
if got := remoteBranchSHA(t, originDir, "main"); got != originMain {
t.Errorf("origin main moved from %q to %q", originMain, got)
}
if got := remoteBranchSHA(t, backupDir, "feature/work"); got != "" {
t.Errorf("backup gained %q", got)
}
}
func TestPushPreflightReportsRemoteRefusalOutput(t *testing.T) {
repoDir, _, backupDir, operator := setupPushRemotesRepo(t)
// Give backup a history the local branch does not contain, so the write is
// a non-fast-forward the remote refuses.
runGit(t, repoDir, "checkout", "-b", "diverged")
writeFile(t, repoDir, "diverged.txt", "diverged\n")
runGit(t, repoDir, "add", ".")
runGit(t, repoDir, "commit", "-m", "diverged")
runGit(t, repoDir, "push", "backup", "HEAD:refs/heads/feature/work")
runGit(t, repoDir, "checkout", "feature/work")
before := remoteBranchSHA(t, backupDir, "feature/work")
result, err := operator.PushPreflight(context.Background(), PushOptions{Remote: "backup"})
if err != nil {
t.Fatalf("PushPreflight() returned a transport error rather than a result: %v", err)
}
if result.Success {
t.Fatalf("PushPreflight() = %+v, want the refusal reported", result)
}
if result.Error == "" && result.Output == "" {
t.Error("refusal reported neither an error nor the remote's output")
}
if got := remoteBranchSHA(t, backupDir, "feature/work"); got != before {
t.Errorf("backup moved from %q to %q", before, got)
}
}
func TestPushPreflightRedactsCredentialsInRemoteOutput(t *testing.T) {
repoDir, _, _, operator := setupPushRemotesRepo(t)
const secret = "s3cr3t-token"
runGit(t, repoDir, "remote", "add", "secured", "https://user:"+secret+"@example.invalid/repo.git")
result, err := operator.PushPreflight(context.Background(), PushOptions{Remote: "secured"})
if err != nil {
t.Fatalf("PushPreflight() error = %v", err)
}
if result.Success {
t.Fatal("preflight against an unreachable host succeeded")
}
if strings.Contains(result.Output, secret) || strings.Contains(result.Error, secret) {
t.Errorf("credential leaked: output=%q error=%q", result.Output, result.Error)
}
}
func TestPushPreflightDoesNotRunLocalPrePushHook(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("pre-push hook is a POSIX shell script")
}
repoDir, _, _, operator := setupPushRemotesRepo(t)
marker := filepath.Join(t.TempDir(), "hook-ran")
hookPath := filepath.Join(repoDir, ".git", "hooks", "pre-push")
hookScript := "#!/bin/sh\ntouch " + marker + "\nexit 0\n"
if err := os.WriteFile(hookPath, []byte(hookScript), 0o755); err != nil {
t.Fatal(err)
}
result, err := operator.PushPreflight(context.Background(), PushOptions{Remote: "backup"})
if err != nil {
t.Fatalf("PushPreflight() error = %v", err)
}
if !result.Success {
t.Fatalf("PushPreflight() = %+v, want success", result)
}
if _, statErr := os.Stat(marker); statErr == nil {
t.Error("PushPreflight() ran the local pre-push hook, want it bypassed")
}
}
func TestPushPreflightReportsOperationInProgress(t *testing.T) {
_, _, _, operator := setupPushRemotesRepo(t)
if !operator.tryLock("test") {
t.Fatal("tryLock() = false, want the lock")
}
defer operator.unlock()
_, err := operator.PushPreflight(context.Background(), PushOptions{Remote: "backup"})
if err != ErrOperationInProgress {
t.Fatalf("PushPreflight() error = %v, want ErrOperationInProgress", err)
}
}
func TestPushPreflightAppliesSameRefusalOrdering(t *testing.T) {
_, _, backupDir, operator := setupPushRemotesRepo(t)
for _, tc := range []struct {
name string
opts PushOptions
want string
}{
{name: "upstream unsupported", opts: PushOptions{Remote: "backup", SetUpstream: true}, want: pushRemoteUpstreamUnsupportedErrorCode},
{name: "invalid expected branch", opts: PushOptions{ExpectedBranch: "bad..branch"}, want: pushBranchInvalidErrorCode},
{name: "unknown remote name", opts: PushOptions{Remote: "missing"}, want: pushRemoteNotFoundErrorCode},
{name: "unmatched remote url", opts: PushOptions{Remote: "https://example.test/nope.git"}, want: pushRemoteURLUnmatchedErrorCode},
{name: "branch mismatch", opts: PushOptions{ExpectedBranch: "feature/other"}, want: pushBranchMismatchErrorCode},
{name: "invalid branch outranks resolution", opts: PushOptions{Remote: "missing", ExpectedBranch: "bad..branch"}, want: pushBranchInvalidErrorCode},
} {
t.Run(tc.name, func(t *testing.T) {
result, err := operator.PushPreflight(context.Background(), tc.opts)
if err != nil {
t.Fatalf("PushPreflight() error = %v", err)
}
if result.ErrorCode != tc.want {
t.Errorf("ErrorCode = %q, want %q", result.ErrorCode, tc.want)
}
// Preflight publishes no baseline, so this code is unreachable.
if result.ErrorCode == pushBranchMismatchAfterBaselineErrorCode {
t.Error("preflight reported a post-baseline mismatch")
}
})
}
if got := remoteBranchSHA(t, backupDir, "feature/work"); got != "" {
t.Errorf("backup gained %q during refusals", got)
}
}
@@ -0,0 +1,441 @@
package process
import (
"context"
"errors"
"fmt"
"os/exec"
"sort"
"strings"
"github.com/kandev/kandev/internal/common/securityutil"
)
// Stable error codes for the configured-push-target contract. Callers match on
// these rather than on message text.
const (
pushRemoteNotFoundErrorCode = "push_remote_not_found"
pushRemoteURLUnmatchedErrorCode = "push_remote_url_unmatched"
pushRemoteFanoutErrorCode = "push_remote_fanout"
pushRemoteConfigUnreadableErrorCode = "push_remote_config_unreadable"
pushRemoteContributionConflictErrorCode = "push_remote_contribution_conflict"
pushRemoteUpstreamUnsupportedErrorCode = "push_remote_upstream_unsupported"
pushBranchInvalidErrorCode = "push_branch_invalid"
pushBranchMismatchErrorCode = "push_branch_mismatch"
pushBranchMismatchAfterBaselineErrorCode = "push_branch_mismatch_after_baseline"
pushBranchDetachedErrorCode = "push_branch_detached"
pushNoRemoteConfiguredErrorCode = "push_no_remote_configured"
)
// defaultPushRemote is the remote the push contract targets when a request
// names none and no contribution routing applies.
const defaultPushRemote = "origin"
// PushOptions carries the inputs of a push or push-preflight request.
//
// Remote is an explicit push target, given either as a configured remote name
// or as a remote URL that must resolve to an already-configured remote.
// ExpectedBranch is the branch the caller believes it is publishing. Both are
// optional; absent reproduces the behavior of a request that names neither.
type PushOptions struct {
Force bool
SetUpstream bool
Remote string
ExpectedBranch string
}
// normalized trims both string inputs. A value that is empty after the trim is
// absent, so a whitespace-only input is an omitted input rather than a refusal.
func (o PushOptions) normalized() PushOptions {
o.Remote = strings.TrimSpace(o.Remote)
o.ExpectedBranch = strings.TrimSpace(o.ExpectedBranch)
return o
}
// pushRefusal is a refusal carrying one of the stable error codes above.
type pushRefusal struct {
code string
message string
expectedBranch string
currentBranch string
baselinePublished bool
}
func (r *pushRefusal) apply(result *GitOperationResult) {
result.ErrorCode = r.code
result.Error = r.message
result.ExpectedBranch = r.expectedBranch
result.CurrentBranch = r.currentBranch
result.BaselinePublished = r.baselinePublished
}
// branchMismatchRefusal reports the expected and current branches so a caller
// can see what it asked for and what the checkout was actually on. A detached
// HEAD is reported as an empty current branch, not as the literal "HEAD".
func branchMismatchRefusal(expected, current string, baselinePublished bool) *pushRefusal {
code := pushBranchMismatchErrorCode
if baselinePublished {
code = pushBranchMismatchAfterBaselineErrorCode
}
shown := current
if shown == "" {
shown = "a detached HEAD"
}
return &pushRefusal{
code: code,
message: fmt.Sprintf("expected to publish %q but the checkout is on %s", expected, shown),
expectedBranch: expected,
currentBranch: current,
baselinePublished: baselinePublished,
}
}
// remoteConfig is a configured remote and its effective push URL set: the
// remote's push URLs when it has at least one, otherwise the single entry
// holding its fetch URL. A remote with neither has an empty set.
type remoteConfig struct {
name string
pushURLs []string
}
// configuredRemotes reads every configured remote and its effective push URL
// set. A remote whose name fails the ref allowlist is skipped: it cannot be
// named by the name form either, so it is unreachable through this contract in
// both directions.
func (g *GitOperator) configuredRemotes(ctx context.Context) ([]remoteConfig, error) {
output, err := g.runGitCommand(ctx, "remote")
if err != nil {
return nil, fmt.Errorf("failed to list remotes: %w", err)
}
var remotes []remoteConfig
for _, line := range strings.Split(output, "\n") {
name := strings.TrimSpace(line)
if name == "" || !securityutil.IsValidBranchName(name) {
continue
}
urls, err := g.effectivePushURLs(ctx, name)
if err != nil {
return nil, err
}
remotes = append(remotes, remoteConfig{name: name, pushURLs: urls})
}
return remotes, nil
}
func (g *GitOperator) effectivePushURLs(ctx context.Context, name string) ([]string, error) {
pushURLs, err := g.remoteConfigValues(ctx, name, "pushurl")
if err != nil {
return nil, err
}
if len(pushURLs) > 0 {
return pushURLs, nil
}
return g.remoteConfigValues(ctx, name, "url")
}
// remoteConfigValues reads one multi-valued remote config key. Git exits 1 when
// the key is simply unset, which is an empty set rather than a read failure.
func (g *GitOperator) remoteConfigValues(ctx context.Context, name, key string) ([]string, error) {
output, err := g.runGitCommand(ctx, "config", "--get-all", "remote."+name+"."+key)
if err != nil {
if isGitConfigKeyUnset(err) {
return nil, nil
}
return nil, fmt.Errorf("failed to read remote.%s.%s: %w", name, key, err)
}
var values []string
for _, line := range strings.Split(output, "\n") {
if trimmed := strings.TrimSpace(line); trimmed != "" {
values = append(values, trimmed)
}
}
return values, nil
}
func isGitConfigKeyUnset(err error) bool {
var exitErr *exec.ExitError
return errors.As(err, &exitErr) && exitErr.ExitCode() == 1
}
// resolvePushTarget maps an explicit push target onto a configured remote name.
// Resolution never mutates configuration and never places a caller-supplied URL
// on a git command line: only the resolved name reaches a command.
func (g *GitOperator) resolvePushTarget(ctx context.Context, target string) (string, *pushRefusal) {
remotes, err := g.configuredRemotes(ctx)
if err != nil {
return "", &pushRefusal{
code: pushRemoteConfigUnreadableErrorCode,
message: "failed to read the checkout's remote configuration",
}
}
// The permissive ref allowlist is the name/URL discriminator. A value read
// as a name still has to match an already-configured remote, so a
// permissive reading cannot reach a destination on its own.
if securityutil.IsValidBranchName(target) {
for _, remote := range remotes {
if remote.name == target {
return remote.name, nil
}
}
return "", &pushRefusal{
code: pushRemoteNotFoundErrorCode,
message: fmt.Sprintf("no configured remote named %q", target),
}
}
return resolvePushTargetURL(remotes, target)
}
// resolvePushTargetURL matches a URL against each remote's effective push URL
// set. Only a single-entry set matches: a remote that carries the value
// alongside other push URLs would publish to destinations the caller never
// named, so it is refused rather than matched. A caller that wants that
// fan-out names the remote instead.
func resolvePushTargetURL(remotes []remoteConfig, target string) (string, *pushRefusal) {
var matches []string
fanout := false
for _, remote := range remotes {
if len(remote.pushURLs) == 1 && remote.pushURLs[0] == target {
matches = append(matches, remote.name)
continue
}
for _, url := range remote.pushURLs {
if url == target {
fanout = true
break
}
}
}
if len(matches) > 0 {
// Several remotes can legitimately carry the same URL; they all address
// the same destination, so first by byte order is well defined.
sort.Strings(matches)
return matches[0], nil
}
if fanout {
return "", &pushRefusal{
code: pushRemoteFanoutErrorCode,
message: "the requested push URL belongs to a remote that publishes to additional URLs; name the remote instead",
}
}
return "", &pushRefusal{
code: pushRemoteURLUnmatchedErrorCode,
message: "no configured remote publishes to the requested URL",
}
}
// currentBranch reads HEAD as a symbolic ref so that a detached HEAD, reported
// as an empty branch, is distinguishable from a branch literally named "HEAD".
// rev-parse --abbrev-ref returns that literal for a detached checkout and so
// cannot make this distinction.
func (g *GitOperator) currentBranch(ctx context.Context) (string, error) {
output, err := g.runGitCommand(ctx, "symbolic-ref", "HEAD")
if err == nil {
ref := strings.TrimSpace(output)
if !strings.HasPrefix(ref, "refs/heads/") {
return "", nil
}
return strings.TrimPrefix(ref, "refs/heads/"), nil
}
// symbolic-ref fails both for a detached HEAD and for an unreadable
// checkout. A resolvable HEAD commit distinguishes the two.
if _, headErr := g.runGitCommand(ctx, "rev-parse", "--verify", "HEAD"); headErr == nil {
return "", nil
}
return "", fmt.Errorf("failed to read HEAD: %w", err)
}
// pushPlan is the destination a push or preflight resolved to, together with
// the refspec that reaches it.
type pushPlan struct {
remote string
branch string
refspec string
// explicit records that the request named a push target. A push reports
// the destination fields only in that case, so a request naming none keeps
// the result shape it has today.
explicit bool
// routed records that contribution routing selected the destination.
routed bool
// baselineEligible records that empty-remote first publication applies:
// the origin path with no contribution routing.
baselineEligible bool
}
// contributionRouted reports whether a contribution binding selects the
// destination for this operator.
func (g *GitOperator) contributionRouted() bool {
return g.contributionDestination != nil || g.remoteContribution != nil
}
// resolvePushPlan evaluates the refusal order from the contribution conflict
// through the first expected-branch verification, then yields the destination.
// Every refusal it returns is raised before any remote is contacted, so a
// refused request leaves the checkout, its refs, and every remote unchanged.
// requireConfiguredRemote is set by preflight, which answers whether a
// destination is writable and so must report a missing default remote. A push
// leaves that to the push command itself.
func (g *GitOperator) resolvePushPlan(
ctx context.Context, opts PushOptions, requireConfiguredRemote bool,
) (*pushPlan, *pushRefusal) {
if refusal := g.refuseIncompatibleOptions(opts); refusal != nil {
return nil, refusal
}
remote := ""
if opts.Remote != "" {
resolved, refusal := g.resolvePushTarget(ctx, opts.Remote)
if refusal != nil {
return nil, refusal
}
remote = resolved
}
if requireConfiguredRemote && remote == "" && !g.contributionRouted() {
if refusal := g.requireDefaultRemote(ctx); refusal != nil {
return nil, refusal
}
}
current, err := g.currentBranch(ctx)
if err != nil {
return nil, &pushRefusal{message: err.Error()}
}
if remote != "" && opts.ExpectedBranch == "" && current == "" {
return nil, &pushRefusal{
code: pushBranchDetachedErrorCode,
message: "cannot publish a detached HEAD without an expected branch",
}
}
if opts.ExpectedBranch != "" && opts.ExpectedBranch != current {
return nil, branchMismatchRefusal(opts.ExpectedBranch, current, false)
}
return g.buildPushPlan(ctx, opts, remote, current)
}
// refuseIncompatibleOptions covers the request-shape refusals, which are the
// cheapest and so are evaluated before any configuration or checkout read.
func (g *GitOperator) refuseIncompatibleOptions(opts PushOptions) *pushRefusal {
if opts.Remote != "" && g.contributionRouted() {
return &pushRefusal{
code: pushRemoteContributionConflictErrorCode,
message: "an explicit push target cannot be combined with a configured contribution",
}
}
if opts.Remote != "" && opts.SetUpstream {
return &pushRefusal{
code: pushRemoteUpstreamUnsupportedErrorCode,
message: "an explicit push target cannot set upstream tracking",
}
}
if opts.ExpectedBranch != "" && !securityutil.IsValidExpectedBranchName(opts.ExpectedBranch) {
return &pushRefusal{
code: pushBranchInvalidErrorCode,
message: fmt.Sprintf("invalid expected branch %q", opts.ExpectedBranch),
}
}
return nil
}
// buildPushPlan selects the destination and refspec. The contribution paths
// keep the refspec and branch read they use today; only the explicit-target
// path is new.
func (g *GitOperator) buildPushPlan(ctx context.Context, opts PushOptions, remote, current string) (*pushPlan, *pushRefusal) {
if remote != "" {
branch := opts.ExpectedBranch
if branch == "" {
branch = current
}
return &pushPlan{
remote: remote,
branch: branch,
refspec: "HEAD:refs/heads/" + branch,
explicit: true,
baselineEligible: remote == defaultPushRemote,
}, nil
}
// The paths below name no explicit target. With no expected branch they
// must behave exactly as they do today, including reading the branch
// through getCurrentBranch. With an expected branch, that value is used
// directly instead of a separate read: verifyExpectedBranch confirms HEAD
// is still on it immediately before the push runs, and a refspec built
// from an earlier, independent read could name a branch that check never
// saw.
branch := opts.ExpectedBranch
if branch == "" {
var err error
branch, err = g.getCurrentBranch(ctx)
if err != nil {
return nil, &pushRefusal{message: err.Error()}
}
}
switch {
case g.contributionDestination != nil:
return &pushPlan{
remote: g.contributionDestination.ContributionRemoteName(),
branch: branch,
refspec: branch,
routed: true,
}, nil
case g.remoteContribution != nil:
return &pushPlan{
remote: g.remoteContribution.ContributionRemoteName(),
branch: g.remoteContribution.HeadBranch,
refspec: "HEAD:refs/heads/" + g.remoteContribution.HeadBranch,
routed: true,
}, nil
default:
return &pushPlan{
remote: defaultPushRemote,
branch: branch,
refspec: branch,
baselineEligible: true,
}, nil
}
}
// verifyExpectedBranch is the second verification. It is the last git read
// before the push command, so nothing Kandev issues can move HEAD between the
// check and the push.
func (g *GitOperator) verifyExpectedBranch(ctx context.Context, expected string, baselinePublished bool) *pushRefusal {
if expected == "" {
return nil
}
current, err := g.currentBranch(ctx)
if err != nil {
return &pushRefusal{message: err.Error(), baselinePublished: baselinePublished}
}
if current != expected {
return branchMismatchRefusal(expected, current, baselinePublished)
}
return nil
}
// reportDestination populates the destination fields a successful push reports.
// A push that named no explicit target omits them so an existing consumer sees
// an unchanged result shape.
func (p *pushPlan) reportDestination(result *GitOperationResult) {
if !p.explicit {
return
}
result.PushedRemote = p.remote
result.PushedBranch = p.branch
}
// requireDefaultRemote reports a checkout that has nowhere to publish, ahead of
// the branch guards so a caller learns that rather than a branch it cannot act
// on yet.
func (g *GitOperator) requireDefaultRemote(ctx context.Context) *pushRefusal {
remotes, err := g.configuredRemotes(ctx)
if err != nil {
return &pushRefusal{
code: pushRemoteConfigUnreadableErrorCode,
message: "failed to read the checkout's remote configuration",
}
}
for _, remote := range remotes {
if remote.name == defaultPushRemote {
return nil
}
}
return &pushRefusal{
code: pushNoRemoteConfiguredErrorCode,
message: "the checkout has no " + defaultPushRemote + " remote to publish to",
}
}
@@ -0,0 +1,219 @@
package process
import (
"context"
"os"
"path/filepath"
"testing"
)
// setupPushTargetRepo builds a checkout with one commit on "feature/work" and
// no remotes. Tests add exactly the remotes they need.
func setupPushTargetRepo(t *testing.T) (string, *GitOperator) {
t.Helper()
root := t.TempDir()
repoDir := filepath.Join(root, "repo")
runGit(t, root, "init", "-b", "main", repoDir)
runGit(t, repoDir, "config", "user.name", "Task User")
runGit(t, repoDir, "config", "user.email", "task@example.com")
if err := os.WriteFile(filepath.Join(repoDir, "seed.txt"), []byte("seed\n"), 0o644); err != nil {
t.Fatal(err)
}
runGit(t, repoDir, "add", "seed.txt")
runGit(t, repoDir, "commit", "-m", "seed")
runGit(t, repoDir, "checkout", "-b", "feature/work")
tracker := NewWorkspaceTracker(repoDir, newTestLogger(t))
tracker.SetBaseBranch("main")
return repoDir, NewGitOperator(repoDir, newTestLogger(t), tracker)
}
func TestResolvePushTargetTrimsAndTreatsEmptyAsAbsent(t *testing.T) {
for _, tc := range []struct {
name string
opts PushOptions
remote string
expected string
}{
{name: "surrounding whitespace", opts: PushOptions{Remote: " backup ", ExpectedBranch: " main "}, remote: "backup", expected: "main"},
{name: "whitespace only is absent", opts: PushOptions{Remote: " ", ExpectedBranch: "\t\n"}, remote: "", expected: ""},
{name: "empty stays absent", opts: PushOptions{}, remote: "", expected: ""},
} {
t.Run(tc.name, func(t *testing.T) {
got := tc.opts.normalized()
if got.Remote != tc.remote {
t.Errorf("Remote = %q, want %q", got.Remote, tc.remote)
}
if got.ExpectedBranch != tc.expected {
t.Errorf("ExpectedBranch = %q, want %q", got.ExpectedBranch, tc.expected)
}
})
}
}
func TestResolvePushTargetDiscriminatesNameFromURL(t *testing.T) {
repoDir, operator := setupPushTargetRepo(t)
runGit(t, repoDir, "remote", "add", "backup", "https://example.test/backup.git")
name, refusal := operator.resolvePushTarget(context.Background(), "backup")
if refusal != nil {
t.Fatalf("name form refused: %+v", refusal)
}
if name != "backup" {
t.Errorf("name form resolved to %q, want %q", name, "backup")
}
name, refusal = operator.resolvePushTarget(context.Background(), "https://example.test/backup.git")
if refusal != nil {
t.Fatalf("URL form refused: %+v", refusal)
}
if name != "backup" {
t.Errorf("URL form resolved to %q, want %q", name, "backup")
}
}
func TestResolvePushTargetRejectsCaseAndPrefixVariants(t *testing.T) {
repoDir, operator := setupPushTargetRepo(t)
runGit(t, repoDir, "remote", "add", "backup", "https://example.test/backup.git")
for _, target := range []string{"Backup", "BACKUP", "back", "backup2"} {
t.Run(target, func(t *testing.T) {
_, refusal := operator.resolvePushTarget(context.Background(), target)
if refusal == nil {
t.Fatalf("resolvePushTarget(%q) matched; want refusal", target)
}
if refusal.code != pushRemoteNotFoundErrorCode {
t.Errorf("code = %q, want %q", refusal.code, pushRemoteNotFoundErrorCode)
}
})
}
}
func TestResolvePushTargetReportsUnknownRemoteName(t *testing.T) {
_, operator := setupPushTargetRepo(t)
_, refusal := operator.resolvePushTarget(context.Background(), "missing")
if refusal == nil || refusal.code != pushRemoteNotFoundErrorCode {
t.Fatalf("refusal = %+v, want %q", refusal, pushRemoteNotFoundErrorCode)
}
}
func TestResolvePushTargetMatchesSingleEntryPushURL(t *testing.T) {
repoDir, operator := setupPushTargetRepo(t)
// pushurl shadows url, so the fetch URL must not match once a push URL exists.
runGit(t, repoDir, "remote", "add", "backup", "https://example.test/fetch.git")
runGit(t, repoDir, "config", "remote.backup.pushurl", "https://example.test/push.git")
name, refusal := operator.resolvePushTarget(context.Background(), "https://example.test/push.git")
if refusal != nil {
t.Fatalf("push URL refused: %+v", refusal)
}
if name != "backup" {
t.Errorf("resolved %q, want %q", name, "backup")
}
_, refusal = operator.resolvePushTarget(context.Background(), "https://example.test/fetch.git")
if refusal == nil || refusal.code != pushRemoteURLUnmatchedErrorCode {
t.Fatalf("shadowed fetch URL refusal = %+v, want %q", refusal, pushRemoteURLUnmatchedErrorCode)
}
}
func TestResolvePushTargetPicksFirstByByteOrder(t *testing.T) {
repoDir, operator := setupPushTargetRepo(t)
shared := "https://example.test/shared.git"
runGit(t, repoDir, "remote", "add", "zulu", shared)
runGit(t, repoDir, "remote", "add", "alpha", shared)
runGit(t, repoDir, "remote", "add", "mike", shared)
name, refusal := operator.resolvePushTarget(context.Background(), shared)
if refusal != nil {
t.Fatalf("shared URL refused: %+v", refusal)
}
if name != "alpha" {
t.Errorf("resolved %q, want %q", name, "alpha")
}
}
func TestResolvePushTargetReportsUnmatchedURL(t *testing.T) {
repoDir, operator := setupPushTargetRepo(t)
runGit(t, repoDir, "remote", "add", "backup", "https://example.test/backup.git")
_, refusal := operator.resolvePushTarget(context.Background(), "https://example.test/other.git")
if refusal == nil || refusal.code != pushRemoteURLUnmatchedErrorCode {
t.Fatalf("refusal = %+v, want %q", refusal, pushRemoteURLUnmatchedErrorCode)
}
}
func TestResolvePushTargetReportsFanoutRemote(t *testing.T) {
repoDir, operator := setupPushTargetRepo(t)
runGit(t, repoDir, "remote", "add", "mirror", "https://example.test/a.git")
runGit(t, repoDir, "config", "--add", "remote.mirror.pushurl", "https://example.test/a.git")
runGit(t, repoDir, "config", "--add", "remote.mirror.pushurl", "https://example.test/b.git")
_, refusal := operator.resolvePushTarget(context.Background(), "https://example.test/a.git")
if refusal == nil || refusal.code != pushRemoteFanoutErrorCode {
t.Fatalf("refusal = %+v, want %q", refusal, pushRemoteFanoutErrorCode)
}
// A matching single-entry remote takes precedence over the fan-out refusal.
runGit(t, repoDir, "remote", "add", "direct", "https://example.test/a.git")
name, refusal := operator.resolvePushTarget(context.Background(), "https://example.test/a.git")
if refusal != nil {
t.Fatalf("single-entry remote refused: %+v", refusal)
}
if name != "direct" {
t.Errorf("resolved %q, want %q", name, "direct")
}
}
func TestResolvePushTargetReportsUnreadableRemoteConfig(t *testing.T) {
repoDir, operator := setupPushTargetRepo(t)
runGit(t, repoDir, "remote", "add", "backup", "https://example.test/backup.git")
// A config file git cannot parse must not read as "no such remote".
if err := os.WriteFile(filepath.Join(repoDir, ".git", "config"), []byte("[remote \"backup\"\n"), 0o644); err != nil {
t.Fatal(err)
}
_, refusal := operator.resolvePushTarget(context.Background(), "backup")
if refusal == nil || refusal.code != pushRemoteConfigUnreadableErrorCode {
t.Fatalf("refusal = %+v, want %q", refusal, pushRemoteConfigUnreadableErrorCode)
}
}
func TestGitOperatorCurrentBranchReportsDetachedHeadAsEmpty(t *testing.T) {
repoDir, operator := setupPushTargetRepo(t)
branch, err := operator.currentBranch(context.Background())
if err != nil {
t.Fatalf("currentBranch() error = %v", err)
}
if branch != "feature/work" {
t.Errorf("branch = %q, want %q", branch, "feature/work")
}
runGit(t, repoDir, "checkout", "--detach", "HEAD")
branch, err = operator.currentBranch(context.Background())
if err != nil {
t.Fatalf("currentBranch() on detached HEAD error = %v", err)
}
if branch != "" {
t.Errorf("detached branch = %q, want empty", branch)
}
}
// TestGitOperatorCurrentBranchReportsSymbolicRefOutsideHeadsAsEmpty covers a
// HEAD that resolves as a symbolic ref, but not under refs/heads/. Detached
// state is determined by whether HEAD resolves under refs/heads/, not merely
// by whether it fails to resolve at all.
func TestGitOperatorCurrentBranchReportsSymbolicRefOutsideHeadsAsEmpty(t *testing.T) {
repoDir, operator := setupPushTargetRepo(t)
runGit(t, repoDir, "symbolic-ref", "HEAD", "refs/remotes/origin/main")
branch, err := operator.currentBranch(context.Background())
if err != nil {
t.Fatalf("currentBranch() on a non-refs/heads symbolic ref error = %v", err)
}
if branch != "" {
t.Errorf("branch = %q, want empty: HEAD does not resolve under refs/heads/", branch)
}
}
@@ -83,7 +83,7 @@ func TestGitOperatorRemoteContributionRoutesPushesAndPreflightToSource(t *testin
operator := NewGitOperator(repoDir, newTestLogger(t), nil)
operator.setRemoteContribution(binding)
preflight, err := operator.PushPreflight(context.Background())
preflight, err := operator.PushPreflight(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("PushPreflight returned error: %v", err)
}
@@ -93,8 +93,11 @@ func TestGitOperatorRemoteContributionRoutesPushesAndPreflightToSource(t *testin
if got := strings.TrimSpace(runGit(t, originDir, "rev-parse", "refs/heads/feature/contribution")); got != headSHA {
t.Fatalf("preflight changed source branch: %q != %q", got, headSHA)
}
if preflight.PushedRemote != "" || preflight.PushedBranch != "" {
t.Fatalf("contribution-routed preflight = %+v, want PushedRemote and PushedBranch omitted", preflight)
}
forced, err := operator.Push(context.Background(), true, false)
forced, err := operator.Push(context.Background(), PushOptions{Force: true})
if err != nil {
t.Fatalf("forced Push returned error: %v", err)
}
@@ -102,7 +105,7 @@ func TestGitOperatorRemoteContributionRoutesPushesAndPreflightToSource(t *testin
t.Fatalf("forced Push = %+v, want a contribution force-push rejection", forced)
}
pushed, err := operator.Push(context.Background(), false, false)
pushed, err := operator.Push(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("Push returned error: %v", err)
}
@@ -218,7 +221,7 @@ func TestGitOperatorContributionDestinationPushesWithoutChangingPullRemote(t *te
operator := NewGitOperator(repoDir, newTestLogger(t), nil)
operator.setContributionDestination(destination)
pushed, err := operator.Push(context.Background(), false, false)
pushed, err := operator.Push(context.Background(), PushOptions{})
if err != nil || !pushed.Success {
t.Fatalf("Push = %+v, err = %v", pushed, err)
}
@@ -260,7 +263,7 @@ func TestGitOperatorPush_PreservesExistingUpstream(t *testing.T) {
}
gitOp := NewGitOperator(suffixedDir, log, nil)
result, err := gitOp.Push(context.Background(), false, false)
result, err := gitOp.Push(context.Background(), PushOptions{})
if err != nil {
t.Fatalf("Push returned error: %v", err)
}
@@ -86,6 +86,21 @@ func IsValidDefaultBranchName(branch string) bool {
return IsValidBaseBranchRef(branch) && !IsGitSymbolicRef(branch)
}
// IsValidExpectedBranchName validates a caller-supplied expected branch: the
// IsValidBranchName allowlist plus git check-ref-format's rejection of a
// trailing "/" and consecutive "//", plus a reject on git's symbolic
// pseudo-refs. It deliberately does not route through IsValidBaseBranchRef,
// which strips an "origin/" prefix and so validates a different string than
// the caller supplied. An expected branch is used verbatim, both in the
// comparison against HEAD and in the push refspec, so the value that is
// checked must be the value that is used.
func IsValidExpectedBranchName(branch string) bool {
if strings.HasSuffix(branch, "/") || strings.Contains(branch, "//") {
return false
}
return IsValidBranchName(branch) && !IsGitSymbolicRef(branch)
}
// IsKnownSafeGitFlag returns true if the argument is a known safe git flag used by this codebase.
// This prevents argument injection where user input could introduce malicious flags.
// Only flags actually used by the Kandev codebase are whitelisted.
@@ -111,7 +126,7 @@ func IsKnownSafeGitFlag(arg string) bool {
"--format", "--format=", "--stat", "--shortstat", "--numstat", "-p", "-A",
"--amend", "--allow-empty", "--soft", "--mixed", "--hard",
"--cached", "--force", "--source=HEAD", "--staged", "--worktree",
"--dry-run", "--get-all", "--first-parent", "--is-ancestor",
"--dry-run", "--no-verify", "--get-all", "--first-parent", "--is-ancestor",
"--refs",
"--src-prefix=", "--dst-prefix=",
}
@@ -120,3 +120,37 @@ func TestIsKnownSafeGitFlagRejectsRebaseAndAbortVariants(t *testing.T) {
}
}
}
func TestIsValidExpectedBranchName(t *testing.T) {
for _, tc := range []struct {
branch string
want bool
}{
{branch: "main", want: true},
{branch: "feature/work", want: true},
{branch: "release/v1.2.3", want: true},
{branch: "", want: false},
{branch: "HEAD", want: false},
{branch: "ORIG_HEAD", want: false},
{branch: "FETCH_HEAD", want: false},
{branch: "MERGE_HEAD", want: false},
{branch: "main/", want: false},
{branch: "a//b", want: false},
{branch: "a..b", want: false},
{branch: "main.lock", want: false},
{branch: "-flag", want: false},
{branch: "has space", want: false},
// A local branch may legally carry this name, and it is validated as
// itself rather than as the "main" IsValidBaseBranchRef would strip to.
{branch: "origin/main", want: true},
// Distinguishes this validator from IsValidBaseBranchRef, which strips
// the prefix and then rejects the leading dash of the remainder.
{branch: "origin/-dash", want: true},
} {
t.Run(tc.branch, func(t *testing.T) {
if got := IsValidExpectedBranchName(tc.branch); got != tc.want {
t.Errorf("IsValidExpectedBranchName(%q) = %v, want %v", tc.branch, got, tc.want)
}
})
}
}