Skip to content
Merged
59 changes: 59 additions & 0 deletions .github/workflows/backend-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,65 @@ jobs:
path: packages/api/dist
retention-days: 2

typecheck:
name: TypeScript type checks
needs: build
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
- uses: actions/checkout@v4

- name: Use Node.js 20.19
uses: actions/setup-node@v4
with:
node-version: '20.19'

- name: Restore node_modules cache
id: cache-node-modules
uses: actions/cache@v4
with:
path: |
node_modules
api/node_modules
packages/api/node_modules
packages/data-provider/node_modules
packages/data-schemas/node_modules
key: node-modules-backend-${{ runner.os }}-20.19-${{ hashFiles('package-lock.json') }}

- name: Install dependencies
if: steps.cache-node-modules.outputs.cache-hit != 'true'
run: npm ci

- name: Download data-provider build
uses: actions/download-artifact@v4
with:
name: build-data-provider
path: packages/data-provider/dist

- name: Download data-schemas build
uses: actions/download-artifact@v4
with:
name: build-data-schemas
path: packages/data-schemas/dist

- name: Download api build
uses: actions/download-artifact@v4
with:
name: build-api
path: packages/api/dist

- name: Type check data-provider
run: npx tsc --noEmit -p packages/data-provider/tsconfig.json

- name: Type check data-schemas
run: npx tsc --noEmit -p packages/data-schemas/tsconfig.json

- name: Type check @librechat/api
run: npx tsc --noEmit -p packages/api/tsconfig.json

- name: Type check @librechat/client
run: npx tsc --noEmit -p packages/client/tsconfig.json

circular-deps:
name: Circular dependency checks
needs: build
Expand Down
25 changes: 17 additions & 8 deletions packages/api/src/admin/config.handler.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import type { Response } from 'express';
import type { ServerRequest } from '~/types/http';
import { createAdminConfigHandlers } from './config';

function mockReq(overrides = {}) {
Expand All @@ -7,23 +9,30 @@ function mockReq(overrides = {}) {
body: {},
query: {},
...overrides,
};
} as Partial<ServerRequest> as ServerRequest;
}

interface MockRes {
statusCode: number;
body: undefined | { config?: unknown; error?: string; [key: string]: unknown };
status: jest.Mock;
json: jest.Mock;
}

