FOLLOW-UP: Fix defer-in-loop temp-file leak in validateMermaidDiagrams (from PR #272)

[adnaan]

Context

Pre-existing bug surfaced during the Claude review of PR #272 (feat(page): site-wide include root via ParseFileInSite). Not introduced by that PR — it lives in cmd/tinkerdown/commands/validate.go's validateMermaidDiagrams function and predates the include-site-root work.

Source PR: #272
Suggested by: @claude (round-4 review)

The bug

// cmd/tinkerdown/commands/validate.go (~line 214)
defer os.Remove(tmpFile) // inside a for loop

defer registrations run when the enclosing function returns, not at the end of each loop iteration. For a validator that walks N markdown files with M mermaid diagrams each, that means N×M temp files stay on disk (and open if any handle is held) until validateMermaidDiagrams finally returns.

Practical consequence: a directory with hundreds of mermaid-bearing pages can accumulate hundreds of stale temp files during a single tinkerdown validate run, only cleaned up at process exit. Not catastrophic on most machines, but unbounded by design and surprising.

Suggested fix

Replace the deferred remove with an explicit call at the end of the loop body, after the temp file is no longer needed (typically after the chromedp.Run that uses it):

for _, diagram := range diagrams {
    tmpFile := writeTempDiagram(...)
    err := chromedp.Run(ctx, ...)
    os.Remove(tmpFile)            // explicit, runs each iteration
    if err != nil { ... }
}

If the loop has multiple early-continue/return points where cleanup matters, the cleanest pattern is to hoist the loop body into a small helper function that owns the lifecycle of one tmpFile — defer inside a function-per-iteration is correct.

Acceptance

• Run tinkerdown validate <large-mermaid-corpus> and observe lsof / temp-dir size during the run — both should stay bounded across iterations rather than growing N×M.
• No regression in mermaid-validation behavior.

Original review excerpt

defer in a loop defers until the function returns, not the next iteration, so all temp files stay open for the lifetime of validateMermaidDiagrams. Not introduced here but since you're in the file — easy to fix by calling os.Remove(tmpFile) at the end of the loop body (after chromedp.Run) instead of using defer.

— from PR #272 round-4 Claude review

---

This issue was filed automatically from a PR review follow-up.

[claude]

Claude finished @adnaan's task in 1m 13s —— View job • claude/issue-273-20260509-1018 • Create PR ➔

---

Fix: defer-in-loop temp-file leak in validateMermaidDiagrams

• Gather context and read issue
• Read cmd/tinkerdown/commands/validate.go to understand current structure
• Implement fix for defer-in-loop bug
• Commit and push changes

What was done

The defer os.Remove(tmpFile) at validate.go:216 was inside the for i, match := range matches loop. Because defer runs when the enclosing function returns — not at the end of each loop iteration — all N×M temp files accumulated on disk until validateMermaidDiagrams returned.

Fix: Hoisted the per-diagram loop body into a new helper validateOneMermaidDiagram(ctx, i, mermaidCode). defer os.Remove(tmpFile) inside a function is correct — it fires when that function returns, so each temp file is cleaned up after each diagram is validated.

// Before: defer runs at validateMermaidDiagrams() return — all N×M files accumulate
for i, match := range matches {
    tmpFile := fmt.Sprintf("/tmp/mermaid-validate-%d.html", i)
    os.WriteFile(tmpFile, ...)
    defer os.Remove(tmpFile)   // ← bug: deferred to outer function
    chromedp.Run(ctx, ...)
}

// After: helper function scopes the defer correctly — one file at a time
for i, match := range matches {
    hasError, err := validateOneMermaidDiagram(ctx, i, match[1])
    ...
}

func validateOneMermaidDiagram(ctx context.Context, i int, mermaidCode string) (bool, error) {
    tmpFile := fmt.Sprintf("/tmp/mermaid-validate-%d.html", i)
    os.WriteFile(tmpFile, ...)
    defer os.Remove(tmpFile)   // ← correct: fires when this function returns
    chromedp.Run(ctx, ...)
}