diff --git a/acceptance/sidecar_test.go b/acceptance/sidecar_test.go index 93ff4f48..2a394cfe 100644 --- a/acceptance/sidecar_test.go +++ b/acceptance/sidecar_test.go @@ -365,7 +365,11 @@ func TestSidecarsSshSyncFlags(t *testing.T) { env := testenv.NewTestEnv(t) env.CircleCIURL = srv.URL - result := binary.RunCLI(t, tt.args, env, env.HomeDir) + // sync runs git operations (branch detection) before opening SSH, so + // the working directory must be a git repo. + workDir := gitrepo.SetupGitRepo(t, "test-org", "test-repo") + + result := binary.RunCLI(t, tt.args, env, workDir) // Commands should fail at SSH key step, not at flag parsing assert.Assert(t, result.ExitCode != 0, "expected non-zero exit (SSH fails)") diff --git a/internal/cmd/sidecar.go b/internal/cmd/sidecar.go index 987e3287..ae66ad39 100644 --- a/internal/cmd/sidecar.go +++ b/internal/cmd/sidecar.go @@ -409,7 +409,11 @@ func newSidecarSyncCmd() *cobra.Command { if err != nil { return err } - err = sidecar.Sync(cmd.Context(), client, sidecarID, identityFile, authSock, workdir, newStatusFunc(io)) + cwd, err := os.Getwd() + if err != nil { + return fmt.Errorf("sync: %w", err) + } + err = sidecar.Sync(cmd.Context(), client, sidecarID, identityFile, authSock, workdir, cwd, newStatusFunc(io)) if err != nil { if _, ok := errors.AsType[*sidecar.RemoteBaseError](err); ok { return &userError{ @@ -827,7 +831,7 @@ Example: // Step 4: Sync files to sidecar. if !skipSync { - if err := sidecarSetupSync(cmd.Context(), client, sidecarID, identityFile, authSock, status); err != nil { + if err := sidecarSetupSync(cmd.Context(), client, sidecarID, identityFile, authSock, dir, status); err != nil { return err } } @@ -917,11 +921,11 @@ func sidecarSetupEnsureSSHKey(identityFile string, status iostream.StatusFunc) e func sidecarSetupSync( ctx context.Context, client *circleci.Client, - sidecarID, identityFile, authSock string, + sidecarID, identityFile, authSock, cwd string, status iostream.StatusFunc, ) error { status(iostream.LevelStep, "Syncing files to sidecar...") - err := sidecar.Sync(ctx, client, sidecarID, identityFile, authSock, "", status) + err := sidecar.Bootstrap(ctx, client, sidecarID, identityFile, authSock, "", cwd, status) if err == nil { return nil } diff --git a/internal/gitutil/gitutil.go b/internal/gitutil/gitutil.go index 5bf011c6..2f4fccc9 100644 --- a/internal/gitutil/gitutil.go +++ b/internal/gitutil/gitutil.go @@ -60,17 +60,39 @@ func MergeBase() (string, error) { } } - out, err = exec.Command("git", "rev-parse", "origin/HEAD").Output() + out, err = exec.Command("git", "merge-base", "HEAD", "origin/HEAD").Output() if err != nil { return "", fmt.Errorf("resolve remote base: no upstream tracking branch or origin/HEAD found") } sha := strings.TrimSpace(string(out)) if sha == "" { - return "", fmt.Errorf("resolve remote base: origin/HEAD is empty") + return "", fmt.Errorf("resolve remote base: could not determine merge-base with origin/HEAD") } return sha, nil } +// HeadRef returns the SHA of the current HEAD commit in the repo at cwd. +func HeadRef(cwd string) (string, error) { + cmd := exec.Command("git", "rev-parse", "HEAD") + cmd.Dir = cwd + out, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("resolve HEAD: %w", err) + } + sha := strings.TrimSpace(string(out)) + if sha == "" { + return "", fmt.Errorf("resolve HEAD: empty output") + } + return sha, nil +} + +// IsAncestor returns true if candidate is an ancestor of head in the repo at cwd. +func IsAncestor(candidate, head, cwd string) bool { + cmd := exec.Command("git", "merge-base", "--is-ancestor", candidate, head) + cmd.Dir = cwd + return cmd.Run() == nil +} + // GeneratePatch generates a binary diff from the given base commit, // including untracked files. It temporarily stages untracked files // with git add -N and resets them after generating the diff. diff --git a/internal/sidecar/active.go b/internal/sidecar/active.go index 7d48bb31..1d8ce7f1 100644 --- a/internal/sidecar/active.go +++ b/internal/sidecar/active.go @@ -13,9 +13,11 @@ import ( // ActiveSidecar holds the currently active sidecar for a project. type ActiveSidecar struct { - SidecarID string `json:"sidecar_id"` - Name string `json:"name,omitempty"` - Workspace string `json:"workspace,omitempty"` + SidecarID string `json:"sidecar_id"` + Name string `json:"name,omitempty"` + Workspace string `json:"workspace,omitempty"` + BaselineRef string `json:"baseline_ref,omitempty"` // commit SHA checked out on sidecar + BaselineBranch string `json:"baseline_branch,omitempty"` // local branch name at bootstrap time } // sidecarFileName returns the name of the sidecar state file. When sessionID diff --git a/internal/sidecar/sync.go b/internal/sidecar/sync.go index 5d491c9b..cd42101d 100644 --- a/internal/sidecar/sync.go +++ b/internal/sidecar/sync.go @@ -2,9 +2,7 @@ package sidecar import ( "context" - "errors" "fmt" - "os" "path/filepath" "strings" @@ -45,159 +43,216 @@ func persistWorkspace(ctx context.Context, workspace string) error { return SaveActive(ctx, *active) } -// Sync synchronises local changes to a sidecar over SSH. -// It ensures the workspace base exists, clones the repo into workdir if absent, -// then resets to the remote base and applies a patch of local changes. -// workdir overrides the destination path; defaults to /workspace/. -func Sync(ctx context.Context, - client *circleci.Client, sidecarID, identityFile, authSock, workdir string, status iostream.StatusFunc) error { +// inRepo builds a remote shell command that runs gitCmd inside repoPath. +// Uses double-quote wrapping so that ShellEscape (single-quoted) paths inside +// work correctly on the sidecar's SSH server. +func inRepo(repoPath, gitCmd string) string { + return fmt.Sprintf(`sh -c "cd %s && %s"`, ShellEscape(repoPath), gitCmd) +} + +// Bootstrap clones (or updates) the sidecar workspace from GitHub, checks out +// the best available local HEAD commit, applies a patch to reach exact local +// state, and persists BaselineRef + BaselineBranch. +// +// If the branch is pushed, the sidecar lands on the exact local HEAD and the +// patch covers only uncommitted working-tree changes. If the branch is not +// pushed, the sidecar lands on the merge-base and the patch carries all +// unpushed commits as file changes. +func Bootstrap(ctx context.Context, + client *circleci.Client, sidecarID, identityFile, authSock, workdir, cwd string, status iostream.StatusFunc) error { session, err := OpenSession(ctx, client, sidecarID, identityFile, authSock) if err != nil { return err } - cwd, err := os.Getwd() - if err != nil { - return fmt.Errorf("sync: %w", err) - } - org, repo, err := gitremote.DetectOrgAndRepo(cwd) if err != nil { - return fmt.Errorf("sync: %w", err) + return fmt.Errorf("bootstrap: %w", err) } repoPath := ResolveWorkspace(ctx, workdir, repo) - if err := persistWorkspace(ctx, repoPath); err != nil { status(iostream.LevelWarn, fmt.Sprintf("Could not save workspace: %v", err)) } - // Try once and exit here if it worked - err = syncWorkspace(ctx, status, org, repo, repoPath, session) - if err == nil { - status(iostream.LevelDone, "Synced") - return nil - } - // We should only try again if the failure was in the apply phase. - if !errors.Is(err, errApplyFailed) { - return err + // Ensure the parent directory exists on the sidecar. + parentDir := filepath.Dir(repoPath) + if result, err := ExecOverSSH(ctx, session, "mkdir -p "+ShellEscape(parentDir), nil, nil); err != nil { + return fmt.Errorf("bootstrap: mkdir: %w", err) + } else if result.ExitCode != 0 { + return fmt.Errorf("bootstrap: mkdir -p %s: %s", parentDir, result.Stderr) } - // Second attempt - after deleting the remote folder - status(iostream.LevelWarn, fmt.Sprintf("Local %s/%s drifted from remote: %s (%s) - attempting clean", - org, repo, repoPath, err)) - - // Delete the remote working directory - if result, err := ExecOverSSH(ctx, session, "rm -rf "+ShellEscape(repoPath), nil, nil); err != nil { - return fmt.Errorf("sync: rm %s: %w", repoPath, err) - } else if result.ExitCode != 0 { - return fmt.Errorf("sync: rm %s: %s", repoPath, result.Stderr) + // Clone if absent, otherwise fetch to bring the clone up to date. + testResult, err := ExecOverSSH(ctx, session, "test -d "+ShellEscape(repoPath), nil, nil) + if err != nil { + return fmt.Errorf("bootstrap: check repo dir: %w", err) + } + repoURL := fmt.Sprintf("https://github.com/%s/%s.git", org, repo) + if testResult.ExitCode != 0 { + status(iostream.LevelInfo, fmt.Sprintf("Cloning %s/%s into %s...", org, repo, repoPath)) + cloneCmd := fmt.Sprintf("git clone %s %s", ShellEscape(repoURL), ShellEscape(repoPath)) + if result, err := ExecOverSSH(ctx, session, cloneCmd, nil, nil); err != nil { + return fmt.Errorf("bootstrap: clone: %w", err) + } else if result.ExitCode != 0 { + return fmt.Errorf("bootstrap: clone failed: %s", result.Stderr) + } + } else { + status(iostream.LevelInfo, fmt.Sprintf("Updating %s/%s at %s...", org, repo, repoPath)) + if result, err := ExecOverSSH(ctx, session, inRepo(repoPath, "git fetch origin"), nil, nil); err != nil { + return fmt.Errorf("bootstrap: fetch: %w", err) + } else if result.ExitCode != 0 { + return fmt.Errorf("bootstrap: fetch failed: %s", result.Stderr) + } } - if err := syncWorkspace(ctx, status, org, repo, repoPath, session); err != nil { - return fmt.Errorf("sync retry: %w", err) + // Resolve the checkout target: exact HEAD when pushed, merge-base otherwise. + branch, err := gitutil.CurrentBranch() + if err != nil { + return fmt.Errorf("bootstrap: %w", err) + } + headSHA, err := gitutil.HeadRef(cwd) + if err != nil { + return fmt.Errorf("bootstrap: %w", err) } - status(iostream.LevelDone, "Synced") - return nil -} + checkoutSHA := headSHA + if !gitutil.IsBranchPushed() { + status(iostream.LevelInfo, "Branch not pushed; checking out merge-base instead.") + checkoutSHA, err = gitutil.MergeBase() + if err != nil { + return &RemoteBaseError{Err: err} + } + } -var errApplyFailed = errors.New("apply failed") + // Reset and clean before checkout so a dirty working tree never blocks it. + if result, err := ExecOverSSH(ctx, session, inRepo(repoPath, "git reset --hard HEAD && git clean -fd"), nil, nil); err != nil { + return fmt.Errorf("bootstrap: pre-checkout reset: %w", err) + } else if result.ExitCode != 0 { + return fmt.Errorf("bootstrap: pre-checkout reset: %s", result.Stderr) + } -func syncWorkspace(ctx context.Context, status iostream.StatusFunc, org, repo, repoPath string, session *Session) error { - status(iostream.LevelInfo, fmt.Sprintf("Assessing %s/%s on remote: %s...", org, repo, repoPath)) + if result, err := ExecOverSSH(ctx, session, inRepo(repoPath, "git checkout "+checkoutSHA), nil, nil); err != nil { + return fmt.Errorf("bootstrap: checkout: %w", err) + } else if result.ExitCode != 0 { + return fmt.Errorf("bootstrap: checkout %s: %s", checkoutSHA, result.Stderr) + } - // Ensure the parent directory exists on the sidecar. - parentDir := filepath.Dir(repoPath) - if result, err := ExecOverSSH(ctx, session, "mkdir -p "+ShellEscape(parentDir), nil, nil); err != nil { - return fmt.Errorf("sync: mkdir %s: %w", parentDir, err) + // Reset to the exact SHA and clean (handles any detached HEAD drift). + if result, err := ExecOverSSH(ctx, session, inRepo(repoPath, "git reset --hard "+checkoutSHA+" && git clean -fd"), nil, nil); err != nil { + return fmt.Errorf("bootstrap: reset: %w", err) } else if result.ExitCode != 0 { - return fmt.Errorf("sync: mkdir -p %s: %s", parentDir, result.Stderr) + return fmt.Errorf("bootstrap: reset: %s", result.Stderr) } - // Clone into remote workspace if not already present. - testResult, err := ExecOverSSH(ctx, session, "test -d "+ShellEscape(repoPath), nil, nil) + patch, err := gitutil.GeneratePatch(checkoutSHA) if err != nil { - return fmt.Errorf("sync: check repo dir: %w", err) + return fmt.Errorf("bootstrap: %w", err) } - if testResult.ExitCode != 0 { - repoURL := fmt.Sprintf("https://github.com/%s/%s.git", org, repo) - var cloneCmd string - if gitutil.IsBranchPushed() { - branch, err := gitutil.CurrentBranch() - if err != nil { - return fmt.Errorf("sync: %w", err) - } - cloneCmd = fmt.Sprintf("git clone --branch %s %s %s", - ShellEscape(branch), ShellEscape(repoURL), ShellEscape(repoPath), - ) - } else { - status(iostream.LevelInfo, "Branch not pushed to remote; cloning default branch instead.") - cloneCmd = fmt.Sprintf("git clone %s %s", - ShellEscape(repoURL), ShellEscape(repoPath), - ) + if patch != "" { + status(iostream.LevelInfo, fmt.Sprintf("Applying patch (%d bytes)...", len(patch))) + if result, err := ExecOverSSH(ctx, session, inRepo(repoPath, "git apply"), strings.NewReader(patch), nil); err != nil { + return fmt.Errorf("bootstrap: apply patch: %w", err) + } else if result.ExitCode != 0 { + return fmt.Errorf("bootstrap: apply patch: %s", result.Stderr) } + } - status(iostream.LevelInfo, fmt.Sprintf("Cloning %s/%s into %s...", org, repo, repoPath)) - cloneResult, err := ExecOverSSH(ctx, session, cloneCmd, nil, nil) - if err != nil { - return fmt.Errorf("sync: clone: %w", err) - } - if cloneResult.ExitCode != 0 { - detail := cloneResult.Stderr - if detail == "" { - detail = "git clone exited with a non-zero status" - } - return fmt.Errorf("sync: clone failed: %s", detail) - } + // Persist the baseline so incremental syncs know their anchor. + active, err := LoadActive(ctx) + if err != nil { + return fmt.Errorf("bootstrap: load active: %w", err) + } + if active == nil { + active = &ActiveSidecar{SidecarID: sidecarID} + } + active.BaselineRef = checkoutSHA + active.BaselineBranch = branch + if err := SaveActive(ctx, *active); err != nil { + status(iostream.LevelWarn, fmt.Sprintf("Could not save baseline: %v", err)) } - status(iostream.LevelInfo, fmt.Sprintf("Synchronising local %s/%s to remote: %s...", org, repo, repoPath)) + status(iostream.LevelDone, "Bootstrapped") + return nil +} + +// Sync sends an incremental patch from the stored BaselineRef to the sidecar. +// It triggers Bootstrap automatically when the baseline is stale: no baseline +// recorded, the local branch changed, or BaselineRef is no longer an ancestor +// of HEAD (e.g. after a rebase). +func Sync(ctx context.Context, + client *circleci.Client, sidecarID, identityFile, authSock, workdir, cwd string, status iostream.StatusFunc) error { - base, err := gitutil.MergeBase() + active, err := LoadActive(ctx) if err != nil { - return &RemoteBaseError{Err: err} + return fmt.Errorf("sync: load active sidecar: %w", err) } - patch, err := gitutil.GeneratePatch(base) + currentBranch, err := gitutil.CurrentBranch() if err != nil { - return err + return fmt.Errorf("sync: %w", err) } - if patch == "" { - status(iostream.LevelInfo, "No local changes relative to remote base.") - return nil + headSHA, err := gitutil.HeadRef(cwd) + if err != nil { + return fmt.Errorf("sync: %w", err) } - status(iostream.LevelInfo, fmt.Sprintf("Synchronising %d bytes.", len(patch))) + needsBootstrap := active == nil || + active.BaselineRef == "" || + active.BaselineBranch != currentBranch || + !gitutil.IsAncestor(active.BaselineRef, headSHA, cwd) - resetCmd := fmt.Sprintf( - `sh -c "cd %s && git reset --hard %s && git clean -fd"`, - ShellEscape(repoPath), ShellEscape(base), - ) - resetResult, err := ExecOverSSH(ctx, session, resetCmd, nil, nil) + if needsBootstrap { + if active != nil && active.BaselineBranch != "" && active.BaselineBranch != currentBranch { + status(iostream.LevelInfo, fmt.Sprintf("Branch changed from %q to %q; re-bootstrapping...", active.BaselineBranch, currentBranch)) + } else if active != nil && active.BaselineRef != "" { + status(iostream.LevelInfo, "Baseline is stale; re-bootstrapping...") + } + return Bootstrap(ctx, client, sidecarID, identityFile, authSock, workdir, cwd, status) + } + + session, err := OpenSession(ctx, client, sidecarID, identityFile, authSock) if err != nil { return err } - if resetResult.ExitCode != 0 { - detail := resetResult.Stderr - if detail == "" { - detail = "git reset exited with a non-zero status" + + repoPath := active.Workspace + if repoPath == "" { + _, repo, detErr := gitremote.DetectOrgAndRepo(cwd) + if detErr != nil { + return fmt.Errorf("sync: %w", detErr) } - return fmt.Errorf("git reset failed (exit code: %d): %s", resetResult.ExitCode, detail) + repoPath = ResolveWorkspace(ctx, workdir, repo) } - applyCmd := fmt.Sprintf("git -C %s apply", ShellEscape(repoPath)) - applyResult, err := ExecOverSSH(ctx, session, applyCmd, strings.NewReader(patch), nil) + status(iostream.LevelInfo, fmt.Sprintf("Syncing from baseline %s...", active.BaselineRef[:12])) + + patch, err := gitutil.GeneratePatch(active.BaselineRef) if err != nil { - return err + return fmt.Errorf("sync: %w", err) } - if applyResult.ExitCode != 0 { - detail := applyResult.Stderr - if detail == "" { - detail = "git apply exited with a non-zero status" - } - return fmt.Errorf("%w (exit code: %d): %s", errApplyFailed, applyResult.ExitCode, detail) + + // Always reset+clean to guarantee a clean working tree, even with no patch. + resetCmd := inRepo(repoPath, "git reset --hard "+active.BaselineRef+" && git clean -fd") + if result, err := ExecOverSSH(ctx, session, resetCmd, nil, nil); err != nil { + return fmt.Errorf("sync: reset: %w", err) + } else if result.ExitCode != 0 { + return fmt.Errorf("sync: reset: %s", result.Stderr) + } + + if patch == "" { + status(iostream.LevelDone, "Synced (no local changes)") + return nil } + + status(iostream.LevelInfo, fmt.Sprintf("Applying patch (%d bytes)...", len(patch))) + if result, err := ExecOverSSH(ctx, session, inRepo(repoPath, "git apply"), strings.NewReader(patch), nil); err != nil { + return fmt.Errorf("sync: apply patch: %w", err) + } else if result.ExitCode != 0 { + return fmt.Errorf("sync: apply patch: %s", result.Stderr) + } + + status(iostream.LevelDone, "Synced") return nil } diff --git a/internal/sidecar/sync_test.go b/internal/sidecar/sync_test.go index 010e0333..a36fd905 100644 --- a/internal/sidecar/sync_test.go +++ b/internal/sidecar/sync_test.go @@ -16,17 +16,16 @@ import ( "github.com/CircleCI-Public/chunk-cli/internal/testing/gitrepo" ) -// TestSync_NonApplyFailureReturnsImmediately verifies that Sync does not send a -// "rm -rf" cleanup command when syncWorkspace fails for a reason other than a -// git-apply failure. MUT-013 caught this gap by inverting the errApplyFailed -// check, which caused Sync to retry (and rm -rf the remote workspace) for all -// failure types, not just patch-apply failures. -func TestSync_NonApplyFailureReturnsImmediately(t *testing.T) { +// TestSync_StaleBaselineBootstrapsAndReturnsRemoteBaseError verifies that Sync +// triggers Bootstrap when no baseline is stored, and that Bootstrap surfaces a +// RemoteBaseError when the branch is not pushed and MergeBase fails (no upstream +// tracking branch). Crucially, no rm -rf must be issued at any point. +func TestSync_StaleBaselineBootstrapsAndReturnsRemoteBaseError(t *testing.T) { keyFile, pubKey := fakes.GenerateSSHKeypair(t) - // SSH server: all commands succeed (exitCode 0), so mkdir-p and test-d pass. - // syncWorkspace then calls gitutil.MergeBase(), which fails because the test - // repo has no upstream tracking branch — a non-errApplyFailed error. + // SSH server: all commands succeed (exitCode 0), so mkdir-p, test-d, fetch + // all pass. Bootstrap then calls gitutil.MergeBase() locally, which fails + // because the test repo has no upstream tracking branch. sshSrv := fakes.NewSSHServer(t, pubKey) sshSrv.SetResult("", 0) @@ -46,21 +45,19 @@ func TestSync_NonApplyFailureReturnsImmediately(t *testing.T) { cl := newClient(t, srv.URL) noopStatus := iostream.StatusFunc(func(_ iostream.Level, _ string) {}) - err := sidecar.Sync(context.Background(), cl, "sb-1", keyFile, "", "", noopStatus) + err := sidecar.Sync(context.Background(), cl, "sb-1", keyFile, "", "", repoDir, noopStatus) // Sync must return an error (MergeBase failed — no upstream branch). assert.Assert(t, err != nil, "expected Sync to return an error") - // The error must be a RemoteBaseError, not an apply failure. + // The error must be a RemoteBaseError. var remoteBaseErr *sidecar.RemoteBaseError assert.Assert(t, errors.As(err, &remoteBaseErr), "expected RemoteBaseError, got: %T %v", err, err) - // Critically: no rm -rf must have been sent. With MUT-013, Sync would treat - // the RemoteBaseError as a retryable apply failure and issue a rm -rf before - // the second syncWorkspace attempt. + // No rm -rf must have been sent at any point. for _, cmd := range sshSrv.Commands() { assert.Assert(t, !strings.Contains(cmd, "rm -rf"), - "Sync must not send rm -rf for non-apply failures; got command: %q", cmd) + "Sync must not send rm -rf; got command: %q", cmd) } }