Two gaps in the referenced-file-id capping:
The file_id branch matched File rows with no ownership check, and file ids
inside message/conversation documents are caller-supplied (saveMessage spreads
request params into the update). A crafted or imported message referencing
another user's file_id would therefore set that user's File.expiredAt during a
forced-retention cascade or migration, and the storage sweeper would later
delete their object. The referenced-id branch now always filters on File.user
(dropped entirely when the caller's user id is not a castable ObjectId string —
fail closed), in both capConversationFiles and the bulk tag/project scope. The
conversationId branch stays unscoped: those rows are server-created by the
conversation's own processes, and an id collision can only shorten the
colliding owner's files.
Assistants conversations persist thread uploads on Conversation.file_ids
(saveUserMessage/syncMessages), which the parent projections never read, so
files stored only there kept expiredAt: null after conversion. A shared
conversationSeedFileIds helper now seeds the id collection from both files and
file_ids across the cascade, saveConvo, the bulk cascade, and both sweep
passes.
collectConversationFileIds only read Message.files, but agent/tool outputs
(code artifacts, generated images) are stored on Message.attachments with their
own file_id references, and those File rows also commonly lack a
conversationId. A forced-retention conversion or migration could therefore
convert the conversation and messages while leaving attachment file metadata at
expiredAt: null, so storage cleanup never swept it.
Match messages carrying either field and collect file ids from both arrays.
Message-attachment uploads create File rows without a conversationId — they are
referenced only from Message.files[].file_id and the conversation's files array
(only Code Interpreter outputs set conversationId). The conversation-scoped file
caps therefore missed regular attachments entirely: converting a pre-existing
chat left their expiredAt null, so storage cleanup never swept them after the
conversation and messages expired.
Collect the referenced file ids (conversation.files plus one pass over the
chat's messages) and extend the file caps to match file_id as well as
conversationId, in the single cascade, saveConvo, the bulk tag/project cascade,
and both sweep passes. The id scan runs at conversion/alignment time only —
post-conversion uploads always receive a deadline at upload, so conforming-
parent writes keep the cheap conversationId-scoped cap. A file shared across
chats is capped to the earliest converting chat's deadline, consistent with
cap-don't-extend.
saveMessage's forced-retention cascade converts an old project-assigned chat to
isTemporary: true, but message-only writes (branch/artifact/abort saves) have no
saveConvo afterward to recompute the project's cached stats, so the count and
lastConversationId kept pointing at a chat the project view no longer shows.
Make cascadeForcedConversationRetention report whether it converted the parent
row and refresh the owning project's stats from saveMessage when it did. The
applyForcedRetention refresh is gated on the same signal, so conforming parents
no longer trigger a redundant stats recompute on every edit/feedback write.
saveConvo only ran the message/share/file caps when the touched conversation
itself needed forced conversion, so an archive/pin/title save on an
already-temporary chat with an earlier deadline skipped the child caps and any
lagging rows from before the mode switch or a partial backfill kept
expiredAt: null or a later deadline past the parent's TTL.
Run the child caps whenever forced retention resolves an active deadline for a
pre-existing conversation, matching the single cascade's self-healing behavior;
each cap is an indexed no-op once the chat's children conform. The old
"leaves messages untouched once ephemeral" expectation pinned the skip this
change removes, so it now asserts the permanent row is healed instead.
Both the migration sweep and the convert-on-touch cascade skipped a parent that
already satisfied the forced-retention gap filter, so an already-temporary chat
with dependent rows that lag behind it (permanent shares or later-window files
created before the mode switch, or left by a partial earlier backfill) kept
those children past the parent's TTL.
Cap the children independently of the parent gap check: the single cascade runs
the message/share/file caps before its parent conversion regardless of whether
the parent needs converting (each cap is an indexed no-op once children
conform; the bulk tag/project cascade already worked this way), and the sweep
gains an alignment pass that walks already-conforming temporary conversations
and caps their children to each parent's own deadline, reported via a new
`aligned` count. The cap helpers now return their modified counts to support
that reporting.
The migration sweep converted project-assigned conversations to
isTemporary: true, which visibleProjectConversationFilter excludes, but never
recomputed the owning projects' cached stats, leaving conversationCount and
lastConversationId pointing at chats the project view no longer shows.
Collect each converted conversation's project membership in the sweep (deduped
user/chatProjectId pairs returned alongside the counts; the sweep cannot refresh
directly without a circular dependency on the chat-project methods) and have the
migration recompute every affected project's stats inside the tenant context,
reporting the refreshed count.
saveConvo's findOneAndUpdate wrote isTemporary/expiredAt on the conversation
before the message/share/file backfills ran, so a child failure left the parent
conforming; the next save reloaded it, conversationNeedsForcedRetention returned
false, and the skipped child rows stayed permanent even though retries appeared
to succeed.
Run the child backfills before the parent conversion, matching the cascade and
sweep ordering, so a failed child leaves the conversation non-conforming and the
next save re-runs the whole backfill.
The migration loaded getAppConfig() with no tenant and swept every conversation
under runAsSystem, so in a multi-tenant deployment with per-tenant retention
config overrides it force-expired all tenants' chats using the system config's
window, even tenants that never enabled ephemeral retention.
Enumerate tenants from the conversations, and for each run getAppConfig and the
sweep inside tenantStorage.run({ tenantId }) so every query is scoped to that
tenant; skip a tenant whose resolved config is not ephemeral. Single-tenant and
pre-tenancy deployments (no tenantId) keep the existing system-config sweep, and
untenanted rows in a mixed deployment are left untouched (they cannot be scoped)
with a warning rather than converted cross-tenant.
cascadeForcedConversationRetention converted the parent conversation first and
only backfilled its messages, shares, and files when that update modified a row.
If a child backfill threw after the parent was converted, a later forced-retention
write saw the parent already conforming (modifiedCount 0) and skipped the
children, leaving not-yet-updated messages, shares, or files permanent.
Gate on an in-memory conversationNeedsForcedRetention check and backfill the
children before marking the parent conforming, so a failed child leaves the
conversation non-conforming and the next write re-runs the whole cascade. The
early return preserves the no-op-when-already-conforming behavior.
saveConvo's inline forced-retention backfill converted the conversation and
capped its messages and shared links but left File records at expiredAt: null,
so archive/pin/title updates on a pre-existing chat under ephemeral mode left
its uploads in storage after the conversation and messages TTL out. File cleanup
only sweeps rows whose own expiredAt is set.
Cap the conversation's files alongside messages and shares, matching the other
forced-retention cascades.
applyForcedRetention (message edit/feedback/share/tag writes) converts a chat to
isTemporary: true, which visibleProjectConversationFilter excludes, but unlike
saveConvo it never recomputed the owning project's cached stats. A pre-existing
project chat converted this way left the project's conversationCount and
lastConversationId pointing at a chat the workspace no longer shows.
After the cascade, recompute the stats of every touched chat's project (the
single conversation for applyForcedRetention, all tagged chats for
applyForcedRetentionToTag), matching saveConvo's retention-visibility refresh.
Scoped to conversations carrying a chatProjectId, so it is a no-op for chats
outside any project.
The migration sweep caps a conversation's File records, but the runtime
convert-on-touch cascades did not: converting a pre-existing chat through a
message save, edit, feedback, tag, or project write set deadlines on the
conversation, messages, and shares while leaving File documents at
expiredAt: null. File cleanup only sweeps rows whose own expiredAt is set, so
those uploads outlived the ephemeral chat's TTL.
Cap the conversation's files alongside messages and shares in both
cascadeForcedConversationRetention (single) and the bulk tag/project cascade,
reusing capConversationFiles and threading the File model through the cascade
helpers and their callers. Files are scoped by conversationId (a unique per-chat
id) since File.user is an ObjectId rather than the string user id.
assignConversationToProject captured the findOneAndUpdate result before running
the forced-retention conversion, so under ephemeral mode the returned object was
a stale pre-conversion snapshot. The client writes that object straight into the
active conversation and the React Query cache, so an assigned/removed permanent
chat was cached as non-temporary with no expiredAt until a later refetch.
Run the retention conversion before capturing the updated document so the
findOneAndUpdate result carries the isTemporary/expiredAt fields alongside the
chatProjectId change. A no-op outside forced retention.
The migration sweep converted permanent conversations, messages, and shares to
the forced (ephemeral) window but left File records untouched. Files use a
retention-scoped expiredAt that application code sweeps (getExpiredFiles only
returns files whose own expiredAt is set), so uploads from migrated chats kept
expiredAt: null and lingered in storage after the conversation and messages
TTL out.
Add capConversationFiles and cap each converted conversation's files to the
same per-conversation deadline in sweepForcedRetention (and thread the File
model through the migration). Files with no expiration or a later one are set
to the deadline; files already expiring sooner are preserved. Scoped by
conversationId alone since File.user is an ObjectId rather than the string user
id the other cascades filter on. Under ephemeral retention every
conversation-scoped file is meant to expire, so no persistent-agent-file
exclusion is needed here.
The edit/feedback path calls applyForcedRetention with capExpiryToConversation,
which unconditionally set the touched message's expiredAt to the parent/fresh
forced deadline. When that message already expired sooner (an older message
keeps its creation-time deadline while later saves refreshed the parent's), the
$set extended it past its own schedule, overwriting the earlier deadline that
capForcedRetentionToParent had just preserved.
Cap the touched message to min(existing expiredAt, forcedExpiredAt) via the same
$min/$ifNull pipeline forceConversationMessagesTemporary already uses, so a
message with a sooner deadline keeps it and a permanent message still receives
the forced one.
Project membership writes bypassed forced retention: assigning a chat to a
project, removing it, or deleting the project rewrote the conversation row
(chatProjectId) without setting isTemporary/expiredAt. On a deployment that
switched to ephemeral mode, touching an older permanent chat this way left it
visible and non-expiring.
Load the interface config on the assign and delete project routes and cascade
forced retention through the touched conversation(s): assign/remove caps the
single conversation, its messages, and its shares; delete caps every member
chat before unassigning. Extract the bucketed tag cascade into a shared helper
so the project cascade reuses the same conversion/backfill/cap logic, and add a
shared resolveForcedRetentionDate helper. All paths are a no-op outside forced
retention, so non-ephemeral behavior is unchanged.
Two hardening fixes on the ephemeral-retention backfill:
- forceConversationMessagesTemporary and the by-tag cascade cap each
message with `$min: [{ $ifNull: ['$expiredAt', forced] }, forced]`.
A legacy permanent message (`expiredAt` null/missing) now reliably
receives the forced deadline instead of depending on `$min`'s null
handling, so the TTL index can remove it.
- sweepForcedRetention converts a conversation's messages and shares
before marking the conversation itself conforming. If a child backfill
throws, the conversation stays non-conforming and the gap-filtered
query picks it up again, keeping the migration safe to re-run.
Forced ephemeral retention only converts conversations that are
subsequently written (convert-on-touch), so enabling the mode on a
deployment with existing data left untouched permanent chats
(isTemporary: false/expiredAt: null) satisfying the visibility filter
and never expiring.
Add a sweepForcedRetention helper that streams every non-conforming
conversation and converts it, its messages, and its shares to the
forced window, capping each to the earlier of its own deadline and the
window so it never extends data scheduled to expire sooner and never
lets a message outlive its conversation. Expose it through a
config/migrate-ephemeral-retention.js script (and npm run
migrate:ephemeral-retention) that loads the app config, refuses to run
unless ephemeral mode is enabled (or --force), and supports --dry-run.
The sweep is idempotent and safe to re-run.
Two leftover paths still extended data that was already scheduled to
expire sooner when forcing ephemeral retention:
- forceConversationMessagesTemporary (and the by-tag message backfill)
rewrote every gap-matched message to the forced deadline, overwriting
carried-over `all`-mode messages whose own per-message TTL was already
sooner. Use a $min aggregation update so each message keeps its earlier
deadline while still being marked temporary and capped if later.
- File uploads under ephemeral retention skipped the conversation lookup
and got a fresh TTL, so a new attachment to a chat with a preserved
earlier expiry could outlive the TTL-deleted conversation. Cap ephemeral
file expiry to the minimum of the fresh window and the active parent
conversation expiry, mirroring the message and shared-link paths.
saveConvo now preserves a conversation's earlier ephemeral deadline
instead of refreshing it, so the message cap can no longer be opt-in.
A normal send to an existing ephemeral conversation whose parent
already expires sooner saved the new message with a fresh, later TTL;
saveConvo kept the parent at the earlier deadline and the cascade
skipped (parent already conforms), leaving the new message to outlive
the deleted conversation.
Cap every forced-retention message save to the parent's deadline, not
just the opt-in message-only paths. Message-only saves still backfill
the parent's other messages and shares; normal saves only cap the
message itself and let saveConvo and the cascade handle the rest.
Centralize the earlier-deadline rule in a capForcedRetentionExpiry
helper and apply it everywhere a forced-retention deadline is written:
capForcedRetentionToParent, the conversation cascade (which now re-reads
the parent and caps to it), and saveConvo's ephemeral branch (which
previously always refreshed to a fresh window, extending a conversation
that already expired sooner). No forced-retention path lengthens an
existing earlier expiry now.
Also scope the retainAgentFiles exception to RetentionMode.ALL, since
isAllDataRetention matches ephemeral too and would otherwise let agent
files persist past an ephemeral conversation's deadline.
The bookmark-tag and conversation ids passed to the forced-retention
helpers come from untyped request bodies, so a crafted PUT /api/tags
body like {"tag": {"$gt": ""}} reached Conversation.find({ tags }) as a
query operator and matched every tagged conversation instead of one,
bulk-converting them under ephemeral retention (NoSQL operator
injection). The same applied to req.body.conversationId on POST.
Guard applyForcedRetention and applyForcedRetentionToTag to ignore any
non-string conversationId/messageId/tag, and pass a guaranteed string
from the tag rename route.
Per-conversation tag writes already convert their conversation under
forced ephemeral retention, but global bookmark-tag renames and deletes
rewrite conversation rows via Conversation.updateMany without setting
isTemporary/expiredAt. A permanent chat tagged before the install
switched to ephemeral, touched only by a tag rename or delete, would
keep its retention fields unset and stay visible and non-expiring.
Add cascadeForcedRetentionByTag plus an applyForcedRetentionToTag method
that bulk-converts every conversation carrying a tag (and backfills its
messages and caps its shares) through the shared gap filter, so it never
extends a chat that already expires sooner. Load the interface config on
PUT /:tag and DELETE /:tag and route those writes through it.
A message-only forced save (branch/artifact/abort/edit) to a parent
carrying an active expiry sooner than the freshly computed ephemeral
window only capped when the parent was already temporary. An all-mode
parent (isTemporary: false) with an earlier active deadline skipped the
cap, so the touched message took the later window and the cascade then
rewrote the parent and its messages to that later date, extending data
that was meant to expire sooner.
Cap on any active parent expiredAt earlier than the forced window,
regardless of isTemporary, and feed the capped deadline into the
conversation cascade so it converts the parent without extending it.
Bookmark-tag writes update Conversation rows directly without saveConvo, so under
ephemeral retention adding a tag to a chat (createConversationTag) or changing a
chat's tag list (updateTagsForConversation) left an older permanent conversation
with isTemporary/expiredAt unset, keeping it visible and non-expiring.
Make applyForcedRetention's messageId optional so it can run the conversation
cascade alone, load app config on the per-conversation tag routes (POST /api/tags
when adding to a conversation, PUT /api/tags/convo/:conversationId), and enforce
retention after the write.
A SharedLink embeds a snapshot of the conversation (message refs and file
snapshots) and its TTL index keys off expiredAt alone, while shared-link reads
use activeExpirationFilter. So a permanent share (expiredAt null) created before
an install switched to ephemeral stayed publicly readable indefinitely after the
forced-temporary conversation and messages TTL out.
Add capConversationSharedLinks and call it wherever forced retention converts a
conversation - saveConvo's backfill, the shared cascade, and the cap-to-parent
path - so existing shares with no expiration or a later one are capped to the
forced deadline and expire with the conversation.
When a message-only forced save caps to a parent that is already temporary and
expires before the freshly computed window, the gated cascade leaves the parent
untouched (modifiedCount 0) and skips the message backfill. Older messages with
expiredAt null or a later deadline then survive after the parent's TTL deletes
the conversation.
Extract the cap logic into capForcedRetentionToParent, shared by saveMessage and
applyForcedRetention, which now also backfills the conversation's non-conforming
messages to the parent's earlier deadline. The hot normal-send path does not cap,
so it keeps the gated cascade with no extra per-message work.
Two more message-write paths bypassed ephemeral enforcement:
- The edit and feedback endpoints call updateMessage directly, without loading
retention config, so editing an older permanent message after a switch to
ephemeral left the message and its conversation non-temporary and visible.
Load config on those routes and run a new applyForcedRetention helper after the
update, which stamps the message and cascades the conversation/messages.
- The sendError and denyRequest middleware save messages with retention config
but never call saveConvo, so a validation/model error or denied-request message
could outlive its conversation. Pass capExpiryToConversation like the other
message-only paths.
Extract the conversation cascade into a shared cascadeForcedConversationRetention
helper used by both saveMessage and applyForcedRetention.
Capping every forced message save to the parent expiry broke the normal send
paths: POST /api/messages/:conversationId and BaseClient.saveMessageToDatabase
call saveConvo right after saveMessage, refreshing the conversation to a fresh
TTL. The message kept the older parent deadline, so the message TTL index could
delete the just-sent message while the conversation stayed visible until the
later deadline.
Gate the cap behind a capExpiryToConversation flag that only the message-only
callers (branch, artifact, abort) set, since those never run saveConvo. Normal
sends leave the message on its fresh deadline, which the following saveConvo
refresh keeps aligned. The conversion/re-cap cascade still runs for every forced
save.
A message-only forced save (branch, artifact, abort) does not run saveConvo, so
on an existing ephemeral conversation whose expiredAt is already sooner than the
freshly computed window the parent was left untouched while the message received
the later deadline. The conversation could then be TTL-deleted first, leaving the
new or edited message orphaned until its later expiry.
Read the parent once before saving the message and cap the message deadline to an
already-temporary parent that expires sooner, so the message never outlives its
conversation. The same read gates the existing conversion/re-cap cascade, which
keeps extending or shortening the parent when that is the correct action.
The forced-retention cascade only matched parents that were not yet temporary,
so switching from a longer temporary TTL to a shorter ephemeral one skipped
conversations already marked isTemporary: true. Their older messages kept the
longer deadline and could outlive the forced ephemeral window.
Match parents and messages whose expiration is missing or later than the forced
deadline, even when already temporary, via a shared gap filter. The cascade
re-caps those documents and stays a no-op once they already expire within the
forced window.
The forced-retention cascade keyed conversion off expiredAt: null, so a switch
from all to ephemeral retention skipped conversations that were already
isTemporary: false with a future expiredAt. Message-only paths (branch, artifact,
abort) then produced a temporary message under a parent that stayed visible in
history via the active non-temporary branch of the visibility filter.
Gate the cascade on isTemporary !== true instead so saveMessage, saveConvo, and
the message backfill all convert non-temporary parents and their messages
regardless of an existing active expiration, while remaining a no-op once a
conversation is already temporary.
Enabling ephemeral retention on an installation with pre-existing permanent
chats only stamped a TTL on whichever document a route happened to save.
saveConvo gave the conversation an expiredAt while its existing messages kept
expiredAt: null, so the messages outlived the conversation under the message
TTL index. Conversely, message-only routes (branch, artifact edits) marked the
message temporary while leaving the parent conversation permanent and visible.
Cascade the chosen expiration across both documents on conversion: saveConvo
backfills the conversation's existing messages when it first converts a
permanent conversation, and saveMessage converts the parent conversation (and
backfills its messages) when forced retention applies to a message. Both are
gated to the one-time permanent-to-ephemeral transition to avoid per-save
overhead.
getSharedLinkExpiration starts a brand-new retention window when the source
conversation still has an active expiration, letting a share outlive the
conversation whose messages it embeds. Cap the share at the earlier of the
source expiration and a freshly created window, and fall back to the source
expiration when window creation fails.
The Assistants chat path saved messages via recordMessage, which bypasses
retention, while saveConvo forced the conversation temporary. In ephemeral
mode the conversation expired but its messages persisted with no expiredAt.
Route saveUserMessage, saveAssistantMessage and syncMessages through
saveMessage so messages inherit the same forced retention as the conversation.
The messages routes (branch, artifact, post), share-link create/patch, and
the agent chat-abort route read req.config.interfaceConfig but never ran
configMiddleware, so req.config was undefined and retention was skipped.
Under ephemeral (and all) this let branched/edited/aborted messages and
shared links persist without isTemporary/expiredAt, outliving the chat.
Apply configMiddleware to those routes so retention is enforced
consistently. Add a getSharedLinkExpiration ephemeral test and keep route
test middleware mocks in sync.
Extends the `retentionMode` interface option with a new `ephemeral` value
that forces every conversation to be temporary and applies expiration
deadlines to all data. Unlike `all` (which keeps chats visible until they
expire), `ephemeral` marks all chats temporary so they are hidden from
history and search, with the per-chat toggle locked on.
- Add RetentionMode.EPHEMERAL plus isAllDataRetention and
isForcedTemporaryRetention helpers
- Force isTemporary and expiry on conversations, messages, imports, and files
- Force the TEMPORARY_CHAT permission on so the locked indicator is shown
- Lock the temporary-chat toggle in the UI and force new/existing chats temporary
- Document the new mode in librechat.example.yaml
* chore: add @langchain/langgraph-checkpoint-mongodb for HITL durable resume
* feat: HITL tool approval runtime — backend (Slice B)
- endpoints.agents.checkpointer config + durable Mongo checkpointer (seam over the app
connection; SDK MemorySaver fallback) with a TTL index + deleteThread pruning
- HITL run wiring (PreToolUse policy hook + humanInTheLoop) attached in createRun, fully
inert when toolApproval.enabled is off
- interrupt gate (pause job -> requires_action + emit on_pending_action) and a resume
route that rebuilds the run from the durable checkpoint and run.resume()s it
- atomic single-winner resolve; agent-consistency guard; expireStaleApprovals terminal
event; checkpoint pruned on every non-paused completion (thread_id == conversationId)
* feat: HITL tool approval UI — frontend (Slice B)
approve/reject/edit/respond + ask-user controls in the tool card (OAuth-button precedent),
batch-aware single submit, live + reconnect (resumeState.pendingAction) wiring, and resume
mutations posting to /agents/chat/resume.
* fix(hitl): decouple ApprovalProvider from chat context
ApprovalProvider is now pure state (safe to mount in provider-less / shared / test
renders); the context-dependent submit moved to a useResumeSubmit hook the cards call.
Part imports getAskUserQuestionPart from ~/utils/approval directly so suites that
partial-mock ~/utils render Part without throwing.
* fix(hitl): address Codex review — backend
- P1: enforce per-tool allowed_decisions on resume (reject a crafted decision the
policy disallows) via findDisallowedDecisions
- prune the durable checkpoint on user-abort of a paused run, and before a fresh
HITL turn, so a new turn cannot rehydrate an expired/aborted interrupt (thread_id
is the stable conversationId)
- persist + use isTemporary and the original parentMessageId on resume (temporary
chats stay temporary; initializeAgent scopes thread files off the right parent)
- generate a deferred first-turn title BEFORE completeJob so its event reaches the
client and the final event carries the real title
- moderateText: skip when there is no text (tool-approval resume) and moderate the
ask-user answer, instead of denying on an empty input
* fix(hitl): address Codex review — frontend
- render ToolApproval for ANY paused agent tool card (bash/code/file/etc.), not just
the generic ToolCall, by wrapping the tool-card branch in Part (moved the rendering
out of ToolCall)
- findPendingActionMessageIndex only matches an assistant message, never the user
message (the underscore-strip could target the user bubble before the assistant
placeholder exists)
* fix(hitl): address Codex re-review
- title eligibility checks the user message’s parent (first turn), not the response’s
parent — the previous check could never be true and skipped title generation
- use client.buildResponseMetadata() for the resumed message so contextUsage /
thoughtSignatures survive (the abort-only helper dropped them)
- moderate decisions[].responseText (the respond action’s user text)
- give /chat/abort req.config (configMiddleware) so the HITL checkpoint prune on abort
actually runs
- read resume state BEFORE setContentParts so the in-memory store does not lose the
pre-pause seed content
- count resumes against LIMIT_CONCURRENT_MESSAGES (increment/decrement) so paused-then-
resumed turns cannot bypass the limit
- require actionId on resume so a body without it cannot resolve the current action
* fix(hitl): address Codex re-review (round 3) — resume fidelity
Bring the lean resume path to parity with sendMessage for things it bypassed:
- carry userMCPAuthMap into the rebuilt run so approved MCP tools keep the user's creds
- seed initialSessions (buildInitialToolSessions) so approved code/file/skill tools have
the pre-pause uploaded-file context (esp. cross-replica / after restart)
- await client.artifactPromises and persist them as response attachments (else tool
artifacts created after the pause vanish on reload / for late subscribers)
- merge metadata: cumulative usage (+ summary marker) from the job, contextUsage /
thoughtSignatures from the client — fixes the round-2 regression that underreported
post-resume cost
* fix(hitl): address Codex re-review (round 4) — resume hardening
- resume: require an EXACT paused agent_id match (reject omitted/ephemeral
agent_id, not just a different one) and reject an endpoint mismatch, so a
request can't rebuild the claimed checkpoint on a different graph
- moderateText: also moderate a tool-approval decision's reject `reason` and
stringified `editedArguments`, not just `responseText`
- request: re-mark the paused response `unfinished:true` after BaseClient saves
it as completed, so an expired / never-resumed approval doesn't leave a
"finished" response in history; the resume path overwrites it on success
* test(hitl): route-level integration test for the resume controller
Adds api/server/controllers/agents/__tests__/resume.spec.js, a supertest
integration test that drives the real ResumeAgentController over the full
pause -> approve -> resume -> finalize lifecycle with the SDK run, durable
checkpointer, Mongo, and concurrency cache mocked. The pure decision/liveness
helpers run for real via requireActual, so the guard ladder is exercised end to
end rather than stubbed.
25 cases covering:
- the authorization / staleness / agent-and-endpoint / actionId guard ladder
- tool_approval validation (undecided tool call, policy-disallowed decision)
- ask_user_question answer requirement
- the concurrency gate (429) and the atomic single-winner claim (409)
- the happy path: ACK, run reconstruction, decision->SDK mapping, finalize
(save the now-finished response, emit done, complete job, prune checkpoint)
- first-turn title generation before stream completion
- re-pause (no double finalize), abort-during-resume (no double finalize),
and the resume-failure terminal path (emitError + completeJob + prune)
* test(hitl): strengthen resume coverage + add approval util tests
Acts on a self-audit of the new resume integration test.
resume.spec.js (25 -> 32 cases):
- replace the tautological emitDone assertion (it only checked the hardcoded
`final: true`) with a structural check of the finalEvent payload —
responseMessage content/id/unfinished, requestMessage identity, title
- cover the previously-unwalked finalize branches: tool-artifact attachments
(null-filtered), the aggregatedContent fallback when live content is empty,
and client response-metadata attachment
- add guard cases: unsupported pending-action type (400) and the
pre-multi-tenancy null-tenantId pass-through (must not 403)
- add error-path cases: first-turn title generation throwing must still
finalize, and a completeJob failure during a resume error must force a
terminal job state via the last-resort updateJob
client/src/utils/approval.spec.ts (new, 15 cases):
- applyPendingAction tool_approval: join by tool_call_id not position,
skip completed calls, default allowed_decisions to [], referential
stability when nothing changes
- applyPendingAction ask_user_question: append, idempotent replace on replay,
non-array content coercion
- getAskUserQuestionPart type guard; findPendingActionMessageIndex
assistant-only resolution (never resolves to the user bubble)
* fix(hitl): address Codex re-review (round 5)
Five findings verified against the code before fixing:
- resume: require an EXACT endpoint match (like agent_id) — a resume that OMITS
endpoint must not fall through, since the shared chat middleware treats a
missing/non-agents endpoint as the ephemeral agent and could rebuild the
claimed checkpoint on a different graph
- resume: filter malformed content parts before saving the finished response,
matching the normal AgentClient path (a resumed turn could otherwise persist
an empty/invalid tool_call part that breaks reload/rendering)
- resume: accumulate tool artifacts across pause segments — persist them on
re-pause and MERGE (not overwrite) at finalize, so artifacts produced before
a second approval pause aren't dropped by the next rebuilt client
- approval (client): findPendingActionMessageIndex returns -1 when a provided
responseMessageId isn't found, so the caller retries instead of attaching the
prompt/approval to a prior assistant reply; fall back to the last assistant
only when no responseMessageId is given
- RedisJobStore: make appendChunk extend-only (XADD + EXPIRE-if-shorter via a
single eval) so the on_pending_action chunk emitted after a pause can't reset
the chunk-stream TTL back to the running window and evict pre-pause content
before the approval is resolved
Tests: +endpoint-omitted/unsupported-type/malformed-filter/attachment-merge/
re-pause-persist cases in resume.spec.js (36); ask-retry -1 semantics in
approval.spec.ts (16); extend-only TTL assertion in the RedisJobStore Redis
integration spec.
* test(hitl): mongodb-memory-server integration test for the checkpointer seam
The checkpointer unit spec covers config/selection with no DB connection; this
exercises the durable Mongo seam against a real (in-memory) MongoDB — the part
correctness actually depends on:
- getAgentCheckpointer builds a real MongoDBSaver when Mongo is connected and
setup() creates the TTL index (expireAfterSeconds) on the checkpoint collection
- memory type returns undefined (SDK MemorySaver fallback) even when connected
- saver is memoized per resolved config
- deleteAgentCheckpoint prunes a thread's persisted checkpoint (the cross-turn
isolation guarantee: turn N+1 on the same conversationId can't rehydrate it)
- pruning is thread-scoped — deleting one conversation leaves others intact
- undefined threadId is a no-op
* fix(hitl): address Codex re-review (round 6)
Four findings verified against the code before fixing:
- messageFilterPii: scan the resume payload's user-authored text (ask-user
`answer`, and a tool-approval decision's `respond` text, `reject` reason, and
edited tool arguments) — the shared /resume route ran through the PII filter
but it only inspected req.body.text, so a blocked token rode the resume
payload back into the model/tool (mirrors the earlier moderateText fix)
- resume: re-prime skill files invoked in the pre-pause segment before rebuilding
the run, so an approved code/file-backed tool keeps the injected skill-file
session refs instead of running without them (mirrors the normal path's
primeInvokedSkills; the pre-pause content stands in for the message payload)
- hitl: pin the graph identity. Persist a fingerprint of the graph-determining
request fields (endpoint, agent_id, model, spec, ephemeralAgent — normalized)
on the pending action at pause, and reject a resume whose recomputed
fingerprint differs. This closes the ephemeral-agent gap, where agent_id is
undefined so the id guard can't tell two ephemeral configs apart
- resume: reject incomplete edit/respond decisions (findIncompleteDecisions) —
an `edit` without an object editedArguments or a `respond` without non-empty
responseText is 400'd before mapping, rather than defaulting to {} / '' and
resuming with behavior the user never approved
Tests: incomplete-decision + fingerprint match/mismatch cases in resume.spec.js
(41); findIncompleteDecisions + computeAgentRequestFingerprint unit tests; and
resume-field PII cases in messageFilterPii.spec.ts.
* fix(hitl): address Codex re-review (round 7)
Four findings verified against the code before fixing:
- RedisJobStore: clear `agent_id` on createJob (add it to staleHitlFields). The
job hash is keyed by conversationId and reused across turns; updateMetadata
only writes agent_id when truthy, so a conversation that switched from a saved
agent to an ephemeral/no-agent turn kept the old id and the resume guard
rejected the valid pause as a different agent. (real correctness bug)
- fingerprint: include `promptPrefix` in computeAgentRequestFingerprint, and
re-send it on resume (ResumeAgentFields + buildResumeFields). Ephemeral agents
derive their system instructions from promptPrefix, so a resume changing it
previously passed the pin and rebuilt different instructions. (completes the
round-6 fingerprint)
- resume: the re-pause branch now persists the segment's accumulated CONTENT
(filtered), not just artifacts, so an approval that expires/reaps without a
final resume no longer loses everything streamed during the resumed segment.
- request: carry `manualSkills`/`alwaysAppliedSkills` on the persisted user
message so a resumed turn's reconstructed requestMessage keeps its skill pills
instead of dropping them until a full reload.
Deferred (narrow, no safe contained fix yet — see PR thread replies):
- resume rebuild without `addedConvo` for a multi-conversation/added-agent pane
- cross-replica re-prime of manually-selected (not model-invoked) skill files
Tests: stale-agent createJob clearing (Redis integration), promptPrefix
fingerprint match/mismatch (resume.spec.js + policy.spec.ts), re-pause content
persistence (resume.spec.js).
* fix(hitl): address Codex re-review (round 8)
Five findings verified against the code before fixing; the headline is a durable-
resume correctness fix (the fingerprint had surfaced it as a 403):
- resume durability (the important one): persist the graph-determining request
fields (endpoint, agent_id, model, spec, promptPrefix, ephemeralAgent) on the
pending action as `resumeContext`, and REPLAY them onto the resume request via
a router-level middleware that runs before buildEndpointOption. The client
can't reconstruct the ephemeral-agent config after a reload/cross-session, so
the round-6/7 fingerprint would 403 a valid durable resume — and even without
it the rebuilt agent would lose its tools. Replaying server-side rebuilds the
SAME graph regardless of client state (and a crafted resume can't swap it; the
fingerprint still matches because the body is restored first).
- RedisJobStore: also clear `isTemporary` on createJob (same class as agent_id):
a prior temporary turn's flag would otherwise survive a reused conversation
hash and a later non-temporary resume would save its response as temporary.
- resume: persist `contextMeta` (context-window calibration) onto the saved
response like BaseClient does, so the next turn can seed its pruner.
- request: carry manualSkills/alwaysAppliedSkills into the onStart metadata
update (not just the preliminary one it overwrites), so a resumed turn's
requestMessage keeps its skill pills.
Deferred (narrow — see thread reply):
- saved-agent edited WHILE a run is paused: agent_id matches but the definition
changed; needs an agent version/config hash, which is a larger change for a
narrow window.
Tests: resumeContext pick/apply + round-trip (policy.spec.ts), contextMeta +
manualSkills-on-requestMessage (resume.spec.js), isTemporary clearing (Redis
integration).
* style(hitl): prettier line-wrap in policy.spec.ts (R8 lint fix)
* fix(hitl): address Codex re-review (round 9)
Five findings, all fixed (addedConvo — deferred in rounds 7/8 — is now trivial
thanks to the round-8 replay):
- replay addedConvo: add it to RESUME_CONTEXT_KEYS so the resume middleware
restores the parallel/secondary-agent config from the paused request; the
client can't reconstruct it, and it determines the rebuilt graph.
- skill pills (the real fix this time): the round-8 onStart metadata write was
overwritten by trackUserMessage (the authoritative userMessage writer). Carry
manualSkills/alwaysAppliedSkills in the emitted `created` message and persist
them in trackUserMessage; widen UserMessageMeta + SerializableJobData.userMessage.
- execute-code files on resume: seed the paused user message's own files onto
req.body.files before initializeClient — they're excluded from the
parent-walk code-session rebuild, so an approved code/read-file tool would
otherwise resume without them.
- in-memory pending-action UI: route ApprovalEvents.ON_PENDING_ACTION in the
resume replay/pending-event loops to applyPendingActionToMessages (mirror the
live handler), so a pause that lands in the snapshot window still renders its
approval controls instead of sitting paused with no UI.
- abort isTemporary: the /chat/abort partial-save now sources isTemporary from
the job metadata, not req.body (the stop button posts only conversationId), so
aborting a paused temporary chat no longer persists an orphaned partial.
Tests: addedConvo in pickResumeContext (policy.spec.ts), file-restore on resume
(resume.spec.js), abort-from-job-isTemporary (abort.spec.js).
* fix(hitl): address Codex re-review (round 10) — resume/expiry races
Three concurrency/coherence findings, verified against the code before fixing:
- expiry-sweep CAS scope: both stale-approval sweeps (GenerationJobManager
expireStaleApprovals and the RedisJobStore requires_action cleanup) called
expire()/transitionStatus WITHOUT the observed pendingAction.actionId, so the
CAS only checked status===requires_action. Between the read and the CAS a user
could resolve the observed action and the run re-pause on a FRESH action; the
stale sweep would then abort that valid new pause. Now both pass the observed
actionId as expectActionId, so the CAS only fires for the action read as stale
(a re-paused action has a different id → no-op).
- resume graph cache: resumeCompletion cached the rebuilt graph (created with
messages:[]) via setGraph; RedisJobStore.getContentParts prefers a cached
graph over reconstructing from the chunk log, so a same-replica reload/status
poll mid-resume returned aggregatedContent missing the pre-pause content. Skip
setGraph on resume so introspection falls back to the complete chunk
reconstruction (setContentParts still seeds the in-memory store).
- pending-action UI: applyPendingActionToMessages scheduled a SINGLE
animation-frame retry then dropped the pending action; Recoil/React updates can
take several frames under load, leaving a valid requires_action run with no
approval controls. Retry across frames (bounded at 120) until the target
message commits.
Test: expire() with a mismatched expectedActionId no-ops while the matching id
expires (pendingAction.spec.ts).
* chore(deps): update @librechat/agents to version 3.2.53 and @langchain/langgraph to version 1.4.7 in package-lock.json and related package.json files
* refactor(hitl): add resolveToolApprovalPolicy seam for layered policy
Extract the single point where tool-approval policy is resolved for a turn
(`resolveToolApprovalPolicy`) and route the run call site through it instead
of reading `endpoints.agents.toolApproval` inline.
Behaviour-preserving: only the `endpoint` layer is wired today, so the result
is identical to reading the app policy directly. The `agent` and `skills`
layers are reserved seams with documented precedence (endpoint owns the
`enabled` kill switch; agent overrides mode/allow/deny/ask/reason; skills may
only tighten), so future per-agent and per-skill policy plumbing lands in one
function rather than at the `createRun` site. Adds focused unit tests.
* fix(hitl): address Codex re-review (round 11) — resume hardening
F1 (P2, security) — applyResumeContext now DELETES any RESUME_CONTEXT_KEY
absent from the persisted context, so the resume body carries exactly the
graph-determining fields the pause had. Previously only defined keys were
overwritten, leaving a client-supplied `addedConvo` (which the request
fingerprint does not cover) in place — a crafted resume could rebuild a
single-agent checkpoint as a different multi-agent graph/tool set.
F3 (P2) — the resume route ACKs (res.json) before initializeClient, so a
post-ACK getMCPRequestContext(req, res) saw the response as finished and
returned undefined, leaving the resumed run without its run-scoped MCP
connection store (approved MCP / OAuth-overlay tools then ran without their
request-scoped connections). Pre-seed the store with a null res +
cleanupOnResponse:false before the ACK and tear it down in the finally,
mirroring the normal stream path (request.js). userMCPAuthMap was already
preserved separately, so credentials were not lost — only the connection store.
Declined: the ApprovalContext NEW_CONVO guard (P2) is a false positive — the
`created` SSE event updates the conversation atom before any pause renders, so
the id is concrete by click time (details in the PR thread).
Tests: policy.spec (absent-key delete) + resume.spec (MCP context pre-seed/cleanup order).
* fix(hitl): address Codex re-review (round 12) — resume fidelity + multi-tool UI
F4 (P2) — temporal prompt vars: resume rebuilt the agent without restoring
req.conversationCreatedAt or req.body.timezone, so {{current_datetime}}-style
vars compiled a different system prompt than the paused graph (resume wall-clock,
unzoned). Add 'timezone' to RESUME_CONTEXT_KEYS (persisted at pause, replayed by
the resume middleware) and restore conversationCreatedAt from the convo before
initializeClient — mirroring the normal path's resolveConversationCreatedAt.
F5 (P2) — multi-tool approval: applyPendingActionToMessages stopped retrying once
ANY tool-call part was tagged, so siblings that rendered on later frames never got
approval controls and the resume route 400'd the partial batch. Add
countTaggedApprovalParts and keep the bounded RAF retry going until every
action_request is tagged (ask_user_question unchanged — one synthetic part).
F6 (P3) — Edit accepted `null`/`[]` (valid JSON, non-object), enabling Submit for
a value the resume route rejects via findIncompleteDecisions. Mirror the server's
plain-object check in the client (store + editIsValid) so Submit only enables for
an accepted value.
Tests: policy.spec (timezone round-trip), resume.spec (conversationCreatedAt
restore), approval.spec (countTaggedApprovalParts).
* fix(hitl): address Codex re-review (round 13) — recurse into subagent approvals
F9 (P2) — a tool paused INSIDE a subagent has its tool_call_id in the parent
subagent tool_call's nested `subagent_content`, not as a top-level message part.
applyToolApproval and countTaggedApprovalParts only scanned top-level content, so
the approval never attached and the round-12 retry loop counted 0 tagged parts and
spun to its frame cap with no controls. Both now recurse into `subagent_content`
(immutably, so React refs update): the nested call gets tagged and is counted, so
the retry terminates. Added approval.spec cases for the nested tag + count.
Note: surfacing the interactive approve/reject controls inside the subagent view is
a deliberate follow-up — ToolApproval -> useResumeSubmit -> useChatContext crashes
when rendered in the portaled subagent dialog (outside the chat/approval providers),
so that needs the controls scoped to the in-provider inline render (or the dialog
wrapped with the providers). This commit fixes the data/traversal layer only.
F7 (discovered-tool history on resume) and F8 (redis chunk TTL pause race) were
verified false positives — see the PR threads.
* fix(hitl): address Codex re-review (round 14) — resume fidelity + expiry relay
F13 (P2) — manualSkills are graph-determining (skill allowed-tools union into the
tool set before tools load) but weren't replayed, so a reload lost the skill tools
and a crafted resume could inject a different skill past the fingerprint. Add
'manualSkills' to RESUME_CONTEXT_KEYS (same replay-only pattern as timezone/
addedConvo; the delete-absent half blocks injection). Not alwaysAppliedSkills —
that's resolved server-side from the DB, not req.body.
F12 (P2) — the resume final SSE built requestMessage from job.metadata.userMessage
(persisted without files), so attachments vanished from the user bubble on resume.
Spread the already-restored req.body.files onto it, matching the normal path.
F11 (P2) — multi-replica approval expiry: RedisJobStore.cleanupRequiresActionIndex
on another replica can win the requires_action->aborted CAS (it sets the hash error
but has no event transport), and the local sweep then skips because the job is no
longer requires_action, so a client subscribed here never gets the terminal error
until the reap path. expireStaleApprovals now relays APPROVAL_EXPIRED_ERROR for a
locally-subscribed job already aborted FOR approval expiry (error-string gated,
idempotent via the errorEvent flag). emitError already publishes cross-replica.
Tests: policy.spec (manualSkills round-trip + inject-drop), resume.spec (final
requestMessage carries restored files).
* fix(hitl): render approval controls for subagent-nested tool pauses (F10)
Round-13 made applyToolApproval/countTaggedApprovalParts recurse into
subagent_content (data), but SubagentDialogPart rendered nested TOOL_CALL parts
with <ToolCall> only and never mounted <ToolApproval>, so a tool paused inside a
subagent showed no controls and the run was unresolvable.
Render <ToolApproval> in SubagentDialogPart's TOOL_CALL branch when the nested
tool_call carries an approval and isn't yet resolved, mirroring the top-level
Part.tsx render. The subagent dialog portals (OGDialog → ReactDOM.createPortal),
but React context flows through the React tree, not the DOM tree, so ToolApproval
resolves ApprovalProvider/ChatContext and the controls work + submit.
Also harden useResumeSubmit: read ChatContext via useContext (non-throwing)
instead of the throwing useChatContext wrapper, so the cards never crash when
rendered outside a ChatContext.Provider (e.g. a search/citation render that passes
chat context as a prop) — they degrade to inert (buildResumeFields returns null).
* style(hitl): re-sort run.ts imports after dev rebase
* fix(hitl): address Codex re-review (round 15) — resume content fidelity
F14 (P2) — hide_sequential_outputs was applied in chatCompletion before
saving/emitting content but not on resume, so a sequential-agent chain that
pauses for HITL and resumes persisted/emitted intermediate outputs the setting
is meant to hide. Extracted the filter into applyHideSequentialOutputsFilter()
and call it from both chatCompletion and resumeCompletion (after handleRunInterrupt,
covering the finalize + re-pause reads of client.contentParts).
F16 (P2) — on a reloaded HITL pause, the DB already holds the paused user row +
partial assistant row; useResumeOnLoad fed those as submission.messages, then
finalHandler/createdHandler appended the same pair via requestMessage/responseMessage,
duplicating the turn (buildTree doesn't dedupe children by messageId). buildSubmission-
FromResumeState now strips the paused user/response rows (by messageId, incl. the
padded/unpadded response id) from submission.messages — they're re-supplied by the
placeholders + final event. Frontend-only; live (non-reload) pause path untouched.
Deferred: F15 (collapsed-card subagent approval registration/visibility) — see thread.
Tests: client.test (filter keeps last + tool_call parts / no-op when off),
useResumeOnLoad.spec (paused pair stripped from submission.messages).
* fix(hitl): address Codex re-review (round 16) — chunk TTL, slot, job replacement
F17 (P2) — chunk-stream TTL on pause-before-chunk. CHUNK_APPEND_LUA derived its
ceiling only from the chunk key's current TTL, so when the chunks key didn't exist
at pause (fire-and-forget append in flight, or an ask-user pause before any chunk),
the on_pending_action append created the stream with only the 20m running TTL while
the approval window is 24h — content evicted before resume. The Lua now also reads
the job key (KEYS[2]); when status == requires_action it takes max(running, TTL(jobKey))
(the approval window transitionStatus set), else the running TTL. Extend-only preserved;
gated on paused status so normal runs never inflate. Both keys share {streamId} (cluster-safe).
F19 (P2) — with LIMIT_CONCURRENT_MESSAGES, the approval prompt was emitted before the
original request released its slot, so a fast Approve got /resume 429'd. handleRunInterrupt
now releases the slot (idempotent via pendingRequestReleased) right after the pause, before
the prompt; the request.js pause branch and resume.js finally only release if it didn't
(no double-release).
F20 (P2) — finalizeResumedTurn never checked the job wasn't replaced before emitDone/
completeJob/saveMessage, so a stale resume could clobber a newer turn that reused the
conversationId. Added the createdAt guard the normal request path uses (skip finalization
when the live job's createdAt != the paused job's).
Deferred: F18 (subagent_content not reconstructed on Redis resume) — joins the subagent
cluster (F15). See thread.
Tests: RedisJobStore integration (pause-before-chunk gets approval TTL; running stays short),
resume.spec (skip finalization on replacement; no double slot release on re-pause).
* 🛡️ fix: Guard HITL terminal side-effects against job replacement
Jobs are keyed by streamId == conversationId, so a new request REPLACES the
running one on the same conversation. The replaced generation's tail must not
clobber the live generation's state. Each path now re-reads the live job and
compares createdAt against the generation's captured identity before acting.
- Thread the generation's createdAt onto the client (request.js + resume.js)
as client.jobCreatedAt — the identity every guard compares against.
- handleRunInterrupt: skip approvals.pause when this run is no longer the live
job, so a stale interrupt can't flip the NEWER job to requires_action.
- chatCompletion finally: skip the checkpoint prune when replaced, so an older
run's late finally can't delete the newer run's resume checkpoint.
- resume catch-path: gate emitError/completeJob/prune behind a stillLive check
(fail-open if the read throws), mirroring finalizeResumedTurn's success guard.
- Persist the turn's uploaded files on job.metadata.userMessage (authoritative
trackUserMessage writer) and prefer them on resume over the user DB row, whose
save can still be racing a fast /resume.
Tests: 13 guard-predicate cases in jobReplacement.spec.js.
* 🔁 fix: Harden HITL resume — ownership re-check, file seeding, deferred-tool replay
Three follow-ups to the round-17 job-replacement guards (Codex review 4594099963):
- G1 (resume.js): the success-path ownership guard runs at the START of
finalizeResumedTurn, but saveMessage + first-turn title generation await long
enough for a new request to replace the job on the same conversationId. Re-read
the live job immediately before emitDone/completeJob/prune so the terminal writes
can't tear down the REPLACEMENT job — mirrors the catch-path guard.
- G2 (request.js): onStart's metadata/chunk writes that persist the turn's files
are fire-and-forget, so a fast approval could read job.metadata.userMessage before
files landed. Seed files into getPreliminaryUserMessage instead — that write is
AWAITED before the run starts, so files are durable before any interrupt can emit.
- G3 (run.ts + client.js + resume.js + IJobStore.ts): the resumed graph is rebuilt
with messages: [], so createRun's tool_search-discovery scan finds nothing. A
deferred tool discovered earlier in the turn (and targeted by the paused call) was
therefore absent from the rebuilt schema-only toolMap — resume would throw "unknown
tool" (no loadRuntimeTools fallback is wired). Capture discovered tool names at
pause via extractDiscoveredToolsFromHistory(run.getRunMessages()), persist them on
job.metadata.discoveredTools, and replay them into createRun's new discoveredToolNames
input (merged with message-extracted names, gated on hasAnyDeferredTools — inert
otherwise). A new createRun test proves the deferred tool is promoted with the replay
and absent without it (reproducing the bug).
Tests: real createRun deferred-replay suite (run-summarization.test.ts) + G1/G2/G3
guard predicates (jobReplacement.spec.js). Full suite green.
* 🔒 fix: Close HITL resume metadata + file-substitution + pause-race gaps
Four findings on the round-18 commit (Codex review 4594430222):
- H1 (P1, regression in round-18 G3): the discoveredTools captured at pause never
reached resume — three metadata allowlists dropped it: GenerationJobManager
.updateMetadata, RedisJobStore.deserializeJob, and buildJobFacade (plus the
GenerationJobMetadata type). Added discoveredTools to all four, so the deferred-tool
replay actually works end-to-end (in-memory store already kept it via Object.assign).
- H2 (P2, security): /resume honored a client-supplied `files` array, letting a crafted
client resume an approved code/read-file tool against a DIFFERENT file set than the one
approved (files aren't in the resume fingerprint/context). Resume now ALWAYS sources
files from the paused job (metadata → DB row), clearing any client-supplied set.
- H3 (P2, ephemeral fidelity): non-default model parameters (temperature, max tokens,
custom endpoint params) were lost on resume — ephemeral agents derive them from the
request body, which the resume payload omits. Capture the resolved model_parameters in
resumeContext at pause and replay them onto the body on resume (excluding `model`, which
is replayed via the fingerprinted RESUME_CONTEXT_KEYS path). Saved agents already source
these from the DB.
- H4 (P2, Redis race): a pause landing between the resume snapshot and the Pub/Sub
subscription reached neither resumeState.pendingAction nor (Redis) pendingEvents, and
approval events aren't persisted to replayEvents — the client attached to a paused job
with no approval UI. subscribeWithResume now re-reads the live job AFTER subscribing and
surfaces the pending action if the snapshot missed it (live read, no staleness).
Tests: discoveredTools metadata round-trip + subscribeWithResume re-read (pendingAction
.spec.ts); client-file substitution rejection (resume.spec.js); model-parameter replay
predicate (jobReplacement.spec.js).
* 🧹 fix: Clear stale discovered tools, release slot on claim error, extend run-step TTL
Three follow-ups on the round-19 commit (Codex review 4594783691):
- I1 (P2): the round-19 discoveredTools field wasn't cleared on Redis streamId reuse.
HSET only overwrites listed fields and handleRunInterrupt only writes discoveredTools
when THIS turn discovers a deferred tool — so a replacement turn that pauses without its
own discovery inherited the prior run's tool names and force-loaded undiscovered deferred
tools on resume. Added discoveredTools to createJob's staleHitlFields HDEL list (the
in-memory store already builds a fresh object, so it was Redis-only).
- I2 (P2): with LIMIT_CONCURRENT_MESSAGES, approvals.resolve runs after the slot increment
but before the run's try/finally, so a store/Redis error there leaked the slot until the
counter TTL expired (spurious 429s on retry of the still-paused approval). Wrapped the
claim in try/catch that decrements the slot and returns 500.
- I3 (P3): saveRunSteps did SET ... EX running unconditionally, resetting the run-steps key
to the 20-min running TTL even while the job is paused for the longer approval window —
a reload after that window lost the tool timeline. Now uses a paused-window TTL script
mirroring the chunk-stream no-shrink behavior (extends to the approval window when the
job hash is requires_action).
Also fixes a latent strict-tsc cast error in the round-19 pendingAction test.
Tests: claim-throws-releases-slot (resume.spec.js); discoveredTools cleared on reuse +
saveRunSteps preserves the paused TTL (RedisJobStore integration, USE_REDIS).
* 🛡️ fix: Guard fast-resume save race, gate HITL to resumable routes, expire on stale submit
Three findings on the round-20 commit (Codex review 4595045652):
- J2 (P1): a fast /resume can claim + finalize the COMPLETED response while the original
request's pause branch is still awaiting `response.databasePromise`; the later
unfinished-save then overwrites the completed content. Re-check the job is still paused on
THIS generation's action (a claim leaves requires_action; a replacement bumps createdAt)
before marking the row unfinished; fail open on a read error.
- J3 (P1): the tool-approval wiring (humanInTheLoop + PreToolUse hook + checkpointer) was
applied to EVERY createRun caller when toolApproval.enabled, but the OpenAI-compatible and
Responses controllers never inspect run.getInterrupt() or persist a pending action — an
approval-gated tool would pause there with no approval surface or resume endpoint and the
route would emit a normal final response / [DONE] with the tool call dangling. Gate the
wiring on a new createRun `hitlCapable` flag, set only by AgentClient (chat + resume).
- J4 (P2): a stale-action 409 on submit returned without driving expiry, leaving the job
requires_action with a dead action until the periodic sweeper ran — any attached SSE client
got no terminal event and the stream appeared to hang. Extracted GenerationJobManager
.expireApproval(streamId, actionId) (expire CAS + terminal SSE, shared with the sweeper) and
call it from the resume route when the observed action is stale.
J1 (nested subagent approval controls not mounting while the details dialog is closed) is a
valid frontend issue in the deferred subagent-HITL path — tracked separately (replied on the
thread) since the fix touches the shared dialog primitive and needs UI verification.
Tests: HITL-gate both directions (run-summarization.test.ts); expire-on-stale-submit
(resume.spec.js); fast-resume unfinished-save guard predicate (jobReplacement.spec.js).
* 💄 style: Wrap captureAgents signature to satisfy prettier (CI lint)
Skill files were primed into the sandbox at `/mnt/data/{skillName}/...`,
but the read_file/create_file/edit_file tool descriptions and the
read_file bash-fallback hints all assume the `skills/{skillName}/...`
namespace (sandbox cwd is `/mnt/data`). Agents therefore reached for
`./skills/my-skill/...` in bash and missed ~100% of the time.
- Add shared `SKILL_FILE_PREFIX` to agents/skills.ts (moved out of
handlers.ts; single source of truth across the three layers).
- Prefix the prime upload filenames and session names with `skills/` in
skillFiles.ts so the physical mount matches the model-facing namespace;
recover the bare relativePath by stripping `skills/{name}/`.
- Canonicalize the read_file bash-fallback hints to
`/mnt/data/skills/{skillName}/{relativePath}` so the implicit
`{name}/...` addressing form is corrected too.
Closes#13961
Opening the agent editor fetched the full `versions` array (each a complete
config snapshot) alongside the agent, so agents with large histories were slow
to open. Version history is now loaded only when the user opens it.
- Add `getAgentWithVersionCount` (aggregation: version count, no versions array)
and `getAgentVersions` data-schemas methods.
- `getAgentHandler` returns the version count without the heavy array; add
`GET /agents/:id/versions` (EDIT-gated) for lazy retrieval.
- Add `useGetAgentVersionsQuery`; VersionPanel reads current config from the
cached expanded query and fetches versions on open. Revert keeps the expanded
cache and versions query in sync.
The getSharedLink query sorts by updatedAt, but the sharedlinks
collection had no updatedAt index. Azure Cosmos DB for MongoDB
(RU-based) rejects sorts on non-indexed fields, causing an immediate
500 on GET /api/share/link/:conversationId whenever a conversation is
opened. Standard MongoDB is unaffected.
The message-edit route recomputed a user message's tokenCount from the edited
`text` alone, ignoring its persisted `quotes`. But the send path re-prepends
those quotes into the prompt on every turn (mergeQuotedText), so after editing a
quoted message the stored tokenCount under-reported by the whole quote block,
skewing the context gauge and any other tokenCount consumer.
The full-recount path now fetches the message's quotes and counts the merged
text+quotes via a new `mergeQuotedTextForCount` helper in packages/api (mirrors
the send path), so the stored count stays authoritative. The incremental
content-part path is left as-is: it deltas only the edited part and preserves the
rest of the count (incl. the quote contribution), and applies to content-array
messages rather than text+quotes user turns.
Deferred follow-up from #13953.
* ♻️ refactor: Compute Context Gauge Client-Side, Drop Projection Endpoint
The /api/endpoints/context-projection endpoint re-fetched a conversation's
messages from Mongo and re-tokenized them to project the context gauge for
snapshot-less branches. The browser already holds those messages and their
per-message tokenCounts, so this duplicated work on the request path (an
unbounded read + server-side BPE tokenization until it was later capped).
Move the snapshot-less estimate fully client-side, from the in-memory index:
- sumBranch accumulates an uncalibrated char/4 estimate (estTokens) for
count-less messages (imports / pre-feature) under the same summary cutoff
- useTokenUsage folds estTokens (calibrated via the existing calibrationFamily
ratio) into the existing fallback; known per-message counts render unchanged
- delete the endpoint, controller, rate limiter, route, the getMessageTextStats
data-schemas method, and the data-provider surface (endpoint/key/type/service/query)
No DB read, no server tokenization, no rate-limit knobs; the gauge recomputes
reactively from the index. Net -793 lines.
* 🩹 fix: Count quotes and object-form content in client context estimate
Address Codex review on the client-side context estimate:
- messageChars now reads object-form content text (part.text.value), not
only string text/think, so imported / pre-feature messages whose body
lives in content parts are no longer estimated as zero.
- Count-less user messages include their merged quote excerpts in the
estimate, mirroring what the send path prepends into the prompt.
* 🩹 fix: Cap over-window estimate and surface estimated tokens in breakdown
Address remaining Codex review on the client-side context estimate:
- Clamp the snapshot-less estimate's displayed usedTokens to maxTokens. The
send path prunes an over-window branch before calling the model, so the
gauge never actually exceeds the window; this avoids impossible values
(e.g. 50k / 8k) without re-introducing client-side pruning.
- Surface the calibrated count-less estimate as its own "Estimated" row in
the breakdown popover, so a branch of only count-less imported / pre-feature
messages is no longer shown as Input 0 / Output 0 under a non-zero header.
* 🩹 fix: Refine client context estimate per Codex re-review
- Drop calibration from the snapshot-less estimate. The removed projection
never actually calibrated (the client never sent a ratio), and a ratio
inflated by provider-injected context over-estimates visible imported text.
- Exclude reasoning (think) / error parts from the estimate; the send path
strips them, so they are not part of the next call's context.
- Fold quote text into the estimate even when a tokenCount is present, since
the edit route recounts tokenCount from text only and drops the merged quote.
* 🩹 fix: Recount quoted user turns instead of topping up the stored count
The previous round added quote chars on top of a quoted message's stored
tokenCount, which double-counts the common (unedited) case where the count
already includes the merged quote prompt. Match the removed projection
instead: for quoted user turns, ignore the stored count and estimate the
full merged text. This both avoids the double-count and still corrects the
stale text-only count an edit leaves behind.
* 🩹 fix: Trust stored counts for quoted turns; count tool-call parts
- Quoted user turns: revert to trusting a present tokenCount. The send path's
stored count already includes the merged quote (and any calibration), and
the client's char/4 path is coarser, so recounting regressed normal turns.
Only count-less messages estimate quotes from text.
- Count tool-call name/args/output for count-less assistant messages; the
formatter sends them back as context, so omitting them under-reported
imported branches with tool history.
* 🩹 fix: Exclude in-flight tail from estimate to avoid resume double-count
On resume the live path seeds liveTokens from the partial response and also
writes that content into the messages cache, where the count-less response
is estimated into estTokens too — double-counting the in-flight output on the
snapshot-less estimate path. sumBranch now exposes the tail message's own
estimate (tailEstTokens); the estimate path drops it while a stream is live,
so the in-flight response is counted once (via liveTokens). The breakdown's
Estimated row uses the same in-flight-adjusted value.
* 🩹 fix: Recount quoted user turns in context estimate (match send path)
A quoted user turn's stored tokenCount is unreliable for the gauge: a
text-only Save edit recomputes it from text alone, and the send path
(needsCanonicalTokenCount in agents/client.js) recounts the quote-merged
prompt every turn regardless of the stored value. Mirror that on the client
— estimate quoted turns from the merged text+quotes and ignore the stored
count — so snapshot-less branches don't under-report by the quote block.
Reverts the earlier "trust the count" assumption, which the server disproves.
* 🧹 chore: Route useResumableSSE diagnostics through the frontend logger
Convert the [ResumableSSE]/[Debug] console.log and console.error diagnostics
to the gated frontend `logger` (client/src/utils/logger), splitting the tag
from the message so object arguments are passed through as real args (logged
expandably, not stringified) and the logs stay tag-filterable and off the
production console unless explicitly enabled. All log statements preserved;
nothing removed.
* 🩹 fix: Prefer content over text when estimating count-less messages
A stopped agent response is saved with both a `text` field and a structured
`content` array, and the send path formats from content. messageChars
early-returned on `text`, dropping the content array (and the tool-call tokens
it carries) from the snapshot-less estimate — also making the tool_call
handling dead for such messages. Prefer content when present, fall back to text.
* 🔑 fix: Honor User-Provided MCP API Key Instead of Forcing OAuth
OAuth auto-detection probes the server without credentials and treats a
`WWW-Authenticate: Bearer` 401 as an OAuth requirement. A static bearer
API-key server answers an unauthenticated probe with the same challenge,
so servers configured with "API Key / each user provides their own / Bearer"
were misclassified as `requiresOAuth: true` and connected via the OAuth path,
ignoring the user's saved key (status stuck yellow, tool calls demand OAuth).
The API-key exemption in detection was scoped to `source === 'admin'` only.
Broaden it to any `apiKey` config in both detection sites (inspector startup
detection and runtime placeholder-URL detection), since API-key and OAuth auth
are mutually exclusive in the schema.
* 🔒 fix: Skip inspection probe for user API keys; honor explicit OAuth
Addresses two Codex findings on the API-key OAuth-detection fix:
- Skip the capability probe during inspection when apiKey.source is 'user'.
The user's key is supplied per-user at connect time, so an unauthenticated
probe at create/update would 401 against a bearer server and fail the save
(servers are inspected on the raw, pre-transform config with no auth header).
Same treatment already applied to customUserVars/obo/OAuth servers.
- Only short-circuit detection to non-OAuth when no explicit 'oauth' block is
configured, so an explicit OAuth config takes precedence if both are set.
Applied to both detection sites for consistency.
DataTable.spec failed with "Too many re-renders" (35 tests). Root cause: @tanstack/react-virtual is measurement-driven, and jsdom has no real layout, so its re-render loop never converges. This went unnoticed because packages/client had no jest CI job (only the client workspace runs jest in frontend-review.yml).
- DataTable: only read the virtualizer (getVirtualItems/getTotalSize) when virtualization is active; the non-virtualized branch renders rows directly, so engaging it for small tables was wasted render-phase work.
- Spec: mock @tanstack/react-virtual, since jsdom can't exercise real virtualization layout.
- Add a test:ci script to @librechat/client and a Tests: @librechat/client CI job so packages/client specs run on every frontend PR.
* ⬆️ chore: Migrate off deprecated @ariakit/react-core to @ariakit/react-components
@ariakit/react-core and its dependency @ariakit/core are deprecated (split into successor packages) and emit install-time warnings. @ariakit/react already ships the non-deprecated @ariakit/react-components transitively; the only direct use of react-core was the SelectRenderer deep import in ControlCombobox, which is now sourced from @ariakit/react-components/select/select-renderer (identical symbol and subpath). Both deprecated packages drop out of the lockfile and react-components dedupes to the single version @ariakit/react pins.
* ✅ test: Resolve ESM-only @ariakit split packages in jest
@ariakit/react-components and its peers are ESM-only (type: module) and declare only an import export condition, so jest's CJS resolver can't load them when @librechat/client's CJS build requires SelectRenderer. Add a custom jest resolver that resolves these @ariakit/* split packages with the import condition, and extend transformIgnorePatterns so babel transpiles them to CJS. Applied to both the client and packages/client jest configs.
