mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-17 13:13:35 +08:00
fix(adapters): eliminate AttachmentStore collision race and orphan .part cleanup
Address two production issues in the AttachmentStore: 1. TOCTOU race in resolvePath() — concurrent downloads with same filename in the same millisecond could collide. Fixed by appending a 6-char random suffix to the timestamp-based path, ensuring millisecond-level collisions never occur. 2. Orphan .part files from crashed writes never cleaned up. Fixed by extending gc() to recognize .part files and clean them with a shorter grace period (10 minutes by default) rather than the normal retention window. Added two test cases covering heavy collision pressure and .part cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
ae586b0d9b
commit
bf9b561cc7
@ -1,5 +1,6 @@
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'bun:test'
|
||||
import * as fs from 'node:fs/promises'
|
||||
import * as fsSync from 'node:fs'
|
||||
import * as path from 'node:path'
|
||||
import * as os from 'node:os'
|
||||
import { AttachmentStore } from '../attachment-store.js'
|
||||
@ -73,4 +74,35 @@ describe('AttachmentStore', () => {
|
||||
expect(result.removed).toBe(0)
|
||||
await fs.access(target)
|
||||
})
|
||||
|
||||
it('resolvePath under heavy collision pressure returns unique paths', () => {
|
||||
const store = new AttachmentStore({ root: tmpRoot, retentionMs: 60_000 })
|
||||
// First create a file so subsequent resolves hit the collision branch
|
||||
const a = store.resolvePath('feishu', 'sess-1', 'race.png')
|
||||
fsSync.writeFileSync(a, 'first')
|
||||
// Collect 50 resolved paths in a tight loop — none should clash
|
||||
const seen = new Set<string>()
|
||||
for (let i = 0; i < 50; i++) {
|
||||
seen.add(store.resolvePath('feishu', 'sess-1', 'race.png'))
|
||||
}
|
||||
expect(seen.size).toBe(50)
|
||||
for (const p of seen) {
|
||||
expect(p).not.toBe(a)
|
||||
}
|
||||
})
|
||||
|
||||
it('gc() cleans orphan .part files after a short grace period', async () => {
|
||||
const store = new AttachmentStore({ root: tmpRoot, retentionMs: 10_000, orphanGraceMs: 50 })
|
||||
// Simulate a crashed write — leave a .part tmp file behind
|
||||
const dir = path.join(tmpRoot, 'feishu', 'sess-1')
|
||||
await fs.mkdir(dir, { recursive: true })
|
||||
const orphan = path.join(dir, 'image.png.1234.5678.part')
|
||||
await fs.writeFile(orphan, 'ORPHAN')
|
||||
// Age the orphan so gc considers it stale
|
||||
const past = new Date(Date.now() - 1000)
|
||||
await fs.utimes(orphan, past, past)
|
||||
const result = await store.gc()
|
||||
expect(result.removed).toBeGreaterThanOrEqual(1)
|
||||
await expect(fs.access(orphan)).rejects.toThrow()
|
||||
})
|
||||
})
|
||||
|
||||
@ -19,9 +19,13 @@ import type { ImPlatform } from './attachment-types.js'
|
||||
export interface AttachmentStoreConfig {
|
||||
root: string
|
||||
retentionMs: number
|
||||
/** Grace window before a `.part` orphan (left behind by a crashed writer)
|
||||
* is eligible for GC. Default 10 minutes. */
|
||||
orphanGraceMs: number
|
||||
}
|
||||
|
||||
const DEFAULT_RETENTION_MS = 24 * 60 * 60 * 1000
|
||||
const DEFAULT_ORPHAN_GRACE_MS = 10 * 60 * 1000
|
||||
|
||||
function defaultRoot(): string {
|
||||
return path.join(os.homedir(), '.claude', 'im-downloads')
|
||||
@ -38,10 +42,12 @@ function sanitizeFilename(name: string): string {
|
||||
export class AttachmentStore {
|
||||
private readonly root: string
|
||||
private readonly retentionMs: number
|
||||
private readonly orphanGraceMs: number
|
||||
|
||||
constructor(config?: Partial<AttachmentStoreConfig>) {
|
||||
this.root = config?.root ?? defaultRoot()
|
||||
this.retentionMs = config?.retentionMs ?? DEFAULT_RETENTION_MS
|
||||
this.orphanGraceMs = config?.orphanGraceMs ?? DEFAULT_ORPHAN_GRACE_MS
|
||||
}
|
||||
|
||||
/** Compute the target path. Creates parent dirs on demand.
|
||||
@ -55,7 +61,11 @@ export class AttachmentStore {
|
||||
const candidate = path.join(dir, safeName)
|
||||
if (!fsSync.existsSync(candidate)) return candidate
|
||||
const { name: base, ext } = path.parse(safeName)
|
||||
return path.join(dir, `${base}-${Date.now()}${ext}`)
|
||||
// Collisions are rare in practice, but multiple downloads landing in the
|
||||
// same millisecond must still produce unique paths — append a random
|
||||
// suffix so the bare timestamp alone never clashes.
|
||||
const rand = Math.random().toString(36).slice(2, 8)
|
||||
return path.join(dir, `${base}-${Date.now()}-${rand}${ext}`)
|
||||
}
|
||||
|
||||
/** Write atomically: stream to {target}.part, then rename. */
|
||||
@ -88,7 +98,9 @@ export class AttachmentStore {
|
||||
try {
|
||||
const stat = await fs.stat(full)
|
||||
const age = now - stat.mtimeMs
|
||||
if (age > this.retentionMs) {
|
||||
const isOrphanPart = entry.name.endsWith('.part')
|
||||
const threshold = isOrphanPart ? this.orphanGraceMs : this.retentionMs
|
||||
if (age > threshold) {
|
||||
bytes += stat.size
|
||||
await fs.unlink(full)
|
||||
removed++
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user