🏁 fix: Invalidate Message Cache on Stream 404 Instead of Showing Error#12411
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses an SSE resume edge-case (treating 404/stream expired as a successful completion by refetching messages) and also introduces a new DB-backed admin configuration override system (schemas/models/methods, API handlers/routes, and package export/build changes).
Changes:
- Client: on SSE resume 404, invalidate message queries and clear cached stream-status instead of surfacing
STREAM_EXPIRED. - Backend: add
Configschema/model/methods plus AppConfig override resolution + caching service, and expose new admin config handlers/routes. - Build/exports: refactor
@librechat/data-schemascapability exports and switch rollup output to preserved modules with new subpath exports.
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/data-schemas/src/types/systemGrant.ts | Repoints SystemCapability type import to new admin types. |
| packages/data-schemas/src/types/index.ts | Exports new config and admin types. |
| packages/data-schemas/src/types/config.ts | Adds Config/IConfig types for DB overrides. |
| packages/data-schemas/src/types/admin.ts | Adds admin-facing types + capability typing helpers. |
| packages/data-schemas/src/systemCapabilities.ts | Removes legacy capabilities module (moved/refactored). |
| packages/data-schemas/src/schema/systemGrant.ts | Updates capability imports to new admin capabilities/types. |
| packages/data-schemas/src/schema/index.ts | Exports new configSchema. |
| packages/data-schemas/src/schema/config.ts | Adds Mongoose schema for config overrides with indexes. |
| packages/data-schemas/src/models/index.ts | Registers new Config model. |
| packages/data-schemas/src/models/config.ts | Adds Config model factory + tenant isolation plugin. |
| packages/data-schemas/src/methods/systemGrant.ts | Updates imports to new admin capabilities/types. |
| packages/data-schemas/src/methods/systemGrant.spec.ts | Updates tests to use new admin capabilities/types. |
| packages/data-schemas/src/methods/index.ts | Wires new ConfigMethods into createMethods. |
| packages/data-schemas/src/methods/config.ts | Adds CRUD/query methods for config overrides. |
| packages/data-schemas/src/methods/config.spec.ts | Adds unit tests for config methods. |
| packages/data-schemas/src/index.ts | Switches root export from legacy capabilities module to new admin export. |
| packages/data-schemas/src/app/resolution.ts | Adds deep-merge resolution of base config + DB overrides. |
| packages/data-schemas/src/app/resolution.spec.ts | Adds tests for override merge behavior + prototype-pollution stripping. |
| packages/data-schemas/src/app/index.ts | Exports new app resolution helper(s). |
| packages/data-schemas/src/admin/index.ts | Exposes admin module entrypoint. |
| packages/data-schemas/src/admin/capabilities.ts | New canonical capabilities + implication utilities + reserved base principal id. |
| packages/data-schemas/rollup.config.js | Changes rollup outputs to preserveModules with per-module artifacts. |
| packages/data-schemas/package.json | Bumps version, adds sideEffects:false, and adds subpath export for capabilities. |
| packages/data-provider/package.json | Bumps version and restricts published files to dist. |
| packages/api/src/middleware/capabilities.ts | Exports CapabilityUser and allows hasConfigCapability(..., null) broad checks. |
| packages/api/src/index.ts | Exports new admin module entrypoint. |
| packages/api/src/app/service.ts | Adds createAppConfigService for base config caching + DB override merging. |
| packages/api/src/app/service.spec.ts | Adds tests for app config caching/merge behavior and cache key scoping. |
| packages/api/src/app/index.ts | Exports app config service from app module. |
| packages/api/src/admin/index.ts | Exports admin config handler factory/types. |
| packages/api/src/admin/config.ts | Adds admin config handlers with validation + capability gating. |
| packages/api/src/admin/config.spec.ts | Unit tests for field-path validation helpers. |
| packages/api/src/admin/config.handler.spec.ts | Handler tests covering authz/validation invariants. |
| client/src/hooks/SSE/useResumeOnLoad.ts | Marks convo “processed” when no active stream job found to avoid repeated checks. |
| client/src/hooks/SSE/useResumableSSE.ts | On 404, invalidate messages + clear stream-status cache instead of injecting STREAM_EXPIRED. |
| client/src/hooks/SSE/tests/useResumableSSE.spec.ts | Updates tests to expect cache invalidation/removal instead of error handler call. |
| api/server/services/Config/app.js | Replaces legacy getAppConfig impl with createAppConfigService wrapper. |
| api/server/routes/index.js | Registers new admin config router export. |
| api/server/routes/admin/config.js | Adds /api/admin/config routes wired to new handlers and capability middleware. |
| api/server/middleware/config/app.js | Passes userId and tenantId into getAppConfig for scoped caching/overrides. |
| api/server/index.js | Mounts /api/admin/config route. |
| AGENTS.md | Documents naming/file-organization conventions. |
| .gitattributes | Enforces LF endings for husky hooks and shell scripts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e2e3cec to
e250236
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2502366e5
ℹ️ 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".
When a 404 (stream expired) is received during SSE resume, the generation has already completed and messages are persisted in the database. Instead of injecting an error message into the cache, invalidate the messages query so react-query refetches from the DB. Also clear stale stream status cache and step maps to prevent retries and memory leaks.
Prevents useResumeOnLoad from repeatedly re-checking the same conversation when the stream status returns inactive. The ref still resets on conversation change, so navigating away and back will correctly re-check. Also wait for background refetches to settle (isFetching) before acting on inactive status, preventing stale cached active:false from suppressing a valid resume.
Verify message cache invalidation, stream status removal, clearStepMaps, and setIsSubmitting(false) on the 404 path.
e250236 to
fdbb512
Compare
Remove unused ErrorTypes import in test, add queryClient to useCallback dependency array in useResumableSSE.
|
@codex review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e126a7b016
ℹ️ 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".
#12411) * fix: Invalidate message cache on STREAM_EXPIRED instead of showing error When a 404 (stream expired) is received during SSE resume, the generation has already completed and messages are persisted in the database. Instead of injecting an error message into the cache, invalidate the messages query so react-query refetches from the DB. Also clear stale stream status cache and step maps to prevent retries and memory leaks. * fix: Mark conversation as processed when no active job found Prevents useResumeOnLoad from repeatedly re-checking the same conversation when the stream status returns inactive. The ref still resets on conversation change, so navigating away and back will correctly re-check. Also wait for background refetches to settle (isFetching) before acting on inactive status, preventing stale cached active:false from suppressing a valid resume. * test: Update useResumableSSE spec for cache invalidation on 404 Verify message cache invalidation, stream status removal, clearStepMaps, and setIsSubmitting(false) on the 404 path. * fix: Resolve lint warnings from CI Remove unused ErrorTypes import in test, add queryClient to useCallback dependency array in useResumableSSE. * Reorder import statements in useResumableSSE.ts
danny-avila#12411) * fix: Invalidate message cache on STREAM_EXPIRED instead of showing error When a 404 (stream expired) is received during SSE resume, the generation has already completed and messages are persisted in the database. Instead of injecting an error message into the cache, invalidate the messages query so react-query refetches from the DB. Also clear stale stream status cache and step maps to prevent retries and memory leaks. * fix: Mark conversation as processed when no active job found Prevents useResumeOnLoad from repeatedly re-checking the same conversation when the stream status returns inactive. The ref still resets on conversation change, so navigating away and back will correctly re-check. Also wait for background refetches to settle (isFetching) before acting on inactive status, preventing stale cached active:false from suppressing a valid resume. * test: Update useResumableSSE spec for cache invalidation on 404 Verify message cache invalidation, stream status removal, clearStepMaps, and setIsSubmitting(false) on the 404 path. * fix: Resolve lint warnings from CI Remove unused ErrorTypes import in test, add queryClient to useCallback dependency array in useResumableSSE. * Reorder import statements in useResumableSSE.ts
danny-avila#12411) * fix: Invalidate message cache on STREAM_EXPIRED instead of showing error When a 404 (stream expired) is received during SSE resume, the generation has already completed and messages are persisted in the database. Instead of injecting an error message into the cache, invalidate the messages query so react-query refetches from the DB. Also clear stale stream status cache and step maps to prevent retries and memory leaks. * fix: Mark conversation as processed when no active job found Prevents useResumeOnLoad from repeatedly re-checking the same conversation when the stream status returns inactive. The ref still resets on conversation change, so navigating away and back will correctly re-check. Also wait for background refetches to settle (isFetching) before acting on inactive status, preventing stale cached active:false from suppressing a valid resume. * test: Update useResumableSSE spec for cache invalidation on 404 Verify message cache invalidation, stream status removal, clearStepMaps, and setIsSubmitting(false) on the 404 path. * fix: Resolve lint warnings from CI Remove unused ErrorTypes import in test, add queryClient to useCallback dependency array in useResumableSSE. * Reorder import statements in useResumableSSE.ts
Summary
Fixed a UX bug where returning to a completed conversation triggered a
STREAM_EXPIREDerror message, and addressed two related reliability issues in the resumable SSE resume flow.errorHandlercall on SSE 404 with aqueryClient.invalidateQuerieson the messages cache, causing React Query to refetch the persisted messages from the DB instead of injecting an error into the UI. The generation completed successfully — the messages are already there.clearStepMaps()to the 404 code path, matching every other terminal path in the hook. The omission caused run-step and tool-call map entries to leak for the lifetime of the hook instance when a stream was found completed on resume.queryClient.removeQueries, preventinguseResumeOnLoadfrom re-attempting a resume against a cached (and now invalid) status.useResumeOnLoadwhen the stream status returns inactive, eliminating repeated status checks on the same conversation within the same session. Navigation away resets the ref, so returning to the conversation re-checks correctly.isFetchingguard to theuseResumeOnLoadeffect so it waits for therefetchOnMountbackground fetch to settle before acting. Without this, a stale cachedactive: falseresult would mark the conversation as processed before the fresh response arrived, silently suppressing a valid resume.Change Type
Testing
Updated the
useResumableSSEspec to cover the new 404 behavior with explicit assertions:mockErrorHandleris not calledmockInvalidateQueriesis called with{ queryKey: ['messages', CONV_ID] }mockRemoveQueriesis called with{ queryKey: ['streamStatus', CONV_ID] }mockClearStepMapsis calledmockSetIsSubmittingis called withfalseTest Configuration:
For manual verification:
STREAM_EXPIREDerror banner.isFetchingguard prevents an earlyactive: falseresult from suppressing the resume when the background refetch returnsactive: true.Checklist