diff --git a/internal/tools/create_project_from_template.go b/internal/tools/create_project_from_template.go index b697027..ab2c687 100644 --- a/internal/tools/create_project_from_template.go +++ b/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, "" +} diff --git a/internal/tools/create_project_from_template_test.go b/internal/tools/create_project_from_template_test.go index 2f3b1b3..093d531 100644 --- a/internal/tools/create_project_from_template_test.go +++ b/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