function mockRes() {
const res = {
const res: MockRes = {
statusCode: 200,
body: undefined,
status: jest.fn((code) => {
status: jest.fn((code: number) => {
res.statusCode = code;
return res;
}),
json: jest.fn((data) => {
json: jest.fn((data: MockRes['body']) => {
res.body = data;
return res;
}),
};
return res;
return res as Partial<Response> as Response & MockRes;
}

function createHandlers(overrides = {}) {
Expand Down Expand Up @@ -93,7 +102,7 @@ describe('createAdminConfigHandlers', () => {
await handlers.getConfig(req, res);

expect(res.statusCode).toBe(200);
expect(res.body.config).toEqual(config);
expect(res.body!.config).toEqual(config);
});

it('returns 400 for invalid principalType', async () => {
Expand Down Expand Up @@ -191,7 +200,7 @@ describe('createAdminConfigHandlers', () => {
await handlers.deleteConfigField(req, res);

expect(res.statusCode).toBe(400);
expect(res.body.error).toContain('query parameter');
expect(res.body!.error).toContain('query parameter');
});

it('rejects unsafe field paths', async () => {
Expand Down Expand Up @@ -408,7 +417,7 @@ describe('createAdminConfigHandlers', () => {
await handlers.getBaseConfig(req, res);

expect(res.statusCode).toBe(200);
expect(res.body.config).toEqual({ interface: { endpointsMenu: true } });
expect(res.body!.config).toEqual({ interface: { endpointsMenu: true } });
});
});
});
22 changes: 17 additions & 5 deletions packages/api/src/app/service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
import type { AppConfig } from '@librechat/data-schemas';
import { createAppConfigService } from './service';

/** Extends AppConfig with mock fields used by merge behavior tests. */
interface TestConfig extends AppConfig {
restricted?: boolean;
x?: string;
interface?: { endpointsMenu?: boolean; [key: string]: boolean | undefined };
}

/**
* Creates a mock cache that simulates Keyv's namespace behavior.
* Keyv stores keys internally as `namespace:key` but its API (get/set/delete)
Expand All @@ -18,7 +26,9 @@ function createMockCache(namespace = 'app_config') {
return Promise.resolve(true);
}),
/** Mimic Keyv's opts.store structure for key enumeration in clearOverrideCache */
opts: { store: { keys: () => store.keys() } },
opts: { store: { keys: () => store.keys() } } as {
store?: { keys: () => IterableIterator<string> };
},
_store: store,
};
}
Expand Down Expand Up @@ -123,8 +133,10 @@ describe('createAppConfigService', () => {

const config = await getAppConfig({ role: 'ADMIN' });

expect(config.interface.endpointsMenu).toBe(false);
expect(config.endpoints).toEqual(['openAI']);
// Test data uses mock fields that don't exist on AppConfig to verify merge behavior
const merged = config as TestConfig;
expect(merged.interface?.endpointsMenu).toBe(false);
expect(merged.endpoints).toEqual(['openAI']);
});

it('caches merged result with TTL', async () => {
Expand Down Expand Up @@ -199,7 +211,7 @@ describe('createAppConfigService', () => {
const config = await getAppConfig({ role: 'ADMIN' });

expect(mockGetConfigs).toHaveBeenCalledTimes(2);
expect((config as Record<string, unknown>).restricted).toBe(true);
expect((config as TestConfig).restricted).toBe(true);
});

it('does not short-circuit other users when one user has no overrides', async () => {
Expand All @@ -216,7 +228,7 @@ describe('createAppConfigService', () => {
const config = await getAppConfig({ role: 'ADMIN' });

expect(mockGetConfigs).toHaveBeenCalledTimes(2);
expect((config as Record<string, unknown>).x).toBe('admin-only');
expect((config as TestConfig).x).toBe('admin-only');
});

it('falls back to base config on getApplicableConfigs error', async () => {
Expand Down
53 changes: 23 additions & 30 deletions packages/api/src/auth/openid.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import { Types } from 'mongoose';
import { ErrorTypes } from 'librechat-data-provider';
import { logger } from '@librechat/data-schemas';
import type { IUser, UserMethods } from '@librechat/data-schemas';
import { findOpenIDUser } from './openid';

function newId() {
return new Types.ObjectId();
}

jest.mock('@librechat/data-schemas', () => ({
...jest.requireActual('@librechat/data-schemas'),
logger: {
Expand All @@ -24,7 +29,7 @@ describe('findOpenIDUser', () => {
describe('Primary condition searches', () => {
it('should find user by openidId', async () => {
const mockUser: IUser = {
_id: 'user123',
_id: newId(),
provider: 'openid',
openidId: 'openid_123',
email: 'user@example.com',
Expand All @@ -51,7 +56,7 @@ describe('findOpenIDUser', () => {

it('should find user by idOnTheSource', async () => {
const mockUser: IUser = {
_id: 'user123',
_id: newId(),
provider: 'openid',
idOnTheSource: 'source_123',
email: 'user@example.com',
Expand All @@ -78,7 +83,7 @@ describe('findOpenIDUser', () => {

it('should find user by both openidId and idOnTheSource', async () => {
const mockUser: IUser = {
_id: 'user123',
_id: newId(),
provider: 'openid',
openidId: 'openid_123',
idOnTheSource: 'source_123',
Expand Down Expand Up @@ -109,16 +114,14 @@ describe('findOpenIDUser', () => {
describe('Email-based searches', () => {
it('should find user by email when primary conditions fail and openidId matches', async () => {
const mockUser: IUser = {
_id: 'user123',
_id: newId(),
provider: 'openid',
openidId: 'openid_123',
email: 'user@example.com',
username: 'testuser',
} as IUser;

mockFindUser
.mockResolvedValueOnce(null)
.mockResolvedValueOnce(mockUser);
mockFindUser.mockResolvedValueOnce(null).mockResolvedValueOnce(mockUser);

const result = await findOpenIDUser({
openidId: 'openid_123',
Expand Down Expand Up @@ -179,7 +182,7 @@ describe('findOpenIDUser', () => {
describe('Provider conflict handling', () => {
it('should return error when user has different provider', async () => {
const mockUser: IUser = {
_id: 'user123',
_id: newId(),
provider: 'google',
email: 'user@example.com',
username: 'testuser',
Expand All @@ -204,16 +207,14 @@ describe('findOpenIDUser', () => {

it('should reject email fallback when existing openidId does not match token sub', async () => {
const mockUser: IUser = {
_id: 'user123',
_id: newId(),
provider: 'openid',
openidId: 'openid_456',
email: 'user@example.com',
username: 'testuser',
} as IUser;

mockFindUser
.mockResolvedValueOnce(null)
.mockResolvedValueOnce(mockUser);
mockFindUser.mockResolvedValueOnce(null).mockResolvedValueOnce(mockUser);

const result = await findOpenIDUser({
openidId: 'openid_123',
Expand All @@ -230,16 +231,14 @@ describe('findOpenIDUser', () => {

it('should allow email fallback when existing openidId matches token sub', async () => {
const mockUser: IUser = {
_id: 'user123',
_id: newId(),
provider: 'openid',
openidId: 'openid_123',
email: 'user@example.com',
username: 'testuser',
} as IUser;

mockFindUser
.mockResolvedValueOnce(null)
.mockResolvedValueOnce(mockUser);
mockFindUser.mockResolvedValueOnce(null).mockResolvedValueOnce(mockUser);

const result = await findOpenIDUser({
openidId: 'openid_123',
Expand All @@ -258,7 +257,7 @@ describe('findOpenIDUser', () => {
describe('User migration scenarios', () => {
it('should prepare user for migration when email exists without openidId', async () => {
const mockUser: IUser = {
_id: 'user123',
_id: newId(),
email: 'user@example.com',
username: 'testuser',
// No provider and no openidId - needs migration
Expand Down Expand Up @@ -287,16 +286,14 @@ describe('findOpenIDUser', () => {

it('should reject when user already has a different openidId', async () => {
const mockUser: IUser = {
_id: 'user123',
_id: newId(),
provider: 'openid',
openidId: 'existing_openid',
email: 'user@example.com',
username: 'testuser',
} as IUser;

mockFindUser
.mockResolvedValueOnce(null)
.mockResolvedValueOnce(mockUser);
mockFindUser.mockResolvedValueOnce(null).mockResolvedValueOnce(mockUser);

const result = await findOpenIDUser({
openidId: 'openid_123',
Expand All @@ -313,16 +310,14 @@ describe('findOpenIDUser', () => {

it('should reject when user has no provider but a different openidId', async () => {
const mockUser: IUser = {
_id: 'user123',
_id: newId(),
openidId: 'existing_openid',
email: 'user@example.com',
username: 'testuser',
// No provider field — tests a different branch than openid-provider mismatch
} as IUser;

mockFindUser
.mockResolvedValueOnce(null)
.mockResolvedValueOnce(mockUser);
mockFindUser.mockResolvedValueOnce(null).mockResolvedValueOnce(mockUser);

const result = await findOpenIDUser({
openidId: 'openid_123',
Expand Down Expand Up @@ -422,16 +417,14 @@ describe('findOpenIDUser', () => {

it('should pass email to findUser for case-insensitive lookup (findUser handles normalization)', async () => {
const mockUser: IUser = {
_id: 'user123',
_id: newId(),
provider: 'openid',
openidId: 'openid_123',
email: 'user@example.com',
username: 'testuser',
} as IUser;

mockFindUser
.mockResolvedValueOnce(null)
.mockResolvedValueOnce(mockUser);
mockFindUser.mockResolvedValueOnce(null).mockResolvedValueOnce(mockUser);

const result = await findOpenIDUser({
openidId: 'openid_123',
Expand Down Expand Up @@ -460,7 +453,7 @@ describe('findOpenIDUser', () => {

it('should reject email fallback when openidId is empty and user has a stored openidId', async () => {
const mockUser: IUser = {
_id: 'user123',
_id: newId(),
provider: 'openid',
openidId: 'existing-real-id',
email: 'user@example.com',
Expand Down
Loading
Loading