feat: transactional & recoverable workspace ops (issue #59, Tasks 1-3)

cmd/create.go

@@ -41,6 +41,29 @@ var createCmd = &cobra.Command{
 		repos := discover.FindAllRepos(cfg.RepoDirs)
 		repoMap := discover.RepoMap(repos)
 
+		// Resolve and validate branch and name BEFORE any source cloning, so a bad
+		// name, missing branch, or aborted prompt never triggers a network clone.
+		// (Cloning is source acquisition; a successful clone is intentionally
+		// retained even if workspace creation later fails.)
+		branch := createBranch
+		if branch == "" {
+			if console.IsTerminal(os.Stdin) {
+				branch = console.Prompt("Branch name")
+			}
+			if branch == "" {
+				exitError("Branch is required: --branch / -b")
+			}
+		}
+		var name string
+		if len(args) > 0 {
+			name = args[0]
+		} else {
+			name = deriveName(branch)
+		}
+		if err := workspace.ValidateWorkspaceName(name); err != nil {
+			exitError(err.Error())
+		}
+
 		var repoNames []string
 
 		// Resolve repos from preset
@@ -59,6 +82,17 @@ var createCmd = &cobra.Command{
 			for i := range repoNames {
 				repoNames[i] = strings.TrimSpace(repoNames[i])
 			}
+			// Validate non-URL repo names against known repos BEFORE cloning any
+			// URLs, so a mixed list with an unknown local repo is rejected without a
+			// network clone.
+			for _, name := range repoNames {
+				if gitops.IsGitURL(name) {
+					continue
+				}
+				if _, ok := repoMap[name]; !ok {
+					exitError("Unknown repo: " + name + ". Available: " + strings.Join(repoNamesList(repos), ", "))
+				}
+			}
 			// Clone any remote git URLs into the first repo_dir (mirrors add-repo).
 			// This lets a resolver pass an unmatched repo as a clone URL.
 			for i, name := range repoNames {
@@ -145,31 +179,12 @@ var createCmd = &cobra.Command{
 		}
 
 		// Validate repos exist
-		for _, name := range repoNames {
-			if _, ok := repoMap[name]; !ok {
-				exitError("Unknown repo: " + name + ". Available: " + strings.Join(repoNamesList(repos), ", "))
-			}
-		}
-
-		// Branch — prompt if omitted and in a terminal
-		branch := createBranch
-		if branch == "" {
-			if console.IsTerminal(os.Stdin) {
-				branch = console.Prompt("Branch name")
-			}
-			if branch == "" {
-				exitError("Branch is required: --branch / -b")
+		for _, rn := range repoNames {
+			if _, ok := repoMap[rn]; !ok {
+				exitError("Unknown repo: " + rn + ". Available: " + strings.Join(repoNamesList(repos), ", "))
 			}
 		}
 
-		// Name
-		var name string
-		if len(args) > 0 {
-			name = args[0]
-		} else {
-			name = deriveName(branch)
-		}
-
 		// --replace: delete the current workspace (detected from cwd) before creating the new one.
 		replacedName := ""
 		if createReplace {
@@ -226,11 +241,12 @@ var createCmd = &cobra.Command{
 			opts.BranchMode = workspace.BranchModeTrack
 		}
 
-		if err := workspace.NewService().CreateWithOpts(name, opts); err != nil {
+		if res := workspace.NewService().CreateWithResult(name, opts); res.NonZeroExit() {
+			renderRepoOutcomes(res)
 			if replacedName != "" {
-				exitError("failed to create new workspace (old workspace " + replacedName + " was already deleted): " + err.Error())
+				exitError("failed to create new workspace (old workspace " + replacedName + " was already deleted): " + res.Message)
 			}
-			exitError(err.Error())
+			exitError(res.Message)
 		}
 
 		// Fire post_create hook if configured
@@ -242,10 +258,10 @@ var createCmd = &cobra.Command{
 			vars.SourceTitle = source.Title
 		}
 		if err := lifecycle.Run("post_create", vars); err != nil && !errors.Is(err, lifecycle.ErrNoHook) {
-			if lifecycle.ShouldAbort(err) {
-				exitError(err.Error())
-			}
+			// A post-create hook failure is a partial outcome: the workspace is
+			// valid but a lifecycle step failed, so exit non-zero.
 			console.Warning(err.Error())
+			os.Exit(1)
 		}
 	},
 }

cmd/create_cli_test.go

@@ -0,0 +1,109 @@
+package cmd
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// runGit is a tiny test helper.
+func cmdTestGit(t *testing.T, dir string, args ...string) {
+	t.Helper()
+	c := exec.Command("git", args...)
+	c.Dir = dir
+	c.Env = append(os.Environ(), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null")
+	if out, err := c.CombinedOutput(); err != nil {
+		t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, out)
+	}
+}
+
+// TestCreateCommandExitsNonZeroAndRetainsClone builds the gw binary and runs a
+// real `gw create` that clones a local file:// source and then fails (the target
+// workspace name already exists). It proves two contract points at the command
+// layer: (1) a failed create exits non-zero, and (2) the successfully cloned
+// source repo is retained on disk despite the failure.
+func TestCreateCommandExitsNonZeroAndRetainsClone(t *testing.T) {
+	if testing.Short() {
+		t.Skip("builds a binary; skipped in -short")
+	}
+	tmp := t.TempDir()
+	home := filepath.Join(tmp, "home")
+	repoDir := filepath.Join(tmp, "repos")
+	if err := os.MkdirAll(repoDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+
+	// A local source repo to clone via file:// .
+	src := filepath.Join(tmp, "srcrepo")
+	cmdTestGit(t, tmp, "init", "-q", src)
+	cmdTestGit(t, src, "config", "user.email", "t@t.co")
+	cmdTestGit(t, src, "config", "user.name", "t")
+	if err := os.WriteFile(filepath.Join(src, "f"), []byte("x"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	cmdTestGit(t, src, "add", ".")
+	cmdTestGit(t, src, "commit", "-qm", "init")
+
+	// Build the gw binary.
+	bin := filepath.Join(tmp, "gw")
+	build := exec.Command("go", "build", "-o", bin, "./gw")
+	build.Dir = mustModuleCmdDir(t)
+	if out, err := build.CombinedOutput(); err != nil {
+		t.Fatalf("build gw: %v\n%s", err, out)
+	}
+
+	env := append(os.Environ(), "HOME="+home)
+
+	// Initialize grove with our repo dir.
+	initCmd := exec.Command(bin, "init", repoDir)
+	initCmd.Env = env
+	if out, err := initCmd.CombinedOutput(); err != nil {
+		t.Fatalf("gw init: %v\n%s", err, out)
+	}
+
+	// Create the workspace name first so the later create collides.
+	url := "file://" + src
+	// Seed an existing workspace named "srcrepo" (derived name) via a normal
+	// create from a plain local repo so the second create hits "already exists".
+	plain := filepath.Join(repoDir, "plain")
+	cmdTestGit(t, repoDir, "init", "-q", "plain")
+	cmdTestGit(t, plain, "config", "user.email", "t@t.co")
+	cmdTestGit(t, plain, "config", "user.name", "t")
+	os.WriteFile(filepath.Join(plain, "f"), []byte("x"), 0o644)
+	cmdTestGit(t, plain, "add", ".")
+	cmdTestGit(t, plain, "commit", "-qm", "init")
+
+	seed := exec.Command(bin, "create", "dup-ws", "-b", "feat/seed", "-r", "plain")
+	seed.Env = env
+	if out, err := seed.CombinedOutput(); err != nil {
+		t.Fatalf("seed create: %v\n%s", err, out)
+	}
+
+	// Now create "dup-ws" again but with a clone URL — the clone should happen
+	// (source acquisition), then the create fails because dup-ws exists.
+	fail := exec.Command(bin, "create", "dup-ws", "-b", "feat/x", "-r", url)
+	fail.Env = env
+	out, err := fail.CombinedOutput()
+	if err == nil {
+		t.Fatalf("expected non-zero exit for duplicate create, got success:\n%s", out)
+	}
+	if _, ok := err.(*exec.ExitError); !ok {
+		t.Fatalf("expected exit error, got %T: %v", err, err)
+	}
+	// The cloned source must be retained despite the failure.
+	if _, statErr := os.Stat(filepath.Join(repoDir, "srcrepo")); statErr != nil {
+		t.Fatalf("cloned source repo must be retained after failed create: %v", statErr)
+	}
+}
+
+// mustModuleCmdDir returns the cmd/ directory (this test file's directory).
+func mustModuleCmdDir(t *testing.T) string {
+	t.Helper()
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	return wd // tests run in the package dir (cmd/)
+}

cmd/outcome.go

@@ -0,0 +1,35 @@
+package cmd
+
+import (
+	"fmt"
+	"io"
+	"os"
+
+	"github.com/nicksenap/grove/internal/workspace"
+)
+
+// renderRepoOutcomes prints each per-repository outcome to stderr so automation
+// and humans both see complete detail before a non-zero exit. The public
+// machine-readable envelope is a separate concern (issue #63).
+func renderRepoOutcomes(res *workspace.OperationResult) {
+	fprintRepoOutcomes(os.Stderr, res)
+}
+
+// fprintRepoOutcomes writes the ordered per-repository outcomes to w.
+func fprintRepoOutcomes(w io.Writer, res *workspace.OperationResult) {
+	for _, r := range res.Repos {
+		line := fmt.Sprintf("  %-20s %s", r.RepoName, r.Status)
+		if r.Phase != "" {
+			line += " (" + r.Phase + ")"
+		}
+		if r.Err != nil {
+			line += ": " + r.Err.Error()
+		} else if r.Message != "" {
+			line += ": " + r.Message
+		}
+		fmt.Fprintln(w, line)
+	}
+	if res.RecordID != "" {
+		fmt.Fprintf(w, "  recovery record: %s (run: gw doctor)\n", res.RecordID)
+	}
+}

cmd/outcome_test.go

@@ -0,0 +1,48 @@
+package cmd
+
+import (
+	"bytes"
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/nicksenap/grove/internal/state"
+	"github.com/nicksenap/grove/internal/workspace"
+)
+
+func TestFprintRepoOutcomesRendersAllRepos(t *testing.T) {
+	res := &workspace.OperationResult{
+		Kind:      state.OpCreate,
+		Workspace: "ws",
+		Status:    workspace.OutcomePending,
+		RecordID:  "op-123",
+		Repos: []workspace.RepoOutcome{
+			{RepoName: "api", Status: state.RepoDone, Phase: "provision"},
+			{RepoName: "web", Status: state.RepoFailed, Phase: "provision", Err: errors.New("boom")},
+		},
+	}
+	var buf bytes.Buffer
+	fprintRepoOutcomes(&buf, res)
+	out := buf.String()
+	for _, want := range []string{"api", "web", "boom", "op-123", "recovery record"} {
+		if !strings.Contains(out, want) {
+			t.Fatalf("output missing %q:\n%s", want, out)
+		}
+	}
+}
+
+func TestOperationResultExitMapping(t *testing.T) {
+	cases := map[workspace.OutcomeStatus]bool{
+		workspace.OutcomeSuccess:   false,
+		workspace.OutcomeCancelled: false,
+		workspace.OutcomePartial:   true,
+		workspace.OutcomeFailed:    true,
+		workspace.OutcomePending:   true,
+	}
+	for status, wantNonZero := range cases {
+		res := &workspace.OperationResult{Status: status}
+		if res.NonZeroExit() != wantNonZero {
+			t.Fatalf("status %s: NonZeroExit=%v want %v", status, res.NonZeroExit(), wantNonZero)
+		}
+	}
+}