Skip to content

Commit 15988d2

Browse files
authored
fix: preserve workspace-specific usage quotas + harden test isolation (#161)
Dedupe usage accounts by workspace identity (accountId+organizationId) first, refresh token fallback; keep freshest occurrence; skip disabled/identity-less. Harden resolveCodexUsageActiveAccount against sparse/disabled slots. Active-marker match by workspace key. Fix test isolation: debounced-save tests flush+dispose own managers before afterAll.
1 parent a939c6b commit 15988d2

5 files changed

Lines changed: 315 additions & 24 deletions

File tree

lib/codex-usage.ts

Lines changed: 101 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -490,23 +490,92 @@ export async function ensureCodexUsageAccessToken(params: {
490490
return { accessToken, refreshed: true, persisted };
491491
}
492492

493+
/**
494+
* Normalize an account identity field to a trimmed string.
495+
*
496+
* Non-string values collapse to an empty string so callers can treat
497+
* "missing" and "blank" identity parts uniformly when building dedupe keys.
498+
*/
499+
function normalizeUsageIdentityPart(value: string | undefined): string {
500+
return typeof value === "string" ? value.trim() : "";
501+
}
502+
503+
/**
504+
* Derive a stable usage-quota dedupe key for an account.
505+
*
506+
* A single OAuth credential can authorize multiple ChatGPT workspaces, each
507+
* exposing distinct `accountId` / `organizationId` values while sharing one
508+
* refresh token. Quota deduplication must therefore key on workspace identity
509+
* first so separate workspaces are not collapsed into one row, falling back to
510+
* the refresh token only when no workspace identity is available.
511+
*
512+
* Keys are emitted as `JSON.stringify` arrays (tagged `"workspace"` or
513+
* `"refresh"`) so values containing delimiter characters cannot collide.
514+
*
515+
* @param account - Stored account metadata to derive the key from.
516+
* @returns A unique identity key, or `undefined` when the account carries no
517+
* workspace identity and no refresh token.
518+
*/
519+
export function getUsageAccountDedupeKey(
520+
account: AccountMetadataV3,
521+
): string | undefined {
522+
const accountId = normalizeUsageIdentityPart(account.accountId);
523+
const organizationId = normalizeUsageIdentityPart(account.organizationId);
524+
if (accountId || organizationId) {
525+
return JSON.stringify(["workspace", accountId, organizationId]);
526+
}
527+
528+
const refreshToken = normalizeUsageIdentityPart(account.refreshToken);
529+
return refreshToken ? JSON.stringify(["refresh", refreshToken]) : undefined;
530+
}
531+
532+
/**
533+
* Collect the indices of accounts that represent distinct usage quotas.
534+
*
535+
* Disabled accounts are skipped. Accounts with no usable identity — no
536+
* `accountId`, no `organizationId`, and no `refreshToken`, i.e. those for which
537+
* {@link getUsageAccountDedupeKey} returns `undefined` — are also dropped, since
538+
* they cannot be attributed to a quota and have no token to query. Entries
539+
* sharing the same dedupe key are collapsed to a single index.
540+
*
541+
* When a workspace key appears more than once (e.g. an account re-added after a
542+
* token re-issue), the *last* (most recently added) occurrence is kept so the
543+
* freshest credential is queried — keeping the first occurrence could surface
544+
* an invalidated refresh token after re-auth. First-appearance order is still
545+
* used for display stability.
546+
*
547+
* @param storage - The account storage to scan.
548+
* @returns Storage indices of unique, enabled, identifiable usage accounts in
549+
* first-appearance order, each pointing at its freshest occurrence.
550+
*/
493551
export function deduplicateUsageAccountIndices(storage: AccountStorageV3): number[] {
494-
const seenTokens = new Set<string>();
495-
const uniqueIndices: number[] = [];
552+
const indexByIdentity = new Map<string, number>();
496553
for (let i = 0; i < storage.accounts.length; i += 1) {
497554
const account = storage.accounts[i];
498555
if (!account) continue;
499-
const refreshToken =
500-
typeof account.refreshToken === "string"
501-
? account.refreshToken.trim()
502-
: "";
503-
if (refreshToken && seenTokens.has(refreshToken)) continue;
504-
if (refreshToken) seenTokens.add(refreshToken);
505-
uniqueIndices.push(i);
556+
if (account.enabled === false) continue;
557+
const key = getUsageAccountDedupeKey(account);
558+
if (!key) continue;
559+
// Map keeps first-insertion key order (stable display) while overwriting
560+
// the value so the latest occurrence's index wins (freshest credential).
561+
indexByIdentity.set(key, i);
506562
}
507-
return uniqueIndices;
563+
return [...indexByIdentity.values()];
508564
}
509565

566+
/**
567+
* Resolve which account's usage quota should be shown as active.
568+
*
569+
* Starts from the persisted active index (preferring the Codex family index)
570+
* and then prefers the most-recently-used enabled account by `lastUsed`, so the
571+
* displayed quota tracks the credential actually serving requests. Disabled
572+
* accounts are ignored, and invalid/missing `lastUsed` values are treated as
573+
* oldest.
574+
*
575+
* @param storage - The account storage to inspect.
576+
* @returns The selected account and its index, or `null` when no enabled
577+
* account is available.
578+
*/
510579
export function resolveCodexUsageActiveAccount(
511580
storage: AccountStorageV3,
512581
): UsageAccountSelection | null {
@@ -519,25 +588,41 @@ export function resolveCodexUsageActiveAccount(
519588
Math.min(storage.accounts.length - 1, Math.trunc(numericIndex)),
520589
);
521590
const activeAccount = storage.accounts[index];
522-
if (!activeAccount) return null;
591+
if (
592+
!activeAccount &&
593+
storage.accounts.every((account) => !account || account.enabled === false)
594+
) {
595+
return null;
596+
}
523597

598+
// An enabled active account with a missing/invalid `lastUsed` must fall back
599+
// to 0 (same as every other enabled account), not -1. Using -1 would let a
600+
// lower-index enabled account with `lastUsed` 0 win the `0 > -1` comparison
601+
// and steal the active marker before the active account's own iteration.
602+
const activeEnabled = !!activeAccount && activeAccount.enabled !== false;
524603
const activeLastUsed =
525-
typeof activeAccount.lastUsed === "number" && Number.isFinite(activeAccount.lastUsed)
604+
activeEnabled &&
605+
typeof activeAccount?.lastUsed === "number" &&
606+
Number.isFinite(activeAccount.lastUsed)
526607
? activeAccount.lastUsed
527-
: 0;
528-
let newestIndex = index;
608+
: activeEnabled
609+
? 0
610+
: -1;
611+
let newestIndex = activeEnabled ? index : -1;
529612
let newestLastUsed = activeLastUsed;
530613
for (let i = 0; i < storage.accounts.length; i += 1) {
531614
const account = storage.accounts[i];
615+
if (!account || account.enabled === false) continue;
532616
const lastUsed =
533-
typeof account?.lastUsed === "number" && Number.isFinite(account.lastUsed)
617+
typeof account.lastUsed === "number" && Number.isFinite(account.lastUsed)
534618
? account.lastUsed
535619
: 0;
536-
if (account && lastUsed > newestLastUsed) {
620+
if (lastUsed > newestLastUsed) {
537621
newestIndex = i;
538622
newestLastUsed = lastUsed;
539623
}
540624
}
625+
if (newestIndex < 0) return null;
541626

542627
const account = storage.accounts[newestIndex];
543628
return account ? { index: newestIndex, account } : null;

lib/tools/codex-limits.ts

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
fetchCodexUsage,
1212
formatUsageLimitSummary,
1313
formatUsageLimitTitle,
14+
getUsageAccountDedupeKey,
1415
hasUsageWindow,
1516
parseCodexUsagePayload,
1617
resolveCodexUsageAccountId,
@@ -26,6 +27,18 @@ import {
2627
import { normalizeToolOutputFormat, renderJsonOutput } from "../runtime.js";
2728
import type { ToolContext } from "./index.js";
2829

30+
/**
31+
* Build the `codex-limits` tool.
32+
*
33+
* The tool fetches and renders Codex usage quotas for each unique account.
34+
* Accounts are deduplicated by workspace identity via
35+
* {@link getUsageAccountDedupeKey} so distinct workspaces are shown separately,
36+
* while the active-account marker is only applied when an account matches both
37+
* the active refresh token and the active workspace dedupe key.
38+
*
39+
* @param ctx - Shared {@link ToolContext} providing UI runtime and account helpers.
40+
* @returns The `codex-limits` {@link ToolDefinition}.
41+
*/
2942
export function createCodexLimitsTool(ctx: ToolContext): ToolDefinition {
3043
const {
3144
resolveUiRuntime,
@@ -97,14 +110,45 @@ export function createCodexLimitsTool(ctx: ToolContext): ToolDefinition {
97110
activeIndex < storage.accounts.length
98111
? storage.accounts[activeIndex]?.refreshToken?.trim() || undefined
99112
: undefined;
113+
const activeAccount =
114+
typeof activeIndex === "number" &&
115+
activeIndex >= 0 &&
116+
activeIndex < storage.accounts.length
117+
? storage.accounts[activeIndex]
118+
: undefined;
119+
const activeUsageKey = activeAccount
120+
? getUsageAccountDedupeKey(activeAccount)
121+
: undefined;
122+
// If the active account was deduplicated out of uniqueIndices (e.g. it
123+
// is a later occurrence of a workspace whose first occurrence carries a
124+
// re-issued refresh token), warn so the missing `[active]` marker is
125+
// diagnosable. The key-based match below still recovers the marker.
126+
if (
127+
typeof activeIndex === "number" &&
128+
activeIndex >= 0 &&
129+
activeIndex < storage.accounts.length &&
130+
!uniqueIndices.includes(activeIndex)
131+
) {
132+
logWarn(
133+
`[${PLUGIN_NAME}] active account index ${activeIndex} was deduplicated out of the usage list; matching the active workspace by identity instead.`,
134+
);
135+
}
100136
let storageChanged = false;
101137
const jsonAccounts: Array<Record<string, unknown>> = [];
102138

103139
for (const i of uniqueIndices) {
104140
const account = storage.accounts[i];
105141
if (!account) continue;
106-
const sharesActiveCredential =
107-
!!activeRefreshToken && account.refreshToken === activeRefreshToken;
142+
const accountUsageKey = getUsageAccountDedupeKey(account);
143+
// Match the active account by workspace identity first: two entries
144+
// for the same workspace can carry different refresh tokens (e.g. a
145+
// re-issued token after re-add), so an exact token match alone would
146+
// drop the `[active]` marker. Fall back to refresh-token equality for
147+
// accounts that have no workspace identity (token-only dedupe key).
148+
const sharesActiveCredential = activeUsageKey
149+
? accountUsageKey === activeUsageKey
150+
: !!activeRefreshToken &&
151+
account.refreshToken === activeRefreshToken;
108152
const displayIndex =
109153
sharesActiveCredential && typeof activeIndex === "number"
110154
? activeIndex

test/codex-usage.test.ts

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,111 @@ describe("codex usage helpers", () => {
8787
],
8888
};
8989

90-
expect(deduplicateUsageAccountIndices(storage)).toEqual([0, 2]);
90+
expect(deduplicateUsageAccountIndices(storage)).toEqual([1, 2]);
9191
expect(resolveCodexUsageActiveAccount(storage)).toMatchObject({
9292
index: 2,
9393
account: { accountId: "acc-2" },
9494
});
9595
});
9696

97+
it("keeps same-token workspace entries distinct, skips disabled, and prefers the freshest duplicate", () => {
98+
const storage: AccountStorageV3 = {
99+
version: 3,
100+
activeIndex: 0,
101+
accounts: [
102+
{ refreshToken: "r1", accountId: "acc-1", organizationId: "org-1", addedAt: 0, lastUsed: 0 },
103+
{ refreshToken: "r1", accountId: "acc-2", organizationId: "org-2", addedAt: 0, lastUsed: 0 },
104+
{ refreshToken: "r2", accountId: "acc-3", enabled: false, addedAt: 0, lastUsed: 50 },
105+
{ refreshToken: "r3", accountId: "acc-1", organizationId: "org-1", addedAt: 0, lastUsed: 0 },
106+
],
107+
};
108+
109+
// org-1 (key W) appears at index 0 and again at index 3 (re-added with a
110+
// fresh token r3); org-2 (key X) at index 1; index 2 disabled. Display
111+
// order follows first appearance (W then X), but W resolves to its
112+
// freshest occurrence (index 3, token r3), not the stale index 0.
113+
expect(deduplicateUsageAccountIndices(storage)).toEqual([3, 1]);
114+
expect(resolveCodexUsageActiveAccount(storage)).toMatchObject({
115+
index: 0,
116+
account: { accountId: "acc-1" },
117+
});
118+
});
119+
120+
it("deduplicates workspace identities without delimiter collisions", () => {
121+
const storage: AccountStorageV3 = {
122+
version: 3,
123+
activeIndex: 0,
124+
accounts: [
125+
{ refreshToken: "r1", accountId: "acc:1", organizationId: "org", addedAt: 0, lastUsed: 0 },
126+
{ refreshToken: "r2", accountId: "acc", organizationId: "1:org", addedAt: 0, lastUsed: 0 },
127+
],
128+
};
129+
130+
expect(deduplicateUsageAccountIndices(storage)).toEqual([0, 1]);
131+
});
132+
133+
it("handles sparse/undefined account slots without throwing", () => {
134+
const storage = {
135+
version: 3,
136+
activeIndex: 5,
137+
accounts: [
138+
undefined,
139+
{ refreshToken: "r1", accountId: "acc-1", organizationId: "org-1", addedAt: 0, lastUsed: 10 },
140+
],
141+
} as unknown as AccountStorageV3;
142+
143+
expect(() => resolveCodexUsageActiveAccount(storage)).not.toThrow();
144+
expect(resolveCodexUsageActiveAccount(storage)).toMatchObject({
145+
index: 1,
146+
account: { accountId: "acc-1" },
147+
});
148+
});
149+
150+
it("returns null when every account slot is empty or disabled", () => {
151+
const storage = {
152+
version: 3,
153+
activeIndex: 0,
154+
accounts: [
155+
undefined,
156+
{ refreshToken: "r2", accountId: "acc-2", enabled: false, addedAt: 0, lastUsed: 0 },
157+
],
158+
} as unknown as AccountStorageV3;
159+
160+
expect(resolveCodexUsageActiveAccount(storage)).toBeNull();
161+
});
162+
163+
it("keeps the active account when its lastUsed is missing", () => {
164+
const storage = {
165+
version: 3,
166+
activeIndex: 1,
167+
accounts: [
168+
{ refreshToken: "r1", accountId: "acc-1", organizationId: "org-1", addedAt: 0, lastUsed: 0 },
169+
{ refreshToken: "r2", accountId: "acc-2", organizationId: "org-2", addedAt: 0 },
170+
],
171+
} as unknown as AccountStorageV3;
172+
173+
// The active account (index 1) has no lastUsed. It must not lose the
174+
// marker to index 0's lastUsed:0 via a 0 > -1 comparison.
175+
expect(resolveCodexUsageActiveAccount(storage)).toMatchObject({
176+
index: 1,
177+
account: { accountId: "acc-2" },
178+
});
179+
});
180+
181+
it("drops accounts that have no workspace identity and no refresh token", () => {
182+
const storage: AccountStorageV3 = {
183+
version: 3,
184+
activeIndex: 0,
185+
accounts: [
186+
{ addedAt: 0, lastUsed: 0 },
187+
{ refreshToken: "r1", accountId: "acc-1", organizationId: "org-1", addedAt: 0, lastUsed: 0 },
188+
],
189+
};
190+
191+
// The identity-less entry (index 0) yields no dedupe key and is excluded.
192+
expect(deduplicateUsageAccountIndices(storage)).toEqual([1]);
193+
});
194+
97195
it("uses the most recently persisted request account for usage display", () => {
98196
const storage: AccountStorageV3 = {
99197
version: 3,

0 commit comments

Comments
 (0)