summaryrefslogtreecommitdiff
path: root/internal
diff options
context:
space:
mode:
authorPaul Buetow <paul@buetow.org>2026-07-22 18:53:43 +0300
committerPaul Buetow <paul@buetow.org>2026-07-22 18:53:43 +0300
commit456cb2d3be55d431ba507367764ccfd13b13b4b4 (patch)
tree71c241dcf9fd963661023779e8edefc30ad7a4be /internal
parent23ecaa2c7f731a4f7188aec2404594641faf2f7f (diff)
fix(errcheck): handle all unchecked error returns without changing behaviormain
errcheck ./... flagged unchecked HTTP response Close(), file Close(), os.RemoveAll(), fmt.Scanln(), and fmt.Fprintf() calls across codeberg, github, release, showcase, and sync. None of these were bugs causing incorrect behavior today, but leaving them unchecked hid real failure modes (e.g. a lagging NFS mount failing a file Close() after writes, which this codebase has hit before per commit 23ecaa2). - internal/codeberg/codeberg.go, internal/github/github.go, internal/release/release.go: added a small closeResponseBody(resp) helper per package and used it for all deferred resp.Body.Close() calls. The body is always fully read (or abandoned on an earlier error) by the time Close() runs, so the error is intentionally discarded - matching the explicit `_ = ...` discard convention already used elsewhere in this repo (e.g. showcase.go's os.RemoveAll on the worktree-add failure path). - internal/release/release.go: the two fmt.Scanln(&response) prompts now explicitly discard the return values; a Scanln error already leaves response == "", which the existing y/yes check already treats as a safe decline, so behavior is unchanged. - internal/showcase/code_extractor.go, images.go, language_detector.go: added a shared closeFile(*os.File) helper (package showcase) for the read-only file Close() calls, matching the same discard rationale. copyFile's destination Close() is the one write-side case where a close failure is real data-loss information, so it now uses a named return to surface it via err without masking any earlier error. - internal/showcase/showcase.go: the deferred os.RemoveAll(tempRoot) now explicitly discards its error, matching the sibling `_ =` call three lines above in the same function. - internal/sync/branch_analyzer.go: GenerateDeleteScript's per-repository fmt.Fprintf(file, ...) calls are now checked and wrapped with fmt.Errorf(...: %w), matching the error-wrapping convention already used by writeBranchDeletionBlock right below it in the same file (the repeated repo-header writes were pulled into a new writeDeleteScriptRepoHeader helper to keep this readable). The script file's defer file.Close() now uses a named return so a close failure is reported instead of silently discarded. Added focused tests for the two behavior-relevant paths: copyFile's missing-source/success paths after the named-return change, and PromptConfirmation's empty-input-declines/explicit-yes behavior after touching the Scanln call. Trivial defer-Close discards elsewhere are not additionally unit tested per the task's guidance against overengineering. Verified: go build ./..., go vet ./..., errcheck ./... (clean), and go test ./... all pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Diffstat (limited to 'internal')
-rw-r--r--internal/codeberg/codeberg.go21
-rw-r--r--internal/github/github.go21
-rw-r--r--internal/release/release.go43
-rw-r--r--internal/release/release_prompt_test.go74
-rw-r--r--internal/showcase/code_extractor.go14
-rw-r--r--internal/showcase/images.go17
-rw-r--r--internal/showcase/images_test.go55
-rw-r--r--internal/showcase/language_detector.go4
-rw-r--r--internal/showcase/showcase.go6
-rw-r--r--internal/sync/branch_analyzer.go66
10 files changed, 266 insertions, 55 deletions
diff --git a/internal/codeberg/codeberg.go b/internal/codeberg/codeberg.go
index f881be9..9bff36e 100644
--- a/internal/codeberg/codeberg.go
+++ b/internal/codeberg/codeberg.go
@@ -42,6 +42,15 @@ type Client struct {
var _ forge.RepoClient = (*Client)(nil)
var _ forge.RepoDescriptionClient = (*Client)(nil)
+// closeResponseBody closes an HTTP response body. By the time this runs the
+// body has already been fully read (or the caller is bailing out on an
+// earlier error), so a close failure here cannot change the outcome that was
+// already determined - the error is intentionally discarded rather than
+// treated as actionable.
+func closeResponseBody(resp *http.Response) {
+ _ = resp.Body.Close()
+}
+
// NewClient creates a new Codeberg API client
func NewClient(token, org string) *Client {
c := &Client{
@@ -97,7 +106,7 @@ func (c *Client) GetRepo(repoName string) (Repository, bool, error) {
if err != nil {
return repo, false, err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode == 404 {
return repo, false, nil
@@ -149,7 +158,7 @@ func (c *Client) UpdateRepoDescription(repoName, description string) error {
if err != nil {
return err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode != http.StatusOK {
b, _ := io.ReadAll(resp.Body)
@@ -242,7 +251,7 @@ func (c *Client) listReposPage(url string) ([]Repository, error) {
if err != nil {
return nil, fmt.Errorf("failed to fetch repositories: %w", err)
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("API returned status %d", resp.StatusCode)
@@ -273,7 +282,7 @@ func (c *Client) RepoExists(repoName string) (bool, error) {
if err != nil {
return false, err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
return resp.StatusCode == 200, nil
}
@@ -316,7 +325,7 @@ func (c *Client) CreateRepo(repoName, description string, private bool) error {
if err != nil {
return err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode != http.StatusCreated {
// Read the response body to get more detailed error information
@@ -365,7 +374,7 @@ func (c *Client) DeleteRepo(repoName string) error {
if err != nil {
return err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
body, _ := io.ReadAll(resp.Body)
return forge.DeleteStatusError(resp.StatusCode, string(body))
diff --git a/internal/github/github.go b/internal/github/github.go
index a05d09f..4d09093 100644
--- a/internal/github/github.go
+++ b/internal/github/github.go
@@ -23,6 +23,15 @@ type Client struct {
var _ forge.RepoClient = (*Client)(nil)
var _ forge.RepoDescriptionClient = (*Client)(nil)
+// closeResponseBody closes an HTTP response body. By the time this runs the
+// body has already been fully read (or the caller is bailing out on an
+// earlier error), so a close failure here cannot change the outcome that was
+// already determined - the error is intentionally discarded rather than
+// treated as actionable.
+func closeResponseBody(resp *http.Response) {
+ _ = resp.Body.Close()
+}
+
// NewClient creates a new GitHub API client
func NewClient(token, org string) *Client {
return &Client{
@@ -104,7 +113,7 @@ func (c *Client) RepoExists(repoName string) (bool, error) {
if err != nil {
return false, err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode == 200 {
return true, nil
@@ -166,7 +175,7 @@ func (c *Client) CreateRepo(repoName, description string, private bool) error {
if err != nil {
return err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode == 201 {
var createResp CreateRepoResponse
@@ -216,7 +225,7 @@ func (c *Client) GetRepo(repoName string) (Repository, bool, error) {
if err != nil {
return repo, false, err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode == 404 {
return repo, false, nil
@@ -269,7 +278,7 @@ func (c *Client) UpdateRepoDescription(repoName, description string) error {
if err != nil {
return err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode != 200 {
b, _ := io.ReadAll(resp.Body)
@@ -339,7 +348,7 @@ func (c *Client) listPublicReposPage(url string) ([]Repository, error) {
if err != nil {
return nil, err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
@@ -388,7 +397,7 @@ func (c *Client) DeleteRepo(repoName string) error {
if err != nil {
return err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
body, _ := io.ReadAll(resp.Body)
return forge.DeleteStatusError(resp.StatusCode, string(body))
diff --git a/internal/release/release.go b/internal/release/release.go
index fc414c9..5608f33 100644
--- a/internal/release/release.go
+++ b/internal/release/release.go
@@ -38,6 +38,15 @@ type Manager struct {
aiTool string
}
+// closeResponseBody closes an HTTP response body. By the time this runs the
+// body has already been fully read (or the caller is bailing out on an
+// earlier error), so a close failure here cannot change the outcome that was
+// already determined - the error is intentionally discarded rather than
+// treated as actionable.
+func closeResponseBody(resp *http.Response) {
+ _ = resp.Body.Close()
+}
+
// NewManager creates a new release manager
func NewManager(workDir string) *Manager {
return &Manager{
@@ -79,7 +88,7 @@ func (m *Manager) EnsureCodebergReleasesEnabled(owner, repo string) error {
if err != nil {
return err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode != 200 {
body, _ := io.ReadAll(resp.Body)
return fmt.Errorf("failed to get repo info: %s - %s", resp.Status, string(body))
@@ -112,7 +121,7 @@ func (m *Manager) EnsureCodebergReleasesEnabled(owner, repo string) error {
if err != nil {
return err
}
- defer patchResp.Body.Close()
+ defer closeResponseBody(patchResp)
if patchResp.StatusCode != 200 {
pbody, _ := io.ReadAll(patchResp.Body)
return fmt.Errorf("failed to enable releases: %s - %s", patchResp.Status, string(pbody))
@@ -544,7 +553,7 @@ func (m *Manager) GetGitHubReleases(owner, repo string) ([]string, error) {
if err != nil {
return nil, err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode == 404 {
// Repository might not exist on GitHub
@@ -588,7 +597,7 @@ func (m *Manager) GetCodebergReleases(owner, repo string) ([]string, error) {
if err != nil {
return nil, err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode == 404 {
// Repository might not exist on Codeberg
@@ -669,7 +678,7 @@ func (m *Manager) CreateGitHubRelease(owner, repo, tag, releaseNotes string) err
if err != nil {
return err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode != 201 {
body, _ := io.ReadAll(resp.Body)
@@ -722,7 +731,7 @@ func (m *Manager) CreateCodebergRelease(owner, repo, tag, releaseNotes string) e
if err != nil {
return err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode != 201 {
body, _ := io.ReadAll(resp.Body)
@@ -739,7 +748,7 @@ func (m *Manager) CreateCodebergRelease(owner, repo, tag, releaseNotes string) e
probeReq.Header.Set("Authorization", "token "+m.codebergToken)
}
if probeResp, perr2 := httpclient.Do(probeReq); perr2 == nil {
- defer probeResp.Body.Close()
+ defer closeResponseBody(probeResp)
if probeResp.StatusCode == 200 {
// Try to detect if releases are disabled
var repoInfo struct {
@@ -768,7 +777,7 @@ func (m *Manager) CreateCodebergRelease(owner, repo, tag, releaseNotes string) e
if rerr != nil {
return rerr
}
- defer retryResp.Body.Close()
+ defer closeResponseBody(retryResp)
if retryResp.StatusCode != 201 {
rbody, _ := io.ReadAll(retryResp.Body)
return fmt.Errorf("failed to create Codeberg release after enabling releases: %s - %s", retryResp.Status, string(rbody))
@@ -812,7 +821,11 @@ func PromptConfirmation(message string) bool {
fmt.Printf("%s [y/N]: ", message)
var response string
- fmt.Scanln(&response)
+ // Scanln can return an error for empty input (e.g. a bare newline) or if
+ // stdin is closed; either way response stays "" and the check below
+ // already treats that as a decline, so there is nothing extra to do
+ // with the error here.
+ _, _ = fmt.Scanln(&response)
response = strings.ToLower(strings.TrimSpace(response))
return response == "y" || response == "yes"
@@ -828,7 +841,9 @@ func PromptConfirmationWithNotes(message, releaseNotes string) bool {
fmt.Printf("%s [y/N]: ", message)
var response string
- fmt.Scanln(&response)
+ // See PromptConfirmation: a Scanln error still leaves response == "",
+ // which the check below already treats as a decline.
+ _, _ = fmt.Scanln(&response)
response = strings.ToLower(strings.TrimSpace(response))
return response == "y" || response == "yes"
@@ -856,7 +871,7 @@ func (m *Manager) UpdateGitHubRelease(owner, repo, tag, releaseNotes string) err
if err != nil {
return err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode != 200 {
body, _ := io.ReadAll(resp.Body)
@@ -898,7 +913,7 @@ func (m *Manager) UpdateGitHubRelease(owner, repo, tag, releaseNotes string) err
if err != nil {
return err
}
- defer updateResp.Body.Close()
+ defer closeResponseBody(updateResp)
if updateResp.StatusCode != 200 {
body, _ := io.ReadAll(updateResp.Body)
@@ -929,7 +944,7 @@ func (m *Manager) UpdateCodebergRelease(owner, repo, tag, releaseNotes string) e
if err != nil {
return err
}
- defer resp.Body.Close()
+ defer closeResponseBody(resp)
if resp.StatusCode != 200 {
body, _ := io.ReadAll(resp.Body)
@@ -970,7 +985,7 @@ func (m *Manager) UpdateCodebergRelease(owner, repo, tag, releaseNotes string) e
if err != nil {
return err
}
- defer updateResp.Body.Close()
+ defer closeResponseBody(updateResp)
if updateResp.StatusCode != 200 {
body, _ := io.ReadAll(updateResp.Body)
diff --git a/internal/release/release_prompt_test.go b/internal/release/release_prompt_test.go
new file mode 100644
index 0000000..fdb1d27
--- /dev/null
+++ b/internal/release/release_prompt_test.go
@@ -0,0 +1,74 @@
+package release
+
+import (
+ "os"
+ "testing"
+)
+
+// withStdin temporarily replaces os.Stdin with r for the duration of fn, then
+// restores the original value. Not run in parallel with other tests since
+// os.Stdin is a shared global.
+func withStdin(t *testing.T, r *os.File, fn func()) {
+ t.Helper()
+
+ original := os.Stdin
+ os.Stdin = r
+ defer func() { os.Stdin = original }()
+
+ fn()
+}
+
+// TestPromptConfirmation_EmptyInputDeclines verifies that when Scanln
+// returns an error (e.g. a bare newline, which fmt.Scanln reports as
+// "unexpected newline"), PromptConfirmation still safely defaults to
+// declining rather than panicking or hanging - this is the discarded-error
+// behavior that PromptConfirmation's fmt.Scanln call relies on.
+func TestPromptConfirmation_EmptyInputDeclines(t *testing.T) {
+ r, w, err := os.Pipe()
+ if err != nil {
+ t.Fatalf("failed to create pipe: %v", err)
+ }
+ defer func() { _ = r.Close() }()
+
+ if _, err := w.WriteString("\n"); err != nil {
+ t.Fatalf("failed to write to pipe: %v", err)
+ }
+ if err := w.Close(); err != nil {
+ t.Fatalf("failed to close pipe writer: %v", err)
+ }
+
+ var got bool
+ withStdin(t, r, func() {
+ got = PromptConfirmation("Proceed?")
+ })
+
+ if got {
+ t.Fatal("PromptConfirmation() = true for empty input, want false")
+ }
+}
+
+// TestPromptConfirmation_YesInputConfirms is a regression check that the
+// happy path (explicit "y") still works after touching the Scanln call.
+func TestPromptConfirmation_YesInputConfirms(t *testing.T) {
+ r, w, err := os.Pipe()
+ if err != nil {
+ t.Fatalf("failed to create pipe: %v", err)
+ }
+ defer func() { _ = r.Close() }()
+
+ if _, err := w.WriteString("y\n"); err != nil {
+ t.Fatalf("failed to write to pipe: %v", err)
+ }
+ if err := w.Close(); err != nil {
+ t.Fatalf("failed to close pipe writer: %v", err)
+ }
+
+ var got bool
+ withStdin(t, r, func() {
+ got = PromptConfirmation("Proceed?")
+ })
+
+ if !got {
+ t.Fatal("PromptConfirmation() = false for \"y\" input, want true")
+ }
+}
diff --git a/internal/showcase/code_extractor.go b/internal/showcase/code_extractor.go
index 92ee1e4..7c40573 100644
--- a/internal/showcase/code_extractor.go
+++ b/internal/showcase/code_extractor.go
@@ -9,6 +9,16 @@ import (
"strings"
)
+// closeFile closes a file opened for reading. These reads are best-effort
+// (shebang sniffing, line counting, snippet extraction) and the file is never
+// written to, so a close failure here cannot affect correctness - the error
+// is intentionally discarded rather than treated as actionable. Shared by
+// the other showcase package files that open read-only files (images.go,
+// language_detector.go).
+func closeFile(file *os.File) {
+ _ = file.Close()
+}
+
// extractCodeSnippet extracts a random code snippet from the repository
func extractCodeSnippet(repoPath string, languages []LanguageStats) (string, string, error) {
if len(languages) == 0 {
@@ -113,7 +123,7 @@ func extractCodeSnippet(repoPath string, languages []LanguageStats) (string, str
matched = true
}
}
- file.Close()
+ closeFile(file)
}
}
@@ -185,7 +195,7 @@ func extractSnippetFromFile(filePath string, minLines, maxLines int) (string, er
if err != nil {
return "", err
}
- defer file.Close()
+ defer closeFile(file)
// Read all lines
var lines []string
diff --git a/internal/showcase/images.go b/internal/showcase/images.go
index b6fe6d7..2c09a8e 100644
--- a/internal/showcase/images.go
+++ b/internal/showcase/images.go
@@ -226,19 +226,28 @@ func isGitHostedImage(url string) bool {
strings.Contains(url, "codeberg.page")
}
-// copyFile copies a file from src to dst
-func copyFile(src, dst string) error {
+// copyFile copies a file from src to dst. The destination is a write, so
+// unlike the read-only closeFile helper used elsewhere in this package, a
+// failure to close it is reported via the named return: Sync() already
+// covers the common flush-failure case, but a close error on a lagging
+// network filesystem is still real data-loss information worth surfacing,
+// and it must not mask an earlier, more specific error.
+func copyFile(src, dst string) (err error) {
sourceFile, err := os.Open(src)
if err != nil {
return err
}
- defer sourceFile.Close()
+ defer closeFile(sourceFile)
destFile, err := os.Create(dst)
if err != nil {
return err
}
- defer destFile.Close()
+ defer func() {
+ if cerr := destFile.Close(); cerr != nil && err == nil {
+ err = fmt.Errorf("failed to close destination file %s: %w", dst, cerr)
+ }
+ }()
_, err = io.Copy(destFile, sourceFile)
if err != nil {
diff --git a/internal/showcase/images_test.go b/internal/showcase/images_test.go
new file mode 100644
index 0000000..c88da00
--- /dev/null
+++ b/internal/showcase/images_test.go
@@ -0,0 +1,55 @@
+package showcase
+
+import (
+ "os"
+ "path/filepath"
+ "testing"
+)
+
+// TestCopyFile_CopiesContent verifies the basic success path still works
+// after copyFile was changed to use a named return so that a destination
+// Close() error can be surfaced without changing the happy-path result.
+func TestCopyFile_CopiesContent(t *testing.T) {
+ t.Parallel()
+
+ dir := t.TempDir()
+ src := filepath.Join(dir, "source.txt")
+ dst := filepath.Join(dir, "dest.txt")
+ want := "hello showcase\n"
+
+ if err := os.WriteFile(src, []byte(want), 0644); err != nil {
+ t.Fatalf("failed to write source file: %v", err)
+ }
+
+ if err := copyFile(src, dst); err != nil {
+ t.Fatalf("copyFile() returned error: %v", err)
+ }
+
+ got, err := os.ReadFile(dst)
+ if err != nil {
+ t.Fatalf("failed to read destination file: %v", err)
+ }
+ if string(got) != want {
+ t.Fatalf("copyFile() wrote %q, want %q", string(got), want)
+ }
+}
+
+// TestCopyFile_MissingSourceReturnsError verifies that a missing source file
+// still produces an error (and never a nil error masked by the deferred
+// destination-close handling) and does not leave a destination file behind.
+func TestCopyFile_MissingSourceReturnsError(t *testing.T) {
+ t.Parallel()
+
+ dir := t.TempDir()
+ src := filepath.Join(dir, "does-not-exist.txt")
+ dst := filepath.Join(dir, "dest.txt")
+
+ err := copyFile(src, dst)
+ if err == nil {
+ t.Fatal("copyFile() with missing source returned nil error, want non-nil")
+ }
+
+ if _, statErr := os.Stat(dst); statErr == nil {
+ t.Fatal("copyFile() with missing source created a destination file, want none")
+ }
+}
diff --git a/internal/showcase/language_detector.go b/internal/showcase/language_detector.go
index 692f048..9a42be1 100644
--- a/internal/showcase/language_detector.go
+++ b/internal/showcase/language_detector.go
@@ -201,7 +201,7 @@ func detectLanguages(repoPath string) (languages []LanguageStats, documentation
}
}
}
- file.Close()
+ closeFile(file)
}
}
@@ -281,7 +281,7 @@ func countFileLines(path string) (int, error) {
if err != nil {
return 0, err
}
- defer file.Close()
+ defer closeFile(file)
scanner := bufio.NewScanner(file)
lines := 0
diff --git a/internal/showcase/showcase.go b/internal/showcase/showcase.go
index ae938cb..9582c1f 100644
--- a/internal/showcase/showcase.go
+++ b/internal/showcase/showcase.go
@@ -350,7 +350,11 @@ func (g *Generator) prepareStatsRepoPath(repoName, repoPath string) (string, fun
}
cleanup := func() error {
- defer os.RemoveAll(tempRoot)
+ // Best-effort cleanup of the temporary worktree root, matching the
+ // discard above for the same call on the worktree-add failure path:
+ // this is scratch space, and by the time we get here the actual
+ // worktree removal below is the operation whose error matters.
+ defer func() { _ = os.RemoveAll(tempRoot) }()
if _, err := runCommandWithCustomTimeout(45*time.Second, "git", "-C", repoPath, "worktree", "remove", "--force", worktreePath); err != nil {
return fmt.Errorf("failed to remove temporary worktree for %s: %w", repoName, err)
diff --git a/internal/sync/branch_analyzer.go b/internal/sync/branch_analyzer.go
index eaa6cdc..6b51198 100644
--- a/internal/sync/branch_analyzer.go
+++ b/internal/sync/branch_analyzer.go
@@ -518,8 +518,11 @@ func writeBranchDeletionBlock(writer io.Writer, branches []BranchInfo, reviewBra
return nil
}
-// GenerateDeleteScript generates a shell script file to delete all abandoned branches
-func (s *Syncer) GenerateDeleteScript() (string, error) {
+// GenerateDeleteScript generates a shell script file to delete all abandoned branches.
+// The return value uses named results so that a failure to close the script file
+// (e.g. a delayed flush error on a lagging filesystem) is reported via err instead
+// of being silently discarded, without masking any earlier, more specific error.
+func (s *Syncer) GenerateDeleteScript() (scriptPath string, err error) {
if len(s.abandonedReports) == 0 {
return "", nil
}
@@ -539,7 +542,7 @@ func (s *Syncer) GenerateDeleteScript() (string, error) {
// Generate script filename with timestamp
timestamp := time.Now().Format("20060102_150405")
- scriptPath := filepath.Join(s.workDir, fmt.Sprintf("delete_abandoned_branches_%s.sh", timestamp))
+ scriptPath = filepath.Join(s.workDir, fmt.Sprintf("delete_abandoned_branches_%s.sh", timestamp))
scriptBaseName := filepath.Base(scriptPath)
// Create the script file
@@ -547,7 +550,11 @@ func (s *Syncer) GenerateDeleteScript() (string, error) {
if err != nil {
return "", fmt.Errorf("failed to create script file: %w", err)
}
- defer file.Close()
+ defer func() {
+ if cerr := file.Close(); cerr != nil && err == nil {
+ err = fmt.Errorf("failed to close script file %s: %w", scriptPath, cerr)
+ }
+ }()
if err := writeDeleteScriptTemplate(file, "deleteScriptPreamble", deleteScriptTemplateData{
GeneratedAt: time.Now().Format("2006-01-02 15:04:05"),
@@ -567,24 +574,15 @@ func (s *Syncer) GenerateDeleteScript() (string, error) {
continue
}
- fmt.Fprintf(file, "# ======================================\n")
- fmt.Fprintf(file, "# Repository: %s\n", repoName)
- fmt.Fprintf(file, "# ======================================\n")
- fmt.Fprintf(file, "echo\n")
- fmt.Fprintf(file, "echo \"📁 Processing repository: %s\"\n", repoName)
- fmt.Fprintf(file, "cd \"%s/%s\" || { echo \"Failed to change to repository directory\"; exit 1; }\n\n", s.workDir, repoName)
-
- // Find main branch for review mode
- fmt.Fprintf(file, "if [[ \"$MODE\" == \"review\" || \"$MODE\" == \"review-full\" ]]; then\n")
- fmt.Fprintf(file, " main_branch=$(find_main_branch)\n")
- fmt.Fprintf(file, " if [[ -z \"$main_branch\" ]]; then\n")
- fmt.Fprintf(file, " echo -e \"${RED}⚠️ No main/master branch found in %s${NC}\"\n", repoName)
- fmt.Fprintf(file, " fi\n")
- fmt.Fprintf(file, "fi\n\n")
+ if err := writeDeleteScriptRepoHeader(file, s.workDir, repoName); err != nil {
+ return scriptPath, err
+ }
// Process regular abandoned branches
if len(report.AbandonedBranches) > 0 {
- fmt.Fprintf(file, "# Regular abandoned branches\n")
+ if _, err := fmt.Fprintf(file, "# Regular abandoned branches\n"); err != nil {
+ return scriptPath, fmt.Errorf("failed to write regular branches header for %s: %w", repoName, err)
+ }
if err := writeBranchDeletionBlock(file, report.AbandonedBranches, "regular", "🔸 Deleting branch: "); err != nil {
return scriptPath, err
}
@@ -592,7 +590,9 @@ func (s *Syncer) GenerateDeleteScript() (string, error) {
// Process ignored abandoned branches
if len(report.AbandonedIgnoredBranches) > 0 {
- fmt.Fprintf(file, "# Ignored abandoned branches\n")
+ if _, err := fmt.Fprintf(file, "# Ignored abandoned branches\n"); err != nil {
+ return scriptPath, fmt.Errorf("failed to write ignored branches header for %s: %w", repoName, err)
+ }
if err := writeBranchDeletionBlock(file, report.AbandonedIgnoredBranches, "ignored", "🔹 Deleting ignored branch: "); err != nil {
return scriptPath, err
}
@@ -612,3 +612,29 @@ func (s *Syncer) GenerateDeleteScript() (string, error) {
return scriptPath, nil
}
+
+// writeDeleteScriptRepoHeader writes the per-repository banner and the
+// review-mode main-branch check at the top of each repository's block in
+// the generated delete script.
+func writeDeleteScriptRepoHeader(file *os.File, workDir, repoName string) error {
+ lines := []string{
+ "# ======================================\n",
+ fmt.Sprintf("# Repository: %s\n", repoName),
+ "# ======================================\n",
+ "echo\n",
+ fmt.Sprintf("echo \"📁 Processing repository: %s\"\n", repoName),
+ fmt.Sprintf("cd \"%s/%s\" || { echo \"Failed to change to repository directory\"; exit 1; }\n\n", workDir, repoName),
+ "if [[ \"$MODE\" == \"review\" || \"$MODE\" == \"review-full\" ]]; then\n",
+ " main_branch=$(find_main_branch)\n",
+ " if [[ -z \"$main_branch\" ]]; then\n",
+ fmt.Sprintf(" echo -e \"${RED}⚠️ No main/master branch found in %s${NC}\"\n", repoName),
+ " fi\n",
+ "fi\n\n",
+ }
+ for _, line := range lines {
+ if _, err := fmt.Fprint(file, line); err != nil {
+ return fmt.Errorf("failed to write repository header for %s: %w", repoName, err)
+ }
+ }
+ return nil
+}