Summary
getOpenAIClient() caches a single module-level client and never invalidates it. Once built, it is reused for the lifetime of the process regardless of whether the credentials that built it are still the ones in effect.
P1 #11 on the mobile-consumable list. Narrower than it first looks -- see the scope note -- but a real defect on the exact flow a mobile settings screen performs.
Evidence
src/llm/openai.ts:96-110:
let openAIInstance: OpenAI | null = null;
async function getOpenAIClient(): Promise<OpenAI> {
if (!openAIInstance) {
const apiKey = getEnv('OPENAI_API_KEY');
if (!apiKey) {
throw new Error('OPENAI_API_KEY not found in environment variables');
}
openAIInstance = new OpenAI(buildOpenAIClientOptions({ apiKey }));
await Logger.debug('OpenAI client initialized');
}
return openAIInstance;
}The cache key is null-vs-not-null. It is not keyed on the API key, the base URL, or the fetch implementation.
Why this matters
A long-lived process where credentials can change -- which is what a mobile app is -- gets the wrong client:
- User opens the app with no key, or a stale one.
- Something triggers a call; the client is constructed and cached.
- User pastes a working key into settings.
- Every subsequent request still uses the old client and keeps failing, with an auth error pointing at a key the user can see is correct in the UI.
The user's only recovery is to force-quit the app, and nothing on screen suggests that. The same applies to a server process that rotates a key without restarting.
Scope note - narrower than the original list implied
getOpenAIClient() is the fallback path. It is reached from src/llm/openai.ts:159 only when an OpenAIService was constructed without per-instance options. An Agent created with apiKey / baseURL / fetch on its config builds its own client and never touches the singleton -- that half of P1 #11 ("credentials on config") is already done.
So this affects callers using the module-level convenience functions, and any Agent constructed without explicit credentials that relies on the environment. That is still the default path in most examples.
Required changes
Key the cache on what actually determines client identity, rather than on existence:
let cachedClient: OpenAI | null = null;
let cachedIdentity: string | null = null;
async function getOpenAIClient(): Promise<OpenAI> {
const apiKey = getEnv('OPENAI_API_KEY');
if (!apiKey) throw new Error('OPENAI_API_KEY not found in environment variables');
const baseURL = getEnv('OPENAI_BASE_URL') ?? '';
const identity = `${apiKey} ${baseURL}`;
if (cachedClient === null || cachedIdentity !== identity) {
cachedClient = new OpenAI(buildOpenAIClientOptions({ apiKey }));
cachedIdentity = identity;
}
return cachedClient;
}Two details that are not incidental:
- Include the base URL in the identity. Pointing at a different gateway with the same key is a different client, and a key-only cache serves the wrong endpoint.
- Never log the identity. It contains the secret. The existing
Logger.debug('OpenAI client initialized') is fine; anything printing the key, or a prefix of it, is not.
Optionally also export resetOpenAIClient() for tests and for a settings screen that wants to force a rebuild without depending on the cache rule.
Suggested tests
- changing
OPENAI_API_KEY between two calls yields a different client instance
- the pair: an unchanged key yields the same instance -- otherwise "never cache" passes the first test while removing the caching this function exists for
- changing only the base URL also yields a new instance
- a missing key still throws, and throws before any client is constructed
Scope boundary
This issue touches src/llm/openai.ts only. It is deliberately disjoint from the sibling issue filed alongside it (#4487, confined to src/agent/simple.ts), so the two can be worked in parallel with no merge conflicts.
Summary
getOpenAIClient()caches a single module-level client and never invalidates it. Once built, it is reused for the lifetime of the process regardless of whether the credentials that built it are still the ones in effect.P1 #11 on the mobile-consumable list. Narrower than it first looks -- see the scope note -- but a real defect on the exact flow a mobile settings screen performs.
Evidence
src/llm/openai.ts:96-110:The cache key is null-vs-not-null. It is not keyed on the API key, the base URL, or the
fetchimplementation.Why this matters
A long-lived process where credentials can change -- which is what a mobile app is -- gets the wrong client:
The user's only recovery is to force-quit the app, and nothing on screen suggests that. The same applies to a server process that rotates a key without restarting.
Scope note - narrower than the original list implied
getOpenAIClient()is the fallback path. It is reached fromsrc/llm/openai.ts:159only when anOpenAIServicewas constructed without per-instance options. AnAgentcreated withapiKey/baseURL/fetchon its config builds its own client and never touches the singleton -- that half of P1 #11 ("credentials on config") is already done.So this affects callers using the module-level convenience functions, and any
Agentconstructed without explicit credentials that relies on the environment. That is still the default path in most examples.Required changes
Key the cache on what actually determines client identity, rather than on existence:
Two details that are not incidental:
Logger.debug('OpenAI client initialized')is fine; anything printing the key, or a prefix of it, is not.Optionally also export
resetOpenAIClient()for tests and for a settings screen that wants to force a rebuild without depending on the cache rule.Suggested tests
OPENAI_API_KEYbetween two calls yields a different client instanceScope boundary
This issue touches
src/llm/openai.tsonly. It is deliberately disjoint from the sibling issue filed alongside it (#4487, confined tosrc/agent/simple.ts), so the two can be worked in parallel with no merge conflicts.