Clean up temp dir if opening c1file results in an error.

[ggreer]

Summary by CodeRabbit

• Bug Fixes
  • Improved validation to reject negative encoder concurrency values, preventing invalid configurations.
  • Enhanced error handling to attempt cleanup of temporary resources on failure and surface any cleanup errors alongside the original error.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

---

Walkthrough

Adds an early validation rejecting negative encoderConcurrency; on underlying C1File creation failure, invokes cleanupDbDir(dbFilePath, err) and returns the original error augmented with any cleanup error. cleanupDbDir now stats the path, skips cleanup for missing/non-file paths, and merges cleanup errors.

Changes

Cohort / File(s)	Summary
C1File creation & cleanup pkg/dotc1z/c1file.go	Add early check encoderConcurrency >= 0 in NewC1ZFile; remove redundant later check; on underlying C1File creation failure return cleanupDbDir(dbFilePath, err); extend cleanupDbDir to stat dbFilePath, skip cleanup for non-existent or directory paths, perform parent-dir cleanup for files, and augment returned error with any cleanup failure.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant NewC1ZFile
  participant UnderlyingC1File
  participant OS as "OS/fs"

  Caller->>NewC1ZFile: call NewC1ZFile(opts)
  NewC1ZFile->>NewC1ZFile: apply options
  NewC1ZFile-->>Caller: if encoderConcurrency < 0 → return error
  NewC1ZFile->>UnderlyingC1File: attempt create/open
  UnderlyingC1File-->>NewC1ZFile: error
  NewC1ZFile->>OS: stat(dbFilePath)
  alt path missing
    OS-->>NewC1ZFile: ENOENT
    NewC1ZFile-->>Caller: return original error
  else path is directory
    OS-->>NewC1ZFile: mode=dir
    NewC1ZFile-->>Caller: return original error (skip cleanup)
  else path is file
    OS->>OS: remove parent temp dir
    OS-->>NewC1ZFile: cleanup result
    NewC1ZFile-->>Caller: return original error augmented with cleanup result (if any)
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I checked the numbers, neat and spry,
Nixed negatives that slipped on by.
I swept the tmpdir, left no mess,
Wrapped errors gently—no distress.
Hop along, builds pass by! 🌿

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding cleanup of temp directory when c1file opening fails, which matches the core functionality described in the raw summary.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch ggreer/cleanup-temp-dir-on-error

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/dotc1z/c1file.go`:
- Around line 227-229: The validation for options.encoderConcurrency incorrectly
reports "must be greater than 0" while the check allows 0 (which is valid and
uses GOMAXPROCS); update the error message in the encoder concurrency check (the
code referencing options.encoderConcurrency in c1file.go) to state that the
value cannot be negative (e.g., "encoder concurrency must be >= 0" or "encoder
concurrency cannot be negative; 0 uses GOMAXPROCS") so it matches the validation
logic and comment.
- Around line 253-256: The call to cleanupDbDir is passing tmpDir (a directory)
but cleanupDbDir expects a file path and does filepath.Dir(), which would remove
the parent temp folder; change the error cleanup call in the NewC1File error
branch to pass dbFilePath instead of tmpDir (i.e., in the block after
NewC1File(ctx, dbFilePath, c1fopts...) where err != nil) so cleanupDbDir
receives the actual DB file path and removes the correct directory.