Files
LibreChat/api/server/routes/oauth.test.js
Danny Avila 2ef7bdfbc2 feat: Immediate Conversation Title Generation (#13395)
*  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
2026-06-02 16:40:57 -04:00

192 lines
6.0 KiB
JavaScript

const express = require('express');
const request = require('supertest');
const originalDomainClient = process.env.DOMAIN_CLIENT;
process.env.DOMAIN_CLIENT = 'http://client.test';
const mockLogger = {
warn: jest.fn(),
error: jest.fn(),
info: jest.fn(),
debug: jest.fn(),
};
const mockOAuthHandler = jest.fn((_req, res) => res.status(204).end());
const mockOpenIDCallbackMiddleware = jest.fn((_req, _res, next) => next());
let mockOpenIDCallbackAuthenticatorOptions;
const mockCreateOpenIDCallbackAuthenticator = jest.fn((options) => {
mockOpenIDCallbackAuthenticatorOptions = options;
return mockOpenIDCallbackMiddleware;
});
const mockBuildOAuthFailureLog = jest.fn(({ provider, req, err, info, defaultMessage }) => ({
provider,
code: err?.code ?? info?.code ?? info?.error ?? req.query?.error,
name: err?.name ?? info?.name,
message:
err?.message ??
info?.message ??
info?.error_description ??
req.query?.error_description ??
defaultMessage,
cause_code: err?.cause?.code ?? info?.cause?.code,
cause_name: err?.cause?.name ?? info?.cause?.name,
has_code: req.query?.code != null,
has_state: req.query?.state != null,
query_error: req.query?.error,
query_error_description: req.query?.error_description,
path: req.path,
forwarded_for: req.headers?.['x-forwarded-for'],
user_agent: req.headers?.['user-agent'],
}));
const mockGetOAuthFailureMessage = jest.fn(
(req) =>
req.session?.messages?.pop() ??
req.query?.error_description ??
req.query?.error ??
'OAuth authentication failed',
);
const mockRedirectToAuthFailure = jest.fn((res, { clientDomain, authFailedError }) =>
res.redirect(`${clientDomain}/login?redirect=false&error=${authFailedError}`),
);
const mockPassportAuthenticate = jest.fn(() => (_req, _res, next) => next());
jest.mock('passport', () => ({
authenticate: (...args) => mockPassportAuthenticate(...args),
}));
jest.mock('openid-client', () => ({
randomState: jest.fn(() => 'random-state'),
}));
jest.mock('@librechat/data-schemas', () => ({
logger: mockLogger,
}));
jest.mock('librechat-data-provider', () => ({
...jest.requireActual('librechat-data-provider'),
ErrorTypes: {
AUTH_FAILED: 'auth_failed',
},
}));
jest.mock('@librechat/api', () => ({
buildOAuthFailureLog: (...args) => mockBuildOAuthFailureLog(...args),
createOpenIDCallbackAuthenticator: (...args) => mockCreateOpenIDCallbackAuthenticator(...args),
createSetBalanceConfig: jest.fn(() => (_req, _res, next) => next()),
getOAuthFailureMessage: (...args) => mockGetOAuthFailureMessage(...args),
redirectToAuthFailure: (...args) => mockRedirectToAuthFailure(...args),
}));
jest.mock('~/server/middleware', () => ({
checkDomainAllowed: jest.fn((_req, _res, next) => next()),
loginLimiter: jest.fn((_req, _res, next) => next()),
logHeaders: jest.fn((_req, _res, next) => next()),
}));
jest.mock('~/server/controllers/auth/oauth', () => ({
createOAuthHandler: jest.fn(() => mockOAuthHandler),
}));
jest.mock('~/models', () => ({
findBalanceByUser: jest.fn(),
upsertBalanceFields: jest.fn(),
}));
jest.mock('~/server/services/Config', () => ({
getAppConfig: jest.fn(),
}));
afterAll(() => {
if (originalDomainClient === undefined) {
delete process.env.DOMAIN_CLIENT;
return;
}
process.env.DOMAIN_CLIENT = originalDomainClient;
});
function getOAuthRouter() {
jest.resetModules();
return require('./oauth');
}
function createApp(sessionMessages) {
const app = express();
app.use((req, _res, next) => {
if (sessionMessages) {
req.session = { messages: [...sessionMessages] };
}
next();
});
app.use('/oauth', getOAuthRouter());
app.use((err, _req, res, _next) => {
res.status(500).json({ message: err.message });
});
return app;
}
describe('OAuth route failure logging', () => {
beforeEach(() => {
mockLogger.warn.mockClear();
mockLogger.error.mockClear();
mockLogger.info.mockClear();
mockLogger.debug.mockClear();
mockOAuthHandler.mockClear();
mockOpenIDCallbackMiddleware.mockClear();
mockBuildOAuthFailureLog.mockClear();
mockGetOAuthFailureMessage.mockClear();
mockRedirectToAuthFailure.mockClear();
mockPassportAuthenticate.mockClear();
mockOpenIDCallbackAuthenticatorOptions = undefined;
mockPassportAuthenticate.mockImplementation(() => (_req, _res, next) => next());
mockOpenIDCallbackMiddleware.mockImplementation((_req, _res, next) => next());
});
it('wires the package OpenID callback middleware into the route', async () => {
const app = createApp();
await request(app)
.get('/oauth/openid/callback?code=secret-code&state=secret-state')
.expect(204);
expect(mockOpenIDCallbackAuthenticatorOptions).toEqual({
passport: expect.objectContaining({ authenticate: expect.any(Function) }),
logger: mockLogger,
clientDomain: 'http://client.test',
authFailedError: 'auth_failed',
});
expect(mockOpenIDCallbackMiddleware).toHaveBeenCalledWith(
expect.any(Object),
expect.any(Object),
expect.any(Function),
);
expect(mockOAuthHandler).toHaveBeenCalled();
});
it('logs structured fallback errors without using Unknown OAuth error', async () => {
const app = createApp();
const response = await request(app)
.get('/oauth/error?error=access_denied&error_description=Denied%20by%20provider')
.set('x-forwarded-for', '203.0.113.10')
.expect(302);
expect(response.headers.location).toBe(
'http://client.test/login?redirect=false&error=auth_failed',
);
expect(mockLogger.warn).toHaveBeenCalledWith(
'[OAuth] Authentication failed',
expect.objectContaining({
provider: 'unknown',
code: 'access_denied',
message: 'Denied by provider',
query_error: 'access_denied',
query_error_description: 'Denied by provider',
has_code: false,
has_state: false,
forwarded_for: '203.0.113.10',
}),
);
expect(JSON.stringify(mockLogger.warn.mock.calls[0])).not.toContain('Unknown OAuth error');
});
});