🔬 ci: Add TypeScript Type Checks to Backend Workflow and Fix All Type Errors#12451
🔬 ci: Add TypeScript Type Checks to Backend Workflow and Fix All Type Errors#12451danny-avila merged 8 commits intodevfrom
Conversation
…rce files - Constrain ConfigSection to string keys via `string & keyof TCustomConfig` - Replace broken `z` import from data-provider with TCustomConfig derivation - Add `_id: Types.ObjectId` to IUser matching other Document interfaces - Add `federatedTokens` and `openidTokens` optional fields to IUser - Type mongoose model accessors as `Model<IRole>` and `Model<IUser>` - Widen `getPremiumRate` param to accept `number | null` - Widen `bulkWriteAclEntries` ops to untyped `AnyBulkWriteOperation[]` - Fix `getUserPrincipals` return type to use `PrincipalType` enum - Add non-null assertions for `connection.db` in migration files - Import DailyRotateFile constructor directly instead of relying on broken module augmentation across mismatched node_modules trees - Add winston-daily-rotate-file as devDependency for type resolution
- Replace arbitrary test keys with valid TCustomConfig properties in config.spec - Use non-null assertions for permission objects in role.methods.spec - Replace `.SHARED_GLOBAL` access with `.not.toHaveProperty()` for legacy field - Add non-null assertions for balance, writeRate, readRate in spendTokens.spec - Update mock user _id to use ObjectId in user.test - Remove unused Schema import in tenantIndexes.spec
…nd test files - Widen getUserPrincipals dep type in capabilities middleware - Fix federatedTokens type in createSafeUser return - Use proper mock req type for read-only properties in preAuthTenant.spec - Replace `as IUser` casts with ObjectId-typed mocks in openid/oidc specs - Use TokenExchangeMethodEnum values instead of string literals in MCP specs - Fix SessionStore type compatibility in sessionCache specs - Replace `catch (error: any)` with `(error as Error)` in redis specs - Remove invalid properties from test data in initialize and MCP specs - Add String.prototype.isWellFormed declaration for sanitizeTitle spec
- Add default values for destructured bindings in OGDialogTemplate - Replace broken ExtendedFile import with inline type in FileIcon
Add a `typecheck` job that runs `tsc --noEmit` on all four TypeScript workspaces (data-provider, data-schemas, @librechat/api, @librechat/client) after the build step. Catches type errors that rollup builds may miss.
There was a problem hiding this comment.
Pull request overview
Adds explicit TypeScript type-checking to backend PR CI and resolves the resulting TS errors across the monorepo packages (schemas, API, client package, and tests), primarily by tightening types and adjusting test mocks to match runtime shapes.
Changes:
- Add a dedicated
tsc --noEmit“typecheck” job tobackend-review.ymlafter build artifacts are produced. - Fix/align core typings (e.g.,
IUser, principal typing, model typing, config section typing) to satisfy stricttsc. - Update numerous tests/mocks to use correct ObjectId/enums/nullability and fix a few runtime typings (winston rotate transport import, Redis scan iterator typing, etc.).
Reviewed changes
Copilot reviewed 38 out of 39 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/data-schemas/src/types/user.ts | Align IUser with ObjectId _id and add OIDC token fields. |
| packages/data-schemas/src/types/admin.ts | Re-derive ConfigSection from TCustomConfig instead of z/configSchema imports. |
| packages/data-schemas/src/migrations/tenantIndexes.ts | Add non-null assertion for connection.db. |
| packages/data-schemas/src/migrations/tenantIndexes.spec.ts | Adjust mongoose db typing to satisfy strict null checks. |
| packages/data-schemas/src/migrations/promptGroupIndexes.ts | Add non-null assertion for connection.db. |
| packages/data-schemas/src/methods/userGroup.ts | Type principalType as PrincipalType enum in return values. |
| packages/data-schemas/src/methods/userGroup.spec.ts | Update assertions to match ObjectId _id typing. |
| packages/data-schemas/src/methods/user.ts | Type mongoose model access + adjust lean() return typing and scoring map. |
| packages/data-schemas/src/methods/user.test.ts | Fix mock user _id to be an ObjectId. |
| packages/data-schemas/src/methods/tx.ts | Widen getPremiumRate arg to accept null. |
| packages/data-schemas/src/methods/spendTokens.spec.ts | Add non-null assertions to match nullable result shapes. |
| packages/data-schemas/src/methods/role.ts | Type mongoose Role model accessor as Model<IRole>. |
| packages/data-schemas/src/methods/role.methods.spec.ts | Add non-null assertions / cast records for permission map access. |
| packages/data-schemas/src/methods/config.spec.ts | Update config payloads to match typed config structure. |
| packages/data-schemas/src/methods/aclEntry.ts | Relax bulk write op typing to satisfy TS. |
| packages/data-schemas/src/config/winston.ts | Import DailyRotateFile directly and instantiate transport. |
| packages/data-schemas/src/config/meiliLogger.ts | Import DailyRotateFile directly and instantiate transport. |
| packages/data-schemas/package.json | Add winston-daily-rotate-file to devDependencies for local builds/tests. |
| packages/client/src/svgs/FileIcon.tsx | Remove ExtendedFile dependency and type progress explicitly. |
| packages/client/src/components/OGDialogTemplate.tsx | Fix destructuring defaults for legacy/non-legacy selection props. |
| packages/api/src/utils/sanitizeTitle.spec.ts | Add local global typing for String.prototype.isWellFormed. |
| packages/api/src/utils/oidc.spec.ts | Switch test user type from data-provider TUser to schemas IUser. |
| packages/api/src/utils/graph.spec.ts | Switch test user type to IUser and clean up mock typing. |
| packages/api/src/utils/env.ts | Type federatedTokens on safe user object more precisely. |
| packages/api/src/middleware/preAuthTenant.spec.ts | Type req mock more precisely (headers/ip/path). |
| packages/api/src/middleware/capabilities.ts | Update injected getUserPrincipals signature to match new typing. |
| packages/api/src/mcp/registry/cache/tests/ServerConfigsCacheRedisAggregateKey.cache_integration.spec.ts | Adjust spy call assertions to satisfy TS typing. |
| packages/api/src/mcp/registry/cache/tests/ServerConfigsCacheRedis.perf_benchmark.manual.spec.ts | Type Redis client for scanIterator usage. |
| packages/api/src/mcp/tests/MCPOAuthTokenStorage.test.ts | Align clientInfo test objects with expected shape. |
| packages/api/src/mcp/tests/MCPOAuthSecurity.test.ts | Mock fetch with required preconnect field for type compatibility. |
| packages/api/src/mcp/tests/MCPOAuthFlow.test.ts | Use TokenExchangeMethodEnum and tighten flow result typing. |
| packages/api/src/endpoints/custom/initialize.spec.ts | Adjust request body mocks to satisfy request typing. |
| packages/api/src/cache/tests/redisClients.cache_integration.spec.ts | Remove any in catches; cast to Error for message access. |
| packages/api/src/cache/tests/cacheFactory/sessionCache.cache_integration.spec.ts | Type session store union and bridge to test data shape. |
| packages/api/src/auth/openid.spec.ts | Update mocked users to use ObjectId _id. |
| packages/api/src/app/service.spec.ts | Introduce TestConfig helper type for mock-only fields in config tests. |
| packages/api/src/admin/config.handler.spec.ts | Strongly type request/response mocks and body assertions. |
| package-lock.json | Lockfile updates for dependency/version changes. |
| .github/workflows/backend-review.yml | Add typecheck job running tsc --noEmit across workspaces. |
Comments suppressed due to low confidence (1)
packages/data-schemas/src/methods/aclEntry.ts:382
- Changing
bulkWriteAclEntriesfromAnyBulkWriteOperation<IAclEntry>[]to the non-genericAnyBulkWriteOperation[]removes schema-level type checking for bulk ops, making it easy to pass operations that don’t match the AclEntry model shape. If the generic was causing a TS mismatch, consider narrowing withAnyBulkWriteOperation<IAclEntry>[](or a small wrapper/alias) rather than dropping the type parameter entirely.
async function bulkWriteAclEntries(
ops: AnyBulkWriteOperation[],
options?: { session?: ClientSession },
) {
const AclEntry = mongoose.models.AclEntry as Model<IAclEntry>;
return tenantSafeBulkWrite(AclEntry, ops, options || {});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1117737 to
bf0592e
Compare
…nsport The `winston-daily-rotate-file` package ships a module augmentation for `winston/lib/winston/transports`, but it fails when winston and winston-daily-rotate-file resolve from different node_modules trees (which happens in this monorepo due to npm hoisting). Add a local `.d.ts` declaration that augments the same module path from within data-schemas' compilation unit, so `tsc --noEmit` passes while keeping the original runtime pattern (`new winston.transports.DailyRotateFile`).
bf0592e to
d84582f
Compare
- Restore typed `AnyBulkWriteOperation<AclEntry>[]` on bulkWriteAclEntries, cast to untyped only at the tenantSafeBulkWrite call site (Finding 1) - Type `findUser` model accessor consistently with `findUsers` (Finding 2) - Replace numbered Record casts with `.not.toHaveProperty()` in role.methods.spec for SHARED_GLOBAL assertions (Finding 6) - Use per-test ObjectIds instead of shared testUserId in openid.spec (Finding 4) - Replace inline `import()` type annotations with top-level SessionData import in sessionCache spec (Finding 5) - Add `packages/client/node_modules` to typecheck CI cache paths (Finding 7) - Remove extraneous blank line in user.ts searchUsers (Finding 8)
- Restore typed `AnyBulkWriteOperation<AclEntry>[]` on bulkWriteAclEntries, cast to untyped only at the tenantSafeBulkWrite call site (Finding 1) - Type `findUser` model accessor consistently with `findUsers` (Finding 2) - Replace numbered Record casts with `.not.toHaveProperty()` in role.methods.spec for SHARED_GLOBAL assertions (Finding 6) - Use per-test ObjectIds instead of shared testUserId in openid.spec (Finding 4) - Replace inline `import()` type annotations with top-level SessionData import in sessionCache spec (Finding 5) - Add `packages/client/node_modules` to typecheck CI cache paths (Finding 7) - Remove extraneous blank line in user.ts searchUsers (Finding 8)
95e5e45 to
32d8501
Compare
- Restore typed `AnyBulkWriteOperation<AclEntry>[]` on bulkWriteAclEntries, cast to untyped only at the tenantSafeBulkWrite call site (Finding 1) - Type `findUser` model accessor consistently with `findUsers` (Finding 2) - Replace numbered Record casts with `.not.toHaveProperty()` in role.methods.spec for SHARED_GLOBAL assertions (Finding 6) - Use per-test ObjectIds instead of shared testUserId in openid.spec (Finding 4) - Replace inline `import()` type annotations with top-level SessionData import in sessionCache spec (Finding 5) - Add `packages/client/node_modules` to typecheck CI cache paths (Finding 7) - Remove extraneous blank line in user.ts searchUsers (Finding 8)
32d8501 to
e42b454
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
- Restore typed `AnyBulkWriteOperation<AclEntry>[]` on bulkWriteAclEntries,
cast to untyped only at the tenantSafeBulkWrite call site (Finding 1)
- Type `findUser` model accessor consistently with `findUsers` (Finding 2)
- Replace inline `import('mongoose').ClientSession` with top-level import type
- Use `toHaveLength` for spy assertions in playwright-expect spec file
- Replace numbered Record casts with `.not.toHaveProperty()` in
role.methods.spec for SHARED_GLOBAL assertions
- Use per-test ObjectIds instead of shared testUserId in openid.spec
- Replace inline `import()` type annotations with top-level SessionData
import in sessionCache spec
- Remove extraneous blank line in user.ts searchUsers
e42b454 to
01a445e
Compare
- Extract OIDCTokens interface in user.ts; deduplicate across IUser fields and oidc.ts FederatedTokens (Finding 4) - Move String.isWellFormed declaration from spec file to project-level src/types/es2024-string.d.ts (Finding 5) - Replace verbose `= undefined` defaults in OGDialogTemplate with null coalescing pattern (Finding 6) - Replace `Record<string, unknown>` TestConfig with named interface containing explicit test fields (Finding 7)
Summary
I added a
tsc --noEmittype-check job to the backend review CI workflow and fixed all ~180 TypeScript type errors across the four workspace packages. Previously, type errors could slip through CI because the only TypeScript validation came implicitly through rollup builds, which are more lenient than stricttscchecking.typecheckjob tobackend-review.ymlthat runstsc --noEmitondata-provider,data-schemas,@librechat/api, and@librechat/clientafter the build stepConfigSectiontype derivation by replacing brokenzimport from data-provider withTCustomConfig-based derivation_id: Types.ObjectIdtoIUserinterface, consistent with other Document types (IAclEntry,IGroup, etc.)federatedTokensandopenidTokensoptional fields toIUsermongoose.models.*accessors asModel<IRole>andModel<IUser>instead of relying on unsafe castsDailyRotateFileconstructor directly instead of relying on broken winston module augmentation caused by mismatchednode_modulestreesgetUserPrincipalsreturn type to usePrincipalTypeenum instead ofstringgetPremiumRateparameter to acceptnumber | nullmatching its runtime behaviorObjectId-typed mock IDs, correct enum values, non-null assertions for test-guaranteed data, and typed mock request objectsOGDialogTemplatedestructuring defaults and remove brokenExtendedFileimport inFileIconChange Type
Testing
Verified all four workspaces pass
tsc --noEmitwith zero errors:Confirmed
tscexits with code 2 and surfaces error output when a type error is introduced, ensuring the CI job will properly fail and block the PR.Rebased onto latest
dev(including the tenant isolation PR) and re-verified all checks pass after conflict resolution.Test Configuration:
Checklist