Add HTTP client timeout to prevent indefinite hangs in self-update

internal/fs/fs_test.go

@@ -1,42 +1,42 @@
 package fs
 
 import (
-    "os"
-    "path/filepath"
-    "testing"
+	"os"
+	"path/filepath"
+	"testing"
 )
 
 func TestEnsureWriteReadExists(t *testing.T) {
-    dir := t.TempDir()
-    d := filepath.Join(dir, "subdir")
-    if err := EnsureDir(d); err != nil {
-        t.Fatalf("EnsureDir failed: %v", err)
-    }
-
-    p := filepath.Join(d, "file.txt")
-    data := []byte("hello world")
-    if err := WriteFile(p, data); err != nil {
-        t.Fatalf("WriteFile failed: %v", err)
-    }
-
-    got, err := ReadFile(p)
-    if err != nil {
-        t.Fatalf("ReadFile failed: %v", err)
-    }
-
-    if string(got) != string(data) {
-        t.Fatalf("content mismatch: %s", string(got))
-    }
-
-    if !Exists(p) {
-        t.Fatalf("Exists should be true for %s", p)
-    }
-
-    // Non-existent file
-    if Exists(filepath.Join(dir, "nope.txt")) {
-        t.Fatalf("Exists should be false for non-existent file")
-    }
-
-    // cleanup test file
-    os.Remove(p)
+	dir := t.TempDir()
+	d := filepath.Join(dir, "subdir")
+	if err := EnsureDir(d); err != nil {
+		t.Fatalf("EnsureDir failed: %v", err)
+	}
+
+	p := filepath.Join(d, "file.txt")
+	data := []byte("hello world")
+	if err := WriteFile(p, data); err != nil {
+		t.Fatalf("WriteFile failed: %v", err)
+	}
+
+	got, err := ReadFile(p)
+	if err != nil {
+		t.Fatalf("ReadFile failed: %v", err)
+	}
+
+	if string(got) != string(data) {
+		t.Fatalf("content mismatch: %s", string(got))
+	}
+
+	if !Exists(p) {
+		t.Fatalf("Exists should be true for %s", p)
+	}
+
+	// Non-existent file
+	if Exists(filepath.Join(dir, "nope.txt")) {
+		t.Fatalf("Exists should be false for non-existent file")
+	}
+
+	// cleanup test file
+	os.Remove(p)
 }

internal/markdown/markdown_test.go

