Skip to content

Commit d22bae3

Browse files
committed
fix(log): await cleanup and sort files before deletion
- Await cleanup() in init() to prevent race condition where cleanup could run concurrently with file creation - Sort files before slicing to ensure oldest files are deleted, not arbitrary files based on filesystem order The glob scan returns files in arbitrary order, so slice(0, -10) was deleting random files instead of the oldest ones. Adding sort() ensures lexicographic order which matches chronological order for the YYYY-MM-DDTHHMMSS.log filename format.
1 parent 2e4fe97 commit d22bae3

File tree

2 files changed

+142
-2
lines changed

2 files changed

+142
-2
lines changed

packages/opencode/src/util/log.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export namespace Log {
5757

5858
export async function init(options: Options) {
5959
if (options.level) level = options.level
60-
cleanup(Global.Path.log)
60+
await cleanup(Global.Path.log)
6161
if (options.print) return
6262
logpath = path.join(
6363
Global.Path.log,
@@ -83,7 +83,7 @@ export namespace Log {
8383
)
8484
if (files.length <= 5) return
8585

86-
const filesToDelete = files.slice(0, -10)
86+
const filesToDelete = files.sort().slice(0, -10)
8787
await Promise.all(filesToDelete.map((file) => fs.unlink(file).catch(() => {})))
8888
}
8989

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
import { describe, test, expect, beforeEach, afterEach, setSystemTime } from "bun:test"
2+
import { Log } from "../../src/util/log"
3+
import { Global } from "../../src/global"
4+
import fs from "fs/promises"
5+
import path from "path"
6+
7+
describe("Log.init", () => {
8+
const CLEANUP_THRESHOLD = 5
9+
10+
beforeEach(clearLogDir)
11+
afterEach(clearLogDir)
12+
13+
test("creates a new log file with timestamp name", async () => {
14+
setSystemTime(new Date("2026-01-07T15:00:00.000Z"))
15+
16+
await Log.init({ print: false })
17+
18+
const logFile = Log.file()
19+
expect(logFile).toBeTruthy()
20+
expect(path.basename(logFile)).toBe("2026-01-07T150000.log")
21+
expect(await Bun.file(logFile).exists()).toBe(true)
22+
23+
setSystemTime()
24+
})
25+
26+
test("creates dev.log when dev option is true", async () => {
27+
await Log.init({ print: false, dev: true })
28+
29+
const logFile = Log.file()
30+
expect(path.basename(logFile)).toBe("dev.log")
31+
expect(await Bun.file(logFile).exists()).toBe(true)
32+
})
33+
34+
test("does not delete files when at threshold", async () => {
35+
const startingAt = new Date("2020-01-01")
36+
const oldFiles = createLogFiles(CLEANUP_THRESHOLD, startingAt, ONE_DAY)
37+
await writeFiles(oldFiles)
38+
39+
await Log.init({ print: false })
40+
41+
for (const filename of oldFiles) {
42+
const file = getLogFile(filename)
43+
expect(await file.exists()).toBe(true)
44+
}
45+
})
46+
47+
test("does not delete files when threshold exceeded by 5", async () => {
48+
const startingAt = new Date("2020-01-01")
49+
const oldFiles = createLogFiles(CLEANUP_THRESHOLD + 5, startingAt, ONE_DAY)
50+
await writeFiles(oldFiles)
51+
52+
await Log.init({ print: false })
53+
54+
for (const filename of oldFiles) {
55+
const file = getLogFile(filename)
56+
expect(await file.exists()).toBe(true)
57+
}
58+
})
59+
60+
test("deletes the oldest file when threshold exceeded by 6", async () => {
61+
const startingAt = new Date("2020-01-01")
62+
const oldFiles = createLogFiles(CLEANUP_THRESHOLD + 6, startingAt, ONE_DAY)
63+
await writeFiles(oldFiles)
64+
65+
await Log.init({ print: false })
66+
67+
const file = getLogFile("2020-01-01T000000.log")
68+
expect(await file.exists()).toBe(false)
69+
})
70+
71+
test("preserves the newest 10 files when threshold exceeded by 6", async () => {
72+
const startingAt = new Date("2020-01-01")
73+
const oldFiles = createLogFiles(CLEANUP_THRESHOLD + 6, startingAt, ONE_DAY)
74+
const newestFiles = oldFiles.slice(-10)
75+
await writeFiles(oldFiles)
76+
77+
await Log.init({ print: false })
78+
79+
for (const filename of newestFiles) {
80+
const file = getLogFile(filename)
81+
expect(await file.exists()).toBe(true)
82+
}
83+
})
84+
85+
test("does not delete dev.log during cleanup", async () => {
86+
const startingAt = new Date("2020-01-01")
87+
const oldFiles = createLogFiles(CLEANUP_THRESHOLD + 6, startingAt, ONE_DAY)
88+
await writeFiles([...oldFiles, "dev.log"])
89+
90+
await Log.init({ print: false })
91+
92+
const file = getLogFile("dev.log")
93+
expect(await file.exists()).toBe(true)
94+
})
95+
96+
test("creates new log file after cleanup runs", async () => {
97+
setSystemTime(new Date("2025-11-02T00:00:00.000Z"))
98+
const startingAt = new Date("2020-01-01")
99+
const oldFiles = createLogFiles(CLEANUP_THRESHOLD + 6, startingAt, ONE_DAY)
100+
await writeFiles(oldFiles)
101+
102+
await Log.init({ print: false })
103+
104+
const oldestFile = getLogFile("2020-01-01T000000.log")
105+
const newFile = getLogFile("2025-11-02T000000.log")
106+
expect(await oldestFile.exists()).toBe(false)
107+
expect(await newFile.exists()).toBe(true)
108+
109+
setSystemTime()
110+
})
111+
})
112+
113+
async function clearLogDir() {
114+
const existingLogFiles = await fs.readdir(Global.Path.log).catch(() => [])
115+
116+
await Promise.all(
117+
existingLogFiles.map((existingLogFile) => {
118+
const filepath = path.join(Global.Path.log, existingLogFile)
119+
return fs.unlink(filepath).catch(() => {})
120+
}),
121+
)
122+
}
123+
124+
function writeFiles(filenames: string[]) {
125+
return Promise.all(filenames.map((f) => Bun.write(path.join(Global.Path.log, f), "test")))
126+
}
127+
128+
function getLogFile(filename: string) {
129+
return Bun.file(path.join(Global.Path.log, filename))
130+
}
131+
132+
const ONE_DAY = 1000 * 60 * 60 * 24
133+
134+
function createLogFiles(count: number, startDate: Date, increment = ONE_DAY): string[] {
135+
return Array.from({ length: count }, (_, index) => {
136+
const creationOffset = index * increment
137+
const fileCreatedAt = new Date(startDate.getTime() + creationOffset)
138+
return fileCreatedAt.toISOString().split(".")[0].replace(/:/g, "") + ".log"
139+
})
140+
}

0 commit comments

Comments
 (0)