fix: invalidate cached OpenAI client when credentials change

[praisonai-triage-agent]

Fixes #4488

Summary

getOpenAIClient() in src/praisonai-ts/src/llm/openai.ts cached a single module-level client keyed on null-vs-not-null, so a changed OPENAI_API_KEY (or base URL) was ignored for the process lifetime — every subsequent request kept using the stale client. This is the fallback env-only path used by the module-level convenience functions and any Agent constructed without explicit credentials.

Changes

• Key the cache on client identity (OPENAI_API_KEY + OPENAI_BASE_URL) instead of existence, rebuilding when either changes.
• Include the base URL in the identity and forward it to the client so pointing at a different gateway serves the right endpoint.
• The identity string is never logged (it contains the secret).
• Export resetOpenAIClient() to force a rebuild (tests / settings screens).

Tests

New tests/unit/llm/openai-client-cache.test.ts covers:

• changed key → different instance
• unchanged key → same instance
• changed base URL → different instance
• missing key → throws before constructing
• resetOpenAIClient() forces a rebuild

npm run build and the OpenAI unit suites pass (17/17).

Scope is confined to src/praisonai-ts/src/llm/openai.ts (+ its test), disjoint from #4487.

Generated with Claude Code

[praisonai-triage-agent]

@coderabbitai review

[praisonai-triage-agent]

/review

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[greptile-apps]

Greptile Summary

The PR updates the TypeScript OpenAI fallback client cache so API-key or base-URL changes rebuild the client, while explicit resets invalidate both cached fields.

• Keys cached clients by the captured API key and base URL.
• Forwards the environment-provided base URL into newly constructed clients.
• Returns the invocation-local client to make overlapping initialization and reset safe.
• Adds focused tests for identity changes, reuse, missing credentials, resets, and overlapping calls.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/praisonai-ts/src/llm/openai.ts	Replaces the unkeyed singleton with an identity-keyed cache and safely returns the locally constructed client across asynchronous logging.
src/praisonai-ts/tests/unit/llm/openai-client-cache.test.ts	Adds unit coverage for credential and endpoint invalidation, stable reuse, reset behavior, missing keys, and the previously reported races.

_{Reviews (2): Last reviewed commit: "fix: return locally-built client to clos..." | Re-trigger Greptile}

[MervinPraison]

@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Review changes in this PR. Python SDK: praisonaiagents, praisonai. TypeScript SDK: src/praisonai-ts/. Do NOT modify src/praisonai-rust. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding.

Phase 1: Review per AGENTS.md

1. Protocol-driven: check heavy implementations vs core SDK
2. Backward compatible: ensure zero feature regressions
3. Performance: no hot-path regressions
4. SDK value: review in depth whether the change genuinely adds value to the SDK — never add features for the sake of adding them. It must strengthen the SDK (simpler, more user-friendly, robust, world-class, secure). If it does not clearly add value, request changes or recommend rejecting/closing rather than merging scope creep
5. Do not bloat the Agent class with additional params — only if absolutely required; we already support many params.
6. Repo routing: agent-callable tools → PraisonAI-Tools; lifecycle plugins → PraisonAI-Plugins; optional sandbox backends → PraisonAI-Plugins (praisonai.sandbox entry point) — request changes if wrongly added to praisonaiagents/

Phase 2: FIX Valid Issues
7. For any VALID bugs or architectural flaws found by Gemini, CodeRabbit, Qodo, Copilot, or any other reviewer: implement the fix
8. Also independently identify and fix any gaps or issues you find in the changed code — do not rely only on prior reviewer feedback
9. Push all code fixes directly to THIS branch (do NOT create a new PR)
10. Comment a summary of exact files modified and what you skipped

Phase 3: Final Verdict
11. If all issues are resolved, approve the PR / close the Issue
12. If blocking issues remain, request changes / leave clear action items

[praisonai-triage-agent]

Claude finished @MervinPraison's task —— View job

---

Final Architecture Review — PR #4505 ✅

• Read AGENTS.md + changed files
• Validate Greptile P1 "shared cache return race" → VALID, fixed
• Fix race + push (26980342a)
• Build & test — build OK, tests/unit/llm 153 passed / 18 skipped
• Final verdict → Approve

Phase 1 — Architecture (per AGENTS.md)

• Scope & routing: Correctly confined to the TypeScript SDK (src/praisonai-ts/src/llm/openai.ts + its test). Not an agent tool or lifecycle plugin — belongs in-repo, not in PraisonAI-Tools/Plugins. ✔
• Lightweight & no scope creep: Fixes a real defect (rotated OPENAI_API_KEY/OPENAI_BASE_URL ignored for process lifetime on the env-only fallback path). Keying the cache on credential identity is the minimal correct fix. ✔
• Agent class: No new Agent params, no bloat. resetOpenAIClient() is a small, justified test/settings escape hatch. ✔
• Backward compatible: Same signature/return; explicit-credential OpenAIService path untouched; base URL only forwarded when set. ✔
• Performance: Fast-path returns cached client on the first if; identity string is built cheaply and never logged (contains the secret). ✔

Phase 2 — Valid issue fixed

Greptile P1 — Shared cache return race (openai.ts:121) — confirmed valid.
Original code wrote cachedClient, then await Logger.debug(...) (a yield point), then returned the shared field. During that await a concurrent call with different credentials, or resetOpenAIClient(), could replace/null the field — so the first caller got the wrong client or null.

Fix: build the client into a const client, assign the cache, log, then return client (the local) — immune to concurrent mutation. Also short-circuits the cache-hit path before any await.

if (cachedClient !== null && cachedIdentity === identity) {
    return cachedClient;
}
const client = new OpenAI(buildOpenAIClientOptions({ apiKey, ...(baseURL ? { baseURL } : {}) }));
cachedClient = client;
cachedIdentity = identity;
await Logger.debug('OpenAI client initialized');
return client;

Added 2 regression tests: overlapping calls with different keys return distinct clients; reset() racing an in-flight init never returns null. 7/7 cache tests pass.

Files modified

• src/praisonai-ts/src/llm/openai.ts — return locally-built client; short-circuit cache hit
• src/praisonai-ts/tests/unit/llm/openai-client-cache.test.ts — +2 race regression tests

Skipped: nothing outstanding. (src/praisonai-rust untouched; no Python changes needed.)

Verdict

Approve. The race flagged by Greptile is resolved and covered by tests; the change is minimal, backward-compatible, and secure (secret never logged). Note: I cannot submit a formal GitHub approval for security reasons — this comment is my review.
· 26980342a