mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-07-01 20:01:35 +00:00
6dbf9d5ad3
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* 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)
|
||
|---|---|---|
| .. | ||
| controllers | ||
| middleware | ||
| routes | ||
| services | ||
| utils | ||
| cleanup.js | ||
| experimental.js | ||
| index.js | ||
| index.metrics.spec.js | ||
| index.spec.js | ||
| socialLogins.js | ||
| socialLogins.spec.js | ||
| telemetry.js | ||
| telemetry.spec.js | ||