mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-06-16 07:51:32 +03:00
* ⚡ 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
192 lines
6.0 KiB
JavaScript
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');
|
|
});
|
|
});
|