mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
10 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
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
|
||
|
|
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 |
||
|
|
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'. |
||
|
|
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 |
||
|
|
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. |
||
|
|
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). |
||
|
|
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 |
||
|
|
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 |
||
|
|
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
|
||
|
|
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 |