fix(usage): correct per-account cost attribution

Drop the detail.source fallback and dead numeric-key lookup in the usage
transformer so unmapped auth_index rows go to the 'unknown' bucket instead of
mis-keying cost; null out today_cost when an email cost-key is shared by more
than one account (duplicate-email providers) instead of double-displaying the
combined spend.
This commit is contained in:
Tam Nhu Tran committed 2026-06-07 16:37:59 -04:00
1 parent 205c2f3cd6
commit cc7ac553e3
4 files changed
+170 -16

No files matched your search

+112
View File
@@ -736,3 +736,115 @@ describe('force-fresh: paused accounts and concurrency cap (finding #7)', () =>
expect(pausedRow?.paused).toBe(true);
});
});
// ============================================================================
// Finding #11: codex duplicate-email cost double-count
// ============================================================================
describe('today_cost: duplicate-email accounts get null (finding #11)', () => {
// When two codex accounts share the same email, both rows must report today_cost: null
// instead of the combined total that would otherwise be double-counted.
async function buildRouter(accounts: object[], costMap: Record<string, number>) {
const { createBarRouter } = await import('../../../src/web-server/routes/bar-routes');
const { resetForceFreshDebounce: resetDebounce } = await import('../../../src/web-server/routes/bar-routes');
const app = express();
app.use(express.json());
const router = createBarRouter({
// eslint-disable-next-line @typescript-eslint/no-explicit-any
getAllAccountsSummary: () => ({ codex: accounts }) as any,
getCachedQuota: () => null,
setCachedQuota: () => {},
invalidateQuotaCache: () => {},
fetchAccountQuota: async () => makeQuotaResult(),
getTodayCostByAccount: () => costMap,
loadCliproxyDetails: async () => [],
runHealthChecks: async () => makeHealthReport(),
});
app.use('/api/bar', router);
const srv = await new Promise<Server>((resolve, reject) => {
const instance = app.listen(0, '127.0.0.1');
instance.once('error', reject);
instance.once('listening', () => resolve(instance));
});
const addr = srv.address();
if (!addr || typeof addr === 'string') throw new Error('No server address');
resetDebounce();
return { srv, url: `http://127.0.0.1:${(addr as { port: number }).port}` };
}
it('two codex accounts sharing an email both get today_cost: null (not the doubled total)', async () => {
const sharedEmail = 'shared@example.com';
const accounts = [
{
id: `${sharedEmail}#free`,
email: sharedEmail,
provider: 'codex',
nickname: 'free-codex',
tier: 'free',
paused: false,
isDefault: false,
tokenFile: 'codex-shared-free.json',
createdAt: '2026-01-01T00:00:00.000Z',
},
{
id: `${sharedEmail}#pro`,
email: sharedEmail,
provider: 'codex',
nickname: 'pro-codex',
tier: 'pro',
paused: false,
isDefault: true,
tokenFile: 'codex-shared-pro.json',
createdAt: '2026-01-01T00:00:00.000Z',
},
];
// The cost map only has a combined total for the shared email
const costMap = { [sharedEmail]: 5.00 };
const { srv, url } = await buildRouter(accounts, costMap);
const { body } = await getJson<BarSummaryRow[]>(url, '/api/bar/summary');
await new Promise<void>((resolve) => srv.close(() => resolve()));
expect(body.length).toBe(2);
// Both rows must be null — the cost is unknowable per-account when email is shared
expect(body[0].today_cost).toBeNull();
expect(body[1].today_cost).toBeNull();
// Neither should show the combined total
expect(body[0].today_cost).not.toBe(5.00);
expect(body[1].today_cost).not.toBe(5.00);
});
it('unique-email account still shows its individual cost', async () => {
const accounts = [
{
id: 'unique@example.com',
email: 'unique@example.com',
provider: 'codex',
nickname: 'unique-codex',
tier: 'pro',
paused: false,
isDefault: true,
tokenFile: 'codex-unique.json',
createdAt: '2026-01-01T00:00:00.000Z',
},
];
const costMap = { 'unique@example.com': 3.75 };
const { srv, url } = await buildRouter(accounts, costMap);
const { body } = await getJson<BarSummaryRow[]>(url, '/api/bar/summary');
await new Promise<void>((resolve) => srv.close(() => resolve()));
expect(body.length).toBe(1);
// Unique email — cost is attributable
expect(body[0].today_cost).toBeCloseTo(3.75);
});
});
@@ -87,9 +87,11 @@ const twoAccountResponse = makeResponse([
},
]);
// Fix #7/#13/#15: buildAuthIndexToAccountMap stores String(auth_index) keys only.
// The map must use string keys so accountMap.get(String(detail.auth_index)) resolves correctly.
const authFileMap: Map<number | string, string> = new Map([
[0, 'alice@example.com'],
[1, 'bob@example.com'],
['0', 'alice@example.com'],
['1', 'bob@example.com'],
]);
// ============================================================================
@@ -109,14 +111,25 @@ describe('extractCliproxyUsageHistoryDetails with accountMap', () => {
expect(bobDetails).toHaveLength(1); // auth_index 1 appears once
});
it('falls back to detail.source when auth_index not in accountMap', async () => {
it('leaves accountId undefined when auth_index is not in accountMap (no source fallback)', async () => {
// Fix #7/#13/#15: detail.source is a CLIProxy source label, not an email.
// Using it as a cost key caused mis-attribution. When auth_index is absent from the
// map, accountId must be undefined so getTodayCostByAccount buckets under 'unknown'.
const { extractCliproxyUsageHistoryDetails } = await import('../../../../src/web-server/usage/cliproxy-usage-transformer');
const partialMap: Map<number | string, string> = new Map([[0, 'alice@example.com']]);
// Use string key matching buildAuthIndexToAccountMap's String(auth_index) output
const partialMap: Map<number | string, string> = new Map([['0', 'alice@example.com']]);
const details = extractCliproxyUsageHistoryDetails(twoAccountResponse, partialMap);
const unknownAccount = details.find((d) => d.accountId === 'old-source-b');
expect(unknownAccount).toBeDefined();
// auth_index 1 (bob) is not in the partial map — must be undefined, not 'old-source-b'
const bobDetail = details.find((d) => d.accountId === undefined && !d.accountId);
// There should be exactly one detail with no accountId (bob's request)
const unmappedDetails = details.filter((d) => d.accountId === undefined);
expect(unmappedDetails).toHaveLength(1);
// Confirm it is NOT keyed under the source string
const sourceFallback = details.find((d) => d.accountId === 'old-source-b');
expect(sourceFallback).toBeUndefined();
void bobDetail; // suppress lint
});
it('does not include accountId when no accountMap is provided (backward compat)', async () => {
@@ -255,7 +268,9 @@ describe('getTodayCostByAccount', () => {
expect(result['alice@example.com']).toBeCloseTo(aliceCostFromDetails, 10);
});
it('details without accountId are grouped under fallback source key', async () => {
it('details without accountId are grouped under the "unknown" key', async () => {
// Fix #7/#13/#15: when no accountMap is provided, accountId is undefined on all details.
// getTodayCostByAccount buckets these under 'unknown' — not under detail.source.
const { getTodayCostByAccount } = await import('../../../../src/web-server/usage/data-aggregator');
const { extractCliproxyUsageHistoryDetails } = await import('../../../../src/web-server/usage/cliproxy-usage-transformer');
@@ -263,9 +278,12 @@ describe('getTodayCostByAccount', () => {
const details = extractCliproxyUsageHistoryDetails(twoAccountResponse);
const result = getTodayCostByAccount(details, TODAY);
// With no accountId, details with cost > 0 should still contribute
// grouped under some non-empty key derived from the detail
expect(Object.values(result).some((v) => v > 0)).toBe(true);
// All costs should be accumulated under the literal key 'unknown'
expect(typeof result['unknown']).toBe('number');
expect(result['unknown']).toBeGreaterThan(0);
// No source-string keys should appear in the result
expect(result['old-source-a']).toBeUndefined();
expect(result['old-source-b']).toBeUndefined();
});
});