From 849a1c964eca4ca10cdd608fa8705cbb6a6e9d54 Mon Sep 17 00:00:00 2001 From: James Johns <192176108+jamesjohnsdev@users.noreply.github.com> Date: Sun, 21 Jun 2026 19:30:51 +1000 Subject: [PATCH] fix(delete): remove snapshot only after local file deletion succeeds A failed os.Remove on the issue file left the snapshot deleted, disabling modified detection for that issue on subsequent syncs. Closes #17 --- cmd/delete.go | 8 +++-- cmd/delete_test.go | 88 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 cmd/delete_test.go diff --git a/cmd/delete.go b/cmd/delete.go index cc12e47..3e59592 100644 --- a/cmd/delete.go +++ b/cmd/delete.go @@ -42,15 +42,17 @@ var deleteCmd = &cobra.Command{ fmt.Printf("%s #%d from GitHub\n", color.RedString("Deleted"), iss.Number) } - // Clean up originals snapshot if present - origPath := filepath.Join(originalsDir(root), fmt.Sprintf("%d.md", iss.Number)) - _ = os.Remove(origPath) } if err := os.Remove(iss.Path); err != nil { return err } + if iss.Number != 0 { + origPath := filepath.Join(originalsDir(root), fmt.Sprintf("%d.md", iss.Number)) + _ = os.Remove(origPath) + } + fmt.Printf("%s %s: %s\n", color.RedString("Deleted"), args[0], iss.Title) return nil }, diff --git a/cmd/delete_test.go b/cmd/delete_test.go new file mode 100644 index 0000000..7de931c --- /dev/null +++ b/cmd/delete_test.go @@ -0,0 +1,88 @@ +package cmd + +import ( + "os" + "path/filepath" + "testing" + + "github.com/jamesjohnsdev/issues/internal/issue" +) + +func TestDeleteCmd(t *testing.T) { + t.Run("local-only issue is deleted", func(t *testing.T) { + parent := makeProjectDir(t, []issueFixture{ + {"T1-local-bug.md", issue.Issue{Title: "Local bug", State: "open"}}, + }) + chdirTo(t, parent) + injectStdin(t, "") + _ = captureStdout(t, func() { + if err := deleteCmd.RunE(deleteCmd, []string{"T1"}); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + files := readMDFiles(t, filepath.Join(parent, issuesDirName, "open")) + if len(files) != 0 { + t.Errorf("expected file deleted, got %d files", len(files)) + } + }) + + t.Run("synced issue snapshot deleted after successful local delete", func(t *testing.T) { + parent := makeProjectDir(t, []issueFixture{ + {"5-synced-bug.md", issue.Issue{Number: 5, Title: "Synced bug", State: "open"}}, + }) + issRoot := filepath.Join(parent, issuesDirName) + origDir := originalsDir(issRoot) + if err := os.MkdirAll(origDir, 0755); err != nil { + t.Fatal(err) + } + origPath := filepath.Join(origDir, "5.md") + if err := os.WriteFile(origPath, []byte("snapshot"), 0644); err != nil { + t.Fatal(err) + } + chdirTo(t, parent) + injectStdin(t, "n\n") + _ = captureStdout(t, func() { + if err := deleteCmd.RunE(deleteCmd, []string{"5"}); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + if files := readMDFiles(t, filepath.Join(parent, issuesDirName, "open")); len(files) != 0 { + t.Errorf("expected local file deleted, got %d files", len(files)) + } + if _, err := os.Stat(origPath); !os.IsNotExist(err) { + t.Error("expected snapshot deleted, but it still exists") + } + }) + + t.Run("synced issue snapshot preserved when local delete fails", func(t *testing.T) { + parent := makeProjectDir(t, []issueFixture{ + {"5-synced-bug.md", issue.Issue{Number: 5, Title: "Synced bug", State: "open"}}, + }) + issRoot := filepath.Join(parent, issuesDirName) + origDir := originalsDir(issRoot) + if err := os.MkdirAll(origDir, 0755); err != nil { + t.Fatal(err) + } + origPath := filepath.Join(origDir, "5.md") + if err := os.WriteFile(origPath, []byte("snapshot"), 0644); err != nil { + t.Fatal(err) + } + openDir := filepath.Join(parent, issuesDirName, "open") + if err := os.Chmod(openDir, 0555); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(openDir, 0755) }) + chdirTo(t, parent) + injectStdin(t, "n\n") + var deleteErr error + _ = captureStdout(t, func() { + deleteErr = deleteCmd.RunE(deleteCmd, []string{"5"}) + }) + if deleteErr == nil { + t.Fatal("expected error from failed local delete, got nil") + } + if _, err := os.Stat(origPath); os.IsNotExist(err) { + t.Error("snapshot was deleted despite local delete failing") + } + }) +}