📎 fix: Route Unrecognized File Types via supportedMimeTypes Config#12508
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a silent attachment drop in the server message-building pipeline by ensuring previously “uncategorized” MIME types are still forwarded to providers, and additionally introduces an auth-aware startup config query key on the client to prevent stale unauthenticated config caching after login.
Changes:
- Route unrecognized attachment MIME types through the
documentspipeline inBaseClient.processAttachmentsso they’re encoded/sent instead of dropped. - Add
startupConfigKey(isAuthenticated)and update multiple client call sites to use the new query key shape. - Adjust login/query enablement flow and update a unit test mock to preserve new data-provider exports.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/hooks/SSE/useEventHandlers.ts | Uses new startupConfigKey when reading cached startup config during SSE handling. |
| client/src/hooks/Input/useQueryParams.ts | Uses new startupConfigKey when reading cached startup config for query-param-driven behavior. |
| client/src/hooks/Input/useQueryParams.spec.ts | Updates ~/data-provider mock strategy to preserve real exports like startupConfigKey. |
| client/src/hooks/Endpoint/useEndpoints.ts | Changes interface config fallback to defaults and adds getConfigDefaults usage. |
| client/src/hooks/Conversations/useNavigateToConvo.tsx | Uses new startupConfigKey when applying model spec effects on navigation. |
| client/src/hooks/Chat/useChatFunctions.ts | Uses new startupConfigKey when building chat submission payloads. |
| client/src/hooks/AuthContext.tsx | Re-enables queries via AuthContext after authentication state is applied. |
| client/src/data-provider/Endpoints/queries.ts | Introduces startupConfigKey and updates useGetStartupConfig to use it. |
| client/src/data-provider/Auth/mutations.ts | Disables queries during login mutation; re-enables on error and defers success re-enable to AuthContext. |
| client/src/components/Nav/SettingsTabs/Data/ImportConversations.tsx | Uses new startupConfigKey when reading import size limits. |
| client/src/components/Chat/Menus/Endpoints/ModelSelector.tsx | Hoists default interface config to a module constant. |
| api/app/clients/BaseClient.js | Adds fallback categorization so unknown MIME types go through documents handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+26
to
+33
| /** | ||
| * Auth-aware query key so unauthenticated (login page) and authenticated | ||
| * (chat page) configs are cached independently, preventing stale | ||
| * unauthenticated config from persisting after login. | ||
| */ | ||
| export const startupConfigKey = (isAuthenticated: boolean) => | ||
| [QueryKeys.startupConfig, isAuthenticated] as const; | ||
|
|
There was a problem hiding this comment.
PR description/title focus on routing unrecognized file types through the documents pipeline, but this file introduces a new auth-aware startupConfigKey and changes startup config caching behavior. Please either update the PR description to cover this behavioral change (and rationale/impact), or split it into a separate PR to keep review and risk scoped.
Comment on lines
+96
to
+112
| // Mock data-provider hooks while preserving real exports like startupConfigKey | ||
| jest.mock('~/data-provider', () => { | ||
| const actual = jest.requireActual<typeof import('~/data-provider')>('~/data-provider'); | ||
| return { | ||
| ...actual, | ||
| useGetAgentByIdQuery: jest.fn(() => ({ | ||
| data: null, | ||
| isLoading: false, | ||
| error: null, | ||
| })), | ||
| useListAgentsQuery: jest.fn(() => ({ | ||
| data: null, | ||
| isLoading: false, | ||
| error: null, | ||
| })), | ||
| }; | ||
| }); |
There was a problem hiding this comment.
This test now preserves the real startupConfigKey export, but the suite’s queryClient.getQueryData mock still appears to key off a plain 'startupConfig' string. Since production code now calls getQueryData(startupConfigKey(true)) (an array key), the mock should be updated to recognize the new key shape (or expose a mocked startupConfigKey that returns the old key) to avoid false negatives/failing tests.
Comment on lines
+96
to
+112
| // Mock data-provider hooks while preserving real exports like startupConfigKey | ||
| jest.mock('~/data-provider', () => { | ||
| const actual = jest.requireActual<typeof import('~/data-provider')>('~/data-provider'); | ||
| return { | ||
| ...actual, | ||
| useGetAgentByIdQuery: jest.fn(() => ({ | ||
| data: null, | ||
| isLoading: false, | ||
| error: null, | ||
| })), | ||
| useListAgentsQuery: jest.fn(() => ({ | ||
| data: null, | ||
| isLoading: false, | ||
| error: null, | ||
| })), | ||
| }; | ||
| }); |
There was a problem hiding this comment.
Using jest.requireActual('~/data-provider') in this mock will load the full data-provider module graph during the unit test, which can introduce side effects and makes the test more brittle (it depends on other mocks being set up correctly). Consider mocking only the needed exports (e.g., provide a lightweight startupConfigKey implementation alongside the hook mocks) instead of spreading the entire actual module.
danny-avila
force-pushed
the
claude/gracious-easley
branch
from
19ed4cc to
f31b6df
Compare
In processAttachments, files not matching the hardcoded mime type categories (image, PDF, video, audio) were silently dropped. Now resolves the endpoint's file config and checks the file type against supportedMimeTypes before routing to the documents pipeline. Files not matching any config are still skipped (original behavior). Closes #12482
Remove restrictive mime type filter in encodeAndFormatDocuments that only allowed PDFs and application/* types. Add a generic encoding path for non-PDF, non-Bedrock files using the provider's native format (Anthropic base64 document, OpenAI file block, Google media block). Files are already validated upstream by supportedMimeTypes.
danny-avila
force-pushed
the
claude/gracious-easley
branch
from
f31b6df to
3ef63eb
Compare
danny-avila
changed the title
- Add file.type truthiness check before checkType to prevent coercion of null/undefined to string 'null'/'undefined' - Cache mergedFileConfig and endpointFileConfig on the instance so addPreviousAttachments doesn't recompute per message
- Extract formatDocumentBlock helper to eliminate ~30 lines of duplicate provider-dispatch code between PDF and generic paths - Add size validation in generic encoding path using configuredFileSizeLimit (was fetched but unused) - Guard Bedrock from generic path — non-bedrockDocumentFormats types are now skipped instead of silently tracking metadata - Only push metadata to result.files when a document block was actually created, preventing silent inconsistent state - Enable Anthropic citations for text/plain, text/html, text/markdown (supported by Anthropic's document API) - Fix != to !== for Providers.AZURE comparison - Add 9 tests covering all four provider branches, Bedrock exclusion, size limit enforcement, and unhandled provider
filename parameter is string | undefined but OpenAIFileBlock and OpenAIInputFileBlock require string. Default to 'document' when filename is undefined.
Owner
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 361da1a7cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
Agent runs can have agent.provider set to a base provider (e.g., openAI) while agent.endpoint is a custom endpoint name. Using provider for the getEndpointFileConfig lookup bypassed custom endpoint supportedMimeTypes config. Now uses agent.endpoint, matching the pattern in addDocuments.
Bedrock only supports types in bedrockDocumentFormats. Previously, getFileStream was called for all files and unsupported types were discarded after download. Now pre-filters the file list for Bedrock to avoid unnecessary network and memory overhead for large unsupported attachments.
- Remove redundant ?? null intermediaries; use instance properties directly in the else-if condition - Add JSDoc @type annotations for _mergedFileConfig and _endpointFileConfig in the constructor
- Hoist configuredFileSizeLimit above the loop to avoid recomputing mergeFileConfig per file - Replace Buffer.from decode with base64 length formula in the generic size check to avoid unnecessary heap allocation - Use nullish coalescing (??) for filename fallback - Clean up test: remove unnecessary type cast, use createMockRequest helper for size-limit test - Add 14 tests for processAttachments categorization logic covering supportedMimeTypes routing, null/undefined guards, standard type passthrough, and edge cases
FileConfig.checkType is typed as optional. Use optional chaining to satisfy strict type checking.
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b2cf1dce0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
…eric routing - Return early from encodeAndFormatDocuments when the provider is neither document-supported nor Bedrock, avoiding unnecessary getFileStream calls for providers that would discard all results - Add !isBedrock guard to the supportedMimeTypes fallback branch in processAttachments so permissive patterns like '.*' don't route non-Bedrock types into documents that would be silently dropped - Add test for Bedrock + non-Bedrock-document-type skipping
Remove !isBedrock guard from the generic supportedMimeTypes routing branch. If a user configures permissive supportedMimeTypes for a Bedrock endpoint, the upload validation already accepted the file. The encoding layer pre-filters to Bedrock-supported types before fetching streams, so unsupported types are handled there without silently dropping files the user explicitly allowed.
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c108f8a09
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
6 tasks
yidianyiko
pushed a commit
to yidianyiko/LibreChat
that referenced
this pull request
Apr 13, 2026
…anny-avila#12508) * fix: check supportedMimeTypes before routing unrecognized file types In processAttachments, files not matching the hardcoded mime type categories (image, PDF, video, audio) were silently dropped. Now resolves the endpoint's file config and checks the file type against supportedMimeTypes before routing to the documents pipeline. Files not matching any config are still skipped (original behavior). Closes danny-avila#12482 * feat: encode generic document types for supported providers Remove restrictive mime type filter in encodeAndFormatDocuments that only allowed PDFs and application/* types. Add a generic encoding path for non-PDF, non-Bedrock files using the provider's native format (Anthropic base64 document, OpenAI file block, Google media block). Files are already validated upstream by supportedMimeTypes. * fix: guard file.type and cache file config in processAttachments - Add file.type truthiness check before checkType to prevent coercion of null/undefined to string 'null'/'undefined' - Cache mergedFileConfig and endpointFileConfig on the instance so addPreviousAttachments doesn't recompute per message * refactor: harden generic document encoding with validation and tests - Extract formatDocumentBlock helper to eliminate ~30 lines of duplicate provider-dispatch code between PDF and generic paths - Add size validation in generic encoding path using configuredFileSizeLimit (was fetched but unused) - Guard Bedrock from generic path — non-bedrockDocumentFormats types are now skipped instead of silently tracking metadata - Only push metadata to result.files when a document block was actually created, preventing silent inconsistent state - Enable Anthropic citations for text/plain, text/html, text/markdown (supported by Anthropic's document API) - Fix != to !== for Providers.AZURE comparison - Add 9 tests covering all four provider branches, Bedrock exclusion, size limit enforcement, and unhandled provider * fix: resolve filename type mismatch in formatDocumentBlock filename parameter is string | undefined but OpenAIFileBlock and OpenAIInputFileBlock require string. Default to 'document' when filename is undefined. * fix: use endpoint name for file config lookup in processAttachments Agent runs can have agent.provider set to a base provider (e.g., openAI) while agent.endpoint is a custom endpoint name. Using provider for the getEndpointFileConfig lookup bypassed custom endpoint supportedMimeTypes config. Now uses agent.endpoint, matching the pattern in addDocuments. * perf: filter non-Bedrock files before fetching streams Bedrock only supports types in bedrockDocumentFormats. Previously, getFileStream was called for all files and unsupported types were discarded after download. Now pre-filters the file list for Bedrock to avoid unnecessary network and memory overhead for large unsupported attachments. * refactor: clean up processAttachments file config handling - Remove redundant ?? null intermediaries; use instance properties directly in the else-if condition - Add JSDoc @type annotations for _mergedFileConfig and _endpointFileConfig in the constructor * refactor: harden document encoding and add routing tests - Hoist configuredFileSizeLimit above the loop to avoid recomputing mergeFileConfig per file - Replace Buffer.from decode with base64 length formula in the generic size check to avoid unnecessary heap allocation - Use nullish coalescing (??) for filename fallback - Clean up test: remove unnecessary type cast, use createMockRequest helper for size-limit test - Add 14 tests for processAttachments categorization logic covering supportedMimeTypes routing, null/undefined guards, standard type passthrough, and edge cases * fix: use optional chaining for checkType in routing tests FileConfig.checkType is typed as optional. Use optional chaining to satisfy strict type checking. * fix: skip stream fetches for unsupported providers, block Bedrock generic routing - Return early from encodeAndFormatDocuments when the provider is neither document-supported nor Bedrock, avoiding unnecessary getFileStream calls for providers that would discard all results - Add !isBedrock guard to the supportedMimeTypes fallback branch in processAttachments so permissive patterns like '.*' don't route non-Bedrock types into documents that would be silently dropped - Add test for Bedrock + non-Bedrock-document-type skipping * fix: respect supportedMimeTypes config for Bedrock endpoints Remove !isBedrock guard from the generic supportedMimeTypes routing branch. If a user configures permissive supportedMimeTypes for a Bedrock endpoint, the upload validation already accepted the file. The encoding layer pre-filters to Bedrock-supported types before fetching streams, so unsupported types are handled there without silently dropping files the user explicitly allowed.
jcbartle
pushed a commit
to jcbartle/LibreChat
that referenced
this pull request
May 11, 2026
…anny-avila#12508) * fix: check supportedMimeTypes before routing unrecognized file types In processAttachments, files not matching the hardcoded mime type categories (image, PDF, video, audio) were silently dropped. Now resolves the endpoint's file config and checks the file type against supportedMimeTypes before routing to the documents pipeline. Files not matching any config are still skipped (original behavior). Closes danny-avila#12482 * feat: encode generic document types for supported providers Remove restrictive mime type filter in encodeAndFormatDocuments that only allowed PDFs and application/* types. Add a generic encoding path for non-PDF, non-Bedrock files using the provider's native format (Anthropic base64 document, OpenAI file block, Google media block). Files are already validated upstream by supportedMimeTypes. * fix: guard file.type and cache file config in processAttachments - Add file.type truthiness check before checkType to prevent coercion of null/undefined to string 'null'/'undefined' - Cache mergedFileConfig and endpointFileConfig on the instance so addPreviousAttachments doesn't recompute per message * refactor: harden generic document encoding with validation and tests - Extract formatDocumentBlock helper to eliminate ~30 lines of duplicate provider-dispatch code between PDF and generic paths - Add size validation in generic encoding path using configuredFileSizeLimit (was fetched but unused) - Guard Bedrock from generic path — non-bedrockDocumentFormats types are now skipped instead of silently tracking metadata - Only push metadata to result.files when a document block was actually created, preventing silent inconsistent state - Enable Anthropic citations for text/plain, text/html, text/markdown (supported by Anthropic's document API) - Fix != to !== for Providers.AZURE comparison - Add 9 tests covering all four provider branches, Bedrock exclusion, size limit enforcement, and unhandled provider * fix: resolve filename type mismatch in formatDocumentBlock filename parameter is string | undefined but OpenAIFileBlock and OpenAIInputFileBlock require string. Default to 'document' when filename is undefined. * fix: use endpoint name for file config lookup in processAttachments Agent runs can have agent.provider set to a base provider (e.g., openAI) while agent.endpoint is a custom endpoint name. Using provider for the getEndpointFileConfig lookup bypassed custom endpoint supportedMimeTypes config. Now uses agent.endpoint, matching the pattern in addDocuments. * perf: filter non-Bedrock files before fetching streams Bedrock only supports types in bedrockDocumentFormats. Previously, getFileStream was called for all files and unsupported types were discarded after download. Now pre-filters the file list for Bedrock to avoid unnecessary network and memory overhead for large unsupported attachments. * refactor: clean up processAttachments file config handling - Remove redundant ?? null intermediaries; use instance properties directly in the else-if condition - Add JSDoc @type annotations for _mergedFileConfig and _endpointFileConfig in the constructor * refactor: harden document encoding and add routing tests - Hoist configuredFileSizeLimit above the loop to avoid recomputing mergeFileConfig per file - Replace Buffer.from decode with base64 length formula in the generic size check to avoid unnecessary heap allocation - Use nullish coalescing (??) for filename fallback - Clean up test: remove unnecessary type cast, use createMockRequest helper for size-limit test - Add 14 tests for processAttachments categorization logic covering supportedMimeTypes routing, null/undefined guards, standard type passthrough, and edge cases * fix: use optional chaining for checkType in routing tests FileConfig.checkType is typed as optional. Use optional chaining to satisfy strict type checking. * fix: skip stream fetches for unsupported providers, block Bedrock generic routing - Return early from encodeAndFormatDocuments when the provider is neither document-supported nor Bedrock, avoiding unnecessary getFileStream calls for providers that would discard all results - Add !isBedrock guard to the supportedMimeTypes fallback branch in processAttachments so permissive patterns like '.*' don't route non-Bedrock types into documents that would be silently dropped - Add test for Bedrock + non-Bedrock-document-type skipping * fix: respect supportedMimeTypes config for Bedrock endpoints Remove !isBedrock guard from the generic supportedMimeTypes routing branch. If a user configures permissive supportedMimeTypes for a Bedrock endpoint, the upload validation already accepted the file. The encoding layer pre-filters to Bedrock-supported types before fetching streams, so unsupported types are handled there without silently dropping files the user explicitly allowed.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
I fixed a silent failure where files with mime types not matching the hardcoded categories (image, PDF, video, audio) were dropped in
BaseClient.processAttachments, never reaching the provider. The upload succeeded and the file appeared in the UI, but was never included in the API request.supportedMimeTypesconfig fromfileConfiginsideprocessAttachmentsand check unrecognized file types against it before routing to the documents pipeline.file.typeagainst null/undefined before callingcheckTypeto prevent coercion to string'null'/'undefined'with permissive patterns like'.*'._mergedFileConfigand_endpointFileConfigon the instance soaddPreviousAttachmentsdoesn't recompute per historical message.agent.endpoint(notagent.provider) for thegetEndpointFileConfiglookup, matching the pattern inaddDocumentsand ensuring custom endpoint MIME configs are resolved correctly for agent runs.formatDocumentBlockhelper to eliminate ~30 lines of duplicate provider-dispatch code between the PDF and generic encoding paths.configuredFileSizeLimit.configuredFileSizeLimitabove the per-file loop to avoid recomputingmergeFileConfigon every iteration.bedrockDocumentFormatstypes are now skipped instead of silently tracking metadata with no document sent.result.fileswhen a document block was actually created, preventing silent inconsistent state.text/plain,text/html,text/markdown(supported by Anthropic's document API).!=to!==forProviders.AZUREcomparison; use??instead of||for filename fallback.@typeannotations for_mergedFileConfigand_endpointFileConfigin the constructor.Closes #12482
Change Type
Testing
librechat.yamlwith broadsupportedMimeTypes:.xlsx,.csv,.txt) to a conversation using that endpoint.supportedMimeTypesconfig matching the file type, unrecognized types are still skipped.cd packages/api && npx jest document processAttachments— 63 tests pass (40 existing + 9 encoding + 14 routing).Checklist