Skip to content

🪝 feat: Human-in-the-Loop Runtime - Tool Approval + Ask-User-Question (Slice B) - #13942

Merged
danny-avila merged 35 commits into
devfrom
feat/hitl-tool-approval
Jun 29, 2026
Merged

danny-avila merged 35 commits into
devfrom
feat/hitl-tool-approval

Conversation

@danny-avila

@danny-avila danny-avila commented Jun 24, 2026 •

Copy link
Copy Markdown
Collaborator

🪝 feat: Human-in-the-Loop Runtime — Tool Approval and Ask-User-Question (Slice B)

Builds the HITL runtime on top of the Slice A scaffolding (#12938) so an agent can pause mid-run, hand control to the human, and resume from exactly where it left off — end to end, across workers and restarts.

This PR ships two distinct interrupt types that share one durable pause/resume engine:

Tool approval Ask-user-question
Trigger The agent is about to call a tool that policy says needs review The agent invokes the AskUserQuestion tool to get clarification
It is a… permission gate clarification channel
User does approve · reject · edit args · respond (substitute a result) answers free-form text, or picks a curated option
Resume payload { decisions: [{ tool_call_id, decision, reason?, editedArguments?, responseText? }] } { answer: string }
SDK payload type tool_approval ask_user_question

Both flow through the same PendingAction envelope, the same POST /agents/chat/resume route, the same durable checkpoint, and the same SSE stream — they differ only in the decision payload and the card UI. Feature-complete; off by default (opt-in via endpoints.agents.toolApproval.enabled).


How it works (shared lifecycle)

run hits an interrupt (tool needs review │ agent asks a question)
        │
        ▼
pause job (requires_action) + suspend LangGraph state to a durable checkpoint
        │  emit `on_pending_action` over SSE  (+ a TTL'd Redis pointer for reconnect)
        ▼
user decides/answers in the card ──▶ POST /agents/chat/resume
        │
        ▼
atomic single-winner resolve ─▶ REBUILD the run from the checkpoint ─▶ run.resume(value)
        │
        ▼
continuation streams over the SAME SSE ─▶ finalize (save message, complete job, prune checkpoint)
  • One durable checkpoint (Mongo) is the source of truth; the Redis pendingAction is just a TTL'd UX pointer. thread_id == conversationId, so the checkpoint is pruned on every non-paused completion (otherwise turn N+1 would rehydrate turn N's interrupt). A Mongo TTL index is the backstop.
  • Resume-by-reconstruction: the original request's background generation is free to exit when the run pauses; resume rebuilds a fresh Run from the checkpoint, so any replica can resume and paused runs survive a restart.
  • Race-safe: two clicks / two tabs racing to resume can't both drive the run — ApprovalLifecycle.resolve (atomic compare-and-set from Slice A) admits a single winner; the loser gets 409. Resume also re-acquires a concurrency slot (the paused turn released its slot), so paused-then-resumed turns can't bypass LIMIT_CONCURRENT_MESSAGES.

Configuration

Everything is off until you opt in. Minimal zero-config enable (checkpoints persist to the app's MongoDB automatically):

endpoints:
  agents:
    toolApproval:
      enabled: true          # master switch (admin-level kill switch when false)

Full surface:

endpoints:
  agents:
    toolApproval:
      enabled: true
      mode: default          # default | dontAsk | bypass  (user-facing "stop asking me" = bypass)
      ask:   ['mcp:*', 'execute_code']   # globs that require approval
      deny:  ['delete_*']                # globs that are always blocked (wins even under bypass)
      allow: ['search_*']                # globs that auto-approve
      reason: 'Review {tool} before it runs'   # optional prompt template ({tool} interpolated)
    checkpointer:
      type: mongo            # mongo (default, durable, cross-replica) | memory (dev/single-process)
      ttl: 86400             # approval window in seconds (drives Mongo TTL + pending-action expiry in lockstep)
  • Policy evaluation order is the SDK's: deny → bypass → allow → ask → dontAsk → fallthrough(ask). List entries are globs; mcp:server:* scopes a rule to one MCP server. Subagents inherit the parent's mode.
  • enabled: false is the admin kill switch (no checkpointer, no hooks, no prompts). Users toggle prompting with mode: bypass, not by disabling.

Backend

  • Config (data-provider): endpoints.agents.toolApproval (policy) and endpoints.agents.checkpointer (type, ttl, optional collection names).
  • agents/checkpointer.ts — MongoDBSaver over the app's mongoose connection (seam; undefined ⇒ SDK MemorySaver fallback for memory/not-connected), memoized; deleteAgentCheckpoint for terminal pruning.
  • agents/hitl/runtime.ts — buildHITLRunWiring (PreToolUse policy hook + humanInTheLoop), spread into createRun's config only when enabled (inert otherwise).
  • agents/hitl/policy.ts — maps the YAML policy → SDK ToolPolicyConfig.
  • agents/hitl/resume.ts — wire→SDK decision mapping for both types (mapToolApprovalResolutions, mapAskUserAnswer), plus fail-closed validation (findUndecidedToolCalls, findDisallowedDecisions).
  • AgentClient.handleRunInterrupt (pause + emit on_pending_action) and resumeCompletion (rebuild + run.resume, carrying userMCPAuthMap + initialSessions so approved MCP/code/file tools keep their creds and file context). request.js skips finalize on pause and re-marks the paused response unfinished.
  • POST /agents/chat/resume (controllers/agents/resume.js) — reuses chat.js middleware; authorizes (owner / tenant / exact agent_id + endpoint / requires_action / live actionId), validates the decision against the pending payload, atomically resolves (single winner), then drives the rebuilt run and finalizes (merging multi-segment tool artifacts, filtering malformed parts, reconciling cumulative usage).
  • GenerationJobManager.expireStaleApprovals — emits a terminal SSE event when an approval window lapses with a client attached; abort and expiry both prune the checkpoint.
  • Moderation: when OPENAI_MODERATION is on, every user-authored resume field is moderated — the ask-user answer, a respond substitute result, a reject reason, and edited tool arguments.

Frontend

  • Tool-approval card — approve / reject / edit (JSON args) / respond controls in the tool card (mirrors the OAuth-button precedent), batch-aware (one submit covers every paused call in the batch), gated on each call having a decision.
  • Ask-user-question card (AskUserQuestion.tsx) — renders the question (+ optional description), curated option buttons when the agent supplied options, and a free-form textarea; either path resumes the run.
  • applyPendingAction dispatches on interrupt type to map the pause onto the response message; live on_pending_action and reconnect (resumeState.pendingAction) both resolve to the in-flight assistant message (retrying until its placeholder exists, never attaching to a prior reply). Submit POSTs to /resume; the continuation arrives on the existing SSE.
  • ApprovalProvider is pure state; the context-dependent submit lives in a useResumeSubmit hook used only by the cards (rendered in chat view).

Safety & correctness hardening

Driven to convergence over several adversarial-review rounds:

  • Fail-closed decision validation — a crafted resume can't approve a tool the policy restricted (e.g. to reject/respond); unknown decisions reject rather than approve.
  • Exact agent_id + endpoint match on resume — a resume that omits or swaps either is rejected, so a claimed checkpoint can't be rebuilt on a different graph.
  • Single-winner resolve — atomic CAS; racing submits get 409, never a double-driven run / double bill.
  • Cross-turn checkpoint isolation — prune on completion, on abort, and before a fresh turn; TTL backstop.
  • Extend-only Redis chunk TTL — the post-pause on_pending_action append can no longer reset the chunk-stream TTL back to the running window and evict pre-pause content before resume.
  • Multi-segment artifact accumulation — tool artifacts produced across successive pause segments are merged onto the saved message instead of being overwritten by the last segment.
  • Malformed-part filtering on resumed save, matching the normal agents path.

Tests

  • Unit (packages/api): decision mapping + fail-closed validation (hitl/resume.spec.ts), policy mapping (hitl/policy.spec.ts), HITL run wiring (hitl/runtime.spec.ts), checkpointer config/selection (checkpointer.spec.ts).
  • Route-level integration (controllers/agents/__tests__/resume.spec.js, 36 cases): the full guard ladder (auth / tenant / agent+endpoint / actionId / staleness / concurrency / single-winner) and the pause→resume→finalize lifecycle for both interrupt types, plus re-pause, abort-during-resume, resume-failure, artifact accumulation, malformed-part filtering, and title generation.
  • Frontend util (client/src/utils/approval.spec.ts, 16 cases): tool-call join by id, skip-completed, idempotent ask-question replay, and assistant-only message resolution.
  • Redis integration (RedisJobStore.stream_integration.spec.ts): extend-only chunk TTL across a pause.

Notes / follow-ups

  • Default stays off until the broader rollout; the flip is a one-line config change.
  • Tools that pause should be idempotent — LangGraph re-runs the interrupted node's batch on resume (SDK contract).
  • Resume sources agent fields from the active conversation; HITL inside a parallel added-conversation pane could rebuild with the main pane's agent (resume still targets the right job by conversationId).

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2be08a6da0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if (undecided.length > 0) {
return { status: 400, error: 'Every paused tool call must be decided', undecided };
}
return { resumeValue: mapToolApprovalResolutions(resolutions) };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Enforce allowed approval decisions before resuming

For a tool_approval pending action, this path only verifies that every requested tool_call_id has some resolution; it never checks the submitted decision against the corresponding review_configs[].allowed_decisions. When a HITL policy intentionally limits a tool to, for example, reject/respond, a crafted POST to /api/agents/chat/resume can still send { decision: 'approve' } and it is passed straight to the SDK, bypassing the approval policy. Please validate each resolution against the pending payload before mapping it.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5abd66f. Added findDisallowedDecisions(payload, resolutions) and the resume route now 403s a decision that isn't in the matching review_configs[].allowed_decisions (fails closed for a tool_call_id with no review config). Unit-tested in resume.spec.ts.

Comment on lines +304 to +305
approval={toolCall.approval}
toolCallId={toolCall.id}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Render approvals for specialized tool cards

With toolApproval.enabled and a policy/default that asks for all tools, only this generic ToolCall branch receives approval props. Special-cased branches above it, such as web_search, bash_tool, file_search/retrieval, handoff, and image-gen render their own components and never render ToolApproval, so a paused job for those tools remains requires_action with no approve/reject controls in the UI. Render the approval controls before these early returns or thread the metadata through each specialized component.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d7402db. Part now wraps the whole tool-call branch and renders ToolApproval for ANY paused agent tool (bash/code/file/handoff/image-gen/etc.), not just the generic card; moved the rendering out of ToolCall so it's uniform.

Comment thread client/src/utils/approval.ts Outdated
Comment on lines +172 to +175
const unpadded = responseMessageId.replace(/_+$/, '');
const unpaddedIdx = messages.findIndex((message) => message.messageId === unpadded);
if (unpaddedIdx >= 0) {
return unpaddedIdx;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid matching pending actions to the user message

In the live pause-before-render case, responseMessageId is normally the user message id with a trailing underscore, so stripping underscores can match the just-created user message before the assistant placeholder exists. For ask_user_question this immediately appends the answer UI to the user message and never returns -1 for the retry path, leaving the actual paused assistant turn without the prompt. Please only fall back to an assistant/child response message, or return -1 until the response placeholder is present.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d7402db. findPendingActionMessageIndex now only ever matches an assistant message (isCreatedByUser === false); removed the underscore-strip fallback that could resolve to the user bubble, so it returns -1 until the assistant placeholder exists and the rAF retry fires.

* @access Private
* @returns {void}
*/
router.post('/resume', resumeController);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip moderation for resume payloads without text

When OPENAI_MODERATION is enabled, this new route still runs through the router-level moderateText middleware above it; that middleware reads only req.body.text and posts it to moderation, but resume bodies contain decisions or answer instead. In moderation-enabled deployments, approve/reject/edit submissions have no text, so the middleware can deny the request before ResumeController runs. Mount /resume outside that middleware or have the middleware skip/handle resume payloads explicitly.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5abd66f. moderateText now returns next() when there's nothing to moderate (a tool-approval resume carries decisions, no text), and moderates the ask-user answer as user text — instead of posting an empty input and denying the request.

if (!job || job.status !== 'requires_action' || !isPendingActionExpired(job)) {
continue;
}
const expired = await this._approvals.expire(streamId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prune checkpoints when approvals expire

When a pending action reaches expiresAt, this aborts the job but leaves the durable LangGraph checkpoint for Mongo TTL cleanup. TTL deletion is not immediate, so /chat/status can report the job inactive while the stale checkpoint is still present; if the user starts a new turn in the same conversation during that window, createRun reuses the same thread_id (conversationId) and can rehydrate the expired interrupt instead of starting fresh. The same terminal cleanup is needed for user-aborted paused jobs, which also delete the job without deleting the checkpoint.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5abd66f. The abort route now prunes the checkpoint for a run aborted while paused, and a fresh HITL turn prunes any orphan before processStream — so a new turn in the same conversation can't rehydrate an expired/aborted interrupt before the Mongo TTL reclaims it. (Kept out of GenerationJobManager to avoid a stream→agents dependency.)

// Awaited (not fire-and-forget) so the job's terminal write lands before the
// checkpoint prune, and so a failure here doesn't race the controller's error path.
try {
await GenerationJobManager.completeJob(streamId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Generate deferred titles before completing jobs

For a first-turn pause, the deferred title is generated only after completeJob runs. completeJob aborts/removes the stream runtime, and emitChunk returns when the runtime is missing or aborted, so the onTitleGenerated title event below cannot reach the live client; the final event already sent above still contains New Chat. Generate the title before completing the stream, or include the generated title in the final event.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5abd66f. The deferred first-turn title is now generated BEFORE emitDone/completeJob, so its title event still reaches the live client and the final event carries the real title instead of "New Chat".

Comment on lines +237 to +245
return {
conversationId,
endpoint: conversation?.endpoint,
endpointType: conversation?.endpointType,
agent_id: conversation?.agent_id,
model: conversation?.model,
spec: conversation?.spec,
ephemeralAgent: getEphemeralAgent(conversationId),
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve temporary-chat state on resume

Temporary chats depend on isTemporary being sent with the request, and the resume controller later passes only req.body.isTemporary to saveMessage. This resume payload omits that flag even though ResumeAgentFields supports it, so if a temporary chat pauses for approval, the resumed response is saved as a normal persisted message. Include the temporary flag from chat state or persist it on the paused job metadata.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5abd66f. isTemporary is now persisted on the job at creation and read from job metadata on resume (authoritative — not trusting the resume body), so a temporary chat that pauses stays temporary.

Comment on lines +229 to +233
const result = await initializeClient({
req,
res,
endpointOption: req.body.endpointOption,
signal: job.abortController.signal,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore the original parent before client initialization

The resume POST body does not include the original parentMessageId, so initializeClient runs with req.body.parentMessageId unset; setting client.parentMessageId afterward is too late for initializeAgent, which uses the request parent while loading/scoping thread files and code artifacts. In a paused turn that resumes a tool needing prior thread resources, the rebuilt graph can get the wrong file scope. Seed the original parent from job.metadata.userMessage.parentMessageId before calling initializeClient.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5abd66f. The original thread parent is now seeded from job.metadata.userMessage.parentMessageId into req.body.parentMessageId BEFORE initializeClient, so initializeAgent scopes thread files / code artifacts off the correct parent.

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d7402db00c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread api/server/controllers/agents/resume.js Outdated
// title instead of "New Chat". Best-effort — a failure must not fail the resumed turn.
if (
addTitle &&
parentMessageId === Constants.NO_PARENT &&

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use the original parent when deciding title generation

For a valid resumed turn parentMessageId was set above to the user message id so the response can be saved under the user message; that means this first-turn check can never be true when userMessage?.text exists. Any new conversation that pauses for HITL before completion will therefore skip addTitle and finish as "New Chat". This should check the original user message parent (for example userMessage.parentMessageId === Constants.NO_PARENT) rather than the response's parent id.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 25cacf6. Title eligibility now uses the user message's own parent (isFirstTurn = userMessage.parentMessageId === NO_PARENT) instead of the response's parent, which was always set to the user message id and so never matched.

Comment thread api/server/controllers/agents/resume.js Outdated
if (meta.agent_id ?? req.body?.agent_id) {
responseMessage.agent_id = meta.agent_id ?? req.body.agent_id;
}
const responseMetadata = jobData ? buildAbortedResponseMetadata(jobData) : null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve completion metadata on resumed saves

When a paused run resumes and completes normally, saving metadata through buildAbortedResponseMetadata drops fields that the normal completion path persists via client.buildResponseMetadata(), notably contextUsage and any collected thoughtSignatures. For HITL-resumed messages this makes reloads lose the context breakdown and can break providers that need persisted thought signatures on subsequent turns; the resume save path should merge/use the normal completion metadata instead of the abort-only helper.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 25cacf6. The resumed message now uses client.buildResponseMetadata() (falling back to the abort helper) so contextUsage / thoughtSignatures are preserved on resumed completions.

Comment on lines +28 to +29
} else if (typeof req.body.answer === 'string') {
safeText = req.body.answer;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Moderate tool-approval response text

When OPENAI_MODERATION is enabled, this only adds text or ask-user answer to the moderation inputs, but a tool-approval resume can include user-authored decisions[].responseText from the new Respond action. Because tool-approval requests have no text/answer, the middleware falls through to the empty-input next() path and sends that substitute result to the agent without the configured moderation check.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 25cacf6. moderateText now also feeds decisions[].responseText (the respond action's user-authored substitute result) into the moderation inputs.

Comment on lines +297 to +298
const agentsCfg = req.config?.endpoints?.agents;
if (isHITLEnabled(agentsCfg?.toolApproval)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Load config before pruning aborted checkpoints

The /chat/abort route is declared before chatRouter.use(configMiddleware), so in this context req.config is normally unset; as a result agentsCfg is undefined and isHITLEnabled(...) is always false. Aborting a paused HITL run therefore never calls deleteAgentCheckpoint, leaving the durable LangGraph checkpoint around until TTL instead of pruning it on abort as intended.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 25cacf6. /chat/abort now runs configMiddleware so req.config is set and the HITL checkpoint prune on abort actually executes. (The prune-before-fresh-run guard already covered the rehydration risk, but this restores the prompt cleanup.)

Comment thread api/server/controllers/agents/resume.js Outdated
Comment on lines +258 to +262
if (client.contentParts) {
GenerationJobManager.setContentParts(streamId, client.contentParts);
}

const resumeState = await GenerationJobManager.getResumeState(streamId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Seed content before replacing in-memory state

In the in-memory job store, setContentParts replaces the stored pre-pause content array for this stream. Calling it before getResumeState() means same-process resumes read back the brand-new empty client array, so seedContent is empty and the finalized resumed message loses everything streamed before the approval prompt. Read the resume state before swapping the content reference, or seed from the old state first.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 25cacf6. The resume controller now reads getResumeState() (seedContent) BEFORE setContentParts(), so the in-memory store's content swap can't blank the pre-pause seed.

Comment thread api/server/controllers/agents/resume.js Outdated
// Atomically claim the resume. The single winner drives the run; a racing second
// submit (double-click, two tabs) gets false and must not re-drive — that would
// re-execute tools and double-bill.
const claimed = await GenerationJobManager.approvals.resolve(streamId, pendingAction.actionId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Enforce concurrency limits when resuming

The original request decrements the pending-request counter when it pauses, but this resume path claims the action and starts resumeCompletion without calling the same checkAndIncrementPendingRequest/decrement flow used by normal generations. With LIMIT_CONCURRENT_MESSAGES enabled, a user can pause several runs one at a time and then resume them all concurrently, bypassing the configured generation limit.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 25cacf6. The resume path now calls checkAndIncrementPendingRequest (429 when over) and decrements in finally, so paused-then-resumed turns count against LIMIT_CONCURRENT_MESSAGES.

Comment thread api/server/controllers/agents/resume.js Outdated
if (job.status !== 'requires_action' || isPendingActionStale({ pendingAction })) {
return res.status(409).json({ error: 'No live pending action to resume' });
}
if (actionId && pendingAction.actionId !== actionId) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require actionId on resume decisions

Because this mismatch check only runs when actionId is present, a POST that omits it is accepted for whichever pending action is current. For ask_user_question, any non-empty answer then resolves the latest prompt, so a stale or malformed client can answer a different question instead of getting the intended stale-action 409; the route should require the action id the UI already sends.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 25cacf6. actionId is now required (400 when absent); a body without it can no longer resolve whatever action is current.

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 25cacf6c25

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

endpointOption: req.body.endpointOption,
signal: job.abortController.signal,
});
client = result.client;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve MCP auth when resuming tools

When resuming a paused MCP tool approval, this drops result.userMCPAuthMap; the normal path passes that map into sendMessage, and AgentClient.chatCompletion then sets config.configurable.userMCPAuthMap before tool execution. The fresh run created for resume therefore executes approved MCP tools without the user's stored credentials, so OAuth/user-authenticated MCP calls fail after approval even though they would work before the pause.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2efd9d0. The controller passes result.userMCPAuthMap into resumeCompletion, which sets config.configurable.userMCPAuthMap before run.resume, so an approved MCP tool runs with the user's stored credentials.

Comment on lines +1764 to +1769
run = await createRun({
agents,
// State (messages, tool calls) is rehydrated from the checkpoint by
// run.resume; createRun only needs the agents to rebuild the graph.
messages: [],
runId: this.responseMessageId,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Seed tool sessions on rebuilt resumes

This fresh createRun omits the initialSessions that the normal path builds with buildInitialToolSessions before creating the graph. For a paused approval on code/file/skill-backed tools, especially after a restart or another replica handles the resume, the rebuilt graph has no Graph.sessions entries for the uploaded or primed code-env files, so the approved tool runs without the file/session context that was present before the interrupt.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2efd9d0. resumeCompletion now builds initialSessions via buildInitialToolSessions({ agents }) and passes it to createRun, so the rebuilt graph has the code-env/skill file sessions the pre-pause run had.

? liveContent
: ((await GenerationJobManager.getResumeState(streamId))?.aggregatedContent ?? []);

const responseMessage = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include resumed tool attachments before saving

When the approved continuation creates tool artifacts such as code files, images, or UI resources, resumeCompletion pushes promises into client.artifactPromises, but this manual finalization never awaits them the way BaseClient.sendMessage does before saving. Those artifacts may stream live, but they are omitted from the persisted response and final SSE payload, so files generated after a HITL pause disappear after refresh or for late subscribers.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2efd9d0. finalizeResumedTurn now awaits client.artifactPromises and sets responseMessage.attachments = (await Promise.all(...)).filter(Boolean) (mirroring BaseClient.sendMessage) before saving + emitting the final event.

Comment thread api/server/controllers/agents/resume.js Outdated
// resumed turn finished normally, so the abort-only helper would drop those fields.
// Fall back to the job's tracked usage if the client metadata isn't available.
const responseMetadata =
client?.buildResponseMetadata?.() ?? (jobData ? buildAbortedResponseMetadata(jobData) : null);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve pre-pause usage metadata

When a paused run resumes with additional model/tool-loop calls, client.buildResponseMetadata() only sees the fresh resume client's usage/context sinks, while jobData.tokenUsage already contains the cumulative pre- and post-pause events. Because this ?? prefers the partial live metadata whenever it is non-empty, the final save overwrites the response metadata with only the post-resume usage/context, underreporting cost and usage after approval.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — a regression from the round-2 metadata change. Fixed in 2efd9d0: cumulative usage (+ summaryUsedTokens) now comes from buildAbortedResponseMetadata(jobData), merged with contextUsage/thoughtSignatures from client.buildResponseMetadata(); cumulative usage wins.

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2efd9d02ea

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread api/server/controllers/agents/resume.js Outdated
// Agent B's graph. The conversation's agent is stable, so a correct client always
// sends the right one — reject a mismatch rather than silently swapping agents.
const originalAgentId = job.metadata?.agent_id;
if (originalAgentId && req.body.agent_id && req.body.agent_id !== originalAgentId) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require the paused agent id on resume

When the paused job has metadata.agent_id, this guard only rejects mismatches if the resume body also includes agent_id. A crafted or buggy resume request can omit agent_id (or send a non-agents endpoint so canAccessAgentFromBody treats it as the ephemeral-agent path before buildEndpointOption) and pass this check, after which the claimed checkpoint is rebuilt from request-supplied/ephemeral agent settings instead of the agent that originally paused. That undermines the same-agent invariant described here and can resume or fail the pending action on the wrong graph; reject whenever originalAgentId is set and the body does not exactly match it, or force the stored id into the request before endpoint reconstruction.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 84b9080. The guard now requires an exact paused-agent match: if (originalAgentId && req.body.agent_id !== originalAgentId) return 403, so a resume that omits agent_id (or sends an ephemeral / non-agents id) is rejected rather than allowed to rebuild Agent A's checkpoint on a different graph. Added a parallel endpoint-mismatch guard for the same reason.

Comment on lines +44 to +45
if (typeof decision?.responseText === 'string' && decision.responseText.length > 0) {
inputs.push(decision.responseText);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Moderate all approval-supplied text

When OPENAI_MODERATION is enabled and a HITL tool approval allows reject or edit, this loop only submits responseText for moderation, but the same resume path later maps reason into a reject decision and editedArguments into the tool's updated input. A user can therefore put content that would be denied in the normal message/ask/respond paths into a reject reason or edited JSON and have it delivered to the resumed run/tool without the moderation check; include those user-authored fields (stringifying edited arguments if needed) in inputs before allowing the resume.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 84b9080. moderateText now also moderates a tool-approval decision's reject reason and its stringified editedArguments (wrapped in try/catch for unstringifiable args), in addition to responseText — so every user-authored string a resume can inject into the model now passes moderation like a typed message.

if (client?.pendingApproval) {
if (response?.databasePromise) {
try {
await response.databasePromise;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Don't save paused turns as completed responses

On the HITL pause path, response.databasePromise is the normal BaseClient save for the assistant response, and that response is not marked unfinished or terminal. Awaiting it here persists the paused tool-call as a completed assistant message even though no decision has been made; if the approval later expires, the expiry path only aborts the job/emits an SSE error and never fixes the DB row, so history shows a finished response that never actually ran. Skip this response save on pause or persist it explicitly as unfinished/error when the pause aborts or expires.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 84b9080. After BaseClient persists the response as unfinished:false, the pause teardown now re-saves it with unfinished:true (guarded on response.messageId, errors logged not thrown). An expired or never-resumed approval therefore no longer leaves a 'finished' response in history; the resume path overwrites it with the completed message on success.

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

Latest fbdfa6322 adds test coverage on top of the round-4 fixes: a route-level integration test for the resume controller (api/server/controllers/agents/__tests__/resume.spec.js, 32 cases over the full guard ladder + pause→resume→finalize lifecycle) and pure-util tests for the approval mapping (client/src/utils/approval.spec.ts, 15 cases). Please review the latest head.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fbdfa63222

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1221 to +1224
await GenerationJobManager.emitChunk(streamId, {
event: ApprovalEvents.ON_PENDING_ACTION,
data: pendingAction,
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve Redis chunks for the full approval window

When Redis-backed streams pause for approval, approvals.pause() first moves the job to requires_action and extends the chunk key TTL to the approval/checkpointer TTL, but this subsequent emitChunk persists the on_pending_action event through RedisJobStore.appendChunk, which unconditionally resets the chunk stream TTL back to the shorter running TTL. With the defaults, a user who approves after ~20 minutes but before the 24-hour approval expiry can still resume from the checkpoint, but getResumeState()/finalization no longer has the pre-pause aggregated content, so the saved response can lose the tool call and earlier assistant content. Emit before the pause transition or make appendChunk preserve the requires_action TTL.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e812b61. appendChunk is now extend-only: a single eval does XADD then EXPIRE to the running TTL ONLY when the current TTL is shorter. A paused job's chunk key (extended to the approval window by transitionStatus) therefore keeps its long TTL when the on_pending_action chunk is appended, so getResumeState() still has the pre-pause content within the approval window. Redis integration test added asserting the TTL stays long after the post-pause append (and XADD still ran).

Comment on lines +320 to +322
if (client.pendingApproval) {
logger.debug(`[ResumeAgentController] Re-paused for approval: ${streamId}`);
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist artifacts before returning on a re-pause

When a resumed segment executes an artifact-producing tool and then pauses again for another approval, this early return skips the only path that awaits client.artifactPromises and attaches them to the response. The next resume rebuilds a fresh client with a new artifact promise array, so even if the turn later completes successfully, artifacts produced before the second pause are not linked on the saved message and disappear on reload/late subscription. Persist or carry forward the current segment's attachments before leaving the job in requires_action.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e812b61. The re-pause branch now awaits client.artifactPromises and persists them (partial $set, unfinished) before returning, and finalizeResumedTurn MERGES the row's existing attachments with the final segment's (deduped) instead of overwriting — so artifacts produced before a second approval pause survive to the final save. Tests: re-pause persists artifacts; finalize merges prior + new.

Comment thread api/server/controllers/agents/resume.js Outdated
Comment on lines +81 to +84
const content =
liveContent.length > 0
? liveContent
: ((await GenerationJobManager.getResumeState(streamId))?.aggregatedContent ?? []);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Filter malformed tool-call parts on resumed finalization

The normal agents path runs filterMalformedContentParts(this.contentParts) before saving a response, but the resume finalizer saves the live client.contentParts array directly. If the content aggregator leaves an empty/malformed tool_call part in a resumed HITL turn, that invalid part is persisted and sent in the final event even though the non-HITL path would strip it, which can break reload/rendering for that message. Apply the same filter before assigning content.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e812b61. finalizeResumedTurn now runs filterMalformedContentParts on the content before saving, matching the normal AgentClient path, so a resumed turn can't persist an empty/malformed tool_call part. Test added asserting a malformed part is stripped from the saved content.

Comment on lines +236 to +238
const originalEndpoint = job.metadata?.endpoint;
if (originalEndpoint && req.body.endpoint && req.body.endpoint !== originalEndpoint) {
return res.status(403).json({ error: 'Cannot resume on a different endpoint' });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require the original endpoint on resume

Because this check only rejects a different endpoint when the request supplies one, a crafted resume POST can omit endpoint while still sending the saved agent_id. I checked the shared chat middleware path: canAccessAgentFromBody treats a missing/non-agents endpoint as the ephemeral agent, and the agents buildOptions path loads Constants.EPHEMERAL_AGENT_ID before this controller runs, so the claimed checkpoint can be resumed on a different graph instead of the persisted agent. Require the body endpoint to match the recorded endpoint whenever the job has one.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e812b61. The endpoint guard now requires an exact match like agent_id — if (originalEndpoint && req.body.endpoint !== originalEndpoint) return 403 — so a resume that OMITS endpoint is rejected instead of falling through to the ephemeral-agent path. Verified the chat middleware doesn't normalize a missing endpoint. Test added: 403 when the resume omits the paused endpoint.

Comment thread client/src/utils/approval.ts Outdated
Comment on lines +182 to +185
/** Fall back to the last assistant message (the in-flight response placeholder). */
for (let i = messages.length - 1; i >= 0; i--) {
if (isAssistant(messages[i])) {
return i;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Retry ask prompts instead of attaching to prior replies

When an ask_user_question pending action arrives before the current assistant placeholder has been inserted, the exact responseMessageId lookup misses. In an existing conversation this fallback returns the previous assistant message; applyAskUserQuestion always appends a synthetic part, so the prompt is rendered on the previous reply and the retry path never runs. Return -1 when a responseMessageId was provided but not found, so the caller retries instead of mutating an older assistant message.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e812b61. findPendingActionMessageIndex now returns the exact-match result directly when a responseMessageId is provided (i.e. -1 on a miss), so the caller (useResumableSSE requestAnimationFrame retry) defers instead of attaching the prompt to a prior assistant reply. The last-assistant fallback now runs only when no responseMessageId is given. Tests updated to assert -1 for provided-but-unmatched (user-collision + not-found) and to keep the no-id fallback.

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

e812b612e addresses all five findings from the previous review (endpoint exact-match, ask-prompt retry, cross-segment artifact accumulation, extend-only chunk TTL, malformed-part filter), each with a regression test. Please review the latest head.

@danny-avila danny-avila changed the title 🪝 feat: HITL Tool Approval Runtime (Slice B) 🪝 feat: Human-in-the-Loop Runtime — Tool Approval + Ask-User-Question (Slice B) Jun 25, 2026
@danny-avila danny-avila changed the title 🪝 feat: Human-in-the-Loop Runtime — Tool Approval + Ask-User-Question (Slice B) 🪝 feat: Human-in-the-Loop Runtime - Tool Approval + Ask-User-Question (Slice B) Jun 25, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

https://github.com/danny-avila/LibreChat/blob/e812b612eae9d75a5166b03252db046bee343163/packages/api/src/stream/implementations/RedisJobStore.ts#L1269-L1271
P2 Badge Validate pendingAction payload shape

This Redis cold-resume parser accepts any pending action with a known payload type, but the resume route and frontend then assume tool_approval.action_requests and review_configs are arrays. If Redis contains an older or malformed record such as { actionId, payload: { type: 'tool_approval' } }, it is surfaced as a live prompt instead of stale and the next resume/status render can throw rather than cleanly expiring the action. Validate the payload shape before returning it.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

* @access Private
* @returns {void}
*/
router.post('/resume', resumeController);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply the PII filter to resume answers

This new /resume endpoint accepts user-authored answer and decisions[] text, but the PII middleware mounted above it only inspects req.body.text and quoted text. In deployments with messageFilter.pii configured, an ask-user answer or a tool-approval respond/edit payload containing a blocked token bypasses that filter and is sent back into the model/tool on resume. Please extend the PII candidates for the resume fields before enabling this route.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1aa5e16. messageFilterPii now adds the resume payload's user-authored fields to the PII scan candidates — the ask-user answer, and each tool-approval decision's responseText, reject reason, and stringified editedArguments — mirroring the moderateText change. Tests added in messageFilterPii.spec.ts for answer / responseText / reason / editedArguments rejection + a clean-answer pass-through.

Comment thread api/server/controllers/agents/client.js Outdated
// Seed code-env / skill tool sessions so an approved code/file/skill-backed tool
// runs with the same uploaded-file context the pre-pause run had — the rebuilt
// graph otherwise has no `Graph.sessions` entries (especially cross-replica).
const initialSessions = buildInitialToolSessions({ agents });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Re-prime skill sessions before resume

When the original paused turn had invoked a skill with files, the normal path calls primeInvokedSkills(payload) and passes its initialSessions into buildInitialToolSessions; this resume path rebuilds a fresh graph with only agent primed files. Because the rebuilt graph otherwise has no Graph.sessions, approving a code/bash tool after such a pause loses the skill file session refs and the resumed tool runs without the injected skill files. Recompute or carry the skill initialSessions before createRun.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1aa5e16. resumeCompletion now re-primes skill files before rebuilding the run: it calls this.options.primeInvokedSkills([{ role: 'assistant', content: seedContent }]) (the pre-pause aggregated content carries the skill tool_calls) and feeds the resulting skillSessions into buildInitialToolSessions, so an approved code/file-backed tool keeps the injected skill-file session refs. Best-effort (try/catch) and only runs when there's pre-pause content.

model: responseModel,
// Persist the originating agent so a HITL resume can refuse to rebuild this
// paused run on a different agent (see resume.js).
agent_id: endpointOption.agent_id ?? req.body?.agent_id,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist ephemeral agent config for resume

For ephemeral-agent chats, buildOptions uses Constants.EPHEMERAL_AGENT_ID only when loading the agent but returns no endpointOption.agent_id, so this metadata field is usually undefined. A paused ephemeral run then skips the exact-agent guard on /resume; a resume request with the same endpoint but different ephemeralAgent or model parameters can rebuild the checkpoint against a different graph/tool set before executing the approved tool. Persist the resolved ephemeral identity/config, or reject unpinned resumes, so resume reconstructs the same graph.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1aa5e16. Pinned the graph identity with a fingerprint: at pause, computeAgentRequestFingerprint({endpoint, endpointType, agent_id, model, spec, ephemeralAgent}) (ephemeralAgent normalized for key/array order) is stored on the pending action; on resume it's recomputed from the request and a mismatch is 403'd. This closes the ephemeral-agent case where agent_id is undefined so the id guard couldn't tell two configs apart. Fingerprint rides in the already-JSON-serialized pendingAction (no new store plumbing). Tests: fingerprint match/mismatch in resume.spec.js + computeAgentRequestFingerprint unit tests (stability, field-change, ephemeral normalization/swap).

Comment on lines +39 to +41
return { type: 'edit', updatedInput: resolution.editedArguments ?? {} };
case 'respond':
return { type: 'respond', responseText: resolution.responseText ?? '' };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject incomplete edit/respond decisions

When a crafted or buggy resume request chooses an allowed edit or respond decision but omits the required editedArguments or responseText, these defaults turn it into an empty tool input or empty synthetic tool result instead of rejecting it. That can resume the paused run with behavior the user did not actually approve, especially for tools where {} is a valid but broad argument shape. Validate those required fields before mapping the decisions.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1aa5e16. Added findIncompleteDecisions (packages/api/src/agents/hitl/resume.ts), wired into resolveResumeValue before mapping: an edit without an object editedArguments or a respond without a non-empty responseText now returns 400 (with the offending tool_call_ids) instead of falling through to toSdkDecision's {} / '' defaults. Unit tests + resume.spec.js route cases (incomplete edit/respond → 400, complete edit → 200).

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

1aa5e16f4 addresses all four findings from the previous review (resume-payload PII filtering, skill-session re-priming, ephemeral-agent fingerprint pinning, incomplete edit/respond rejection), each with regression tests. Please review the latest head.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1aa5e16f46

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +241 to +244
agent_id: conversation?.agent_id,
model: conversation?.model,
spec: conversation?.spec,
ephemeralAgent: getEphemeralAgent(conversationId),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve addedConvo when resuming paused runs

When a HITL pause happens during a multi-conversation/added-agent turn, this resume payload rebuilds the run without addedConvo, even though the agents endpoint builder reads req.body.addedConvo to add the secondary agent into endpointOption. The resumed Run can therefore be constructed with a different graph/tool set than the checkpoint expects, causing resume failures or executing the continuation without the added conversation context. Include the original addedConvo (or persist/replay it from the job metadata) and pin it in the fingerprint as well.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged — deferring with reason rather than shipping a no-op. addedConvo isn't carried on the main conversation object (it's the secondary-pane state), so adding addedConvo: conversation?.addedConvo to the resume payload would always be undefined and pin nothing. A correct fix needs to source the added-pane convo on resume + pin it; HITL inside a parallel added-conversation pane is a narrow combination. Tracking as a follow-up; the fingerprint mechanism is in place to extend once the added-pane state is plumbed to the resume path.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now fixed in 409666a — and cleanly, thanks to the round-8 replay. addedConvo is in RESUME_CONTEXT_KEYS, so the restoreResumeContext middleware replays it from the paused request before buildEndpointOption; the parallel/secondary-agent config is reconstructed server-side without the client needing to re-send it. (This is the round-7/8 deferral resolved — the replay mechanism made it a one-liner.) Test: pickResumeContext includes addedConvo.

Comment on lines +1781 to +1782
const primed = await this.options.primeInvokedSkills([
{ role: 'assistant', content: seedContent },

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Re-prime manual skill sessions on Redis resumes

This re-prime path only scans seedContent, but in Redis/cross-replica resumes that content is reconstructed from streamed run-step/message events; manually selected skill cards are added later with this.contentParts.unshift(...) and are not streamed into Redis. If a user manually selects a file-backed skill and the turn pauses before completion, resuming after reload or on another replica still finds no skill tool call here, so approved code/file tools run without the skill file session refs the user selected.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged — deferring with reason. Manually-selected skills aren't model-invoked tool_calls, so they're not in seedContent (or the streamed Redis content) for the round-6 re-prime to find. A correct fix needs the manual-skill set carried durably (this round persists manualSkills on the job userMessage as a building block) AND a re-resolve/prime path that avoids double-injecting their content parts on resume — the content/session interaction is intricate enough that a rushed fix risks duplication. Narrow case (manual file-backed skill + pause + cross-replica/reload); tracking as a follow-up on top of the metadata now persisted.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: 409666a doesn't fully close the manual-skill cross-replica priming (separate from the pills fix), but it now persists manualSkills durably on job.metadata.userMessage — the building block for a follow-up that re-resolves them on resume.

Comment on lines +159 to +165
const canonical = JSON.stringify({
endpoint: fields.endpoint ?? null,
endpointType: fields.endpointType ?? null,
agent_id: fields.agent_id ?? null,
model: fields.model ?? null,
spec: fields.spec ?? null,
ephemeralAgent: normalizeEphemeralAgent(fields.ephemeralAgent),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Pin ephemeral instructions in the resume fingerprint

For ephemeral agents, loadEphemeralAgent derives the agent instructions from promptPrefix/model parameters, but the fingerprint only includes endpoint/model/spec/capabilities. A resume request that omits or changes promptPrefix still passes this pin and rebuilds the checkpoint with different instructions before continuing the approved tool flow. Include the graph-affecting model parameters/prompt prefix in the pinned data, or persist and replay the original endpoint option.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 446bb47. promptPrefix is now part of computeAgentRequestFingerprint, and it's re-sent on resume — added to ResumeAgentFields and buildResumeFields (sourced from conversation.promptPrefix, symmetric with model/spec, so legit resumes still match). A resume that changes the ephemeral instructions now mismatches the pin → 403. Tests: promptPrefix match/mismatch in resume.spec.js + computeAgentRequestFingerprint unit tests.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

x

Comment on lines +454 to +459
if (client.pendingApproval) {
logger.debug(`[ResumeAgentController] Re-paused for approval: ${streamId}`);
// Persist any artifacts this segment produced before the fresh client (next
// resume) drops them — finalize later merges them onto the saved message.
await persistRePauseArtifacts({ req, client, job, conversationId });
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist streamed content before returning on re-pause

When a resumed segment streams additional text/tool calls and then pauses again, this branch only persists artifacts before returning. If the second approval expires or the job is later cleaned up without a final resume, the saved unfinished response still contains only the earlier pause's content, so the user loses everything streamed during this resumed segment after reload/history. Save the accumulated content (like finalization does, but still unfinished) along with artifacts before leaving the job in requires_action.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 446bb47. The re-pause branch now persists the segment's accumulated CONTENT (filtered via filterMalformedContentParts, same source as finalize) in addition to artifacts, saved as a partial unfinished $set. If the second approval expires/reaps without a final resume, the row keeps everything streamed this segment; a successful resume's finalize overwrites it. Test added (re-pause persists content, no finalize).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(round-8 reply stands)

Comment on lines +284 to +286
requestMessage: userMessage
? sanitizeMessageForTransmit({ ...userMessage, conversationId, isCreatedByUser: true })
: null,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Preserve user-message skill metadata on resumed final events

For turns submitted with manual or always-applied skills, the live user message carries manualSkills/alwaysAppliedSkills, but the resumed final event reconstructs requestMessage from the stripped job metadata. The client final handler replaces the current user bubble with this object and only restores files, so the skill pills disappear immediately after a resumed turn completes until a full reload refetches the DB row. Carry these fields in the pending metadata or hydrate the saved user message before emitting the final event.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 446bb47. getPreliminaryUserMessage now carries manualSkills/alwaysAppliedSkills onto the persisted job userMessage, so the resumed final event's requestMessage keeps the skill pills (the client's final handler replaces the user bubble from this object) instead of dropping them until a reload.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

superseded

Comment on lines +1409 to +1410
if (metadata.agent_id) {
updates.agent_id = metadata.agent_id;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear stale agent_id when a new Redis job omits it

Redis reuses the conversation id as the job hash key, so a new turn HSETs over the previous hash. This update only writes agent_id when the new metadata has a truthy value; if a conversation switches from a saved agent to an ephemeral/no-agent turn, the old agent_id remains in Redis and the resume guard later rejects the valid pause as a different agent. Clear agent_id on create/update when absent, or write an explicit empty value, so the guard sees the current run's identity.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 446bb47. Added agent_id to staleHitlFields in RedisJobStore.createJob, so a reused conversation hash drops the prior turn's agent_id; updateMetadata then re-sets it only when the new turn has one. A conversation that switches saved-agent → ephemeral no longer keeps a stale id that made the resume guard reject the valid pause. (InMemoryJobStore already replaces the whole job object, so it was unaffected.) Redis integration test added asserting the clear.

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

446bb474e fixes 4 of the 6 findings (stale agent_id clearing, promptPrefix in the fingerprint, re-pause content persistence, skill-pill metadata), each with tests. The remaining 2 (addedConvo on resume; cross-replica manual-skill priming) are narrow and deferred with reasons in-thread rather than shipping fragile/no-op changes. Please review the latest head.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 446bb474e9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +122 to +124
...(Array.isArray(manualSkills) && manualSkills.length > 0 && { manualSkills }),
...(Array.isArray(alwaysAppliedSkills) &&
alwaysAppliedSkills.length > 0 && { alwaysAppliedSkills }),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve skill metadata after onStart

When a HITL turn reaches sendMessage, these preliminary fields are overwritten by the later onStart/trackUserMessage metadata update with the BaseClient user message; BaseClient only adds manualSkills/alwaysAppliedSkills after onStart. In the normal pause→resume path, finalizeResumedTurn builds requestMessage from job.metadata.userMessage, so turns with manual or always-applied skills still lose their skill pills in the final streamed message despite these added fields. Carry the fields into the later metadata update, or merge instead of replacing the preliminary user message.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 02ddd12. The skill fields are now carried into the onStart updateMetadata (sourced from req.body, since they aren't on userMsg yet at onStart) — that's the update that overwrote the preliminary one. finalizeResumedTurn's requestMessage is built from job.metadata.userMessage, so the pills now survive. Test asserts the resumed requestMessage carries manualSkills/alwaysAppliedSkills.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-fixed properly in 409666a — you were right that the onStart updateMetadata was overwritten. The authoritative writer is trackUserMessage (via the created-event emitChunk), which rebuilt userMessage from a curated subset. Now the emitted created message carries manualSkills/alwaysAppliedSkills (from req.body, since BaseClient adds them after onStart), and trackUserMessage persists them; widened UserMessageMeta + SerializableJobData.userMessage. So job.metadata.userMessage keeps the pills end-to-end (and the live created event renders them immediately too).

// Ephemeral agents derive their instructions from promptPrefix — re-send it so
// the resumed run rebuilds the same graph and matches the server fingerprint.
promptPrefix: conversation?.promptPrefix,
ephemeralAgent: getEphemeralAgent(conversationId),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore ephemeral agent config before resume

For paused ephemeral-agent turns after a page reload, this reads from the in-memory Recoil atom, which defaults to null and is not reconstructed by the status/resume-state path. The server now fingerprints the original req.body.ephemeralAgent, so submitting an approval with ephemeralAgent: null for a turn that enabled MCP/tools/skills will hit the resume fingerprint guard and return 403, making durable resume fail exactly in the reload/cross-session case it is meant to support. Persist the original ephemeral config with the pending action or restore the atom from the resume state before building this payload.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 02ddd12 — and this was the important one: it surfaced that durable resume of an ephemeral-agent-with-tools turn was broken after reload (the client can't reconstruct the ephemeralAgent Recoil atom, so the rebuilt agent lost its tools — the fingerprint just turned that into a 403). Fixed by persisting the graph-determining fields as pendingAction.resumeContext at pause and REPLAYING them onto the resume request in a router middleware that runs before buildEndpointOption. The server now sources the config, so reload/cross-replica rebuilds the same graph (and a crafted swap is impossible; the fingerprint still matches because the body is restored first). Tests: pickResumeContext/applyResumeContext + round-trip.

Comment on lines +253 to +254
if (Object.keys(responseMetadata).length > 0) {
responseMessage.metadata = responseMetadata;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist contextMeta on resumed responses

When a resumed run records a calibration ratio, resumeCompletion stores it on client.contextMeta, but this finalize path only persists metadata and never copies contextMeta onto responseMessage like BaseClient.sendMessage does. After any HITL resume that recalibrates context usage, the saved parent response lacks contextMeta, so the next turn cannot seed the pruner from the previous run and falls back to uncalibrated token accounting. Add responseMessage.contextMeta = client.contextMeta before saving when it is present.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 02ddd12. finalizeResumedTurn now sets responseMessage.contextMeta = client.contextMeta (when present) before saving, mirroring BaseClient.sendMessage, so a resumed turn's context-window calibration is persisted and the next turn can seed its pruner. Test added.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(round-8 reply stands)

const canonical = JSON.stringify({
endpoint: fields.endpoint ?? null,
endpointType: fields.endpointType ?? null,
agent_id: fields.agent_id ?? null,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include saved-agent revisions in the fingerprint

When a saved agent is edited while a run is paused, the pause and resume requests still hash the same agent_id, so this fingerprint guard passes even though initializeClient reloads the current agent definition and rebuilds a different graph/tool implementation for the checkpoint. Approving after an agent/tool/action edit can therefore fail to resume or execute the pending call against a changed tool set; persist and compare an agent version/config hash, not just the stable id.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged — deferring with reason. Catching a saved agent that's EDITED while a specific run is paused needs a persisted agent version/config hash (the agent_id is stable; the definition is mutable), compared on resume. That's a larger change for a narrow window (editing an agent during the seconds-to-minutes a particular run is awaiting approval), and the resumeContext replay added this round already pins the ephemeral/no-saved-agent case. Tracking as a follow-up; flagging rather than shipping a heavier agent-version mechanism mid-review.

Comment on lines 238 to 243
const staleHitlFields: Array<keyof SerializableJobData> = [
'pendingAction',
'pendingActionId',
'lastActiveAt',
'agent_id',
];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear stale temporary state on Redis job reuse

Redis createJob overlays a fresh job onto any existing hash for the same conversation, but the clear-list omits the newly persisted isTemporary field. If a previous paused/expired temporary turn left isTemporary=1 and a later non-temporary client/API request omits isTemporary, updateMetadata does not write false, so a subsequent HITL resume reads the stale flag and saves the completed response as temporary/non-persistent. Clear isTemporary here as another per-turn field or always write a false default.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 02ddd12. Added isTemporary to staleHitlFields in createJob (same class as the agent_id fix): a reused conversation hash now drops a prior temporary turn's flag, so a later non-temporary resume no longer saves its response as temporary. Redis integration test extended to assert both agent_id and isTemporary are cleared.

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

02ddd12d0 fixes 4 of 5: the durable-resume regression (persist+replay the graph config server-side, so reload/cross-replica rebuilds the same ephemeral graph), isTemporary stale-clear, contextMeta persistence, and the manualSkills metadata placement. The 5th (saved-agent edited mid-pause) is deferred with a reason in-thread (needs an agent version hash; narrow). Please review the latest head.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 02ddd12d06

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +175 to +177
'promptPrefix',
'ephemeralAgent',
] as const;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist addedConvo in resume context

When a paused run was started with addedConvo, resuming rebuilds a different graph because this allowlist omits that field. The agents build path reads req.body.addedConvo into endpointOption and initialize.js uses it to populate agentConfigs for parallel-agent execution, but the resume POST does not send it and applyResumeContext cannot restore it, so HITL approval in an added-conversation/parallel-agent run resumes without those agents and can fail or continue against the wrong graph. Include addedConvo in the persisted context and fingerprint.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 409666a — cleanly, thanks to the round-8 replay. Added addedConvo to RESUME_CONTEXT_KEYS, so restoreResumeContext replays it from the paused request before buildEndpointOption; the parallel/secondary-agent config is reconstructed server-side. (The round-7/8 deferral, now a one-liner once replay existed.) Test: pickResumeContext includes addedConvo.

Comment on lines +471 to +475
...(Array.isArray(req.body?.manualSkills) &&
req.body.manualSkills.length > 0 && { manualSkills: req.body.manualSkills }),
...(Array.isArray(req.body?.alwaysAppliedSkills) &&
req.body.alwaysAppliedSkills.length > 0 && {
alwaysAppliedSkills: req.body.alwaysAppliedSkills,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve skill metadata after created-event tracking

For turns with manual or always-applied skills, these fields are written here but then the immediately following GenerationJobManager.emitChunk(...created...) calls trackUserMessage, which rewrites jobData.userMessage with only messageId/parent/text/quotes. In the in-memory store this deterministically drops the skill fields before a HITL resume reads job.metadata.userMessage, and in Redis it is at best racing, so the resumed final event still loses the skill pills until a DB refetch. The created-event tracking path needs to carry these fields too or avoid clobbering the richer metadata.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 409666a — you correctly caught that the round-8 onStart write was overwritten by trackUserMessage (the authoritative userMessage writer, via the created-event emitChunk). Now the emitted created message carries manualSkills/alwaysAppliedSkills (from req.body, since BaseClient adds them after onStart) and trackUserMessage persists them; widened UserMessageMeta + SerializableJobData.userMessage. job.metadata.userMessage keeps the pills end to end, and the live created event renders them immediately.

// doesn't carry it. This is the user message's parent (the thread position);
// `client.parentMessageId` below is a different value — the response's parent, i.e.
// the user message id.
req.body.parentMessageId = job.metadata.userMessage?.parentMessageId ?? Constants.NO_PARENT;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include the paused user message when re-priming code files

For a HITL pause in a turn that uploaded execute-code files, resuming with the original parent skips those files during initializeClient: initializeAgent rebuilds code sessions by walking DB messages from req.body.parentMessageId, and execute-code files are explicitly excluded from the broader conversation-file lookup, so starting at the user's parent omits the paused user message's own files. The approved code/read-file tool then resumes without the same uploaded-file session it had before the pause; seed this with the paused user message id (or otherwise pass the saved current-turn files) before reconstruction.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 409666a. The resume controller now restores the paused user message's own files onto req.body.files (from the persisted message via getMessages) before initializeClient — execute-code files are excluded from the parentMessageId code-session walk, so an approved code/read-file tool kept losing them. Test asserts the files reach initializeClient.

Comment on lines +805 to +806
if (data.resumeState?.pendingAction) {
applyPendingActionToMessages(data.resumeState.pendingAction as Agents.PendingAction);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Replay pending approval gap events

This re-applies a pending action only when it was present in the resume snapshot, but in in-memory mode subscribeWithResume() can also return an on_pending_action in pendingEvents when the run pauses between getResumeState() and the subscription. The later pending-events replay falls through to the generic stepHandler, so the approval/ask-user controls are never attached and the stream sits paused with no UI until a full status reload. Handle ApprovalEvents.ON_PENDING_ACTION in the pending-events replay as well.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 409666a. Both resume replay loops (resumeState.replayEvents and pendingEvents) now route ApprovalEvents.ON_PENDING_ACTION to applyPendingActionToMessages (mirroring the live handler), instead of falling through to stepHandler — so a pause that lands in the getResumeState→subscription window renders its approval/ask-user controls instead of sitting paused with no UI.

agent_id: endpointOption.agent_id ?? req.body?.agent_id,
// Persist temporary-chat state so a HITL resume keeps the resumed response
// non-persisted instead of trusting the resume request to re-send the flag.
isTemporary: req.body?.isTemporary,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use saved temporary flag when aborting paused jobs

This stores the original isTemporary flag for HITL resumes, but the abort path still saves partial responses with req.body.isTemporary; the stop button posts only the conversation id. If a temporary chat pauses for approval and the user aborts it, the abort save treats it as non-temporary and can persist an orphaned partial response. The abort endpoint should use abortResult.jobData.isTemporary/job metadata instead of trusting the abort request body.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 409666a. The /chat/abort partial-save now sources isTemporary from jobData?.isTemporary (the persisted job metadata) rather than req.body — the stop button posts only conversationId, so a paused temporary chat that's aborted no longer persists an orphaned non-temporary partial. Test added (abort.spec.js).

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

409666a1d fixes all 5: addedConvo replay (resolving the earlier deferral via the round-8 replay), the real manualSkills fix (trackUserMessage was the overwriter), execute-code file restore on resume, in-memory pending-action UI replay, and abort isTemporary from job metadata. Each with a test. (Note: a couple of replies above landed on the round-8 thread ids by mistake — the substantive replies are on this round's threads.) Please review the latest head.

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

https://github.com/danny-avila/LibreChat/blob/409666a1d8dc1f18c677faeb04a3447f42b4eb37/packages/api/src/stream/implementations/RedisJobStore.ts#L655
P2 Badge Guard Redis cleanup by pending action id

This cleanup path has the same race at the approval boundary: it can read expired action A, then a concurrent resume can resolve A and re-pause on action B before transitionStatus runs. Since the transition has no expectActionId, it can abort the new action B just because the job is again in requires_action. Include the stale action's id in the transition guard so cleanup only expires the record it actually inspected.


https://github.com/danny-avila/LibreChat/blob/409666a1d8dc1f18c677faeb04a3447f42b4eb37/packages/api/src/stream/implementations/RedisJobStore.ts#L655
P2 Badge Guard Redis expiry cleanup by action id

This cleanup path has the same stale-action race as the lifecycle helper: it reads an expired/malformed requires_action job, then performs a status-only transition. If the user resolves that action and the run immediately re-pauses for a new live action before this transitionStatus executes, the cleanup will abort the new action because it does not set expectActionId. For expired actions with an id, include the observed pendingAction.actionId in the transition guard so cleanup cannot terminate a freshly re-paused run.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if (!job || job.status !== 'requires_action' || !isPendingActionExpired(job)) {
continue;
}
const expired = await this._approvals.expire(streamId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Pin expiry sweep to the observed action

When a user submits a decision right as this expiry sweep is running, the sweep can read expired action A, then the resume can resolve A and re-pause on action B before this line executes. Because expire() is called without the action id that was observed, its CAS only checks requires_action and can abort the fresh action B, leaving a valid newly-paused run terminal. Pass the observed job.pendingAction.actionId into expire() so stale expiry work cannot affect a later pause.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 19400a4. Both stale-approval sweeps now pass the observed pendingAction.actionId into the expire CAS: expireStaleApprovals → approvals.expire(streamId, job.pendingAction?.actionId), and the RedisJobStore requires_action cleanup → transitionStatus({..., expectActionId: job.pendingAction?.actionId}). So if the action is resolved and the run re-pauses on a fresh action between the read and the CAS, the CAS no longer matches and no-ops instead of aborting the valid new pause. Test: expire() with a mismatched expectedActionId returns false (pendingAction.spec.ts).

if (!job || job.status !== 'requires_action' || !isPendingActionExpired(job)) {
continue;
}
const expired = await this._approvals.expire(streamId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Pin expiry sweep to the observed action

When this sweep observes expired action A, a user can still resolve A and the continuation can pause again on a fresh action B before this call runs. Calling expire(streamId) without the expectedActionId only checks that the job is currently requires_action, so the stale sweep can abort B even though it has not expired. Pass the action id read from job.pendingAction into expire() so the CAS only expires the specific pending action that was observed as stale.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 19400a4 — same fix as the sibling sweep: the RedisJobStore requires_action cleanup now passes expectActionId: job.pendingAction?.actionId to transitionStatus, so it only aborts the exact action it observed as stale. (Undefined for a missing/malformed pendingAction → falls back to the status-only check, which is correct since there's nothing to protect.)

Comment thread api/server/controllers/agents/client.js Outdated

const streamId = this.options.req?._resumableStreamId;
if (streamId && run.Graph) {
GenerationJobManager.setGraph(streamId, run.Graph);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid caching the rebuilt graph for Redis resume state

When a Redis-backed run resumes, this replaces the stream's local graph cache with the freshly rebuilt graph. RedisJobStore.getContentParts() prefers that local graph over reconstructing from Redis chunks, but the rebuilt resume graph was created with messages: [] and the pre-pause content is only preserved via the Redis append log/client seed. A same-replica reload or status poll during the resumed segment/re-pause can therefore return resumeState.aggregatedContent missing the pre-pause assistant/tool-call content, while cross-replica reconstruction would be complete. Avoid replacing the Redis graph cache on resume, or make the Redis store fall back to chunk reconstruction when the local graph lacks the seeded content.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 19400a4. resumeCompletion no longer calls setGraph on resume — the rebuilt graph is created with messages: [], and RedisJobStore.getContentParts prefers a cached graph over chunk reconstruction, so caching it dropped the pre-pause content on a same-replica reload/poll. Skipping it makes getContentParts fall back to the durable chunk log (complete); setContentParts still seeds the in-memory store with the client's content.

Comment on lines +533 to +535
pendingActionRetryRef.current = requestAnimationFrame(() =>
applyPendingActionToMessages(pendingAction, false),
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep retrying until the approval target renders

When on_pending_action arrives before the assistant/tool-call state update has committed, this schedules only one animation-frame retry and then drops the pending action if the target message is still missing (or the tool-call part still has not rendered). React/Recoil message updates from the immediately preceding created/step events are asynchronous and can take more than one frame under load, so a valid requires_action run can be left with no approval controls until a full reload/status sync. Keep the pending action and retry when messages change, or continue retrying until the exact response/tool-call appears or the stream leaves requires_action.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 19400a4. applyPendingActionToMessages now retries across frames (bounded at 120 ≈ 2s) until the target response/tool-call message commits, instead of a single requestAnimationFrame that dropped the pending action when Recoil/React updates took more than one frame under load. Reuses the existing pendingActionRetryRef + cancelAnimationFrame cleanup.

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

19400a40e fixes all three round-10 findings: the expiry/cleanup sweeps now scope their CAS to the observed actionId (can't abort a re-paused action); resume skips caching the empty-messages graph so same-replica content reconstruction stays complete; and the pending-action UI retries across frames instead of dropping after one. Please review the latest head.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 19400a40e2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danny-avila
danny-avila force-pushed the feat/hitl-tool-approval branch 3 times, most recently from 684f0e7 to 34c9c4b Compare June 25, 2026 21:18

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f4b187d421

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 257 to +261
const staleHitlFields: Array<keyof SerializableJobData> = [
'pendingAction',
'pendingActionId',
'lastActiveAt',
'agent_id',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear stale discovered tools on job replacement

When Redis reuses the same streamId for a fresh turn, HSET only overwrites the fields present in the new job and anything not listed here survives from the prior hash. discoveredTools is now persisted per paused run, but it is not cleared here and handleRunInterrupt only rewrites it when the new turn discovers at least one deferred tool; a replacement turn that later pauses without its own discovery can therefore pass the previous run's tool names into /resume and force-load deferred tools that were not discovered in this run. Please clear discoveredTools alongside the other per-turn HITL fields.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 38e6e2a — added discoveredTools to createJob's staleHitlFields HDEL list, right alongside agent_id/isTemporary (same rationale: handleRunInterrupt only writes it when THIS turn discovered ≥1 deferred tool, so a reused hash would otherwise keep the prior run's names). The in-memory store builds a fresh job object on createJob, so this was Redis-only. Integration test asserts a reused streamId clears discoveredTools.

Comment thread api/server/controllers/agents/resume.js Outdated
// Atomically claim the resume. The single winner drives the run; a racing second
// submit (double-click, two tabs) gets false and must not re-drive — that would
// re-execute tools and double-bill.
const claimed = await GenerationJobManager.approvals.resolve(streamId, pendingAction.actionId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Decrement the resume slot if claim throws

With LIMIT_CONCURRENT_MESSAGES enabled, a store/Redis error thrown by approvals.resolve happens after checkAndIncrementPendingRequest has already incremented the user's counter but before the later try/finally that decrements it. Unlike the false-claim branch, this exception path leaks a concurrency slot until the counter TTL expires, so the same user can get spurious 429s when retrying the still-paused approval. Wrap the claim in the cleanup scope or decrement on this error path.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 38e6e2a. Wrapped the approvals.resolve claim in a try/catch that calls decrementPendingRequest(userId) and returns 500 — the claim runs after the slot increment but before the run's own try/finally, so a store error there previously leaked the slot until the counter TTL expired (spurious 429s on retry of the still-paused approval). Mirrors the existing !claimed (409) release path. New test asserts the slot is released when the claim throws.

'if jt > target then target = jt end ' +
'end ' +
'local cur = redis.call("TTL", KEYS[1]) ' +
'if cur < target then redis.call("EXPIRE", KEYS[1], target) end ' +

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Preserve run-step TTL across approval pauses

When Redis pauses a job, the chunk stream now uses this no-shrink TTL path so late writes cannot shorten the approval window, but run steps still go through saveRunSteps, which does SET ... EX this.ttl.running. If a run-step save is still in flight when a fast tool-approval/ask-user pause transitions the job, or the run-steps key does not exist until that save lands, it resets the run-step key to the 20-minute running TTL while the job/chunks live for the approval window. Users who reload a still-live approval after that lose the run-step/tool timeline even though the approval remains resumable; apply the same extend-only paused TTL logic to run steps.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 38e6e2a. saveRunSteps now uses a paused-window TTL Lua script (RUNSTEPS_SAVE_LUA) instead of an unconditional SET ... EX running: when the job hash is requires_action it extends the run-steps key to the approval window (max of running TTL and the job key's TTL), mirroring the chunk-stream CHUNK_APPEND_LUA no-shrink behavior you flagged earlier. A run-step save landing at/after a pause no longer evicts the tool timeline before resume. Integration test added (parallel to the chunk-TTL one).

…end 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).
@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

All three follow-ups addressed in 38e6e2af5:

  • Clear stale discovered tools on job replacement — discoveredTools added to createJob's staleHitlFields HDEL list (Redis hash reuse).
  • Decrement the resume slot if claim throws — approvals.resolve wrapped in try/catch that releases the slot + returns 500.
  • Preserve run-step TTL across approval pauses — saveRunSteps now uses a paused-window TTL script mirroring the chunk-stream no-shrink behavior.

Integration tests added for the stale-clear and run-step TTL; controller test for the slot release.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 38e6e2af5c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

return (
<>
{toolCall}
<ToolApproval approval={tc.approval} toolCallId={tc.id ?? ''} args={tc.args} />

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep nested approval controls mounted

When a HITL pause occurs inside a subagent, this dialog body is the only place that mounts the nested ToolApproval. Because OGDialogContent is not mounted while the subagent details dialog is closed, the approval card does not register with ApprovalProvider and no approval/submit control is visible until the user happens to open the dialog; in a mixed batch, the top-level card can also submit only the mounted decisions and hit the server's “every paused tool call must be decided” rejection. Render/register the nested pending action from the always-mounted subagent card, or force the dialog content to stay mounted while an approval is pending.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified valid — the nested ToolApproval registers with ApprovalProvider only in a mount effect, and it lives inside OGDialogContent, which Radix mounts only while the subagent details dialog is open; so a nested pause with the dialog closed leaves the card unregistered (no submit control, and a mixed-batch submit omits that decision → the server's "every paused tool call must be decided" rejection). Not fixing in this commit: the cleanest fix (forceMount so the content stays mounted/hidden while a nested approval is pending) requires threading forceMount through the SHARED OGDialogContent primitive (used app-wide) plus CSS/interaction verification I can't do without running the UI. Tracking it with the rest of the subagent-HITL surface (which is the deferred part of this PR) so it gets proper UI review rather than a blind change to a shared component. The backend findings in this review (J2/J3/J4) are fixed here.

isTemporary: req?.body?.isTemporary,
interfaceConfig: req?.config?.interfaceConfig,
},
{ ...response, endpoint: endpointOption.endpoint, unfinished: true, user: userId },

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Guard pause save after fast resume

If the user approves immediately after the pending-action SSE, /resume can finish and save the completed response while this original request is still waiting on response.databasePromise; this later save then writes the pre-pause response back with unfinished: true, overwriting the completed content. Before marking the paused row unfinished, re-check that the job is still the same requires_action action (or skip if it already resumed/completed) so fast approvals cannot corrupt saved history.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1afd1eb. Before marking the paused row unfinished, the pause branch now re-reads the live job and only proceeds while it is STILL requires_action on THIS generation's action — a fast /resume claim transitions the job out of requires_action (and a replacement bumps createdAt), so the guard skips the unfinished-save and the resumed turn's completed content is preserved. Fails open on a read error so a genuinely never-resumed approval isn't left looking "finished". Predicate test added.

// HITL opt-in: the `humanInTheLoop` switch + the PreToolUse policy hook. Spread
// here (not just `compileOptions.checkpointer` above) so an `ask` decision raises
// a real interrupt — without these the run would never pause. Absent when disabled.
...(hitl && { humanInTheLoop: hitl.humanInTheLoop, hooks: hitl.hooks }),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Gate HITL to resumable controllers

With toolApproval.enabled, this spread enables HITL for every createRun caller, but the OpenAI-compatible and Responses controllers call createRun/processStream and never inspect run.getInterrupt() or persist a pending action/resume state. When an API request hits an approval-gated tool, the run can pause at the interrupt and those routes then send a normal final response or [DONE] with no approval surface or resume endpoint, leaving the tool call unresolved. Gate this wiring to callers that implement the HITL lifecycle, or make unsupported routes reject/auto-deny instead of pausing.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1afd1eb. Confirmed: the HITL wiring was gated only on toolApproval.enabled with no per-caller gate — and the OpenAI-compat (openai.js) + Responses (responses.js) controllers call createRun/processStream but never inspect run.getInterrupt() or persist a pending action. Added a hitlCapable flag to createRun (default false) that gates the humanInTheLoop/PreToolUse/checkpointer wiring; only AgentClient's chat + resume paths set it. Non-resumable routes now run identical to the no-HITL path, so an approval-gated tool can't pause where there's no resume endpoint. Test asserts the gate both directions (enabled+capable → wired; enabled+not-capable → no humanInTheLoop, no checkpointer).

Comment thread api/server/controllers/agents/resume.js Outdated
Comment on lines +423 to +424
if (job.status !== 'requires_action' || isPendingActionStale({ pendingAction })) {
return res.status(409).json({ error: 'No live pending action to resume' });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Expire stale actions on submit

When an approval expires just before the user submits, this early stale check returns 409 without driving ApprovalLifecycle.expire(). The job remains in requires_action with its stale pending action until the periodic sweeper runs, so any attached SSE client gets no terminal error/done event and the paused stream appears to hang even though the UI has already reported the action as expired. Explicitly expire/abort the observed action here before returning the 409.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1afd1eb. Extracted GenerationJobManager.expireApproval(streamId, actionId) — the expire CAS (pinned to the observed actionId) + terminal APPROVAL_EXPIRED_ERROR emit, now shared with the periodic sweeper — and the resume route calls it when it observes a just-expired action on submit. The attached SSE client now gets a terminal event immediately instead of the stream hanging until the next sweep. (The status !== requires_action 409 path is left as-is — that job is already terminal, nothing to expire.) Tests assert expire IS driven on a stale action and is NOT on an already-terminal job.

…xpire 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).
@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

Backend findings fixed in 1afd1eb20:

  • Guard pause save after fast resume (P1) — pause branch re-checks the job is still paused on this generation before marking the row unfinished.
  • Gate HITL to resumable controllers (P1) — createRun hitlCapable flag gates the wiring; only AgentClient chat + resume set it.
  • Expire stale actions on submit — extracted expireApproval (expire CAS + terminal SSE) and drive it from the resume route on a stale action.

The frontend finding (nested subagent approval not mounting while the details dialog is closed) is verified valid but tracked with the deferred subagent-HITL surface — the forceMount fix touches the shared dialog primitive and needs UI verification (replied on the thread).

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 1afd1eb209

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danny-avila
danny-avila merged commit 6dbf9d5 into dev Jun 29, 2026
32 checks passed
@danny-avila
danny-avila deleted the feat/hitl-tool-approval branch June 29, 2026 20:56
noobskies added a commit to noobskies/gptchina that referenced this pull request Jul 1, 2026
Merged 113 upstream commits from v0.8.6 (9efe487) up to upstream/main
HEAD ac759ef — v0.8.7 (tag 9e74cc0) plus 28 post-release commits.
Gains Claude Sonnet 5 support (LibreChat-AI#14042), Memory as an agent capability
(LibreChat-AI#13869), and HITL tool approval (LibreChat-AI#12938/LibreChat-AI#13942).

Conflicts resolved (2):
- client/src/components/Chat/Input/ToolsDropdown.tsx — kept upstream's new
  Memory dropdown block live; re-applied our commented-out Code Interpreter
  removal around only the `if (canRunCode && codeEnabled)` block.
- package-lock.json — regenerated from upstream's lockfile + npm install +
  npm install --package-lock-only to re-add @emnapi/{core,runtime,wasi-threads};
  verified clean with `npm ci --dry-run` (Node 24 / npm 11).

Bookkeeping: bumped custom/overrides/upstream-version.txt; added a v0.8.7
sync section to custom/MODIFICATIONS.md documenting the resolution, the
lockfile recipe, the pending migrate:terms-timestamp migration (LibreChat-AI#10810,
run at next deploy), and two known non-blocking test failures (an upstream
memoization timing test that flakes under our lazy Stripe-loading; upstream
Anthropic-API integration tests that need network).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fuuuzzy pushed a commit to fuuuzzy/LibreChat that referenced this pull request Jul 7, 2026
… (Slice B) (LibreChat-AI#13942)

* 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)
danny-avila added a commit that referenced this pull request Jul 7, 2026
… pause/resume

The HITL runtime merged in #13942/#14024/#14025/#14123 already ships the full
ask_user_question lifecycle (payload-agnostic handleRunInterrupt, resume
validation via mapAskUserAnswer, reconnect rehydration, and the client question
card) — but nothing ever raised the interrupt. This adds the producer:

- packages/api/agents/hitl/askUserQuestionTool.ts: LLM-callable tool whose func
  calls the SDK askUserQuestion() helper (LangGraph interrupt() from the tool
  body); zod schema with length caps mirroring AskUserQuestionRequest, plus a
  JSON-schema twin for the schema-only registry
- Registration: agentToolDefinitions, manifest.json (Tools dialog, admin
  filteredTools/includedTools kill switch), basicToolInstances, handleTools
  constructor branch
- run.ts gating: checkpointer now attaches for hitlCapable runs whose agents
  carry the ask tool even with the tool-approval policy disabled (the interrupt
  needs only durability, not humanInTheLoop/hooks); the tool is stripped
  fail-closed from non-HITL callers (OpenAI-compat/Responses) and subagent
  child configs; excluded from eager event execution (interrupts must be
  raised inside the Pregel task frame)
- resume.js: 16k length cap on the answer wire field
- e2e (real Run + FakeChatModel + LazyMongoSaver + supertest resume): tool-body
  interrupt pauses durably with NO approval policy, answer round-trips as the
  ToolMessage content, tool body re-runs once on resume, sequential questions
  re-pause
danny-avila added a commit that referenced this pull request Jul 7, 2026
A resumed run rebuilds the graph from the checkpoint, and the fresh graph
numbers content indices from its own empty contentData — starting at 0. The
resume path seeds the (also fresh) content aggregator with the pre-pause
parts at exactly those indices, so the resumed model turn collided with the
seed: type-matching parts silently MERGED (post-resume text appended into a
pre-pause text block), and type-mismatching parts (a reasoning/think part at
index 0 — any Anthropic reasoning agent) dropped EVERY delta with 'Content
type mismatch', losing the entire post-resume output from the live stream
and the saved message.

Latent since #13942 — tool-approval resumes corrupt content the same way
(probe-verified); it surfaced now because ask_user_question makes pausing a
first-class flow and reasoning models make the loss total.

- createContentIndexOffsetHandlers(handlers, offset): wraps ON_RUN_STEP
  (the single point where a content index enters the pipeline — deltas
  resolve through the aggregator's stepMap) and ON_AGENT_UPDATE's inline
  index; every other handler passes through by reference. Probe-validated:
  resumed output now lands as a new part after the paused tool call.
- resumeCompletion wires it with offset = seedContent.length.
- logToolError: a GraphInterrupt unwinding out of a tool body is the HITL
  pause working as designed — no longer logged as a Tool Error.
danny-avila added a commit that referenced this pull request Jul 8, 2026
…use/resume (#14139)

* feat: ask_user_question tool — agent-initiated questions with durable pause/resume

The HITL runtime merged in #13942/#14024/#14025/#14123 already ships the full
ask_user_question lifecycle (payload-agnostic handleRunInterrupt, resume
validation via mapAskUserAnswer, reconnect rehydration, and the client question
card) — but nothing ever raised the interrupt. This adds the producer:

- packages/api/agents/hitl/askUserQuestionTool.ts: LLM-callable tool whose func
  calls the SDK askUserQuestion() helper (LangGraph interrupt() from the tool
  body); zod schema with length caps mirroring AskUserQuestionRequest, plus a
  JSON-schema twin for the schema-only registry
- Registration: agentToolDefinitions, manifest.json (Tools dialog, admin
  filteredTools/includedTools kill switch), basicToolInstances, handleTools
  constructor branch
- run.ts gating: checkpointer now attaches for hitlCapable runs whose agents
  carry the ask tool even with the tool-approval policy disabled (the interrupt
  needs only durability, not humanInTheLoop/hooks); the tool is stripped
  fail-closed from non-HITL callers (OpenAI-compat/Responses) and subagent
  child configs; excluded from eager event execution (interrupts must be
  raised inside the Pregel task frame)
- resume.js: 16k length cap on the answer wire field
- e2e (real Run + FakeChatModel + LazyMongoSaver + supertest resume): tool-body
  interrupt pauses durably with NO approval policy, answer round-trips as the
  ToolMessage content, tool body re-runs once on resume, sequential questions
  re-pause

* fix: adversarial-review findings — in-graph execution, orphan prunes, endpoint scoping, real kill switch

Pre-PR multi-agent review confirmed 5 defects in the initial commit; all fixed:

1. CRITICAL — the tool never paused on the real agents endpoint: production
   loads tools definitions-only, flipping the SDK ToolNode to event-driven
   dispatch, and the host ON_TOOL_EXECUTE handler runs outside the Pregel task
   frame (under runOutsideTracing), where interrupt() throws and becomes an
   error ToolMessage. Reworked: the ask tool never rides toolDefinitions/
   toolRegistry — on HITL-capable top-level agents a real instance is supplied
   via AgentInputs.graphTools (agents#289, requires @librechat/agents > 3.2.57),
   the SDK's in-graph direct-tool seam; new production-shape e2e pins the
   event-driven mode end to end.
2. CRITICAL — ask-only runs left orphaned interrupted checkpoints (silent
   context duplication on every later turn): both orphan prunes were gated on
   toolApproval.enabled. The pre-turn prune now also fires for ask-capable
   agents (exported agentRequestsAskUserQuestion), and the abort-route prune
   fires when the aborted job carries a pendingAction.
3. MAJOR — self-spawned subagents bypassed the strip (self config resolves from
   the parent's _sourceInputs): fixed SDK-side (buildChildInputs clears
   graphTools) and the tool is now never present on child surfaces host-side.
4. MINOR — the manifest entry leaked into the Assistants tools dialog and the
   legacy plugins endpoint, where tools execute with no run to pause: new
   agentsOnly manifest flag, scoped out of both listings.
5. MINOR — filteredTools/includedTools only hid the tool from the dialog:
   now enforced at run build (strip + no checkpointer), making the admin
   filter a real kill switch for already-saved agents.

* chore: update @librechat/agents dependency to version 3.2.58 in package-lock.json and package.json files

* fix: reject agents-only tools at assistant create/update (Codex round 1)

The tools-dialog scoping keeps ask_user_question out of the assistants
LISTING, but the v1/v2 create/update handlers resolve arbitrary posted tool
strings from the shared getCachedTools map — a REST client or stale saved
payload could still attach it, and the assistants runtime executes tools with
no run to pause, so every call would error. New isAgentsOnlyTool(tool)
(manifest-driven, handles string and function-object shapes) drops such tools
with a warn at all four resolution sites (v1+v2, create+update).

* fix: offset resumed-run content indices past the pre-pause seed

A resumed run rebuilds the graph from the checkpoint, and the fresh graph
numbers content indices from its own empty contentData — starting at 0. The
resume path seeds the (also fresh) content aggregator with the pre-pause
parts at exactly those indices, so the resumed model turn collided with the
seed: type-matching parts silently MERGED (post-resume text appended into a
pre-pause text block), and type-mismatching parts (a reasoning/think part at
index 0 — any Anthropic reasoning agent) dropped EVERY delta with 'Content
type mismatch', losing the entire post-resume output from the live stream
and the saved message.

Latent since #13942 — tool-approval resumes corrupt content the same way
(probe-verified); it surfaced now because ask_user_question makes pausing a
first-class flow and reasoning models make the loss total.

- createContentIndexOffsetHandlers(handlers, offset): wraps ON_RUN_STEP
  (the single point where a content index enters the pipeline — deltas
  resolve through the aggregator's stepMap) and ON_AGENT_UPDATE's inline
  index; every other handler passes through by reference. Probe-validated:
  resumed output now lands as a new part after the paused tool call.
- resumeCompletion wires it with offset = seedContent.length.
- logToolError: a GraphInterrupt unwinding out of a tool body is the HITL
  pause working as designed — no longer logged as a Tool Error.

* fix: unblock live streaming of the resumed segment after an answer

With resume indices now ABSOLUTE (server continues after the pre-pause
parts), the synthetic ask-user-question card was squatting on exactly the
index the resumed segment streams into: applyAskUserQuestion appends the
card at the end of the message content, so on the answering device every
incoming part at that index was blocked and nothing rendered between the
answer submission and the finalize replacing the message.

removeAskUserQuestionPart(message, actionId) strips the pause-scoped card
on successful answer submission (useResumeSubmit onSuccess) — the durable
record of the Q&A is the ask_user_question tool call itself. Pure helper +
specs; same-reference no-op when nothing matches.

* fix: displace the synthetic question card in the streaming content writer

The store-level strip on answer submit wasn't enough: the SSE step handler
keeps its own in-flight copy of the streaming message, so on the answering
device the synthetic ask-user-question card still occupied the ABSOLUTE index
the resumed segment streams into — every delta warned 'Content type mismatch'
(existing ask_user_question vs incoming text) and nothing rendered between the
pending_action and finalize.

Displace the card inside updateContent when any real part claims its slot —
the same displacement pattern as the OAuth prompt part directly above it.
Covers the streaming handler's own copy, reconnecting tabs, and other devices;
once real content streams, the pause is over by definition. Spec drives a
runStep + text delta into the card's index and pins: no mismatch warn, card
gone, text rendered.

* feat: dedicated UI + durable data for completed ask_user_question calls

The completed ask call rendered as a generic tool card labeled 'Cancelled'
with raw (and empty) JSON args. Two layers fixed:

Data: the saved tool_call part had args:'' and no output — streamed arg
chunks carry no tool name so the aggregator drops them (normal tools recover
via the completion event, which never fires for a tool that interrupts
mid-execution and resumes on a rebuilt run with no step id). The resume
controller now stamps the paused ask part with the pendingAction's
authoritative question as args and the user's answer as output
(attachAskUserQuestionAnswer — pure, targets the newest unanswered ask part,
so sequential questions each keep their own answer).

UI: Part.tsx routes ask_user_question tool calls to AskUserQuestionCall — a
compact Q&A record ('Asked a question' header, question, description, 'You
answered: <label>' preferring the picked option's label, or 'No answer was
given' for an abandoned pause) instead of the generic card. New i18n keys;
parseAskUserQuestionArgs degrades to null on malformed model args.

* fix: single question UI per pause + immediate answer display

Two live-turn issues with the new durable Q&A card:

1. Duplicate question on ask: during a live pause the message carries BOTH the
   ask tool_call part (now rendered by AskUserQuestionCall, showing a
   misleading 'No answer was given' while paused) and the synthetic
   interactive card. The durable card now defers while the turn is live and
   unanswered (isSubmitting) — the interactive card owns the question UI until
   it's answered; an abandoned pause still shows its no-answer state once the
   turn settles.

2. 'No answer was given' after answering: the server stamps the answer onto
   the part at resume seed, but the client only received that at finalize. No
   stream emission needed — the client knows the answer it just submitted:
   resolveAskUserQuestionPart (replacing the plain strip on submit success)
   removes the synthetic card AND stamps output/progress onto the newest
   unanswered ask tool_call, seeding args from the synthetic part's question
   when the streamed args were lost — mirroring the server-side
   attachAskUserQuestionAnswer, so the Q&A record shows the answer the moment
   the user submits.

* fix: keep the Q&A record visible while the resumed segment streams

The optimistic output stamp lives in the message store, but the SSE step
handler evolves its own cached copy of the streaming message (created at turn
start) — the first resumed event overwrites the store with that copy, wiping
the stamp, so the Q&A card blinked out during streaming and only returned at
finalize.

Render-layer fallback instead of fighting the handler's copy: submitted
answers are recorded by ask tool_call id when resolveAskUserQuestionPart
stamps the part, and AskUserQuestionCall reads the recorded answer whenever
the part's own output is missing — the record survives any message-copy churn
until finalize delivers the server-stamped part.

* feat: present Ask User as a native builtin in the tools dialog

It ships with the app and pauses the run like a first-class feature, so it
belongs with the builtins (Run Code, Web Search, Memory, ...) rather than in
the third-party plugin list — while its mechanics stay exactly a plugin's:

- BuiltinId += 'ask_user_question' (documented exception: a native TOOL, not
  a capability; selection reads agent.tools, the toggle emits tool-add/remove
  patches instead of a capability field)
- buildCatalog surfaces it as a builtin gated on the same signals as before
  (tools capability on + the server lists the plugin, i.e. not admin-filtered)
  and skips it in the plugin loop so it never double-lists
- On-theme icon: lucide MessageCircleQuestion in a teal chip via the builtin
  icon map, matching the other native entries; the bespoke purple SVG and the
  manifest icon field are gone
- i18n'd name/description keys like the other builtins

* feat: composer popover for answering questions (mentions-style)

Answering moves to the composer, matching the existing mentions/prompts
popover pattern: while an ask_user_question pause is live, a popover anchors
above the textarea with the question as its header, numbered option rows
(hover/click, or ↑/↓ + Enter from the empty composer), and an × to dismiss.
The main textarea doubles as the free-form answer — its placeholder flips to
'Something else...' and form submit routes the text to the paused run as the
answer instead of starting a new turn. Dismissing (× or Escape) restores
normal sends; the inline transcript surfaces stay as before (interactive card
while paused, durable Q&A record after) so the question remains visible in
history.

- findLiveAskUserQuestion (pure, spec'd): newest unanswered synthetic part
  across the conversation IS the popover signal — applied on
  on_pending_action, stripped on answer submit, so visibility tracks the
  pause lifecycle with no extra state
- useLiveAskUserQuestion hook shared by the popover and ChatForm; dismissals
  in a recoil atom so both react
- popover only mounts on the primary composer (index 0), mirroring QuoteButton

* feat: number-key selection + return glyph in the question popover

Pressing 1-9 in the empty composer picks the matching option directly,
mirroring the numbered row chips; the highlighted row shows a return-key
glyph as the Enter affordance. Same empty-composer guard as the arrow keys —
typing a free-form answer is never intercepted.

* refactor: first-class composer answer mode (useAskAnswerMode)

Replaces the bolted-on integration (inline onSubmit interception + raw
capture-phase keydown listeners on the textarea ref) with a single hook that
owns the whole answer mode: live-question derivation, dismissal + highlighted
option (shared recoil state), option selection, free-form submit routing
(submitText returns whether it consumed the submission), and keyboard
handling (handleKeyDown returns whether it consumed the key, composed ahead
of the textarea's normal handler — no more addEventListener).

The popover is now pure rendering off the hook; ChatForm wires placeholder,
onKeyDown, and onSubmit through the same instance. Deliberately scoped to the
composer rather than useSubmitMessage: starters/prompt-commands keep new-turn
semantics (and the existing job-replacement behavior while paused).

* fix: Codex round 2 — inline answer input, approval exemption, pause-time args

F1 (composer submit unreachable while paused — isSubmitting keeps Stop shown
and useTextarea eats Enter): redesigned around it, borrowing Claude Code's
AskUserQuestion semantics. The popover now owns free-form input via an inline
'Other' row (numbered last, 'Something else…'), with select-then-confirm rows
(click/arrows/digits highlight; Submit ↵, Enter, or double-click fires; Skip
dismisses). The composer returns to being a plain composer — no placeholder
swap, no submit interception; Stop keeps meaning stop.

F2: ask_user_question is exempt from the tool-approval prompt unless the
admin explicitly lists it (allow/ask/deny all win) — approving the right to
ask a question was a pure double pause; the tool is side-effect-free.

F3: the question is stamped onto the paused ask tool_call's args at PAUSE
time (attachAskUserQuestionArgs in handleRunInterrupt), so abandoned/expired/
stopped turns persist with the question intact and the record card can render
it — previously only the answer-resume path stamped args.

* fix: fold model-supplied 'Other' options into the inline free-form row

The model can generate its own catch-all option ('Other (type your own)',
value 'other'), duplicating the popover's built-in free-form row — two
other-ish rows, one pickable as a literal answer. Two layers:

- Tool description now tells the model NOT to include catch-all options (the
  answer UI always offers free-form input on its own)
- splitOtherOption (pure, spec'd) folds a catch-all option that arrives
  anyway out of the choice rows and uses its label as the inline input's
  placeholder — conservative match (value 'other', or a label reading as a
  free-form invitation), no false positives on real choices

* fix: single question surface + clean free-form-only popover

Two live-pause confusions: (1) the inline transcript card and the composer
popover both rendered — the card now defers while the popover is up for its
action, returning as the fallback surface when the user dismisses the popover
(and in contexts without a ChatContext, where the popover can't exist);
(2) an options-less question showed a pointless numbered '1 Something else…'
row — free-form-only questions now render the inline input alone, with the
'Type your answer…' placeholder (a folded model 'Other' label still wins).

* feat: the composer is the free-form answer box (like the main chat input)

While a question pause is live, the main chat textarea composes the free-form
answer — placeholder swaps to 'Something else…' (or a folded model 'Other'
label), Enter with text submits the answer through answer-mode key handling
(composed BEFORE useTextarea's submitting-lock, so the lock can't swallow it),
and the Stop button swaps to Send (enabled despite isSubmitting) per the
select-then-confirm design. The popover slims to the question header, numbered
option rows, and Skip/Submit — its inline input is gone since the composer
owns free-form now. Dismissing the popover restores normal composer semantics
(Stop button, normal sends).

* fix: Codex round 3 + real Skip semantics

- Skip now ANSWERS instead of hiding UI (danny): it resumes the run with a
  decline notice ('The user chose not to answer this question.') so the model
  moves on — a client-side dismiss left the run paused until expiry, a hung
  turn. × / Escape remain pure dismiss (switch to the inline card surface).
- P1 (resumed approval tool indices): resumed tool_calls steps whose
  tool_call id matches a seeded UNRESOLVED part now rebind to that seeded
  slot instead of offsetting — the original part resolves in place (output
  attaches) and no duplicate appears; message steps keep the offset, so the
  text-loss fix stands. createContentIndexOffsetHandlers now takes the seed
  array; resolved seeded calls are not rebind targets.
- P2 (stale selection across questions): selection state resets when the
  live actionId changes; the vestigial inline-Other state ('other' selection
  + text atom) is gone — the composer owns free-form.
- P2 (Redis abort path loses the args stamp): the abort route re-stamps the
  question onto the ask tool_call in the reconstructed abort content, so a
  Stop-abandoned question persists with its question intact.
- P2 (malformed args crash): parseAskUserQuestionArgs normalizes untrusted
  shapes (options: {} / non-string entries) instead of throwing in render.

* feat: free-form hint in the question popover footer

Left-aligned in the footer row (opposite Skip/Submit): 'Or type your answer
below' — points open-ended answering at the composer, whose placeholder
already reads 'Something else…'.

* feat: preserve composer drafts across the answer-mode swap

The answer phase gets its own draft key (ask-answer:<actionId>), passed as a
draftId override into useAutoSave — the key change itself drives the existing
save/restore machinery, so the conversation draft (or mid-run PENDING draft)
is stashed when a question pause takes the composer and restored once the
user answers, skips, or dismisses. Ask keys are exempt from the PENDING
migration branch, which would otherwise move-and-delete the stashed draft. A
half-typed answer survives reload/navigation while its question stays live.
Answer submission (option pick, free-form, skip) resets the composer via a
new non-throwing useOptionalChatFormContext, so the swap-back restores into
an empty box even outside ChatView-less render contexts (Share/search).

* fix: rebind resumed steps for ALL seeded tool call ids

The resume controller pre-stamps the user's answer onto the seeded
ask_user_question part, so the unresolved-only rebind predicate treated
it as settled and shifted the tool's re-run step to a fresh offset slot,
leaving a duplicate ask record in streamed/saved content. Tool call ids
are provider-minted per call: a resumed step bearing a seeded id can
only be the interrupted batch re-executing, so rebinding every seeded
id is always correct.

* feat: popover UX round 4 — clickable hint, collapse, click-submit, multiSelect

- Footer hint is a button that focuses the composer; reads 'Type your answer
  below' (no 'Or') when the question has no options.
- Collapse (chevron) hides the popover WITHOUT closing the pause: answer mode
  stays live (placeholder, Enter routing, draft key), the chat card renders
  the question with a ChevronUp affordance to re-expand. x remains dismiss.
- Single-select options submit on a single click; the Submit button renders
  only for multi-select.
- multiSelect end-to-end: tool zod schema + JSON definition twin, wire type,
  client parse, popover check-chips, card toggles, record-card label mapping;
  answer = option values joined ', '; composer Enter and the multi Submit
  button both fold free-form text in with the checked values.
- Hardening from adversarial review: in-flight status guard on every submit
  path (no duplicate resumes on double-click), popover locks while
  submitting, collapsed mode disarms invisible digit/arrow steering, the
  card shares the hook's checked state while the pause is live, the card
  folds catch-all 'Other' options, record mapping is all-or-nothing to avoid
  phantom labels, composer resets only when its text was consumed or the
  draft machinery will restore the stash.

* feat: ask_user_question in model specs and ephemeral agents

A librechat.yaml modelSpec can now equip the tool the same way it equips
webSearch/executeCode/fileSearch/memory:

  modelSpecs:
    list:
      - name: my-spec
        askUserQuestion: true

loadEphemeralAgent pushes the tool name when the spec flag (or the
ephemeralAgent request flag, wired for parity) is set; everything downstream
is the existing persisted-agent machinery — createRun's hitlCapable gating,
graphTools injection, checkpointer attach, subagent strip, and the admin
filteredTools/includedTools kill switch all apply unchanged.

* feat: tense-aware Q&A record label (Asking / Asked)

Shorten the record card header per feedback: 'Asking' while the question is
still unanswered (abandoned/awaiting), 'Asked' once answered — replacing the
single 'Asked a question' label.

* fix: Codex round 4 — added-agent ask parity + preserve answer on failed resume

F1 (added.ts): mirror loadEphemeralAgent's ask_user_question branch in the
added-agent loader so a model spec's askUserQuestion flag (or the ephemeral
request flag) equips added top-level agents too, matching execute_code /
web_search / memory. Two load.spec cases added.

F3 (composer): submitAskAnswer now takes an onSuccess callback and
useAskAnswerMode defers clearing the selection/composer until the resume is
accepted. A failed resume (16k answer-cap 400, expired action, network error)
leaves status re-answerable, so wiping the composer up front lost the user's
only copy of a free-form answer; now it survives for trim/retry.

(F2 — a claimed Tools-capability bypass — was verified NOT reproducible:
agentRequestsAskUserQuestion matches only loaded instances/toolDefinitions/
toolRegistry, all capability-filtered; a raw tools string has no .name and
never triggers the install. Replied on-thread with the probe evidence.)

* fix: Codex round 5 — expired question exits answer mode so its message shows

An expired question (e.g. resume returns the stale-action 409) previously left
the popover open with locked controls and no explanation, because the chat
card — which carries the only 'this action expired' message — was suppressed
by the popover-open guard. Treat 'expired' as no longer active: the popover
closes, the composer reverts to normal, and the card becomes the sole surface
and renders the expired message. 'error' stays active (retryable).

* feat: group ask_user_question calls as their own category

A homogeneous group of ask_user_question tool calls now reads 'Asked N
questions' (present tense 'Asking N questions' while the turn streams) with a
question glyph and no raw-name suffix — mirroring the subagent 'Ran N agents'
category treatment, instead of 'Used N tools — ask_user_question'. Mixed
groups keep 'Used N tools' but humanize the suffix to 'Question' and show a
question icon for the ask entries (TOOL_FRIENDLY_NAME_KEYS + ToolIcon map).
A group only forms at count >= 2, so the plural is always grammatical.
Three ToolCallGroup.test cases cover homogeneous label/icon/suffix, present
tense while streaming, and the mixed-group fallback.

* fix: Codex round 6 — composer submit lock + abort stamp before emit

F7 (composer status lock): the ask submit status lived on ApprovalContext,
a React context mounted only around message content (ContentParts). The
PRIMARY answer surface — the composer in ChatForm — renders outside it, so
useApprovalContext returned the inert FALLBACK: status was always 'idle',
setStatus a no-op. The in-flight double-submit guard (round 4) and the
expired-exits-answer-mode fix (round 5) therefore never engaged for the
composer. Move ask submit status to a global Recoil atom (useAskSubmitStatus)
read/written by the composer, the popover, and the card alike, so a fast
double-click/Enter is actually blocked and expired/error surfaces on every
surface. Tool-approval status stays on the context (unchanged).

F5 (abort stamp before emit): the abort route re-stamped a paused
ask_user_question's args AFTER GenerationJobManager.abortJob had already
emitted the final SSE from the unstamped content, so a Redis/cross-replica
Stop left the live client showing an empty question until reload. abortJob
now takes an optional transformAbortContent applied to the persistable
content BEFORE the final event is built (and returned), so the live client
and the saved message agree. New abort.spec case + updated call assertions.

* feat: gate ask_user_question behind its own agent capability

Add a first-class AgentCapabilities.ask_user_question (in defaultAgentCapabilities,
on by default) so admins can enable/disable questions independently via
endpoints.agents.capabilities, exactly like execute_code / web_search — not
lumped under the generic tools capability.

- ToolService: both filteredTools predicates (definitions-only and instance
  loaders) gate ask_user_question on checkCapability(ask_user_question) before
  the generic tools fallthrough. When off, the tool is dropped from
  toolDefinitions/toolRegistry, so run.ts's agentRequestsAskUserQuestion (which
  keys on the loaded surface) declines to install it and attach a checkpointer —
  the capability is enforced end-to-end at the loader, no run.ts change needed.
- Tools dialog catalog: surface the ask builtin under its own capability rather
  than the generic tools one, so the UI matches the backend gate.
- Tests: ToolService capability on/off filtering + defaults membership; catalog
  builtin visibility keyed on the dedicated capability.

* style: sort imports in ToolCallGroup.test (CI import-order gate)

* fix: Codex round 7 — surface ask-answer errors in the open popover

A failed answer submission (16k reject, network error) sets the ask status to
'error', which — unlike 'expired' — deliberately keeps the question active and
retryable. But the chat card that renders the error message is suppressed while
the popover is open, so a composer/popover answer failed silently. Expose an
'errored' flag from useAskAnswerMode and render a warning line
(com_ui_ask_answer_error) in the popover, so the user gets feedback and retry
guidance without having to collapse/dismiss. It clears automatically on retry
(status flips to 'submitting').

* fix: Codex round 8 — respect IME composition before submitting answers

handleComposerKeyDown runs before useTextarea's composition guard, so with a
CJK/IME keyboard the Enter that commits an in-progress composition was being
intercepted and submitting the partial answer (and the composition buffer can
leave value empty mid-compose, mis-triggering digit/arrow steering too). Bail
at the top when composing — nativeEvent.isComposing, or key==='Process' /
keyCode===229 for Safari's inconsistent reporting — mirroring the existing
composer guard so the character commits normally.

* chore: update `@librechat/agents` to v3.2.60

* 🔧 chore: Update @opentelemetry/core to version 2.9.0 and clean up package-lock.json

* feat: digit shortcuts select options when the popover has focus

Previously a number key (1..N) only selected an option from the empty
composer (handleComposerKeyDown on the textarea) — if focus moved into the
popover (a row/Skip/Submit button clicked or tabbed to), the number keys went
dead. Add handlePopoverKeyDown, wired to the popover container's onKeyDown so
it catches digits bubbling from the focused control: a digit activates its
option exactly like a click (single-select submits, multi toggles). No
highlight/Enter dance on this path — the options are buttons whose action is
the click, and intercepting Enter would fight the focused button. Gated on
active && !locked so it no-ops while a submit is in flight.

* chore: update @librechat/agents to version 3.2.61 and @opentelemetry packages to latest versions
ThomasVuNguyen pushed a commit to ThomasVuNguyen/LibreChat that referenced this pull request Jul 15, 2026
… (Slice B) (LibreChat-AI#13942)

* 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)
ThomasVuNguyen pushed a commit to ThomasVuNguyen/LibreChat that referenced this pull request Jul 15, 2026
…use/resume (LibreChat-AI#14139)

* feat: ask_user_question tool — agent-initiated questions with durable pause/resume

The HITL runtime merged in LibreChat-AI#13942/LibreChat-AI#14024/LibreChat-AI#14025/LibreChat-AI#14123 already ships the full
ask_user_question lifecycle (payload-agnostic handleRunInterrupt, resume
validation via mapAskUserAnswer, reconnect rehydration, and the client question
card) — but nothing ever raised the interrupt. This adds the producer:

- packages/api/agents/hitl/askUserQuestionTool.ts: LLM-callable tool whose func
  calls the SDK askUserQuestion() helper (LangGraph interrupt() from the tool
  body); zod schema with length caps mirroring AskUserQuestionRequest, plus a
  JSON-schema twin for the schema-only registry
- Registration: agentToolDefinitions, manifest.json (Tools dialog, admin
  filteredTools/includedTools kill switch), basicToolInstances, handleTools
  constructor branch
- run.ts gating: checkpointer now attaches for hitlCapable runs whose agents
  carry the ask tool even with the tool-approval policy disabled (the interrupt
  needs only durability, not humanInTheLoop/hooks); the tool is stripped
  fail-closed from non-HITL callers (OpenAI-compat/Responses) and subagent
  child configs; excluded from eager event execution (interrupts must be
  raised inside the Pregel task frame)
- resume.js: 16k length cap on the answer wire field
- e2e (real Run + FakeChatModel + LazyMongoSaver + supertest resume): tool-body
  interrupt pauses durably with NO approval policy, answer round-trips as the
  ToolMessage content, tool body re-runs once on resume, sequential questions
  re-pause

* fix: adversarial-review findings — in-graph execution, orphan prunes, endpoint scoping, real kill switch

Pre-PR multi-agent review confirmed 5 defects in the initial commit; all fixed:

1. CRITICAL — the tool never paused on the real agents endpoint: production
   loads tools definitions-only, flipping the SDK ToolNode to event-driven
   dispatch, and the host ON_TOOL_EXECUTE handler runs outside the Pregel task
   frame (under runOutsideTracing), where interrupt() throws and becomes an
   error ToolMessage. Reworked: the ask tool never rides toolDefinitions/
   toolRegistry — on HITL-capable top-level agents a real instance is supplied
   via AgentInputs.graphTools (agents#289, requires @librechat/agents > 3.2.57),
   the SDK's in-graph direct-tool seam; new production-shape e2e pins the
   event-driven mode end to end.
2. CRITICAL — ask-only runs left orphaned interrupted checkpoints (silent
   context duplication on every later turn): both orphan prunes were gated on
   toolApproval.enabled. The pre-turn prune now also fires for ask-capable
   agents (exported agentRequestsAskUserQuestion), and the abort-route prune
   fires when the aborted job carries a pendingAction.
3. MAJOR — self-spawned subagents bypassed the strip (self config resolves from
   the parent's _sourceInputs): fixed SDK-side (buildChildInputs clears
   graphTools) and the tool is now never present on child surfaces host-side.
4. MINOR — the manifest entry leaked into the Assistants tools dialog and the
   legacy plugins endpoint, where tools execute with no run to pause: new
   agentsOnly manifest flag, scoped out of both listings.
5. MINOR — filteredTools/includedTools only hid the tool from the dialog:
   now enforced at run build (strip + no checkpointer), making the admin
   filter a real kill switch for already-saved agents.

* chore: update @librechat/agents dependency to version 3.2.58 in package-lock.json and package.json files

* fix: reject agents-only tools at assistant create/update (Codex round 1)

The tools-dialog scoping keeps ask_user_question out of the assistants
LISTING, but the v1/v2 create/update handlers resolve arbitrary posted tool
strings from the shared getCachedTools map — a REST client or stale saved
payload could still attach it, and the assistants runtime executes tools with
no run to pause, so every call would error. New isAgentsOnlyTool(tool)
(manifest-driven, handles string and function-object shapes) drops such tools
with a warn at all four resolution sites (v1+v2, create+update).

* fix: offset resumed-run content indices past the pre-pause seed

A resumed run rebuilds the graph from the checkpoint, and the fresh graph
numbers content indices from its own empty contentData — starting at 0. The
resume path seeds the (also fresh) content aggregator with the pre-pause
parts at exactly those indices, so the resumed model turn collided with the
seed: type-matching parts silently MERGED (post-resume text appended into a
pre-pause text block), and type-mismatching parts (a reasoning/think part at
index 0 — any Anthropic reasoning agent) dropped EVERY delta with 'Content
type mismatch', losing the entire post-resume output from the live stream
and the saved message.

Latent since LibreChat-AI#13942 — tool-approval resumes corrupt content the same way
(probe-verified); it surfaced now because ask_user_question makes pausing a
first-class flow and reasoning models make the loss total.

- createContentIndexOffsetHandlers(handlers, offset): wraps ON_RUN_STEP
  (the single point where a content index enters the pipeline — deltas
  resolve through the aggregator's stepMap) and ON_AGENT_UPDATE's inline
  index; every other handler passes through by reference. Probe-validated:
  resumed output now lands as a new part after the paused tool call.
- resumeCompletion wires it with offset = seedContent.length.
- logToolError: a GraphInterrupt unwinding out of a tool body is the HITL
  pause working as designed — no longer logged as a Tool Error.

* fix: unblock live streaming of the resumed segment after an answer

With resume indices now ABSOLUTE (server continues after the pre-pause
parts), the synthetic ask-user-question card was squatting on exactly the
index the resumed segment streams into: applyAskUserQuestion appends the
card at the end of the message content, so on the answering device every
incoming part at that index was blocked and nothing rendered between the
answer submission and the finalize replacing the message.

removeAskUserQuestionPart(message, actionId) strips the pause-scoped card
on successful answer submission (useResumeSubmit onSuccess) — the durable
record of the Q&A is the ask_user_question tool call itself. Pure helper +
specs; same-reference no-op when nothing matches.

* fix: displace the synthetic question card in the streaming content writer

The store-level strip on answer submit wasn't enough: the SSE step handler
keeps its own in-flight copy of the streaming message, so on the answering
device the synthetic ask-user-question card still occupied the ABSOLUTE index
the resumed segment streams into — every delta warned 'Content type mismatch'
(existing ask_user_question vs incoming text) and nothing rendered between the
pending_action and finalize.

Displace the card inside updateContent when any real part claims its slot —
the same displacement pattern as the OAuth prompt part directly above it.
Covers the streaming handler's own copy, reconnecting tabs, and other devices;
once real content streams, the pause is over by definition. Spec drives a
runStep + text delta into the card's index and pins: no mismatch warn, card
gone, text rendered.

* feat: dedicated UI + durable data for completed ask_user_question calls

The completed ask call rendered as a generic tool card labeled 'Cancelled'
with raw (and empty) JSON args. Two layers fixed:

Data: the saved tool_call part had args:'' and no output — streamed arg
chunks carry no tool name so the aggregator drops them (normal tools recover
via the completion event, which never fires for a tool that interrupts
mid-execution and resumes on a rebuilt run with no step id). The resume
controller now stamps the paused ask part with the pendingAction's
authoritative question as args and the user's answer as output
(attachAskUserQuestionAnswer — pure, targets the newest unanswered ask part,
so sequential questions each keep their own answer).

UI: Part.tsx routes ask_user_question tool calls to AskUserQuestionCall — a
compact Q&A record ('Asked a question' header, question, description, 'You
answered: <label>' preferring the picked option's label, or 'No answer was
given' for an abandoned pause) instead of the generic card. New i18n keys;
parseAskUserQuestionArgs degrades to null on malformed model args.

* fix: single question UI per pause + immediate answer display

Two live-turn issues with the new durable Q&A card:

1. Duplicate question on ask: during a live pause the message carries BOTH the
   ask tool_call part (now rendered by AskUserQuestionCall, showing a
   misleading 'No answer was given' while paused) and the synthetic
   interactive card. The durable card now defers while the turn is live and
   unanswered (isSubmitting) — the interactive card owns the question UI until
   it's answered; an abandoned pause still shows its no-answer state once the
   turn settles.

2. 'No answer was given' after answering: the server stamps the answer onto
   the part at resume seed, but the client only received that at finalize. No
   stream emission needed — the client knows the answer it just submitted:
   resolveAskUserQuestionPart (replacing the plain strip on submit success)
   removes the synthetic card AND stamps output/progress onto the newest
   unanswered ask tool_call, seeding args from the synthetic part's question
   when the streamed args were lost — mirroring the server-side
   attachAskUserQuestionAnswer, so the Q&A record shows the answer the moment
   the user submits.

* fix: keep the Q&A record visible while the resumed segment streams

The optimistic output stamp lives in the message store, but the SSE step
handler evolves its own cached copy of the streaming message (created at turn
start) — the first resumed event overwrites the store with that copy, wiping
the stamp, so the Q&A card blinked out during streaming and only returned at
finalize.

Render-layer fallback instead of fighting the handler's copy: submitted
answers are recorded by ask tool_call id when resolveAskUserQuestionPart
stamps the part, and AskUserQuestionCall reads the recorded answer whenever
the part's own output is missing — the record survives any message-copy churn
until finalize delivers the server-stamped part.

* feat: present Ask User as a native builtin in the tools dialog

It ships with the app and pauses the run like a first-class feature, so it
belongs with the builtins (Run Code, Web Search, Memory, ...) rather than in
the third-party plugin list — while its mechanics stay exactly a plugin's:

- BuiltinId += 'ask_user_question' (documented exception: a native TOOL, not
  a capability; selection reads agent.tools, the toggle emits tool-add/remove
  patches instead of a capability field)
- buildCatalog surfaces it as a builtin gated on the same signals as before
  (tools capability on + the server lists the plugin, i.e. not admin-filtered)
  and skips it in the plugin loop so it never double-lists
- On-theme icon: lucide MessageCircleQuestion in a teal chip via the builtin
  icon map, matching the other native entries; the bespoke purple SVG and the
  manifest icon field are gone
- i18n'd name/description keys like the other builtins

* feat: composer popover for answering questions (mentions-style)

Answering moves to the composer, matching the existing mentions/prompts
popover pattern: while an ask_user_question pause is live, a popover anchors
above the textarea with the question as its header, numbered option rows
(hover/click, or ↑/↓ + Enter from the empty composer), and an × to dismiss.
The main textarea doubles as the free-form answer — its placeholder flips to
'Something else...' and form submit routes the text to the paused run as the
answer instead of starting a new turn. Dismissing (× or Escape) restores
normal sends; the inline transcript surfaces stay as before (interactive card
while paused, durable Q&A record after) so the question remains visible in
history.

- findLiveAskUserQuestion (pure, spec'd): newest unanswered synthetic part
  across the conversation IS the popover signal — applied on
  on_pending_action, stripped on answer submit, so visibility tracks the
  pause lifecycle with no extra state
- useLiveAskUserQuestion hook shared by the popover and ChatForm; dismissals
  in a recoil atom so both react
- popover only mounts on the primary composer (index 0), mirroring QuoteButton

* feat: number-key selection + return glyph in the question popover

Pressing 1-9 in the empty composer picks the matching option directly,
mirroring the numbered row chips; the highlighted row shows a return-key
glyph as the Enter affordance. Same empty-composer guard as the arrow keys —
typing a free-form answer is never intercepted.

* refactor: first-class composer answer mode (useAskAnswerMode)

Replaces the bolted-on integration (inline onSubmit interception + raw
capture-phase keydown listeners on the textarea ref) with a single hook that
owns the whole answer mode: live-question derivation, dismissal + highlighted
option (shared recoil state), option selection, free-form submit routing
(submitText returns whether it consumed the submission), and keyboard
handling (handleKeyDown returns whether it consumed the key, composed ahead
of the textarea's normal handler — no more addEventListener).

The popover is now pure rendering off the hook; ChatForm wires placeholder,
onKeyDown, and onSubmit through the same instance. Deliberately scoped to the
composer rather than useSubmitMessage: starters/prompt-commands keep new-turn
semantics (and the existing job-replacement behavior while paused).

* fix: Codex round 2 — inline answer input, approval exemption, pause-time args

F1 (composer submit unreachable while paused — isSubmitting keeps Stop shown
and useTextarea eats Enter): redesigned around it, borrowing Claude Code's
AskUserQuestion semantics. The popover now owns free-form input via an inline
'Other' row (numbered last, 'Something else…'), with select-then-confirm rows
(click/arrows/digits highlight; Submit ↵, Enter, or double-click fires; Skip
dismisses). The composer returns to being a plain composer — no placeholder
swap, no submit interception; Stop keeps meaning stop.

F2: ask_user_question is exempt from the tool-approval prompt unless the
admin explicitly lists it (allow/ask/deny all win) — approving the right to
ask a question was a pure double pause; the tool is side-effect-free.

F3: the question is stamped onto the paused ask tool_call's args at PAUSE
time (attachAskUserQuestionArgs in handleRunInterrupt), so abandoned/expired/
stopped turns persist with the question intact and the record card can render
it — previously only the answer-resume path stamped args.

* fix: fold model-supplied 'Other' options into the inline free-form row

The model can generate its own catch-all option ('Other (type your own)',
value 'other'), duplicating the popover's built-in free-form row — two
other-ish rows, one pickable as a literal answer. Two layers:

- Tool description now tells the model NOT to include catch-all options (the
  answer UI always offers free-form input on its own)
- splitOtherOption (pure, spec'd) folds a catch-all option that arrives
  anyway out of the choice rows and uses its label as the inline input's
  placeholder — conservative match (value 'other', or a label reading as a
  free-form invitation), no false positives on real choices

* fix: single question surface + clean free-form-only popover

Two live-pause confusions: (1) the inline transcript card and the composer
popover both rendered — the card now defers while the popover is up for its
action, returning as the fallback surface when the user dismisses the popover
(and in contexts without a ChatContext, where the popover can't exist);
(2) an options-less question showed a pointless numbered '1 Something else…'
row — free-form-only questions now render the inline input alone, with the
'Type your answer…' placeholder (a folded model 'Other' label still wins).

* feat: the composer is the free-form answer box (like the main chat input)

While a question pause is live, the main chat textarea composes the free-form
answer — placeholder swaps to 'Something else…' (or a folded model 'Other'
label), Enter with text submits the answer through answer-mode key handling
(composed BEFORE useTextarea's submitting-lock, so the lock can't swallow it),
and the Stop button swaps to Send (enabled despite isSubmitting) per the
select-then-confirm design. The popover slims to the question header, numbered
option rows, and Skip/Submit — its inline input is gone since the composer
owns free-form now. Dismissing the popover restores normal composer semantics
(Stop button, normal sends).

* fix: Codex round 3 + real Skip semantics

- Skip now ANSWERS instead of hiding UI (danny): it resumes the run with a
  decline notice ('The user chose not to answer this question.') so the model
  moves on — a client-side dismiss left the run paused until expiry, a hung
  turn. × / Escape remain pure dismiss (switch to the inline card surface).
- P1 (resumed approval tool indices): resumed tool_calls steps whose
  tool_call id matches a seeded UNRESOLVED part now rebind to that seeded
  slot instead of offsetting — the original part resolves in place (output
  attaches) and no duplicate appears; message steps keep the offset, so the
  text-loss fix stands. createContentIndexOffsetHandlers now takes the seed
  array; resolved seeded calls are not rebind targets.
- P2 (stale selection across questions): selection state resets when the
  live actionId changes; the vestigial inline-Other state ('other' selection
  + text atom) is gone — the composer owns free-form.
- P2 (Redis abort path loses the args stamp): the abort route re-stamps the
  question onto the ask tool_call in the reconstructed abort content, so a
  Stop-abandoned question persists with its question intact.
- P2 (malformed args crash): parseAskUserQuestionArgs normalizes untrusted
  shapes (options: {} / non-string entries) instead of throwing in render.

* feat: free-form hint in the question popover footer

Left-aligned in the footer row (opposite Skip/Submit): 'Or type your answer
below' — points open-ended answering at the composer, whose placeholder
already reads 'Something else…'.

* feat: preserve composer drafts across the answer-mode swap

The answer phase gets its own draft key (ask-answer:<actionId>), passed as a
draftId override into useAutoSave — the key change itself drives the existing
save/restore machinery, so the conversation draft (or mid-run PENDING draft)
is stashed when a question pause takes the composer and restored once the
user answers, skips, or dismisses. Ask keys are exempt from the PENDING
migration branch, which would otherwise move-and-delete the stashed draft. A
half-typed answer survives reload/navigation while its question stays live.
Answer submission (option pick, free-form, skip) resets the composer via a
new non-throwing useOptionalChatFormContext, so the swap-back restores into
an empty box even outside ChatView-less render contexts (Share/search).

* fix: rebind resumed steps for ALL seeded tool call ids

The resume controller pre-stamps the user's answer onto the seeded
ask_user_question part, so the unresolved-only rebind predicate treated
it as settled and shifted the tool's re-run step to a fresh offset slot,
leaving a duplicate ask record in streamed/saved content. Tool call ids
are provider-minted per call: a resumed step bearing a seeded id can
only be the interrupted batch re-executing, so rebinding every seeded
id is always correct.

* feat: popover UX round 4 — clickable hint, collapse, click-submit, multiSelect

- Footer hint is a button that focuses the composer; reads 'Type your answer
  below' (no 'Or') when the question has no options.
- Collapse (chevron) hides the popover WITHOUT closing the pause: answer mode
  stays live (placeholder, Enter routing, draft key), the chat card renders
  the question with a ChevronUp affordance to re-expand. x remains dismiss.
- Single-select options submit on a single click; the Submit button renders
  only for multi-select.
- multiSelect end-to-end: tool zod schema + JSON definition twin, wire type,
  client parse, popover check-chips, card toggles, record-card label mapping;
  answer = option values joined ', '; composer Enter and the multi Submit
  button both fold free-form text in with the checked values.
- Hardening from adversarial review: in-flight status guard on every submit
  path (no duplicate resumes on double-click), popover locks while
  submitting, collapsed mode disarms invisible digit/arrow steering, the
  card shares the hook's checked state while the pause is live, the card
  folds catch-all 'Other' options, record mapping is all-or-nothing to avoid
  phantom labels, composer resets only when its text was consumed or the
  draft machinery will restore the stash.

* feat: ask_user_question in model specs and ephemeral agents

A librechat.yaml modelSpec can now equip the tool the same way it equips
webSearch/executeCode/fileSearch/memory:

  modelSpecs:
    list:
      - name: my-spec
        askUserQuestion: true

loadEphemeralAgent pushes the tool name when the spec flag (or the
ephemeralAgent request flag, wired for parity) is set; everything downstream
is the existing persisted-agent machinery — createRun's hitlCapable gating,
graphTools injection, checkpointer attach, subagent strip, and the admin
filteredTools/includedTools kill switch all apply unchanged.

* feat: tense-aware Q&A record label (Asking / Asked)

Shorten the record card header per feedback: 'Asking' while the question is
still unanswered (abandoned/awaiting), 'Asked' once answered — replacing the
single 'Asked a question' label.

* fix: Codex round 4 — added-agent ask parity + preserve answer on failed resume

F1 (added.ts): mirror loadEphemeralAgent's ask_user_question branch in the
added-agent loader so a model spec's askUserQuestion flag (or the ephemeral
request flag) equips added top-level agents too, matching execute_code /
web_search / memory. Two load.spec cases added.

F3 (composer): submitAskAnswer now takes an onSuccess callback and
useAskAnswerMode defers clearing the selection/composer until the resume is
accepted. A failed resume (16k answer-cap 400, expired action, network error)
leaves status re-answerable, so wiping the composer up front lost the user's
only copy of a free-form answer; now it survives for trim/retry.

(F2 — a claimed Tools-capability bypass — was verified NOT reproducible:
agentRequestsAskUserQuestion matches only loaded instances/toolDefinitions/
toolRegistry, all capability-filtered; a raw tools string has no .name and
never triggers the install. Replied on-thread with the probe evidence.)

* fix: Codex round 5 — expired question exits answer mode so its message shows

An expired question (e.g. resume returns the stale-action 409) previously left
the popover open with locked controls and no explanation, because the chat
card — which carries the only 'this action expired' message — was suppressed
by the popover-open guard. Treat 'expired' as no longer active: the popover
closes, the composer reverts to normal, and the card becomes the sole surface
and renders the expired message. 'error' stays active (retryable).

* feat: group ask_user_question calls as their own category

A homogeneous group of ask_user_question tool calls now reads 'Asked N
questions' (present tense 'Asking N questions' while the turn streams) with a
question glyph and no raw-name suffix — mirroring the subagent 'Ran N agents'
category treatment, instead of 'Used N tools — ask_user_question'. Mixed
groups keep 'Used N tools' but humanize the suffix to 'Question' and show a
question icon for the ask entries (TOOL_FRIENDLY_NAME_KEYS + ToolIcon map).
A group only forms at count >= 2, so the plural is always grammatical.
Three ToolCallGroup.test cases cover homogeneous label/icon/suffix, present
tense while streaming, and the mixed-group fallback.

* fix: Codex round 6 — composer submit lock + abort stamp before emit

F7 (composer status lock): the ask submit status lived on ApprovalContext,
a React context mounted only around message content (ContentParts). The
PRIMARY answer surface — the composer in ChatForm — renders outside it, so
useApprovalContext returned the inert FALLBACK: status was always 'idle',
setStatus a no-op. The in-flight double-submit guard (round 4) and the
expired-exits-answer-mode fix (round 5) therefore never engaged for the
composer. Move ask submit status to a global Recoil atom (useAskSubmitStatus)
read/written by the composer, the popover, and the card alike, so a fast
double-click/Enter is actually blocked and expired/error surfaces on every
surface. Tool-approval status stays on the context (unchanged).

F5 (abort stamp before emit): the abort route re-stamped a paused
ask_user_question's args AFTER GenerationJobManager.abortJob had already
emitted the final SSE from the unstamped content, so a Redis/cross-replica
Stop left the live client showing an empty question until reload. abortJob
now takes an optional transformAbortContent applied to the persistable
content BEFORE the final event is built (and returned), so the live client
and the saved message agree. New abort.spec case + updated call assertions.

* feat: gate ask_user_question behind its own agent capability

Add a first-class AgentCapabilities.ask_user_question (in defaultAgentCapabilities,
on by default) so admins can enable/disable questions independently via
endpoints.agents.capabilities, exactly like execute_code / web_search — not
lumped under the generic tools capability.

- ToolService: both filteredTools predicates (definitions-only and instance
  loaders) gate ask_user_question on checkCapability(ask_user_question) before
  the generic tools fallthrough. When off, the tool is dropped from
  toolDefinitions/toolRegistry, so run.ts's agentRequestsAskUserQuestion (which
  keys on the loaded surface) declines to install it and attach a checkpointer —
  the capability is enforced end-to-end at the loader, no run.ts change needed.
- Tools dialog catalog: surface the ask builtin under its own capability rather
  than the generic tools one, so the UI matches the backend gate.
- Tests: ToolService capability on/off filtering + defaults membership; catalog
  builtin visibility keyed on the dedicated capability.

* style: sort imports in ToolCallGroup.test (CI import-order gate)

* fix: Codex round 7 — surface ask-answer errors in the open popover

A failed answer submission (16k reject, network error) sets the ask status to
'error', which — unlike 'expired' — deliberately keeps the question active and
retryable. But the chat card that renders the error message is suppressed while
the popover is open, so a composer/popover answer failed silently. Expose an
'errored' flag from useAskAnswerMode and render a warning line
(com_ui_ask_answer_error) in the popover, so the user gets feedback and retry
guidance without having to collapse/dismiss. It clears automatically on retry
(status flips to 'submitting').

* fix: Codex round 8 — respect IME composition before submitting answers

handleComposerKeyDown runs before useTextarea's composition guard, so with a
CJK/IME keyboard the Enter that commits an in-progress composition was being
intercepted and submitting the partial answer (and the composition buffer can
leave value empty mid-compose, mis-triggering digit/arrow steering too). Bail
at the top when composing — nativeEvent.isComposing, or key==='Process' /
keyCode===229 for Safari's inconsistent reporting — mirroring the existing
composer guard so the character commits normally.

* chore: update `@librechat/agents` to v3.2.60

* 🔧 chore: Update @opentelemetry/core to version 2.9.0 and clean up package-lock.json

* feat: digit shortcuts select options when the popover has focus

Previously a number key (1..N) only selected an option from the empty
composer (handleComposerKeyDown on the textarea) — if focus moved into the
popover (a row/Skip/Submit button clicked or tabbed to), the number keys went
dead. Add handlePopoverKeyDown, wired to the popover container's onKeyDown so
it catches digits bubbling from the focused control: a digit activates its
option exactly like a click (single-select submits, multi toggles). No
highlight/Enter dance on this path — the options are buttons whose action is
the click, and intercepting Enter would fight the focused button. Gated on
active && !locked so it no-ops while a submit is in flight.

* chore: update @librechat/agents to version 3.2.61 and @opentelemetry packages to latest versions
fuuuzzy pushed a commit to fuuuzzy/LibreChat that referenced this pull request Aug 7, 2026
…use/resume (LibreChat-AI#14139)

* feat: ask_user_question tool — agent-initiated questions with durable pause/resume

The HITL runtime merged in LibreChat-AI#13942/LibreChat-AI#14024/LibreChat-AI#14025/LibreChat-AI#14123 already ships the full
ask_user_question lifecycle (payload-agnostic handleRunInterrupt, resume
validation via mapAskUserAnswer, reconnect rehydration, and the client question
card) — but nothing ever raised the interrupt. This adds the producer:

- packages/api/agents/hitl/askUserQuestionTool.ts: LLM-callable tool whose func
  calls the SDK askUserQuestion() helper (LangGraph interrupt() from the tool
  body); zod schema with length caps mirroring AskUserQuestionRequest, plus a
  JSON-schema twin for the schema-only registry
- Registration: agentToolDefinitions, manifest.json (Tools dialog, admin
  filteredTools/includedTools kill switch), basicToolInstances, handleTools
  constructor branch
- run.ts gating: checkpointer now attaches for hitlCapable runs whose agents
  carry the ask tool even with the tool-approval policy disabled (the interrupt
  needs only durability, not humanInTheLoop/hooks); the tool is stripped
  fail-closed from non-HITL callers (OpenAI-compat/Responses) and subagent
  child configs; excluded from eager event execution (interrupts must be
  raised inside the Pregel task frame)
- resume.js: 16k length cap on the answer wire field
- e2e (real Run + FakeChatModel + LazyMongoSaver + supertest resume): tool-body
  interrupt pauses durably with NO approval policy, answer round-trips as the
  ToolMessage content, tool body re-runs once on resume, sequential questions
  re-pause

* fix: adversarial-review findings — in-graph execution, orphan prunes, endpoint scoping, real kill switch

Pre-PR multi-agent review confirmed 5 defects in the initial commit; all fixed:

1. CRITICAL — the tool never paused on the real agents endpoint: production
   loads tools definitions-only, flipping the SDK ToolNode to event-driven
   dispatch, and the host ON_TOOL_EXECUTE handler runs outside the Pregel task
   frame (under runOutsideTracing), where interrupt() throws and becomes an
   error ToolMessage. Reworked: the ask tool never rides toolDefinitions/
   toolRegistry — on HITL-capable top-level agents a real instance is supplied
   via AgentInputs.graphTools (agents#289, requires @librechat/agents > 3.2.57),
   the SDK's in-graph direct-tool seam; new production-shape e2e pins the
   event-driven mode end to end.
2. CRITICAL — ask-only runs left orphaned interrupted checkpoints (silent
   context duplication on every later turn): both orphan prunes were gated on
   toolApproval.enabled. The pre-turn prune now also fires for ask-capable
   agents (exported agentRequestsAskUserQuestion), and the abort-route prune
   fires when the aborted job carries a pendingAction.
3. MAJOR — self-spawned subagents bypassed the strip (self config resolves from
   the parent's _sourceInputs): fixed SDK-side (buildChildInputs clears
   graphTools) and the tool is now never present on child surfaces host-side.
4. MINOR — the manifest entry leaked into the Assistants tools dialog and the
   legacy plugins endpoint, where tools execute with no run to pause: new
   agentsOnly manifest flag, scoped out of both listings.
5. MINOR — filteredTools/includedTools only hid the tool from the dialog:
   now enforced at run build (strip + no checkpointer), making the admin
   filter a real kill switch for already-saved agents.

* chore: update @librechat/agents dependency to version 3.2.58 in package-lock.json and package.json files

* fix: reject agents-only tools at assistant create/update (Codex round 1)

The tools-dialog scoping keeps ask_user_question out of the assistants
LISTING, but the v1/v2 create/update handlers resolve arbitrary posted tool
strings from the shared getCachedTools map — a REST client or stale saved
payload could still attach it, and the assistants runtime executes tools with
no run to pause, so every call would error. New isAgentsOnlyTool(tool)
(manifest-driven, handles string and function-object shapes) drops such tools
with a warn at all four resolution sites (v1+v2, create+update).

* fix: offset resumed-run content indices past the pre-pause seed

A resumed run rebuilds the graph from the checkpoint, and the fresh graph
numbers content indices from its own empty contentData — starting at 0. The
resume path seeds the (also fresh) content aggregator with the pre-pause
parts at exactly those indices, so the resumed model turn collided with the
seed: type-matching parts silently MERGED (post-resume text appended into a
pre-pause text block), and type-mismatching parts (a reasoning/think part at
index 0 — any Anthropic reasoning agent) dropped EVERY delta with 'Content
type mismatch', losing the entire post-resume output from the live stream
and the saved message.

Latent since LibreChat-AI#13942 — tool-approval resumes corrupt content the same way
(probe-verified); it surfaced now because ask_user_question makes pausing a
first-class flow and reasoning models make the loss total.

- createContentIndexOffsetHandlers(handlers, offset): wraps ON_RUN_STEP
  (the single point where a content index enters the pipeline — deltas
  resolve through the aggregator's stepMap) and ON_AGENT_UPDATE's inline
  index; every other handler passes through by reference. Probe-validated:
  resumed output now lands as a new part after the paused tool call.
- resumeCompletion wires it with offset = seedContent.length.
- logToolError: a GraphInterrupt unwinding out of a tool body is the HITL
  pause working as designed — no longer logged as a Tool Error.

* fix: unblock live streaming of the resumed segment after an answer

With resume indices now ABSOLUTE (server continues after the pre-pause
parts), the synthetic ask-user-question card was squatting on exactly the
index the resumed segment streams into: applyAskUserQuestion appends the
card at the end of the message content, so on the answering device every
incoming part at that index was blocked and nothing rendered between the
answer submission and the finalize replacing the message.

removeAskUserQuestionPart(message, actionId) strips the pause-scoped card
on successful answer submission (useResumeSubmit onSuccess) — the durable
record of the Q&A is the ask_user_question tool call itself. Pure helper +
specs; same-reference no-op when nothing matches.

* fix: displace the synthetic question card in the streaming content writer

The store-level strip on answer submit wasn't enough: the SSE step handler
keeps its own in-flight copy of the streaming message, so on the answering
device the synthetic ask-user-question card still occupied the ABSOLUTE index
the resumed segment streams into — every delta warned 'Content type mismatch'
(existing ask_user_question vs incoming text) and nothing rendered between the
pending_action and finalize.

Displace the card inside updateContent when any real part claims its slot —
the same displacement pattern as the OAuth prompt part directly above it.
Covers the streaming handler's own copy, reconnecting tabs, and other devices;
once real content streams, the pause is over by definition. Spec drives a
runStep + text delta into the card's index and pins: no mismatch warn, card
gone, text rendered.

* feat: dedicated UI + durable data for completed ask_user_question calls

The completed ask call rendered as a generic tool card labeled 'Cancelled'
with raw (and empty) JSON args. Two layers fixed:

Data: the saved tool_call part had args:'' and no output — streamed arg
chunks carry no tool name so the aggregator drops them (normal tools recover
via the completion event, which never fires for a tool that interrupts
mid-execution and resumes on a rebuilt run with no step id). The resume
controller now stamps the paused ask part with the pendingAction's
authoritative question as args and the user's answer as output
(attachAskUserQuestionAnswer — pure, targets the newest unanswered ask part,
so sequential questions each keep their own answer).

UI: Part.tsx routes ask_user_question tool calls to AskUserQuestionCall — a
compact Q&A record ('Asked a question' header, question, description, 'You
answered: <label>' preferring the picked option's label, or 'No answer was
given' for an abandoned pause) instead of the generic card. New i18n keys;
parseAskUserQuestionArgs degrades to null on malformed model args.

* fix: single question UI per pause + immediate answer display

Two live-turn issues with the new durable Q&A card:

1. Duplicate question on ask: during a live pause the message carries BOTH the
   ask tool_call part (now rendered by AskUserQuestionCall, showing a
   misleading 'No answer was given' while paused) and the synthetic
   interactive card. The durable card now defers while the turn is live and
   unanswered (isSubmitting) — the interactive card owns the question UI until
   it's answered; an abandoned pause still shows its no-answer state once the
   turn settles.

2. 'No answer was given' after answering: the server stamps the answer onto
   the part at resume seed, but the client only received that at finalize. No
   stream emission needed — the client knows the answer it just submitted:
   resolveAskUserQuestionPart (replacing the plain strip on submit success)
   removes the synthetic card AND stamps output/progress onto the newest
   unanswered ask tool_call, seeding args from the synthetic part's question
   when the streamed args were lost — mirroring the server-side
   attachAskUserQuestionAnswer, so the Q&A record shows the answer the moment
   the user submits.

* fix: keep the Q&A record visible while the resumed segment streams

The optimistic output stamp lives in the message store, but the SSE step
handler evolves its own cached copy of the streaming message (created at turn
start) — the first resumed event overwrites the store with that copy, wiping
the stamp, so the Q&A card blinked out during streaming and only returned at
finalize.

Render-layer fallback instead of fighting the handler's copy: submitted
answers are recorded by ask tool_call id when resolveAskUserQuestionPart
stamps the part, and AskUserQuestionCall reads the recorded answer whenever
the part's own output is missing — the record survives any message-copy churn
until finalize delivers the server-stamped part.

* feat: present Ask User as a native builtin in the tools dialog

It ships with the app and pauses the run like a first-class feature, so it
belongs with the builtins (Run Code, Web Search, Memory, ...) rather than in
the third-party plugin list — while its mechanics stay exactly a plugin's:

- BuiltinId += 'ask_user_question' (documented exception: a native TOOL, not
  a capability; selection reads agent.tools, the toggle emits tool-add/remove
  patches instead of a capability field)
- buildCatalog surfaces it as a builtin gated on the same signals as before
  (tools capability on + the server lists the plugin, i.e. not admin-filtered)
  and skips it in the plugin loop so it never double-lists
- On-theme icon: lucide MessageCircleQuestion in a teal chip via the builtin
  icon map, matching the other native entries; the bespoke purple SVG and the
  manifest icon field are gone
- i18n'd name/description keys like the other builtins

* feat: composer popover for answering questions (mentions-style)

Answering moves to the composer, matching the existing mentions/prompts
popover pattern: while an ask_user_question pause is live, a popover anchors
above the textarea with the question as its header, numbered option rows
(hover/click, or ↑/↓ + Enter from the empty composer), and an × to dismiss.
The main textarea doubles as the free-form answer — its placeholder flips to
'Something else...' and form submit routes the text to the paused run as the
answer instead of starting a new turn. Dismissing (× or Escape) restores
normal sends; the inline transcript surfaces stay as before (interactive card
while paused, durable Q&A record after) so the question remains visible in
history.

- findLiveAskUserQuestion (pure, spec'd): newest unanswered synthetic part
  across the conversation IS the popover signal — applied on
  on_pending_action, stripped on answer submit, so visibility tracks the
  pause lifecycle with no extra state
- useLiveAskUserQuestion hook shared by the popover and ChatForm; dismissals
  in a recoil atom so both react
- popover only mounts on the primary composer (index 0), mirroring QuoteButton

* feat: number-key selection + return glyph in the question popover

Pressing 1-9 in the empty composer picks the matching option directly,
mirroring the numbered row chips; the highlighted row shows a return-key
glyph as the Enter affordance. Same empty-composer guard as the arrow keys —
typing a free-form answer is never intercepted.

* refactor: first-class composer answer mode (useAskAnswerMode)

Replaces the bolted-on integration (inline onSubmit interception + raw
capture-phase keydown listeners on the textarea ref) with a single hook that
owns the whole answer mode: live-question derivation, dismissal + highlighted
option (shared recoil state), option selection, free-form submit routing
(submitText returns whether it consumed the submission), and keyboard
handling (handleKeyDown returns whether it consumed the key, composed ahead
of the textarea's normal handler — no more addEventListener).

The popover is now pure rendering off the hook; ChatForm wires placeholder,
onKeyDown, and onSubmit through the same instance. Deliberately scoped to the
composer rather than useSubmitMessage: starters/prompt-commands keep new-turn
semantics (and the existing job-replacement behavior while paused).

* fix: Codex round 2 — inline answer input, approval exemption, pause-time args

F1 (composer submit unreachable while paused — isSubmitting keeps Stop shown
and useTextarea eats Enter): redesigned around it, borrowing Claude Code's
AskUserQuestion semantics. The popover now owns free-form input via an inline
'Other' row (numbered last, 'Something else…'), with select-then-confirm rows
(click/arrows/digits highlight; Submit ↵, Enter, or double-click fires; Skip
dismisses). The composer returns to being a plain composer — no placeholder
swap, no submit interception; Stop keeps meaning stop.

F2: ask_user_question is exempt from the tool-approval prompt unless the
admin explicitly lists it (allow/ask/deny all win) — approving the right to
ask a question was a pure double pause; the tool is side-effect-free.

F3: the question is stamped onto the paused ask tool_call's args at PAUSE
time (attachAskUserQuestionArgs in handleRunInterrupt), so abandoned/expired/
stopped turns persist with the question intact and the record card can render
it — previously only the answer-resume path stamped args.

* fix: fold model-supplied 'Other' options into the inline free-form row

The model can generate its own catch-all option ('Other (type your own)',
value 'other'), duplicating the popover's built-in free-form row — two
other-ish rows, one pickable as a literal answer. Two layers:

- Tool description now tells the model NOT to include catch-all options (the
  answer UI always offers free-form input on its own)
- splitOtherOption (pure, spec'd) folds a catch-all option that arrives
  anyway out of the choice rows and uses its label as the inline input's
  placeholder — conservative match (value 'other', or a label reading as a
  free-form invitation), no false positives on real choices

* fix: single question surface + clean free-form-only popover

Two live-pause confusions: (1) the inline transcript card and the composer
popover both rendered — the card now defers while the popover is up for its
action, returning as the fallback surface when the user dismisses the popover
(and in contexts without a ChatContext, where the popover can't exist);
(2) an options-less question showed a pointless numbered '1 Something else…'
row — free-form-only questions now render the inline input alone, with the
'Type your answer…' placeholder (a folded model 'Other' label still wins).

* feat: the composer is the free-form answer box (like the main chat input)

While a question pause is live, the main chat textarea composes the free-form
answer — placeholder swaps to 'Something else…' (or a folded model 'Other'
label), Enter with text submits the answer through answer-mode key handling
(composed BEFORE useTextarea's submitting-lock, so the lock can't swallow it),
and the Stop button swaps to Send (enabled despite isSubmitting) per the
select-then-confirm design. The popover slims to the question header, numbered
option rows, and Skip/Submit — its inline input is gone since the composer
owns free-form now. Dismissing the popover restores normal composer semantics
(Stop button, normal sends).

* fix: Codex round 3 + real Skip semantics

- Skip now ANSWERS instead of hiding UI (danny): it resumes the run with a
  decline notice ('The user chose not to answer this question.') so the model
  moves on — a client-side dismiss left the run paused until expiry, a hung
  turn. × / Escape remain pure dismiss (switch to the inline card surface).
- P1 (resumed approval tool indices): resumed tool_calls steps whose
  tool_call id matches a seeded UNRESOLVED part now rebind to that seeded
  slot instead of offsetting — the original part resolves in place (output
  attaches) and no duplicate appears; message steps keep the offset, so the
  text-loss fix stands. createContentIndexOffsetHandlers now takes the seed
  array; resolved seeded calls are not rebind targets.
- P2 (stale selection across questions): selection state resets when the
  live actionId changes; the vestigial inline-Other state ('other' selection
  + text atom) is gone — the composer owns free-form.
- P2 (Redis abort path loses the args stamp): the abort route re-stamps the
  question onto the ask tool_call in the reconstructed abort content, so a
  Stop-abandoned question persists with its question intact.
- P2 (malformed args crash): parseAskUserQuestionArgs normalizes untrusted
  shapes (options: {} / non-string entries) instead of throwing in render.

* feat: free-form hint in the question popover footer

Left-aligned in the footer row (opposite Skip/Submit): 'Or type your answer
below' — points open-ended answering at the composer, whose placeholder
already reads 'Something else…'.

* feat: preserve composer drafts across the answer-mode swap

The answer phase gets its own draft key (ask-answer:<actionId>), passed as a
draftId override into useAutoSave — the key change itself drives the existing
save/restore machinery, so the conversation draft (or mid-run PENDING draft)
is stashed when a question pause takes the composer and restored once the
user answers, skips, or dismisses. Ask keys are exempt from the PENDING
migration branch, which would otherwise move-and-delete the stashed draft. A
half-typed answer survives reload/navigation while its question stays live.
Answer submission (option pick, free-form, skip) resets the composer via a
new non-throwing useOptionalChatFormContext, so the swap-back restores into
an empty box even outside ChatView-less render contexts (Share/search).

* fix: rebind resumed steps for ALL seeded tool call ids

The resume controller pre-stamps the user's answer onto the seeded
ask_user_question part, so the unresolved-only rebind predicate treated
it as settled and shifted the tool's re-run step to a fresh offset slot,
leaving a duplicate ask record in streamed/saved content. Tool call ids
are provider-minted per call: a resumed step bearing a seeded id can
only be the interrupted batch re-executing, so rebinding every seeded
id is always correct.

* feat: popover UX round 4 — clickable hint, collapse, click-submit, multiSelect

- Footer hint is a button that focuses the composer; reads 'Type your answer
  below' (no 'Or') when the question has no options.
- Collapse (chevron) hides the popover WITHOUT closing the pause: answer mode
  stays live (placeholder, Enter routing, draft key), the chat card renders
  the question with a ChevronUp affordance to re-expand. x remains dismiss.
- Single-select options submit on a single click; the Submit button renders
  only for multi-select.
- multiSelect end-to-end: tool zod schema + JSON definition twin, wire type,
  client parse, popover check-chips, card toggles, record-card label mapping;
  answer = option values joined ', '; composer Enter and the multi Submit
  button both fold free-form text in with the checked values.
- Hardening from adversarial review: in-flight status guard on every submit
  path (no duplicate resumes on double-click), popover locks while
  submitting, collapsed mode disarms invisible digit/arrow steering, the
  card shares the hook's checked state while the pause is live, the card
  folds catch-all 'Other' options, record mapping is all-or-nothing to avoid
  phantom labels, composer resets only when its text was consumed or the
  draft machinery will restore the stash.

* feat: ask_user_question in model specs and ephemeral agents

A librechat.yaml modelSpec can now equip the tool the same way it equips
webSearch/executeCode/fileSearch/memory:

  modelSpecs:
    list:
      - name: my-spec
        askUserQuestion: true

loadEphemeralAgent pushes the tool name when the spec flag (or the
ephemeralAgent request flag, wired for parity) is set; everything downstream
is the existing persisted-agent machinery — createRun's hitlCapable gating,
graphTools injection, checkpointer attach, subagent strip, and the admin
filteredTools/includedTools kill switch all apply unchanged.

* feat: tense-aware Q&A record label (Asking / Asked)

Shorten the record card header per feedback: 'Asking' while the question is
still unanswered (abandoned/awaiting), 'Asked' once answered — replacing the
single 'Asked a question' label.

* fix: Codex round 4 — added-agent ask parity + preserve answer on failed resume

F1 (added.ts): mirror loadEphemeralAgent's ask_user_question branch in the
added-agent loader so a model spec's askUserQuestion flag (or the ephemeral
request flag) equips added top-level agents too, matching execute_code /
web_search / memory. Two load.spec cases added.

F3 (composer): submitAskAnswer now takes an onSuccess callback and
useAskAnswerMode defers clearing the selection/composer until the resume is
accepted. A failed resume (16k answer-cap 400, expired action, network error)
leaves status re-answerable, so wiping the composer up front lost the user's
only copy of a free-form answer; now it survives for trim/retry.

(F2 — a claimed Tools-capability bypass — was verified NOT reproducible:
agentRequestsAskUserQuestion matches only loaded instances/toolDefinitions/
toolRegistry, all capability-filtered; a raw tools string has no .name and
never triggers the install. Replied on-thread with the probe evidence.)

* fix: Codex round 5 — expired question exits answer mode so its message shows

An expired question (e.g. resume returns the stale-action 409) previously left
the popover open with locked controls and no explanation, because the chat
card — which carries the only 'this action expired' message — was suppressed
by the popover-open guard. Treat 'expired' as no longer active: the popover
closes, the composer reverts to normal, and the card becomes the sole surface
and renders the expired message. 'error' stays active (retryable).

* feat: group ask_user_question calls as their own category

A homogeneous group of ask_user_question tool calls now reads 'Asked N
questions' (present tense 'Asking N questions' while the turn streams) with a
question glyph and no raw-name suffix — mirroring the subagent 'Ran N agents'
category treatment, instead of 'Used N tools — ask_user_question'. Mixed
groups keep 'Used N tools' but humanize the suffix to 'Question' and show a
question icon for the ask entries (TOOL_FRIENDLY_NAME_KEYS + ToolIcon map).
A group only forms at count >= 2, so the plural is always grammatical.
Three ToolCallGroup.test cases cover homogeneous label/icon/suffix, present
tense while streaming, and the mixed-group fallback.

* fix: Codex round 6 — composer submit lock + abort stamp before emit

F7 (composer status lock): the ask submit status lived on ApprovalContext,
a React context mounted only around message content (ContentParts). The
PRIMARY answer surface — the composer in ChatForm — renders outside it, so
useApprovalContext returned the inert FALLBACK: status was always 'idle',
setStatus a no-op. The in-flight double-submit guard (round 4) and the
expired-exits-answer-mode fix (round 5) therefore never engaged for the
composer. Move ask submit status to a global Recoil atom (useAskSubmitStatus)
read/written by the composer, the popover, and the card alike, so a fast
double-click/Enter is actually blocked and expired/error surfaces on every
surface. Tool-approval status stays on the context (unchanged).

F5 (abort stamp before emit): the abort route re-stamped a paused
ask_user_question's args AFTER GenerationJobManager.abortJob had already
emitted the final SSE from the unstamped content, so a Redis/cross-replica
Stop left the live client showing an empty question until reload. abortJob
now takes an optional transformAbortContent applied to the persistable
content BEFORE the final event is built (and returned), so the live client
and the saved message agree. New abort.spec case + updated call assertions.

* feat: gate ask_user_question behind its own agent capability

Add a first-class AgentCapabilities.ask_user_question (in defaultAgentCapabilities,
on by default) so admins can enable/disable questions independently via
endpoints.agents.capabilities, exactly like execute_code / web_search — not
lumped under the generic tools capability.

- ToolService: both filteredTools predicates (definitions-only and instance
  loaders) gate ask_user_question on checkCapability(ask_user_question) before
  the generic tools fallthrough. When off, the tool is dropped from
  toolDefinitions/toolRegistry, so run.ts's agentRequestsAskUserQuestion (which
  keys on the loaded surface) declines to install it and attach a checkpointer —
  the capability is enforced end-to-end at the loader, no run.ts change needed.
- Tools dialog catalog: surface the ask builtin under its own capability rather
  than the generic tools one, so the UI matches the backend gate.
- Tests: ToolService capability on/off filtering + defaults membership; catalog
  builtin visibility keyed on the dedicated capability.

* style: sort imports in ToolCallGroup.test (CI import-order gate)

* fix: Codex round 7 — surface ask-answer errors in the open popover

A failed answer submission (16k reject, network error) sets the ask status to
'error', which — unlike 'expired' — deliberately keeps the question active and
retryable. But the chat card that renders the error message is suppressed while
the popover is open, so a composer/popover answer failed silently. Expose an
'errored' flag from useAskAnswerMode and render a warning line
(com_ui_ask_answer_error) in the popover, so the user gets feedback and retry
guidance without having to collapse/dismiss. It clears automatically on retry
(status flips to 'submitting').

* fix: Codex round 8 — respect IME composition before submitting answers

handleComposerKeyDown runs before useTextarea's composition guard, so with a
CJK/IME keyboard the Enter that commits an in-progress composition was being
intercepted and submitting the partial answer (and the composition buffer can
leave value empty mid-compose, mis-triggering digit/arrow steering too). Bail
at the top when composing — nativeEvent.isComposing, or key==='Process' /
keyCode===229 for Safari's inconsistent reporting — mirroring the existing
composer guard so the character commits normally.

* chore: update `@librechat/agents` to v3.2.60

* 🔧 chore: Update @opentelemetry/core to version 2.9.0 and clean up package-lock.json

* feat: digit shortcuts select options when the popover has focus

Previously a number key (1..N) only selected an option from the empty
composer (handleComposerKeyDown on the textarea) — if focus moved into the
popover (a row/Skip/Submit button clicked or tabbed to), the number keys went
dead. Add handlePopoverKeyDown, wired to the popover container's onKeyDown so
it catches digits bubbling from the focused control: a digit activates its
option exactly like a click (single-select submits, multi toggles). No
highlight/Enter dance on this path — the options are buttons whose action is
the click, and intercepting Enter would fight the focused button. Gated on
active && !locked so it no-ops while a submit is in flight.

* chore: update @librechat/agents to version 3.2.61 and @opentelemetry packages to latest versions
LogicalAbsurd pushed a commit to LogicalAbsurd/LibreChat that referenced this pull request Aug 27, 2026
… (Slice B) (LibreChat-AI#13942)

* 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)
LogicalAbsurd pushed a commit to LogicalAbsurd/LibreChat that referenced this pull request Aug 27, 2026
…use/resume (LibreChat-AI#14139)

* feat: ask_user_question tool — agent-initiated questions with durable pause/resume

The HITL runtime merged in LibreChat-AI#13942/LibreChat-AI#14024/LibreChat-AI#14025/LibreChat-AI#14123 already ships the full
ask_user_question lifecycle (payload-agnostic handleRunInterrupt, resume
validation via mapAskUserAnswer, reconnect rehydration, and the client question
card) — but nothing ever raised the interrupt. This adds the producer:

- packages/api/agents/hitl/askUserQuestionTool.ts: LLM-callable tool whose func
  calls the SDK askUserQuestion() helper (LangGraph interrupt() from the tool
  body); zod schema with length caps mirroring AskUserQuestionRequest, plus a
  JSON-schema twin for the schema-only registry
- Registration: agentToolDefinitions, manifest.json (Tools dialog, admin
  filteredTools/includedTools kill switch), basicToolInstances, handleTools
  constructor branch
- run.ts gating: checkpointer now attaches for hitlCapable runs whose agents
  carry the ask tool even with the tool-approval policy disabled (the interrupt
  needs only durability, not humanInTheLoop/hooks); the tool is stripped
  fail-closed from non-HITL callers (OpenAI-compat/Responses) and subagent
  child configs; excluded from eager event execution (interrupts must be
  raised inside the Pregel task frame)
- resume.js: 16k length cap on the answer wire field
- e2e (real Run + FakeChatModel + LazyMongoSaver + supertest resume): tool-body
  interrupt pauses durably with NO approval policy, answer round-trips as the
  ToolMessage content, tool body re-runs once on resume, sequential questions
  re-pause

* fix: adversarial-review findings — in-graph execution, orphan prunes, endpoint scoping, real kill switch

Pre-PR multi-agent review confirmed 5 defects in the initial commit; all fixed:

1. CRITICAL — the tool never paused on the real agents endpoint: production
   loads tools definitions-only, flipping the SDK ToolNode to event-driven
   dispatch, and the host ON_TOOL_EXECUTE handler runs outside the Pregel task
   frame (under runOutsideTracing), where interrupt() throws and becomes an
   error ToolMessage. Reworked: the ask tool never rides toolDefinitions/
   toolRegistry — on HITL-capable top-level agents a real instance is supplied
   via AgentInputs.graphTools (agents#289, requires @librechat/agents > 3.2.57),
   the SDK's in-graph direct-tool seam; new production-shape e2e pins the
   event-driven mode end to end.
2. CRITICAL — ask-only runs left orphaned interrupted checkpoints (silent
   context duplication on every later turn): both orphan prunes were gated on
   toolApproval.enabled. The pre-turn prune now also fires for ask-capable
   agents (exported agentRequestsAskUserQuestion), and the abort-route prune
   fires when the aborted job carries a pendingAction.
3. MAJOR — self-spawned subagents bypassed the strip (self config resolves from
   the parent's _sourceInputs): fixed SDK-side (buildChildInputs clears
   graphTools) and the tool is now never present on child surfaces host-side.
4. MINOR — the manifest entry leaked into the Assistants tools dialog and the
   legacy plugins endpoint, where tools execute with no run to pause: new
   agentsOnly manifest flag, scoped out of both listings.
5. MINOR — filteredTools/includedTools only hid the tool from the dialog:
   now enforced at run build (strip + no checkpointer), making the admin
   filter a real kill switch for already-saved agents.

* chore: update @librechat/agents dependency to version 3.2.58 in package-lock.json and package.json files

* fix: reject agents-only tools at assistant create/update (Codex round 1)

The tools-dialog scoping keeps ask_user_question out of the assistants
LISTING, but the v1/v2 create/update handlers resolve arbitrary posted tool
strings from the shared getCachedTools map — a REST client or stale saved
payload could still attach it, and the assistants runtime executes tools with
no run to pause, so every call would error. New isAgentsOnlyTool(tool)
(manifest-driven, handles string and function-object shapes) drops such tools
with a warn at all four resolution sites (v1+v2, create+update).

* fix: offset resumed-run content indices past the pre-pause seed

A resumed run rebuilds the graph from the checkpoint, and the fresh graph
numbers content indices from its own empty contentData — starting at 0. The
resume path seeds the (also fresh) content aggregator with the pre-pause
parts at exactly those indices, so the resumed model turn collided with the
seed: type-matching parts silently MERGED (post-resume text appended into a
pre-pause text block), and type-mismatching parts (a reasoning/think part at
index 0 — any Anthropic reasoning agent) dropped EVERY delta with 'Content
type mismatch', losing the entire post-resume output from the live stream
and the saved message.

Latent since LibreChat-AI#13942 — tool-approval resumes corrupt content the same way
(probe-verified); it surfaced now because ask_user_question makes pausing a
first-class flow and reasoning models make the loss total.

- createContentIndexOffsetHandlers(handlers, offset): wraps ON_RUN_STEP
  (the single point where a content index enters the pipeline — deltas
  resolve through the aggregator's stepMap) and ON_AGENT_UPDATE's inline
  index; every other handler passes through by reference. Probe-validated:
  resumed output now lands as a new part after the paused tool call.
- resumeCompletion wires it with offset = seedContent.length.
- logToolError: a GraphInterrupt unwinding out of a tool body is the HITL
  pause working as designed — no longer logged as a Tool Error.

* fix: unblock live streaming of the resumed segment after an answer

With resume indices now ABSOLUTE (server continues after the pre-pause
parts), the synthetic ask-user-question card was squatting on exactly the
index the resumed segment streams into: applyAskUserQuestion appends the
card at the end of the message content, so on the answering device every
incoming part at that index was blocked and nothing rendered between the
answer submission and the finalize replacing the message.

removeAskUserQuestionPart(message, actionId) strips the pause-scoped card
on successful answer submission (useResumeSubmit onSuccess) — the durable
record of the Q&A is the ask_user_question tool call itself. Pure helper +
specs; same-reference no-op when nothing matches.

* fix: displace the synthetic question card in the streaming content writer

The store-level strip on answer submit wasn't enough: the SSE step handler
keeps its own in-flight copy of the streaming message, so on the answering
device the synthetic ask-user-question card still occupied the ABSOLUTE index
the resumed segment streams into — every delta warned 'Content type mismatch'
(existing ask_user_question vs incoming text) and nothing rendered between the
pending_action and finalize.

Displace the card inside updateContent when any real part claims its slot —
the same displacement pattern as the OAuth prompt part directly above it.
Covers the streaming handler's own copy, reconnecting tabs, and other devices;
once real content streams, the pause is over by definition. Spec drives a
runStep + text delta into the card's index and pins: no mismatch warn, card
gone, text rendered.

* feat: dedicated UI + durable data for completed ask_user_question calls

The completed ask call rendered as a generic tool card labeled 'Cancelled'
with raw (and empty) JSON args. Two layers fixed:

Data: the saved tool_call part had args:'' and no output — streamed arg
chunks carry no tool name so the aggregator drops them (normal tools recover
via the completion event, which never fires for a tool that interrupts
mid-execution and resumes on a rebuilt run with no step id). The resume
controller now stamps the paused ask part with the pendingAction's
authoritative question as args and the user's answer as output
(attachAskUserQuestionAnswer — pure, targets the newest unanswered ask part,
so sequential questions each keep their own answer).

UI: Part.tsx routes ask_user_question tool calls to AskUserQuestionCall — a
compact Q&A record ('Asked a question' header, question, description, 'You
answered: <label>' preferring the picked option's label, or 'No answer was
given' for an abandoned pause) instead of the generic card. New i18n keys;
parseAskUserQuestionArgs degrades to null on malformed model args.

* fix: single question UI per pause + immediate answer display

Two live-turn issues with the new durable Q&A card:

1. Duplicate question on ask: during a live pause the message carries BOTH the
   ask tool_call part (now rendered by AskUserQuestionCall, showing a
   misleading 'No answer was given' while paused) and the synthetic
   interactive card. The durable card now defers while the turn is live and
   unanswered (isSubmitting) — the interactive card owns the question UI until
   it's answered; an abandoned pause still shows its no-answer state once the
   turn settles.

2. 'No answer was given' after answering: the server stamps the answer onto
   the part at resume seed, but the client only received that at finalize. No
   stream emission needed — the client knows the answer it just submitted:
   resolveAskUserQuestionPart (replacing the plain strip on submit success)
   removes the synthetic card AND stamps output/progress onto the newest
   unanswered ask tool_call, seeding args from the synthetic part's question
   when the streamed args were lost — mirroring the server-side
   attachAskUserQuestionAnswer, so the Q&A record shows the answer the moment
   the user submits.

* fix: keep the Q&A record visible while the resumed segment streams

The optimistic output stamp lives in the message store, but the SSE step
handler evolves its own cached copy of the streaming message (created at turn
start) — the first resumed event overwrites the store with that copy, wiping
the stamp, so the Q&A card blinked out during streaming and only returned at
finalize.

Render-layer fallback instead of fighting the handler's copy: submitted
answers are recorded by ask tool_call id when resolveAskUserQuestionPart
stamps the part, and AskUserQuestionCall reads the recorded answer whenever
the part's own output is missing — the record survives any message-copy churn
until finalize delivers the server-stamped part.

* feat: present Ask User as a native builtin in the tools dialog

It ships with the app and pauses the run like a first-class feature, so it
belongs with the builtins (Run Code, Web Search, Memory, ...) rather than in
the third-party plugin list — while its mechanics stay exactly a plugin's:

- BuiltinId += 'ask_user_question' (documented exception: a native TOOL, not
  a capability; selection reads agent.tools, the toggle emits tool-add/remove
  patches instead of a capability field)
- buildCatalog surfaces it as a builtin gated on the same signals as before
  (tools capability on + the server lists the plugin, i.e. not admin-filtered)
  and skips it in the plugin loop so it never double-lists
- On-theme icon: lucide MessageCircleQuestion in a teal chip via the builtin
  icon map, matching the other native entries; the bespoke purple SVG and the
  manifest icon field are gone
- i18n'd name/description keys like the other builtins

* feat: composer popover for answering questions (mentions-style)

Answering moves to the composer, matching the existing mentions/prompts
popover pattern: while an ask_user_question pause is live, a popover anchors
above the textarea with the question as its header, numbered option rows
(hover/click, or ↑/↓ + Enter from the empty composer), and an × to dismiss.
The main textarea doubles as the free-form answer — its placeholder flips to
'Something else...' and form submit routes the text to the paused run as the
answer instead of starting a new turn. Dismissing (× or Escape) restores
normal sends; the inline transcript surfaces stay as before (interactive card
while paused, durable Q&A record after) so the question remains visible in
history.

- findLiveAskUserQuestion (pure, spec'd): newest unanswered synthetic part
  across the conversation IS the popover signal — applied on
  on_pending_action, stripped on answer submit, so visibility tracks the
  pause lifecycle with no extra state
- useLiveAskUserQuestion hook shared by the popover and ChatForm; dismissals
  in a recoil atom so both react
- popover only mounts on the primary composer (index 0), mirroring QuoteButton

* feat: number-key selection + return glyph in the question popover

Pressing 1-9 in the empty composer picks the matching option directly,
mirroring the numbered row chips; the highlighted row shows a return-key
glyph as the Enter affordance. Same empty-composer guard as the arrow keys —
typing a free-form answer is never intercepted.

* refactor: first-class composer answer mode (useAskAnswerMode)

Replaces the bolted-on integration (inline onSubmit interception + raw
capture-phase keydown listeners on the textarea ref) with a single hook that
owns the whole answer mode: live-question derivation, dismissal + highlighted
option (shared recoil state), option selection, free-form submit routing
(submitText returns whether it consumed the submission), and keyboard
handling (handleKeyDown returns whether it consumed the key, composed ahead
of the textarea's normal handler — no more addEventListener).

The popover is now pure rendering off the hook; ChatForm wires placeholder,
onKeyDown, and onSubmit through the same instance. Deliberately scoped to the
composer rather than useSubmitMessage: starters/prompt-commands keep new-turn
semantics (and the existing job-replacement behavior while paused).

* fix: Codex round 2 — inline answer input, approval exemption, pause-time args

F1 (composer submit unreachable while paused — isSubmitting keeps Stop shown
and useTextarea eats Enter): redesigned around it, borrowing Claude Code's
AskUserQuestion semantics. The popover now owns free-form input via an inline
'Other' row (numbered last, 'Something else…'), with select-then-confirm rows
(click/arrows/digits highlight; Submit ↵, Enter, or double-click fires; Skip
dismisses). The composer returns to being a plain composer — no placeholder
swap, no submit interception; Stop keeps meaning stop.

F2: ask_user_question is exempt from the tool-approval prompt unless the
admin explicitly lists it (allow/ask/deny all win) — approving the right to
ask a question was a pure double pause; the tool is side-effect-free.

F3: the question is stamped onto the paused ask tool_call's args at PAUSE
time (attachAskUserQuestionArgs in handleRunInterrupt), so abandoned/expired/
stopped turns persist with the question intact and the record card can render
it — previously only the answer-resume path stamped args.

* fix: fold model-supplied 'Other' options into the inline free-form row

The model can generate its own catch-all option ('Other (type your own)',
value 'other'), duplicating the popover's built-in free-form row — two
other-ish rows, one pickable as a literal answer. Two layers:

- Tool description now tells the model NOT to include catch-all options (the
  answer UI always offers free-form input on its own)
- splitOtherOption (pure, spec'd) folds a catch-all option that arrives
  anyway out of the choice rows and uses its label as the inline input's
  placeholder — conservative match (value 'other', or a label reading as a
  free-form invitation), no false positives on real choices

* fix: single question surface + clean free-form-only popover

Two live-pause confusions: (1) the inline transcript card and the composer
popover both rendered — the card now defers while the popover is up for its
action, returning as the fallback surface when the user dismisses the popover
(and in contexts without a ChatContext, where the popover can't exist);
(2) an options-less question showed a pointless numbered '1 Something else…'
row — free-form-only questions now render the inline input alone, with the
'Type your answer…' placeholder (a folded model 'Other' label still wins).

* feat: the composer is the free-form answer box (like the main chat input)

While a question pause is live, the main chat textarea composes the free-form
answer — placeholder swaps to 'Something else…' (or a folded model 'Other'
label), Enter with text submits the answer through answer-mode key handling
(composed BEFORE useTextarea's submitting-lock, so the lock can't swallow it),
and the Stop button swaps to Send (enabled despite isSubmitting) per the
select-then-confirm design. The popover slims to the question header, numbered
option rows, and Skip/Submit — its inline input is gone since the composer
owns free-form now. Dismissing the popover restores normal composer semantics
(Stop button, normal sends).

* fix: Codex round 3 + real Skip semantics

- Skip now ANSWERS instead of hiding UI (danny): it resumes the run with a
  decline notice ('The user chose not to answer this question.') so the model
  moves on — a client-side dismiss left the run paused until expiry, a hung
  turn. × / Escape remain pure dismiss (switch to the inline card surface).
- P1 (resumed approval tool indices): resumed tool_calls steps whose
  tool_call id matches a seeded UNRESOLVED part now rebind to that seeded
  slot instead of offsetting — the original part resolves in place (output
  attaches) and no duplicate appears; message steps keep the offset, so the
  text-loss fix stands. createContentIndexOffsetHandlers now takes the seed
  array; resolved seeded calls are not rebind targets.
- P2 (stale selection across questions): selection state resets when the
  live actionId changes; the vestigial inline-Other state ('other' selection
  + text atom) is gone — the composer owns free-form.
- P2 (Redis abort path loses the args stamp): the abort route re-stamps the
  question onto the ask tool_call in the reconstructed abort content, so a
  Stop-abandoned question persists with its question intact.
- P2 (malformed args crash): parseAskUserQuestionArgs normalizes untrusted
  shapes (options: {} / non-string entries) instead of throwing in render.

* feat: free-form hint in the question popover footer

Left-aligned in the footer row (opposite Skip/Submit): 'Or type your answer
below' — points open-ended answering at the composer, whose placeholder
already reads 'Something else…'.

* feat: preserve composer drafts across the answer-mode swap

The answer phase gets its own draft key (ask-answer:<actionId>), passed as a
draftId override into useAutoSave — the key change itself drives the existing
save/restore machinery, so the conversation draft (or mid-run PENDING draft)
is stashed when a question pause takes the composer and restored once the
user answers, skips, or dismisses. Ask keys are exempt from the PENDING
migration branch, which would otherwise move-and-delete the stashed draft. A
half-typed answer survives reload/navigation while its question stays live.
Answer submission (option pick, free-form, skip) resets the composer via a
new non-throwing useOptionalChatFormContext, so the swap-back restores into
an empty box even outside ChatView-less render contexts (Share/search).

* fix: rebind resumed steps for ALL seeded tool call ids

The resume controller pre-stamps the user's answer onto the seeded
ask_user_question part, so the unresolved-only rebind predicate treated
it as settled and shifted the tool's re-run step to a fresh offset slot,
leaving a duplicate ask record in streamed/saved content. Tool call ids
are provider-minted per call: a resumed step bearing a seeded id can
only be the interrupted batch re-executing, so rebinding every seeded
id is always correct.

* feat: popover UX round 4 — clickable hint, collapse, click-submit, multiSelect

- Footer hint is a button that focuses the composer; reads 'Type your answer
  below' (no 'Or') when the question has no options.
- Collapse (chevron) hides the popover WITHOUT closing the pause: answer mode
  stays live (placeholder, Enter routing, draft key), the chat card renders
  the question with a ChevronUp affordance to re-expand. x remains dismiss.
- Single-select options submit on a single click; the Submit button renders
  only for multi-select.
- multiSelect end-to-end: tool zod schema + JSON definition twin, wire type,
  client parse, popover check-chips, card toggles, record-card label mapping;
  answer = option values joined ', '; composer Enter and the multi Submit
  button both fold free-form text in with the checked values.
- Hardening from adversarial review: in-flight status guard on every submit
  path (no duplicate resumes on double-click), popover locks while
  submitting, collapsed mode disarms invisible digit/arrow steering, the
  card shares the hook's checked state while the pause is live, the card
  folds catch-all 'Other' options, record mapping is all-or-nothing to avoid
  phantom labels, composer resets only when its text was consumed or the
  draft machinery will restore the stash.

* feat: ask_user_question in model specs and ephemeral agents

A librechat.yaml modelSpec can now equip the tool the same way it equips
webSearch/executeCode/fileSearch/memory:

  modelSpecs:
    list:
      - name: my-spec
        askUserQuestion: true

loadEphemeralAgent pushes the tool name when the spec flag (or the
ephemeralAgent request flag, wired for parity) is set; everything downstream
is the existing persisted-agent machinery — createRun's hitlCapable gating,
graphTools injection, checkpointer attach, subagent strip, and the admin
filteredTools/includedTools kill switch all apply unchanged.

* feat: tense-aware Q&A record label (Asking / Asked)

Shorten the record card header per feedback: 'Asking' while the question is
still unanswered (abandoned/awaiting), 'Asked' once answered — replacing the
single 'Asked a question' label.

* fix: Codex round 4 — added-agent ask parity + preserve answer on failed resume

F1 (added.ts): mirror loadEphemeralAgent's ask_user_question branch in the
added-agent loader so a model spec's askUserQuestion flag (or the ephemeral
request flag) equips added top-level agents too, matching execute_code /
web_search / memory. Two load.spec cases added.

F3 (composer): submitAskAnswer now takes an onSuccess callback and
useAskAnswerMode defers clearing the selection/composer until the resume is
accepted. A failed resume (16k answer-cap 400, expired action, network error)
leaves status re-answerable, so wiping the composer up front lost the user's
only copy of a free-form answer; now it survives for trim/retry.

(F2 — a claimed Tools-capability bypass — was verified NOT reproducible:
agentRequestsAskUserQuestion matches only loaded instances/toolDefinitions/
toolRegistry, all capability-filtered; a raw tools string has no .name and
never triggers the install. Replied on-thread with the probe evidence.)

* fix: Codex round 5 — expired question exits answer mode so its message shows

An expired question (e.g. resume returns the stale-action 409) previously left
the popover open with locked controls and no explanation, because the chat
card — which carries the only 'this action expired' message — was suppressed
by the popover-open guard. Treat 'expired' as no longer active: the popover
closes, the composer reverts to normal, and the card becomes the sole surface
and renders the expired message. 'error' stays active (retryable).

* feat: group ask_user_question calls as their own category

A homogeneous group of ask_user_question tool calls now reads 'Asked N
questions' (present tense 'Asking N questions' while the turn streams) with a
question glyph and no raw-name suffix — mirroring the subagent 'Ran N agents'
category treatment, instead of 'Used N tools — ask_user_question'. Mixed
groups keep 'Used N tools' but humanize the suffix to 'Question' and show a
question icon for the ask entries (TOOL_FRIENDLY_NAME_KEYS + ToolIcon map).
A group only forms at count >= 2, so the plural is always grammatical.
Three ToolCallGroup.test cases cover homogeneous label/icon/suffix, present
tense while streaming, and the mixed-group fallback.

* fix: Codex round 6 — composer submit lock + abort stamp before emit

F7 (composer status lock): the ask submit status lived on ApprovalContext,
a React context mounted only around message content (ContentParts). The
PRIMARY answer surface — the composer in ChatForm — renders outside it, so
useApprovalContext returned the inert FALLBACK: status was always 'idle',
setStatus a no-op. The in-flight double-submit guard (round 4) and the
expired-exits-answer-mode fix (round 5) therefore never engaged for the
composer. Move ask submit status to a global Recoil atom (useAskSubmitStatus)
read/written by the composer, the popover, and the card alike, so a fast
double-click/Enter is actually blocked and expired/error surfaces on every
surface. Tool-approval status stays on the context (unchanged).

F5 (abort stamp before emit): the abort route re-stamped a paused
ask_user_question's args AFTER GenerationJobManager.abortJob had already
emitted the final SSE from the unstamped content, so a Redis/cross-replica
Stop left the live client showing an empty question until reload. abortJob
now takes an optional transformAbortContent applied to the persistable
content BEFORE the final event is built (and returned), so the live client
and the saved message agree. New abort.spec case + updated call assertions.

* feat: gate ask_user_question behind its own agent capability

Add a first-class AgentCapabilities.ask_user_question (in defaultAgentCapabilities,
on by default) so admins can enable/disable questions independently via
endpoints.agents.capabilities, exactly like execute_code / web_search — not
lumped under the generic tools capability.

- ToolService: both filteredTools predicates (definitions-only and instance
  loaders) gate ask_user_question on checkCapability(ask_user_question) before
  the generic tools fallthrough. When off, the tool is dropped from
  toolDefinitions/toolRegistry, so run.ts's agentRequestsAskUserQuestion (which
  keys on the loaded surface) declines to install it and attach a checkpointer —
  the capability is enforced end-to-end at the loader, no run.ts change needed.
- Tools dialog catalog: surface the ask builtin under its own capability rather
  than the generic tools one, so the UI matches the backend gate.
- Tests: ToolService capability on/off filtering + defaults membership; catalog
  builtin visibility keyed on the dedicated capability.

* style: sort imports in ToolCallGroup.test (CI import-order gate)

* fix: Codex round 7 — surface ask-answer errors in the open popover

A failed answer submission (16k reject, network error) sets the ask status to
'error', which — unlike 'expired' — deliberately keeps the question active and
retryable. But the chat card that renders the error message is suppressed while
the popover is open, so a composer/popover answer failed silently. Expose an
'errored' flag from useAskAnswerMode and render a warning line
(com_ui_ask_answer_error) in the popover, so the user gets feedback and retry
guidance without having to collapse/dismiss. It clears automatically on retry
(status flips to 'submitting').

* fix: Codex round 8 — respect IME composition before submitting answers

handleComposerKeyDown runs before useTextarea's composition guard, so with a
CJK/IME keyboard the Enter that commits an in-progress composition was being
intercepted and submitting the partial answer (and the composition buffer can
leave value empty mid-compose, mis-triggering digit/arrow steering too). Bail
at the top when composing — nativeEvent.isComposing, or key==='Process' /
keyCode===229 for Safari's inconsistent reporting — mirroring the existing
composer guard so the character commits normally.

* chore: update `@librechat/agents` to v3.2.60

* 🔧 chore: Update @opentelemetry/core to version 2.9.0 and clean up package-lock.json

* feat: digit shortcuts select options when the popover has focus

Previously a number key (1..N) only selected an option from the empty
composer (handleComposerKeyDown on the textarea) — if focus moved into the
popover (a row/Skip/Submit button clicked or tabbed to), the number keys went
dead. Add handlePopoverKeyDown, wired to the popover container's onKeyDown so
it catches digits bubbling from the focused control: a digit activates its
option exactly like a click (single-select submits, multi toggles). No
highlight/Enter dance on this path — the options are buttons whose action is
the click, and intercepting Enter would fight the focused button. Gated on
active && !locked so it no-ops while a submit is in flight.

* chore: update @librechat/agents to version 3.2.61 and @opentelemetry packages to latest versions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant