mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-17 12:16:59 +00:00
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:
@@ -214,11 +214,18 @@ function buildRow(
|
|||||||
account: AccountInfo,
|
account: AccountInfo,
|
||||||
fetchResult: AccountFetchResult,
|
fetchResult: AccountFetchResult,
|
||||||
costByAccount: Record<string, number>,
|
costByAccount: Record<string, number>,
|
||||||
overallHealth: 'ok' | 'warning' | 'error'
|
overallHealth: 'ok' | 'warning' | 'error',
|
||||||
|
/** Set of cost-keys that are shared by more than one account. Cost is unknowable for these. */
|
||||||
|
sharedCostKeys: ReadonlySet<string>
|
||||||
): BarSummaryRow {
|
): BarSummaryRow {
|
||||||
const { quota, cached, fetchedAt } = fetchResult;
|
const { quota, cached, fetchedAt } = fetchResult;
|
||||||
const costKey = resolveCostKey(account);
|
const costKey = resolveCostKey(account);
|
||||||
|
|
||||||
|
// Fix #11: when multiple accounts share the same cost-key (e.g. two codex accounts with
|
||||||
|
// the same email), we cannot attribute the combined cost to either individual account.
|
||||||
|
// Report null (unknowable) rather than the inflated shared total.
|
||||||
|
const todayCost = sharedCostKeys.has(costKey) ? null : (costByAccount[costKey] ?? 0);
|
||||||
|
|
||||||
if (!quota || !quota.success) {
|
if (!quota || !quota.success) {
|
||||||
// Degraded row: preserve identity fields, null out quota data
|
// Degraded row: preserve identity fields, null out quota data
|
||||||
return {
|
return {
|
||||||
@@ -229,7 +236,7 @@ function buildRow(
|
|||||||
paused: account.paused ?? false,
|
paused: account.paused ?? false,
|
||||||
quota_percentage: null,
|
quota_percentage: null,
|
||||||
next_reset: null,
|
next_reset: null,
|
||||||
today_cost: costByAccount[costKey] ?? 0,
|
today_cost: todayCost,
|
||||||
health: quota?.needsReauth ? 'error' : overallHealth,
|
health: quota?.needsReauth ? 'error' : overallHealth,
|
||||||
cached,
|
cached,
|
||||||
fetchedAt,
|
fetchedAt,
|
||||||
@@ -245,7 +252,7 @@ function buildRow(
|
|||||||
paused: account.paused ?? false,
|
paused: account.paused ?? false,
|
||||||
quota_percentage: extractQuotaPercentage(quota),
|
quota_percentage: extractQuotaPercentage(quota),
|
||||||
next_reset: extractNextReset(quota),
|
next_reset: extractNextReset(quota),
|
||||||
today_cost: costByAccount[costKey] ?? 0,
|
today_cost: todayCost,
|
||||||
health: overallHealth,
|
health: overallHealth,
|
||||||
cached,
|
cached,
|
||||||
fetchedAt,
|
fetchedAt,
|
||||||
@@ -315,6 +322,19 @@ export function createBarRouter(deps: BarRouterDeps): Router {
|
|||||||
const summary = deps.getAllAccountsSummary();
|
const summary = deps.getAllAccountsSummary();
|
||||||
const allAccounts: AccountInfo[] = Object.values(summary).flat();
|
const allAccounts: AccountInfo[] = Object.values(summary).flat();
|
||||||
|
|
||||||
|
// Fix #11: compute which cost-keys are shared by >1 account so buildRow can
|
||||||
|
// report null (unknowable) rather than the combined total for those rows.
|
||||||
|
const costKeyCount = new Map<string, number>();
|
||||||
|
for (const account of allAccounts) {
|
||||||
|
const key = resolveCostKey(account);
|
||||||
|
costKeyCount.set(key, (costKeyCount.get(key) ?? 0) + 1);
|
||||||
|
}
|
||||||
|
const sharedCostKeys = new Set<string>(
|
||||||
|
Array.from(costKeyCount.entries())
|
||||||
|
.filter(([, count]) => count > 1)
|
||||||
|
.map(([key]) => key)
|
||||||
|
);
|
||||||
|
|
||||||
// Fetch quota in parallel with per-account error isolation.
|
// Fetch quota in parallel with per-account error isolation.
|
||||||
// Concurrency is capped to avoid fan-out across large account lists.
|
// Concurrency is capped to avoid fan-out across large account lists.
|
||||||
// Paused accounts are handled inside fetchAccountData (cache/degrade, no live fetch).
|
// Paused accounts are handled inside fetchAccountData (cache/degrade, no live fetch).
|
||||||
@@ -325,7 +345,7 @@ export function createBarRouter(deps: BarRouterDeps): Router {
|
|||||||
const batchRows = await Promise.all(
|
const batchRows = await Promise.all(
|
||||||
batch.map(async (account): Promise<BarSummaryRow> => {
|
batch.map(async (account): Promise<BarSummaryRow> => {
|
||||||
const fetchResult = await fetchAccountData(account, doForceRefresh, deps);
|
const fetchResult = await fetchAccountData(account, doForceRefresh, deps);
|
||||||
return buildRow(account, fetchResult, costByAccount, overallHealth);
|
return buildRow(account, fetchResult, costByAccount, overallHealth, sharedCostKeys);
|
||||||
})
|
})
|
||||||
);
|
);
|
||||||
rows.push(...batchRows);
|
rows.push(...batchRows);
|
||||||
|
|||||||
@@ -69,11 +69,15 @@ function createHistoryDetail(
|
|||||||
const outputTokens = detail.tokens?.output_tokens ?? 0;
|
const outputTokens = detail.tokens?.output_tokens ?? 0;
|
||||||
const cacheReadTokens = detail.tokens?.cached_tokens ?? 0;
|
const cacheReadTokens = detail.tokens?.cached_tokens ?? 0;
|
||||||
|
|
||||||
// Resolve accountId from auth_index → account map, falling back to detail.source
|
// Resolve accountId from auth_index → account map.
|
||||||
|
// buildAuthIndexToAccountMap stores only String(auth_index) keys, so the numeric-key
|
||||||
|
// lookup is dead code and the detail.source fallback mis-attributes cost to a CLIProxy
|
||||||
|
// source label rather than an email. Leave accountId undefined when the index is absent
|
||||||
|
// so getTodayCostByAccount buckets it under 'unknown' and the bar excludes it.
|
||||||
let accountId: string | undefined;
|
let accountId: string | undefined;
|
||||||
if (accountMap !== undefined) {
|
if (accountMap !== undefined) {
|
||||||
const key = String(detail.auth_index);
|
const key = String(detail.auth_index);
|
||||||
accountId = accountMap.get(key) ?? accountMap.get(detail.auth_index) ?? detail.source;
|
accountId = accountMap.get(key);
|
||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|||||||
@@ -736,3 +736,115 @@ describe('force-fresh: paused accounts and concurrency cap (finding #7)', () =>
|
|||||||
expect(pausedRow?.paused).toBe(true);
|
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([
|
const authFileMap: Map<number | string, string> = new Map([
|
||||||
[0, 'alice@example.com'],
|
['0', 'alice@example.com'],
|
||||||
[1, 'bob@example.com'],
|
['1', 'bob@example.com'],
|
||||||
]);
|
]);
|
||||||
|
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
@@ -109,14 +111,25 @@ describe('extractCliproxyUsageHistoryDetails with accountMap', () => {
|
|||||||
expect(bobDetails).toHaveLength(1); // auth_index 1 appears once
|
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 { 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 details = extractCliproxyUsageHistoryDetails(twoAccountResponse, partialMap);
|
||||||
|
|
||||||
const unknownAccount = details.find((d) => d.accountId === 'old-source-b');
|
// auth_index 1 (bob) is not in the partial map — must be undefined, not 'old-source-b'
|
||||||
expect(unknownAccount).toBeDefined();
|
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 () => {
|
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);
|
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 { getTodayCostByAccount } = await import('../../../../src/web-server/usage/data-aggregator');
|
||||||
const { extractCliproxyUsageHistoryDetails } = await import('../../../../src/web-server/usage/cliproxy-usage-transformer');
|
const { extractCliproxyUsageHistoryDetails } = await import('../../../../src/web-server/usage/cliproxy-usage-transformer');
|
||||||
|
|
||||||
@@ -263,9 +278,12 @@ describe('getTodayCostByAccount', () => {
|
|||||||
const details = extractCliproxyUsageHistoryDetails(twoAccountResponse);
|
const details = extractCliproxyUsageHistoryDetails(twoAccountResponse);
|
||||||
const result = getTodayCostByAccount(details, TODAY);
|
const result = getTodayCostByAccount(details, TODAY);
|
||||||
|
|
||||||
// With no accountId, details with cost > 0 should still contribute
|
// All costs should be accumulated under the literal key 'unknown'
|
||||||
// grouped under some non-empty key derived from the detail
|
expect(typeof result['unknown']).toBe('number');
|
||||||
expect(Object.values(result).some((v) => v > 0)).toBe(true);
|
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();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user