fix(snapshot): remove orphaned tmp_pack files during cleanup

[Olusammytee]

Summary

• remove orphaned mp_pack_* files from snapshot git storage during scheduled cleanup
• run the same cleanup path when snapshot git add or write-tree fails
• include cleanup counts in logs for easier diagnosis of disk-space leaks

Fixes #471

[github-actions]

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

1. Open an issue describing the bug/feature (if one doesn't exist)
2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

[kiloconnect]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 files)

• packages/opencode/src/snapshot/index.ts

[Copilot]

Pull request overview

Removes orphaned tmp_pack_* files from the snapshot git storage to prevent disk-space leaks, and adds logging/cleanup on failure paths to aid diagnosis (Fixes #471).

Changes:

• Run cleanupTmpPacks() during scheduled snapshot cleanup and include removal counts in logs.
• Add explicit exit-code handling for git add and git write-tree in Snapshot.track(), invoking tmp-pack cleanup on failure.
• Introduce internal helper cleanupTmpPacks() to delete objects/pack/tmp_pack_* files.

Comments suppressed due to low confidence (2)

packages/opencode/src/snapshot/index.ts:286

• cleanupTmpPacks() returns tmpPacks.length even though individual unlink calls can fail (errors are swallowed). This makes tmpPackRemoved misleading in logs and can hide partial cleanup failures. Consider either (a) counting only successful deletions (e.g., via Promise.allSettled) or (b) renaming the metric to tmpPackFound/tmpPackCandidates and separately tracking failures.

    const files = await fs.readdir(pack).catch(() => [])
    const tmpPacks = files.filter((file) => file.startsWith("tmp_pack_"))
    await Promise.all(tmpPacks.map((file) => fs.unlink(path.join(pack, file)).catch(() => undefined)))
    return tmpPacks.length

packages/opencode/src/snapshot/index.ts:286

• This PR adds new cleanup behavior (cleanupTmpPacks) and new early-return paths for track() when git add / write-tree fail, but there are no tests covering tmp-pack deletion or the new failure handling. Since packages/opencode/test/snapshot/snapshot.test.ts already has extensive coverage for snapshots, consider adding a test that creates a objects/pack/tmp_pack_* file under the snapshot gitdir and asserts Snapshot.cleanup() (and/or the failure path) removes it.

  async function cleanupTmpPacks(git: string) {
    const pack = path.join(git, "objects", "pack")
    const files = await fs.readdir(pack).catch(() => [])
    const tmpPacks = files.filter((file) => file.startsWith("tmp_pack_"))
    await Promise.all(tmpPacks.map((file) => fs.unlink(path.join(pack, file)).catch(() => undefined)))
    return tmpPacks.length

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

cleanupTmpPacks(git) is triggered on git add / write-tree failures. If those failures are caused by a concurrent Snapshot.cleanup() running git gc (or another git maintenance operation) in the same repo, this could delete tmp_pack_* files while repack is still in progress and potentially corrupt the snapshot repository. Consider serializing snapshot git operations + cleanup via an in-process lock (similar to FileTime.withLock) or skipping tmp-pack deletion when a gc/repack lock is present (e.g., gc.pid / objects/pack/*.lock).