fix: prevent transient scheduled task write failures

Cron task metadata writes can overlap closely enough that a timestamp-only
$temp filename is not stable. Harden the atomic write path by using a
collision-resistant temp name and retrying once when rename reports ENOENT.

Constraint: Scheduled task persistence must keep atomic replace semantics in ~/.claude/scheduled_tasks.json
Rejected: Add explicit file locking around every scheduled task write | more coordination overhead than the observed transient rename race warrants
Confidence: high
Scope-risk: narrow
Directive: Keep scheduled task writes collision-resistant and retriable; do not revert to timestamp-only temp file names without reproducing concurrent writes
Tested: bun test src/server/__tests__/scheduled-tasks.test.ts
Not-tested: bun test src/server/__tests__/cron-scheduler.test.ts (existing timeouts and unrelated assertion failures in this checkout)
This commit is contained in:
程序员阿江(Relakkes) 2026-04-08 23:21:28 +08:00
parent 3c5290eec1
commit ce92de29f0
2 changed files with 63 additions and 15 deletions

View File

@ -2,7 +2,7 @@
* Unit tests for CronService, SearchService, and Scheduled Tasks API
*/
import { describe, it, expect, beforeEach, afterEach } from 'bun:test'
import { describe, it, expect, beforeEach, afterEach, spyOn } from 'bun:test'
import * as fs from 'fs/promises'
import * as path from 'path'
import * as os from 'os'
@ -139,6 +139,41 @@ describe('CronService', () => {
service.createTask({ cron: '* * * * *', prompt: '' }),
).rejects.toThrow()
})
it('should retry the atomic write when rename returns ENOENT', async () => {
const originalRename = fs.rename
let renameCalls = 0
const renameSpy = spyOn(fs, 'rename')
renameSpy.mockImplementation(async (...args) => {
renameCalls += 1
if (renameCalls === 1) {
const error = new Error(
'ENOENT: no such file or directory, rename tmp -> scheduled_tasks.json',
) as NodeJS.ErrnoException
error.code = 'ENOENT'
throw error
}
return originalRename(...args)
})
try {
const task = await service.createTask({
cron: '0 9 * * *',
prompt: 'Retry rename once',
})
const tasks = await service.listTasks()
expect(task.id).toBeDefined()
expect(tasks).toHaveLength(1)
expect(tasks[0]?.prompt).toBe('Retry rename once')
expect(renameCalls).toBe(2)
} finally {
renameSpy.mockRestore()
}
})
})
// ─── SearchService tests ────────────────────────────────────────────────────

View File

@ -32,6 +32,8 @@ type TasksFile = {
tasks: CronTask[]
}
const TASKS_FILE_WRITE_ATTEMPTS = 2
export class CronService {
/** 任务文件路径 */
private getTasksFilePath(): string {
@ -134,21 +136,32 @@ export class CronService {
private async writeTasksFile(data: TasksFile): Promise<void> {
const filePath = this.getTasksFilePath()
const dir = path.dirname(filePath)
await fs.mkdir(dir, { recursive: true })
const contents = JSON.stringify(data, null, 2) + '\n'
let lastError: Error | undefined
const tmpFile = `${filePath}.tmp.${Date.now()}`
try {
await fs.writeFile(
tmpFile,
JSON.stringify(data, null, 2) + '\n',
'utf-8',
)
await fs.rename(tmpFile, filePath)
} catch (err) {
await fs.unlink(tmpFile).catch(() => {})
throw ApiError.internal(
`Failed to write scheduled tasks: ${(err as Error).message}`,
)
for (let attempt = 0; attempt < TASKS_FILE_WRITE_ATTEMPTS; attempt++) {
const tmpFile = `${filePath}.tmp.${process.pid}.${Date.now()}.${crypto.randomBytes(6).toString('hex')}`
try {
await fs.mkdir(dir, { recursive: true })
await fs.writeFile(tmpFile, contents, 'utf-8')
await fs.rename(tmpFile, filePath)
return
} catch (err) {
lastError = err as Error
await fs.unlink(tmpFile).catch(() => {})
if (
(err as NodeJS.ErrnoException).code !== 'ENOENT' ||
attempt === TASKS_FILE_WRITE_ATTEMPTS - 1
) {
break
}
}
}
throw ApiError.internal(
`Failed to write scheduled tasks: ${lastError?.message ?? 'unknown error'}`,
)
}
}