@@ -1,65 +1,65 @@
 package markdown
 
 import (
-    "strings"
-    "testing"
-    "time"
+	"strings"
+	"testing"
+	"time"
 
-    "journal-cli/internal/domain"
+	"journal-cli/internal/domain"
 )
 
 func TestGenerateAndParseRoundtrip(t *testing.T) {
-    date := time.Date(2025, 12, 30, 0, 0, 0, 0, time.UTC)
-    entry := domain.NewJournalEntry(date, "daily-human-dev")
-    entry.Mood = "Calm"
-    entry.Energy = "Medium"
-    entry.Highlight = "Wrote tests"
-    entry.Todos = append(entry.Todos, domain.Todo{Text: "Do thing", Done: false})
-    entry.Backlog = append(entry.Backlog, domain.Todo{Text: "Carryover", Done: false})
-    entry.Questions["What did I learn?"] = "Testing roundtrip"
+	date := time.Date(2025, 12, 30, 0, 0, 0, 0, time.UTC)
+	entry := domain.NewJournalEntry(date, "daily-human-dev")
+	entry.Mood = "Calm"
+	entry.Energy = "Medium"
+	entry.Highlight = "Wrote tests"
+	entry.Todos = append(entry.Todos, domain.Todo{Text: "Do thing", Done: false})
+	entry.Backlog = append(entry.Backlog, domain.Todo{Text: "Carryover", Done: false})
+	entry.Questions["What did I learn?"] = "Testing roundtrip"
 
-    md, err := GenerateMarkdown(entry)
-    if err != nil {
-        t.Fatalf("GenerateMarkdown error: %v", err)
-    }
+	md, err := GenerateMarkdown(entry)
+	if err != nil {
+		t.Fatalf("GenerateMarkdown error: %v", err)
+	}
 
-    parsed, err := ParseMarkdown(md)
-    if err != nil {
-        t.Fatalf("ParseMarkdown error: %v", err)
-    }
+	parsed, err := ParseMarkdown(md)
+	if err != nil {
+		t.Fatalf("ParseMarkdown error: %v", err)
+	}
 
-    if !parsed.Date.Equal(entry.Date) {
-        t.Fatalf("date mismatch: got %v want %v", parsed.Date, entry.Date)
-    }
+	if !parsed.Date.Equal(entry.Date) {
+		t.Fatalf("date mismatch: got %v want %v", parsed.Date, entry.Date)
+	}
 
-    if parsed.Template != entry.Template {
-        t.Fatalf("template mismatch: got %s want %s", parsed.Template, entry.Template)
-    }
+	if parsed.Template != entry.Template {
+		t.Fatalf("template mismatch: got %s want %s", parsed.Template, entry.Template)
+	}
 
-    if len(parsed.Todos) != 1 || parsed.Todos[0].Text != "Do thing" {
-        t.Fatalf("todos mismatch: %v", parsed.Todos)
-    }
+	if len(parsed.Todos) != 1 || parsed.Todos[0].Text != "Do thing" {
+		t.Fatalf("todos mismatch: %v", parsed.Todos)
+	}
 
-    if len(parsed.Backlog) != 1 || parsed.Backlog[0].Text != "Carryover" {
-        t.Fatalf("backlog mismatch: %v", parsed.Backlog)
-    }
+	if len(parsed.Backlog) != 1 || parsed.Backlog[0].Text != "Carryover" {
+		t.Fatalf("backlog mismatch: %v", parsed.Backlog)
+	}
 
-    // Keys may include emoji prefixes; find answer by substring match
-    found := false
-    for k, v := range parsed.Questions {
-        if strings.Contains(k, "What did I learn") && v == "Testing roundtrip" {
-            found = true
-            break
-        }
-    }
-    if !found {
-        t.Fatalf("questions mismatch: %v", parsed.Questions)
-    }
+	// Keys may include emoji prefixes; find answer by substring match
+	found := false
+	for k, v := range parsed.Questions {
+		if strings.Contains(k, "What did I learn") && v == "Testing roundtrip" {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Fatalf("questions mismatch: %v", parsed.Questions)
+	}
 }
 
 func TestParseMalformed(t *testing.T) {
-    bad := []byte("no-frontmatter-here")
-    if _, err := ParseMarkdown(bad); err == nil {
-        t.Fatalf("expected error parsing malformed markdown")
-    }
+	bad := []byte("no-frontmatter-here")
+	if _, err := ParseMarkdown(bad); err == nil {
+		t.Fatalf("expected error parsing malformed markdown")
+	}
 }

internal/todo/todo_test.go

@@ -1,18 +1,18 @@
 package todo
 
 import (
-    "path/filepath"
-    "strings"
-    "testing"
-    "time"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
 
-    "journal-cli/internal/fs"
+	"journal-cli/internal/fs"
 )
 
 func TestGetBacklog(t *testing.T) {
-    dir := t.TempDir()
-    // Create a sample previous day file
-    md := `---
+	dir := t.TempDir()
+	// Create a sample previous day file
+	md := `---
 date: 2025-12-29
 template: daily-human-dev
 ---
@@ -25,47 +25,47 @@ template: daily-human-dev
 - [ ] Backlogged task
 `
 
-    path := filepath.Join(dir, "2025-12-29.md")
-    if err := fs.WriteFile(path, []byte(md)); err != nil {
-        t.Fatalf("failed to write test file: %v", err)
-    }
+	path := filepath.Join(dir, "2025-12-29.md")
+	if err := fs.WriteFile(path, []byte(md)); err != nil {
+		t.Fatalf("failed to write test file: %v", err)
+	}
 
-    // Ensure fs.Exists returns true
-    if !fs.Exists(path) {
-        t.Fatalf("expected file to exist: %s", path)
-    }
+	// Ensure fs.Exists returns true
+	if !fs.Exists(path) {
+		t.Fatalf("expected file to exist: %s", path)
+	}
 
-    items, err := GetBacklog(path)
-    if err != nil {
-        t.Fatalf("GetBacklog returned error: %v", err)
-    }
+	items, err := GetBacklog(path)
+	if err != nil {
+		t.Fatalf("GetBacklog returned error: %v", err)
+	}
 
-    if len(items) != 2 {
-        t.Fatalf("expected 2 backlog items (1 todo + 1 backlog), got %d", len(items))
-    }
+	if len(items) != 2 {
+		t.Fatalf("expected 2 backlog items (1 todo + 1 backlog), got %d", len(items))
+	}
 
-    // Basic content checks
-    found := map[string]bool{}
-    for _, it := range items {
-        found[it.Text] = true
-    }
-    if !found["Unchecked task"] || !found["Backlogged task"] {
-        t.Fatalf("unexpected backlog items: %v", found)
-    }
+	// Basic content checks
+	found := map[string]bool{}
+	for _, it := range items {
+		found[it.Text] = true
+	}
+	if !found["Unchecked task"] || !found["Backlogged task"] {
+		t.Fatalf("unexpected backlog items: %v", found)
+	}
 }
 
 func TestGetPreviousJournalPath(t *testing.T) {
-    base := "/tmp/journal"
-    // Use a known date
-    // Previous path should end with 2025-12-29.md
-    // We don't assert separator specifics, just suffix
-    got := GetPreviousJournalPath(base, fsTime())
-    if !strings.HasSuffix(got, "2025-12-29.md") {
-        t.Fatalf("unexpected previous path: %s", got)
-    }
+	base := "/tmp/journal"
+	// Use a known date
+	// Previous path should end with 2025-12-29.md
+	// We don't assert separator specifics, just suffix
+	got := GetPreviousJournalPath(base, fsTime())
+	if !strings.HasSuffix(got, "2025-12-29.md") {
+		t.Fatalf("unexpected previous path: %s", got)
+	}
 }
 
 // fsTime returns a fixed date used by tests
 func fsTime() time.Time {
-    return time.Date(2025, 12, 30, 0, 0, 0, 0, time.UTC)
+	return time.Date(2025, 12, 30, 0, 0, 0, 0, time.UTC)
 }

internal/updater/self_update.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"time"
 )
 
 const repo = "ops295/journal-cli"
@@ -24,8 +25,13 @@ type Release struct {
 func Update() error {
 	fmt.Println("🔍 Checking latest version...")
 
+	// Create HTTP client with timeout
+	client := &http.Client{
+		Timeout: 30 * time.Second,
+	}
+
 	// 1. Fetch latest release info
-	resp, err := http.Get("https://api.github.com/repos/" + repo + "/releases/latest")
+	resp, err := client.Get("https://api.github.com/repos/" + repo + "/releases/latest")
 	if err != nil {
 		return fmt.Errorf("failed to fetch latest release: %w", err)
 	}
@@ -47,7 +53,7 @@ func Update() error {
 		runtime.GOOS,
 		runtime.GOARCH,
 	)
-	
+
 	// Windows binaries usually have .exe extension
 	if runtime.GOOS == "windows" {
 		target += ".exe"
@@ -71,14 +77,14 @@ func Update() error {
 	tmpFile := filepath.Join(os.TempDir(), "journal-new")
 	// Ensure we don't conflict if multiple runs or stale files
 	_ = os.Remove(tmpFile)
-	
+
 	out, err := os.Create(tmpFile)
 	if err != nil {
 		return fmt.Errorf("failed to create temp file: %w", err)
 	}
 	defer out.Close()
 
-	respBin, err := http.Get(url)
+	respBin, err := client.Get(url)
 	if err != nil {
 		return fmt.Errorf("failed to download binary: %w", err)
 	}
@@ -96,7 +102,7 @@ func Update() error {
 	if err := out.Chmod(0755); err != nil {
 		return fmt.Errorf("failed to make binary executable: %w", err)
 	}
-	
+
 	// Close the file explicitly before renaming to ensure all writes are flushed
 	out.Close()
 
@@ -105,14 +111,14 @@ func Update() error {
 	if err != nil {
 		return fmt.Errorf("failed to locate current executable: %w", err)
 	}
-	
+
 	// Resolve symlinks if any (common in some installs), though os.Executable usually handles this.
 	// We'll stick to what os.Executable returns for now.
 
 	backup := current + ".bak"
 
 	fmt.Println("🔄 Replacing binary...")
-	
+
 	// First move the current binary to .bak
 	if err := os.Rename(current, backup); err != nil {
 		// If permission denied, give a helpful hint
@@ -128,7 +134,7 @@ func Update() error {
 		_ = os.Rename(backup, current)
 		return fmt.Errorf("failed to install new binary: %w", err)
 	}
-	
+
 	// Cleanup backup
 	_ = os.Remove(backup)
 

internal/version/version.go

@@ -28,4 +28,3 @@ func GetVersionString() string {
 	}
 	return fmt.Sprintf("v%s", Version)
 }
-