fix(create_project): idempotent rename write completes a stray write+delete split (#53)

The rename path (write new path, then delete old path) is two separate,
non-atomic API calls. If a prior partial run's write succeeded but the paired
delete failed — plausible under the same infra#179 flakiness resume (#50)
exists to work around — a resume would recompute the identical rename and
blind-create (no sha) at a path that already exists, hitting a conflict and
getting stuck needing another resume cycle just to re-report the same thing.

renameEntry now reads the new path first: if it already holds the correct
content (prior write succeeded), the write is skipped and only the
outstanding delete of the old path runs; if the new path exists but differs,
it's updated with the fetched sha instead of blind-created; if the old path
is already gone by delete time, that's treated as done, not a failure.
Mirrors injectDispatchAllow's (#51) read-before-write idempotency pattern.

Test: TestCreateProject_Resume_StrayRenamedOldPath_CompletesCleanly — a stray
old path plus an already-correct new path resolves to a single delete, zero
redundant writes, no partial_failure. All prior create_project tests
unaffected (backward compatible).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

internal/tools/create_project_from_template.go

@@ -374,21 +374,7 @@ func (t *CreateProjectFromTemplate) substituteEntry(ctx context.Context, owner,
 	enc := base64.StdEncoding.EncodeToString([]byte(newContent))
 
 	if renamed {
-		if err := t.upsertRetry(ctx, owner, name, newPath, gitea.UpsertFileArgs{
-			Branch:  branch,
-			Content: enc,
-			Message: fmt.Sprintf("template: substitute + rename %s -> %s", path, newPath),
-		}); err != nil {
-			return "", fmt.Sprintf("write %s: %v", newPath, err)
-		}
-		if _, err := t.c.DeleteFile(ctx, owner, name, path, gitea.DeleteFileArgs{
-			Branch:  branch,
-			Sha:     fc.Sha,
-			Message: fmt.Sprintf("template: drop placeholder path %s", path),
-		}); err != nil {
-			return "", fmt.Sprintf("delete %s: %v", path, err)
-		}
-		return path + " -> " + newPath, ""
+		return t.renameEntry(ctx, owner, name, branch, path, newPath, enc, newContent, fc.Sha)
 	}
 
 	if err := t.upsertRetry(ctx, owner, name, path, gitea.UpsertFileArgs{
@@ -401,3 +387,45 @@ func (t *CreateProjectFromTemplate) substituteEntry(ctx context.Context, owner,
 	}
 	return path, ""
 }
+
+// renameEntry writes newPath then deletes oldPath. Both halves are idempotent
+// so a resume that hits a prior write-succeeded/delete-failed rename (the two
+// are separate, non-atomic API calls) completes cleanly instead of erroring on
+// a blind re-create at a path that already exists (gitea-mcp#53): if newPath
+// already holds the correct content, the write is skipped and only the
+// outstanding delete of oldPath runs; if oldPath is already gone, the delete
+// is a no-op too.
+func (t *CreateProjectFromTemplate) renameEntry(ctx context.Context, owner, name, branch, oldPath, newPath, enc, newContent, oldSha string) (substituted, failure string) {
+	existing, err := t.c.GetFileContents(ctx, owner, name, newPath, branch)
+	switch {
+	case err == nil:
+		decoded, derr := base64.StdEncoding.DecodeString(existing.Content)
+		if derr == nil && string(decoded) == newContent {
+			break // already correct from a prior partial run — skip the write
+		}
+		if writeErr := t.upsertRetry(ctx, owner, name, newPath, gitea.UpsertFileArgs{
+			Branch: branch, Content: enc, Sha: existing.Sha,
+			Message: fmt.Sprintf("template: substitute + rename %s -> %s", oldPath, newPath),
+		}); writeErr != nil {
+			return "", fmt.Sprintf("write %s: %v", newPath, writeErr)
+		}
+	case errors.Is(err, gitea.ErrNotFound):
+		if writeErr := t.upsertRetry(ctx, owner, name, newPath, gitea.UpsertFileArgs{
+			Branch: branch, Content: enc,
+			Message: fmt.Sprintf("template: substitute + rename %s -> %s", oldPath, newPath),
+		}); writeErr != nil {
+			return "", fmt.Sprintf("write %s: %v", newPath, writeErr)
+		}
+	default:
+		return "", fmt.Sprintf("read %s: %v", newPath, err)
+	}
+
+	if _, err := t.c.DeleteFile(ctx, owner, name, oldPath, gitea.DeleteFileArgs{
+		Branch:  branch,
+		Sha:     oldSha,
+		Message: fmt.Sprintf("template: drop placeholder path %s", oldPath),
+	}); err != nil && !errors.Is(err, gitea.ErrNotFound) {
+		return "", fmt.Sprintf("delete %s: %v", oldPath, err)
+	}
+	return oldPath + " -> " + newPath, ""
+}

internal/tools/create_project_from_template_test.go

@@ -329,6 +329,29 @@ func TestCreateProject_Resume_AlreadyFullyDone_IsSuccess(t *testing.T) {
 	assert.Empty(t, out.PartialFailure, "nothing left to do on resume must be success, not a loud failure")
 }
 
+// A resume where a prior partial run's rename write SUCCEEDED but its paired
+// delete FAILED (both are separate, non-atomic API calls) must complete
+// cleanly: recognize the new path is already correct, skip re-writing it, and
+// just finish the outstanding delete of the stray old path (gitea-mcp#53).
+func TestCreateProject_Resume_StrayRenamedOldPath_CompletesCleanly(t *testing.T) {
+	files := map[string]string{
+		// stray: delete never completed in the prior run
+		"cmd/__PROJECT_NAME__/main.go": "package main\nimport \"__MODULE_PATH__/pkg/litellm\"\nconst n = \"__PROJECT_NAME__\"\n",
+		// already correct: the write half of the same prior rename DID complete
+		"cmd/new-svc/main.go": "package main\nimport \"git.d-ma.be/mathias/new-svc/pkg/litellm\"\nconst n = \"new-svc\"\n",
+	}
+	f := newFakeTemplateServerResumed(files, "main")
+	srv := httptest.NewServer(f.handler(t, "template-go-agent", "new-svc"))
+	defer srv.Close()
+
+	out := callTool(t, srv.URL, "template-go-agent", `{"owner":"mathias","name":"new-svc","resume":true}`)
+
+	assert.Empty(t, out.PartialFailure)
+	assert.Contains(t, out.FilesSubstituted, "cmd/__PROJECT_NAME__/main.go -> cmd/new-svc/main.go")
+	assert.NotContains(t, f.puts, "cmd/new-svc/main.go", "already-correct new path must not be rewritten")
+	assert.Contains(t, f.deletes, "cmd/__PROJECT_NAME__/main.go", "the outstanding delete must still happen")
+}
+
 // dispatch_allow injection is idempotent on resume: if .dispatch-allow already
 // has the correct content (from an earlier successful injection), re-invoking
 // must not attempt another write — and must not error the way a naive