fix(live): make chat download more crash resistant

[Zibbp]

• make live chat archive more resilient to crashes

[coderabbitai]

Walkthrough

The change replaces direct JSON array appends with an atomic write strategy using temporary files, includes recovery logic for interrupted writes, adds helper functions for JSON inspection and parsing, and improves crash-safety through directory sync and atomic file replacement.

Changes

Cohort / File(s)	Summary
Atomic File Writing internal/exec/twitch.go	Replaced direct JSON array appends with atomic write strategy using temporary files and directory sync. Added helper functions for JSON inspection and recovery logic for interrupted writes. Reworked appendMessageToJSONArray with improved crash-safety and error handling. Introduced path/filepath import.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: making chat downloads more crash-resistant through atomic writes and recovery logic.
Description check	✅ Passed	The description is related to the changeset, correctly stating the goal to make live chat archive more resilient to crashes.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch misc-fixes-02-15

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/exec/twitch.go (1)

119-124: ⚠️ Potential issue | 🟠 Major

Flock on the data file is ineffective after atomic rename — concurrent writers can lose messages.

After os.Rename(tmpName, filename) replaces the file, the lock held via f.Fd() protects the old (now unlinked) inode, not the new file. A second goroutine/process that opened filename and is blocked on Flock will eventually acquire the lock on the stale inode, read stale data, and its subsequent rename will silently overwrite the first writer's message.

If concurrent access is a real concern (as the comment on line 118 suggests), consider locking a separate, stable lock file (e.g., filename + ".lock") that is never renamed or replaced. This ensures all writers contend on the same inode.

Sketch

-	f, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE, 0o644)
+	// Use a dedicated lock file so the lock survives atomic renames.
+	lockPath := filename + ".lock"
+	lf, err := os.OpenFile(lockPath, os.O_CREATE|os.O_RDWR, 0o644)
+	if err != nil {
+		return fmt.Errorf("failed to open lock file: %w", err)
+	}
+	defer lf.Close()
+	if err := syscall.Flock(int(lf.Fd()), syscall.LOCK_EX); err != nil {
+		return fmt.Errorf("failed to lock file: %w", err)
+	}
+	defer func() { _ = syscall.Flock(int(lf.Fd()), syscall.LOCK_UN) }()
+
+	f, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE, 0o644)
 	if err != nil {
 		return fmt.Errorf("failed to open file: %w", err)
 	}
-	defer f.Close()
+	defer f.Close()
-
-	if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil {
-		return fmt.Errorf("failed to lock file: %w", err)
-	}
-	defer func() {
-		_ = syscall.Flock(int(f.Fd()), syscall.LOCK_UN)
-	}()

Also applies to: 189-191

🧹 Nitpick comments (2)

internal/exec/twitch.go (2)

145-148: Defer removes the temp file even on success — benign but worth noting.

After os.Rename(tmpName, filename) succeeds on line 189, the deferred os.Remove(tmpName) will attempt to remove the (now-renamed) path, which no longer exists — this is a harmless no-op. Similarly, tmp.Close() on the already-closed file descriptor returns an error that is discarded. This is safe as-is, but if you want to be explicit you could nil out tmpName after a successful rename:

// after successful rename
tmpName = "" // prevent deferred Remove

and guard the defer with if tmpName != "" { ... }.

---

253-271: Recovery loop could spin indefinitely on unexpected file content.

The for loop starting at line 255 walks backwards trimming trailing commas. If the file content somehow consists entirely of commas (or commas and whitespace) after the opening [, the loop will eventually hit found==false on line 260-262 and exit. However, if there's a non-comma, non-space byte that is also not valid JSON element content (e.g., a stray ] without matching [ from a deeply corrupted file), the loop will break and treat it as valid prefix content to copy — which might propagate the corruption.

This is an edge case in an already-best-effort recovery path, so it's likely acceptable, but worth a note for future maintainers.