[pull] main from danny-avila:main#152
Merged
Merged
Conversation
* Shared Role-Sync Core
* Environment Configuration
* Browser OpenID Wiring & improved shared component
* API Auth Wiring
* Improved Role Lookup
* added example for sync env
* small simplification
* protect existing manual assigned ADMIN Roles
* fix: Apply OpenID role-sync fallback for present-but-empty claims
Both role-sync call sites skipped on a falsy `openIdRoleValues`, treating an
empty claim string ('') the same as a missing claim and returning before
`selectOpenIdRole` could apply the configured fallback role. An IdP emitting
an empty roles claim for a user with no mapped groups left the stale local
role in place instead of the authoritative fallback.
Skip only when the helper returns `undefined` (missing/invalid), letting an
empty string flow through to fallback selection — consistent with how an
empty array is already handled. Adds regression coverage on both the OpenID
strategy and the remote-agent API auth paths.
* refactor: Address OpenID role-sync review feedback
- role.ts: reuse the shared escapeRegExp util instead of a local escapeRegex
duplicate, matching prompt/skill/user/userGroup methods (Copilot).
- openidStrategy.js / remoteAgentAuth.ts: make the tenantStorage.run callbacks
async so the documented ALS contract is satisfied and tenant context cannot
be lost during Mongoose execution; the wrapped lookups/updates are already
async, so behavior is unchanged (codex P2).
* fix: Harden OpenID role-sync claim and fallback handling
Addresses the second Codex review cycle (P2 findings):
- Apply fallback when the claim is absent: getOpenIdRolesForOpenIdSync now
returns an empty list (not undefined) when the token source exists but has
no usable claim value, so callers still run selection and assign the
configured fallback instead of leaving a stale elevated role. A truly
unavailable source still returns undefined and skips sync.
- Resolve group overage for access tokens too: the _claim_names/_claim_sources
overage path previously only ran for claimSource 'id'; Entra also moves an
oversized groups claim into access tokens, so 'access'+'groups' (the only
source supported by remote-agent API sync) now resolves overage as well.
- Allow system fallback roles for tenant users: getLibreChatRolesForOpenIdSync
treats SystemRoles (e.g. USER) as always-available canonical names, since
they are provisioned globally at startup and a tenant-scoped lookup may not
return them — preventing a spurious 'configured roles do not exist: USER'.
Adds unit and strategy-level coverage for all three.
* fix: Tighten OpenID role-sync tenant scoping and config validation
Addresses the third Codex review cycle:
- Constrain base-user role lookups to base roles (P2): findRolesByNames now
filters to roles with an unset tenantId when no tenant ALS context is active,
so a base user cannot match — and be assigned — a role that only exists within
a tenant. Tenant-scoped lookups remain controlled by the isolation plugin.
- Re-enforce tenant login policy after role sync (P2): when role sync changes a
tenant user's role, the OpenID strategy re-resolves the tenant appConfig and
re-checks allowedDomains, so a token cannot complete login under the previous
role's looser policy.
- Skip role-sync-specific validation when disabled (P3): getOpenIdRoleSyncOptions
returns disabled options before validating role-sync settings, so a stale or
mistyped value no longer breaks OpenID login while the feature is off.
Adds unit and strategy-level coverage for all three.
* fix: Run base role lookups under system context for strict isolation
Follow-up to the base-role scoping fix (Codex P1). With TENANT_ISOLATION_STRICT=true,
the tenant-isolation pre('find') hook throws on a context-less query before the manual
tenantId filter is honored, so base OpenID/remote-agent auth would 500 instead of
validating base roles. findRolesByNames now runs the no-context lookup inside
runAsSystem (SYSTEM_TENANT_ID), bypassing strict-mode injection while still applying an
explicit base-role (tenantId unset) filter. Adds a strict-mode regression test.
---------
Co-authored-by: Peter Rothlaender <peter.rothlaender@ginkgo.com>
* feat: add agent file retention exemption * refactor: centralize agent file retention policy
…ostics (#13471) * Improve OAuth failure logging * Improve OAuth failure logging * test: type oauth failure request helper * refactor: move OpenID callback helper to api package
…LLM (#13472) * 🧪 feat: add e2e playwright tests * 🧪 feat: Add Playwright Recording Harness * test: fix mock playwright config * test: harden mock e2e environment * test: preserve mock dotenv secrets * test: harden mock isolation setup * ci: cache mock e2e builds * test: harden e2e cache and recorder checks * test: preserve data-provider exports in oauth route test * test: isolate mock auth logout state * test: allow isolated logout smoke setup * test: prepare logout smoke auth via api * test: isolate oauth route module mock --------- Co-authored-by: Danny Avila <danny@librechat.ai>
* ⚡ feat: Immediate Conversation Title Generation Generate conversation titles as soon as the request is made (in parallel with the response, from the user's first message) as the new default, fixing the #13318 race where a transient /gen_title 404 left new chats stuck on "New Chat". - Add per-endpoint `titleTiming` ('immediate' | 'final') to baseEndpointSchema; `endpoints.all` acts as the global default, unset = immediate. Resolve via a new `resolveTitleTiming` helper (`all` takes precedence). - Fire title generation in parallel with `sendMessage`; `titleConvo` waits (bounded, abortable) for the agent run and titles from the user input only. Persist after the conversation row exists; defer `disposeClient` until the title settles. - Expose `titleGenerationTiming` via startup config; `useTitleGeneration` fetches eagerly in immediate mode with a bounded 404 retry and never treats a transient 404 as final. Skip title queueing for temporary conversations. - Supersedes #13329 while incorporating its bounded 404-retry. * 🩹 fix: Address Copilot review findings on title timing - Guard against an undefined conversationId in addTitle (skip + warn) so the gen_title cache key can't collide as `userId-undefined` and saveConvo is never called without a conversationId. - Gate the title `useQueries` on `enabled` so no /gen_title request fires while unauthenticated (e.g. after logout) even if the module queue holds IDs. - Drop the stale `conversationId` param from the titleConvo JSDoc. - Add a regression test for the undefined-conversationId guard. * 🧵 fix: Harden immediate-title edge cases from codex review - Cancel in-flight immediate title generation when the request aborts: thread job.abortController.signal through addTitle so pressing Stop on a new chat neither consumes the title model nor surfaces a title for a cancelled turn. - Preserve a locally-applied title when the final SSE event's conversation carries no title yet (built before the title was saved), so long immediate-mode responses no longer revert the chat to "New Chat" until reload. - Guarantee one full post-completion gen_title fetch cycle before giving up, so a `final`-mode title (generated only after the stream ends) is still fetched under a global `immediate` default instead of being stranded. - Add regression tests for the abort propagation and the undefined-conversationId guard. * 🔁 fix: Correct title abort, post-completion refetch, and replacement ordering Follow-up to codex review of the immediate-title fixes: - Use a dedicated title AbortController instead of `job.abortController`. The latter is also aborted by `completeJob` on *successful* completion, which cancelled any title slower than a short response. The title is now cancelled only on a real user Stop or when the stream is replaced; a completed-then- aborted title is discarded (no save, cache cleared) rather than persisted. - Reset (not remove) the post-completion title query: `resetQueries` refetches the mounted observer with a fresh retry budget, whereas `removeQueries` left it stuck in its error state, so the promised post-completion cycle never ran. - Run the job-replacement check before resolving `convoReady`, and on a replaced stream cancel/discard the stale title so a discarded prompt can't persist a title. * 🧷 fix: Tighten title abort ordering and endpoint-level timing resolution Follow-up to codex review: - Abort the title controller before resolving `convoReady` on a stopped turn, so the title task can't resume and persist before the later abort. - Cancel the title and unblock its waits on ANY send failure (not just user aborts): a preflight/quota failure before the run exists otherwise hangs `_waitForRun`, deferring client disposal until the 45s title timeout. - Resolve `titleTiming` for custom endpoints via `getCustomEndpointConfig` (their config lives under `endpoints.custom[]`, not `endpoints[endpoint]`). - Derive the startup `titleGenerationTiming` via `resolveTitleTiming` for the agents endpoint so an endpoint-level `final` (without `endpoints.all`) is honored client-side instead of defaulting to immediate and burning eager gen_title polls. * 🪢 fix: Per-agent title timing and safer abort/replacement handling Follow-up to codex review: - Resolve `titleTiming` from the agent's actual endpoint after initialization, so a per-endpoint `final` override on a custom/provider endpoint backing an (ephemeral) agent is honored instead of always using the `agents` endpoint's value. - Don't preserve a locally-fetched title on a stopped (unfinished) turn: the server cancels and discards that title, so keeping it client-side would diverge from server state and leave the stopped chat titled until reload. - On abort/replacement, only delete the cached title if it still holds THIS task's value — a replacement stream shares the `userId-conversationId` key and may have already cached its own valid title that must not be removed. * 🪞 fix: Mirror AgentClient title-config resolution for titleTiming Per maintainer guidance, keep titleTiming resolution identical to how `AgentClient#titleConvo` already resolves the endpoint config — `endpoints.all` is the intended global override and the agent's actual provider endpoint is used: - Resolve via `endpoints.all ?? endpoints[endpoint] ?? getProviderConfig(endpoint) .customEndpointConfig` (was using `getCustomEndpointConfig` directly). Going through `getProviderConfig` picks up its case-insensitive fallback for normalized provider names (e.g. `openrouter` → `OpenRouter`), so a custom endpoint's `titleTiming` is honored like its other title settings. - Add `titleTiming` to the Azure endpoint schema `.pick()` so `endpoints.azureOpenAI.titleTiming` is no longer silently stripped by Zod. Note: per-endpoint title settings being skipped when `endpoints.all` is present is the existing, intended global-override behavior — not changed here. * 🧪 test: Cover useTitleGeneration effect logic (integration) Adds a deterministic white-box integration test that drives the real hook's React effects with a controllable react-query surface, locking down the stateful decisions that previously had no coverage: - immediate mode fetches a queued conversation while its stream is still active - final mode gates until the stream completes, then becomes eligible - success applies the fetched title to the conversation caches - a 404 while active defers (removeQueries) instead of giving up - a 404 after completion forces a fresh fetch via resetQueries (post-completion remount) * feat: Stream immediate title events * style: Format title SSE handler * test: Preserve data-provider exports in OAuth mock * test: Isolate OAuth route API mock * test: Keep OAuth callback factory capture * fix: Replay streamed title events on resume * fix: Honor agents title timing precedence * style: Format title timing fixes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )