Commit graph

10 commits

Author SHA1 Message Date
Danny Avila
3191f6975a
🏷️ fix: Skip Title Generation for Preempt-Incomplete Turns (#14571)
Some checks failed
Publish `@librechat/client` to NPM / pack (push) Has been cancelled
Publish `librechat-data-provider` to NPM / pack (push) Has been cancelled
Publish `@librechat/data-schemas` to NPM / pack (push) Has been cancelled
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Has been cancelled
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Has been cancelled
GitNexus Index / index (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Has been cancelled
Sync Helm Chart Tags / Ignore non-main push (push) Has been cancelled
Sync Helm Chart Tags / Sync chart tags (push) Has been cancelled
Publish `@librechat/client` to NPM / publish-npm (push) Has been cancelled
Publish `librechat-data-provider` to NPM / publish-npm (push) Has been cancelled
Publish `@librechat/data-schemas` to NPM / publish-npm (push) Has been cancelled
GitNexus Index / post-index (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Has been cancelled
2026-08-01 09:01:13 -04:00
Danny Avila
e7f1838515
feat: Reliable Interrupt & Steer Escalation and Recovery (#14558)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* feat: surface interrupt-steer escalation on waiting messages

The interrupt & steer feature shipped reachable only through the
composer chord, the send-button hovercard, and the composer button; a
message already waiting (queued for after the run, or steered and
parked at the next tool boundary) had no path to it. Both waiting
surfaces now carry one:

- Queued rows get an icon-only ZapOff escalation button beside the
  existing Steer primary. It routes through sendQueuedNow, which now
  takes a preempt option on its live-run path. The tooltip teaches the
  composer chord, derived through resolveComposerKeyDown so a rebound
  or yielded chord is never advertised.
- In-flight steer bubbles get an "Interrupt now" overflow entry with
  the same race rules as Edit: reclaim first, and only a `reclaimed`
  outcome resubmits (via retrySteer with preempt, swapping the chip
  for an interrupting one). `applied` and run-ended-mid-reclaim
  outcomes stop at the existing informational toasts, so the words can
  never land twice. Not offered on a steer already preempting.
- Every during-run overflow menu gains an "Always interrupt instead"
  toggle for steerInterruptsByDefault, next to the existing steer/queue
  default toggle. MenuEntry supports disabled for the new entries.

Only one interrupt can be unresolved at a time: while one preempt is
pending (or the run is paused on approval, where the server 409s),
every escalation control disables instead of racing the same seal.

Ten new tests across both surfaces; 381 green in the affected suites.

* fix: lock escalation across its reclaim window, keep the paused control visible, label as steer

Codex round 1, all three findings.

P2, escalation race. The single-interrupt invariant had a window between
clicking "Interrupt now" and the reclaim resolving, where no preempt
chip existed for the chip-derived gate to see: two bubbles escalated
back-to-back could both resubmit. A shared escalating flag (Jotai,
per-conversation) now covers the window and disables every escalation
control on both surfaces, and a fresh recheck before resubmitting
catches an interrupt armed elsewhere meanwhile (composer chord, queued
row); those words re-home to the queue with an informational toast
instead of breaking the invariant.

P2, unreachable paused state. canSteer is defined as
hasRealConvoId && !pausedOnApproval, so gating the button on canSteer
removed it exactly when it was meant to render disabled; the test only
passed on an impossible stub combination. The render gate is now
duringRunActive && (canSteer || pausedOnApproval), and the test uses the
real invariant.

P2, label semantics. "Interrupt & send now" borrowed the name of the
hard-abort action; this one preserves the partial answer and steers.
Renamed to "Interrupt & steer now" (com_ui_interrupt_steer_now).

Both behavior fixes counterfactually verified; 384 tests green across
the affected suites.

* fix: disable bubble escalation while the run cannot accept a steer

Codex round 2, one P2. Answer mode (ask_user_question) sets
duringRunActive false while pausedOnApproval stays false, since that
flag only detects approval-bearing tool calls. The bubble's escalation
entry stayed enabled there, so clicking it cancelled a healthy waiting
steer and the preempt resubmission bounced off RUN_PAUSED, degrading
the words to the queue. The entry now also disables on
!duringRunActive, matching the queued-row control's gate.

Counterfactually verified: reverting the gate fails the new
answer-mode test.

* fix: recheck live run state after the reclaim, not just at the click

Codex round 3, one P2, and it is the round-1 recheck principle applied
one level deeper: the entry-time disable cannot see a run that pauses
(tool approval, answer mode) while the reclaim round-trip is in flight,
and the .then closure held the render's stale steering controls, so the
resubmit would fire into a RUN_PAUSED rejection after the reclaim had
already surrendered the steer's boundary slot.

The escalation continuation now reads the LIVE controls through a
latest-ref: if the run can no longer accept a steer, the words re-home
to the queue with an informational toast instead of resubmitting, and
the resubmit itself also goes through the live controls.

Counterfactually verified: reading the stale closure instead of the ref
fails the new mid-reclaim pause test.

* refactor: make escalation one atomic server-side arm, in place

Codex round 4: four P2s, every one an interleaving of the same window —
escalation as reclaim-then-repost is a compound, non-atomic operation
whose continuation must revalidate the world (FIFO position lost, ref
assigned too late, no run fence, competing bubble actions). Rounds 1-3
patched that window with a lock and rechecks; round 4 shows the window
itself is the defect, so this removes it instead of guarding it again.

Escalation is now POST /chat/steer/arm: the server flips preempt on the
EXISTING queued item in one atomic store op (new IJobStore.armSteer; a
decode-patch-encode LSET Lua on Redis, an in-place mutation in memory),
fenced to the validated generation and refused once the queue closes.
The handler mirrors the steer POST's preempt contract exactly: durable
flag gated on the owner's recorded capability, volatile requestPreempt
fire-and-forget because the durable flag is the truth resume/handover
re-arm from.

By construction this resolves all four findings: FIFO survives (the
item never moves; the whole queue still drains in instruction order at
the seal), there is no continuation to hold stale controls, the store
op is fenced to the original run, and a competing Edit/Queue/Cancel
either beats the arm (armed:false, chip untouched) or operates on the
armed item, whose cancel already disarms.

The client escalation entry becomes one mutation: armed:true relabels
the chip in place (same steerId, same position), PREEMPT_UNSUPPORTED
and lost races toast honestly, and the round 1-3 machinery — the
escalating lock atom, the latest-ref, the post-reclaim rechecks and
their two toast strings — is deleted rather than extended.

Verified: 7 new handler tests on the real in-memory manager (including
FIFO preservation and the stale-generation fence), 2 Redis integration
tests against real Redis (in-place arm keeps order and every field;
missing/stale/closed all refuse), client suites 396 green.

* fix: decide capability inside the atomic arm, neutralize the lost-race toast

Codex round 5, both findings, both edges of the new arm design rather
than its mechanism.

P2, capability TOCTOU. A HITL resume on a rolling deploy rewrites
preemptCapable for the SAME generation, so the handler's read could go
stale between validation and the flag flip, arming a steer the live
owner cannot seal. armSteer now returns armed | missing | incapable,
with the owner's live capability part of the same atomic predicate as
the generation fence (HGET preemptCapable inside the Lua; the flat job
field, not a metadata blob — the in-memory store reads the same field).
The handler's pre-check is deleted rather than kept alongside; the
store predicate is the single source. New handler test rewrites the
capability after queueing and expects PREEMPT_UNSUPPORTED with the item
left unflagged; the Redis guards test now asserts the incapable refusal
against real Redis.

P2, ambiguous toast. armed:false covers injected, cancelled, re-homed,
and run-over alike, so telling the user the message "already reached
the agent" claimed one specific outcome. The lost-race branch now uses
a neutral message (com_ui_steer_arm_lost_race) and defers to the events
for what actually happened.

* fix: flip the escalation lock synchronously before the arm request

Codex round 6, one P2. Round 4 deleted the escalating flag along with
the reclaim continuation it guarded, but that left the one-interrupt
gate blind during the arm request's own round trip: the chip-derived
check cannot see an arm until its response relabels the chip, so on a
slow connection two bubbles could both arm before either response
landed. Double-arm is harmless server-side now (the run seals once and
drains the whole queue in order), but every escalation control
advertises "one interrupt at a time" by disabling, and the controls
must tell the truth.

The per-conversation escalating flag returns as a pure UX gate: set
synchronously at click, before the mutation, cleared on settlement, and
folded into interruptPending on both surfaces. Unlike its round 1-3
ancestor there is no continuation behind it to guard and no recheck to
pair with it.

Counterfactually verified: without the synchronous set, the two-bubble
race test arms twice. 207 tests green across the Chat Input suites.

* test(e2e): cover escalation of waiting messages through the real seal

Three mock-harness tests on E2E_SLOW_REPLY, a 160-chunk stream with no
tool boundary, so an in-thread steer part can ONLY come from a genuine
mid-stream seal — which makes each test a behavioral proof rather than
a UI check:

- Queued row escalation: the ZapOff button turns a waiting queued
  message into a preempt-armed steer (202 echoes preempt: true) that
  seals and injects, where the sibling steering.spec test proves the
  unescalated path waits for run end instead.
- Bubble in-place arm: an ordinary steer (202 with no preempt echo)
  waits as a bubble, POST /chat/steer/arm answers armed: true, the
  bubble relabels in place (same single bubble, same text, escalation
  no longer offered on reopen), and the armed steer seals mid-stream.
- Always-interrupt toggle: flipped from a waiting row's overflow menu,
  plain Enter now produces a preempt: true steer that seals in the SAME
  run, and the menu offers the way back. An afterEach clears the
  localStorage preference so a mid-test failure cannot leak
  preempt-by-default into the rest of the serial suite.

All three verified locally through the full harness (real backend, mock
LLM, seeded DB): 3 passed in 27s.

* feat: dedicated escalation arrow + shortcut, menu split into actions and preferences

The escalation was still half-hidden: the bubble only offered it inside
the overflow menu, and the tooltip taught the composer chord, which does
a different thing (interrupts with typed text, not this chip). Three
changes make it a first-class command:

- A shared EscalateNowButton (circular arrow, ghost-bordered like the
  composer's interrupt control) is always visible on BOTH surfaces:
  beside each queued row's Steer primary and on every waiting steer
  bubble next to its menu. It disappears once a steer is interrupting.
- A dedicated registry shortcut, escalateSteer (Cmd/Ctrl+Shift+.),
  editing-allowed and rebindable like every other action. Deliberately
  NOT an Enter chord: the composer owns every Enter chord, and the
  yield design rests on no default binding using Enter besides submit.
  Its handler clicks the newest enabled arrow control (bubbles beat
  queued rows), so the shortcut can never diverge from the button, and
  the arrow's tooltip teaches THIS command via the registry display.
- The overflow menus separate one-off actions from sticky behavior
  changes: Edit, Cancel, Queue, then a smaller "Preferences" section
  holding the queueing and always-interrupt toggles, each with the
  standard InfoHoverCard reusing the Settings panel's descriptions.
  "Interrupt & steer now" leaves the menu entirely.

386 client tests green, including a menu-structure test locking the
order and the absence of the escalation entry; bubble escalation tests
drive the visible arrow. The e2e spec's bubble test now clicks the
arrow, and a fourth test drives the dedicated shortcut end to end
through a real mid-stream seal.

* style: bind the escalation arrow to its message (variant A anatomy)

Two same-weight circles in a row read as one control group, leaving the
arrow's ownership ambiguous, and a floating arrow stops meaning anything
once several messages stack. The shared control now carries variant A's
anatomy: a thin divider binds a small SOLID arrow (filled, inverted) to
the message region on its left, and the menu ellipsis stays a bare
glyph, so the two affordances can no longer blur together — and the
divider+arrow pairing repeats cleanly per chip at N messages.

* chore: drop the unused within import CI lint caught

* fix: advertise the escalation shortcut only while the control is live

Codex on the e2e head, one P2: the tooltip appended the chord hint even
while the button was disabled, advertising a shortcut that does nothing
during an approval pause. The flagged control (InterruptNowButton) was
since replaced by the shared EscalateNowButton, which inherited the
pattern; the successor now omits the chord whenever the control is
disabled, matching the rule the during-run hovercard already follows.

* fix: harden steer escalation lifecycle and recovery

* test(e2e): disambiguate accessible steer preferences

* test: align abort persistence coverage with prerequisites

* chore(i18n): remove obsolete steer race message

* chore: normalize imports across steering changes

* test: exercise stream integration on Redis Cluster

* test: scope HITL checkpoints to generation

* test: fix cluster cleanup and locale policy

* fix: keep escalation visible during ask pauses

* fix: fence recovery downgrade and stale predecessors

* fix: require generation owner abort acknowledgement

* fix: validate delayed preempt arms

* test: align final escalation fixtures

* fix: preserve in-memory predecessor abort handoff

* fix: restore controls for recovered queued messages

* test: cover recovered queue controls

* fix: close final steering review gaps
2026-07-31 20:07:56 -04:00
Danny Avila
7bb6651883
🛑 feat: Preemptive Steer - Backend Interrupt & Steer (#14518)
* 🛑 feat: Preemptive Steer — server half (Interrupt & Steer, PR 2 of 3)

Lets the steer route ask the generating replica to seal its live model
stream at the next provider-safe boundary instead of waiting for a tool
step. The run is never aborted, job status never changes, the partial
answer is kept, and generation resumes in the same assistant message
after the injected steer. Consumes the SDK seam in @librechat/agents
(danny-avila/agents#335, #346).

Transport: IEventTransport gains a fenced emitPreempt/onPreempt pair
beside abort. RedisEventTransport fans PREEMPT out on the SAME events
channel and subscription (no new connection, key, or subscribe call);
onPreempt returns a registration-scoped unsubscribe with the same
replacement-safe state-identity guard onAbort uses. InMemory implements
neither — single-process preempt lives entirely in the runtime set.

Runtime state: RuntimeJobState carries the per-generation request set,
createdAt-fenced and capped at STEER_QUEUE_MAX_DEPTH, plus a bounded
`cleared` tombstone so a late cross-replica arm cannot resurrect a
request whose steer already drained. registerPreemptSubscription
mirrors the abort registration's double fence (runtime identity +
generation createdAt); releaseAbortSubscription retires BOTH listeners
and the armed set, so every terminal path drops preempt state for free.
Public surface: requestPreempt (arm + fenced publish, never a rejection
surface, never touches job status), isPreemptRequested (O(1)
level-triggered poll), noteSteersRemoved (drain/cancel bookkeeping +
fenced clear), clearPreemptRequests (empty-boundary disarm).

One drain body, two boundaries: createSteerDrainHook (PostToolBatch)
and createSteerPreemptBoundaryHook (PreemptBoundary) share
drainAndBuildInjections, so the two injection sites cannot drift — the
SDK's provider-safety argument rests on identical HumanMessage shapes.
The shared body builds injections incrementally under a swallow-all
catch (a mid-loop throw still injects what was applied — those parts
are already persisted), clears preempt requests in finally, and
disarms the generation when a boundary drains nothing.

Request path: POST /chat/steer accepts preempt: true. The guard ladder
is unchanged in order and in every status code. A preempt request is
NEVER a rejection reason — without the capability the steer still
enqueues and the 202 echoes preempt: false. Armed strictly after a
successful enqueue; cancel disarms. The capability is read from the
OWNING replica's recorded `preemptCapable` rather than the route
replica's own SDK probe, so a rolling deploy cannot label a steer
"interrupting" that the older owner will only inject at a tool step.

Durable label: SteerQueueItem.preempt → TPendingSteer.preempt, so a
parked/claimed/replayed chip keeps its wording.

Run wiring: createRun registers the PreemptBoundary hook and threads
RunConfig.preemption, both gated on isSteerPreemptSupported() — a
separate probe from isSteeringSupported(), so the client affordance can
never arm against an SDK that only injects at tool boundaries.
buildSteerWiring builds both hooks from one shared closures object, so
preemption survives HITL pause/resume for free.

Honest finalization: an empty preempt boundary persists and emits with
unfinished: true — the same contract an abort gets — re-marked
explicitly because BaseClient has already saved the row as
unfinished: false by that point.

Not changed: no new job status, store method, Lua, SSE event type,
endpoint, or authorization surface. abortJob, completeJob,
transitionStatus, closeAndDrainSteers, getResumeState, emitChunk,
applySteerPart and the whole abort path are untouched.

Tests: 120 packages/api steering specs (preempt lifecycle, tombstone,
fences, caps, terminal release, both-boundary drain parity,
level-triggered poll, request/cancel arming, owner-capability
degradation) plus 5 in api for buildSteerWiring gating, and 2
Redis-gated cross-replica transport specs.

* 🔒 fix: Codex round 2 — evict tombstones, scope the empty-boundary disarm, honest resumes

All four server findings were fresh consequences of the round-1 fixes,
which is the review doing exactly what it should.

- Tombstone cap refused new entries instead of evicting. Every drained
  or cancelled steer is tombstoned, not just preempting ones, so a
  generation that processed 20 steers exhausted the set and the
  late-arm race resurfaced silently. Now evicts oldest-first (Set
  iteration is insertion-ordered), with the budget named
  PREEMPT_TOMBSTONE_MAX rather than an inline expression.

- The empty-boundary disarm I added in round 1 wiped the generation's
  ENTIRE armed set. A second steer can enqueue and arm between the
  atomic drain returning empty and the disarm running — that arm is
  backed by a live, uninjected queue item and must survive. The drain
  now snapshots the armed ids BEFORE draining
  (getArmedPreemptIds) and clearPreemptRequests takes an explicit id
  list instead of clearing everything.

- HITL resume finalized with a hardcoded unfinished: false. The
  boundary hook is re-registered on resume via buildSteerWiring, so a
  resumed segment can end on an empty preempt boundary exactly like a
  fresh one; finalizeResumedTurn now reads getPreemptStats() and the
  halt reason, matching the normal request path.

- Ownership moves on resume, so the job's recorded preemptCapable must
  describe the replica that will actually generate. Refreshed before
  resumeCompletion; a job created on a capable replica that resumes on
  an older one during a rolling deploy no longer acknowledges steers as
  interrupting.

Tests: +3 (scoped disarm sparing a post-snapshot arm, oldest-first
tombstone eviction, id-list disarm). 122 packages/api steering specs
green.

* 🚨 fix: Codex round 3 — deserialize preemptCapable from Redis (feature was dead under Redis)

The P1 here is the most consequential defect in the whole feature, and
it was introduced by round 1's own capability fix.

- `RedisJobStore.serializeJob` writes booleans generically, so
  `preemptCapable` reached Redis — but `deserializeJob` is an EXPLICIT
  field map and had no line for it. Every `getJob()` therefore dropped
  the flag, `job.metadata.preemptCapable` was always undefined, and
  `handleSteerRequest` computed `preemptArmed: false` unconditionally.
  Interrupt & steer would have silently degraded to ordinary
  tool-boundary steering in EVERY Redis deployment — i.e. the feature
  shipping as a no-op in production while passing every in-memory test.
  Now deserialized, with a round-trip assertion in the metadata spec
  that fails (`Received: undefined`) against the unfixed store.

- The resume capability refresh moved from just-before
  `resumeCompletion` to immediately after `approvals.resolve` claims
  the run. That call already flips the job back to `running`, so the
  steer route accepts requests from that instant; leaving the refresh
  135 lines later (across the whole client reconstruction) left a real
  window where a steer read the PREVIOUS owner's capability. Not the
  fully atomic transition Codex suggested — that reaches into the
  approvals Lua — but it shrinks the window from seconds to one await,
  which is proportionate for a label-accuracy issue.

Refuted: "avoid triggering preemption inside subagents". The premise —
that the run-wide poll can seal a subagent stream — does not hold
against the shipped SDK. Child graphs are constructed with
`subagentScope: true` (SubagentExecutor) and `preemption` is NOT
propagated into child inputs, while `canClaimPreemptSeal()` requires
`!subagentScope && preemption != null`. Both conditions fail
independently, so a subagent can never claim a seal and the boundary
cannot fire with `agentId` set. The `input.agentId != null` guard in
the hook is defensive depth, not the thing standing between us and the
described failure.

140 packages/api specs green.

* 🔁 fix: Codex round 4 — re-arm durable interrupt steers when resume moves owners

- An arm lives only in the owning replica's runtime plus a transient
  pub/sub message, while the steer's `preempt` flag is durable on the
  queue item. A HITL resume landing on a different replica therefore
  started with an empty armed set and a poll stuck false, so an
  interrupt the user had already been ACKed for silently waited for an
  ordinary tool boundary. New `GenerationJobManager.rearmQueuedPreempts`
  rebuilds the armed set by peeking the durable queue (fenced on the
  generation) and re-arming every item flagged `preempt`; resume calls
  it right after claiming. Safe by construction: every item peeked is
  still queued, so no drained steer can be resurrected.

- Capability-refresh failure now logs at error rather than warn, but
  deliberately does NOT fail the resume — see the reply on that thread.

Tests: +2 (rebuild from queue arms only the flagged item and reports
the count; a stale generation arms nothing). 124 packages/api steering
specs green.

* 📡 fix: Codex round 5 — acknowledge only what was actually armed

- A cross-replica arm was fire-and-forget: `emitPreempt` logged its own
  publish failure and `requestPreempt` returned void, so the route
  answered `preempt: true` even when the owner never armed a poll. The
  steer still injected at the next tool boundary, but the chip claimed
  an interrupt that could not happen — and unlike HITL resume, an
  ordinary running generation had no durable reconciliation to recover
  it.

  `emitPreempt` now resolves to the subscriber count and rejects on
  failure; `requestPreempt` is async and returns whether the arm truly
  landed (owned locally, or delivered to at least one subscriber). The
  202 reports THAT rather than what was asked for, so the chip relabels
  to ordinary steering exactly as it does for a capability-degraded
  deployment. Errors are swallowed into `false` — an unarmed interrupt
  is a downgrade, never a failed steer.

- The owner capability is re-read immediately before enqueue rather
  than reused from the top of the guard ladder. `checkAgentAccess` and
  file resolution are awaits, so a request can span an entire HITL
  pause/resume that moves ownership to a replica with different
  capability and rewrites that very flag. Only paid for by requests
  that actually asked to interrupt.

Tests: +3 (not-armed when the publish reaches nobody; armed when this
replica owns the generation; a throwing publish downgrades instead of
propagating). 127 packages/api steering specs green.

* 🎯 fix: Codex round 6 — real ownership, confirmed disarms, and a CI regression of my own

Three review findings plus three CI failures the round-5 commit caused.

Review:
- Ownership came from `runtimeState`, which a cross-replica `getJob`
  populates with a FACADE runtime on any replica that merely read the
  job. Matching `createdAt` therefore proved only "we looked at this
  job", so a non-owner could arm nothing and report success. Ownership
  now comes from `ownedJobs`, the actual owner map.
- `armPreemptIds` returns how many ids it accepted, and a local arm is
  only reported as armed when one was. A tombstoned id (its steer
  drained at an ordinary boundary mid-request) no longer answers
  `preempt: true` for an interrupt that cannot happen.
- The cancel disarm is awaited. A dropped clear is worse than a dropped
  arm: the owner keeps a level-triggered request for a steer that no
  longer exists, seals its next chunk and truncates an unrelated
  answer. The boundary drain's own call stays non-blocking — there the
  owner is local, so the disarm is already effective and awaiting the
  informational publish would only delay injection.
- Subscriber count is NOT read as proof of owner receipt: the count
  includes this replica's own facade subscription. A successful publish
  reports armed, a rejected one does not. Documented rather than
  papered over — see the acknowledgement-semantics note on the PR.

CI regressions from round 5, all mine:
- `registerPreemptSubscription` was AWAITED at both runtime-init sites,
  so job creation blocked on a second Redis channel subscription and
  hung when that subscribe was slow. Abort is awaited because a missed
  abort strands a run; a missed preempt only degrades that steer to the
  next tool boundary, so it now registers without gating createJob.
- Two api specs mocked `@librechat/api` without the newly imported
  `isSteerPreemptSupported`, so the call threw before createJob; and one
  exact-match assertion needed the new `preemptCapable` metadata field.
- My own Redis integration spec asserted arm-before-clear ordering,
  which two publishes carry no guarantee of — the receiving tombstone
  exists precisely because of that. Now asserts delivery and payload
  fidelity, order-independent.

158 packages/api specs, 27 api specs green.

* 🧭 fix: Codex round 7 — settle the acknowledgement semantics (Option A)

Round 7's second finding is the incoherence I flagged on the PR: the
route persisted `preempt: true` on the durable queue item while
returning `preempt: false` when delivery could not be confirmed. Those
two then disagreed, and `rearmQueuedPreempts` trusts the DURABLE one —
so a resumed owner would honour an interrupt the client had explicitly
been told degraded to ordinary steering.

Rather than patch the disagreement, this settles the meaning:

`preempt` in the 202 means "queued as an interrupt request", NOT "a
seal is guaranteed". It mirrors `SteerQueueItem.preempt` exactly, so
the response, the durable record, and the resume-time re-arm can never
disagree. The gates that ARE knowable stay — the owner's recorded
capability and a successful enqueue. Everything past that degrades to
the documented fallback of injecting at the next tool boundary.

A route cannot synchronously know whether another replica will seal:
proving it needs a correlated request/response over pub-sub, and even
that only proves the owner heard, not that it is still streaming when
the arm lands. Four rounds of tightening this boolean each surfaced a
narrower case; the sequence does not converge, so the invariant is now
"the flag describes the durable decision" and an unconfirmed arm logs a
warning instead of rewriting the answer.

Also from this round: a failed disarm publish is retried once and its
outcome reported. `handleSteerCancel` keeps `removed: true` — the steer
really did leave the queue, and saying otherwise would make the client
re-show a chip for a steer that can never arrive — and adds
`disarmed: false` so the residual risk is visible rather than swallowed.
Damage stays bounded regardless: the empty-boundary self-clear disarms
the generation after a single seal.

Tests: +1 pinning the response/durable-flag invariant. 159
packages/api specs green.

* 🧹 fix: Codex round 8 — remove the unverifiable disarm signal

Round 8 found the same over-promise on the disarm side that round 7
corrected on the arm side, so this applies the same answer rather than
patching around it.

The `disarmed: false` field added in round 7 was both unreliable and
unused: a resolved publish is not proof the owner heard it (the
delivery count includes this replica's own facade subscription), and it
was never threaded into `CancelSteerResponse` or read by any client. A
signal that claims a certainty the transport cannot provide is worse
than no signal — it invites callers to trust it.

Removed from the response. The retry stays, because it genuinely
reduces the failure rate, and `noteSteersRemoved` still returns whether
the publish succeeded FOR LOGGING, now documented explicitly as
"published without error", not "the owner disarmed".

Disarm is best effort with a bounded, self-healing failure: if the
clear is lost the owner seals once, the empty-boundary self-clear
disarms the generation, and the turn is persisted `unfinished: true`
rather than silently truncated. Tightening that further needs a
correlated request/response over pub-sub with a timeout — noted on the
PR as the deliberate boundary of this design rather than an oversight.

130 packages/api steering specs green.

* 🧽 fix: Codex round 9 — spend snapshot arms on nonempty drains too

The round-6 scoping fix only cleared the pre-drain snapshot when the
drain came back EMPTY. On a nonempty drain the `finally` cleared just
the drained ids, so a stale arm — typically a cancel whose
cross-replica clear was lost — survived the boundary. It would then
immediately seal the continuation meant to answer the steer that had
just been injected, and land on an empty boundary as
`preempt_incomplete`: the interrupt appears to work, and the answer to
it is truncated.

A boundary that runs has spent its seal, so everything armed at
snapshot time is spent whether or not it came back from the drain. The
`finally` now clears the union of the snapshot and the drained ids.
Arms that land AFTER the snapshot are still spared — their queue items
are live and uninjected, which is the property round 6 added.

Also fixes an api-workspace CI failure of mine: `resume.spec.js` stubs
`GenerationJobManager` wholesale, and the round-3/4 resume work added
two calls (`updateMetadata`, `rearmQueuedPreempts`) the stub did not
define, so 34 specs threw. Stub extended.

Tests: +2 (a nonempty drain clears a stale snapshot arm; a nonempty
drain spares an arm that landed mid-drain). Counterfactually verified —
the stale-arm spec fails against the unfixed drain. 132 packages/api
specs, 60 resume specs green.

* fix: never let a failed preempt subscription reject into the void

registerPreemptSubscription is called detached at both sites, so a
rejected Redis SUBSCRIBE became an unhandled rejection — process-fatal
under Node's default --unhandled-rejections=throw. The comment already
promised this path merely degrades steering; it now does.

Swallowed and logged inside the registration rather than at each call
site, so a future third caller cannot reintroduce the trap. Losing the
channel costs this generation's cross-replica preempts, not the server:
same-replica arming is runtime state and still works, and remote arms
fall back to the next tool boundary.

Verified counterfactually — the new spec surfaces SUBSCRIBE failed as an
unhandled rejection against the unfixed registration.

* docs: state the real blast radius of a failed preempt subscription

LibreChat's own entrypoints install a global unhandledRejection handler
that logs and keeps serving, so the escaping rejection this guards was
never fatal to this server — only to another consumer of @librechat/api
that installs no handler. The fix stands either way; the comment just
should not overstate what it prevents.

* test: cover the cross-replica preempt hop with two manager instances

Every other preempt test runs against a single manager, so the hop that
actually carries an interrupt in production had no coverage: the steer POST
lands on whichever replica the balancer picks, which is usually not the one
generating. Non-owner publishes, owner arms, owner's level-triggered poll
flips — none of that was exercised end to end.

Two GenerationJobManagerClass instances are a faithful replica pair here.
runtimeState and ownedJobs are private instance fields, there is no
module-level mutable state between them, and createStreamServices duplicates
a dedicated subscriber connection per call, so separate OS processes would
exercise the same objects over the same Redis.

Both assertions verified counterfactually against real Redis:
- Deleting the preemptCapable deserialization in RedisJobStore fails this
  with 'Expected: true, Received: undefined' — the exact P1 that shipped past
  every in-memory test and would have made the feature a silent no-op on
  every Redis deployment.
- Dropping the non-owner arm publish fails it with 'Received: false'.

* test: remove the fixed sleeps and vacuity from the cross-replica preempt test

Codex round 11, both findings, both on the test I added last commit.

P2 — the 300ms waits were load-bearing. Redis pub/sub never replays and the
owner's SUBSCRIBE is detached, so on a slow CI worker the publish could land
before anyone was listening and the test would fail against correct code.
Now it republishes until the owner's state converges, which is safe because
arms and clears are idempotent set writes keyed by steerId. Side effect: the
tests got ~10x faster (85ms/57ms vs 929ms/606ms) since they finish on
delivery rather than on a timer.

P3 — afterEach destroyed only the transports, leaving each manager alive in
its own cleanup-interval closure, still working against a dead transport.
Now tracks the managers and awaits destroy(), which disposes the job store
and its timer too. Matches how the rest of this file cleans up.

Fixing the sleeps exposed a third problem codex did not flag: the stale-arm
test could pass vacuously, because an undelivered arm and a fenced one look
identical. It now brackets the stale publish between two control arms — the
first proves the owner is listening before the stale one is sent, the second
proves it has had its chance to arrive.

Verified counterfactually against real Redis, and stable over 5 runs:
- dropping the preemptCapable deserialization fails with 'Received: undefined'
- dropping the non-owner arm publish times out both tests
- removing the generation fence fails the stale test with
  ["control-before", "steer-stale", "control-after"] — which also confirms
  the bracketing orders as intended rather than by luck

* fix: gate interrupt on the OWNER's capability alone, not the route's

Codex round 12. The comment above this gate already said 'the OWNER's
recorded capability, not this replica's probe' — and then the code ANDed in
isSteerPreemptSupported(), which is exactly this replica's probe. The
contradiction dates to the original commit; round 6 made the gate
owner-scoped and wrote that comment without removing the local conjunct.

The route never seals. It enqueues and publishes an arm, neither of which
touches the SDK, so during a rolling deploy a steer landing on an
un-upgraded replica silently lost its interrupt even though the owner could
seal. When the route IS the owner the probe is redundant anyway: the flag it
would consult is the one this process wrote at createJob.

The real degradation path is unchanged and still tested — an owner that
recorded no capability relabels to an ordinary steer. The test that pinned
the local probe asserted an impossible same-replica state (capable metadata
plus an incapable local SDK, when the metadata is written from that probe);
it now pins the mixed-SDK direction instead, and fails with
'Expected: true, Received: false' if the probe is put back.

* fix: reconcile arms at handover, and stop holding the 202 on a publish

Codex round 13, two of three findings.

P2 — rearmQueuedPreempts only ever ADDED. A replica that merely read the job
still installs a facade runtime and subscribes, so it can accept an arm and
then miss the best-effort clear that follows the drain. HITL resume promotes
that facade to owner, the union keeps the orphan, and the first resumed
stream seals on a steer no longer in the queue, drains nothing, and
truncates the resumed answer as preempt_incomplete. acquireResumedJobOwnership
only sets ownedJobs, so nothing else was clearing it. The durable queue is
the sole authority at a handover: arms it does not back are now disarmed and
tombstoned, so an in-flight publish cannot revive them either.

Worth recording that my own independent review raised this and my verifier
refuted it. Codex found it separately; two reviewers converging should have
outweighed one refutation.

P2 — the route awaited the arm publish before answering. The 202 reports
capability, not delivery, so the await could not change the response; it only
exposed the caller to Redis latency after the queue item was already durable.
A client that times out and retries mints a second steer while the first
stays queued, injecting the same instruction twice, whereas a lost publish
merely takes the tool-boundary fallback. Detached, with both outcomes logged.

All three tests verified counterfactually: union-only rearm fails the two new
handover specs, and re-awaiting the publish hangs the stalled-publish spec
until jest kills it.

* fix: snapshot arms before reading the queue at handover

Codex round 14 — a regression from my own round-13 fix, and a worse failure
than the one it corrected.

Round 13 read the durable queue first, then tombstoned any armed id the
snapshot did not back. But approvals.resolve reopens steering before
reconciliation runs, so another replica can commit a preempt steer and
publish its arm while the peek is in flight. That arm is then present locally
but absent from a snapshot taken before the steer existed, so a LIVE
interrupt the route already acknowledged got dropped — and tombstoned, which
blocks the re-arm, making it unrecoverable rather than merely late.

Fixed by inverting the two reads rather than by locking or paying a second
round trip. A steer is durably enqueued BEFORE its arm is published, so any
id in an arms-first snapshot was already queued when it was armed, and the
later peek must observe it unless it has since drained — which is exactly the
orphan this reconciliation exists to drop. Arms landing after the snapshot
are simply not candidates.

Also re-checks runtime identity across the await, since the generation can be
replaced while the queue read is in flight.

New spec injects a steer + arm during the peek and verifies it survives;
against the round-13 ordering it fails with Received array: [].

* fix: bound the cancel disarm wait and fence enqueue to its generation

Codex round 15.

P2 — the cancel awaited its disarm publish unbounded. ioredis queues
commands during an outage rather than rejecting, so that await could hang for
the length of the outage with the steer ALREADY durably cancelled; a client
that gives up then treats the cancel as failed and restores a chip for a
steer that can never produce an applied event. Every successful cancel
publishes, so ordinary steers were exposed too, not only preemptive ones.
Now bounded at 1s, with the publish continuing behind it — its retry and
logging are unchanged, it is just no longer in front of the response. This is
the sibling of round 13's arm-publish finding; I fixed one path and left this
one.

P3 — enqueue was not fenced to the generation the capability decision was
made against. The access checks, file resolution and owner re-read are all
awaits, so the run can be replaced before the enqueue: the item then lands on
the REPLACEMENT queue while the durable preempt flag and the arm still name
the previous epoch, the arm is fenced out at the owner, and the 202 promises
an interrupt that cannot happen. enqueueSteer now takes an expected
generation, mirroring drain/peek, and the Redis path enforces it inside
STEER_ENQUEUE_LUA so the check is atomic with the push rather than racing it.

All three new specs verified counterfactually, including the Lua guard
against real Redis (removing it returns 1 where -1 is required).

* fix: fence the steer to its authorized generation, bound resume setup, keep preempt when Redis parks

Codex round 16, all three findings.

P2 — round 15 fenced the enqueue to owner.createdAt, the RE-READ job. Every
guard above it (ownership, tenant, paused-state, agent ACL) ran against the
job read at the top, so if the run was replaced during those awaits the fence
happily accepted the steer into a generation the request was never authorized
against, carrying the wrong agent's metadata. Now rejects on any mismatch
between the validated job and the re-read.

P2 — resume awaited its steering bookkeeping unbounded, after
approvals.resolve had consumed the action and flipped the job to running, and
outside the resume lifecycle's own try/finally. .catch does not fire on a
promise that never settles, which is what ioredis produces during an outage,
so the client times out, its retry gets a 409 for a spent action, and no
cleanup runs. Bounded at 1s with the writes finishing in the background.

P3 — Redis parks leftover steers inside its terminal-transition Lua, which
projects item fields one by one, so preempt was silently dropped and a steer
recovered from /chat/status lost its interrupting label. Added to both
projections.

All three verified counterfactually, two against real Redis. Worth recording
that my first version of the generation-mismatch test was VACUOUS — it faked
a createdAt matching no live job, so the round-15 enqueue fence rejected it
for the wrong reason and the test passed with the guard removed. Rewritten to
replace the run for real; it now fails with 'Expected 404, Received 202'.
2026-07-30 12:36:13 -04:00
Danny Avila
73699b5c25
perf: Reduce Agent Chat Startup Latency (#14423)
* perf: reduce agent chat startup latency

* test: align Redis stream readiness assertions

* perf: overlap remaining agent startup work

* perf: persist initial agent job metadata atomically

* test: add agent startup latency benchmark

* fix: harden resumable agent stream lifecycle

* fix: isolate replacement stream lifecycles

* fix: preserve terminal stream epochs
2026-07-25 07:58:20 -04:00
Danny Avila
9e245aced4
🎟️ fix: Claim Idempotency Keys to Dedup Retried Generation Requests and Prevent Double Billing (#14344)
* 🐛 fix: Dedup retried start-generation requests to prevent duplicate billing

A lost or reset start-generation response makes the client re-POST the
identical payload (up to 3x on network errors). The resumable-stream
controller had no idempotency: createJob unconditionally overwrote the
running job without aborting the prior one, so both requests ran full
LLM completions and both billed while the UI showed only one (#14339).

Add a stable per-submission clientRequestId (uuid, fresh per ask() so a
regenerate differs, reused across the start-generation retries) and an
atomic claim on the job store keyed by userId:clientRequestId. The first
request wins and generates; a retried POST loses the claim and receives
the original stream, which the client subscribes to and replays - no
second billed generation.

- IJobStore.claimIdempotencyKey/releaseIdempotencyKey (in-memory Map+TTL,
  Redis single-key SET NX PX + GET Lua, cluster-safe)
- GenerationJobManager.claimGeneration/releaseGeneration (20m TTL)
- Controller claims before the concurrency check, dedups with a resumed
  response, releases on start-failure/429
- clientRequestId threaded through TSubmission/TPayload/createPayload

* 🐛 fix: Harden start-generation dedup (Codex review)

Address three P2 findings on the idempotency path:

- Resume replay: a deduped retry now subscribes with resume=true so the
  client replays prior content and any pending-action from the running
  stream instead of only live events (cross-replica / HITL correctness).
  startGeneration returns { streamId, resumed } and the response's
  status:'resumed' drives the subscribe mode.
- Wait for the job record: a duplicate that loses the claim now waits
  briefly for the winner to create the job before returning the stream
  (a stream with no job 404s terminally). If the winner has not
  materialized, return 503 SERVER_NOT_READY so the client retries via the
  existing readiness path instead of attaching to a dead stream.
- Release only owned claims: track whether the request actually won the
  claim; the 429 and init-error paths no longer release a claim owned by
  another in-flight generation (fail-open path could erase it and
  re-enable double billing).

Adds controller tests covering dedup, the 503 race fallback, win-then-
create, and claim-release ownership on 429 / fail-open.

* 🐛 fix: Don't trap deduped retries on missing job records (Codex review)

The previous round returned 503 SERVER_NOT_READY when a deduped retry's
job record was absent. But a missing job usually means the original
generation already completed and was cleaned up (cleanupOnComplete) — the
correct recovery is to return the stream and let the client's subscribe
404 handler refetch the persisted messages. The 503 instead trapped the
send in a readiness-retry loop until the client's window expired.

Keep the bounded wait (it still covers the job-about-to-be-created race)
but always return the resumed stream afterward; a gone/never-created job
recovers via the client's existing 404 path instead of being treated as
indefinitely starting. Updated the controller test accordingly.

* 🐛 fix: Gate deduped resume on claim age, not just job presence (Codex review)

Removing the 503 entirely (previous round) reintroduced the inverse race:
if the winning request stalls between claimGeneration and createJob, a
losing duplicate saw no job, returned status:'resumed' anyway, and the
client subscribed to a stream that did not exist yet — the 404 handler
tore the turn down while the winner went on to generate and bill with no
UI attached.

Distinguish the two missing-job cases by claim age (claimedAt now travels
on the claim value):
- fresh claim, no job yet → winner is still starting → 503 SERVER_NOT_READY
  so the client retries via the readiness path (bounded, not indefinite).
- old claim, no job → the original already completed and was cleaned up
  (or the winner died) → attach; the client's 404 handler refetches.

Tests cover both age branches.

* 🐛 fix: Scope dedup fail-open + keep resumed convos on 404 (Codex review)

- Fail-open only on claim acquisition: a store error while checking an
  already-confirmed existing claim no longer falls through to createJob
  (which would start a second billed generation during a Redis hiccup).
  Once claim.existing is known, a job-lookup error returns 503 retry.
- Don't drop a resumed convo on 404: the optimistic-conversation cleanup
  in useResumableSSE now runs only for fresh (non-resume) subscribes. A
  deduped resume whose original completed and was cleaned up 404s, but its
  conversation is persisted and must stay in the sidebar.

Adds a controller test for the job-lookup-error path (503, no createJob).

* 🐛 fix: Reconcile resumed convos on 404 instead of guessing (Codex review)

Round-4's !isResume guard fixed the completed-and-cleaned case (don't drop
a persisted convo) but left the inverse: a new-conversation retry deduped
to a claim whose original worker died before persisting still resumes,
404s, and — with removal skipped — leaves a phantom /c/<streamId> sidebar
entry.

Stop guessing keep-vs-remove on a resume 404. Reconcile against the
server: invalidate the conversations list so a real (persisted) convo
stays and a phantom is dropped. Fresh (non-resume) optimistic streams
still prune immediately. Adds a client test for the resume path.

* 🐛 fix: Finalize failed job before releasing its claim (Codex review)

In the initialization-error catch, the idempotency claim was released
before completeJob(streamId). A racing retry could win the released key
and createJob() the same streamId while this catch was still running, and
completeJob() (not guarded by the original createdAt) would then abort the
replacement. Finalize the failed job first, then release the claim.

Adds a controller test asserting completeJob precedes releaseGeneration.

* 🐛 fix: Clear claims on destroy + survive completeJob failure (Codex review)

- InMemoryJobStore.destroy() now clears the idempotencyClaims map, so a
  reused/reconfigured store instance doesn't dedup a fresh start against a
  torn-down job's stale claim.
- Init-error cleanup: completeJob() is swallowed so a store-hiccup
  rejection can no longer skip the idempotency-key release and the
  pending-request decrement (which would wedge the retry behind the claim
  and leak the concurrency slot). A failed completeJob finalized nothing,
  so releasing afterward still can't abort a later replacement.

Tests: claims cleared on destroy; release + pending decrement still run
when completeJob rejects.
2026-07-21 08:16:31 -04:00
Danny Avila
5eb1c2c107
🖇️ feat: Reference Selected Chat Text with Multi-Quote Popup (#13868)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* 🖇️ feat: Reference Selected Chat Text with Multi-Quote Popup

Add a ChatGPT/Codex-style quote feature: selecting text in any message shows
an 'Add to chat' popup that accumulates removable quote chips above the
composer. On submit, the excerpts are merged into the user message text as
Markdown blockquotes (counted in the user message token count, not a system
message) and persisted on the message so they render on the user bubble and
survive reload.

- packages/api: add getReferencedQuotes + mergeQuotedText helpers (blockquote merge, length/count caps) with unit tests
- BaseClient.sendMessage: temporarily merge req.body.quotes into userMessage.text before buildMessages, restore clean text, persist quotes array
- data-schemas + data-provider: add optional quotes field to message schema/type
- client: pendingQuotesByConvoId atom, QuoteButton selection popup, PendingQuoteChips composer row, MessageQuotes persistent display
- useChatFunctions: drain pending quotes onto the message, carry forward on regenerate
- add localization keys and component/integration tests

* 🧪 test: Add Playwright e2e for chat quote feature

Add e2e/specs/mock/quotes.spec.ts covering select -> 'Add to chat' popup ->
chip -> send -> persistent reference block -> reload, plus multi-select
accumulation and chip removal. Selection is driven programmatically (real DOM
Range + dispatched mouseup) to summon the popup deterministically.

Add data-testid hooks (add-to-chat-button, pending-quote-chips, message-quotes)
to the quote components for stable selectors.

* 🛡️ fix: Address Codex review on quote feature

- Run PII filter + OpenAI moderation over req.body.quotes (P1): quoted excerpts
  are merged into the model-facing user message, so they must clear the same
  filters; a crafted quotes payload could otherwise bypass them. Adds tests.
- Carry quotes through edit/save-and-submit replays (overrideQuotes in
  EditMessage), mirroring overrideManualSkills, so edited turns keep context.
- Hide the quote UI for Assistants endpoints (which bypass BaseClient merge),
  so users can't queue quotes the assistant never receives.
- Clear pending quote/skill queues by resolved conversationId in useClearStates,
  not the UI index, so queued-but-unsent selections don't linger in Recoil.
- Cap queued quotes client-side at 10 to match the backend QUOTE_MAX_COUNT, so
  the composer never shows more quotes than are actually sent.

* 🧵 fix: Durably re-merge quotes + Codex round 2

Address Codex's re-review of the quote feature:

- Durable history re-merge (per maintainer decision): quotes are no longer
  merged at request time and stripped; instead each user message's persisted
  message.quotes is merged into its formatted content in AgentClient.buildMessages
  (new prependQuotes helper) for current AND historical turns. The model
  receives the referenced context on every prompt and the token count stays
  consistent with what was persisted; stored text stays clean for display.
- Attach normalized quotes to the user message in handleStartMethods (before
  getReqData/onStart) so the optimistic bubble, resumable abort metadata, and
  saved row all carry them (fixes the abort-metadata gap).
- Skip the quote drain entirely for Assistants endpoints in useChatFunctions,
  leaving the pending atom intact (UI is already hidden there).
- Normalize req.body.quotes via getReferencedQuotes before moderation/PII so
  only the trimmed/truncated/capped excerpts the model will receive are checked.
- Tests: prependQuotes unit tests; BaseClient quote tests assert early
  attachment + clean text; e2e now verifies the model receives the merged
  blockquote on the current turn and re-merged from history on a later turn
  (new E2E_ASSERT_QUOTE mock marker).

* 🔗 fix: Quote share/memo/abort/PII gaps (Codex round 3)

- Shared links: include quotes in the anonymized projection + SharedMessage
  type (+test) so the /share view renders the same reference blocks as the
  owner, mirroring manualSkills/alwaysAppliedSkills.
- MessageRender memo: compare quotes length so a server/resume copy whose only
  change is the quote list re-renders (the block no longer goes stale/missing).
- Resumable job metadata: include quotes in the userMessage written to
  GenerationJobManager so a reload/reconnect mid-stream reconstructs the chips.
- PII + moderation: also scan the merged blockquote+text exactly as the model
  receives it, so a secret split across a quote and the typed body (each clean
  alone) is caught (+cross-boundary test).
- e2e: make quote-add robust against the auto-scroll-dismisses-selection race
  via a retried select+click helper.

* 🛑 fix: Keep quotes on aborted turn's request message (Codex round 4)

abortMiddleware reconstructs finalEvent.requestMessage from jobData.userMessage
but only copied ids + text; include quotes so a stopped quoted turn keeps its
MessageQuotes in the UI and a regenerate-before-reload still sends the
referenced context. Completes the resumable-metadata fix from the prior round.

* 🧮 fix: Quote recount + preliminary abort metadata (Codex round 5)

- Force a canonical token recount for messages carrying quotes in
  AgentClient.buildMessages, so a plain text-only Save edit (which recomputes
  tokenCount from text alone) can't leave a stale, quote-excluding count that
  undercounts context on later turns — recount from the quote-merged copy
  self-heals it.
- Seed normalized quotes into the preliminary userMessage metadata
  (getPreliminaryUserMessage), so an abort during init/tool-loading (before
  onStart) still reconstructs the stopped turn's MessageQuotes.

*  fix: Add getReferencedQuotes to controller test mocks (CI)

request.js's getPreliminaryUserMessage now calls getReferencedQuotes; the
agents controller specs mock @librechat/api wholesale, so the mock must export
it or the call throws and cascades. Added a faithful mock (normalize/cap,
null when empty) to request.resumeMetadata.spec.js and jobReplacement.spec.js.

* 📐 fix: Quotes in context projection + resumable metadata (Codex round 6)

- Context-usage projection (resolveContextProjection): select message.quotes,
  prepend them into the projected user text, and recount quoted messages so the
  context gauge counts the same prompt the model receives (a text-only Save edit
  no longer makes the gauge undercount / over-report remaining budget).
- Resumable job metadata: trackUserMessage (created-event rewrite) and abortJob
  (final requestMessage) now carry quotes; SerializableJobData.userMessage and
  CreatedEvent.message gained an optional quotes field. With the cross-replica
  created-event spread, stopping/reconnecting a quoted turn after the created
  event keeps its MessageQuotes.

* 💬 feat: Collapse multi-select quotes into one chip with hover popup

Composer feedback: the quote chip area now shows a single chip — the excerpt
text for one selection, or a collapsed "{n} selections" pill for multiple,
with a hover popup (HoverCard) listing every excerpt and a per-item remove. The
chip is taller (py-1.5/text-sm) to read less skinny. Adds com_ui_quote_selections
and com_ui_remove_all_quotes; updates unit + e2e tests (e2e drives the count via
a data-quote-count hook and exercises the hover popup).

*  fix: Make multi-selection quote popup keyboard accessible

The collapsed "{n} selections" pill used a HoverCard, which Radix only opens on
pointer hover — its interactive content was unreachable by keyboard. Replaced it
with a Popover: the trigger is a real button that opens on click / Enter / Space
(focus moves into the list, each excerpt's × is tab-navigable, Escape closes and
restores focus), with hover-open preserved for mouse via controlled open state +
a close grace period. Hover-initiated opens skip auto-focus so they don't pull
focus off the composer. Adds an e2e asserting keyboard open/close.

* 📐 fix: Clamp the Add-to-chat button within the viewport (Codex round 7)

The floating selection button positioned via translate(-50%,-100%) (bottom-center
anchor) but clamped top/left as if they were its top-left, so a selection near
the viewport top or sides could render the button partly/fully offscreen. Now it
measures the button (ref + useLayoutEffect) and computes an on-screen top-left —
clamping by the full width within side margins and flipping below the selection
when there's no room above — with no transform, and stays hidden until measured
so it never flashes at an unclamped spot.

* ↩️ fix: Restore pending quotes on early-abort draft (Codex round 8)

When a turn is stopped before the created event (e.g. during tool/MCP init), the
final handler restores requestMessage.text to the draft, but the pending-quote
atom was already drained on submit — so a retry sent no quotes. The abort
requestMessage now carries quotes (preliminary metadata + abort fixes), so the
three early-abort/no-response draft-restore paths in useEventHandlers now also
re-queue pendingQuotesByConvoId from requestMessage.quotes.

*  fix: Use Ariakit Popover for quote selections (keyboard focus)

The multi-selection popup used a hand-rolled Radix Popover with Popover.Anchor +
a manual button, so Radix had no trigger to return focus to — Escape dumped
focus to the page top. Refactored to Ariakit (the codebase's popover primitive,
per DropdownPopup/Fork): the `PopoverDisclosure` is the real trigger, so Escape
closes and returns focus to the composer instead of the top of the page. Keyboard
opens (Enter/Space) autofocus into the list and tab through each excerpt's remove;
hover opens for mouse with autofocus suppressed so it never pulls focus off the
composer. e2e asserts the keyboard open/navigate/Escape flow keeps focus on a
real control (never BODY).
2026-06-21 08:33:11 -04:00
Danny Avila
055585f9f1
🪢 fix: Tie MCP Cleanup To Resumable Runs (#13769)
Some checks failed
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
Publish `@librechat/client` to NPM / pack (push) Has been cancelled
Publish `librechat-data-provider` to NPM / pack (push) Has been cancelled
Publish `@librechat/data-schemas` to NPM / pack (push) Has been cancelled
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Has been cancelled
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Has been cancelled
GitNexus Index / index (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Has been cancelled
Sync Helm Chart Tags / Ignore non-main push (push) Has been cancelled
Sync Helm Chart Tags / Sync chart tags (push) Has been cancelled
Publish `@librechat/client` to NPM / publish-npm (push) Has been cancelled
Publish `librechat-data-provider` to NPM / publish-npm (push) Has been cancelled
Publish `@librechat/data-schemas` to NPM / publish-npm (push) Has been cancelled
GitNexus Index / post-index (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Has been cancelled
* fix: Clean up request-scoped MCP connections

* test: Format MCP request context spec

* refactor: Move MCP request context to API package
2026-06-15 15:26:03 -04:00
Danny Avila
fd4728232c
🧵 fix: Reject Preliminary Parent Follow-Ups (#13619)
* fix: Reject preliminary parent follow-ups

* chore: Sort frontend imports

* fix: Narrow preliminary parent detection

* fix: Preserve refused submit state

* fix: Propagate refused submit result
2026-06-09 12:06:51 -04:00
Danny Avila
2a956f143d
🪞 fix: Preserve Model Spec Icons Across Stream Resume and Abort (#13603)
Some checks are pending
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Sync Helm Chart Tags / Ignore non-main push (push) Waiting to run
Sync Helm Chart Tags / Sync chart tags (push) Waiting to run
2026-06-08 17:14:21 -04:00
Danny Avila
cb1d536874
📻 fix: Replay MCP OAuth Prompts for Coalesced Connections (#13565)
* fix: Replay MCP OAuth URL for Joined Connections

* chore: Sort MCP OAuth Imports

* test: Restore MCP OAuth Registry Spies

* fix: Replay pending MCP OAuth prompts

* fix: Replay MCP OAuth on Stream Resume

* fix: Preserve MCP OAuth Replay Context

* chore: Format MCP OAuth Replay Context

* test: Expect MCP OAuth Replay Expiry

* fix: Render pending MCP OAuth prompts

* chore: Clean MCP OAuth Replay Type Narrowing

* fix: Stabilize new MCP OAuth chats

* fix: Re-emit cached MCP OAuth prompts

* fix: Replay pending OAuth for selected MCP tools

* fix: Avoid stalling pending MCP OAuth replay

* test: Clean MCP OAuth review findings

* test: Restore MCP OAuth registry spy

* fix: Resolve OAuth Typecheck Regressions

* fix: Harden MCP OAuth replay edge cases

* test: Cover MCP OAuth joined prompt expiry

* test: Mark joined OAuth replay fixture

* test: Use OAuth fixture for joined replay expiry

* fix: Anchor resumed MCP OAuth prompts

* fix: Seed resumable turn metadata before MCP init

* test: Format resume metadata regression

* fix: Prioritize resumable stream routes

* fix: Preserve MCP OAuth resume message tree

* test: Fix MCP OAuth Resume Test Types

* fix: Replay MCP OAuth Regenerate Prompts

* fix: Skip OAuth-only Abort Persistence

* fix: Stabilize OAuth Resume Replay

* fix: Target Non-Tail Regenerate Responses

* fix: Scope Regenerate Step Updates

* fix: Clean Up OAuth Abort State

* fix: Preserve Regenerate Branch Siblings

* fix: Preserve OAuth Resume Branch State

* fix: Preserve OAuth Branch Resume State

* chore: Sort OAuth Resume Imports

* fix: Address OAuth Resume Review Findings

* test: Fix Abort Fixture Typing
2026-06-07 10:45:54 -04:00