Skip to content

fix(usage): serialize addSession to prevent lost updates on concurrent writes #1201

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aakashsharma7 wants to merge 1 commit into
base: main
Choose a base branch
from
+132 −60
Open

fix(usage): serialize addSession to prevent lost updates on concurrent writes #1201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-add-session-concurrency.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@nanocollective/nanocoder': patch
---

Fix race condition in `addSession()` by introducing atomic temp-file writes and serialized in-process locking to prevent lost updates on concurrent session completions.
118 changes: 72 additions & 46 deletions source/usage/storage.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,56 +377,56 @@ test('writeUsageData handles write errors gracefully', t => {
// addSession Tests
// ============================================================================

test('addSession adds new session to empty data', t => {
test('addSession adds new session to empty data', async t => {
const session = createMockSession();

addSession(session);
await addSession(session);

const data = readUsageData();
t.is(data.sessions.length, 1);
t.is(data.sessions[0]!.id, session.id);
t.is(data.totalLifetime, session.tokens.total);
});

test('addSession adds session to beginning of list', t => {
test('addSession adds session to beginning of list', async t => {
const session1 = createMockSession('provider1', 'model1', 1000);
const session2 = createMockSession('provider2', 'model2', 2000);

addSession(session1);
addSession(session2);
await addSession(session1);
await addSession(session2);

const data = readUsageData();
t.is(data.sessions.length, 2);
t.is(data.sessions[0]!.id, session2.id); // Most recent first
t.is(data.sessions[1]!.id, session1.id);
});

test('addSession updates total lifetime tokens', t => {
test('addSession updates total lifetime tokens', async t => {
const session1 = createMockSession('provider1', 'model1', 1000);
const session2 = createMockSession('provider2', 'model2', 2000);

addSession(session1);
addSession(session2);
await addSession(session1);
await addSession(session2);

const data = readUsageData();
t.is(data.totalLifetime, 3000);
});

test('addSession limits sessions to MAX_SESSIONS (100)', t => {
test('addSession limits sessions to MAX_SESSIONS (100)', async t => {
// Add 101 sessions
for (let i = 0; i < 101; i++) {
const session = createMockSession(`provider-${i}`, `model-${i}`, 100);
addSession(session);
await addSession(session);
}

const data = readUsageData();
t.is(data.sessions.length, 100); // Should be limited to 100
});

test('addSession creates daily aggregate', t => {
test('addSession creates daily aggregate', async t => {
const session = createMockSession();

addSession(session);
await addSession(session);

const data = readUsageData();
t.is(data.dailyAggregates.length, 1);
Expand All @@ -439,12 +439,12 @@ test('addSession creates daily aggregate', t => {
t.is(aggregate!.totalTokens, session.tokens.total);
});

test('addSession updates existing daily aggregate', t => {
test('addSession updates existing daily aggregate', async t => {
const session1 = createMockSession('provider1', 'model1', 1000);
const session2 = createMockSession('provider2', 'model2', 2000);

addSession(session1);
addSession(session2);
await addSession(session1);
await addSession(session2);

const data = readUsageData();
const today = new Date().toISOString().split('T')[0];
Expand All @@ -455,14 +455,14 @@ test('addSession updates existing daily aggregate', t => {
t.is(aggregate!.totalTokens, 3000);
});

test('addSession tracks provider stats in daily aggregate', t => {
test('addSession tracks provider stats in daily aggregate', async t => {
const session1 = createMockSession('openai', 'gpt-4', 1000);
const session2 = createMockSession('openai', 'gpt-3.5', 2000);
const session3 = createMockSession('anthropic', 'claude', 3000);

addSession(session1);
addSession(session2);
addSession(session3);
await addSession(session1);
await addSession(session2);
await addSession(session3);

const data = readUsageData();
const today = new Date().toISOString().split('T')[0];
Expand All @@ -473,14 +473,14 @@ test('addSession tracks provider stats in daily aggregate', t => {
t.is(aggregate!.providers.anthropic, 3000);
});

test('addSession tracks model stats in daily aggregate', t => {
test('addSession tracks model stats in daily aggregate', async t => {
const session1 = createMockSession('openai', 'gpt-4', 1000);
const session2 = createMockSession('openai', 'gpt-4', 2000);
const session3 = createMockSession('openai', 'gpt-3.5', 3000);

addSession(session1);
addSession(session2);
addSession(session3);
await addSession(session1);
await addSession(session2);
await addSession(session3);

const data = readUsageData();
const today = new Date().toISOString().split('T')[0];
Expand All @@ -491,19 +491,45 @@ test('addSession tracks model stats in daily aggregate', t => {
t.is(aggregate!.models['gpt-3.5'], 3000);
});

test('addSession limits daily aggregates to MAX_DAILY_AGGREGATES (30)', t => {
test('addSession limits daily aggregates to MAX_DAILY_AGGREGATES (30)', async t => {
// Create sessions with different dates
for (let i = 0; i < 35; i++) {
const session = createMockSession();
// Modify timestamp to be i days ago
session.timestamp = Date.now() - i * 24 * 60 * 60 * 1000;
addSession(session);
await addSession(session);
}

const data = readUsageData();
t.is(data.dailyAggregates.length, 30); // Should be limited to 30
});

test('addSession serializes concurrent calls without lost updates', async t => {
const session1 = createMockSession('provider1', 'model1', 1000);
const session2 = createMockSession('provider2', 'model2', 2000);
const session3 = createMockSession('provider3', 'model3', 3000);

// Simulate simultaneous session completions
await Promise.all([
addSession(session1),
addSession(session2),
addSession(session3),
]);

const data = readUsageData();
t.is(data.sessions.length, 3);
t.is(data.totalLifetime, 6000);

const today = new Date().toISOString().split('T')[0];
const aggregate = data.dailyAggregates.find(agg => agg.date === today);
t.truthy(aggregate);
t.is(aggregate!.sessions, 3);
t.is(aggregate!.totalTokens, 6000);
t.is(aggregate!.providers.provider1, 1000);
t.is(aggregate!.providers.provider2, 2000);
t.is(aggregate!.providers.provider3, 3000);
});

// ============================================================================
// getTodayAggregate Tests
// ============================================================================
Expand All @@ -513,19 +539,19 @@ test('getTodayAggregate returns null when no data exists', t => {
t.is(aggregate, null);
});

test('getTodayAggregate returns null when no sessions today', t => {
test('getTodayAggregate returns null when no sessions today', async t => {
// Add a session from yesterday
const session = createMockSession();
session.timestamp = Date.now() - 24 * 60 * 60 * 1000;
addSession(session);
await addSession(session);

const aggregate = getTodayAggregate();
t.is(aggregate, null);
});

test('getTodayAggregate returns today aggregate', t => {
test('getTodayAggregate returns today aggregate', async t => {
const session = createMockSession('openai', 'gpt-4', 1000);
addSession(session);
await addSession(session);

const aggregate = getTodayAggregate();

Expand All @@ -547,12 +573,12 @@ test('getLastNDaysAggregate returns zero when no data exists', t => {
t.is(result.avgTokensPerDay, 0);
});

test('getLastNDaysAggregate calculates totals for last 7 days', t => {
test('getLastNDaysAggregate calculates totals for last 7 days', async t => {
// Add sessions for the past 5 days
for (let i = 0; i < 5; i++) {
const session = createMockSession('openai', 'gpt-4', 1000);
session.timestamp = Date.now() - i * 24 * 60 * 60 * 1000;
addSession(session);
await addSession(session);
}

const result = getLastNDaysAggregate(7);
Expand All @@ -562,19 +588,19 @@ test('getLastNDaysAggregate calculates totals for last 7 days', t => {
t.is(result.avgTokensPerDay, Math.round(5000 / 7));
});

test('getLastNDaysAggregate filters out older sessions', t => {
test('getLastNDaysAggregate filters out older sessions', async t => {
// Add 3 sessions within last 7 days
for (let i = 0; i < 3; i++) {
const session = createMockSession('openai', 'gpt-4', 1000);
session.timestamp = Date.now() - i * 24 * 60 * 60 * 1000;
addSession(session);
await addSession(session);
}

// Add 2 sessions older than 7 days
for (let i = 8; i < 10; i++) {
const session = createMockSession('openai', 'gpt-4', 1000);
session.timestamp = Date.now() - i * 24 * 60 * 60 * 1000;
addSession(session);
await addSession(session);
}

const result = getLastNDaysAggregate(7);
Expand All @@ -584,12 +610,12 @@ test('getLastNDaysAggregate filters out older sessions', t => {
t.is(result.totalSessions, 3);
});

test('getLastNDaysAggregate handles different day ranges', t => {
test('getLastNDaysAggregate handles different day ranges', async t => {
// Add sessions for the past 10 days
for (let i = 0; i < 10; i++) {
const session = createMockSession('openai', 'gpt-4', 1000);
session.timestamp = Date.now() - i * 24 * 60 * 60 * 1000;
addSession(session);
await addSession(session);
}

const result7 = getLastNDaysAggregate(7);
Expand All @@ -601,12 +627,12 @@ test('getLastNDaysAggregate handles different day ranges', t => {
t.is(result30.totalTokens, 10000);
});

test('getLastNDaysAggregate calculates average correctly', t => {
test('getLastNDaysAggregate calculates average correctly', async t => {
// Add 10 sessions over 5 days
for (let i = 0; i < 10; i++) {
const session = createMockSession('openai', 'gpt-4', 500);
session.timestamp = Date.now() - (i % 5) * 24 * 60 * 60 * 1000;
addSession(session);
await addSession(session);
}

const result = getLastNDaysAggregate(7);
Expand All @@ -621,10 +647,10 @@ test('getLastNDaysAggregate calculates average correctly', t => {
// clearUsageData Tests
// ============================================================================

test('clearUsageData removes usage file', t => {
test('clearUsageData removes usage file', async t => {
// Add some data first
const session = createMockSession();
addSession(session);
await addSession(session);

// Verify data exists
let data = readUsageData();
Expand All @@ -644,9 +670,9 @@ test('clearUsageData handles non-existent file', t => {
t.notThrows(() => clearUsageData());
});

test('clearUsageData is idempotent', t => {
test('clearUsageData is idempotent', async t => {
// Add and clear data
addSession(createMockSession());
await addSession(createMockSession());
clearUsageData();

// Clear again
Expand All @@ -661,7 +687,7 @@ test('clearUsageData is idempotent', t => {
// Integration Tests
// ============================================================================

test('complete usage tracking flow', t => {
test('complete usage tracking flow', async t => {
// Start with empty data
let data = readUsageData();
t.is(data.sessions.length, 0);
Expand All @@ -671,9 +697,9 @@ test('complete usage tracking flow', t => {
const session2 = createMockSession('anthropic', 'claude', 2000);
const session3 = createMockSession('openai', 'gpt-3.5', 1500);

addSession(session1);
addSession(session2);
addSession(session3);
await addSession(session1);
await addSession(session2);
await addSession(session3);

// Verify sessions added
data = readUsageData();
Expand Down
Loading
Loading