49bea57 Merge pull request #65 from bnema/fix/worktree-bootstrap-idempotence · zut

M packages/agent/subagents/batch_test.go +10 -0


@@ -58,6 +58,16 @@ func TestBatchReloadsDurableResult(t *testing.T) {
58	58		if want.Status != BatchSucceeded {
59	59		t.Fatalf("first batch status = %s", want.Status)
60	60		}
	61	+	// WaitBatch returns once the aggregate result is available, which can
	62	+	// precede each child's final metadata write. Wait for that finalisation
	63	+	// before another supervisor reloads the same durable state.
	64	+	for _, childID := range batch.ChildIDs {
	65	+	child := first.Get(childID)
	66	+	if child == nil {
	67	+	t.Fatalf("first supervisor is missing child %q", childID)
	68	+	}
	69	+	child.Wait()
	70	+	}
61	71
62	72		second := newSupervisor()
63	73		t.Cleanup(second.StopAll)


@@ -58,6 +58,16 @@ func TestBatchReloadsDurableResult(t *testing.T) {
58	if want.Status != BatchSucceeded {	58		if want.Status != BatchSucceeded {
59	t.Fatalf("first batch status = %s", want.Status)	59		t.Fatalf("first batch status = %s", want.Status)
60	}	60		}
		61	+	// WaitBatch returns once the aggregate result is available, which can
		62	+	// precede each child's final metadata write. Wait for that finalisation
		63	+	// before another supervisor reloads the same durable state.
		64	+	for _, childID := range batch.ChildIDs {
		65	+	child := first.Get(childID)
		66	+	if child == nil {
		67	+	t.Fatalf("first supervisor is missing child %q", childID)
		68	+	}
		69	+	child.Wait()
		70	+	}
61		71
62	second := newSupervisor()	72		second := newSupervisor()
63	t.Cleanup(second.StopAll)	73		t.Cleanup(second.StopAll)

M packages/agent/tools/create_worktree.go +18 -4


@@ -255,9 +255,6 @@ func (t *CreateWorktreeTool) plan(ctx context.Context, raw json.RawMessage) (cre
255	255		return createWorktreePlan{}, fmt.Errorf("create_worktree: read local worktree configuration: %w", err)
256	256		}
257	257		bootstrapRoot := strings.TrimSpace(args.BootstrapRoot)
258		-	if configured && bootstrapRoot != "" {
259		-	return createWorktreePlan{}, errors.New("create_worktree: worktree root is already configured; change the local Git configuration explicitly before bootstrapping again")
260		-	}
261	258
262	259		rootSource := "existing .worktrees directory"
263	260		configValue := ""

@@ -269,6 +266,23 @@ func (t *CreateWorktreeTool) plan(ctx context.Context, raw json.RawMessage) (cre
269	266		if err != nil {
270	267		return createWorktreePlan{}, err
271	268		}
	269	+	if bootstrapRoot != "" {
	270	+	requestedRoot, requestedDefault, _, err := resolveBootstrapWorktreeRoot(repoRoot, bootstrapRoot)
	271	+	if err != nil {
	272	+	return createWorktreePlan{}, err
	273	+	}
	274	+	configuredCanonical, err := canonicalOrParent(worktreeRoot)
	275	+	if err != nil {
	276	+	return createWorktreePlan{}, fmt.Errorf("create_worktree: resolve configured worktree root: %w", err)
	277	+	}
	278	+	requestedCanonical, err := canonicalOrParent(requestedRoot)
	279	+	if err != nil {
	280	+	return createWorktreePlan{}, fmt.Errorf("create_worktree: resolve bootstrap_root: %w", err)
	281	+	}
	282	+	if defaultRoot != requestedDefault \|\| configuredCanonical != requestedCanonical {
	283	+	return createWorktreePlan{}, errors.New("create_worktree: worktree root is already configured; change the local Git configuration explicitly before bootstrapping again")
	284	+	}
	285	+	}
272	286		rootSource = "local Git config"
273	287		} else {
274	288		defaultPath := filepath.Join(repoRoot, defaultWorktreeRoot)

@@ -280,7 +294,7 @@ func (t *CreateWorktreeTool) plan(ctx context.Context, raw json.RawMessage) (cre
280	294		return bootstrapRequiredPlan(branch, repoRoot), nil
281	295		}
282	296		if exists {
283		-	if bootstrapRoot != "" {
	297	+	if bootstrapRoot != "" && bootstrapRoot != defaultWorktreeRoot {
284	298		return createWorktreePlan{}, errors.New("create_worktree: worktree root is already established; omit bootstrap_root")
285	299		}
286	300		worktreeRoot = defaultPath

 		return createWorktreePlan{}, fmt.Errorf("create_worktree: read local worktree configuration: %w", err)
 	}
 	bootstrapRoot := strings.TrimSpace(args.BootstrapRoot)
-	if configured && bootstrapRoot != "" {
-		return createWorktreePlan{}, errors.New("create_worktree: worktree root is already configured; change the local Git configuration explicitly before bootstrapping again")
-	}
 	rootSource := "existing .worktrees directory"
 	configValue := ""
 		if err != nil {
 			return createWorktreePlan{}, err
 		}
+		if bootstrapRoot != "" {
+			requestedRoot, requestedDefault, _, err := resolveBootstrapWorktreeRoot(repoRoot, bootstrapRoot)
+			if err != nil {
+				return createWorktreePlan{}, err
+			}
+			configuredCanonical, err := canonicalOrParent(worktreeRoot)
+			if err != nil {
+				return createWorktreePlan{}, fmt.Errorf("create_worktree: resolve configured worktree root: %w", err)
+			}
+			requestedCanonical, err := canonicalOrParent(requestedRoot)
+			if err != nil {
+				return createWorktreePlan{}, fmt.Errorf("create_worktree: resolve bootstrap_root: %w", err)
+			}
+			if defaultRoot != requestedDefault || configuredCanonical != requestedCanonical {
+				return createWorktreePlan{}, errors.New("create_worktree: worktree root is already configured; change the local Git configuration explicitly before bootstrapping again")
+			}
+		}
 		rootSource = "local Git config"
 	} else {
 		defaultPath := filepath.Join(repoRoot, defaultWorktreeRoot)
 			return bootstrapRequiredPlan(branch, repoRoot), nil
 		}
 		if exists {
-			if bootstrapRoot != "" {
+			if bootstrapRoot != "" && bootstrapRoot != defaultWorktreeRoot {
 				return createWorktreePlan{}, errors.New("create_worktree: worktree root is already established; omit bootstrap_root")
 			}
 			worktreeRoot = defaultPath

M packages/agent/tools/create_worktree_test.go +78 -2


@@ -60,6 +60,19 @@ func TestCreateWorktreeBootstrapsDefaultAtRepositoryRoot(t *testing.T) {
60	60		if got := gitTestOutput(t, repo, "config", "--local", "--get", worktreeConfigKey); got != defaultWorktreeRoot {
61	61		t.Fatalf("saved worktree root = %q, want %q", got, defaultWorktreeRoot)
62	62		}
	63	+	followupArgs := mustJSON(t, map[string]string{
	64	+	"branch": "feature/default-followup",
	65	+	"bootstrap_root": defaultWorktreeRoot,
	66	+	})
	67	+	if _, err := tool.Preview(context.Background(), followupArgs); err != nil {
	68	+	t.Fatal(err)
	69	+	}
	70	+	if _, err := tool.Execute(context.Background(), followupArgs, nil); err != nil {
	71	+	t.Fatal(err)
	72	+	}
	73	+	if _, err := os.Stat(filepath.Join(repo, defaultWorktreeRoot, "feature", "default-followup")); err != nil {
	74	+	t.Fatalf("configured default root was not reused: %v", err)
	75	+	}
63	76		}
64	77
65	78		func TestCreateWorktreeRequiresBootstrapForUnconfiguredRepository(t *testing.T) {

@@ -93,7 +106,39 @@ func TestCreateWorktreeRequiresBootstrapForUnconfiguredRepository(t *testing.T)
93	106		}
94	107		}
95	108
96		-	func TestCreateWorktreeRejectsBootstrapRootWhenDefaultRootExists(t *testing.T) {
	109	+	func TestCreateWorktreeAcceptsDefaultBootstrapRootWhenDefaultRootExists(t *testing.T) {
	110	+	if _, err := exec.LookPath("git"); err != nil {
	111	+	t.Skip("git is required")
	112	+	}
	113	+	repo := initWorktreeTestRepo(t)
	114	+	if err := os.Mkdir(filepath.Join(repo, defaultWorktreeRoot), 0o755); err != nil {
	115	+	t.Fatal(err)
	116	+	}
	117	+	tool := newCreateWorktreeTestTool(repo)
	118	+	args := mustJSON(t, map[string]string{
	119	+	"branch": "feature/existing-default-root",
	120	+	"bootstrap_root": defaultWorktreeRoot,
	121	+	})
	122	+
	123	+	if _, err := tool.Preview(context.Background(), args); err != nil {
	124	+	t.Fatal(err)
	125	+	}
	126	+	result, err := tool.Execute(context.Background(), args, nil)
	127	+	if err != nil {
	128	+	t.Fatal(err)
	129	+	}
	130	+	if _, err := os.Stat(filepath.Join(repo, defaultWorktreeRoot, "feature", "existing-default-root")); err != nil {
	131	+	t.Fatalf("worktree was not created: %v", err)
	132	+	}
	133	+	if source, _ := result.Details.(map[string]any)["root_source"].(string); source != "existing .worktrees directory" {
	134	+	t.Fatalf("root source = %q", source)
	135	+	}
	136	+	if got := gitTestOutputAllowExit(t, repo, "config", "--local", "--get", worktreeConfigKey); got.exitCode != 1 {
	137	+	t.Fatalf("existing default root saved config: %#v", got)
	138	+	}
	139	+	}
	140	+
	141	+	func TestCreateWorktreeRejectsConflictingBootstrapRootWhenDefaultRootExists(t *testing.T) {
97	142		if _, err := exec.LookPath("git"); err != nil {
98	143		t.Skip("git is required")
99	144		}

@@ -118,6 +163,30 @@ func TestCreateWorktreeRejectsBootstrapRootWhenDefaultRootExists(t *testing.T) {
118	163		}
119	164		}
120	165
	166	+	func TestCreateWorktreeRejectsConflictingBootstrapRootWhenRootIsConfigured(t *testing.T) {
	167	+	if _, err := exec.LookPath("git"); err != nil {
	168	+	t.Skip("git is required")
	169	+	}
	170	+	repo := initWorktreeTestRepo(t)
	171	+	gitTestOutput(t, repo, "config", "--local", worktreeConfigKey, defaultWorktreeRoot)
	172	+	tool := newCreateWorktreeTestTool(repo)
	173	+	args := mustJSON(t, map[string]string{
	174	+	"branch": "feature/conflicting-configured-bootstrap",
	175	+	"bootstrap_root": t.TempDir(),
	176	+	})
	177	+
	178	+	if _, err := tool.Preview(context.Background(), args); err == nil \|\| !strings.Contains(err.Error(), "already configured") {
	179	+	t.Fatalf("preview error = %v, want configured-root error", err)
	180	+	}
	181	+	_, err := tool.Execute(context.Background(), args, nil)
	182	+	if err == nil \|\| !strings.Contains(err.Error(), "already configured") {
	183	+	t.Fatalf("error = %v, want configured-root error", err)
	184	+	}
	185	+	if got := gitTestOutputAllowExit(t, repo, "show-ref", "--verify", "--quiet", "refs/heads/feature/conflicting-configured-bootstrap"); got.exitCode != 1 {
	186	+	t.Fatalf("conflicting bootstrap created branch: %#v", got)
	187	+	}
	188	+	}
	189	+
121	190		func TestCreateWorktreeBootstrapsExternalRootWithoutChangingGitignore(t *testing.T) {
122	191		if _, err := exec.LookPath("git"); err != nil {
123	192		t.Skip("git is required")

@@ -153,7 +222,14 @@ func TestCreateWorktreeBootstrapsExternalRootWithoutChangingGitignore(t *testing
153	222		if state, _ := result.Details.(map[string]any)["state"].(string); state != "created" {
154	223		t.Fatalf("state = %q, want created", state)
155	224		}
156		-	followup, err := tool.Execute(context.Background(), json.RawMessage(`{"branch":"feature/external-followup"}`), nil)
	225	+	followupArgs := mustJSON(t, map[string]string{
	226	+	"branch": "feature/external-followup",
	227	+	"bootstrap_root": externalRoot,
	228	+	})
	229	+	if _, err := tool.Preview(context.Background(), followupArgs); err != nil {
	230	+	t.Fatal(err)
	231	+	}
	232	+	followup, err := tool.Execute(context.Background(), followupArgs, nil)
157	233		if err != nil {
158	234		t.Fatal(err)
159	235		}


@@ -60,6 +60,19 @@ func TestCreateWorktreeBootstrapsDefaultAtRepositoryRoot(t *testing.T) {
60		if got := gitTestOutput(t, repo, "config", "--local", "--get", worktreeConfigKey); got != defaultWorktreeRoot {	60		if got := gitTestOutput(t, repo, "config", "--local", "--get", worktreeConfigKey); got != defaultWorktreeRoot {
61		t.Fatalf("saved worktree root = %q, want %q", got, defaultWorktreeRoot)	61		t.Fatalf("saved worktree root = %q, want %q", got, defaultWorktreeRoot)
62		}	62		}
			63	+	followupArgs := mustJSON(t, map[string]string{
			64	+	"branch": "feature/default-followup",
			65	+	"bootstrap_root": defaultWorktreeRoot,
			66	+	})
			67	+	if _, err := tool.Preview(context.Background(), followupArgs); err != nil {
			68	+	t.Fatal(err)
			69	+	}
			70	+	if _, err := tool.Execute(context.Background(), followupArgs, nil); err != nil {
			71	+	t.Fatal(err)
			72	+	}
			73	+	if _, err := os.Stat(filepath.Join(repo, defaultWorktreeRoot, "feature", "default-followup")); err != nil {
			74	+	t.Fatalf("configured default root was not reused: %v", err)
			75	+	}
63		}	76		}
64			77
65		func TestCreateWorktreeRequiresBootstrapForUnconfiguredRepository(t *testing.T) {	78		func TestCreateWorktreeRequiresBootstrapForUnconfiguredRepository(t *testing.T) {

@@ -93,7 +106,39 @@ func TestCreateWorktreeRequiresBootstrapForUnconfiguredRepository(t *testing.T)
93		}	106		}
94		}	107		}
95			108
96	-	func TestCreateWorktreeRejectsBootstrapRootWhenDefaultRootExists(t *testing.T) {	109	+	func TestCreateWorktreeAcceptsDefaultBootstrapRootWhenDefaultRootExists(t *testing.T) {
			110	+	if _, err := exec.LookPath("git"); err != nil {
			111	+	t.Skip("git is required")
			112	+	}
			113	+	repo := initWorktreeTestRepo(t)
			114	+	if err := os.Mkdir(filepath.Join(repo, defaultWorktreeRoot), 0o755); err != nil {
			115	+	t.Fatal(err)
			116	+	}
			117	+	tool := newCreateWorktreeTestTool(repo)
			118	+	args := mustJSON(t, map[string]string{
			119	+	"branch": "feature/existing-default-root",
			120	+	"bootstrap_root": defaultWorktreeRoot,
			121	+	})
			122	+
			123	+	if _, err := tool.Preview(context.Background(), args); err != nil {
			124	+	t.Fatal(err)
			125	+	}
			126	+	result, err := tool.Execute(context.Background(), args, nil)
			127	+	if err != nil {
			128	+	t.Fatal(err)
			129	+	}
			130	+	if _, err := os.Stat(filepath.Join(repo, defaultWorktreeRoot, "feature", "existing-default-root")); err != nil {
			131	+	t.Fatalf("worktree was not created: %v", err)
			132	+	}
			133	+	if source, _ := result.Details.(map[string]any)["root_source"].(string); source != "existing .worktrees directory" {
			134	+	t.Fatalf("root source = %q", source)
			135	+	}
			136	+	if got := gitTestOutputAllowExit(t, repo, "config", "--local", "--get", worktreeConfigKey); got.exitCode != 1 {
			137	+	t.Fatalf("existing default root saved config: %#v", got)
			138	+	}
			139	+	}
			140	+
			141	+	func TestCreateWorktreeRejectsConflictingBootstrapRootWhenDefaultRootExists(t *testing.T) {
97		if _, err := exec.LookPath("git"); err != nil {	142		if _, err := exec.LookPath("git"); err != nil {
98		t.Skip("git is required")	143		t.Skip("git is required")
99		}	144		}

@@ -118,6 +163,30 @@ func TestCreateWorktreeRejectsBootstrapRootWhenDefaultRootExists(t *testing.T) {
118		}	163		}
119		}	164		}
120			165
			166	+	func TestCreateWorktreeRejectsConflictingBootstrapRootWhenRootIsConfigured(t *testing.T) {
			167	+	if _, err := exec.LookPath("git"); err != nil {
			168	+	t.Skip("git is required")
			169	+	}
			170	+	repo := initWorktreeTestRepo(t)
			171	+	gitTestOutput(t, repo, "config", "--local", worktreeConfigKey, defaultWorktreeRoot)
			172	+	tool := newCreateWorktreeTestTool(repo)
			173	+	args := mustJSON(t, map[string]string{
			174	+	"branch": "feature/conflicting-configured-bootstrap",
			175	+	"bootstrap_root": t.TempDir(),
			176	+	})
			177	+
			178	+	if _, err := tool.Preview(context.Background(), args); err == nil \|\| !strings.Contains(err.Error(), "already configured") {
			179	+	t.Fatalf("preview error = %v, want configured-root error", err)
			180	+	}
			181	+	_, err := tool.Execute(context.Background(), args, nil)
			182	+	if err == nil \|\| !strings.Contains(err.Error(), "already configured") {
			183	+	t.Fatalf("error = %v, want configured-root error", err)
			184	+	}
			185	+	if got := gitTestOutputAllowExit(t, repo, "show-ref", "--verify", "--quiet", "refs/heads/feature/conflicting-configured-bootstrap"); got.exitCode != 1 {
			186	+	t.Fatalf("conflicting bootstrap created branch: %#v", got)
			187	+	}
			188	+	}
			189	+
121		func TestCreateWorktreeBootstrapsExternalRootWithoutChangingGitignore(t *testing.T) {	190		func TestCreateWorktreeBootstrapsExternalRootWithoutChangingGitignore(t *testing.T) {
122		if _, err := exec.LookPath("git"); err != nil {	191		if _, err := exec.LookPath("git"); err != nil {
123		t.Skip("git is required")	192		t.Skip("git is required")

@@ -153,7 +222,14 @@ func TestCreateWorktreeBootstrapsExternalRootWithoutChangingGitignore(t *testing
153		if state, _ := result.Details.(map[string]any)["state"].(string); state != "created" {	222		if state, _ := result.Details.(map[string]any)["state"].(string); state != "created" {
154		t.Fatalf("state = %q, want created", state)	223		t.Fatalf("state = %q, want created", state)
155		}	224		}
156	-	followup, err := tool.Execute(context.Background(), json.RawMessage(`{"branch":"feature/external-followup"}`), nil)	225	+	followupArgs := mustJSON(t, map[string]string{
			226	+	"branch": "feature/external-followup",
			227	+	"bootstrap_root": externalRoot,
			228	+	})
			229	+	if _, err := tool.Preview(context.Background(), followupArgs); err != nil {
			230	+	t.Fatal(err)
			231	+	}
			232	+	followup, err := tool.Execute(context.Background(), followupArgs, nil)
157		if err != nil {	233		if err != nil {
158		t.Fatal(err)	234		t.Fatal(err)
159		}	235		}