From ace59999f9e50688d393d1a34a10a8c0435e5251 Mon Sep 17 00:00:00 2001 From: Rob Morgan Date: Tue, 10 Feb 2026 15:36:11 +0800 Subject: [PATCH] fix: prevent merge conflicts by deferring upstream creation to start Move upstream bare repo creation from `init` to `start` so users can customize metamorph.toml and AGENT_PROMPT.md before the upstream snapshot is taken. `start` now requires a clean working tree and creates the upstream from the committed project state on first run. Also abort failed merges in SyncToProjectDir so the repo is never left in a broken merge state. Co-Authored-By: Claude Opus 4.6 --- cmd/cmd_test.go | 69 +++++++++++++++++++++- cmd/init.go | 103 ++------------------------------- cmd/start.go | 20 ++++++- internal/gitops/gitops.go | 6 +- internal/gitops/gitops_test.go | 55 ++++++++++++++++++ 5 files changed, 150 insertions(+), 103 deletions(-) diff --git a/cmd/cmd_test.go b/cmd/cmd_test.go index 2479679..121601a 100644 --- a/cmd/cmd_test.go +++ b/cmd/cmd_test.go @@ -55,6 +55,10 @@ webhook_url = "" t.Fatal(err) } + if err := os.WriteFile(filepath.Join(dir, ".gitignore"), []byte(".metamorph/\nagent_logs/\n"), 0644); err != nil { + t.Fatal(err) + } + return dir } @@ -169,7 +173,6 @@ func TestInitCreatesAllFiles(t *testing.T) { for _, d := range []string{ constants.TaskLockDir, constants.AgentLogDir, - constants.UpstreamDir, } { info, err := os.Stat(filepath.Join(dir, d)) if err != nil { @@ -398,6 +401,70 @@ func TestStartDryRun(t *testing.T) { } } +func TestStartRejectsDirtyTree(t *testing.T) { + dir := testProjectWithUpstream(t) + + oldWd, _ := os.Getwd() + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + defer func() { _ = os.Chdir(oldWd) }() + + // Create an uncommitted file to make the tree dirty. + if err := os.WriteFile(filepath.Join(dir, "dirty.txt"), []byte("uncommitted"), 0644); err != nil { + t.Fatal(err) + } + + t.Setenv("ANTHROPIC_API_KEY", "sk-test-dummy") + + rootCmd.SetArgs([]string{"start", "--dry-run"}) + err := rootCmd.Execute() + if err == nil { + t.Fatal("expected error for dirty working tree") + } + if !strings.Contains(err.Error(), "uncommitted changes") { + t.Errorf("expected 'uncommitted changes' error, got: %v", err) + } +} + +func TestStartCreatesUpstream(t *testing.T) { + // Create a project with git but no upstream repo. + dir := testProject(t) + gitExec(t, dir, "init") + gitExec(t, dir, "config", "user.name", "test") + gitExec(t, dir, "config", "user.email", "test@test") + gitExec(t, dir, "add", ".") + gitExec(t, dir, "commit", "-m", "initial commit") + + oldWd, _ := os.Getwd() + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + defer func() { _ = os.Chdir(oldWd) }() + + t.Setenv("ANTHROPIC_API_KEY", "sk-test-dummy") + + // Verify upstream doesn't exist yet. + upstreamPath := filepath.Join(dir, constants.UpstreamDir) + if _, err := os.Stat(upstreamPath); err == nil { + t.Fatal("upstream should not exist before start") + } + + rootCmd.SetArgs([]string{"start", "--dry-run"}) + if err := rootCmd.Execute(); err != nil { + t.Fatalf("start --dry-run: %v", err) + } + + // Verify upstream was created. + info, err := os.Stat(upstreamPath) + if err != nil { + t.Fatalf("upstream should exist after start: %v", err) + } + if !info.IsDir() { + t.Error("upstream should be a directory") + } +} + func TestFormatDuration(t *testing.T) { tests := []struct { secs int diff --git a/cmd/init.go b/cmd/init.go index 487adbb..55e2d83 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -3,11 +3,9 @@ package cmd import ( "fmt" "os" - "os/exec" "path/filepath" "github.com/robmorgan/metamorph/internal/constants" - "github.com/robmorgan/metamorph/internal/gitops" "github.com/spf13/cobra" ) @@ -126,27 +124,16 @@ Add project-specific instructions for your agents here. } fmt.Println(" Created .gitignore") - // Initialize upstream bare repo (clones from user's repo for shared history). - if err := gitops.InitUpstream(absDir); err != nil { - return fmt.Errorf("failed to initialize upstream repo: %w", err) - } - fmt.Println(" Initialized upstream repo") - - // Update upstream with full content: clone to temp, copy missing files, commit, push. - upstreamPath := filepath.Join(absDir, constants.UpstreamDir) - if err := syncFilesToUpstream(absDir, upstreamPath); err != nil { - return fmt.Errorf("failed to sync files to upstream: %w", err) - } - fmt.Println(" Synced project files to upstream") - fmt.Printf("\nProject %q initialized successfully!\n\n", projectName) fmt.Println("Next steps:") fmt.Println(" 1. Review and customize metamorph.toml") fmt.Println(" 2. Edit AGENT_PROMPT.md with project-specific instructions") - fmt.Println(" 3. Set credentials (pick one):") + fmt.Println(" 3. Commit the changes:") + fmt.Println(" git add -A && git commit -m \"Initialize metamorph\"") + fmt.Println(" 4. Set credentials (pick one):") fmt.Println(" export CLAUDE_CODE_OAUTH_TOKEN=... # Claude Pro/Max subscription") fmt.Println(" export ANTHROPIC_API_KEY=sk-... # Anthropic API key") - fmt.Println(" 4. Start agents: metamorph start") + fmt.Println(" 5. Start agents: metamorph start") return nil }, @@ -156,85 +143,3 @@ func init() { rootCmd.AddCommand(initCmd) } -// syncFilesToUpstream clones the upstream bare repo to a temp dir, copies project -// files in (only if they don't already exist), commits, and pushes. -func syncFilesToUpstream(projectDir, upstreamPath string) error { - tmpDir, err := os.MkdirTemp("", "metamorph-sync-*") - if err != nil { - return fmt.Errorf("failed to create temp dir: %w", err) - } - defer func() { _ = os.RemoveAll(tmpDir) }() - - cloneDir := filepath.Join(tmpDir, "work") - - // Clone. - if err := runGit(tmpDir, "clone", upstreamPath, cloneDir); err != nil { - return fmt.Errorf("failed to clone upstream: %w", err) - } - - // Copy project files into the clone only if they don't already exist. - filesToSync := []string{"metamorph.toml", constants.ProgressFile} - for _, name := range filesToSync { - dst := filepath.Join(cloneDir, name) - if _, err := os.Stat(dst); err == nil { - continue // already exists in upstream - } - src := filepath.Join(projectDir, name) - data, err := os.ReadFile(src) - if err != nil { - return fmt.Errorf("failed to read %s: %w", name, err) - } - if err := os.WriteFile(dst, data, 0644); err != nil { - return fmt.Errorf("failed to write %s: %w", name, err) - } - } - - // Set identity so commits work without a global git config. - _ = runGit(cloneDir, "config", "user.name", "metamorph") - _ = runGit(cloneDir, "config", "user.email", "metamorph@localhost") - - // Stage and commit only if there are changes. - if err := runGit(cloneDir, "add", "."); err != nil { - return fmt.Errorf("failed to stage files: %w", err) - } - - // Check if there are staged changes before committing. - if err := runGit(cloneDir, "diff", "--cached", "--quiet"); err == nil { - return nil // nothing to commit - } - - if err := runGit(cloneDir, "commit", "-m", "metamorph: sync project files"); err != nil { - return fmt.Errorf("failed to commit: %w", err) - } - - // Detect branch and push. - branch, err := runGitOutput(cloneDir, "rev-parse", "--abbrev-ref", "HEAD") - if err != nil { - return fmt.Errorf("failed to detect branch: %w", err) - } - if err := runGit(cloneDir, "push", "origin", branch); err != nil { - return fmt.Errorf("failed to push: %w", err) - } - - return nil -} - -// runGit executes a git command in the given directory, discarding output. -func runGit(dir string, args ...string) error { - cmd := exec.Command("git", args...) - cmd.Dir = dir - cmd.Stdout = nil - cmd.Stderr = nil - return cmd.Run() -} - -// runGitOutput executes a git command and returns trimmed stdout. -func runGitOutput(dir string, args ...string) (string, error) { - cmd := exec.Command("git", args...) - cmd.Dir = dir - out, err := cmd.Output() - if err != nil { - return "", err - } - return string(out[:len(out)-1]), nil // trim trailing newline -} diff --git a/cmd/start.go b/cmd/start.go index 73d1520..aea0292 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -4,12 +4,14 @@ import ( "fmt" "log/slog" "os" + "os/exec" "path/filepath" "text/tabwriter" "github.com/robmorgan/metamorph/internal/constants" "github.com/robmorgan/metamorph/internal/daemon" "github.com/robmorgan/metamorph/internal/docker" + "github.com/robmorgan/metamorph/internal/gitops" "github.com/spf13/cobra" ) @@ -101,10 +103,24 @@ func runForegroundStart(cmd *cobra.Command) error { cfg.Git.AuthorEmail = email } - // Check that the project has been initialized (upstream repo exists). + // Require a clean working tree before starting. + statusCmd := exec.Command("git", "status", "--porcelain") + statusCmd.Dir = projectDir + statusOut, err := statusCmd.Output() + if err != nil { + return fmt.Errorf("failed to check git status: %w", err) + } + if len(statusOut) > 0 { + return fmt.Errorf("uncommitted changes detected in project directory\n\nPlease commit your changes first:\n git add -A && git commit -m \"your message\"") + } + + // Create upstream bare repo if it doesn't exist yet (first start after init). upstreamPath := filepath.Join(projectDir, constants.UpstreamDir) if _, err := os.Stat(upstreamPath); os.IsNotExist(err) { - return fmt.Errorf("project not initialized: %s not found\n\nRun 'metamorph init' first to set up the project", constants.UpstreamDir) + fmt.Println("Creating upstream repository...") + if err := gitops.InitUpstream(projectDir); err != nil { + return fmt.Errorf("failed to initialize upstream repo: %w", err) + } } // Apply flag overrides to a local copy for display/dry-run purposes. diff --git a/internal/gitops/gitops.go b/internal/gitops/gitops.go index b611955..f3747e8 100644 --- a/internal/gitops/gitops.go +++ b/internal/gitops/gitops.go @@ -3,6 +3,7 @@ package gitops import ( "bytes" "fmt" + "log/slog" "os" "os/exec" "path/filepath" @@ -207,7 +208,10 @@ func SyncToProjectDir(upstreamPath, projectDir string) (string, error) { // Merge FETCH_HEAD. if _, err := git(projectDir, "merge", "FETCH_HEAD", "--no-edit"); err != nil { - return "", fmt.Errorf("gitops: merge failed (resolve conflicts manually): %w", err) + if _, abortErr := git(projectDir, "merge", "--abort"); abortErr != nil { + slog.Warn("gitops: failed to abort merge", "error", abortErr) + } + return "", fmt.Errorf("gitops: merge failed (will retry on next sync): %w", err) } // Get new HEAD. diff --git a/internal/gitops/gitops_test.go b/internal/gitops/gitops_test.go index a68b336..15d10f9 100644 --- a/internal/gitops/gitops_test.go +++ b/internal/gitops/gitops_test.go @@ -673,4 +673,59 @@ func TestSyncToProjectDir(t *testing.T) { t.Errorf("error should have gitops prefix: %v", err) } }) + + t.Run("aborts merge on conflict", func(t *testing.T) { + projectDir, upstreamPath := setupUpstream(t) + + // Push a conflicting change via upstream (simulating an agent). + pusherDir := filepath.Join(t.TempDir(), "agent") + if _, err := git(t.TempDir(), "clone", upstreamPath, pusherDir); err != nil { + t.Fatalf("clone for agent: %v", err) + } + if _, err := git(pusherDir, "config", "user.name", "agent-1"); err != nil { + t.Fatal(err) + } + if _, err := git(pusherDir, "config", "user.email", "agent-1@test"); err != nil { + t.Fatal(err) + } + // Write a file that will conflict. + if err := os.WriteFile(filepath.Join(pusherDir, "conflict.txt"), []byte("agent version\n"), 0644); err != nil { + t.Fatal(err) + } + if _, err := git(pusherDir, "add", "."); err != nil { + t.Fatal(err) + } + if _, err := git(pusherDir, "commit", "-m", "agent: add conflict.txt"); err != nil { + t.Fatal(err) + } + if _, err := git(pusherDir, "push"); err != nil { + t.Fatal(err) + } + + // Create a conflicting change in the project dir. + if err := os.WriteFile(filepath.Join(projectDir, "conflict.txt"), []byte("project version\n"), 0644); err != nil { + t.Fatal(err) + } + if _, err := git(projectDir, "add", "."); err != nil { + t.Fatal(err) + } + if _, err := git(projectDir, "commit", "-m", "project: add conflict.txt"); err != nil { + t.Fatal(err) + } + + // Sync should fail due to merge conflict. + _, err := SyncToProjectDir(upstreamPath, projectDir) + if err == nil { + t.Fatal("expected merge conflict error") + } + if !strings.Contains(err.Error(), "merge failed") { + t.Errorf("expected 'merge failed' in error, got: %v", err) + } + + // Verify merge was aborted — MERGE_HEAD should not exist. + mergeHead := filepath.Join(projectDir, ".git", "MERGE_HEAD") + if _, err := os.Stat(mergeHead); err == nil { + t.Error("MERGE_HEAD exists — merge was not aborted") + } + }) }