Skip to content

Commit c39ca41

Browse files
KhaiTrang1995claude
KhaiTrang1995
and
claude
authored
fix(loop-context): lock the daily-spend state file against concurrent writers (#521)
recordDailySpend() did an unguarded read-modify-write: read the JSON state file, add tokensDelta, write it back, with no locking. Two overlapping invocations for the same pattern -- e.g. two scheduled loops hitting the same pattern around the same time -- both read the same stale total and the second write clobbers the first, silently losing a delta. This directly undermines the daily-budget circuit breaker's purpose: a lost delta means loop-context can under-count cumulative spend and delay or miss the escalation that's supposed to cap cost. loop-worktree already solved the same class of problem for its manifest with a lock-file-plus-poll mutex (open with 'wx', treat an existing lock older than 30s as stale and reclaim it, release in a finally). Applied the same shape here, scoped per pattern's own lock file so unrelated patterns never contend with each other. Test plan: added a regression test that fires two concurrent recordDailySpend calls with the same delta and asserts both land (2000 total, not 1000). Full clean rebuild + npm test: 52/52 passing. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent c74a99c commit c39ca41

3 files changed

Lines changed: 133 additions & 16 deletions

File tree

‎tools/loop-context/dist/daily-spend.js‎

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,60 @@
11
import path from 'node:path';
2-
import { mkdir, readFile, writeFile } from 'node:fs/promises';
2+
import { mkdir, readFile, writeFile, open, unlink, stat } from 'node:fs/promises';
33
/** Today's date in UTC, as YYYY-MM-DD — the daily-spend rollover boundary. */
44
export function todayUTC() {
55
return new Date().toISOString().slice(0, 10);
66
}
77
function statePath(dir, pattern) {
88
return path.join(dir, `daily-spend.${pattern}.json`);
99
}
10+
function lockPath(dir, pattern) {
11+
return path.join(dir, `.daily-spend.${pattern}.lock`);
12+
}
13+
const LOCK_STALE_MS = 30000;
14+
const LOCK_TIMEOUT_MS = 30000;
15+
/**
16+
* Serializes read-modify-write access to one pattern's state file, the same
17+
* lock-file-plus-poll shape loop-worktree's manifest mutex uses. Without
18+
* this, two overlapping invocations (e.g. two scheduled loops hitting the
19+
* same pattern) both read the same stale total and the second write clobbers
20+
* the first, silently losing a delta from the daily-budget circuit breaker.
21+
*/
22+
async function withLock(dir, pattern, fn) {
23+
await mkdir(dir, { recursive: true });
24+
const lock = lockPath(dir, pattern);
25+
const deadline = Date.now() + LOCK_TIMEOUT_MS;
26+
for (;;) {
27+
try {
28+
const handle = await open(lock, 'wx');
29+
await handle.close();
30+
break;
31+
}
32+
catch (err) {
33+
if (err.code !== 'EEXIST')
34+
throw err;
35+
try {
36+
const st = await stat(lock);
37+
if (Date.now() - st.mtimeMs > LOCK_STALE_MS) {
38+
await unlink(lock).catch(() => { });
39+
continue;
40+
}
41+
}
42+
catch {
43+
continue;
44+
}
45+
if (Date.now() > deadline) {
46+
throw new Error(`Timed out waiting for daily-spend lock on "${pattern}". If no other loop-context process is running, delete ${lock} manually.`);
47+
}
48+
await new Promise((resolve) => setTimeout(resolve, 15 + Math.random() * 35));
49+
}
50+
}
51+
try {
52+
return await fn();
53+
}
54+
finally {
55+
await unlink(lock).catch(() => { });
56+
}
57+
}
1058
async function readState(dir, pattern) {
1159
try {
1260
const raw = await readFile(statePath(dir, pattern), 'utf8');
@@ -22,11 +70,13 @@ async function readState(dir, pattern) {
2270
* file from a previous day is treated as if it didn't exist.
2371
*/
2472
export async function recordDailySpend(dir, pattern, tokensDelta) {
25-
const today = todayUTC();
26-
const existing = await readState(dir, pattern);
27-
const carryOver = existing && existing.date === today ? existing.tokensUsedToday : 0;
28-
const state = { date: today, tokensUsedToday: carryOver + tokensDelta };
29-
await mkdir(dir, { recursive: true });
30-
await writeFile(statePath(dir, pattern), JSON.stringify(state, null, 2));
31-
return state;
73+
return withLock(dir, pattern, async () => {
74+
const today = todayUTC();
75+
const existing = await readState(dir, pattern);
76+
const carryOver = existing && existing.date === today ? existing.tokensUsedToday : 0;
77+
const state = { date: today, tokensUsedToday: carryOver + tokensDelta };
78+
await mkdir(dir, { recursive: true });
79+
await writeFile(statePath(dir, pattern), JSON.stringify(state, null, 2));
80+
return state;
81+
});
3282
}

‎tools/loop-context/src/daily-spend.ts‎

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import path from 'node:path';
2-
import { mkdir, readFile, writeFile } from 'node:fs/promises';
2+
import { mkdir, readFile, writeFile, open, unlink, stat } from 'node:fs/promises';
33

44
export interface DailySpendState {
55
date: string;
@@ -15,6 +15,57 @@ function statePath(dir: string, pattern: string): string {
1515
return path.join(dir, `daily-spend.${pattern}.json`);
1616
}
1717

18+
function lockPath(dir: string, pattern: string): string {
19+
return path.join(dir, `.daily-spend.${pattern}.lock`);
20+
}
21+
22+
const LOCK_STALE_MS = 30000;
23+
const LOCK_TIMEOUT_MS = 30000;
24+
25+
/**
26+
* Serializes read-modify-write access to one pattern's state file, the same
27+
* lock-file-plus-poll shape loop-worktree's manifest mutex uses. Without
28+
* this, two overlapping invocations (e.g. two scheduled loops hitting the
29+
* same pattern) both read the same stale total and the second write clobbers
30+
* the first, silently losing a delta from the daily-budget circuit breaker.
31+
*/
32+
async function withLock<T>(dir: string, pattern: string, fn: () => Promise<T>): Promise<T> {
33+
await mkdir(dir, { recursive: true });
34+
const lock = lockPath(dir, pattern);
35+
const deadline = Date.now() + LOCK_TIMEOUT_MS;
36+
for (;;) {
37+
try {
38+
const handle = await open(lock, 'wx');
39+
await handle.close();
40+
break;
41+
} catch (err) {
42+
if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err;
43+
44+
try {
45+
const st = await stat(lock);
46+
if (Date.now() - st.mtimeMs > LOCK_STALE_MS) {
47+
await unlink(lock).catch(() => {});
48+
continue;
49+
}
50+
} catch {
51+
continue;
52+
}
53+
54+
if (Date.now() > deadline) {
55+
throw new Error(
56+
`Timed out waiting for daily-spend lock on "${pattern}". If no other loop-context process is running, delete ${lock} manually.`,
57+
);
58+
}
59+
await new Promise((resolve) => setTimeout(resolve, 15 + Math.random() * 35));
60+
}
61+
}
62+
try {
63+
return await fn();
64+
} finally {
65+
await unlink(lock).catch(() => {});
66+
}
67+
}
68+
1869
async function readState(dir: string, pattern: string): Promise<DailySpendState | null> {
1970
try {
2071
const raw = await readFile(statePath(dir, pattern), 'utf8');
@@ -34,12 +85,14 @@ export async function recordDailySpend(
3485
pattern: string,
3586
tokensDelta: number,
3687
): Promise<DailySpendState> {
37-
const today = todayUTC();
38-
const existing = await readState(dir, pattern);
39-
const carryOver = existing && existing.date === today ? existing.tokensUsedToday : 0;
40-
const state: DailySpendState = { date: today, tokensUsedToday: carryOver + tokensDelta };
88+
return withLock(dir, pattern, async () => {
89+
const today = todayUTC();
90+
const existing = await readState(dir, pattern);
91+
const carryOver = existing && existing.date === today ? existing.tokensUsedToday : 0;
92+
const state: DailySpendState = { date: today, tokensUsedToday: carryOver + tokensDelta };
4193

42-
await mkdir(dir, { recursive: true });
43-
await writeFile(statePath(dir, pattern), JSON.stringify(state, null, 2));
44-
return state;
94+
await mkdir(dir, { recursive: true });
95+
await writeFile(statePath(dir, pattern), JSON.stringify(state, null, 2));
96+
return state;
97+
});
4598
}

‎tools/loop-context/test/daily-spend.test.mjs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,17 @@ test('recordDailySpend persists the state to disk', async () => {
4949
const parsed = JSON.parse(raw);
5050
assert.equal(parsed.tokensUsedToday, 700);
5151
});
52+
53+
test('concurrent recordDailySpend calls for the same pattern do not lose an update', async () => {
54+
const dir = await freshDir();
55+
// Two overlapping invocations (e.g. two scheduled loops hitting the same
56+
// pattern) must not both read the same stale total and clobber each
57+
// other's write -- every delta has to land.
58+
await Promise.all([
59+
recordDailySpend(dir, 'ci-sweeper', 1000),
60+
recordDailySpend(dir, 'ci-sweeper', 1000),
61+
]);
62+
const raw = await readFile(path.join(dir, 'daily-spend.ci-sweeper.json'), 'utf8');
63+
const parsed = JSON.parse(raw);
64+
assert.equal(parsed.tokensUsedToday, 2000);
65+
});

0 commit comments

Comments
 (0)