Skip to content

Commit ee7b855

Browse files
committed
fix footer review findings
1 parent 463696a commit ee7b855

2 files changed

Lines changed: 78 additions & 7 deletions

File tree

index.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1148,16 +1148,26 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
11481148

11491149
const persistedAccountIndicators = new Map<string, PersistedAccountIndicatorEntry>();
11501150
let persistedAccountIndicatorRevision = 0;
1151+
let persistedAccountCountHint = 0;
11511152

11521153
const nextPersistedAccountIndicatorRevision = (): number => {
11531154
persistedAccountIndicatorRevision += 1;
11541155
return persistedAccountIndicatorRevision;
11551156
};
11561157

1158+
const updatePersistedAccountCountHint = (
1159+
count: number | null | undefined,
1160+
): void => {
1161+
if (!Number.isFinite(count) || count === undefined || count === null) {
1162+
return;
1163+
}
1164+
persistedAccountCountHint = Math.max(0, Math.trunc(count));
1165+
};
1166+
11571167
const resolvePersistedAccountSessionID = (
11581168
...candidates: Array<string | null | undefined>
11591169
): string | undefined => {
1160-
for (const candidate of [process.env.CODEX_THREAD_ID, ...candidates]) {
1170+
for (const candidate of [...candidates, process.env.CODEX_THREAD_ID]) {
11611171
const sessionID = candidate?.toString().trim();
11621172
if (sessionID) {
11631173
return sessionID;
@@ -1841,6 +1851,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
18411851
}
18421852

18431853
await saveAccounts(storage);
1854+
updatePersistedAccountCountHint(storage.accounts.length);
18441855

18451856
// Reload manager from disk so we don't overwrite newer rotated
18461857
// refresh tokens with stale in-memory state.
@@ -1916,10 +1927,11 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
19161927
}
19171928
if (!lastUserMessage) return Promise.resolve();
19181929

1919-
const sessionID =
1930+
const sessionID = resolvePersistedAccountSessionID(
19201931
typeof lastUserMessage.info.sessionID === "string"
19211932
? lastUserMessage.info.sessionID
1922-
: undefined;
1933+
: undefined,
1934+
);
19231935
const indicator = getPersistedAccountIndicatorLabel(sessionID);
19241936
if (!indicator) return Promise.resolve();
19251937

@@ -1981,6 +1993,9 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
19811993
}
19821994
let accountManager = await accountManagerPromise;
19831995
cachedAccountManager = accountManager;
1996+
updatePersistedAccountCountHint(
1997+
(await loadAccounts())?.accounts.length ?? accountManager.getAccountCount(),
1998+
);
19841999
const refreshToken = authFallback?.refresh ?? "";
19852000
const needsPersist =
19862001
refreshToken &&
@@ -2854,9 +2869,10 @@ while (attempted.size < Math.max(1, accountCount)) {
28542869

28552870
accountManager.recordSuccess(account, modelFamily, model);
28562871
if (persistAccountFooter) {
2857-
const persistedStorage = await loadAccounts();
2858-
const persistedAccountCount = persistedStorage?.accounts.length ??
2859-
accountManager.getAccountCount();
2872+
const persistedAccountCount =
2873+
persistedAccountCountHint > 0
2874+
? persistedAccountCountHint
2875+
: accountManager.getAccountCount();
28602876
setPersistedAccountIndicator(
28612877
threadIdCandidate,
28622878
account,

test/index.test.ts

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ type PluginType = {
415415
messages: Array<{
416416
info: {
417417
role: string;
418-
sessionID: string;
418+
sessionID?: string;
419419
model?: { providerID: string; modelID: string };
420420
variant?: string;
421421
thinking?: string;
@@ -2184,6 +2184,23 @@ describe("OpenAIOAuthPlugin fetch handler", () => {
21842184
);
21852185
});
21862186

2187+
it("does not reload account storage on the successful footer hot path", async () => {
2188+
await enablePersistedFooter("full-email");
2189+
mockStorage.accounts = [
2190+
{ accountId: "acc-1", email: "user@example.com", refreshToken: "refresh-token" },
2191+
{ accountId: "acc-2", email: "user2@example.com", refreshToken: "refresh-2" },
2192+
];
2193+
const storageModule = await import("../lib/storage.js");
2194+
const { sdk } = await setupPlugin();
2195+
2196+
await sendPersistedAccountRequest(sdk, "session-warmup");
2197+
vi.mocked(storageModule.loadAccounts).mockClear();
2198+
2199+
await sendPersistedAccountRequest(sdk, "session-no-read");
2200+
2201+
expect(storageModule.loadAccounts).not.toHaveBeenCalled();
2202+
});
2203+
21872204
it("decorates the last user message with a label-only indicator when configured", async () => {
21882205
await enablePersistedFooter("label-only");
21892206
const { plugin, sdk } = await setupPlugin();
@@ -2239,6 +2256,44 @@ describe("OpenAIOAuthPlugin fetch handler", () => {
22392256
expect((await readPersistedAccountIndicator(plugin, "session-chat-message")).thinking).toBeUndefined();
22402257
});
22412258

2259+
it("prefers the explicit request session key over CODEX_THREAD_ID", async () => {
2260+
await enablePersistedFooter("full-email");
2261+
process.env.CODEX_THREAD_ID = "env-session";
2262+
const { plugin, sdk } = await setupPlugin();
2263+
2264+
await sendPersistedAccountRequest(sdk, "session-explicit");
2265+
2266+
expect((await readPersistedAccountIndicator(plugin, "session-explicit")).variant).toBe(
2267+
expectedFullIndicator,
2268+
);
2269+
expect((await readPersistedAccountIndicator(plugin, "env-session")).variant).toBeUndefined();
2270+
});
2271+
2272+
it("falls back to CODEX_THREAD_ID in the transform hook when the message session is missing", async () => {
2273+
await enablePersistedFooter("full-email");
2274+
process.env.CODEX_THREAD_ID = "env-fallback";
2275+
const { plugin, sdk } = await setupPlugin();
2276+
2277+
await sendPersistedAccountRequest(sdk);
2278+
2279+
const output: Parameters<PluginType["experimental.chat.messages.transform"]>[1] = {
2280+
messages: [
2281+
{
2282+
info: {
2283+
role: "user",
2284+
model: { providerID: "openai", modelID: "gpt-5.1" },
2285+
},
2286+
parts: [],
2287+
},
2288+
],
2289+
};
2290+
await plugin["experimental.chat.messages.transform"]({}, output);
2291+
2292+
expect(
2293+
output.messages[0]?.info.model?.variant ?? output.messages[0]?.info.variant,
2294+
).toBe(expectedFullIndicator);
2295+
});
2296+
22422297
it("suppresses account-switch info toasts when the footer is enabled and refreshes the visible indicator", async () => {
22432298
await enablePersistedFooter("full-email");
22442299
mockStorage.accounts = [

0 commit comments

Comments
 (0)