Skip to content

🔁 feat: Retry OpenID Discovery After Startup Failures - #15899

Merged
danny-avila merged 5 commits into
devfrom
danny-avila/openid-discovery-retry
Sep 14, 2026
Merged

danny-avila merged 5 commits into
devfrom
danny-avila/openid-discovery-retry

Conversation

@danny-avila

@danny-avila danny-avila commented Sep 14, 2026 •

Copy link
Copy Markdown
Collaborator

Continues #15617 by @insoln. Their three commits are kept as-is (rebased onto dev) so the work stays credited to them. The commits on top add librechat.yaml settings, move strategy registration into packages/api, and make configSchema the single place the defaults are defined.

Summary

What breaks: LibreChat tries OpenID discovery once at startup. If the identity provider or the network is down at that moment, the OpenID strategy is never registered, and OpenID login keeps failing until someone restarts the server.

After this change:

  • If that attempt fails, startup continues as before, and a background timer keeps retrying discovery.
  • Once discovery succeeds, the openid strategy (and openidJwt when token reuse is on) is registered without a restart.
  • By default startup still makes a single attempt, so it takes no longer than it did before. Operators can opt into extra blocking attempts at startup.

Configuration. Each field resolves separately: librechat.yaml first, then the env var, then the openIdDiscoverySchema default. An env value outside the schema bounds is logged and ignored.

Setting librechat.yaml Env var Default Bounds
Discovery attempts before startup continues (0 = background retries only) registration.openidDiscovery.startupAttempts OPENID_DISCOVERY_RETRY_ATTEMPTS 1 0–100
Milliseconds between attempts, at startup and in the background registration.openidDiscovery.retryDelayMs OPENID_DISCOVERY_RETRY_DELAY_MS 5000 100–3,600,000

Mechanism

configureOpenId(app, appConfig)                         api/server/socialLogins.js (wiring only)
└─ registerOpenIdWithRetry({ setupOpenId, registerJwtStrategy, reuseTokens,
                             discovery: appConfig.registration.openidDiscovery, env })
   │                                                    packages/api/src/auth/openid/retry.ts
   ├─ resolveOpenIdDiscovery(discovery, env)            yaml → env (bounds checked) → schema default
   ├─ each startup attempt: setupOpenId() → reuseTokens && registerJwtStrategy(config)
   │     returns null or throws → logged, counted as a failed attempt
   └─ still failing → setTimeout(retry, retryDelayMs).unref(); schedule again until it succeeds

Change Type

  • New feature (non-breaking change which adds functionality)

Testing

  • packages/api: npx jest src/auth/openid/retry.spec.ts, 11/11 pass. Covers:
    • the default of one startup attempt followed by recovery in the background, with no further retries after success
    • extra startup attempts when configured
    • 0 startup attempts
    • background retries continuing after strategy registration throws
    • yaml taking precedence over env vars, field by field
    • env values outside the bounds or unparseable (1, 99999999999, soon, -1, 1.5) falling back to the defaults
  • api: npx jest server/socialLogins.spec.js, 6/6 pass. Covers the wiring passed to the TS module and the existing session and token-reuse behavior.
  • packages/data-provider: npx tsc --noEmit is clean. packages/api: tsc --noEmit shows no errors outside the existing src/agents Providers type mismatch.

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review 93fb3a1

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-14T02:49:06.347476Z 92b2725 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 93fb3a1e99

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread api/server/socialLogins.js Outdated
Comment on lines +44 to +47
async function registerOpenIdStrategies() {
const config = await setupOpenId();
if (!config) {
return false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Move OpenID registration behavior out of /api

This newly added helper puts behavioral logic in the legacy CJS layer: it calls discovery, branches on its result, conditionally constructs/registers a strategy, and determines success for the retry policy. Move this behavior into packages/api and inject the strategy/discovery dependencies so this file only wires configuration and calls the TypeScript module, as required for edits to existing /api files.

AGENTS.md reference: AGENTS.md:L53-L57

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 92b2725. The discovery result check, the token-reuse branch, strategy registration and error handling now live in registerOpenIdWithRetry (packages/api/src/auth/openid/retry.ts). socialLogins.js only passes in setupOpenId, a registerJwtStrategy callback, reuseTokens and the raw config sources. Covered by retry.spec.ts; socialLogins.spec.js checks the wiring.

Comment thread packages/data-provider/src/config.ts Outdated
Comment on lines +2898 to +2900
startupAttempts: z.number().int().min(0).max(100).optional(),
/** Milliseconds between startup and background attempts. Default 5000. */
retryDelayMs: z.number().int().min(100).max(3_600_000).optional(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Define the retry defaults in configSchema

Both new fields are optional without schema defaults, while 3 and 5000 are independently hard-coded in retry.ts and repeated in the example YAML. Consequently getConfigDefaults() cannot provide these defaults and the three definitions can drift; resolve the legacy environment override separately, but make configSchema the canonical definition of the effective defaults.

AGENTS.md reference: AGENTS.md:L66-L68

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 92b2725. openIdDiscoverySchema in config.ts defines the defaults (1 and 5000). registration.openidDiscovery uses openIdDiscoverySchema.partial(), so an unset YAML field stays unset and the env var can still fill it. resolveOpenIdDiscovery falls back to schema.shape[field].parse(undefined), and retry.ts no longer hard-codes either value.

Comment thread packages/api/src/auth/openid/retry.ts Outdated
Comment on lines +3 to +4
const DEFAULT_STARTUP_ATTEMPTS = 3;
const DEFAULT_RETRY_DELAY_MS = 5000;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the existing one-attempt startup default

When an OpenID deployment omits both new settings and its provider is unavailable, this default changes startup from one discovery request followed by immediate continuation to three requests separated by two five-second waits. That adds at least ten seconds to every affected restart, contrary to the repository requirement that a new configurable lever preserve existing default behavior; default to one startup attempt and leave additional blocking retries opt-in.

AGENTS.md reference: AGENTS.md:L66-L68

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 92b2725. The default is now one startup attempt, so startup takes no longer than it did before. Blocking retries at startup are opt-in via startupAttempts, and retries after startup only run in the background with an unref'd timer. Covered by 'makes one startup attempt by default, then recovers in the background'.

Comment thread packages/api/src/auth/openid/retry.ts Outdated
Comment on lines +20 to +25
function parseRetryDelay(value: string | number | undefined): number {
const delay = Number(value);
if (value == null || value === '' || !Number.isFinite(delay) || delay <= 0) {
return DEFAULT_RETRY_DELAY_MS;
}
return delay;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Enforce the retry-delay bounds for environment values

When OPENID_DISCOVERY_RETRY_DELAY_MS supplies the fallback, any finite positive value bypasses the 100..3,600,000 bounds enforced for the equivalent YAML field. For example, 1 creates an indefinite millisecond retry loop against an unavailable provider, while a value above Node's timer range is coerced to roughly 1 ms; apply the same bounds before scheduling the timer.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 92b2725. Env values are checked against the same field schema as the YAML. A value that is out of bounds, not a whole number or unparseable is logged and replaced by the default. Covered by the table test for 1, 99999999999, soon, -1 and 1.5.

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review 92b2725

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 92b272593e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danny-avila danny-avila changed the title 🔁 fix: Retry OpenID Discovery After Startup Failures 🔁 feat: Retry OpenID Discovery After Startup Failures Sep 14, 2026
@danny-avila
danny-avila merged commit 854645f into dev Sep 14, 2026
40 checks passed
@danny-avila
danny-avila deleted the danny-avila/openid-discovery-retry branch September 14, 2026 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants