summaryrefslogtreecommitdiff
path: root/internal/sync
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/sync
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/sync')
-rw-r--r--internal/sync/branch_analyzer.go66
1 files changed, 46 insertions, 20 deletions
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
+}